Bug 33595 - Bug 26628 broke authorization for tools start page
Summary: Bug 26628 broke authorization for tools start page
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 26628
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-24 01:49 UTC by David Cook
Modified: 2024-08-15 13:29 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07
Circulation function:


Attachments
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl (1.34 KB, patch)
2023-05-11 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl (1.39 KB, patch)
2023-05-12 00:17 UTC, David Cook
Details | Diff | Splinter Review
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl (1.49 KB, patch)
2023-05-12 07:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2023-04-24 01:49:52 UTC
When using koha-testing-docker or any other Koha that uses the same domain name (or IP address) for OPAC and staff interface, there is a hole in the staff interface authorization.

If you log into the OPAC as a user with no flags, you can see a restricted view of /cgi-bin/koha/tools/tools-home.pl instead of seeing the login screen.

It's a very limited view, but it's still a view of the staff interface when the user should not be able to see anything. 

--

tools-home.php uses an arrayref instead of a hashref for "flagsrequired" so I'd say there is a subtle bug there.

Looks like Bug 26628 probably caused this regression.
Comment 1 David Cook 2023-04-24 01:53:21 UTC
According to Auth.pm docs, an arrayref needs to be used like this:

$flagsrequired = [ 'a_flag', 'b_flag' ];
$flagsrequired = { 'a_flag' => [ 'sub_a, 'sub_b' ] };

But we're using it like this:

flagsrequired   => [ tools => '*', clubs => '*' ]

--

Looking at C4::Auth::_dispatch now...
Comment 2 David Cook 2023-04-24 01:56:28 UTC
Yeah it looks like the following is syntactically invalid and C4::Auth::_dispatch just silently fails to perform authorization on this page: flagsrequired   => [ tools => '*', clubs => '*' ]
Comment 3 David Cook 2023-04-24 02:07:04 UTC
(In reply to David Cook from comment #2)
> Yeah it looks like the following is syntactically invalid and
> C4::Auth::_dispatch just silently fails to perform authorization on this
> page: flagsrequired   => [ tools => '*', clubs => '*' ]

Ah right because [ tools => '*', clubs => '*' ] is the same as saying ['tools','*','clubs','*']
Comment 4 David Cook 2023-04-24 02:18:06 UTC
Yeah, I don't think there's any way that the change from bug 26628 is going to work without refactoring C4::Auth::_dispatch

I see that t/db_dependent/Auth/haspermission.t has a lot of use of arrayrefs, but I don't think they're doing what we think they're doing.

C4::Auth::_dispatch defaults to authorized when it should actually default to not authorized... so if you feed it something it can't handle it just defaults to authorized.

I think the shortest path here is just to revert Bug 26628 and then improve Koha's authorizations.
Comment 5 David Cook 2023-04-24 02:24:21 UTC
(In reply to David Cook from comment #4)
> I think the shortest path here is just to revert Bug 26628 and then improve
> Koha's authorizations.

It actually doesn't need to be that complicated. In Koha::Auth::Permissions, I create an authorization hashref with the "CAN_user_" prefix. We could easily do a different version like this:

$authz->{clubs} = 1
or
$authz->{clubs}->{'enroll'} = 1

If the flagsrequired is any, then it just needs to test for the existence of the module permission (ie 'clubs'). If it needs to find a specific permission, it checks for the module permission, and then it checks for the subpermission as a key of that permission (or if it's a 1 instead of a ref then you return as authorized).

For backwards compatibility, we just need to make sure we honour "flagsrequired".
Comment 6 Jonathan Druart 2023-05-11 10:08:58 UTC
Created attachment 151065 [details] [review]
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl

If you log into the OPAC as a user with no flags, you can see a restricted view of
/cgi-bin/koha/tools/tools-home.pl instead of seeing the login screen.

Test plan:
Use a patron with catalogue permission only
Login and access the tools home page
=> redirected to the login screen

Add a club sub permission
Login and access the tools home page
=> You see the tools home page with the clubs link

Add a tool sub permission, remove club
Login and access the tools home page
=> You see the tools home page with the relevant link
Comment 7 David Cook 2023-05-12 00:17:25 UTC
Created attachment 151108 [details] [review]
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl

If you log into the OPAC as a user with no flags, you can see a restricted view of
/cgi-bin/koha/tools/tools-home.pl instead of seeing the login screen.

Test plan:
Use a patron with catalogue permission only
Login and access the tools home page
=> redirected to the login screen

Add a club sub permission
Login and access the tools home page
=> You see the tools home page with the clubs link

Add a tool sub permission, remove club
Login and access the tools home page
=> You see the tools home page with the relevant link

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 8 David Cook 2023-05-12 00:19:51 UTC
Thanks, Jonathan. That's much better.

--

Although it makes me realize that "catalogue" isn't actually required for staff access... but I'll open a different report for that...
Comment 9 Marcel de Rooy 2023-05-12 07:10:06 UTC
Created attachment 151114 [details] [review]
Bug 33595: (bug 26628 follow-up) Fix authorization for tools-home.pl

If you log into the OPAC as a user with no flags, you can see a restricted view of
/cgi-bin/koha/tools/tools-home.pl instead of seeing the login screen.

Test plan:
Use a patron with catalogue permission only
Login and access the tools home page
=> redirected to the login screen

Add a club sub permission
Login and access the tools home page
=> You see the tools home page with the clubs link

Add a tool sub permission, remove club
Login and access the tools home page
=> You see the tools home page with the relevant link

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2023-05-16 09:52:07 UTC
Pushed to master for 23.05
Comment 11 Pedro Amorim 2023-06-07 10:22:30 UTC
Nice work everyone!

Pushed to 22.11.x for next release