In Bug 36679 we blocked the AnonymousPatron from logging into SCO. The AnonymousPatron should not log in to anything, staff interface/OPAC/SCO.
Created attachment 167793 [details] [review] Bug 37104: Block AnonymousPatron from logging into staff interface and OPAC This patch blocks the patron set as the anonymous patron from logging into the staff interface and OPAC. To test: 1) In Administration->sys. pref, make sure AnonymousPatron is pointed to an account. 2) Visit that patron's page and set their permissions to superlibrarian ("Access to all librarian functions") 3) Ensure that you know the username and password for this patron and can log in. 4) Visit the OPAC, attempt to log-in with your anon patron. 5) Note that you can log in and nothing happens. 6) Visit the staff interface, attempt to log-in with anon patron. 7) Once again, note that you are able to log-in with no issue. 8) Apply patch and restart_all 9) Attempt to log into the OPAC and staff interface with the patron again. 10) This time, you should get an error message on both pages saying, "Error: You can't log in as the anonymous patron!"
Created attachment 167795 [details] [review] Bug 37104: Block AnonymousPatron from logging into staff interface and OPAC This patch blocks the patron set as the anonymous patron from logging into the staff interface and OPAC. To test: 1) In Administration->sys. pref, make sure AnonymousPatron is pointed to an account. 2) Visit that patron's page and set their permissions to superlibrarian ("Access to all librarian functions") 3) Ensure that you know the username and password for this patron and can log in. 4) Visit the OPAC, attempt to log-in with your anon patron. 5) Note that you can log in and nothing happens. 6) Visit the staff interface, attempt to log-in with anon patron. 7) Once again, note that you are able to log-in with no issue. 8) Apply patch and restart_all 9) Attempt to log into the OPAC and staff interface with the patron again. 10) This time, you should get an error message on both pages saying, "Error: You can't log in as the anonymous patron!"
Created attachment 167798 [details] [review] Bug 37104: Block AnonymousPatron from logging into staff interface and OPAC This patch blocks the patron set as the anonymous patron from logging into the staff interface and OPAC. To test: 1) In Administration->sys. pref, make sure AnonymousPatron is pointed to an account. 2) Visit that patron's page and set their permissions to superlibrarian ("Access to all librarian functions") 3) Ensure that you know the username and password for this patron and can log in. 4) Visit the OPAC, attempt to log-in with your anon patron. 5) Note that you can log in and nothing happens. 6) Visit the staff interface, attempt to log-in with anon patron. 7) Once again, note that you are able to log-in with no issue. 8) Apply patch and restart_all 9) Attempt to log into the OPAC and staff interface with the patron again. 10) This time, you should get an error message on both pages saying, "Error: You can't log in as the anonymous patron!"
Created attachment 167801 [details] [review] Bug 37104: (Follow up) Restructure checkpw code
Created attachment 167802 [details] [review] Bug 37104: Unit tests prove t/db-dependent/Auth.t
Created attachment 167811 [details] [review] Bug 37104: Block AnonymousPatron from logging into staff interface and OPAC This patch blocks the patron set as the anonymous patron from logging into the staff interface and OPAC. To test: 1) In Administration->sys. pref, make sure AnonymousPatron is pointed to an account. 2) Visit that patron's page and set their permissions to superlibrarian ("Access to all librarian functions") 3) Ensure that you know the username and password for this patron and can log in. 4) Visit the OPAC, attempt to log-in with your anon patron. 5) Note that you can log in and nothing happens. 6) Visit the staff interface, attempt to log-in with anon patron. 7) Once again, note that you are able to log-in with no issue. 8) Apply patch and restart_all 9) Attempt to log into the OPAC and staff interface with the patron again. 10) This time, you should get an error message on both pages saying, "Error: You can't log in as the anonymous patron!" Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167812 [details] [review] Bug 37104: (Follow up) Restructure checkpw code Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167813 [details] [review] Bug 37104: Unit tests prove t/db-dependent/Auth.t Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. To run the tests (underscore, rather than a dash): prove t/db_dependent/Auth.t
Created attachment 168268 [details] [review] Bug 37104: Unit tests prove t/db-dependent/Auth.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 168269 [details] [review] Bug 37104: Block AnonymousPatron from logging into staff interface and OPAC This patch blocks the patron set as the anonymous patron from logging into the staff interface and OPAC. To test: 1) In Administration->sys. pref, make sure AnonymousPatron is pointed to an account. 2) Visit that patron's page and set their permissions to superlibrarian ("Access to all librarian functions") 3) Ensure that you know the username and password for this patron and can log in. 4) Visit the OPAC, attempt to log-in with your anon patron. 5) Note that you can log in and nothing happens. 6) Visit the staff interface, attempt to log-in with anon patron. 7) Once again, note that you are able to log-in with no issue. 8) Apply patch and restart_all 9) Attempt to log into the OPAC and staff interface with the patron again. 10) This time, you should get an error message on both pages saying, "Error: You can't log in as the anonymous patron!" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 168270 [details] [review] Bug 37104: (Follow up) Restructure checkpw code Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks Sam, all working as expected now. QA scripts happy and unit tests passing. Passing QA
my $is_anonymous_patron = $patron && ( $patron->borrowernumber eq $anonymous_patron ); Why not use == instead of eq?
Pushed for 24.11! Well done everyone, thank you!
Found a new warning that seems related to this change. [WARN] Use of uninitialized value $anonymous_patron in string eq at /kohadevbox/koha/C4/Auth.pm line 2065.
Created attachment 168592 [details] [review] Bug 37104: (Follow up) Checks for unitialized value of 'anonymous_patron' system pref
(In reply to Jonathan Druart from comment #16) > Found a new warning that seems related to this change. > > [WARN] Use of uninitialized value $anonymous_patron in string eq at > /kohadevbox/koha/C4/Auth.pm line 2065. The issue is due to the 'AnonymousPatron' system preference being a text entry bod, so it could be an empty string. Needed to first check if it is initialized. What do you think of this, Jonathan?
(In reply to Sam Lau from comment #18) > (In reply to Jonathan Druart from comment #16) > > Found a new warning that seems related to this change. > > > > [WARN] Use of uninitialized value $anonymous_patron in string eq at > > /kohadevbox/koha/C4/Auth.pm line 2065. > > The issue is due to the 'AnonymousPatron' system preference being a text > entry bod, so it could be an empty string. Needed to first check if it is > initialized. What do you think of this, Jonathan? Yes, but why did you replace '==' with 'eq'? It shouldn't make much differences, but the other occurrences are using eq.
Created attachment 168666 [details] [review] Bug 37104: (Follow up) Checks for unitialized value of 'anonymous_patron' system pref Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> JD Amended patch: replace '==' with 'eq' for consistency with other occurrences.
Created attachment 168694 [details] [review] Bug 37104: (Follow up) Checks for unitialized value of 'anonymous_patron' system pref Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> JD Amended patch: replace '==' with 'eq' for consistency with other occurrences.
I think I missed this follow-up - grabbing it now.
(In reply to Katrin Fischer from comment #22) > I think I missed this follow-up - grabbing it now. I would add a search on > Pushed to Main that that have the "additional_work_needed" kw. Then it should be removed when you dealt with it :)