Bug 29926

Summary: Add ability for superlibrarians to edit password expiration dates
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: AuthenticationAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: bugzilla, caroline.cyr-la-rose, dpavlin, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29925
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/585
Text to go in the release notes:
Version(s) released in:
22.05.00
Bug Depends on: 29924    
Bug Blocks:    
Attachments: Bug 29926: Add pasword expiration route for API
Bug 29926: Add ability for superlibrarian to view/edit password expiration
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates
Bug 29926: Add pasword expiration route for API
Bug 29926: Add ability for superlibrarian to view/edit password expiration
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates
Bug 29926: Add pasword expiration route for API
Bug 29926: Add ability for superlibrarian to view/edit password expiration
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates
Bug 29926: Add pasword expiration route for API
Bug 29926: Add ability for superlibrarian to view/edit password expiration
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates
Bug 29926: (QA follow-up) API design fixes
Bug 29926: (QA follow-up) Date format missing in spec

Description Nick Clemens (kidclamp) 2022-01-21 17:20:17 UTC
For single patrons and for patches we should allow the superlibrarian to set/update password expiration dates
Comment 1 Nick Clemens (kidclamp) 2022-02-14 16:41:32 UTC
Created attachment 130551 [details] [review]
Bug 29926: Add pasword expiration route for API

To test:
1 - prove -v t/db_dependent/api/v1/patrons_password_expiration.t
Comment 2 Nick Clemens (kidclamp) 2022-02-14 16:41:35 UTC
Created attachment 130552 [details] [review]
Bug 29926: Add ability for superlibrarian to view/edit password expiration

To  test:
 1 - Sign in as a superlibrarian
 2 - Find a patron account with no password expiration set
 3 - View member detials
 4 - note expiration says 'Never'
 5 - Edit patron
 6 - Set patron expiration
 7- Save
 8 - View details, confirm password expiration shows correctly
 9 - Sign in as non-superlibrarian
10 - Confirm you don't see expirationdate on details page
11 - Edit patron and confirm password expiration does not show
12 - Edit HTML and confirm you epxiration date not saved
     <input type="text" name="password_expiration_date" value="2052-05-02">
Comment 3 Nick Clemens (kidclamp) 2022-02-14 16:41:38 UTC
Created attachment 130553 [details] [review]
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates

To test:
1 - Sign in as a superlibrarian
2 - Tools->Batch patron modification
3 - Add patrons with and without pasword expiration set
4 - Confirm dates display correctly on modification page, or show never
5 - Check the box to delete all dates
6 - Confirm patrons now show 'Never'
7 - Edit patrons again, set a date
8 - Confirm date is set correctly
9 - Sign in as a user with catalogue/tools/borrowers permissions, but not superlibrarian
10 - Confirm y9ou do not see, and cannnot edit password expiration dates
Comment 4 Andrew Fuerste-Henry 2022-02-16 17:22:19 UTC
Created attachment 130686 [details] [review]
Bug 29926: Add pasword expiration route for API

To test:
1 - prove -v t/db_dependent/api/v1/patrons_password_expiration.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 5 Andrew Fuerste-Henry 2022-02-16 17:22:23 UTC
Created attachment 130687 [details] [review]
Bug 29926: Add ability for superlibrarian to view/edit password expiration

To  test:
 1 - Sign in as a superlibrarian
 2 - Find a patron account with no password expiration set
 3 - View member detials
 4 - note expiration says 'Never'
 5 - Edit patron
 6 - Set patron expiration
 7- Save
 8 - View details, confirm password expiration shows correctly
 9 - Sign in as non-superlibrarian
10 - Confirm you don't see expirationdate on details page
11 - Edit patron and confirm password expiration does not show
12 - Edit HTML and confirm you epxiration date not saved
     <input type="text" name="password_expiration_date" value="2052-05-02">

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 6 Andrew Fuerste-Henry 2022-02-16 17:22:26 UTC
Created attachment 130688 [details] [review]
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates

To test:
1 - Sign in as a superlibrarian
2 - Tools->Batch patron modification
3 - Add patrons with and without pasword expiration set
4 - Confirm dates display correctly on modification page, or show never
5 - Check the box to delete all dates
6 - Confirm patrons now show 'Never'
7 - Edit patrons again, set a date
8 - Confirm date is set correctly
9 - Sign in as a user with catalogue/tools/borrowers permissions, but not superlibrarian
10 - Confirm y9ou do not see, and cannnot edit password expiration dates

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 7 Katrin Fischer 2022-04-24 11:38:28 UTC
The patch currently doesn't apply, can you please fix?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29926: Add pasword expiration route for API
Using index info to reconstruct a base tree...
A	api/v1/swagger/paths.yaml
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): api/v1/swagger/paths.yaml deleted in HEAD and modified in Bug 29926: Add pasword expiration route for API. Version Bug 29926: Add pasword expiration route for API of api/v1/swagger/paths.yaml left in tree.
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29926: Add pasword expiration route for API
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-29926-Add-pasword-expiration-route-for-API-QJheMn.patch


