Bug 9611 broke the test cases when running without a database (i.e. when packages are being built.) The problem is that the hashing stuff was put into C4::Auth, which makes sense, however this means that C4::Members has to use C4::Auth, and C4::Auth requires a database, and many things use C4::Members... So I'm going to move the hashing stuff into its own place in the Koha namespace.
Created attachment 21866 [details] [review] Bug 11013 - move hashing functions into their own module As the hashing functions were centralised into C4::Auth, this required more modules (C4::Members in particular) to use it. However, C4::Auth uses the database, and so anything that used C4::Members failed to load in the 00-load.t test. This patch moves the hashing into its own small module so this isn't an issue. Test plan: * Ensure that password related functions work: ** Creating a user account ** Logging in ** Changing a password (OPAC and staff client) ** LDAP logins still work like they should * Ensure that tests pass without a database server present Note: there are testing notes for LDAP on bug 9611 that'll bypass the need to set up an LDAP server.
Upgraded the importance, as packages can't be built while this bug is here.
Created attachment 21867 [details] [review] Bug 11013 - move hashing functions into their own module As the hashing functions were centralised into C4::Auth, this required more modules (C4::Members in particular) to use it. However, C4::Auth uses the database, and so anything that used C4::Members failed to load in the 00-load.t test. This patch moves the hashing into its own small module so this isn't an issue. Test plan: * Ensure that password related functions work: ** Creating a user account ** Logging in ** Changing a password (OPAC and staff client) ** LDAP logins still work like they should * Ensure that tests pass without a database server present Note: there are testing notes for LDAP on bug 9611 that'll bypass the need to set up an LDAP server.
Created attachment 21873 [details] [review] [SIGNED-OFF] Bug 11013 - move hashing functions into their own module As the hashing functions were centralised into C4::Auth, this required more modules (C4::Members in particular) to use it. However, C4::Auth uses the database, and so anything that used C4::Members failed to load in the 00-load.t test. This patch moves the hashing into its own small module so this isn't an issue. Test plan: * Ensure that password related functions work: ** Creating a user account ** Logging in ** Changing a password (OPAC and staff client) ** LDAP logins still work like they should * Ensure that tests pass without a database server present Note: there are testing notes for LDAP on bug 9611 that'll bypass the need to set up an LDAP server. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> All Works. No koha-qa errors Test 1) patch applied 2) new user created 3) change pass from opac/staff 4) change pass using LDAP (sort of, same as on Bug 9611) NOTE: Robin, you just sent an email about copyright headers, KohaPassword.pm's reads "...either version 2 of the License,...", coding guidelines suggests other wording and version 3 of GPL.
(In reply to Bernardo Gonzalez Kriegel from comment #4) > NOTE: Robin, you just sent an email about copyright headers, > KohaPassword.pm's reads "...either version 2 of the License,...", > coding guidelines suggests other wording and version 3 of GPL. That was a test! Actually, it was because I copy-pasted from elsewhere and didn't check. You are totally correct. I'll add a follow-up tomorrow.
Created attachment 21887 [details] [review] Bug 11013 - make copyright headers current
Created attachment 21888 [details] [review] [SIGNED-OFF] Bug 11013 - make copyright headers current Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Updated copyright headers ok
Robin, Galen pushed a followup for bug 9611 (3min after the Bernardo's signoff!), moving the password routines into a new module (Koha::AuthUtils). These patchs don't apply anymore.
In fact I think this report can be marked as resolved/duplicate. I let you change the status, except if you disagree.
Yeah, if it solves the same problem, that's good too :) *** This bug has been marked as a duplicate of bug 9611 ***