search

omniauth / omniauth_openid_connect

MIT License
171 stars 187 forks source link

Add id_token_hint to the post logout redirect uri #149

Open CSDUMMI opened 1 year ago

CSDUMMI commented 1 year ago

The OneLogin IdP requires the id_token_hint field to be set to the previously issued access token in the end session uri to perform a logout and redirect the user to the post logout uri.[^1]

The Keycloak IdP requires the id_token_hint field to be set to avoid a confirmation dialog before redirecting them to the post logout uri.[^2]

This PR adds the id_token_hint to support this behavior by the IdPs. I'm open to only enabling this through an option that is disabled by default.

[^1]: See #140 and https://developers.onelogin.com/openid-connect/api/logout [^2]: See keycloak documentation: https://www.keycloak.org/docs/latest/securing_apps/#logout

kitebuggy commented 1 year ago

Great work, thank you.

CSDUMMI commented 1 year ago

The tests fail because access_token is now called by encoded_post_logout_redirect_uri and this function calls client.access_token! if no access token has previously been fetched.

The tests do not mock this function and thus a request to example.com is made - expecting an access token but receiving HTML.

I don't know enough about stubbing in Ruby to stub this particular behavior.

CSDUMMI commented 1 year ago

@stanhu can you review this PR or is there somebody else I can talk to?

drjole commented 1 year ago

Hello everyone! Will this PR be merged at some point? Thanks for your work.

evgenyneu commented 3 weeks ago

I would like this to be merged. Are there any concerns with this code? Need any help?

coberlin commented 3 weeks ago

I also need to send id_token_hint, but this solution doesn't work for my client. I'm using authorization_code flow and the call to access_token fails with "invalid grant" because this flow requires that I send the authorization_code, which is supplied by params['code'] but that is not passed in the logout uri. I could send this code as a parameter, but that seems like overkill to require a fetch of the access_token just to allow logout. My application keeps track of the id_token, and I'd rather pass it as a parameter to the logout uri directly. I have a PR for this.

evgenyneu commented 3 weeks ago

I found out that even if this gets merged, I still can't use it with Amazon Cognito because they're not compliant with the OpenID Connect logout specification. Cognito uses different URL parameters (client_id and logout_uri), see their documentation. Just a heads up! 😄