Description
Marc Véron
2016-06-10 18:35:51 UTC
Perhaps require the library card number associated with the account when requesting the recovery/reset? I wonder how having multiple accounts with the same email address would affect the Mozilla Persona stuff. I seem to recall Chris saying that an email address should only belong to one account, although that's not enforced anywhere... (In reply to David Cook from comment #2) > I wonder how having multiple accounts with the same email address would > affect the Mozilla Persona stuff. > > I seem to recall Chris saying that an email address should only belong to > one account, although that's not enforced anywhere... It's not a realistic expectation. Are parents supposed to start a new e-mail account for every child that has a library card??? (In reply to Christopher Brannon from comment #3) > (In reply to David Cook from comment #2) > > I wonder how having multiple accounts with the same email address would > > affect the Mozilla Persona stuff. > > > > I seem to recall Chris saying that an email address should only belong to > > one account, although that's not enforced anywhere... > > It's not a realistic expectation. Are parents supposed to start a new > e-mail account for every child that has a library card??? That being said, it may have to come down to either logging in with the card number or a combination of e-mail and card number. (In reply to Christopher Brannon from comment #3) > It's not a realistic expectation. Are parents supposed to start a new > e-mail account for every child that has a library card??? That's certainly a valid point. I'm just pointing out that it will probably cause unexpected behaviour when using Mozilla Persona, but I don't know how many people actually use Persona anyway. As for the password reset, yeah I think it would make sense to provide userid or cardnumber as that's the unique identifier used for the patron, no? You could just put in your cardnumber/userid, and then get a message saying: "A password reset email has been send to the email address registered for account test01" Or something like that. We actually have a local password reset service... I should look how I implemented it... I second the idea that the card number/user id should be required to reset a password, since that should always be a unique identifier. Thinking loud about sending information about all library accounts if in password recovery an email with multiple library accounts attached is given: What could be the reasons to have one email address and multiple accounts? - An individual has multiple library accounts, e.g. one for professional purposes and a private one. For exammple a teacher needs books for school and wants to separate that from their private lecture. In that case sending a reset links for multiple accounts would be OK (no privacy breach) because it is the same person. But atm the mail contains only a reset link, no information that would help to identifiy the account. So the individual could reset the wrong password. - A group (family, couple) shares the same email but they have different library accounts In that case, privacy is defined by the group: Either one individual has access to the email or all of the group have access. If all have access, one individual could change the password for an other one. That would be a privacy breach. OK then, it is not a good idea to send reset links for all accounts attached to an email account. IMO the easiest solution would be not to send recovery link(s), but to display an information similar to the one that appears if an email address is not found in the database (Error No account was found with the provided information. Please...) The message could be something like: ------------- Information Multiple library accounts are attached to this email. Please fill the field 'Login' to identify which password you want to reset. Please contact the library if you need further assistance. ------------- Additionally, there could be more enhancements (to be covered by separate bugs): - Have a syspref to prevent multiple accounts with same email (could get complicated for existing installs) - Have a precooked report that displays library accounts sharing one email address Created attachment 63591 [details] [review] Bug 16711: OPAC Password recovery: Handling if multiple accounts have the same mail address To reproduce: - Create 3 Accounts, login names are test01, test02, test03, Email is the same for all. - Go to OPAC -> Password recovery and indicate E-Mail only - You will get an email for only one of the accounts above. To test: - Apply patch, restart memcached and plack - Go to db, delete from borrower_password_recovery; - Try steps above to reproduce. You will get an error message: Account identification with this email address only is ambiguous. Please use the field 'Login' as well. - Verify that other cases work as before (provide valid / invalid login only, provide valid email for an existing account, provide unknown email, provide both login and email with all combinations of valid / invalid) Created attachment 63600 [details] [review] [SIGNED-OFF] Bug 16711: OPAC Password recovery: Handling if multiple accounts have the same mail address To reproduce: - Create 3 Accounts, login names are test01, test02, test03, Email is the same for all. - Go to OPAC -> Password recovery and indicate E-Mail only - You will get an email for only one of the accounts above. To test: - Apply patch, restart memcached and plack - Go to db, delete from borrower_password_recovery; - Try steps above to reproduce. You will get an error message: Account identification with this email address only is ambiguous. Please use the field 'Login' as well. - Verify that other cases work as before (provide valid / invalid login only, provide valid email for an existing account, provide unknown email, provide both login and email with all combinations of valid / invalid) Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 64064 [details] [review] Added the code from bug 18616, which allows patron to also use their card number as a login id since there were merge conflicts with that patch. Comment on attachment 64064 [details] [review] Added the code from bug 18616, which allows patron to also use their card number as a login id since there were merge conflicts with that patch. Made 18616 dependent on 16711 and moved David's patch back to 18616, and signed off 18616 Still applies on current master opac-password-recovery.pl: $search_results = [ Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ) ]; $resultCount = Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ) -> count; It seems we could just not get the results as an array and use count directly, casting scalar seems unnecessary as well here i.e. $search_results = Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ); ... if ( not $search_results ) { ... elsif ( $username && $search_results->count > 1) ... etc. Created attachment 64598 [details] [review] Bug 16711: (QA-followup) Use count directly See comment # 13 Hope it's OK to set back to SO. Created attachment 64604 [details] [review] Bug 16711: OPAC Password recovery: Handling if multiple accounts have the same mail address To reproduce: - Create 3 Accounts, login names are test01, test02, test03, Email is the same for all. - Go to OPAC -> Password recovery and indicate E-Mail only - You will get an email for only one of the accounts above. To test: - Apply patch, restart memcached and plack - Go to db, delete from borrower_password_recovery; - Try steps above to reproduce. You will get an error message: Account identification with this email address only is ambiguous. Please use the field 'Login' as well. - Verify that other cases work as before (provide valid / invalid login only, provide valid email for an existing account, provide unknown email, provide both login and email with all combinations of valid / invalid) Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 64605 [details] [review] Bug 16711: (QA-followup) Use count directly See comment # 13 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Patches will be pushed squashed. Pushed to master for 17.11, thanks to everybody involved! Enhancement not pushed to 17.05.x I wonder if we should treat this as a bug instead, as it causes a problem for libraries with family accounts. What do you think? (In reply to Katrin Fischer from comment #21) > I wonder if we should treat this as a bug instead, as it causes a problem > for libraries with family accounts. What do you think? +1 Sent back to the RMaint queue. (In reply to Katrin Fischer from comment #21) > I wonder if we should treat this as a bug instead, as it causes a problem > for libraries with family accounts. What do you think? Ok I'm in. Patch is small indeed Pushed to 17.05.x, will be in 17.05.02. If userid was mandatory, there would be no problem and this form would be more protected against attacks I's say. In some libraries the patrons simply don't know their userid as it's not used by the library. Koha generates it, but the patrons don't know about it. So I don't think it should be mandatory. There is another patch I think would be very useful for 17.05 and 16.11 - bug 18616, that will also allow to use the cardnumber. Contains strings - holding this for next 16.11.x release. This patch has been pushed to 16.11.x and will be in 16.11.10. Sorry, 16.11.11! Pushed to 16.05.x, for 16.05.16 release |