Bug 19579 - PatronSelfRegistrationEmailMustBeUnique does not prevent duplicates when using PatronSelfRegistrationVerifyByEmail
Summary: PatronSelfRegistrationEmailMustBeUnique does not prevent duplicates when usin...
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-03 21:54 UTC by BASE Library Consortium
Modified: 2018-05-11 14:12 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19579: Do not confirm the registration if email already used - PatronSelfRegistrationEmailMustBeUnique (1.98 KB, patch)
2017-12-22 17:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19579: Do not confirm the registration if email already used - PatronSelfRegistrationEmailMustBeUnique (2.02 KB, patch)
2018-05-10 15:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19579: Do not confirm the registration if email already used - PatronSelfRegistrationEmailMustBeUnique (2.12 KB, patch)
2018-05-11 07:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description BASE Library Consortium 2017-11-03 21:54:53 UTC
With PatronSelfRegistrationVerifyByEmail set to Require and PatronSelfRegistrationEmailMustBeUnique set to Consider a patron can still self register many times using the same email address.

Test case
1. Patron submits self registration form using the same email address 3 times
2. Patron receives 3 verification emails
3. Patron clicks on 3 verify token URLs
4. Staff client now shows the 3 patrons
5. Patron logs into 3 accounts on OPAC
6. Patron cannot change personal details because email address already exists
Comment 1 Jonathan Druart 2017-12-22 17:57:45 UTC Comment hidden (obsolete)
Comment 2 Charles Farmer 2018-04-06 20:43:06 UTC
Unless I'm misconfiguring something, I'm not able to send myself more than 1 registration message at a time, with or without the patch.
Comment 3 Jonathan Druart 2018-04-06 21:44:23 UTC
(In reply to Charles Farmer from comment #2)
> Unless I'm misconfiguring something, I'm not able to send myself more than 1
> registration message at a time, with or without the patch.

Do not click on the link you received before registering the second time.
Comment 4 Owen Leonard 2018-05-10 15:30:28 UTC
Created attachment 75226 [details] [review]
Bug 19579: Do not confirm the registration if email already used - PatronSelfRegistrationEmailMustBeUnique

If PatronSelfRegistrationVerifyByEmail and
PatronSelfRegistrationEmailMustBeUnique are set, it should not be
possible to register twice with the same email.
However the test is made on already created patron cards when the
registration is done.
Which means it is possible to register several times with the same
email address and click on the registration link to finalise the
registration.

This patch adds a test when the registration link is clicked and display
the "Registration invalid" generic message if the same email is used

Test plan:
1. Patron submits self registration form using the same email address 3
   times
2. Patron receives 3 verification emails
3. Patron clicks on 3 verify token URLs
   => Only the first registration should succeed, the 2 others must fail

Maybe we should display a more specific message?

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Marcel de Rooy 2018-05-11 06:34:22 UTC
QA: Looking here
Comment 6 Marcel de Rooy 2018-05-11 07:22:01 UTC
Created attachment 75256 [details] [review]
Bug 19579: Do not confirm the registration if email already used - PatronSelfRegistrationEmailMustBeUnique

If PatronSelfRegistrationVerifyByEmail and
PatronSelfRegistrationEmailMustBeUnique are set, it should not be
possible to register twice with the same email.
However the test is made on already created patron cards when the
registration is done.
Which means it is possible to register several times with the same
email address and click on the registration link to finalise the
registration.

This patch adds a test when the registration link is clicked and display
the "Registration invalid" generic message if the same email is used

Test plan:
1. Patron submits self registration form using the same email address 3
times
2. Patron receives 3 verification emails
3. Patron clicks on 3 verify token URLs
=> Only the first registration should succeed, the 2 others must fail

Maybe we should display a more specific message?

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2018-05-11 07:26:49 UTC
Side note:
It is still possible to register twice on the same email address. The pref name PatronSelfRegistrationEmailMustBeUnique implicitly conflicts with that. Note that imo allowing two registrations is probably preferred but the pref name could be seen as confusing..

Also if the email address is in one of the other columns like emailpro, the registration can be verified since we only look at the column email.
Comment 8 Jonathan Druart 2018-05-11 14:12:50 UTC
Pushed to master for 18.05, thanks to everybody involved!