Bug 19133 - Password recovery routes
Summary: Password recovery routes
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-17 11:17 UTC by Lari Taskula
Modified: 2020-11-09 16:23 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19133: Password recovery via REST API (20.92 KB, patch)
2017-08-24 14:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133: Add an ability to link patron to third party service to reset their password (13.76 KB, patch)
2017-08-24 14:01 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133-2: Allow third party services to send password recovery emails instead of Koha (13.49 KB, patch)
2017-08-24 14:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133: Password recovery via REST API (23.48 KB, patch)
2019-10-22 13:14 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133: Add an ability to link patron to third party service to reset their password (13.95 KB, patch)
2019-10-22 13:14 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133: Allow third party services to send password recovery emails instead of Koha (19.32 KB, patch)
2019-10-22 13:15 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19133: Password recovery via REST API (23.55 KB, patch)
2020-10-30 20:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: Add an ability to link patron to third party service to reset their password (14.28 KB, patch)
2020-10-30 20:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: Allow third party services to send password recovery emails instead of Koha (13.52 KB, patch)
2020-10-30 20:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) Remove unrelated change (1.29 KB, patch)
2020-10-30 20:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) More contemporary language (10.86 KB, patch)
2020-10-30 20:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) Correct exception description (800 bytes, patch)
2020-10-30 20:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: Fix tests (1.52 KB, patch)
2020-10-30 20:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19133: Password recovery via REST API (23.61 KB, patch)
2020-11-03 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: Add an ability to link patron to third party service to reset their password (14.34 KB, patch)
2020-11-03 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: Allow third party services to send password recovery emails instead of Koha (13.57 KB, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) Remove unrelated change (1.36 KB, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) More contemporary language (10.92 KB, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) Correct exception description (870 bytes, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: Fix tests (1.59 KB, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: (QA follow-up) Fix QA script issues (3.19 KB, patch)
2020-11-03 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19133: Fix atomic updates (2.20 KB, patch)
2020-11-09 16:23 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2017-08-17 11:17:12 UTC
Add password recovery functionality to REST API. 

Support third party integrations for reset link: by default, use existing password recovery page in Koha OPAC, but also have the ability to link patron to external system to complete their password recovery.
Comment 1 Lari Taskula 2017-08-24 14:00:41 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
Comment 2 Lari Taskula 2017-08-24 14:01:12 UTC
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
Comment 3 Lari Taskula 2017-08-24 14:02:35 UTC
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
Comment 4 Lari Taskula 2017-08-29 14:36:17 UTC
Needs a rebase, above patches have dependencies that are blocked and can be removed. Will get back to this later.
Comment 5 Lari Taskula 2019-10-22 13:14:43 UTC
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
Comment 6 Lari Taskula 2019-10-22 13:14:57 UTC
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
Comment 7 Lari Taskula 2019-10-22 13:15:27 UTC
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
Comment 8 Tomás Cohen Arazi 2020-10-30 20:02:51 UTC
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>
Comment 9 Tomás Cohen Arazi 2020-10-30 20:02:58 UTC
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>
Comment 10 Tomás Cohen Arazi 2020-10-30 20:03:06 UTC
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>
Comment 11 Tomás Cohen Arazi 2020-10-30 20:03:15 UTC
Created attachment 112755 [details] [review]
Bug 19133: (QA follow-up) Remove unrelated change

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2020-10-30 20:03:24 UTC
Created attachment 112756 [details] [review]
Bug 19133: (QA follow-up) More contemporary language

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2020-10-30 20:03:34 UTC
Created attachment 112757 [details] [review]
Bug 19133: (QA follow-up) Correct exception description

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2020-10-30 20:03:41 UTC
Created attachment 112758 [details] [review]
Bug 19133: Fix tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2020-10-30 20:04:28 UTC
Ok, I rebased it and made some cleanup. I will try to rewrite the tests with newer style and stuff.
Comment 16 Tomás Cohen Arazi 2020-10-30 20:06:24 UTC
If this gets a sign-off, I will do a final QA round with the tests changes I proposed.
Comment 17 Tomás Cohen Arazi 2020-10-30 20:12:19 UTC
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.
Comment 18 Tomás Cohen Arazi 2020-10-30 20:14:31 UTC
This is also missing some tests for the Recovery.pm changes (url param).
Comment 19 Kyle M Hall 2020-11-03 17:31:47 UTC
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>
Comment 20 Kyle M Hall 2020-11-03 17:31:59 UTC
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>
Comment 21 Kyle M Hall 2020-11-03 17:32:03 UTC
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>
Comment 22 Kyle M Hall 2020-11-03 17:32:06 UTC
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>
Comment 23 Kyle M Hall 2020-11-03 17:32:09 UTC
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>
Comment 24 Kyle M Hall 2020-11-03 17:32:12 UTC
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>
Comment 25 Kyle M Hall 2020-11-03 17:32:15 UTC
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>
Comment 26 Kyle M Hall 2020-11-03 17:32:18 UTC
Created attachment 112944 [details] [review]
Bug 19133: (QA follow-up) Fix QA script issues

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Lari Taskula 2020-11-03 19:49:06 UTC
(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.
Comment 28 Tomás Cohen Arazi 2020-11-03 19:51:04 UTC
(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.
Comment 29 Jonathan Druart 2020-11-09 16:22:45 UTC
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.
Comment 30 Jonathan Druart 2020-11-09 16:23:51 UTC
Created attachment 113328 [details] [review]
Bug 19133: Fix atomic updates