Bug 29926 - Add ability for superlibrarians to edit password expiration dates
Summary: Add ability for superlibrarians to edit password expiration dates
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 29924
Blocks:
  Show dependency treegraph
 
Reported: 2022-01-21 17:20 UTC by Nick Clemens
Modified: 2022-08-30 15:45 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 29926: Add pasword expiration route for API (8.29 KB, patch)
2022-02-14 16:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarian to view/edit password expiration (7.83 KB, patch)
2022-02-14 16:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates (6.07 KB, patch)
2022-02-14 16:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add pasword expiration route for API (8.34 KB, patch)
2022-02-16 17:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarian to view/edit password expiration (7.88 KB, patch)
2022-02-16 17:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates (6.12 KB, patch)
2022-02-16 17:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29926: Add pasword expiration route for API (8.41 KB, patch)
2022-04-25 12:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarian to view/edit password expiration (7.88 KB, patch)
2022-04-25 12:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates (6.11 KB, patch)
2022-04-25 12:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29926: Add pasword expiration route for API (8.48 KB, patch)
2022-04-26 13:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarian to view/edit password expiration (7.94 KB, patch)
2022-04-26 13:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29926: Add ability for superlibrarians to batch edit password expiration dates (6.17 KB, patch)
2022-04-26 13:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29926: (QA follow-up) API design fixes (7.50 KB, patch)
2022-04-26 13:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29926: (QA follow-up) Date format missing in spec (2.80 KB, patch)
2022-04-26 13:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 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 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 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 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 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 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 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 🦄