Bug 37711 - IdP auto-register should work on the staff interface
Summary: IdP auto-register should work on the staff interface
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: David Cook
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-22 14:49 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-09-30 01:30 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
The 'auto-register' feature can now be enabled in the staff interface. Previously, this functionality was only available in the OPAC and could not be used from the staff side.
Version(s) released in:
Circulation function:


Attachments
Bug 37711: DB update (4.14 KB, patch)
2024-08-23 15:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DBIC schema (2.82 KB, patch)
2024-08-23 15:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: API changes (3.02 KB, patch)
2024-08-23 15:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (9.71 KB, patch)
2024-08-23 15:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (10.37 KB, patch)
2024-08-23 15:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DB update (4.21 KB, patch)
2024-11-06 17:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37711: DBIC schema (2.88 KB, patch)
2024-11-06 17:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37711: API changes (3.08 KB, patch)
2024-11-06 17:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (9.78 KB, patch)
2024-11-06 17:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (10.44 KB, patch)
2024-11-06 17:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37711: DB update (4.24 KB, patch)
2025-08-07 20:44 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DBIC schema (2.89 KB, patch)
2025-08-07 20:44 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: API changes (3.08 KB, patch)
2025-08-07 20:44 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (11.10 KB, patch)
2025-08-07 20:44 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (9.32 KB, patch)
2025-08-07 20:45 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DB update (4.25 KB, patch)
2025-09-26 15:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DBIC schema (2.88 KB, patch)
2025-09-26 15:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: API changes (3.08 KB, patch)
2025-09-26 15:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (10.72 KB, patch)
2025-09-26 15:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (9.31 KB, patch)
2025-09-26 15:17 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (11.95 KB, patch)
2025-09-26 17:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (9.31 KB, patch)
2025-09-26 17:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (13.95 KB, patch)
2025-09-29 12:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (13.95 KB, patch)
2025-09-29 13:02 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Test patch description (13.95 KB, patch)
2025-09-29 13:22 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Test patch description 2 (13.95 KB, patch)
2025-09-29 13:23 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Test patch description 1 (13.95 KB, patch)
2025-09-29 13:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Test patch description 2 (13.95 KB, patch)
2025-09-29 13:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Test patch description 3 (13.95 KB, patch)
2025-09-29 13:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Debug test (13.95 KB, patch)
2025-09-29 13:24 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 37711: DB update (4.30 KB, patch)
2025-09-30 01:29 UTC, David Cook
Details | Diff | Splinter Review
Bug 37711: DBIC schema (2.94 KB, patch)
2025-09-30 01:30 UTC, David Cook
Details | Diff | Splinter Review
Bug 37711: API changes (3.13 KB, patch)
2025-09-30 01:30 UTC, David Cook
Details | Diff | Splinter Review
Bug 37711: Add support for staff interface auto-register on IdP (9.37 KB, patch)
2025-09-30 01:30 UTC, David Cook
Details | Diff | Splinter Review
Bug 37711: Domains CRUD update (14.00 KB, patch)
2025-09-30 01:30 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-08-22 14:49:58 UTC
Right now it only works if the login happens in the OPAC.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-08-23 15:29:18 UTC
Created attachment 170643 [details] [review]
Bug 37711: DB update
Comment 2 Tomás Cohen Arazi (tcohen) 2024-08-23 15:29:20 UTC
Created attachment 170644 [details] [review]
Bug 37711: DBIC schema
Comment 3 Tomás Cohen Arazi (tcohen) 2024-08-23 15:29:23 UTC
Created attachment 170645 [details] [review]
Bug 37711: API changes

