Bug 31378 - Add a generic OAuth2/OIDC client implementation
Summary: Add a generic OAuth2/OIDC client implementation
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords:
: 28420 30988 31965 (view as bug list)
Depends on:
Blocks: 31851 31900 32133 33517 33518 34164 35617 32138 32139 32140 32141 32354 33699 33790 33859
  Show dependency treegraph
 
Reported: 2022-08-16 23:09 UTC by Tomás Cohen Arazi
Modified: 2023-12-20 14:34 UTC (History)
20 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/670
Text to go in the release notes:
This feature introduces a way to integrate Koha with any OAuth2/OIDC identity provider. It also prepares the ground for later adding more protocols and prioritizing authentication methods/identity providers.
Version(s) released in:
22.11.00


Attachments
Bug 31378: DB changes (10.12 KB, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.47 KB, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add dependencies (774 bytes, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (2.96 KB, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.45 KB, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.57 KB, patch)
2022-10-05 12:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.83 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.77 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.54 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.04 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.34 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (2.87 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.03 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.86 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (906 bytes, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (48.59 KB, patch)
2022-10-05 12:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (57.97 KB, patch)
2022-10-05 12:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.23 KB, patch)
2022-10-07 14:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: DB changes (10.16 KB, patch)
2022-10-13 09:40 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.50 KB, patch)
2022-10-13 09:40 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add dependencies (813 bytes, patch)
2022-10-13 09:40 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.00 KB, patch)
2022-10-13 09:40 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.49 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.61 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.87 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.81 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.58 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.08 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.38 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (2.91 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.07 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.90 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (946 bytes, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (48.63 KB, patch)
2022-10-13 09:41 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.01 KB, patch)
2022-10-13 09:42 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.27 KB, patch)
2022-10-13 09:42 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 31378: DB changes (10.18 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.52 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add dependencies (827 bytes, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.01 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.50 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.62 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.88 KB, patch)
2022-10-13 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.82 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.59 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.09 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.39 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (2.93 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.08 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.92 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (959 bytes, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (48.64 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.02 KB, patch)
2022-10-13 11:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.29 KB, patch)
2022-10-13 11:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.52 KB, patch)
2022-10-14 02:28 UTC, David Cook
Details | Diff | Splinter Review
Bug 31378: DB changes (10.12 KB, patch)
2022-11-03 02:05 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: DB changes (10.12 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.47 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add dependencies (774 bytes, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (2.96 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.45 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.57 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.83 KB, patch)
2022-11-03 12:35 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.77 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.54 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.04 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.34 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (2.92 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.03 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.86 KB, patch)
2022-11-03 12:36 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (906 bytes, patch)
2022-11-03 12:37 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.40 KB, patch)
2022-11-03 12:37 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (57.97 KB, patch)
2022-11-03 12:37 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.53 KB, patch)
2022-11-03 12:37 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: (follow-up) do all FIXME pointed by David (67.43 KB, patch)
2022-11-03 12:37 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.45 KB, patch)
2022-11-03 12:38 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.53 KB, patch)
2022-11-03 12:38 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: DB changes (10.12 KB, patch)
2022-11-04 12:48 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.47 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add dependencies (774 bytes, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (2.96 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.45 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.57 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.83 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.77 KB, patch)
2022-11-04 12:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.54 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.04 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.34 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (2.92 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.03 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.86 KB, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (906 bytes, patch)
2022-11-04 12:50 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.40 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (57.97 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.53 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: (follow-up) do all FIXME pointed by David (67.43 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.45 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.54 KB, patch)
2022-11-04 12:51 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31378: DB changes (10.23 KB, patch)
2022-11-04 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.58 KB, patch)
2022-11-04 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add dependencies (884 bytes, patch)
2022-11-04 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.07 KB, patch)
2022-11-04 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.57 KB, patch)
2022-11-04 14:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.68 KB, patch)
2022-11-04 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.94 KB, patch)
2022-11-04 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.88 KB, patch)
2022-11-04 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.64 KB, patch)
2022-11-04 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.15 KB, patch)
2022-11-04 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.45 KB, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (3.02 KB, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.14 KB, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.92 KB, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (961 bytes, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.53 KB, patch)
2022-11-04 15:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.09 KB, patch)
2022-11-04 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.61 KB, patch)
2022-11-04 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (follow-up) Fix QA concerns (66.58 KB, patch)
2022-11-04 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.45 KB, patch)
2022-11-04 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.60 KB, patch)
2022-11-04 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (QA follow-up) Tidy tests (31.79 KB, patch)
2022-11-04 15:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: DB changes (10.29 KB, patch)
2022-11-04 18:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.63 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add dependencies (939 bytes, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.12 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.61 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.73 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.99 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.93 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.70 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.20 KB, patch)
2022-11-04 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.50 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (3.08 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.19 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.97 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (1016 bytes, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.56 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.13 KB, patch)
2022-11-04 18:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.59 KB, patch)
2022-11-04 18:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: (follow-up) Fix QA concerns (66.62 KB, patch)
2022-11-04 18:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.50 KB, patch)
2022-11-04 18:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.66 KB, patch)
2022-11-04 18:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: (QA follow-up) Tidy tests (31.84 KB, patch)
2022-11-04 18:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31378: DB changes (10.29 KB, patch)
2022-11-08 14:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.63 KB, patch)
2022-11-08 14:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add dependencies (941 bytes, patch)
2022-11-08 14:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.13 KB, patch)
2022-11-08 14:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.62 KB, patch)
2022-11-08 14:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.74 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (4.99 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.94 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.70 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.21 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.51 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (3.08 KB, patch)
2022-11-08 14:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.20 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (2.97 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (1018 bytes, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.58 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.14 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.66 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (follow-up) Fix QA concerns (66.63 KB, patch)
2022-11-08 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.50 KB, patch)
2022-11-08 14:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.66 KB, patch)
2022-11-08 14:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (QA follow-up) Tidy tests (31.84 KB, patch)
2022-11-08 14:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Add domain settings to IdP add page (11.66 KB, patch)
2022-11-08 14:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: DB changes (10.35 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: DBIC updates [DO NOT PUSH] (2.69 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add dependencies (1003 bytes, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es) (3.19 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Provider(s) class(es) (13.67 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Koha::Auth::Client* modules (9.80 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add Auth mojo plugin (5.05 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add API routes (8.99 KB, patch)
2022-11-08 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add auth providers template plugin (2.76 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth (3.27 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add OPAC support for authentication providers (8.57 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add STAFF support for authentication providers (3.14 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add accessor methods for JSON attributes (9.26 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add manage_authentication_providers permission (3.03 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter (1.05 KB, patch)
2022-11-08 15:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add authentication provider endpoints (47.62 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add an administration interface for authentication sources (58.20 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests (163.65 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: (follow-up) Fix QA concerns (66.68 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Use base URLs for redirect uri (1.57 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: (follow-up) catch startup error when missing Oauth provider table (1.72 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: (QA follow-up) Tidy tests (31.90 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: Add domain settings to IdP add page (11.71 KB, patch)
2022-11-08 15:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: DBIC Build (1.53 KB, patch)
2022-11-08 15:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31378: (follow-up) MySQL 8 doens't allow default for longtext (2.85 KB, patch)
2022-11-08 22:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: (QA follow-up) Defer store until JSON fields are populated (2.69 KB, patch)
2022-11-18 15:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31378: Tests shouldn't remove all patrons (3.07 KB, patch)
2022-11-22 21:23 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 Tomás Cohen Arazi 2022-08-16 23:09:30 UTC
We need a generic client implementation that allows setting different authentication services and configurations.

It needs to:
- work for the
- work for the staff interface
- have a CRUD page for dealing with authentication services
- be designed so we eventually move other authentication mechanisms into this CRUD (e.g. CAS? LDAP?)
- allow setting configurations per-service and per-domain (i.e. an OIDC service might allow authenticating several (campus?) domains and we should handle that).
- provide a prettier integration with the login form than CAS and friends

Follow-ups:
- Prioritize IdP's when displaying login options
- Icon / user-oriented presentation

Not all of this needs to be implemented on this bug report. Should be splitted.
Comment 1 Tomás Cohen Arazi 2022-08-16 23:12:14 UTC
Forgot to mention (and is very important):

- Implementation should use a library to handle OAuth2 (as opposed to the current googleopenidconnect script, which does things manually). Mojolicious::Plugin::OAuth2 is already packaged in the staging repo for this purpose (mtj++)
Comment 2 David Cook 2022-08-16 23:51:18 UTC
+1

Here for authoring, testing, or QAing.
Comment 3 Tomás Cohen Arazi 2022-10-05 12:17:32 UTC
Created attachment 141362 [details] [review]
Bug 31378: DB changes
Comment 4 Tomás Cohen Arazi 2022-10-05 12:17:37 UTC
Created attachment 141363 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]
Comment 5 Tomás Cohen Arazi 2022-10-05 12:17:43 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2022-10-05 12:17:47 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2022-10-05 12:17:52 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2022-10-05 12:17:58 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2022-10-05 12:18:03 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2022-10-05 12:18:08 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2022-10-05 12:18:14 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2022-10-05 12:18:19 UTC Comment hidden (obsolete)
Comment 13 Tomás Cohen Arazi 2022-10-05 12:18:30 UTC Comment hidden (obsolete)
Comment 14 Tomás Cohen Arazi 2022-10-05 12:18:35 UTC Comment hidden (obsolete)
Comment 15 Tomás Cohen Arazi 2022-10-05 12:18:40 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi 2022-10-05 12:18:46 UTC Comment hidden (obsolete)
Comment 17 Tomás Cohen Arazi 2022-10-05 12:18:51 UTC Comment hidden (obsolete)
Comment 18 Tomás Cohen Arazi 2022-10-05 12:18:56 UTC Comment hidden (obsolete)
Comment 19 Tomás Cohen Arazi 2022-10-05 12:19:02 UTC Comment hidden (obsolete)
Comment 20 Tomás Cohen Arazi 2022-10-05 12:36:55 UTC
Notes for testing:

KTD (master) comes with the koha-staging repo already configured, so in order to test this patches you can just run:

$ apt install libmojolicious-plugin-oauth2-perl

After installing the dependency, you need to restart Plack (as usual):

$ koha-plack --restart kohadev

The OAuth2 Mojolicious plugin is feeded with the configured authentication providers at (plack) startup time, so in order for new providers to be picked, restart Plack, just to be sure.

Enjoy, provide any constructive comments you can, early.
Comment 21 Katrin Fischer 2022-10-07 07:55:23 UTC
I just noticed that you need to run the database update for Koha to work. Not sure if this could cause problem with the web installer.

Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load application from file "/kohadevbox/koha/api/v1/app.pl": DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Table 'koha_kohadev.auth_providers' doesn't exist at /kohadevbox/koha/Koha/Objects.pm line 312
Comment 22 Nick Clemens 2022-10-07 14:53:28 UTC
Created attachment 141514 [details] [review]
Bug 31378: Use base URLs for redirect uri
Comment 23 David Cook 2022-10-12 22:15:49 UTC
Are there test plans for the UIs?

I'm keen to test this out (although might be worth saving my SO for the QA stage).
Comment 24 PTFS Europe Sandboxes 2022-10-13 09:40:41 UTC
Created attachment 141771 [details] [review]
Bug 31378: DB changes

Signed-off-by: Katrin <me@myemail.com>
Comment 25 PTFS Europe Sandboxes 2022-10-13 09:40:46 UTC
Created attachment 141772 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Katrin <me@myemail.com>
Comment 26 PTFS Europe Sandboxes 2022-10-13 09:40:50 UTC
Created attachment 141773 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Katrin <me@myemail.com>
Comment 27 PTFS Europe Sandboxes 2022-10-13 09:40:55 UTC
Created attachment 141774 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Katrin <me@myemail.com>
Comment 28 PTFS Europe Sandboxes 2022-10-13 09:41:00 UTC
Created attachment 141775 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Katrin <me@myemail.com>
Comment 29 PTFS Europe Sandboxes 2022-10-13 09:41:06 UTC
Created attachment 141776 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Katrin <me@myemail.com>
Comment 30 PTFS Europe Sandboxes 2022-10-13 09:41:10 UTC
Created attachment 141777 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Katrin <me@myemail.com>
Comment 31 PTFS Europe Sandboxes 2022-10-13 09:41:15 UTC
Created attachment 141778 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Katrin <me@myemail.com>
Comment 32 PTFS Europe Sandboxes 2022-10-13 09:41:21 UTC
Created attachment 141779 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Katrin <me@myemail.com>
Comment 33 PTFS Europe Sandboxes 2022-10-13 09:41:26 UTC
Created attachment 141780 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Katrin <me@myemail.com>
Comment 34 PTFS Europe Sandboxes 2022-10-13 09:41:31 UTC
Created attachment 141781 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Katrin <me@myemail.com>
Comment 35 PTFS Europe Sandboxes 2022-10-13 09:41:38 UTC
Created attachment 141782 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Katrin <me@myemail.com>
Comment 36 PTFS Europe Sandboxes 2022-10-13 09:41:43 UTC
Created attachment 141783 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Katrin <me@myemail.com>
Comment 37 PTFS Europe Sandboxes 2022-10-13 09:41:48 UTC
Created attachment 141784 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin <me@myemail.com>
Comment 38 PTFS Europe Sandboxes 2022-10-13 09:41:53 UTC
Created attachment 141785 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin <me@myemail.com>
Comment 39 PTFS Europe Sandboxes 2022-10-13 09:41:57 UTC
Created attachment 141786 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Katrin <me@myemail.com>
Comment 40 PTFS Europe Sandboxes 2022-10-13 09:42:02 UTC
Created attachment 141787 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Katrin <me@myemail.com>
Comment 41 PTFS Europe Sandboxes 2022-10-13 09:42:08 UTC
Created attachment 141788 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Katrin <me@myemail.com>
Comment 42 Katrin Fischer 2022-10-13 10:13:04 UTC
Was not me, but the sandbox was created in my name. We'll fix sign-off lines.
Comment 43 Martin Renvoize 2022-10-13 11:57:14 UTC
Created attachment 141801 [details] [review]
Bug 31378: DB changes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 44 Martin Renvoize 2022-10-13 11:57:21 UTC
Created attachment 141802 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 45 Martin Renvoize 2022-10-13 11:57:27 UTC
Created attachment 141803 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 46 Martin Renvoize 2022-10-13 11:57:37 UTC
Created attachment 141804 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 47 Martin Renvoize 2022-10-13 11:57:43 UTC
Created attachment 141805 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 48 Martin Renvoize 2022-10-13 11:57:49 UTC
Created attachment 141806 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 49 Martin Renvoize 2022-10-13 11:57:55 UTC
Created attachment 141807 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 50 Martin Renvoize 2022-10-13 11:58:01 UTC
Created attachment 141808 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 51 Martin Renvoize 2022-10-13 11:58:07 UTC
Created attachment 141809 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 52 Martin Renvoize 2022-10-13 11:58:13 UTC
Created attachment 141810 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 53 Martin Renvoize 2022-10-13 11:58:19 UTC
Created attachment 141811 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 54 Martin Renvoize 2022-10-13 11:58:25 UTC
Created attachment 141812 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 55 Martin Renvoize 2022-10-13 11:58:31 UTC
Created attachment 141813 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 56 Martin Renvoize 2022-10-13 11:58:37 UTC
Created attachment 141814 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 57 Martin Renvoize 2022-10-13 11:58:43 UTC
Created attachment 141815 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 58 Martin Renvoize 2022-10-13 11:58:50 UTC
Created attachment 141816 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 59 Martin Renvoize 2022-10-13 11:58:56 UTC
Created attachment 141817 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 60 Martin Renvoize 2022-10-13 11:59:02 UTC
Created attachment 141818 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 61 Martin Renvoize 2022-10-13 11:59:49 UTC
Just fixing up signoff lines.. I think David offered to QA this so I'll set him as assigned for it :)
Comment 62 David Cook 2022-10-14 00:24:22 UTC
(In reply to Katrin Fischer from comment #21)
> I just noticed that you need to run the database update for Koha to work.
> Not sure if this could cause problem with the web installer.
> 
> Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load
> application from file "/kohadevbox/koha/api/v1/app.pl":
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Table 'koha_kohadev.auth_providers' doesn't exist at
> /kohadevbox/koha/Koha/Objects.pm line 312

This is certainly an issue. I'm looking at follow-ups...
Comment 63 David Cook 2022-10-14 00:25:55 UTC
(In reply to Martin Renvoize from comment #58)
> To test:
> 1. Apply this patch
> 2. Run:
>    $ kshell
>   k$ prove t/db_dependent/api/v1/auth_providers.t
> => SUCCESS: Tests pass!
> 3. Sign off :-D

Looks like the unit test file is t/db_dependent/Koha/Auth/Provider.t these days.
Comment 64 Tomás Cohen Arazi 2022-10-14 00:57:15 UTC
(In reply to David Cook from comment #62)
> (In reply to Katrin Fischer from comment #21)
> > I just noticed that you need to run the database update for Koha to work.
> > Not sure if this could cause problem with the web installer.
> > 
> > Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load
> > application from file "/kohadevbox/koha/api/v1/app.pl":
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Table 'koha_kohadev.auth_providers' doesn't exist at
> > /kohadevbox/koha/Koha/Objects.pm line 312
> 
> This is certainly an issue. I'm looking at follow-ups...

I think the old GoogleOpenId... stuff removal is to blame, as we decided to defer it. Will look tomorrow morning
Comment 65 Tomás Cohen Arazi 2022-10-14 00:58:14 UTC
(In reply to David Cook from comment #63)
> (In reply to Martin Renvoize from comment #58)
> > To test:
> > 1. Apply this patch
> > 2. Run:
> >    $ kshell
> >   k$ prove t/db_dependent/api/v1/auth_providers.t
> > => SUCCESS: Tests pass!
> > 3. Sign off :-D
> 
> Looks like the unit test file is t/db_dependent/Koha/Auth/Provider.t these
> days.

There should be tests for the providers CRUD in there as well.

git diff origin/master --name-only
Comment 66 Tomás Cohen Arazi 2022-10-14 01:10:41 UTC
(In reply to David Cook from comment #62)
> (In reply to Katrin Fischer from comment #21)
> > I just noticed that you need to run the database update for Koha to work.
> > Not sure if this could cause problem with the web installer.
> > 
> > Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load
> > application from file "/kohadevbox/koha/api/v1/app.pl":
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Table 'koha_kohadev.auth_providers' doesn't exist at
> > /kohadevbox/koha/Koha/Objects.pm line 312
> 
> This is certainly an issue. I'm looking at follow-ups...

Please comment about the code, propose things and stuff so this shines for the release. Follow-ups will come out of course. Thanks for your time!
Comment 67 David Cook 2022-10-14 02:24:03 UTC
(In reply to Tomás Cohen Arazi from comment #66)
> Please comment about the code, propose things and stuff so this shines for
> the release. Follow-ups will come out of course. Thanks for your time!

I've spent the last few hours working on this one, and have a ton of feedback. Hopefully all pretty easy to fix I think...
Comment 68 David Cook 2022-10-14 02:24:34 UTC
I'll shortly add a follow-up to catch the error of the missing database update. 

In theory, with a package install, the database will be automatically upgraded before the web app is started up. But some people will still need to use the web installer, so we don't want it to fail. (Those people will need to restart their Koha before they're able to use the new auth functionality of course.)

Test plan 1:
1. apt install libmojolicious-plugin-oauth2-perl
2. restart_all
3. koha-upgrade-schema kohadev
4. koha-plack --restart kohadev
5. prove t/db_dependent/Koha/Auth/Provider.t

So far so good.

Test plan 2:
1. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
2. Click on "Authentication providers"
(#NOTE: I would've preferred "Identity Providers" since that's a more common industry term.)
3. Click "New authentication provider"
(#NOTE: The UI looks poorly formatted, but I think that's probably a symptom of the new staff interface styling. Not a blocker for me. This can be fixed later...)
(#NOTE: There is a bug with the "Configuration" where "Add new X configuration" doesn't work if you've manually changed anything in the text box. Not a blocker for me, but will need to be fixed at some point. Same goes for "Add default OIDC mapping.)
(#NOTE: There's not enough documentation/help text on how to use this UI. I'm figuring it out through trial and error, but a bit of help text for "Code", "Description", and "Icon URL" at a minimum would be good.)
(#NOTE: With the new Staff Interface, there should be a "Help" link on the right hand side, but I don't see it for this new functionality. I think that's a blocker.)
(#NOTE: I don't really like having to include raw JSON in this UI. This could be made more beautiful.)
(#NOTE: "Code" doesn't appear to be restricted or validated in any way. We should stick to alphanumeric codes. This is borderline... but I think it's a blocker. We need to set the rule before people start using it.)
(#NOTE: We need to add help text at the bottom of this page that says this auth provider won't be available until after a Koha restart.)
4. Click on "Manage Domains"
5. Click "Edit" for first and only line
(#NOTE: It's not clear what a "Domain" is in this context. This needs more help text/documentation. I'd say that's a blocker. From past code review in Koha for OIDC, "Domain" referred to email domain. That really should spelled out clearly. )
6. The breadcrumb doesn't show the auth provider code on the "Edit authentication provider domain" like it does on the "Authentication provider domains" page


4. koha-plack --restart kohadev
(#NOTE: This is an unfortunate step but necessary because of the plugin being used. Auth providers are rarely set up, so not a big drama.)

4. In an Incognito tab or different browser, go to http://localhost:8081/
(#NOTE: In my opinion, we should *not* be allowing staff login by default. While "Auto register" is "Don't allow" by default, we should keep the staff interface as locked down as possible. Not a blocker but an observation...)

5. When I try to login with Keycloak, I get the following error in Koha:
[{"message":"Malformed query string","path":"\/query\/session_state"}]
(#NOTE: Newer OIDC providers will provide session_state in the authentication response. I'll turn this off in Keycloak. Folks can read more about session_state at https://openid.net/specs/openid-connect-session-1_0.html)

6. Now I'm getting this error on the Koha Staff Interface login page (since the user doesn't exist in Koha and I have auto-register turned off):
There was an error authenticating to external identity provider
Exception 'Koha::Exceptions::Auth::Unauthorized' thrown 'External auth user cannot access resource' with code => 401
(#NOTE: I don't think printing the exception on the staff interface is a good idea. Let's remove that.)

7. After adding my user to Koha and giving staff permissions, I'm able to log in. Very nice!

8. Go to http://localhost:8080/ and click "Log in with Keycloak IdP"
9. Since I already have a session in Keycloak, I'm logged into the OPAC with no login. Very good!

10. If I logout of Keycloak and try to log back into http://localhost:8080/ via Keycloak, I get the following:
There was an error authenticating to external identity provider
Can't call method "auto_register" on an undefined value at /kohadevbox/koha/Koha/REST/Plugin/Auth.pm line 66.

(#NOTE: In the code I see "tranverse_hash" but it should be "traverse_hash" in English.)

11. Turn on auto register for all blank domain...
12. restart_all in ktd
13. Try to log into OPAC and Staff Interface
14. Neither works... 
(#NOTE: In Koha/REST/Plugin/Auth.pm, it looks like auto-register should only work for OPAC. In theory, I like that, although I suppose the workaround would be to auto-register for the OPAC, and then your account would exist for the Staff Interface anyway...)

15. I tried "Update on login" using the blank domain and a domain of "prosentient.com.au" and both failed to update my firstname and surname on login. That's a blocker...
Comment 69 David Cook 2022-10-14 02:25:29 UTC
Here is the TL;DR version:

Question:
1. I noticed that the default "Mapping configuration" is just 1-to-1 for strings, but Oauth/OIDC claims can be complex data structures. How do you handle when the claim is an array or an object? The most common one I see in the real world is the "address" object, but there are others too.

Looking at Koha/Auth/Client/OAuth.pm and Koha/Auth/Client.pm, I think that you have added code for this. Would it be "address.street_address" in the Mapping to get the "street_address" value out of the "address" claim? 

The code is uncommented and a bit obscure, so I can't tell if you can handle array values, although I'm less worried about them. Can you?


TODO:
0. Add missing "Help" link to all new pages
1. Restrict "Code" to alphanumeric code
2. Add help text to "New authentication provider" UI
3. Add help text to "Edit authentication provider domain" UI
4. Fix breadcrumb on "Edit authentication provider domain" UI
5. Remove exception text from Koha staff interface login page
6. Remove exception text from Koha OPAC login page
7. Think about changing "tranverse_hash" to "traverse_hash" 
8. Fix "Auto register"
9. Fix "Update on login"
Comment 70 David Cook 2022-10-14 02:28:18 UTC
Created attachment 141853 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.
Comment 71 David Cook 2022-10-14 02:34:34 UTC
QA tool complaints:

 FAIL   Koha/Auth/Client/OAuth.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 FAIL   Koha/Auth/Provider/Domain.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 FAIL   Koha/Exceptions/Auth.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 FAIL   Koha/REST/Plugin/Auth.pm
   FAIL   pod coverage
                POD is missing for 'register'

 FAIL   Koha/Schema/Result/AuthProvider.pm
   FAIL   pod coverage
                POD is missing for koha_object_class
                POD is missing for koha_objects_class

 FAIL   Koha/Schema/Result/AuthProviderDomain.pm
   FAIL   pod coverage
                POD is missing for koha_object_class
                POD is missing for koha_objects_class

 FAIL   admin/authentication_providers.pl
   FAIL   file permissions
                File must have the exec flag

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/admin/authentication_providers.tt
   FAIL   forbidden patterns
                forbidden pattern: simple-quote string (line 407)
                forbidden pattern: tab char (line 427)
                forbidden pattern: tab char (line 442)
                forbidden pattern: simple-quote string (line 449)
                forbidden pattern: simple-quote string (line 450)
                forbidden pattern: simple-quote string (line 456)

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for AuthProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for AuthProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for AuthProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for AuthProvider at /kohadevbox/koha/Koha/Objects.pm line 524
Comment 72 David Cook 2022-10-14 02:37:51 UTC
At line 407 of koha-tmpl/intranet-tmpl/prog/en/modules/admin/authentication_providers.tt, "Not a valid JSON" should be "Not valid JSON" since JSON refers to the format and not the data itself.
Comment 73 David Cook 2022-10-14 02:39:34 UTC
(In reply to Tomás Cohen Arazi from comment #65)
> There should be tests for the providers CRUD in there as well.
> 
> git diff origin/master --name-only

Not that I can see. Just t/db_dependent/Koha/Auth/Provider.t

Maybe there are some missing patches
Comment 74 David Cook 2022-10-14 02:50:28 UTC
Another problem:

If you have a "blank" domain and a specific domain, the "blank" domain is used instead of the specific domain.

If you want to set different "Default library" and "Default category" based on email domain, it won't work if you have a "blank" domain.

It also means the "Auto register" and "Update on login" wouldn't work right... if they were working...

I think the only thing that does work is having "Allow staff: No" in "blank" domain and "Allow staff: Yes" in specific domain. ("Allow opac: no" for "blank" would also work but that would be an unlikely scenario.)
Comment 75 David Cook 2022-10-14 02:54:05 UTC
One last thing: the styling on the login buttons on the Staff Interface and OPAC have little to no contrast which is hard on the eyes. 

It also makes the Staff Interface buttons look strange when you have multiple providers with descriptions of different lengths.
Comment 76 David Cook 2022-10-14 02:55:30 UTC
One last last thing:

It would be good to have a "Sequence" value for the providers, so that you could arrange them on the Staff Interface and OPAC in a desired order rather than the order that they were added to Koha.
Comment 77 Katrin Fischer 2022-10-14 07:56:08 UTC
(In reply to David Cook from comment #62)
> (In reply to Katrin Fischer from comment #21)
> > I just noticed that you need to run the database update for Koha to work.
> > Not sure if this could cause problem with the web installer.
> > 
> > Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load
> > application from file "/kohadevbox/koha/api/v1/app.pl":
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Table 'koha_kohadev.auth_providers' doesn't exist at
> > /kohadevbox/koha/Koha/Objects.pm line 312
> 
> This is certainly an issue. I'm looking at follow-ups...

Thx David :) Read your comment below and I think it describes the problem well, package installations being probably ok, but others could get trapped with the web installer not coming up to tell them about the missing package.
Comment 78 Agustín Moyano 2022-10-14 20:51:50 UTC
Hi everyone, it's been a long time since I wrote to you!

Thanks so much David for so extensive testing.. I'll try to address as much points as possible.

(In reply to David Cook from comment #68)


> (#NOTE: I would've preferred "Identity Providers" since that's a more common
> industry term.)

Nice point. I'll talk to Tomas if we change it

> (#NOTE: The UI looks poorly formatted, but I think that's probably a symptom
> of the new staff interface styling. Not a blocker for me. This can be fixed
> later...)

Sorry, it's been a long time since I've done anything related to Koha.. If you could point me to a well formated page, I'll try to stick to the style as much as possible

> (#NOTE: There is a bug with the "Configuration" where "Add new X
> configuration" doesn't work if you've manually changed anything in the text
> box. Not a blocker for me, but will need to be fixed at some point. Same
> goes for "Add default OIDC mapping.)

I do not believe this patches are at the latest version.. I've added a jquery-validator to check if your jsons are valid before saving. I imagine this is a patch for now, and we need a way to edit a json in a proper way

> (#NOTE: There's not enough documentation/help text on how to use this UI.

Yes, I've been coding this between jobs so I had no time to document, but surely we will add some follow-ups.

> I'm figuring it out through trial and error, but a bit of help text for
> "Code", "Description", and "Icon URL" at a minimum would be good.)

Code should be a unique name for your Identity Provider, the Description is the text that will appear in the login button, and Icon URL should be a URL where we can fetch an icon for login button

> (#NOTE: With the new Staff Interface, there should be a "Help" link on the
> right hand side, but I don't see it for this new functionality. I think
> that's a blocker.)

Ok, I'll ask Tomas for help about how this shoul be done

> (#NOTE: I don't really like having to include raw JSON in this UI. This
> could be made more beautiful.)

Me neither, there should be a JSON editor or something.. does Koha have one?

> (#NOTE: "Code" doesn't appear to be restricted or validated in any way. We
> should stick to alphanumeric codes. This is borderline... but I think it's a
> blocker. We need to set the rule before people start using it.)

Good point. Will be done

> (#NOTE: We need to add help text at the bottom of this page that says this
> auth provider won't be available until after a Koha restart.)

Yes, but we have plans to make it so that you wont need to restart Koha for the provider to be available (a PR to Mojolicious::Plugin::OAuth2 project)

> 4. Click on "Manage Domains"
> 5. Click "Edit" for first and only line
> (#NOTE: It's not clear what a "Domain" is in this context. This needs more
> help text/documentation. I'd say that's a blocker. From past code review in
> Koha for OIDC, "Domain" referred to email domain. That really should spelled
> out clearly. )

Agree. Domain in this case is, as you said, email domain.

> 6. The breadcrumb doesn't show the auth provider code on the "Edit
> authentication provider domain" like it does on the "Authentication provider
> domains" page

I'll check it

> 
> 
> 4. koha-plack --restart kohadev
> (#NOTE: This is an unfortunate step but necessary because of the plugin
> being used. Auth providers are rarely set up, so not a big drama.)
> 
> 4. In an Incognito tab or different browser, go to http://localhost:8081/
> (#NOTE: In my opinion, we should *not* be allowing staff login by default.
> While "Auto register" is "Don't allow" by default, we should keep the staff
> interface as locked down as possible. Not a blocker but an observation...)

Ok, that's easily done

> 
> 5. When I try to login with Keycloak, I get the following error in Koha:
> [{"message":"Malformed query string","path":"\/query\/session_state"}]
> (#NOTE: Newer OIDC providers will provide session_state in the
> authentication response. I'll turn this off in Keycloak. Folks can read more
> about session_state at
> https://openid.net/specs/openid-connect-session-1_0.html)
> 
> 6. Now I'm getting this error on the Koha Staff Interface login page (since
> the user doesn't exist in Koha and I have auto-register turned off):
> There was an error authenticating to external identity provider
> Exception 'Koha::Exceptions::Auth::Unauthorized' thrown 'External auth user
> cannot access resource' with code => 401
> (#NOTE: I don't think printing the exception on the staff interface is a
> good idea. Let's remove that.)

As I said, I do not believe these patches to be at the latest version.. now the user gets redirected to the login page (staff or opac) with error message

> 
> 7. After adding my user to Koha and giving staff permissions, I'm able to
> log in. Very nice!
> 
> 8. Go to http://localhost:8080/ and click "Log in with Keycloak IdP"
> 9. Since I already have a session in Keycloak, I'm logged into the OPAC with
> no login. Very good!
> 
> 10. If I logout of Keycloak and try to log back into http://localhost:8080/
> via Keycloak, I get the following:
> There was an error authenticating to external identity provider
> Can't call method "auto_register" on an undefined value at
> /kohadevbox/koha/Koha/REST/Plugin/Auth.pm line 66.

Strange, I'll have to check that out

> 
> (#NOTE: In the code I see "tranverse_hash" but it should be "traverse_hash"
> in English.)

typo... sorry

> 
> 11. Turn on auto register for all blank domain...
> 12. restart_all in ktd
> 13. Try to log into OPAC and Staff Interface
> 14. Neither works... 
> (#NOTE: In Koha/REST/Plugin/Auth.pm, it looks like auto-register should only
> work for OPAC. In theory, I like that, although I suppose the workaround
> would be to auto-register for the OPAC, and then your account would exist
> for the Staff Interface anyway...)
> 
> 15. I tried "Update on login" using the blank domain and a domain of
> "prosentient.com.au" and both failed to update my firstname and surname on
> login. That's a blocker...

Yep, I think I forgot to code that part

Thanks!
Comment 79 Agustín Moyano 2022-10-14 20:58:46 UTC
(In reply to David Cook from comment #74)
> Another problem:
> 
> If you have a "blank" domain and a specific domain, the "blank" domain is
> used instead of the specific domain.

Strange, the behavior should be the opposite. The logic was that you could define multiple domains (even partially) and the domain with the longest matching text should win. If there was no matching domain, then go with the default.

> 
> If you want to set different "Default library" and "Default category" based
> on email domain, it won't work if you have a "blank" domain.

Maybe I've done something wrong with the regexp, and if the email domain doesn't match, you won't see that "default library" and "default category" configured for the domain

> 
> It also means the "Auto register" and "Update on login" wouldn't work
> right... if they were working...

I've fixed auto register in latest patches, but for it to work, you need to define userid or cardnumber in the mapper.

> 
> I think the only thing that does work is having "Allow staff: No" in "blank"
> domain and "Allow staff: Yes" in specific domain. ("Allow opac: no" for
> "blank" would also work but that would be an unlikely scenario.)
Comment 80 David Cook 2022-10-17 22:54:30 UTC
(In reply to Agustín Moyano from comment #78)
> > (#NOTE: The UI looks poorly formatted, but I think that's probably a symptom
> > of the new staff interface styling. Not a blocker for me. This can be fixed
> > later...)
> 
> Sorry, it's been a long time since I've done anything related to Koha.. If
> you could point me to a well formated page, I'll try to stick to the style
> as much as possible

I'm not familiar with the new staff UI myself, but I think you'll be able to see what I mean at a glance. On CRUD pages, the labels and fields aren't aligned correctly in all cases at the moment.
 
> > (#NOTE: With the new Staff Interface, there should be a "Help" link on the
> > right hand side, but I don't see it for this new functionality. I think
> > that's a blocker.)
> 
> Ok, I'll ask Tomas for help about how this shoul be done

Sounds good. I think it might involve the Koha/Manual.pm file. (I just noticed it on a different issue report earlier today.)
 
> > (#NOTE: I don't really like having to include raw JSON in this UI. This
> > could be made more beautiful.)
> 
> Me neither, there should be a JSON editor or something.. does Koha have one?

Not that I know of. Rather, I meant it would make sense to just have a regular HTML form and translate that into JSON on the backend. 

> As I said, I do not believe these patches to be at the latest version.. now
> the user gets redirected to the login page (staff or opac) with error message

Looking forward to testing the next round!

> > 10. If I logout of Keycloak and try to log back into http://localhost:8080/
> > via Keycloak, I get the following:
> > There was an error authenticating to external identity provider
> > Can't call method "auto_register" on an undefined value at
> > /kohadevbox/koha/Koha/REST/Plugin/Auth.pm line 66.
> 
> Strange, I'll have to check that out

I wasn't able to reproduce it again so might've just been a bit glitchy due to restarts/Keycloak logouts/etc. Might not need to worry about it.
Comment 81 David Cook 2022-10-17 22:56:29 UTC
(In reply to Agustín Moyano from comment #79)
> > If you have a "blank" domain and a specific domain, the "blank" domain is
> > used instead of the specific domain.
> 
> Strange, the behavior should be the opposite. The logic was that you could
> define multiple domains (even partially) and the domain with the longest
> matching text should win. If there was no matching domain, then go with the
> default.

Worth double-checking that one I reckon.

> > If you want to set different "Default library" and "Default category" based
> > on email domain, it won't work if you have a "blank" domain.
> 
> Maybe I've done something wrong with the regexp, and if the email domain
> doesn't match, you won't see that "default library" and "default category"
> configured for the domain

How is the domain supposed to be configured? Perhaps I wasn't inputting it as expected either?
 
> I've fixed auto register in latest patches, but for it to work, you need to
> define userid or cardnumber in the mapper.

You'll definitely need to indicate that on the UI pages.
Comment 82 David Cook 2022-10-17 23:07:36 UTC
Looks like "soft" feature freeze is Oct 28, so if you can get the updates patches up ASAP, I'll make time for QA
Comment 83 Katrin Fischer 2022-10-18 06:42:14 UTC
For the missing help link the last patch on bug 31162 might be interesting. I also noticed the help missing there and it was due to changes to the page structure with the new staff design.

For the JSON editor: Maybe CodeMirror that we use for SQL and YAML can do it?
Comment 84 Tomás Cohen Arazi 2022-10-20 23:22:09 UTC
(In reply to Katrin Fischer from comment #83)
> For the missing help link the last patch on bug 31162 might be interesting.
> I also noticed the help missing there and it was due to changes to the page
> structure with the new staff design.
> 
> For the JSON editor: Maybe CodeMirror that we use for SQL and YAML can do it?

I felt like using the CodeMirror plugin would be enough. Maybe we should make the 'Create' button be a dropdown instead, for choosing the protocol, and make the form be already pre-populated, so we remove the buttons too.
Comment 85 David Cook 2022-10-24 23:41:14 UTC
I'm not too worried about the JSON editor at this point. More of a nice to have but not essential.

Agustin, are we going to get an updated version before the end of this week? 

I could make time to QA this before the soft feature freeze this Friday.
Comment 86 Aleisha Amohia 2022-10-25 01:44:03 UTC
*** Bug 31965 has been marked as a duplicate of this bug. ***
Comment 87 David Cook 2022-10-25 02:59:10 UTC
Comment on attachment 141807 [details] [review]
Bug 31378: Add Auth mojo plugin

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

::: Koha/REST/Plugin/Auth.pm
@@ +83,5 @@
> +        'auth.session' => sub {
> +            my ( $c, $patron ) = @_;
> +            my $userid     = $patron->userid;
> +            my $cardnumber = $patron->cardnumber;
> +            my $cgi        = CGI->new;

I don't think that we should be using the CGI module in the Mojolicious REST API, since we're not using CGI. This is hacky.

@@ +89,5 @@
> +            $cgi->param( userid            => $userid );
> +            $cgi->param( cardnumber        => $cardnumber );
> +            $cgi->param( auth_client_login => 1 );
> +
> +            my ( $status, $cookie, $session_id ) = C4::Auth::check_api_auth($cgi);

This is a suboptimal design for at least a couple of reasons: 1) it requires a CGI object; 2) it creates a session for the API interface rather than the OPAC/Staff interfaces; 3) it's not to purpose - we're not checking auth. We know that we're just creating a new session.

I've been saying for years that we need to standardize session creation, and it's still true.

In the meantime, a minimal session can be created as simply as this:

$session->param( 'number',       $patron->borrowernumber );
$session->param( 'id',           $patron->userid );
$session->param( 'cardnumber',   $patron->cardnumber );
$session->param( 'firstname',    $patron->firstname );
$session->param( 'surname',      $patron->surname );
$session->param( 'branch',       $patron->branchcode );
$session->param( 'branchname',   $patron->library->branchname );
$session->param( 'flags',        $patron->flags );
$session->param( 'emailaddress', $patron->email );
$session->param( 'ip',           $session->remote_addr() );
$session->param( 'lasttime',     time() );
$session->param( 'interface',    $type);

Then you can either pass the $session object or the session ID as the return value.
Comment 88 David Cook 2022-10-25 03:03:59 UTC
(In reply to David Cook from comment #87)
> I've been saying for years that we need to standardize session creation, and
> it's still true.

Recently, I did a little patch for that at Bug 31050, but I haven't had an urgent reason to continue on it.

It would be perfect for a situation like this though. (Of course, I thought of it while working on Koha plugins, other auth solutions, etc.)
Comment 89 Tomás Cohen Arazi 2022-10-29 13:58:17 UTC
Hi all, a couple follow-ups have been pushed to the branch and are ready for testing. We couldn't make some new tests we attempted to write for login work.

It would be great to have them, but not sure if required. Why help there is welcome. We ran out of fuel yesterday
Comment 90 Katrin Fischer 2022-10-30 10:05:29 UTC
The branch doesn't seem to be linked, can you please remind us where to find the updated patches?
Comment 91 Aleisha Amohia 2022-11-01 21:58:24 UTC
Is this the link? https://gitlab.com/thekesolutions/Koha/-/commits/oauth_client
Comment 92 Katrin Fischer 2022-11-01 21:59:28 UTC
(In reply to Aleisha Amohia from comment #91)
> Is this the link?
> https://gitlab.com/thekesolutions/Koha/-/commits/oauth_client

Hi Aleisha, Tomas added it to the URL field after my last comment, it matches with yours.
Comment 93 David Cook 2022-11-01 22:44:07 UTC
(In reply to Tomás Cohen Arazi from comment #89)
> Hi all, a couple follow-ups have been pushed to the branch and are ready for
> testing. We couldn't make some new tests we attempted to write for login
> work.
> 
> It would be great to have them, but not sure if required. Why help there is
> welcome. We ran out of fuel yesterday

Does this mean that the tests are failing and you'd like help to fix the tests?
Comment 94 Tomás Cohen Arazi 2022-11-01 22:56:54 UTC
(In reply to David Cook from comment #93)
> (In reply to Tomás Cohen Arazi from comment #89)
> > Hi all, a couple follow-ups have been pushed to the branch and are ready for
> > testing. We couldn't make some new tests we attempted to write for login
> > work.
> > 
> > It would be great to have them, but not sure if required. Why help there is
> > welcome. We ran out of fuel yesterday
> 
> Does this mean that the tests are failing and you'd like help to fix the
> tests?

It means an attempt from Agustín to write login tests didn't go well. We're can live without those.

I'd like you to test that branch, which solves things that were reported, including a rename. That branch probably needs a tidy, I haven't checked that last commit (before the tests).
Comment 95 David Cook 2022-11-01 23:09:07 UTC
(In reply to Tomás Cohen Arazi from comment #94)
> It means an attempt from Agustín to write login tests didn't go well. We're
> can live without those.

Ok I think I understand. I won't worry too much about failing tests for this go around then.
 
> I'd like you to test that branch, which solves things that were reported,
> including a rename. That branch probably needs a tidy, I haven't checked
> that last commit (before the tests).

The follow up commits seem to be a mix of the tests and core code changes, so those would need to be re-organised before QAing and pushing I think. 

--

I suppose Aleisha or myself could QA, so I suppose either of us could sign off. I'll have a look in any case.
Comment 96 Tomás Cohen Arazi 2022-11-01 23:23:32 UTC
Just skip the last commit (wip trying to add tests).

I noticed we need to redo Nick's and your patches
Comment 97 David Cook 2022-11-02 00:32:50 UTC
(In reply to Tomás Cohen Arazi from comment #96)
> Just skip the last commit (wip trying to add tests).
> 
> I noticed we need to redo Nick's and your patches

It looks like the last commit includes some updates besides tests though despite the commit name?
Comment 98 Tomás Cohen Arazi 2022-11-02 00:37:59 UTC
(In reply to David Cook from comment #97)
>
> It looks like the last commit includes some updates besides tests though
> despite the commit name?

Skip the tests themselves, unless you really want to look at them
Comment 99 David Cook 2022-11-02 02:38:15 UTC
Ok. Starting testing now...
Comment 100 David Cook 2022-11-02 02:40:37 UTC
Looks like we're 547 commits behind master... and for some reason "qa" think the "qa-prev-commit" is 1 commit behind.

Getting a number of failures:

testing 1 commit(s) (applied to 46c299f 'd6 Bug 31378: Rename Auth Provider to')

Processing files before patches
Configure idp routes=====>| 10 / 10 (100.00%)

Processing files after patches
|========================>| 10 / 10 (100.00%)

 OK     Koha/Auth/Client.pm
 OK     Koha/REST/V1/Auth/Identity/Provider/Domains.pm
 OK     api/v1/swagger/definitions/identity_provider.yaml
 OK     api/v1/swagger/definitions/identity_provider_domain.yaml
 OK     api/v1/swagger/paths/auth.yaml
 OK     installer/data/mysql/atomicupdate/bug_31378.pl
 OK     installer/data/mysql/kohastructure.sql
 FAIL   t/db_dependent/Koha/REST/Plugin/Auth/IdP.t
   FAIL   file permissions
                File must have the exec flag
   FAIL   forbidden patterns
                forbidden pattern: trailing space char (line 55)
                forbidden pattern: trailing space char (line 123)

 FAIL   t/db_dependent/api/v1/idp.t
   FAIL   file permissions
                File must have the exec flag
   FAIL   forbidden patterns
                forbidden pattern: trailing space char (line 195)
                forbidden pattern: trailing space char (line 258)
                forbidden pattern: trailing space char (line 301)
                forbidden pattern: Use of Data::Printer (line 310)
                forbidden pattern: Use of Data::Printer, should only be used for devlopment purpose (line 310)
   FAIL   spelling
                 wiht ==> with
                 wihtout ==> without
   FAIL   valid
                Can't locate Crypt/OpenSSL/RSA.pm in @INC (you may need to install the Crypt::OpenSSL::RSA module) (@INC contains: /kohadevbox/koha /kohadevbox/koha/lib /kohadevbox/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/
perl/5.32.1 /usr/local/share/perl/5.32.1 /usr/lib/x86_64-linux-gnu/perl5/5.32 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.32 /usr/share/perl/5.32 /usr/local/lib/site_perl)

 FAIL   t/lib/IdP/ExternalIdP.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666


Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - c649ebc111
Comment 101 David Cook 2022-11-02 02:42:11 UTC
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
Auto-merging installer/data/mysql/mandatory/userpermissions.sql
CONFLICT (content): Merge conflict in installer/data/mysql/mandatory/userpermissions.sql
error: could not apply b466baa415... Bug 31378: Add manage_authentication_providers permission
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply b466baa415... Bug 31378: Add manage_authentication_providers permission
Comment 102 David Cook 2022-11-02 02:44:20 UTC
Started trying to fix the merge conflicts but looks like there's a fair number. 

This is something that should've been done before moving it to "Needs Signoff" I reckon.
Comment 103 David Cook 2022-11-02 02:52:08 UTC
I'll still keep testing to see what I can see though...
Comment 104 David Cook 2022-11-02 05:12:46 UTC
Test plan 1:
1. apt install libmojolicious-plugin-oauth2-perl
2. koha-upgrade-schema kohadev
3. restart_all

<Note that Nick and David's patches from Bugzilla are still needed for Gitlab branch...>

Test plan 2:
1. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
2. Click on "Identity providers"
3. Click "New identity provider"
(#NOTE: UI formatting looks OK on Koha 22.06, but I wonder what it'll look like on Koha 22.11...)
(#FIXME: "Add default OAuth..." buttons still don't work if you make any changes in the text box before pressing the button.)
(#FIXME: There's still not enough help text on the UI to explain how to fill it all in.)
(#NOTE: It probably makes more sense to default to "OIDC" than "OAuth" since the former is more common with common identity providers)
4. Enter relevant details and click "Submit"
(#FIXME: "Code" shouldn't be able to contain non-alphanumeric characters)
(#FIXME: Putting in garbage values causes the page to return to /cgi-bin/koha/admin/identity_providers.pl with no error messages)
(#FIXME: There needs to be a warning that integration won't work until after an application restart...)
5. Click on "Manage Domains"
(#FIXME: It's not clear what "domains" means in this context. It should say something like "Identity provider email domains")
6. Click "Edit" on the default domains
(#FIXME: The "Default library" and "Default category" are initially set to empty, but you can't set them to empty in the "Edit" interface)
(#FIXME: The breadcrumb says "Domains for" instead of "Domains for Test")

7. koha-plack --restart kohadev

8. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that doesn't exist in Koha
(#FIXME: When authentication fails, the end user sees the following message:
There was an error authenticating to external identity provider

Can't call method "auto_register" on an undefined value at /kohadevbox/koha/Koha/REST/Plugin/Auth/IdP.pm line 66.
)

9. Change "Identity provider" default domain to "Auto register"
10. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that doesn't exist in Koha
(#FIXME: When authentication fails, the end user sees the following message:
There was an error authenticating to external identity provider

Can't call method "auto_register" on an undefined value at /kohadevbox/koha/Koha/REST/Plugin/Auth/IdP.pm line 66.
)
(#FIXME: The auto register feature doens't work. It needs to be fixed.)

11. Create a Koha user to match your IdP user
12. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that does exist in Koha
13. Login succeeds

14. Kill IdP session and logout of Koha

15. Change default domain to "Update on login"

16. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that does exist in Koha
(#FIXME: The update on login feature doesn't work. It needs to be fixed.)

17. Change default domain to "Allow staff" logins
18. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that does exist in Koha
19. Note that you get a normal "Error: You do not have permission to access this page" message. That's good.

20. Add "catalogue" permission to the Koha user
21. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that does exist in Koha
22. Login succeeds

23. In an Incognito window or different browser, go to http://localhost:8081, and try logging in with a user that doesn't exist in Koha
(#FIXME: When authentication fails, the end user sees the following message:
Error: Session timed out.
Please log in again
There was an error authenticating to external identity provider

Exception 'Koha::Exceptions::Auth::Unauthorized' thrown 'External auth user cannot access resource' with code => 401
)

24. Add a new domain for that matches the email domain of your IdP/Koha user.
25. Mark "Allow opac" and "Allow staff" as "No"

26. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that does exist in Koha
(#FIXME: Koha login happens even though the specific domain says it shouldn't be allowed)
27. In an Incognito window or different browser, go to http://localhost:8081, and try logging in with a user that does exist in Koha
(#FIXME: Koha login happens even though the specific domain says it shouldn't be allowed)

--

I imagine that there are other problems, but I think that's a fairly thorough analysis of the core functionality.
Comment 105 David Cook 2022-11-02 05:19:12 UTC
I appreciate the replacement of "Authentication provider" being changed to "Identity provider" and "tranverse_hash" being changed to "traverse_hash", but it looks like every other issue I described last time is still present in the Gitlab version...

I have to fail this again, because several of the features don't work. 

Before I test this again, I'd like someone to confirm that they've looked at my "FIXME" comments, and fixed them (or at least provide a reason for not fixing them).
Comment 106 David Cook 2022-11-02 05:25:16 UTC
Overall, I think that we're on the right track, and I'm so excited for this feature. 

Thanks for all the work, Theke.
Comment 107 Agustín Moyano 2022-11-03 02:05:02 UTC
Created attachment 143056 [details] [review]
Bug 31378: DB changes
Comment 108 Agustín Moyano 2022-11-03 02:16:06 UTC
(In reply to David Cook from comment #105)
> I appreciate the replacement of "Authentication provider" being changed to
> "Identity provider" and "tranverse_hash" being changed to "traverse_hash",
> but it looks like every other issue I described last time is still present
> in the Gitlab version...
> 
> I have to fail this again, because several of the features don't work. 
> 
> Before I test this again, I'd like someone to confirm that they've looked at
> my "FIXME" comments, and fixed them (or at least provide a reason for not
> fixing them).

Hi David, could you please try again?

I've attended to all your valuable FIXME comments in the last patch. 

Please let me know what you think.

Thanks so much!
Comment 109 Agustín Moyano 2022-11-03 12:35:15 UTC
Created attachment 143087 [details] [review]
Bug 31378: DB changes
Comment 110 Agustín Moyano 2022-11-03 12:35:24 UTC
Created attachment 143088 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]
Comment 111 Agustín Moyano 2022-11-03 12:35:30 UTC
Created attachment 143089 [details] [review]
Bug 31378: Add dependencies
Comment 112 Agustín Moyano 2022-11-03 12:35:37 UTC
Created attachment 143090 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)
Comment 113 Agustín Moyano 2022-11-03 12:35:44 UTC
Created attachment 143091 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)
Comment 114 Agustín Moyano 2022-11-03 12:35:51 UTC
Created attachment 143092 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules
Comment 115 Agustín Moyano 2022-11-03 12:35:59 UTC
Created attachment 143093 [details] [review]
Bug 31378: Add Auth mojo plugin
Comment 116 Agustín Moyano 2022-11-03 12:36:06 UTC
Created attachment 143094 [details] [review]
Bug 31378: Add API routes
Comment 117 Agustín Moyano 2022-11-03 12:36:13 UTC
Created attachment 143095 [details] [review]
Bug 31378: Add auth providers template plugin
Comment 118 Agustín Moyano 2022-11-03 12:36:24 UTC
Created attachment 143096 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth
Comment 119 Agustín Moyano 2022-11-03 12:36:36 UTC
Created attachment 143097 [details] [review]
Bug 31378: Add OPAC support for authentication providers
Comment 120 Agustín Moyano 2022-11-03 12:36:45 UTC
Created attachment 143098 [details] [review]
Bug 31378: Add STAFF support for authentication providers
Comment 121 Agustín Moyano 2022-11-03 12:36:53 UTC
Created attachment 143099 [details] [review]
Bug 31378: Add accessor methods for JSON attributes
Comment 122 Agustín Moyano 2022-11-03 12:36:59 UTC
Created attachment 143100 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 123 Agustín Moyano 2022-11-03 12:37:06 UTC
Created attachment 143101 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 124 Agustín Moyano 2022-11-03 12:37:18 UTC
Created attachment 143102 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 125 Agustín Moyano 2022-11-03 12:37:27 UTC
Created attachment 143103 [details] [review]
Bug 31378: Add an administration interface for authentication sources
Comment 126 Agustín Moyano 2022-11-03 12:37:39 UTC
Created attachment 143104 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests
Comment 127 Agustín Moyano 2022-11-03 12:37:52 UTC
Created attachment 143105 [details] [review]
Bug 31378: (follow-up) do all FIXME pointed by David
Comment 128 Agustín Moyano 2022-11-03 12:38:04 UTC
Created attachment 143106 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 129 Agustín Moyano 2022-11-03 12:38:13 UTC
Created attachment 143107 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.
Comment 130 David Cook 2022-11-04 05:58:49 UTC
(In reply to Agustín Moyano from comment #108)
> Hi David, could you please try again?
> 
> I've attended to all your valuable FIXME comments in the last patch. 
> 
> Please let me know what you think.
> 
> Thanks so much!

Thanks, Agustín.

I don't have time to thoroughly test it today, and I'm away on Monday, but I'll do a quick glance over it now.
Comment 131 David Cook 2022-11-04 05:59:40 UTC
First things first...staying in Failed QA due to qa tool failures:

--

testing 21 commit(s) (applied to ba01d76 '78 Bug 31713: Fix permissions')

Processing files before patches
Configure idp routes=====>| 48 / 48 (100.00%)

Processing files after patches
|========================>| 48 / 48 (100.00%)

 OK     C4/Auth.pm
 OK     Koha/Auth/Client.pm
 FAIL   Koha/Auth/Client/OAuth.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/Auth/Provider.pm
 OK     Koha/Auth/Provider/Domain.pm
 OK     Koha/Auth/Provider/Domains.pm
 OK     Koha/Auth/Provider/OAuth.pm
 OK     Koha/Auth/Provider/OIDC.pm
 OK     Koha/Auth/Providers.pm
 OK     Koha/Exceptions.pm
 FAIL   Koha/Exceptions/Auth.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/REST/Plugin/Auth.pm
 OK     Koha/REST/V1.pm
 OK     Koha/REST/V1/Auth/Provider/Domains.pm
 OK     Koha/REST/V1/Auth/Providers.pm
 OK     Koha/REST/V1/OAuth/Client.pm
 OK     Koha/Schema/Result/AuthProvider.pm
 OK     Koha/Schema/Result/AuthProviderDomain.pm
 OK     Koha/Schema/Result/Branch.pm
 OK     Koha/Schema/Result/Category.pm
 OK     Koha/Schema/Result/IdentityProviderDomain.pm
 OK     Koha/Template/Plugin/AuthClient.pm
 OK     admin/authentication_providers.pl
 OK     admin/identity_providers.pl
 OK     api/v1/swagger/definitions/auth_provider.yaml
 OK     api/v1/swagger/definitions/auth_provider_domain.yaml
 OK     api/v1/swagger/paths/auth.yaml
 OK     api/v1/swagger/paths/oauth.yaml
 OK     api/v1/swagger/paths/public_oauth.yaml
 OK     api/v1/swagger/swagger.yaml
 OK     installer/data/mysql/atomicupdate/bug_31378.pl
 OK     installer/data/mysql/kohastructure.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/authentication_provider_domains.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/authentication_providers.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_providers.tt
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for IdentityProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for IdentityProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for IdentityProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
   FAIL   valid_template
                DBIx::Class::Schema::source(): Can't find source for IdentityProvider at /kohadevbox/koha/Koha/Objects.pm line 524


 OK     t/db_dependent/Koha/Auth/Client.t
 OK     t/db_dependent/Koha/Auth/Provider.t
 OK     t/db_dependent/Koha/REST/Plugin/Auth/IdP.t
 OK     t/db_dependent/api/v1/idp.t
 FAIL   t/lib/IdP/ExternalIdP.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666


Processing additional checks OK!
Comment 132 David Cook 2022-11-04 06:17:09 UTC
Looks like Koha/Template/Plugin/AuthClient.pm might be having an issue with Koha/Auth/Identity/Providers.pm...

--

Test plan 1:
0. Apply patches
1. apt install libmojolicious-plugin-oauth2-perl
2. koha-upgrade-schema kohadev
3. restart_all

Test plan 2:
1. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
2. Click on "Identity providers"
3. Click "New identity provider"
4. Enter relevant details and click "Submit"
5. Click on "Manage Domains"
6. Click "Edit" on the default domains
7. Set "Allow staff" and allow update on login and autoregister and click "Submit"

8. koha-plack --restart kohadev

9. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that doesn't exist in Koha

--

I'm getting this error:

"There was an error authenticating to external identity provider

No configuration found for your provider"

I'm seeing the following error in the logs on Starman startup:

[2022/11/04 06:16:26] [WARN] Warning: Failed to fetch oauth configuration: Can't locate object method "search" via package "Koha::Auth::Providers" (perhaps you forgot to load "Koha::Auth::Providers"?) at /kohadevbox/koha/Koha/REST/V1.pm
line 143.
[2022/11/04 06:16:26] [WARN] Warning: Failed to fetch oauth configuration: Can't locate object method "search" via package "Koha::Auth::Providers" (perhaps you forgot to load "Koha::Auth::Providers"?) at /kohadevbox/koha/Koha/REST/V1.pm
line 143.

--

Looks like a typo.
Comment 133 David Cook 2022-11-04 06:19:41 UTC
Agustín, how are you testing this during development?

In my docker-compose.yml, I use the following:

    keycloak:
        image: jboss/keycloak
        networks:
            - kohanet
        environment:
            KEYCLOAK_USER: <username>
            KEYCLOAK_PASSWORD: <password>
        ports:
            - "8082:8080"

If you are interested in using Keycloak for testing, I can give a few more instructions on how to set up the realm, the client, the user, and where to find the details for the Koha configuration.

--

That actually reminds me that I don't have a test plan for the "OAuth" Protocol option. Does it still work with the OIDC option since OIDC is just a wrapper around OAuth?

--

Anyway, that's all for me for now!
Comment 134 Agustín Moyano 2022-11-04 12:48:56 UTC
Created attachment 143186 [details] [review]
Bug 31378: DB changes
Comment 135 Agustín Moyano 2022-11-04 12:49:07 UTC
Created attachment 143187 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]
Comment 136 Agustín Moyano 2022-11-04 12:49:15 UTC
Created attachment 143188 [details] [review]
Bug 31378: Add dependencies
Comment 137 Agustín Moyano 2022-11-04 12:49:23 UTC
Created attachment 143189 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)
Comment 138 Agustín Moyano 2022-11-04 12:49:30 UTC
Created attachment 143190 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)
Comment 139 Agustín Moyano 2022-11-04 12:49:37 UTC
Created attachment 143191 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules
Comment 140 Agustín Moyano 2022-11-04 12:49:44 UTC
Created attachment 143192 [details] [review]
Bug 31378: Add Auth mojo plugin
Comment 141 Agustín Moyano 2022-11-04 12:49:52 UTC
Created attachment 143193 [details] [review]
Bug 31378: Add API routes
Comment 142 Agustín Moyano 2022-11-04 12:50:06 UTC
Created attachment 143194 [details] [review]
Bug 31378: Add auth providers template plugin
Comment 143 Agustín Moyano 2022-11-04 12:50:14 UTC
Created attachment 143195 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth
Comment 144 Agustín Moyano 2022-11-04 12:50:21 UTC
Created attachment 143196 [details] [review]
Bug 31378: Add OPAC support for authentication providers
Comment 145 Agustín Moyano 2022-11-04 12:50:29 UTC
Created attachment 143197 [details] [review]
Bug 31378: Add STAFF support for authentication providers
Comment 146 Agustín Moyano 2022-11-04 12:50:37 UTC
Created attachment 143198 [details] [review]
Bug 31378: Add accessor methods for JSON attributes
Comment 147 Agustín Moyano 2022-11-04 12:50:45 UTC
Created attachment 143199 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 148 Agustín Moyano 2022-11-04 12:50:52 UTC
Created attachment 143200 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 149 Agustín Moyano 2022-11-04 12:51:03 UTC
Created attachment 143202 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 150 Agustín Moyano 2022-11-04 12:51:15 UTC
Created attachment 143204 [details] [review]
Bug 31378: Add an administration interface for authentication sources
Comment 151 Agustín Moyano 2022-11-04 12:51:25 UTC
Created attachment 143205 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests
Comment 152 Agustín Moyano 2022-11-04 12:51:35 UTC
Created attachment 143206 [details] [review]
Bug 31378: (follow-up) do all FIXME pointed by David
Comment 153 Agustín Moyano 2022-11-04 12:51:48 UTC
Created attachment 143207 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 154 Agustín Moyano 2022-11-04 12:51:56 UTC
Created attachment 143208 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.
Comment 155 Agustín Moyano 2022-11-04 12:54:40 UTC
(In reply to David Cook from comment #132)
> Looks like Koha/Template/Plugin/AuthClient.pm might be having an issue with
> Koha/Auth/Identity/Providers.pm...
> 
> --
> 
> Test plan 1:
> 0. Apply patches
> 1. apt install libmojolicious-plugin-oauth2-perl
> 2. koha-upgrade-schema kohadev
> 3. restart_all
> 
> Test plan 2:
> 1. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
> 2. Click on "Identity providers"
> 3. Click "New identity provider"
> 4. Enter relevant details and click "Submit"
> 5. Click on "Manage Domains"
> 6. Click "Edit" on the default domains
> 7. Set "Allow staff" and allow update on login and autoregister and click
> "Submit"
> 
> 8. koha-plack --restart kohadev
> 
> 9. In an Incognito window or different browser, go to http://localhost:8080,
> and try logging in with a user that doesn't exist in Koha
> 
> --
> 
> I'm getting this error:
> 
> "There was an error authenticating to external identity provider
> 
> No configuration found for your provider"
> 
> I'm seeing the following error in the logs on Starman startup:
> 
> [2022/11/04 06:16:26] [WARN] Warning: Failed to fetch oauth configuration:
> Can't locate object method "search" via package "Koha::Auth::Providers"
> (perhaps you forgot to load "Koha::Auth::Providers"?) at
> /kohadevbox/koha/Koha/REST/V1.pm
> line 143.
> [2022/11/04 06:16:26] [WARN] Warning: Failed to fetch oauth configuration:
> Can't locate object method "search" via package "Koha::Auth::Providers"
> (perhaps you forgot to load "Koha::Auth::Providers"?) at
> /kohadevbox/koha/Koha/REST/V1.pm
> line 143.
> 
> --
> 
> Looks like a typo.

Sorry, I applied your patch but didn't update Koha::Auth::Providers to Koha::Auth::Identity::Providers

Now it should work

Thanks!
Comment 156 Tomás Cohen Arazi 2022-11-04 14:58:25 UTC
Created attachment 143215 [details] [review]
Bug 31378: DB changes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 157 Tomás Cohen Arazi 2022-11-04 14:58:33 UTC
Created attachment 143216 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 158 Tomás Cohen Arazi 2022-11-04 14:58:41 UTC
Created attachment 143217 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 159 Tomás Cohen Arazi 2022-11-04 14:58:49 UTC
Created attachment 143218 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 160 Tomás Cohen Arazi 2022-11-04 14:58:59 UTC
Created attachment 143219 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 161 Tomás Cohen Arazi 2022-11-04 14:59:13 UTC
Created attachment 143220 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 162 Tomás Cohen Arazi 2022-11-04 14:59:31 UTC
Created attachment 143221 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 163 Tomás Cohen Arazi 2022-11-04 14:59:40 UTC
Created attachment 143222 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 164 Tomás Cohen Arazi 2022-11-04 14:59:48 UTC
Created attachment 143223 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 165 Tomás Cohen Arazi 2022-11-04 14:59:57 UTC
Created attachment 143224 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 166 Tomás Cohen Arazi 2022-11-04 15:00:07 UTC
Created attachment 143225 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 167 Tomás Cohen Arazi 2022-11-04 15:00:20 UTC
Created attachment 143226 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 168 Tomás Cohen Arazi 2022-11-04 15:00:29 UTC
Created attachment 143227 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 169 Tomás Cohen Arazi 2022-11-04 15:00:37 UTC
Created attachment 143228 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 170 Tomás Cohen Arazi 2022-11-04 15:00:45 UTC
Created attachment 143229 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 171 Tomás Cohen Arazi 2022-11-04 15:00:59 UTC
Created attachment 143230 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Comment 172 Tomás Cohen Arazi 2022-11-04 15:01:11 UTC
Created attachment 143231 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 173 Tomás Cohen Arazi 2022-11-04 15:01:22 UTC
Created attachment 143232 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 174 Tomás Cohen Arazi 2022-11-04 15:01:31 UTC
Created attachment 143233 [details] [review]
Bug 31378: (follow-up) Fix QA concerns

Several FIXME comments added on the report addressed here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 175 Tomás Cohen Arazi 2022-11-04 15:01:41 UTC
Created attachment 143234 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 176 Tomás Cohen Arazi 2022-11-04 15:01:53 UTC
Created attachment 143235 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 177 Tomás Cohen Arazi 2022-11-04 15:02:02 UTC
Created attachment 143236 [details] [review]
Bug 31378: (QA follow-up) Tidy tests

This patch perltidys the tests added lately. It also removes some
commented debugging lines.

Note: There's an attempt to write tests on the full auth workflow that
is commented out but left there on purpose for now.
Comment 178 Tomás Cohen Arazi 2022-11-04 15:17:50 UTC
I restored Lucasz signature that was lost at some point.
Comment 179 Nick Clemens 2022-11-04 18:10:59 UTC
Created attachment 143266 [details] [review]
Bug 31378: DB changes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 180 Nick Clemens 2022-11-04 18:11:06 UTC
Created attachment 143267 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 181 Nick Clemens 2022-11-04 18:11:12 UTC
Created attachment 143268 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 182 Nick Clemens 2022-11-04 18:11:18 UTC
Created attachment 143269 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 183 Nick Clemens 2022-11-04 18:11:25 UTC
Created attachment 143270 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 184 Nick Clemens 2022-11-04 18:11:31 UTC
Created attachment 143271 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 185 Nick Clemens 2022-11-04 18:11:37 UTC
Created attachment 143272 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 186 Nick Clemens 2022-11-04 18:11:43 UTC
Created attachment 143273 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 187 Nick Clemens 2022-11-04 18:11:49 UTC
Created attachment 143274 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 188 Nick Clemens 2022-11-04 18:11:56 UTC
Created attachment 143275 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 189 Nick Clemens 2022-11-04 18:12:03 UTC
Created attachment 143277 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 190 Nick Clemens 2022-11-04 18:12:10 UTC
Created attachment 143278 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 191 Nick Clemens 2022-11-04 18:12:17 UTC
Created attachment 143279 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 192 Nick Clemens 2022-11-04 18:12:23 UTC
Created attachment 143280 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 193 Nick Clemens 2022-11-04 18:12:29 UTC
Created attachment 143281 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 194 Nick Clemens 2022-11-04 18:12:36 UTC
Created attachment 143282 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 195 Nick Clemens 2022-11-04 18:12:48 UTC
Created attachment 143283 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 196 Nick Clemens 2022-11-04 18:13:00 UTC
Created attachment 143284 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 197 Nick Clemens 2022-11-04 18:13:07 UTC
Created attachment 143285 [details] [review]
Bug 31378: (follow-up) Fix QA concerns

Several FIXME comments added on the report addressed here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 198 Nick Clemens 2022-11-04 18:13:13 UTC
Created attachment 143286 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 199 Nick Clemens 2022-11-04 18:13:19 UTC
Created attachment 143287 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 200 Nick Clemens 2022-11-04 18:13:26 UTC
Created attachment 143288 [details] [review]
Bug 31378: (QA follow-up) Tidy tests

This patch perltidys the tests added lately. It also removes some
commented debugging lines.

Note: There's an attempt to write tests on the full auth workflow that
is commented out but left there on purpose for now.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 201 Tomás Cohen Arazi 2022-11-07 13:36:57 UTC
(In reply to David Cook from comment #133)
> Agustín, how are you testing this during development?
> 
> In my docker-compose.yml, I use the following:
> 
>     keycloak:
>         image: jboss/keycloak
>         networks:
>             - kohanet
>         environment:
>             KEYCLOAK_USER: <username>
>             KEYCLOAK_PASSWORD: <password>
>         ports:
>             - "8082:8080"
> 
> If you are interested in using Keycloak for testing, I can give a few more
> instructions on how to set up the realm, the client, the user, and where to
> find the details for the Koha configuration.

I've just added an --sso option switch to the 'ktd' command, using the examples you provided (with minor tweaks).

Thanks!
Comment 202 David Cook 2022-11-08 02:19:16 UTC
(In reply to Tomás Cohen Arazi from comment #201)
> I've just added an --sso option switch to the 'ktd' command, using the
> examples you provided (with minor tweaks).

I use a bit of a custom setup, but I'll include your docker-compose.sso.yml :D
Comment 203 David Cook 2022-11-08 03:57:37 UTC
(In reply to David Cook from comment #202)
> (In reply to Tomás Cohen Arazi from comment #201)
> > I've just added an --sso option switch to the 'ktd' command, using the
> > examples you provided (with minor tweaks).
> 
> I use a bit of a custom setup, but I'll include your docker-compose.sso.yml
> :D

Looks like the docker-compose.sso.yml still needs some work, but I've emailed about fixing that.
Comment 204 David Cook 2022-11-08 06:09:32 UTC
Test plan 1:
0. Apply patches
1. apt install libmojolicious-plugin-oauth2-perl
2. koha-upgrade-schema kohadev
3. restart_all

Test plan 2a (Quarkus Keycloak):
1. Go to http://<workstation_ip>:8082/
2. Click "Administration Console"
3. Login to master realm
4. Click on dropdown menu that says "master"
5. Click "Create Realm"
6. Type "test" into "Realm name" and click "Create"
7. Click "Clients"
8. Click "Create client"
9. Type "test" into "Client ID"
10. Click "Next"
11. Set "Client authentication" to "On"
12. Click "Save"
13. Add "Valid redirect URIs" of "http://localhost:8080/*" and "http://localhost:8081/*"
14. Click "Credentials" tab and note the "Client secret"
15. Click "Advanced" tab and turn "Exclude Session State From Authentication Response" to "On" then click "Save"
16. Click "Users" on left nav
17. Click "Create new user"
18. Type "kohadev" into "Username"
19. Type "kohadev@koha-community.org" into Email"
20. Type "Firstname" into "First name"
21. Type "Lastname" into "Last name"
22. Click "Create"
23. Click "Credentials" tab and click "Set password"
24. Choose a password, turn off "Temporary", and click "Save"
25. Click "Realm settings" from the left nav
26. Click "OpenID Endpoint Configuration" at the bottom of the page

Test plan 2b:
1. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
2. Click on "Identity providers"
3. Click "New identity provider"
4a. Type "test" into "Code"
4b. Type "Test" into "Description"
4c. Choose "OIDC" for "Protocol"
4d. Click "Add default OIDC configuration"
4e. Click "Add default OIDC mapping"
4f. Update "Configuration" to match Keycloak details for "key", "secret", and "well_known_url"
5. Click on "Manage Domains"
6. Click "Edit" on the default domains
7. Set "Allow staff" and allow update on login and autoregister and click "Submit"

8. koha-plack --restart kohadev
9. Update OPACBaseURL and staffClientBaseURL to http://localhost:8080 and http://localhost:8081 respectively

Test plan 3:
1. In an Incognito window or different browser, go to http://localhost:8080, and try logging in with a user that doesn't exist in Koha

--

I got the following error: 

There was an error authenticating to external identity provider

DBIC result _type isn't of the _type Category at /usr/share/perl5/DBIx/Class/Storage/BlockRunner.pm line 130.

Looks like auto register doesn't work still...

I changed the email address for the "koha" user to "kohadev@koha-community.org" at http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51

Login works, but it doesn't update the user details on login either. 

Going to have to fail this again.
Comment 205 David Cook 2022-11-08 06:13:44 UTC
When I add a Domain rule that forbids login for staff interface or OPAC, it says "No configuration found for your email domain".

I think you could just leave it as "There was an error authenticating to external identity provider".


But that's just a comment and not a blocker.
Comment 206 David Cook 2022-11-08 06:17:34 UTC
(In reply to David Cook from comment #204)
> I got the following error: 
> 
> There was an error authenticating to external identity provider
> 
> DBIC result _type isn't of the _type Category at
> /usr/share/perl5/DBIx/Class/Storage/BlockRunner.pm line 130.
> 
> Looks like auto register doesn't work still...
 
It looks like I had forgotten to provide "Default library" and "Default category".

When I do provide "Default library" and "Default category", it does auto register a new patron! Yay!

If I provide a "Default category" but no "Default library", then I get this error:

"There was an error authenticating to external identity provider

Broken FK constraint"

--

I think for these two scenarios, you just shouldn't show the error on the Web UI. Log it to the log files and leave it at that. It's enough to tell the end user that "There was an error authentication to external identity provider.
Comment 207 David Cook 2022-11-08 06:24:31 UTC
I've double-checked that my ID token and UserInfo responses include "given_name" and "family_name" and they do.

I've noticed with the auto register that it isn't populating the "firstname" and "surname" fields.

That's probably why the update on login isn't working either.

--

So I think fixes required are:

1) Hide overly revealing error messages in Web UI
2) Fix mapping so that auto-register "update on login" works and auto register includes mappings beyond just email.
Comment 208 David Cook 2022-11-08 06:26:29 UTC
(In reply to David Cook from comment #207)
> So I think fixes required are:
> 
> 1) Hide overly revealing error messages in Web UI
> 2) Fix mapping so that auto-register "update on login" works and auto
> register includes mappings beyond just email.

I notice in the help text that it says "If you plan to use auto register feature, either userid or cardnumber must be present in this mapping". 

That doesn't appear to be true in my testing.
Comment 209 Tomás Cohen Arazi 2022-11-08 13:18:24 UTC
(In reply to David Cook from comment #208)
> (In reply to David Cook from comment #207)
> > So I think fixes required are:
> > 
> > 1) Hide overly revealing error messages in Web UI
> > 2) Fix mapping so that auto-register "update on login" works and auto
> > register includes mappings beyond just email.
> 
> I notice in the help text that it says "If you plan to use auto register
> feature, either userid or cardnumber must be present in this mapping". 
> 
> That doesn't appear to be true in my testing.

Any NOT NULL attribute on the borrowers table needs to be mapped for auto-register. I don't really understand this one.
Comment 210 Tomás Cohen Arazi 2022-11-08 14:51:11 UTC
Created attachment 143396 [details] [review]
Bug 31378: DB changes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 211 Tomás Cohen Arazi 2022-11-08 14:51:24 UTC
Created attachment 143397 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 212 Tomás Cohen Arazi 2022-11-08 14:51:34 UTC
Created attachment 143398 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 213 Tomás Cohen Arazi 2022-11-08 14:51:43 UTC
Created attachment 143399 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 214 Tomás Cohen Arazi 2022-11-08 14:51:52 UTC
Created attachment 143400 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 215 Tomás Cohen Arazi 2022-11-08 14:52:04 UTC
Created attachment 143401 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 216 Tomás Cohen Arazi 2022-11-08 14:52:13 UTC
Created attachment 143402 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 217 Tomás Cohen Arazi 2022-11-08 14:52:24 UTC
Created attachment 143403 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 218 Tomás Cohen Arazi 2022-11-08 14:52:32 UTC
Created attachment 143404 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 219 Tomás Cohen Arazi 2022-11-08 14:52:39 UTC
Created attachment 143405 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 220 Tomás Cohen Arazi 2022-11-08 14:52:47 UTC
Created attachment 143406 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 221 Tomás Cohen Arazi 2022-11-08 14:52:55 UTC
Created attachment 143407 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 222 Tomás Cohen Arazi 2022-11-08 14:53:04 UTC
Created attachment 143408 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 223 Tomás Cohen Arazi 2022-11-08 14:53:12 UTC
Created attachment 143409 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 224 Tomás Cohen Arazi 2022-11-08 14:53:20 UTC
Created attachment 143410 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 225 Tomás Cohen Arazi 2022-11-08 14:53:28 UTC
Created attachment 143411 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 226 Tomás Cohen Arazi 2022-11-08 14:53:36 UTC
Created attachment 143412 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 227 Tomás Cohen Arazi 2022-11-08 14:53:44 UTC
Created attachment 143413 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 228 Tomás Cohen Arazi 2022-11-08 14:53:52 UTC
Created attachment 143414 [details] [review]
Bug 31378: (follow-up) Fix QA concerns

Several FIXME comments added on the report addressed here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 229 Tomás Cohen Arazi 2022-11-08 14:54:02 UTC
Created attachment 143415 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 230 Tomás Cohen Arazi 2022-11-08 14:54:10 UTC
Created attachment 143416 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 231 Tomás Cohen Arazi 2022-11-08 14:54:18 UTC
Created attachment 143417 [details] [review]
Bug 31378: (QA follow-up) Tidy tests

This patch perltidys the tests added lately. It also removes some
commented debugging lines.

Note: There's an attempt to write tests on the full auth workflow that
is commented out but left there on purpose for now.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 232 Tomás Cohen Arazi 2022-11-08 14:54:26 UTC
Created attachment 143418 [details] [review]
Bug 31378: Add domain settings to IdP add page

This patch introduces a small form for an initial domain configuration
to the 'New identity provider' page, to avoid confusion on testers about
required settings for things to work.

To test:
1. Apply this patch
2. Verify you are now offered a configuration section for domain
   settings. Fields like 'Default category' and 'Default library' are
   mandatory.
3. Choosing some values makes them get stored on 'Submit'

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 233 Martin Renvoize 2022-11-08 15:23:00 UTC
Created attachment 143420 [details] [review]
Bug 31378: DB changes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 234 Martin Renvoize 2022-11-08 15:23:07 UTC
Created attachment 143421 [details] [review]
Bug 31378: DBIC updates [DO NOT PUSH]

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 235 Martin Renvoize 2022-11-08 15:23:18 UTC
Created attachment 143422 [details] [review]
Bug 31378: Add dependencies

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 236 Martin Renvoize 2022-11-08 15:23:27 UTC
Created attachment 143423 [details] [review]
Bug 31378: Add Koha::Auth::Provider::Domain(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 237 Martin Renvoize 2022-11-08 15:23:37 UTC
Created attachment 143424 [details] [review]
Bug 31378: Add Koha::Auth::Provider(s) class(es)

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 238 Martin Renvoize 2022-11-08 15:23:45 UTC
Created attachment 143425 [details] [review]
Bug 31378: Add Koha::Auth::Client* modules

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 239 Martin Renvoize 2022-11-08 15:23:52 UTC
Created attachment 143426 [details] [review]
Bug 31378: Add Auth mojo plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 240 Martin Renvoize 2022-11-08 15:23:59 UTC
Created attachment 143427 [details] [review]
Bug 31378: Add API routes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 241 Martin Renvoize 2022-11-08 15:24:07 UTC
Created attachment 143428 [details] [review]
Bug 31378: Add auth providers template plugin

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 242 Martin Renvoize 2022-11-08 15:24:14 UTC
Created attachment 143429 [details] [review]
Bug 31378: Add the API-based auth mechanism to C4::Auth::check_api_auth

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 243 Martin Renvoize 2022-11-08 15:24:24 UTC
Created attachment 143430 [details] [review]
Bug 31378: Add OPAC support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 244 Martin Renvoize 2022-11-08 15:24:31 UTC
Created attachment 143431 [details] [review]
Bug 31378: Add STAFF support for authentication providers

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 245 Martin Renvoize 2022-11-08 15:24:39 UTC
Created attachment 143432 [details] [review]
Bug 31378: Add accessor methods for JSON attributes

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 246 Martin Renvoize 2022-11-08 15:24:48 UTC
Created attachment 143433 [details] [review]
Bug 31378: Add manage_authentication_providers permission

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 247 Martin Renvoize 2022-11-08 15:24:55 UTC
Created attachment 143434 [details] [review]
Bug 31378: Add 'parameter' parameter to Koha::Exceptions::MissingParameter

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 248 Martin Renvoize 2022-11-08 15:25:03 UTC
Created attachment 143435 [details] [review]
Bug 31378: Add authentication provider endpoints

This patch adds routes for handling authentication providers to the REST
API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_providers.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 249 Martin Renvoize 2022-11-08 15:25:12 UTC
Created attachment 143436 [details] [review]
Bug 31378: Add an administration interface for authentication sources

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 250 Martin Renvoize 2022-11-08 15:25:20 UTC
Created attachment 143437 [details] [review]
Bug 31378: Rename Auth Provider to Identity Provider and add Client.t tests

Signed-off-by: Lukasz Koszyk <lukasz.koszyk@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 251 Martin Renvoize 2022-11-08 15:25:29 UTC
Created attachment 143438 [details] [review]
Bug 31378: (follow-up) Fix QA concerns

Several FIXME comments added on the report addressed here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 252 Martin Renvoize 2022-11-08 15:25:36 UTC
Created attachment 143439 [details] [review]
Bug 31378: Use base URLs for redirect uri

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 253 Martin Renvoize 2022-11-08 15:25:43 UTC
Created attachment 143440 [details] [review]
Bug 31378: (follow-up) catch startup error when missing Oauth provider table

In theory, this shouldn't come up with a package install, but
it could happen with other installations, so best to let it get to
the web installer.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 254 Martin Renvoize 2022-11-08 15:25:50 UTC
Created attachment 143441 [details] [review]
Bug 31378: (QA follow-up) Tidy tests

This patch perltidys the tests added lately. It also removes some
commented debugging lines.

Note: There's an attempt to write tests on the full auth workflow that
is commented out but left there on purpose for now.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 255 Martin Renvoize 2022-11-08 15:25:57 UTC
Created attachment 143442 [details] [review]
Bug 31378: Add domain settings to IdP add page

This patch introduces a small form for an initial domain configuration
to the 'New identity provider' page, to avoid confusion on testers about
required settings for things to work.

To test:
1. Apply this patch
2. Verify you are now offered a configuration section for domain
   settings. Fields like 'Default category' and 'Default library' are
   mandatory.
3. Choosing some values makes them get stored on 'Submit'

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 256 Martin Renvoize 2022-11-08 15:26:05 UTC
Created attachment 143443 [details] [review]
Bug 31378: DBIC Build

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 257 Tomás Cohen Arazi 2022-11-08 17:57:04 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 258 Tomás Cohen Arazi 2022-11-08 17:58:48 UTC
Thanks everyone. Any issues you might find, please file follow-up bugs. Fixes/enhancements are welcome of course.
Comment 259 Tomás Cohen Arazi 2022-11-08 22:29:08 UTC
Created attachment 143450 [details] [review]
Bug 31378: (follow-up) MySQL 8 doens't allow default for longtext

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 260 David Cook 2022-11-08 22:43:07 UTC
(In reply to Tomás Cohen Arazi from comment #209)
> > I notice in the help text that it says "If you plan to use auto register
> > feature, either userid or cardnumber must be present in this mapping". 
> > 
> > That doesn't appear to be true in my testing.
> 
> Any NOT NULL attribute on the borrowers table needs to be mapped for
> auto-register. I don't really understand this one.

Both userid and cardnumber can be null though. Auto-register works without them being mapped, although perhaps it shouldn't.
Comment 261 David Cook 2022-11-08 22:45:28 UTC
(In reply to Tomás Cohen Arazi from comment #258)
> Thanks everyone. Any issues you might find, please file follow-up bugs.
> Fixes/enhancements are welcome of course.

Sounds good. I'll give it another test this morning, and open follow-up bugs for any issues I find. 

Glad to see this one in master in any case!
Comment 262 David Cook 2022-11-09 00:18:22 UTC
I've raised bug 32138 and bug 32139 as follow-ups, but they just relate to some configuration-related fixes. They should be easy to fix, so I'll write patches for them shortly. 

When they're fixed, I think that I'll be happy enough with this feature. (I'll probably raise 1 more follow-up soon but it's a minor UI tweak.)

I also raised bug 32140 because I'm a bit concerned that the "interface" is set to "api" rather than the relevant "intranet" or "opac", but I haven't observed any obvious problems with that yet, so... I'm not making it a priority right now.

--

Thanks again to Agustin and Tomas and everyone else for making this happen. 

I've carried a local customization to enable OIDC auth in Koha since 2014, so I'll be glad to put that to rest, and just focus on helping make this core OIDC client the best it can be.
Comment 263 Marcel de Rooy 2022-11-09 12:28:24 UTC
Error while loading /etc/koha/plack.psgi: Can't load application from file "/usr/share/koha/api/v1/app.pl": Can't locate Mojolicious/Plugin/OAuth2.pm in @INC 

Should we mark this as dependency for Mason please?

apt-get install libmojolicious-plugin-oauth2-perl
apt-get install libmojolicious-plugin-renderfile-perl
Comment 264 Tomás Cohen Arazi 2022-11-09 12:40:37 UTC
Hola, Marcel!

(In reply to Marcel de Rooy from comment #263)
> Error while loading /etc/koha/plack.psgi: Can't load application from file
> "/usr/share/koha/api/v1/app.pl": Can't locate Mojolicious/Plugin/OAuth2.pm
> in @INC 
> 
> Should we mark this as dependency for Mason please?
> 
> apt-get install libmojolicious-plugin-oauth2-perl
> apt-get install libmojolicious-plugin-renderfile-perl

Dependencies are packaged (by Mason) and already present on the latest KTD images. Thanks for reporting, I will mention it on the list for other devs to be aware!
Comment 265 David Cook 2022-11-09 22:42:26 UTC
(In reply to Marcel de Rooy from comment #263)
> Error while loading /etc/koha/plack.psgi: Can't load application from file
> "/usr/share/koha/api/v1/app.pl": Can't locate Mojolicious/Plugin/OAuth2.pm
> in @INC 
> 
> Should we mark this as dependency for Mason please?
> 
> apt-get install libmojolicious-plugin-oauth2-perl
> apt-get install libmojolicious-plugin-renderfile-perl

I can see 'Mojolicious::Plugin::RenderFile' in cpanfile, but I can't see 'Mojolicious::Plugin::OAuth2'.

I think cpanfile might need an update? 

I think debian/control gets updated at release time...
Comment 266 Tomás Cohen Arazi 2022-11-10 00:30:11 UTC
(In reply to David Cook from comment #265)
> (In reply to Marcel de Rooy from comment #263)
> > Error while loading /etc/koha/plack.psgi: Can't load application from file
> > "/usr/share/koha/api/v1/app.pl": Can't locate Mojolicious/Plugin/OAuth2.pm
> > in @INC 
> > 
> > Should we mark this as dependency for Mason please?
> > 
> > apt-get install libmojolicious-plugin-oauth2-perl
> > apt-get install libmojolicious-plugin-renderfile-perl
> 
> I can see 'Mojolicious::Plugin::RenderFile' in cpanfile, but I can't see
> 'Mojolicious::Plugin::OAuth2'.

The third patch introduced it.
Comment 267 David Cook 2022-11-10 02:20:50 UTC
(In reply to Tomás Cohen Arazi from comment #266)
> The third patch introduced it.

Ahhh, I see it now. I expected it to be in the usual alphabetical order, but I do see it now. All good.
Comment 270 Katrin Fischer 2022-11-14 07:54:46 UTC
*** Bug 30988 has been marked as a duplicate of this bug. ***
Comment 271 David Cook 2022-11-14 22:18:00 UTC
*** Bug 28420 has been marked as a duplicate of this bug. ***
Comment 272 Tomás Cohen Arazi 2022-11-18 15:30:49 UTC
Created attachment 144071 [details] [review]
Bug 31378: (QA follow-up) Defer store until JSON fields are populated

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 273 Tomás Cohen Arazi 2022-11-22 21:23:42 UTC
Created attachment 144175 [details] [review]
Bug 31378: Tests shouldn't remove all patrons

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 274 Caroline Cyr La Rose 2023-04-28 20:24:09 UTC
Hello everyone! 
This is a bit technical for me, but I attempted to add the information to the manual. Would any of you who worked on this have the time to read what I wrote and let me know if I mangled anything? (It's not very long)

https://gitlab.com/koha-community/koha-manual/-/merge_requests/670

Thanks!
Caroline