Bug 41045 - Suggestions manage permissions added to patrons who previously had no permissions in that category
Summary: Suggestions manage permissions added to patrons who previously had no permiss...
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 33363
Blocks:
  Show dependency treegraph
 
Reported: 2025-10-17 12:20 UTC by Emmi Takkinen
Modified: 2025-10-24 05:58 UTC (History)
16 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 41045: Fix BZ 33363 database update (1.74 KB, patch)
2025-10-21 03:40 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emmi Takkinen 2025-10-17 12:20:51 UTC
Bug 33363 separated suggestions_manage permission in three permissions for creating, updating, and deleting suggetions. In bugs test plan and in its atomicupdate file it is stated that these permissions should have been added only for those users who previously had suggestions_manage permissions. However there is a typo on SQL insert statement and new permissions have been added to all users with full catalogue permissions:

>INSERT IGNORE INTO user_permissions (borrowernumber, module_bit, code) SELECT borrowernumber, 12, 'suggestions_create' FROM borrowers WHERE flags & (1 << 2)
Comment 1 David Cook 2025-10-20 05:57:58 UTC
I'm just about to finish for the day, so I get you to just rephrase this one? What's "full catalogue permissions"?
Comment 2 Katrin Fischer 2025-10-20 06:24:44 UTC
(In reply to David Cook from comment #1)
> I'm just about to finish for the day, so I get you to just rephrase this
> one? What's "full catalogue permissions"?

Usually referring to the module level checkbox instead of sub-permissions.
Comment 3 David Cook 2025-10-20 22:41:17 UTC
Now that it's the start of the day, I understand much better.

Looking at userflags, bit 2 is "catalogue" (hence "full catalogue permissions"). It looks like that WHERE should've been "WHERE flags & (1 << 12)", since bit 12 is "suggestions".
Comment 4 David Cook 2025-10-20 22:45:59 UTC
This is an interesting problem. 

We should fix this atomic update in "main", 24.11, 25.05, and 25.11 so that newly upgraded sites don't suffer this same problem. 

But for sites that are already upgraded... I don't know that we can do any automated remediation, because we can't know which users we updated automatically and which were given these permissions after the fact. We might just have to add a warning in the release notes telling people to review the permissions of all their users because of this mistake that was made.

--

Locally, I created a permission audit tool many years ago within Koha. It doesn't have the best UI but it does provide an overview of all permissions for all patrons. Something like that would be useful in this situation... otherwise perhaps an SQL report we could provide in the release notes so that people can double-check their users.
Comment 5 David Cook 2025-10-20 23:01:19 UTC
(In reply to David Cook from comment #4)
> But for sites that are already upgraded... I don't know that we can do any
> automated remediation, because we can't know which users we updated
> automatically and which were given these permissions after the fact.

That being said... if a user has "suggestions_create" and "suggestions_delete" and not "suggestions_manage", that's pretty suspicious.
Comment 6 David Cook 2025-10-21 03:40:06 UTC
Created attachment 188182 [details] [review]
Bug 41045: Fix BZ 33363 database update

This patch fixes the BZ 33363 database update, so that it
selects uses with the "suggestions" permission rather than the "catalogue"
permission.
Comment 7 David Cook 2025-10-21 03:41:16 UTC
I've attached the patch that fixes the problem for future upgrades, but I'll see what people think regarding the need for any additional patches for trying to address past problematic upgrades.
Comment 8 David Cook 2025-10-21 03:43:12 UTC
Note I'm going to immediately backport this patch locally myself... and I'm planning on writing some queries to find any users with that have only "suggestions_create" and "suggestions_delete" but not "suggestions_manage"...
Comment 9 David Cook 2025-10-24 05:58:49 UTC
(In reply to David Cook from comment #8)
> Note I'm going to immediately backport this patch locally myself... and I'm
> planning on writing some queries to find any users with that have only
> "suggestions_create" and "suggestions_delete" but not "suggestions_manage"...

There's probably a better way to do this, but this is the query I used in the end:

select borrowernumber,group_concat(code) as perms 
from user_permissions where module_bit = 12 
group by borrowernumber 
having perms like '%suggestions_create%' and 
 perms like '%suggestions_delete%' and
 perms not like '%suggestions_manage%' ;

It's possible that some users legitimately had that set of permissions, but it seemed unlikely to me, so I wrote a Perl script to blitz them.