Skip to content
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 url_state to SignoutRequest #1818

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

richjyoung
Copy link

@richjyoung richjyoung commented Jan 22, 2025

Extends the fix added for #293 in PR #1212 to also cover sign out.

For background I believe the same principles regarding state for sign in within the linked issue also apply in the sign out case as well. I am working with a multi-tenant system that has a proxy to resolve the actual hostname for the redirect_uri, which also needs to send a signed-out user back to the application start page on the right hostname.

Without this fix, I would need to configure every potential hostname as a valid redirect URI in the IDP client config (whilst I think Keycloak can allow wildcards, it's not recommended and others such as Entra ID do not).

This change replicates the concatenated internal opaque state value with a custom url_state value and delimiter as used in sign in. Tests have been added to ensure the url_state can still be added even if the sign out request does not need to persist any client state.

Apologies for not raising an issue first, in tweaking the code to see if it was viable I basically had the PR already written.

Checklist

  • This PR makes changes to the public API
  • I have included links for closing relevant issue numbers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant