Bug 33708 - OAuth/OIDC authentication for the staff interface requires OPAC enabled
Summary: OAuth/OIDC authentication for the staff interface requires OPAC enabled
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 33675
  Show dependency treegraph
 
Reported: 2023-05-09 21:01 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2023-12-28 20:47 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07
Circulation function:


Attachments
Bug 33708: Make staff interface login not require public API (OAuth/OIDC) (3.06 KB, patch)
2023-05-09 21:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 33708: Provide non-public endpoint for OAuth/OIDC for staff interface (1.70 KB, patch)
2023-05-10 01:36 UTC, David Cook
Details | Diff | Splinter Review
Bug 33708: Allow anonymous access to OAuth endpoints (1.17 KB, patch)
2023-05-10 02:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 33708: Make staff interface login not require public API (OAuth/OIDC) (3.11 KB, patch)
2023-05-10 23:40 UTC, David Cook
Details | Diff | Splinter Review
Bug 33708: Provide non-public endpoint for OAuth/OIDC for staff interface (1.75 KB, patch)
2023-05-10 23:40 UTC, David Cook
Details | Diff | Splinter Review
Bug 33708: Allow anonymous access to OAuth endpoints (1.22 KB, patch)
2023-05-10 23:40 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2023-05-09 21:01:04 UTC
Because of a mistake, the code that generates the redirect URL on the OAuth/OIDC handshake is using the `/public`namespace regardless of the interface we are acessing. This has the consequence of requiring the public-facing API enabled.

This is wrong!
Comment 1 Tomás Cohen Arazi (tcohen) 2023-05-09 21:11:19 UTC
Created attachment 150905 [details] [review]
Bug 33708: Make staff interface login not require public API (OAuth/OIDC)

This patch makes the URL for staff login not point to the `/public`
namespace. The behavior is not changed for the protocol, but as
`/public` requires several settings to be available, it effectively
requires to enable the OPAC, the public API, etc. This patch
diferentiates both to solve the problem.

I've tested following the Wiki instructions to set keycloak [1] using
the *--sso* switch for `ktd` as well [2].

It is important to set the following URLs as allowed redirect in order
to replicate the issue and verify the fix:

http://localhost:8080/api/v1/public/oauth/login/test/opac
http://localhost:8081/api/v1/oauth/login/test/staff

To test:
1. Login into the staff interface using the SSO link:
=> FAIL: Results in a 'Bad redirect URL' error
2. Apply this patch and repeat 1
=> SUCCESS: You get a permission denied error or you just login,
depending on your setup.

[1] https://wiki.koha-community.org/wiki/Testing_SSO
[2] ktd --sso up -d

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Cook 2023-05-10 01:06:35 UTC
Looking at this now...
Comment 3 David Cook 2023-05-10 01:15:07 UTC
Not all URLs are updated it seems. The path for the URL for the button "Log in with Keycloak" on the staff interface is still /api/v1/public/oauth/login/keycloak/staff 

If you disable the sysprefs "RESTPublicAnonymousRequests" and "RESTPublicAPI", you'll see the following error {"error":"Configuration prevents the usage of this endpoint by unprivileged users"}
Comment 4 David Cook 2023-05-10 01:30:43 UTC
If I fix the URL, then I get a 404.

I see the paths in api/v1/swagger/paths/oauth.yaml but it doesn't appear in http://localhost:8081/api/v1/.html

Ah because it's missing from api/v1/swagger/swagger.yaml
Comment 5 David Cook 2023-05-10 01:35:43 UTC
If I fix the URL and the Swagger, then I get a 401 trying to access http://localhost:8081/api/v1/oauth/login/keycloak/staff

I think I thought about this a bit when the OAuth/OIDC functionality was being developed. 

--

(The public endpoint actually has a similar problem. If you have disabled OpacPublic, you have to enable RESTPublicAnonymousRequests in order to use the OAuth/OIDC for the OPAC.)
Comment 6 David Cook 2023-05-10 01:36:56 UTC
Created attachment 150918 [details] [review]
Bug 33708: Provide non-public endpoint for OAuth/OIDC for staff interface

This change fixes the definition for the non-public endpoint for the OAuth/OIDC
implementation.

It also uses the non-public endpoint for the staff interface UI.
Comment 7 David Cook 2023-05-10 02:45:14 UTC
Created attachment 150919 [details] [review]
Bug 33708: Allow anonymous access to OAuth endpoints

Users needs anonymous access to OAuth endpoints so that they can
login, and then use authenticated access for other endpoints.
Comment 8 David Cook 2023-05-10 02:46:08 UTC
Tomas, take a look at my patches and let me know what you think.

With all 3 patches, I've tested OIDC on staff interface and OPAC, and gotten them working with their respective URLs.
Comment 9 Tomás Cohen Arazi (tcohen) 2023-05-10 11:40:03 UTC
(In reply to David Cook from comment #8)
> Tomas, take a look at my patches and let me know what you think.
> 
> With all 3 patches, I've tested OIDC on staff interface and OPAC, and gotten
> them working with their respective URLs.

They look great. I was really tired last night when I submitted. Thanks!
Comment 10 David Cook 2023-05-10 23:32:00 UTC
(In reply to Tomás Cohen Arazi from comment #9)
> They look great. I was really tired last night when I submitted. Thanks!

No worries. How do we want to do the sign offs for these?
Comment 11 David Cook 2023-05-10 23:40:45 UTC
Created attachment 151037 [details] [review]
Bug 33708: Make staff interface login not require public API (OAuth/OIDC)

This patch makes the URL for staff login not point to the `/public`
namespace. The behavior is not changed for the protocol, but as
`/public` requires several settings to be available, it effectively
requires to enable the OPAC, the public API, etc. This patch
diferentiates both to solve the problem.

I've tested following the Wiki instructions to set keycloak [1] using
the *--sso* switch for `ktd` as well [2].

It is important to set the following URLs as allowed redirect in order
to replicate the issue and verify the fix:

http://localhost:8080/api/v1/public/oauth/login/test/opac
http://localhost:8081/api/v1/oauth/login/test/staff

To test:
1. Login into the staff interface using the SSO link:
=> FAIL: Results in a 'Bad redirect URL' error
2. Apply this patch and repeat 1
=> SUCCESS: You get a permission denied error or you just login,
depending on your setup.

[1] https://wiki.koha-community.org/wiki/Testing_SSO
[2] ktd --sso up -d

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 12 David Cook 2023-05-10 23:40:48 UTC
Created attachment 151038 [details] [review]
Bug 33708: Provide non-public endpoint for OAuth/OIDC for staff interface

This change fixes the definition for the non-public endpoint for the OAuth/OIDC
implementation.

It also uses the non-public endpoint for the staff interface UI.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 13 David Cook 2023-05-10 23:40:51 UTC
Created attachment 151039 [details] [review]
Bug 33708: Allow anonymous access to OAuth endpoints

Users needs anonymous access to OAuth endpoints so that they can
login, and then use authenticated access for other endpoints.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 14 David Cook 2023-05-10 23:41:30 UTC
(In reply to David Cook from comment #10)
> (In reply to Tomás Cohen Arazi from comment #9)
> > They look great. I was really tired last night when I submitted. Thanks!
> 
> No worries. How do we want to do the sign offs for these?

I've added my sign off to the patches. If you want to do the same for my last two, then I think we could move this to "Signed Off"?
Comment 15 Tomás Cohen Arazi (tcohen) 2023-05-18 14:36:07 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 16 Matt Blenkinsop 2023-06-08 16:51:51 UTC
Nice work everyone!

Pushed to stable for 22.11.x