This patch makes the API representation of the identity providers
domains match the new metadata.
Comment 4 Tomás Cohen Arazi (tcohen) 2024-08-23 15:29:25 UTC
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
Comment 5 Tomás Cohen Arazi (tcohen) 2024-08-23 15:29:28 UTC
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
Comment 6 Martin Renvoize (ashimema) 2024-11-06 17:25:45 UTC
Created attachment 174090 [details] [review]
Bug 37711: DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-11-06 17:25:47 UTC
Created attachment 174091 [details] [review]
Bug 37711: DBIC schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2024-11-06 17:25:50 UTC
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>
Comment 9 Martin Renvoize (ashimema) 2024-11-06 17:25:52 UTC
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>
Comment 10 Martin Renvoize (ashimema) 2024-11-06 17:25:55 UTC
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>
Comment 11 Martin Renvoize (ashimema) 2024-11-06 17:26:25 UTC
All working well in testing.. permissions mapping could be fun but is for a followup bug I reckon.
Comment 12 David Cook 2024-11-06 22:53:40 UTC
(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...
Comment 13 Tomás Cohen Arazi (tcohen) 2025-08-07 20:44:49 UTC
Created attachment 185244 [details] [review]
Bug 37711: DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Tomás Cohen Arazi (tcohen) 2025-08-07 20:44:53 UTC
Created attachment 185245 [details] [review]
Bug 37711: DBIC schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Tomás Cohen Arazi (tcohen) 2025-08-07 20:44:56 UTC
Created attachment 185246 [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>
Comment 16 Tomás Cohen Arazi (tcohen) 2025-08-07 20:44:59 UTC
Created attachment 185247 [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>
Comment 17 Tomás Cohen Arazi (tcohen) 2025-08-07 20:45:02 UTC
Created attachment 185248 [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>
Comment 18 Tomás Cohen Arazi (tcohen) 2025-08-07 20:45:30 UTC
Rebased.
Comment 19 Martin Renvoize (ashimema) 2025-09-22 21:21:53 UTC
Hmm.. It's been a while.. I'm going to upgrade my SO to a straight QA as there's seems to be hardly anyone with the knowledge to dig in and QA here.
Comment 20 David Cook 2025-09-23 01:37:30 UTC
(In reply to Martin Renvoize (ashimema) from comment #19)
> Hmm.. It's been a while.. I'm going to upgrade my SO to a straight QA as
> there's seems to be hardly anyone with the knowledge to dig in and QA here.

It's been tough to make this one a priority since my libraries just need self-registration via the OPAC, but I do have the knowledge... I'll take a little look too just to double-check...

My only real concern was the permissions thing, but I think that's left out here, fortunately.
Comment 21 David Cook 2025-09-23 01:42:15 UTC
Failing QA, as there's an inconsistency between the atomic update and kohastructure.sql
Comment 22 David Cook 2025-09-23 01:43:31 UTC
Comment on attachment 185244 [details] [review]
Bug 37711: DB update

Review of attachment 185244 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_37711.pl
@@ +22,5 @@
> +        if ( !column_exists( 'identity_provider_domains', 'auto_register_staff' ) ) {
> +            $dbh->do(
> +                q{
> +                ALTER TABLE identity_provider_domains
> +                    ADD COLUMN `auto_register_staff` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Allow user auto register (OPAC)'

The comment here should be:
'Allow user auto register (Staff interface)'
Comment 23 David Cook 2025-09-23 01:46:40 UTC
Comment on attachment 185247 [details] [review]
Bug 37711: Domains CRUD update

Review of attachment 185247 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt
@@ +322,5 @@
>          </div>
>      [% END %]
>  
> +    <div id="toolbar" class="btn-toolbar">
> +        <a class="btn btn-default" id="new_identity_provider_domain" href="/cgi-bin/koha/admin/identity_providers.pl?domain_ops=1&amp;identity_provider_id=[%- identity_provider_id | html -%]&amp;op=add_form"

I thought that we were using & instead of &amp; these days, but not really an issue...
Comment 24 David Cook 2025-09-23 03:02:43 UTC
Sorry I was going to keep testing but I've got caught up in other things...
Comment 25 David Cook 2025-09-23 04:34:23 UTC
Failed QA for a few more things:

- If you select "Auto register" "Yes" on http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form, and then you go to review the domain for that IdP, it'll say "Auto register (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow".

- When viewing the "Identity provider domains for '<idp>'", there are 2 tables. One appears to be the old table format which  has only 1 column "Auto register". This is the table that is populated. Beneath that table there is another one with the correct 2 "Auto register" columns, but no data is provided.
Comment 26 David Cook 2025-09-23 04:35:39 UTC
(In reply to David Cook from comment #25)
> - When viewing the "Identity provider domains for '<idp>'", there are 2
> tables. One appears to be the old table format which  has only 1 column
> "Auto register". This is the table that is populated. Beneath that table
> there is another one with the correct 2 "Auto register" columns, but no data
> is provided.

Note that this 2nd table appears correctly when you go into edit a particular domain for an IdP, so this is probably an easy fix on the listing view page
Comment 27 Tomás Cohen Arazi (tcohen) 2025-09-23 14:49:31 UTC
Thanks for the valuable feedback, dcook!
Comment 28 David Cook 2025-09-25 00:44:20 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #27)
> Thanks for the valuable feedback, dcook!

No worries. I tried 😅
Comment 29 Tomás Cohen Arazi (tcohen) 2025-09-26 15:04:04 UTC
(In reply to David Cook from comment #25)
> Failed QA for a few more things:
> 
> - If you select "Auto register" "Yes" on
> http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form,
> and then you go to review the domain for that IdP, it'll say "Auto register
> (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow".
> 
> - When viewing the "Identity provider domains for '<idp>'", there are 2
> tables. One appears to be the old table format which  has only 1 column
> "Auto register". This is the table that is populated. Beneath that table
> there is another one with the correct 2 "Auto register" columns, but no data
> is provided.

Ooof. That's a terrible rebase :-D
Comment 30 Tomás Cohen Arazi (tcohen) 2025-09-26 15:17:31 UTC
Created attachment 186978 [details] [review]
Bug 37711: DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 31 Tomás Cohen Arazi (tcohen) 2025-09-26 15:17:34 UTC
Created attachment 186979 [details] [review]
Bug 37711: DBIC schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 32 Tomás Cohen Arazi (tcohen) 2025-09-26 15:17:37 UTC
Created attachment 186980 [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>
Comment 33 Tomás Cohen Arazi (tcohen) 2025-09-26 15:17:40 UTC
Created attachment 186981 [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>
Comment 34 Tomás Cohen Arazi (tcohen) 2025-09-26 15:17:44 UTC
Created attachment 186982 [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>
Comment 35 Tomás Cohen Arazi (tcohen) 2025-09-26 15:20:09 UTC
(In reply to David Cook from comment #22)
> > +                ALTER TABLE identity_provider_domains
> > +                    ADD COLUMN `auto_register_staff` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Allow user auto register (OPAC)'
> 
> The comment here should be:
> 'Allow user auto register (Staff interface)'

Fixed inline. Good catch!

> > +        <a class="btn btn-default" id="new_identity_provider_domain" href="/cgi-bin/koha/admin/identity_providers.pl?domain_ops=1&amp;identity_provider_id=[%- identity_provider_id | html -%]&amp;op=add_form"
> 
> I thought that we were using & instead of &amp; these days, but not really
> an issue...

The file is full of those. Maybe a separate bug to review how we build URLs globally and add a check somewhere?

> - If you select "Auto register" "Yes" on
> http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form,
> and then you go to review the domain for that IdP, it'll say "Auto register
> (OPAC):" "Don't allow" and "Auto register (Staff)" "Don't allow".
> 
> - When viewing the "Identity provider domains for '<idp>'", there are 2
> tables. One appears to be the old table format which  has only 1 column
> "Auto register". This is the table that is populated. Beneath that table
> there is another one with the correct 2 "Auto register" columns, but no data
> is provided.

Bad rebase. Solved.
Comment 36 Tomás Cohen Arazi (tcohen) 2025-09-26 17:56:13 UTC
Created attachment 187000 [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>
Comment 37 Tomás Cohen Arazi (tcohen) 2025-09-26 17:56:17 UTC
Created attachment 187001 [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>
Comment 38 David Cook 2025-09-29 06:07:49 UTC
Marking "Failed QA".

When adding an identity provider for the first time via http://localhost:8081/cgi-bin/koha/admin/identity_providers.pl?op=add_form there is only one "Auto register" option.

If you select "Yes", when you go to review the domains for that IdP, you'll see that it says "No" for both "Auto register (OPAC)" and "Auto register (Staff)".

--

Other than that... I think it's all good. Once that last bit is fixed, I think it'll be good to go. The functionality all works as expected otherwise.
Comment 39 Tomás Cohen Arazi (tcohen) 2025-09-29 12:37:31 UTC
Created attachment 187034 [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>
Comment 40 Tomás Cohen Arazi (tcohen) 2025-09-29 13:02:28 UTC
Created attachment 187037 [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>
Comment 41 Tomás Cohen Arazi (tcohen) 2025-09-29 13:22:39 UTC Comment hidden (obsolete)
Comment 42 Tomás Cohen Arazi (tcohen) 2025-09-29 13:23:05 UTC Comment hidden (obsolete)
Comment 43 Tomás Cohen Arazi (tcohen) 2025-09-29 13:24:07 UTC Comment hidden (obsolete)
Comment 44 Tomás Cohen Arazi (tcohen) 2025-09-29 13:24:18 UTC Comment hidden (obsolete)
Comment 45 Tomás Cohen Arazi (tcohen) 2025-09-29 13:24:28 UTC Comment hidden (obsolete)
Comment 46 Tomás Cohen Arazi (tcohen) 2025-09-29 13:24:49 UTC Comment hidden (obsolete)
Comment 47 Tomás Cohen Arazi (tcohen) 2025-09-29 13:43:03 UTC
I fixed the issue, @dcook.

Sorry, for the noise, I decided to fix the 'Network error' git-bz was returning and used this bug to debug it.
Comment 48 David Cook 2025-09-30 01:29:57 UTC
Created attachment 187068 [details] [review]
Bug 37711: DB update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 49 David Cook 2025-09-30 01:30:00 UTC
Created attachment 187069 [details] [review]
Bug 37711: DBIC schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 50 David Cook 2025-09-30 01:30:04 UTC
Created attachment 187070 [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>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 51 David Cook 2025-09-30 01:30:08 UTC
Created attachment 187071 [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>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 52 David Cook 2025-09-30 01:30:11 UTC
Created attachment 187072 [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>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 53 David Cook 2025-09-30 01:30:29 UTC
Looking good. Thanks, Tomas!