| Summary: | IdP auto-register should work on the staff interface | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | Authentication | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | Signed Off --- | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | dcook, dpavlin, martin.renvoize, nick, tomascohen |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 37711: DB update
Bug 37711: DBIC schema Bug 37711: API changes Bug 37711: Domains CRUD update Bug 37711: Add support for staff interface auto-register on IdP Bug 37711: DB update Bug 37711: DBIC schema Bug 37711: API changes Bug 37711: Domains CRUD update Bug 37711: Add support for staff interface auto-register on IdP |
||
|
Description
Tomás Cohen Arazi (tcohen)
2024-08-22 14:49:58 UTC
Created attachment 170643 [details] [review] Bug 37711: DB update Created attachment 170644 [details] [review] Bug 37711: DBIC schema Created attachment 170645 [details] [review] Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Created attachment 170646 [details] [review] Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Created attachment 170647 [details] [review] Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Created attachment 174090 [details] [review] Bug 37711: DB update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 174091 [details] [review] Bug 37711: DBIC schema Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 174092 [details] [review] Bug 37711: API changes This patch makes the API representation of the identity providers domains match the new metadata. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 174093 [details] [review] Bug 37711: Domains CRUD update This patch adapts the `Identity provider domains` CRUD page to handle the new configuration options. To test: 1. Have an IdP with some domains set. Make sure there's at least one domain with auto-register enabled and one with it disabled. 2. Apply this patches 3. Run: $ ktd --shell k$ updatedatabase => SUCCESS: Updates successfully 4. Run: k$ yarn build 5. Refresh the IdP domains page => SUCCESS: The domains table shows both OPAC and staff auto-register options. => SUCCESS: The staff auto-register is disabled for both domains => SUCCESS: The OPAC auto-register matches what was set for each domain regarding auto-register (remember that before this patchset, auto-register only works for the OPAC). 6. Play with editing/adding/removing domain configurations => SUCCESS: Things work as expected, values are persisted on the DB correctly. 7. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 174094 [details] [review] Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> All working well in testing.. permissions mapping could be fun but is for a followup bug I reckon. (In reply to Martin Renvoize (ashimema) from comment #11) > All working well in testing.. permissions mapping could be fun but is for a > followup bug I reckon. Agreed. That one is scarier... |