check_password is implemented in two places: 1. In Koha::Patron::set_password https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Patron.pm#L774 2. In Koha::Patron::store https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Patron.pm#L167 The call to the plugin inside Koha::Patron::store comes before this line: $self = $self->SUPER::store; which means there is not yet a borrowernumber to pass to the plugin, and this never worked.
Created attachment 114721 [details] [review] Bug 27311 - Don't pass borrowernumber to check_password Bug 22706 added the check_password plugin hook, taking both a borrowernumber and the raw password as arguments. The hook is called in two places in Koha::Patron, but because of how the code that calls the hook is placed, passing borrowernumber to the plugin inside "sub store" never worked. This patch removes the passing of the borrowernumber from both calls. A tiny plugin that uses the check_password hooks can ve found here: https://github.com/Libriotech/koha-plugin-pin To test: - Enable plugins - Optionally install the PIN plugin - Add and edit users in all possible ways, including -- Setting a password for a new user -- Changing a password in the staff client -- Changing a password in the OPAC -- Creating a disabled user account Sign off, if all this works as expected.
I understand why you remove it from ->store (because in the creation block), but why from set_password?
(In reply to Magnus Enger from comment #0) > which means there is not yet a borrowernumber to pass to the plugin, and > this never worked. I was just about to say this when I noticed the bug reference...
1. Shouldn't this have a test to account for the change? 2. Processing additional checks * Commit title does not start with 'Bug XXXXX: ' - 68f151152e1