-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add client_id and id_token_hint to IdP logout #493
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for the PR. Nicely done.
I had trouble finding doc about why the client_id is needed but here it is: https://www.keycloak.org/docs/latest/release_notes/index.html#oidc-logout-changes
Could you sign your commits and rebase your branch on this project's master?
Thanks for the review! I've implemented the suggestions, rebased onto master, signed the commit, and added some more info on the use of the parameters. Should be good to go :) |
This adds two parameters to the end_session_endpoint IdP URL which the user gets redirected to when singleLogout is triggered. These paramters are: - client_id: the client ID of the current session's provider. 'OPTIONAL' as per the relevant OpenID specification. - id_token_hint: the raw id_token that was obtained during the code callback of this session's login flow (set in session variable `oidc.id_token`). 'RECOMMENDED' by the relevant OpenID specification [1]. Some providers (e.g. node-oidc-provider[2] and Keycloak[3]) require this when using the code OAuth flow. Because passing id_token_hint reveals the id_token to the user agent, a app setting was also added to optionally turn this behaviour off (default is turned on). Builds upon PR nextcloud#373 / issue nextcloud#336 Fixes issue nextcloud#449 [1]: https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout [2]: https://github.com/panva/node-oidc-provider/blob/c243bf6b6663c41ff3e75c09b95fb978eba87381/lib/actions/end_session.js#L32 [3]: https://www.keycloak.org/docs/latest/release_notes/index.html#oidc-logout-changes Signed-off-by: Pieter Fiers <pieter@pfiers.net>
Argh, must've copy pasted that incorrectly, fixed now. Also fixed my commit author not matching the public key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Edited to add links to docs/code examples
This adds two parameters to the end_session_endpoint IdP URL which the
user gets redirected to when singleLogout is triggered.
These paramters are:
as per the relevant OpenID specification.
callback of this session's login flow (set in session variable
oidc.id_token
). 'RECOMMENDED' by the relevant OpenID specification 1.Some providers (e.g. node-oidc-provider 2 and Keycloak 3) require this when using the code OAuth flow.
Because passing id_token_hint reveals the id_token to the user agent, a
app setting was also added to optionally turn this behaviour off (default is
turned on).
Builds upon PR #373 / issue #336
Fixes issue #449
Signed-off-by: Pieter Fiers pieter@pfiers.net