Patron registration- unable to log in with password entered during registration

Bug #1579225 reported by Christine Burns
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Fix Released

Bug Description

Tested on EG 2.10

Register new patron -> record password entered on registration -> attempt to log in to staff client / OPAC with new account -> "Login failed. Please check your Server Hostname, Username, Password, and your CAPS LOCK key." -> password entered on registration screen does not work

Retrieve patron F1 -> edit -> edit password -> Save - > attempt to log in to staff client / OPAC -> successful

I have tested the following

1) use system generated password (random 4 digit code shown in password field during registration) = Login failed
2) enter specific password in both password fields during registration = Login failed
3) password from phone number (Library setting - Patron password from phone# -> enter daytime phone # -> password field is populated with last 4 digits of daytime phone #) = Login failed

Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

We've tested this on our own testing servers running 2.10.1 and 2.10.2 and I also tested using Equinox's community server running 2.10.2 (regular client and web client) and the bug exists on all the servers.

tags: added: patron
Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

I have also tested this using MassLNC's 2.9.2 community server and can confirm that this is not an issue in 2.9.2.

Revision history for this message
Galen Charlton (gmc) wrote :

For the sake of anybody else doing some initial investigation, here are comments I made on IRC:

[18:26:04] <gmcharlt> yep, it's the password hashing
[18:26:40] <gmcharlt> during registration, the hashed password is getting set to CRYPT( MD5( pw_salt || MD5(MD5(real_password)) ), pw_salt )
[18:26:56] <gmcharlt> when comparisons are expecting CRYPT( MD5( pw_salt || MD5(real_password) ), pw_salt )
[18:27:08] <gmcharlt> I'll poke more after dinner
[18:30:26] <gmcharlt> ok, seeing more: during the dance, a new actor.usr is created
[18:30:50] <gmcharlt> the password trigger on actor.usr sets au.passwd to md5(real_password)
[18:31:22] <gmcharlt> *then*, the new row gets retrieved and passed on to the rest of the process, including a call to actor.set_passwd
[18:31:32] <gmcharlt> and that's where the extraneous md5 is coming from
[18:31:59] <gmcharlt> more anon

Revision history for this message
Galen Charlton (gmc) wrote :

Here's a hacky patch to have the password work for a newly-registered patron:

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/ b/Open-ILS/src/perlmods/lib/OpenILS/Application/
index 1e0593d..dfd0562 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/
@@ -408,12 +408,15 @@ sub update_patron {
     my $barred_hook = '';

     if($patron->isnew()) {
+ # store original password to avoid double-hashing it later
+ my $saved_passwd = $patron->passwd;
         ( $new_patron, $evt ) = _add_patron($e, _clone_patron($patron));
         return $evt if $evt;
         if($U->is_true($patron->barred)) {
             return $e->die_event unless
                 $e->allowed('BAR_PATRON', $patron->home_ou);
+ $new_patron->passwd($saved_passwd);
     } else {
         $new_patron = $patron;

Changed in evergreen:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Galen Charlton (gmc) wrote :

I've pushed a better patch to the collab/gmcharlt/lp1579225_patron_registration_password_mgmt branch in the working repository. Feedback welcome; this still seems pretty hacky to me, and I wonder if we're not better off using database triggers to populate and update actor.passwd.

Galen Charlton (gmc)
tags: added: regression
Revision history for this message
Dan Wells (dbw2) wrote :

I am certainly not opposed to a db-level solution, but this could at least be an interim fix. Tested the cases listed in the patch, and also a number of cases involving existing users with non-migrated passwords, including whether the passwords still migrate successfully on login. Everything I tested worked as expected.

I still want others to have a chance to weigh in, but here is my signoff:;a=shortlog;h=refs/heads/collab/dbwells/lp1579225_patron_registration_password_mgmt_signoff

Thanks, Galen!

Revision history for this message
Mike Rylander (mrylander) wrote :

I'd normally say that a pure in-db solution would be preferable to maintain as much separation between the business logic code and the stored passwords, but Galen's approach changes the least amount of code to get the job done, and that's more important for a quick release. Less lines of new code means fewer chances for bugs.

Here's my pile-on sign-off:;a=shortlog;h=refs/heads/collab/miker/lp1579225_patron_registration_password_mgmt_signoff2

Revision history for this message
Kathy Lussier (klussier) wrote :

I'll add my sign-off to the pile too. I tested new patron registrations, updating records with password changes, updating passwords without password changes. It all worked as expected.;a=shortlog;h=refs/heads/collab/kmlussier/lp1579225_patron_registration_password_mgmt_signoff3

Galen Charlton (gmc)
Changed in evergreen:
milestone: none → 2.10.4
milestone: 2.10.4 → 2.10.3
tags: added: pullrequest
Revision history for this message
Galen Charlton (gmc) wrote :

Thanks for the testing, everybody! I've pushed the patch to master and rel_2_10, along with a live_t regression test I wrote as part of doing a final check of the patch.

Galen Charlton (gmc)
Changed in evergreen:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.