Description
Lari Taskula
2017-08-17 11:17:12 UTC
Created attachment 66415 [details] [review] Bug 19133: Password recovery via REST API This patch adds password recovery functionality to REST API. POST /patrons/password/recovery Creates a new password recovery request. Takes a JSON body with "email" and one of "userid" or "cardnumber" as parameters. Sends patron PASSWORD_RESET email. POST /patrons/password/recovery/complete Completes pending password recovery. Takes a JSON body with "uuid", "new_password" and "confirm_new_password" as parameters. If passwords meet Koha's password requirements, changes patron's password to the requested new password. To test: 1. prove t/db_dependent/api/v1/passwordrecovery.t 2. Send POST requests to the endpoints listed above. See the description of endpoints above for required parameters. 3. Check message_queue (or your email if you actually send messages in the queue) to make sure your password recovery was enqueued 4. Observe your password has been changed once you completed the recovery process Created attachment 66416 [details] [review] Bug 19133: Add an ability to link patron to third party service to reset their password Adds additional functionality to POST /patrons/password/recovery. Pass a new parameter "custom_link" to send patron a recovery email that contains this link instead of Koha OPAC's opac-password-recovery.pl page. This way third party services can link Koha patrons to their own service to have passwords recovered. By default, forbids all custom links. To enable specific services, use the new system preference OpacResetPasswordHostWhitelist. This preference contains a list of allowed host names to which it is allowed to link the patron in recovery email. Use case in steps (to understand the feature better): 1. Library uses a discovery software (e.g. VuFind, from now on VF) and has disabled Koha's OPAC 2. VF uses Koha REST API 3. Patrons can login to VF with their Koha userid and password 4. Patron goes to VF login screen, but has forgotten their password 5. Patron requests their password to be reset in VF 6. VF sends a POST request to /api/v1/patrons/password/recovery with patron's userid and email as parameters, as well as custom_link to a password reset page in VF 7. Koha sends patron an email containing a link to VF password reset page (instead of Koha OPAC's opac-password-recovery.pl) 8. Patron reads their email and resets password in VF To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/passwordrecovery.t 3. Modify OpacResetPasswordHostWhitelist system preference. Add "anything" to its value. 4. Send a POST request to /api/v1/patrons/password/recovery with parameters "email", "userid" and "custom_link" (custom_link value e.g. "https://anything/reset-password.pl?token={uuid}") 5. You should get a HTTP 201 response 6. Check your message_queue. Your password recovery email should now contain the link you provided in step 4 7. Repeat step 4 but this time use something else as custom_link value 8. You should get a response that this host name is not accepted Created attachment 66419 [details] [review] Bug 19133-2: Allow third party services to send password recovery emails instead of Koha Adds a new parameter "skip_mail" to POST /api/v1/patrons/password/recovery. This allows third party services to avoid Koha enqueueing recovery email to message queue. Instead, letter content and the link containing uuid is returned. Requires permission to do so. Adds a new permission borrowers.get_password_reset_uuid that allows the above. To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/passwordrecovery.t 3. Add yourself borrowers.get_password_reset_uuid permission 4. Send POST request to /api/v1/patrons/password/recovery containing a JSON body { "email": "patrons@email.com", "userid": "patrons userid", "custom_link": "https://allowed/reset.pl?token={uuid}", "skip_mail": true } (Make sure custom link is allowed by the system preference introduced in previous patch) 5. Check that the message contents and other related data is returned 6. Check borrower_password_recovery table - there should be a new entry for you 7. Check message_queue table - this message should not be enqueued Needs a rebase, above patches have dependencies that are blocked and can be removed. Will get back to this later. Created attachment 94551 [details] [review] Bug 19133: Password recovery via REST API This patch adds password recovery functionality to REST API. POST /public/patrons/password/recovery Creates a new password recovery request. Takes a JSON body with "email" and one of "userid" or "cardnumber" as parameters. Sends patron PASSWORD_RESET email. POST /public/patrons/password/recovery/complete Completes pending password recovery. Takes a JSON body with "uuid", "new_password" and "confirm_new_password" as parameters. If passwords meet Koha's password requirements, changes patron's password to the requested new password. To test: 1. prove t/db_dependent/api/v1/patrons_password_recovery.t 2. Send POST requests to the endpoints listed above. See the description of endpoints above for required parameters. 3. Check message_queue (or your email if you actually send messages in the queue) to make sure your password recovery was enqueued 4. Observe your password has been changed once you completed the recovery process Sponsored-by: Koha-Suomi Oy Created attachment 94553 [details] [review] Bug 19133: Add an ability to link patron to third party service to reset their password Adds additional functionality to POST /public/patrons/password/recovery. Pass a new parameter "custom_link" to send patron a recovery email that contains this link instead of Koha OPAC's opac-password-recovery.pl page. This way third party services can link Koha patrons to their own service to have passwords recovered. By default, forbids all custom links. To enable specific services, use the new system preference OpacResetPasswordHostWhitelist. This preference contains a list of allowed host names to which it is allowed to link the patron in recovery email. Use case in steps (to understand the feature better): 1. Library uses a discovery software (e.g. VuFind, from now on VF) and has disabled Koha's OPAC 2. VF uses Koha REST API 3. Patrons can login to VF with their Koha userid and password 4. Patron goes to VF login screen, but has forgotten their password 5. Patron requests their password to be reset in VF 6. VF sends a POST request to /api/v1/patrons/password/recovery with patron's userid and email as parameters, as well as custom_link to a password reset page in VF 7. Koha sends patron an email containing a link to VF password reset page (instead of Koha OPAC's opac-password-recovery.pl) 8. Patron reads their email and resets password in VF To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Modify OpacResetPasswordHostWhitelist system preference. Add "anything" to its value. 4. Send a POST request to /api/v1/public/atrons/password/recovery with parameters "email", "userid" and "custom_link" (custom_link value e.g. "https://anything/reset-password.pl?token={uuid}") 5. You should get a HTTP 201 response 6. Check your message_queue. Your password recovery email should now contain the link you provided in step 4 7. Repeat step 4 but this time use something else as custom_link value 8. You should get a response that this host name is not accepted Sponsored-by: Koha-Suomi Oy Created attachment 94554 [details] [review] Bug 19133: Allow third party services to send password recovery emails instead of Koha Adds a new parameter "skip_mail" to POST /api/v1/public/patrons/password/recovery. This allows third party services to avoid Koha enqueueing recovery email to message queue. Instead, letter content and the link containing uuid is returned. Requires permission to do so. Adds a new permission borrowers.get_password_reset_uuid that allows the above. To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Add yourself borrowers.get_password_reset_uuid permission 4. Send POST request to /api/v1/public/patrons/password/recovery containing a JSON body { "email": "patrons@email.com", "userid": "patrons userid", "custom_link": "https://allowed/reset.pl?token={uuid}", "skip_mail": true } (Make sure custom link is allowed by the system preference introduced in previous patch) 5. Check that the message contents and other related data is returned 6. Check borrower_password_recovery table - there should be a new entry for you 7. Check message_queue table - this message should not be enqueued Sponsored-by: Koha-Suomi Oy Created attachment 112751 [details] [review] Bug 19133: Password recovery via REST API This patch adds password recovery functionality to REST API. POST /public/patrons/password/recovery Creates a new password recovery request. Takes a JSON body with "email" and one of "userid" or "cardnumber" as parameters. Sends patron PASSWORD_RESET email. POST /public/patrons/password/recovery/complete Completes pending password recovery. Takes a JSON body with "uuid", "new_password" and "confirm_new_password" as parameters. If passwords meet Koha's password requirements, changes patron's password to the requested new password. To test: 1. prove t/db_dependent/api/v1/patrons_password_recovery.t 2. Send POST requests to the endpoints listed above. See the description of endpoints above for required parameters. 3. Check message_queue (or your email if you actually send messages in the queue) to make sure your password recovery was enqueued 4. Observe your password has been changed once you completed the recovery process Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112752 [details] [review] Bug 19133: Add an ability to link patron to third party service to reset their password Adds additional functionality to POST /public/patrons/password/recovery. Pass a new parameter "custom_link" to send patron a recovery email that contains this link instead of Koha OPAC's opac-password-recovery.pl page. This way third party services can link Koha patrons to their own service to have passwords recovered. By default, forbids all custom links. To enable specific services, use the new system preference OpacResetPasswordHostWhitelist. This preference contains a list of allowed host names to which it is allowed to link the patron in recovery email. Use case in steps (to understand the feature better): 1. Library uses a discovery software (e.g. VuFind, from now on VF) and has disabled Koha's OPAC 2. VF uses Koha REST API 3. Patrons can login to VF with their Koha userid and password 4. Patron goes to VF login screen, but has forgotten their password 5. Patron requests their password to be reset in VF 6. VF sends a POST request to /api/v1/patrons/password/recovery with patron's userid and email as parameters, as well as custom_link to a password reset page in VF 7. Koha sends patron an email containing a link to VF password reset page (instead of Koha OPAC's opac-password-recovery.pl) 8. Patron reads their email and resets password in VF To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Modify OpacResetPasswordHostWhitelist system preference. Add "anything" to its value. 4. Send a POST request to /api/v1/public/atrons/password/recovery with parameters "email", "userid" and "custom_link" (custom_link value e.g. "https://anything/reset-password.pl?token={uuid}") 5. You should get a HTTP 201 response 6. Check your message_queue. Your password recovery email should now contain the link you provided in step 4 7. Repeat step 4 but this time use something else as custom_link value 8. You should get a response that this host name is not accepted Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112753 [details] [review] Bug 19133: Allow third party services to send password recovery emails instead of Koha Adds a new parameter "skip_mail" to POST /api/v1/public/patrons/password/recovery. This allows third party services to avoid Koha enqueueing recovery email to message queue. Instead, letter content and the link containing uuid is returned. Requires permission to do so. Adds a new permission borrowers.get_password_reset_uuid that allows the above. To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Add yourself borrowers.get_password_reset_uuid permission 4. Send POST request to /api/v1/public/patrons/password/recovery containing a JSON body { "email": "patrons@email.com", "userid": "patrons userid", "custom_link": "https://allowed/reset.pl?token={uuid}", "skip_mail": true } (Make sure custom link is allowed by the system preference introduced in previous patch) 5. Check that the message contents and other related data is returned 6. Check borrower_password_recovery table - there should be a new entry for you 7. Check message_queue table - this message should not be enqueued Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112755 [details] [review] Bug 19133: (QA follow-up) Remove unrelated change Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112756 [details] [review] Bug 19133: (QA follow-up) More contemporary language Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112757 [details] [review] Bug 19133: (QA follow-up) Correct exception description Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 112758 [details] [review] Bug 19133: Fix tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Ok, I rebased it and made some cleanup. I will try to rewrite the tests with newer style and stuff. If this gets a sign-off, I will do a final QA round with the tests changes I proposed. QA question: POST recovery/complete vs POST recover/completion completion reads more RESTful. Does the recovery_request have an id? maybe the recovery_request is the one to get the completion: POST /recovery_request/<ID/token>/completion I'd prefer that. Looking forward for your comments to move forward. This is also missing some tests for the Recovery.pm changes (url param). Created attachment 112937 [details] [review] Bug 19133: Password recovery via REST API This patch adds password recovery functionality to REST API. POST /public/patrons/password/recovery Creates a new password recovery request. Takes a JSON body with "email" and one of "userid" or "cardnumber" as parameters. Sends patron PASSWORD_RESET email. POST /public/patrons/password/recovery/complete Completes pending password recovery. Takes a JSON body with "uuid", "new_password" and "confirm_new_password" as parameters. If passwords meet Koha's password requirements, changes patron's password to the requested new password. To test: 1. prove t/db_dependent/api/v1/patrons_password_recovery.t 2. Send POST requests to the endpoints listed above. See the description of endpoints above for required parameters. 3. Check message_queue (or your email if you actually send messages in the queue) to make sure your password recovery was enqueued 4. Observe your password has been changed once you completed the recovery process Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112938 [details] [review] Bug 19133: Add an ability to link patron to third party service to reset their password Adds additional functionality to POST /public/patrons/password/recovery. Pass a new parameter "custom_link" to send patron a recovery email that contains this link instead of Koha OPAC's opac-password-recovery.pl page. This way third party services can link Koha patrons to their own service to have passwords recovered. By default, forbids all custom links. To enable specific services, use the new system preference OpacResetPasswordHostWhitelist. This preference contains a list of allowed host names to which it is allowed to link the patron in recovery email. Use case in steps (to understand the feature better): 1. Library uses a discovery software (e.g. VuFind, from now on VF) and has disabled Koha's OPAC 2. VF uses Koha REST API 3. Patrons can login to VF with their Koha userid and password 4. Patron goes to VF login screen, but has forgotten their password 5. Patron requests their password to be reset in VF 6. VF sends a POST request to /api/v1/patrons/password/recovery with patron's userid and email as parameters, as well as custom_link to a password reset page in VF 7. Koha sends patron an email containing a link to VF password reset page (instead of Koha OPAC's opac-password-recovery.pl) 8. Patron reads their email and resets password in VF To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Modify OpacResetPasswordHostWhitelist system preference. Add "anything" to its value. 4. Send a POST request to /api/v1/public/atrons/password/recovery with parameters "email", "userid" and "custom_link" (custom_link value e.g. "https://anything/reset-password.pl?token={uuid}") 5. You should get a HTTP 201 response 6. Check your message_queue. Your password recovery email should now contain the link you provided in step 4 7. Repeat step 4 but this time use something else as custom_link value 8. You should get a response that this host name is not accepted Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112939 [details] [review] Bug 19133: Allow third party services to send password recovery emails instead of Koha Adds a new parameter "skip_mail" to POST /api/v1/public/patrons/password/recovery. This allows third party services to avoid Koha enqueueing recovery email to message queue. Instead, letter content and the link containing uuid is returned. Requires permission to do so. Adds a new permission borrowers.get_password_reset_uuid that allows the above. To test: 1. Apply patch and run updatedatabase.pl 2. prove t/db_dependent/api/v1/patrons_password_recovery.t 3. Add yourself borrowers.get_password_reset_uuid permission 4. Send POST request to /api/v1/public/patrons/password/recovery containing a JSON body { "email": "patrons@email.com", "userid": "patrons userid", "custom_link": "https://allowed/reset.pl?token={uuid}", "skip_mail": true } (Make sure custom link is allowed by the system preference introduced in previous patch) 5. Check that the message contents and other related data is returned 6. Check borrower_password_recovery table - there should be a new entry for you 7. Check message_queue table - this message should not be enqueued Sponsored-by: Koha-Suomi Oy Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112940 [details] [review] Bug 19133: (QA follow-up) Remove unrelated change Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112941 [details] [review] Bug 19133: (QA follow-up) More contemporary language Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112942 [details] [review] Bug 19133: (QA follow-up) Correct exception description Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112943 [details] [review] Bug 19133: Fix tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 112944 [details] [review] Bug 19133: (QA follow-up) Fix QA script issues Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to Tomás Cohen Arazi from comment #18) > This is also missing some tests for the Recovery.pm changes (url param). Hi Tomas, and Kyle, nice to see you testing and QA'ing this. I can confirm the missing tests - Recovery.pm::SendPasswordRecoveryEmail() is missing tests for the changes introduced in the second and third patch (url and skip_mail parameters). The main functionality is tested in the REST tests but of course we should still add unit tests to SendPasswordRecoveryEmail itself. (In reply to Lari Taskula from comment #27) > (In reply to Tomás Cohen Arazi from comment #18) > > This is also missing some tests for the Recovery.pm changes (url param). > > Hi Tomas, and Kyle, nice to see you testing and QA'ing this. I can confirm > the missing tests - Recovery.pm::SendPasswordRecoveryEmail() is missing > tests for the changes introduced in the second and third patch (url and > skip_mail parameters). The main functionality is tested in the REST tests > but of course we should still add unit tests to SendPasswordRecoveryEmail > itself. We are almost on feature freeze. If the RM would consider this for inclusion (I would include it, as it is not going to break any existing feature) I can help with the missing tests. 1. I am getting an horrible stack of errors when running the tests: $ prove t/db_dependent/api/v1/patrons_password_recovery.t t/db_dependent/api/v1/patrons_password_recovery.t .. unable to establish SMTP connection to localhost port 25 Trace begun at /usr/share/perl5/Email/Sender/Transport/SMTP.pm line 194 Email::Sender::Transport::SMTP::_throw('Email::Sender::Transport::SMTP=HASH(0x55ca6a314d18)', 'unable to establish SMTP connection to localhost port 25') called at /usr/share/perl5/Email/Sender/Transport/SMTP.pm line 144 [...] 2. This cannot be pushed without missing tests. 3. + unless (C4::Context->preference('OpacPasswordChange') and + C4::Context->preference('OpacPasswordReset')) + { + return $c->render(status => 403, openapi => { + error => 'Password recovery is disabled.' + }); + } hum? OpacPasswordReset never existed, it's OpacResetPassword. But I think $patron->category->can_reset_password (from bug 21890) should be used here instead. Created attachment 113328 [details] [review] Bug 19133: Fix atomic updates |