[Bug 38363] New: get_template_and_user and checkauth don't use C4::Output for rendering auth pages
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Bug ID: 38363 Summary: get_template_and_user and checkauth don't use C4::Output for rendering auth pages Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org I was testing the output of HTTP headers using C4::Output::output_html_with_http_headers, and I was going crazy wondering why my headers weren't coming through on http://localhost:8081 And the reason is that authentication pages don't use this function! C4::Auth::checkauth and C4::Auth::get_template_and_user do their our outputs which leads to duplication. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38365 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #1 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 188465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188465&action=edit Bug 38363: Use output_html_with_http_headers in C4::Auth The following C4::Auth subroutines - checkauth() - get_template_and_user() were in some cases outputting html outside of C4::Output's output mechanisms. It is important to ensure the consistency of our HTTP responses. Having multiple output mechanisms for what is essentialy the same type of response not only leads to unneccessary code duplication and complexity but possibly even bugs at some point in the future. For these reasons it is better to output only through C4::Output, when possible. This patch does that for the two aforementioned C4::Auth cases. To test unit tests: 1. prove t/db_dependent/Auth.t As for checkauth(), the change only applies to failed logins. To test checkauth: 1. Log in to OPAC with invalid credentials 2. Observe "You entered an incorrect username or password" 3. Log in to staff client with invalid credentials 4. Observe "Invalid username or password" As for get_template_and_user(), the change only applies to SCO/SCI user navigating outside of the self service modules. To test get_template_and_user: 1. Enable system preference WebBasedSelfCheck 2. Create a new test patron and note down their username & password 3. Grant them a permission self_checkout_module (under Self check modules) 4. Navigate to http://OPAC/cgi-bin/koha/sco/sco-main.pl 5. Log in with the test patron 6. Observe successful login (should display "Self-checkout system" page) 7. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 8. Observe "Log in to your account" page, ie. you have been kicked out 9. Modify your test patron's permissions 10. Remove permission "self_checkout_module" and add "self_checkin_module" 11. Enable system preference SelfCheckInModule 12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl 13. Log in using your test patron's credentials 14. Observe "Self check-in" page 15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 16. Observe "Log in to your account" page, ie. you have been kicked out -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lari.taskula@hypernova.fi --- Comment #2 from Lari Taskula <lari.taskula@hypernova.fi> --- After this patch we now set some response headers that were previously unset in these two C4::Auth cases (see commit message). - Status: 200 OK (since we are dealing with failed logins and permission issues it should really be either 401 or 403. But since we were not explicitly setting a 4xx response code before either, I see this being outside the scope of this Bug) - Charset from lowercase utf-8 -> uppercase UTF-8 (probably does not matter) - Cache-control and Pragma: no-cache - Content-Style-Type: text/css - Content-Script-Type: text/javascript - Access-Control-Allow-Origin: value of preference 'AccessControlAllowOrigin' -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |41104 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41104 [Bug 41104] Samesite HTTP response header being set in C4::Auth::checkauth() -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #3 from Lari Taskula <lari.taskula@hypernova.fi> --- Looking at git history, I noticed that we have attempted to use output_html_with_http_headers here before (Bug 10074), but the change was reverted. https://github.com/Koha-Community/Koha/commit/3536b10acf223b3381915353284149... -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #188465|0 |1 is obsolete| | --- Comment #4 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 188475 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188475&action=edit Bug 38363: Use output_html_with_http_headers in C4::Auth The following C4::Auth subroutines - checkauth() - get_template_and_user() were in some cases outputting html outside of C4::Output's output mechanisms. It is important to ensure the consistency of our HTTP responses. Having multiple output mechanisms for what is essentialy the same type of response not only leads to unneccessary code duplication and complexity but possibly even bugs at some point in the future. For these reasons it is better to output only through C4::Output, when possible. This patch does that for the two aforementioned C4::Auth cases. To test unit tests: 1. prove t/db_dependent/Auth.t As for checkauth(), the change only applies to unauthenticated state and failed logins. To test checkauth: 1. Enable system preference opacuserlogin 2. Navigate to OPAC 3. Observe OPAC loading successfully 4. Log in to OPAC with invalid credentials 5. Observe "You entered an incorrect username or password" 6. Navigate to staff client without being logged in (logout if you are in) 7. Observe staff client login screen loading successfully 8. Log in to staff client with invalid credentials 9. Observe "Invalid username or password" As for get_template_and_user(), the change only applies to SCO/SCI user navigating outside of the self service modules. To test get_template_and_user: 1. Enable system preference WebBasedSelfCheck 2. Create a new test patron and note down their username & password 3. Grant them a permission self_checkout_module (under Self check modules) 4. Navigate to http://OPAC/cgi-bin/koha/sco/sco-main.pl 5. Log in with the test patron 6. Observe successful login (should display "Self-checkout system" page) 7. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 8. Observe "Log in to your account" page, ie. you have been kicked out 9. Modify your test patron's permissions 10. Remove permission "self_checkout_module" and add "self_checkin_module" 11. Enable system preference SelfCheckInModule 12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl 13. Log in using your test patron's credentials 14. Observe "Self check-in" page 15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 16. Observe "Log in to your account" page, ie. you have been kicked out -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #5 from Lari Taskula <lari.taskula@hypernova.fi> --- (In reply to Lari Taskula from comment #3)
Looking at git history, I noticed that we have attempted to use output_html_with_http_headers here before (Bug 10074), but the change was reverted.
https://github.com/Koha-Community/Koha/commit/ 3536b10acf223b33819153532841494c816d6375 There's plenty of depth into this topic (Bug 6554 and encoding issues). It's a bit too much for me at this moment.
It seems Bug 10074 was reverted as a part of larger set of reverts (Bug 6554 comment 103 , https://github.com/Koha-Community/Koha/commits/16753c750d97385ad243329aa6a13...). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=10074 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Don't know that this needs a dependency on bug 41104? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to Lari Taskula from comment #5)
(In reply to Lari Taskula from comment #3)
Looking at git history, I noticed that we have attempted to use output_html_with_http_headers here before (Bug 10074), but the change was reverted.
https://github.com/Koha-Community/Koha/commit/ 3536b10acf223b33819153532841494c816d6375 There's plenty of depth into this topic (Bug 6554 and encoding issues). It's a bit too much for me at this moment.
It seems Bug 10074 was reverted as a part of larger set of reverts (Bug 6554 comment 103 , https://github.com/Koha-Community/Koha/commits/ 16753c750d97385ad243329aa6a1328352749bb1/).
I've tried out the interface using es-ES and I don't see any encoding problems. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to Lari Taskula from comment #4)
12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl 13. Log in using your test patron's credentials 14. Observe "Self check-in" page 15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 16. Observe "Log in to your account" page, ie. you have been kicked out
I didn't get kicked out, but that's because I was using a superlibrarian (as I wasn't perfecting following the test plan). Looks like the SCI has different parameters than SCO... interesting. But that's a pre-existing thing. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #188475|0 |1 is obsolete| | --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Created attachment 188722 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188722&action=edit Bug 38363: Use output_html_with_http_headers in C4::Auth The following C4::Auth subroutines - checkauth() - get_template_and_user() were in some cases outputting html outside of C4::Output's output mechanisms. It is important to ensure the consistency of our HTTP responses. Having multiple output mechanisms for what is essentialy the same type of response not only leads to unneccessary code duplication and complexity but possibly even bugs at some point in the future. For these reasons it is better to output only through C4::Output, when possible. This patch does that for the two aforementioned C4::Auth cases. To test unit tests: 1. prove t/db_dependent/Auth.t As for checkauth(), the change only applies to unauthenticated state and failed logins. To test checkauth: 1. Enable system preference opacuserlogin 2. Navigate to OPAC 3. Observe OPAC loading successfully 4. Log in to OPAC with invalid credentials 5. Observe "You entered an incorrect username or password" 6. Navigate to staff client without being logged in (logout if you are in) 7. Observe staff client login screen loading successfully 8. Log in to staff client with invalid credentials 9. Observe "Invalid username or password" As for get_template_and_user(), the change only applies to SCO/SCI user navigating outside of the self service modules. To test get_template_and_user: 1. Enable system preference WebBasedSelfCheck 2. Create a new test patron and note down their username & password 3. Grant them a permission self_checkout_module (under Self check modules) 4. Navigate to http://OPAC/cgi-bin/koha/sco/sco-main.pl 5. Log in with the test patron 6. Observe successful login (should display "Self-checkout system" page) 7. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 8. Observe "Log in to your account" page, ie. you have been kicked out 9. Modify your test patron's permissions 10. Remove permission "self_checkout_module" and add "self_checkin_module" 11. Enable system preference SelfCheckInModule 12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl 13. Log in using your test patron's credentials 14. Observe "Self check-in" page 15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 16. Observe "Log in to your account" page, ie. you have been kicked out Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |lari.taskula@hypernova.fi |ity.org | --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Looks good to me. Thanks, Lari! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #11 from Lari Taskula <lari.taskula@hypernova.fi> --- (In reply to David Cook from comment #6)
Don't know that this needs a dependency on bug 41104?
This Bug 38363 modifies the same line as Bug 41104. In fact I initially discovered the bug in Bug 41104 while working on this Bug 38363, but I decided to fix it in another Bug 41104. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to Lari Taskula from comment #11)
(In reply to David Cook from comment #6)
Don't know that this needs a dependency on bug 41104?
This Bug 38363 modifies the same line as Bug 41104. In fact I initially discovered the bug in Bug 41104 while working on this Bug 38363, but I decided to fix it in another Bug 41104.
Yeah I thought about that a bit more later. I guess it depends on how QA treats it. Hopefully both get done at the same time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 189617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189617&action=edit Bug 38363: Use output_html_with_http_headers in C4::Auth The following C4::Auth subroutines - checkauth() - get_template_and_user() were in some cases outputting html outside of C4::Output's output mechanisms. It is important to ensure the consistency of our HTTP responses. Having multiple output mechanisms for what is essentialy the same type of response not only leads to unneccessary code duplication and complexity but possibly even bugs at some point in the future. For these reasons it is better to output only through C4::Output, when possible. This patch does that for the two aforementioned C4::Auth cases. To test unit tests: 1. prove t/db_dependent/Auth.t As for checkauth(), the change only applies to unauthenticated state and failed logins. To test checkauth: 1. Enable system preference opacuserlogin 2. Navigate to OPAC 3. Observe OPAC loading successfully 4. Log in to OPAC with invalid credentials 5. Observe "You entered an incorrect username or password" 6. Navigate to staff client without being logged in (logout if you are in) 7. Observe staff client login screen loading successfully 8. Log in to staff client with invalid credentials 9. Observe "Invalid username or password" As for get_template_and_user(), the change only applies to SCO/SCI user navigating outside of the self service modules. To test get_template_and_user: 1. Enable system preference WebBasedSelfCheck 2. Create a new test patron and note down their username & password 3. Grant them a permission self_checkout_module (under Self check modules) 4. Navigate to http://OPAC/cgi-bin/koha/sco/sco-main.pl 5. Log in with the test patron 6. Observe successful login (should display "Self-checkout system" page) 7. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 8. Observe "Log in to your account" page, ie. you have been kicked out 9. Modify your test patron's permissions 10. Remove permission "self_checkout_module" and add "self_checkin_module" 11. Enable system preference SelfCheckInModule 12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl 13. Log in using your test patron's credentials 14. Observe "Self check-in" page 15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl (you can get there by editing the address bar of your web browser) 16. Observe "Log in to your account" page, ie. you have been kicked out Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #188722|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 --- Comment #14 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Laura.escamilla@bywatersolu | |tions.com Status|Pushed to main |Needs documenting --- Comment #15 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- This is an enhancement and will not be backported to the 25.05.x branch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38365 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #16 from David Nind <david@davidnind.com> --- Architectural change, no changes required to the manual. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38365 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38363 Bug 38363 depends on bug 41104, which changed state. Bug 41104 Summary: Samesite HTTP response header being set in C4::Auth::checkauth() https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41104 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org