Wondering: This is kind of an "undocumented" extra for the superlibrarians, that is not covered by any of the permissions. How can we document or should this not better be a new permission?
Comment 8 Nick Clemens (kidclamp) 2022-04-25 12:05:21 UTC
Created attachment 133759 [details] [review]
Bug 29926: Add pasword expiration route for API

To test:
1 - prove -v t/db_dependent/api/v1/patrons_password_expiration.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 9 Nick Clemens (kidclamp) 2022-04-25 12:05:25 UTC
Created attachment 133760 [details] [review]
Bug 29926: Add ability for superlibrarian to view/edit password expiration

To  test:
 1 - Sign in as a superlibrarian
 2 - Find a patron account with no password expiration set
 3 - View member detials
 4 - note expiration says 'Never'
 5 - Edit patron
 6 - Set patron expiration
 7- Save
 8 - View details, confirm password expiration shows correctly
 9 - Sign in as non-superlibrarian
10 - Confirm you don't see expirationdate on details page
11 - Edit patron and confirm password expiration does not show
12 - Edit HTML and confirm you epxiration date not saved
     <input type="text" name="password_expiration_date" value="2052-05-02">

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 10 Nick Clemens (kidclamp) 2022-04-25 12:05:29 UTC
Created attachment 133761 [details] [review]
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates

To test:
1 - Sign in as a superlibrarian
2 - Tools->Batch patron modification
3 - Add patrons with and without pasword expiration set
4 - Confirm dates display correctly on modification page, or show never
5 - Check the box to delete all dates
6 - Confirm patrons now show 'Never'
7 - Edit patrons again, set a date
8 - Confirm date is set correctly
9 - Sign in as a user with catalogue/tools/borrowers permissions, but not superlibrarian
10 - Confirm y9ou do not see, and cannnot edit password expiration dates

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Comment 11 Tomás Cohen Arazi 2022-04-26 13:13:46 UTC
Created attachment 133887 [details] [review]
Bug 29926: Add pasword expiration route for API

To test:
1 - prove -v t/db_dependent/api/v1/patrons_password_expiration.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2022-04-26 13:13:57 UTC
Created attachment 133888 [details] [review]
Bug 29926: Add ability for superlibrarian to view/edit password expiration

To  test:
 1 - Sign in as a superlibrarian
 2 - Find a patron account with no password expiration set
 3 - View member detials
 4 - note expiration says 'Never'
 5 - Edit patron
 6 - Set patron expiration
 7- Save
 8 - View details, confirm password expiration shows correctly
 9 - Sign in as non-superlibrarian
10 - Confirm you don't see expirationdate on details page
11 - Edit patron and confirm password expiration does not show
12 - Edit HTML and confirm you epxiration date not saved
     <input type="text" name="password_expiration_date" value="2052-05-02">

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2022-04-26 13:14:03 UTC
Created attachment 133889 [details] [review]
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates

To test:
1 - Sign in as a superlibrarian
2 - Tools->Batch patron modification
3 - Add patrons with and without pasword expiration set
4 - Confirm dates display correctly on modification page, or show never
5 - Check the box to delete all dates
6 - Confirm patrons now show 'Never'
7 - Edit patrons again, set a date
8 - Confirm date is set correctly
9 - Sign in as a user with catalogue/tools/borrowers permissions, but not superlibrarian
10 - Confirm y9ou do not see, and cannnot edit password expiration dates

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2022-04-26 13:14:09 UTC
Created attachment 133890 [details] [review]
Bug 29926: (QA follow-up) API design fixes

This patch makes the following changes to the spec:

* Password being the resource and expiration_date an attribute for it,
  so reorganizing things and also renaming the route.
* Be it undefined or defined, expiration date is only one and thus
  should use the PUT verb (as in overwrite).
* Minor bug 30194-related fixes.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2022-04-26 13:14:15 UTC
Created attachment 133891 [details] [review]
Bug 29926: (QA follow-up) Date format missing in spec

On fixing the spec to have `format: date` I noticed the tests were
expecting explosions because of date handling not being done. The
OpenAPI plugin does this correctly when you set the format right.

So, I adapted the tests so they expect 400 and return the type error.

We don't usually add such tests (i.e. test the plugin does its job
correctly) but it doesn't hurt to keep them just in case something
really changes badly there (plugin bug?).

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Fridolin Somers 2022-05-06 20:34:20 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