Bug 27345 - C4::Auth::get_template_and_user is missing some permissions for superlibrarian
Summary: C4::Auth::get_template_and_user is missing some permissions for superlibrarian
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-05 14:09 UTC by Jonathan Druart
Modified: 2021-12-13 21:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.02,20.05.08


Attachments
Bug 27345: Add missing global permissions for superlibrarian (2.19 KB, patch)
2021-01-05 14:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27345: Add missing global permissions for superlibrarian (2.25 KB, patch)
2021-01-07 00:17 UTC, David Cook
Details | Diff | Splinter Review
Bug 27345: Add missing global permissions for superlibrarian (2.30 KB, patch)
2021-01-08 13:40 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-01-05 14:09:04 UTC
When a superlibrarian is logged in, C4::Auth::get_template_and_user pass the CAN_user_$flag to the template, but some are missing:
suggestions, lists, cash_management

So far they are not used in the template but it will avoid a developer to spend time on it if we fix it now.
Comment 1 Jonathan Druart 2021-01-05 14:10:05 UTC
Created attachment 114873 [details] [review]
Bug 27345: Add missing global permissions for superlibrarian

When a superlibrarian is logged in, C4::Auth::get_template_and_user pass the CAN_user_$flag to the template, but some are missing:
suggestions, lists, cash_management

So far they are not used in the template but it will avoid a developer to spend time on it if we fix it now.

Test plan:
Compare with installer/data/mysql/mandatory/userflags.sql
Note that we don't need selfcheck
Comment 2 David Cook 2021-01-05 22:46:20 UTC
This could actually be a good opportunity to do some refactoring...

If you go to https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=26791&attachment=113660 and click "Auth.pm", you'll see a function called "get_authz_from_flags".

That could be further optimized by fetching the permissions from userflags (as you note) to dynamically make sure that the superlibrarian has all the right permissions.

By the way, if we refactor it along the lines of what I've done on Bug 26791, we could add unit tests too. 

Originally I wanted to refactor C4::Auth::get_template_and_user, but I figured there was no hope doing it alone. But if you're interested in proving it, I'm happy to work with you.
Comment 3 Katrin Fischer 2021-01-06 00:00:46 UTC
I think maybe the refactor could go onto a separate bug? We will want to keep this small and easy for easy backport with little risk.
Comment 4 David Cook 2021-01-06 04:55:09 UTC
(In reply to Katrin Fischer from comment #3)
> I think maybe the refactor could go onto a separate bug? We will want to
> keep this small and easy for easy backport with little risk.

I figured we'd have to test to the same degree in both cases so thought refactoring would be more efficient here but no worries if not.
Comment 5 Jonathan Druart 2021-01-06 09:27:48 UTC
I think we should keep this patch as simple as it is.
Comment 6 David Cook 2021-01-07 00:16:08 UTC
(In reply to Jonathan Druart from comment #5)
> I think we should keep this patch as simple as it is.

Sure thing
Comment 7 David Cook 2021-01-07 00:16:55 UTC
My test plan:

1. Go to http://localhost:8081/cgi-bin/koha/mainpage.pl
2. Note that you can see all the modules and options that you can usually see
Comment 8 David Cook 2021-01-07 00:17:30 UTC
Created attachment 114901 [details] [review]
Bug 27345: Add missing global permissions for superlibrarian

When a superlibrarian is logged in, C4::Auth::get_template_and_user pass the CAN_user_$flag to the template, but some are missing:
suggestions, lists, cash_management

So far they are not used in the template but it will avoid a developer to spend time on it if we fix it now.

Test plan:
Compare with installer/data/mysql/mandatory/userflags.sql
Note that we don't need selfcheck

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 9 Nick Clemens 2021-01-08 13:40:28 UTC
Created attachment 114962 [details] [review]
Bug 27345: Add missing global permissions for superlibrarian

When a superlibrarian is logged in, C4::Auth::get_template_and_user pass the CAN_user_$flag to the template, but some are missing:
suggestions, lists, cash_management

So far they are not used in the template but it will avoid a developer to spend time on it if we fix it now.

Test plan:
Compare with installer/data/mysql/mandatory/userflags.sql
Note that we don't need selfcheck

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Jonathan Druart 2021-01-08 14:23:28 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 11 Fridolin Somers 2021-01-11 12:51:58 UTC
Pushed to 20.11.x for 20.11.02
Comment 12 Andrew Fuerste-Henry 2021-01-19 21:47:41 UTC
Pushed to 20.05.x for 20.05.08
Comment 13 Victor Grousset/tuxayo 2021-01-22 01:38:33 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.