I tried in our production system (22.05.11) to change the username on my own account (an account with superlibrarian privileges) and it let to a white page blank except for the text "Internal Server Error." I wasn't able to access any page in Koha until I cleared my browser's cookies. When I test in master I see this error: Can't call method "password_expired" on an undefined value at /kohadevbox/koha/C4/Auth.pm line 1780 I think the problem happens only when you change the username field but you enter your existing password.
Created attachment 151609 [details] [review] Bug 33815: Do not explode if logged in user modify their own userid If the logged in librarian modifies their own userid they will get the following error when submitting the form: Can't call method "password_expired" on an undefined value at /kohadevbox/koha/C4/Auth.pm line 1780 We could handle this situation and flag the session as expired. Better would be to deal with this specific user case and update the cookie (?)
Created attachment 151610 [details] [review] Bug 33815: Add a hint on the patron edit form
Is a logout acceptable?
We need tests for the first patch.
Created attachment 151633 [details] [review] Bug 33815: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 151634 [details] [review] Bug 33815: Do not explode if logged in user modify their own userid If the logged in librarian modifies their own userid they will get the following error when submitting the form: Can't call method "password_expired" on an undefined value at /kohadevbox/koha/C4/Auth.pm line 1780 We could handle this situation and flag the session as expired. Better would be to deal with this specific user case and update the cookie (?)
Created attachment 151635 [details] [review] Bug 33815: Add a hint on the patron edit form
Created attachment 151642 [details] [review] Bug 33815: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 151643 [details] [review] Bug 33815: Do not explode if logged in user modify their own userid If the logged in librarian modifies their own userid they will get the following error when submitting the form: Can't call method "password_expired" on an undefined value at /kohadevbox/koha/C4/Auth.pm line 1780 We could handle this situation and flag the session as expired. Better would be to deal with this specific user case and update the cookie (?) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
I might have an idea for this one...
(In reply to David Cook from comment #10) > I might have an idea for this one... The ID that can't be changed is borrowernumber, so in check_cookie_auth() we should get $patron using $session->param('number') instead of userid/cardnumber; But... the $session will still contain the wrong details. In theory, this same problem applies beyond just userid/id. If the user's details are changed and the session details are used anywhere else, they'll be wrong until they log out and log back in. (Of course, that's not uncommon among many systems.)
(In reply to David Cook from comment #11) > But... the $session will still contain the wrong details. In theory, this > same problem applies beyond just userid/id. If the user's details are > changed and the session details are used anywhere else, they'll be wrong > until they log out and log back in. (Of course, that's not uncommon among > many systems.) As I suspected, at line 559 of ./members/memberentry.pl there is a C4::Auth::haspermission check that uses C4::Context->userenv->{id} which is populated from the database session which is the wrong value, and that triggers a permission/authorization failure (if you use borrowernumber to find the patron to get through check_cookie_auth). -- This is tricky. I suppose we could update/refresh the session, if we stored "updated_on" in the $session as well, and then checked that against the database at authentication time. That said, for an authenticate related change like userid perhaps it is best to expire the session, as a critical aspect of the user account has been changed. If we stored "updated_on" in the $session, we could also notify users if their user id has been changed. This would probably be easier on the staff interface than the OPAC.
I would be in favor of a perfect solution being a later follow-up to a good solution!
(In reply to Owen Leonard from comment #13) > I would be in favor of a perfect solution being a later follow-up to a good > solution! Agreed although I don't know if there is a perfect solution for this one! I'll look at QAing this shortly...
Created attachment 151715 [details] [review] Bug 33815: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 151716 [details] [review] Bug 33815: Do not explode if logged in user modify their own userid If the logged in librarian modifies their own userid they will get the following error when submitting the form: Can't call method "password_expired" on an undefined value at /kohadevbox/koha/C4/Auth.pm line 1780 We could handle this situation and flag the session as expired. Better would be to deal with this specific user case and update the cookie (?) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: David Cook <dcook@prosentient.com.au>
Pushed to master for 23.05. Nice work everyone, thanks!
Will this be backported to 22.11.xx?
Nice work everyone! Pushed to stable for 22.11.x
*** Bug 31431 has been marked as a duplicate of this bug. ***