Bug 39055 - Unauthenticated are not redirected properly in reports module after login
Summary: Unauthenticated are not redirected properly in reports module after login
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: David Cook
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-02-06 15:56 UTC by Lucas Gass (lukeg)
Modified: 2026-02-03 05:53 UTC (History)
9 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 39055: wip (2.22 KB, patch)
2026-01-16 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through (12.65 KB, patch)
2026-01-19 00:00 UTC, David Cook
Details | Diff | Splinter Review
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through (12.68 KB, patch)
2026-01-28 16:44 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 39055: Fix mojo/acquisition/vendors.t (999 bytes, patch)
2026-02-02 11:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through (12.75 KB, patch)
2026-02-03 00:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 39055: Fix mojo/acquisition/vendors.t (1.03 KB, patch)
2026-02-03 00:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 39055: Add more login_op checks to middleware (2.34 KB, patch)
2026-02-03 00:32 UTC, David Cook
Details | Diff | Splinter Review
Bug 39055: Add change to Koha::App::Plugin::CSRF and fix unit test (6.86 KB, patch)
2026-02-03 00:32 UTC, David Cook
Details | Diff | Splinter Review
Bug 39055: Fix login condition when using op and login_op (1.76 KB, patch)
2026-02-03 05:53 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 Lucas Gass (lukeg) 2025-02-06 15:56:36 UTC
This seems to be a recent change in behavior. 

To reproduce: 

1. Find/create a report, run it and copy the URL. Something like "http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run"
2. Logout, or use a private browsing tab, and use the URL from above. 
3. You'll be promoted to login fist, do that.
4. Once you are logged in you'll be taken to the reports home page instead of your specific report.
Comment 1 Steve, OSLRI, USA 2025-02-06 16:09:20 UTC
I'll just add that I do see the report id in the login form in a hidden field, so it does carry to the login form, but gets lost after login.
Comment 2 Brendan Lawlor 2025-02-20 19:16:48 UTC
I found that when using a local login, after authentication the op param is updated to cud-login. After authentication the id param is not in the url, but it is included in the payload. The report doesn't load because the op param is not longer 'run'

Interestingly after an SSO authentication the page loads with the complete original url and the report loads
Comment 3 Steve, OSLRI, USA 2025-02-20 20:27:57 UTC
Confirming that we see that it works via SSO here as well.  Seems to be limited to local login.
Comment 4 Jonathan Druart 2026-01-16 14:31:11 UTC
Owen reported the same problem on suggestion/suggestion.pl

I am stuck here, what I am trying to do won't work as we don't sent back the modified CGI object to the controller.

Any ideas, David?
Comment 5 Jonathan Druart 2026-01-16 14:31:50 UTC
Created attachment 191568 [details] [review]
Bug 39055: wip

Patch from commit 1c381a0
Comment 6 David Cook 2026-01-18 23:36:44 UTC
(In reply to Jonathan Druart from comment #4)
> Owen reported the same problem on suggestion/suggestion.pl
> 
> I am stuck here, what I am trying to do won't work as we don't sent back the
> modified CGI object to the controller.
> 
> Any ideas, David?

Yeah, that's an interesting one. 

A lot of other apps have /login* endpoints which may or may not redirect back to a sanitized referrer. That would be easy to handle. But Koha has always allowed login on any page, which has its pros and cons...

The simplest option would probably be change the login not to use the "op" parameter. We could change it to "login_op". We would need to update Koha/Middleware/CSRF.pm to handle this special case because a login POST could contain an "op" without a "cud-" prefix.

Alternatively, we could try to hack the inputs to stuff the query string params back in via a Plack::Middleware or through C4/Auth.pm but I don't like that idea as it's dirty/hacky.
Comment 7 David Cook 2026-01-19 00:00:50 UTC
Created attachment 191617 [details] [review]
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through

This patch changes "op" to "login_op" when logging in, so that the original op can
flow through to whichever controller is processing the login and subsequent op.

Test plan:
0. apply the patch and koha-plack --restart kohadev
1. Without logging in, go to the following:
http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run
2. Log in and note that you can see the report
3. Try logging into the OPAC
4. Try logging into the self-checkout
5. Create a new self-reg account and try logging in with the auto-filled details
6. Test anything else you can think to test here
Comment 8 David Cook 2026-01-19 00:06:04 UTC
I've done a patch for my "login_op" idea and it seems to work pretty well. 

It's the only practical idea I've got at the moment I think.
Comment 9 Jonathan Druart 2026-01-19 08:51:35 UTC
Yes I agree with the solution, ready for signoff.
Comment 10 Brendan Lawlor 2026-01-28 16:44:37 UTC
Created attachment 192121 [details] [review]
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through

This patch changes "op" to "login_op" when logging in, so that the original op can
flow through to whichever controller is processing the login and subsequent op.

Test plan:
0. apply the patch and koha-plack --restart kohadev
1. Without logging in, go to the following:
http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run
2. Log in and note that you can see the report
3. Try logging into the OPAC
4. Try logging into the self-checkout
5. Create a new self-reg account and try logging in with the auto-filled details
6. Test anything else you can think to test here

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 11 Jonathan Druart 2026-02-02 11:42:54 UTC
Created attachment 192308 [details] [review]
Bug 39055: Fix mojo/acquisition/vendors.t

Patch from commit 2c5d7b3
Comment 12 Jonathan Druart 2026-02-02 11:44:08 UTC
t/db_dependent/mojo/csrf.t is failing

Shouldn't we add a case in the middleware to force login_op to be POSTed?
Comment 13 David Cook 2026-02-02 23:59:44 UTC
(In reply to Jonathan Druart from comment #12)
> Shouldn't we add a case in the middleware to force login_op to be POSTed?

Yeah, that's probably true. I think there are fail safes in C4/Auth.pm but yeah good point.
Comment 14 David Cook 2026-02-03 00:31:53 UTC
Created attachment 192350 [details] [review]
Bug 39055: Change "op" to "login_op" when logging in to allow original op to flow through

This patch changes "op" to "login_op" when logging in, so that the original op can
flow through to whichever controller is processing the login and subsequent op.

Test plan:
0. apply the patch and koha-plack --restart kohadev
1. Without logging in, go to the following:
http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run
2. Log in and note that you can see the report
3. Try logging into the OPAC
4. Try logging into the self-checkout
5. Create a new self-reg account and try logging in with the auto-filled details
6. Test anything else you can think to test here

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 15 David Cook 2026-02-03 00:31:56 UTC
Created attachment 192351 [details] [review]
Bug 39055: Fix mojo/acquisition/vendors.t

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 16 David Cook 2026-02-03 00:32:00 UTC
Created attachment 192352 [details] [review]
Bug 39055: Add more login_op checks to middleware

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 17 David Cook 2026-02-03 00:32:03 UTC
Created attachment 192353 [details] [review]
Bug 39055: Add change to Koha::App::Plugin::CSRF and fix unit test

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 18 David Cook 2026-02-03 01:14:04 UTC
I forgot to test it properly and it broke... and I'm now in the middle of other things. I'll come back to this...
Comment 19 David Cook 2026-02-03 05:53:39 UTC
Created attachment 192354 [details] [review]
Bug 39055: Fix login condition when using op and login_op