-
Notifications
You must be signed in to change notification settings - Fork 188
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 audience
to the client_options
#179
Add audience
to the client_options
#179
Conversation
I've come across an issue where the `identifier` wasn't equal to the `audience` in the token. This resulted in verification errors because currently it will verify the `aud` against the `identifier` if no `audience` is specified. In this PR, I introduced the `audience` as `client_options` and will pass this along in the `verify!` of the `decoded_id_token` so the openid_connect gem [can handle the expected audience](https://github.com/nov/openid_connect/blob/e1eb8ea962af43752b1aed2c1063a3e24f96c5bc/lib/openid_connect/response_object/id_token.rb#L30-L32)
@stanhu @bufferoverflow just a friendly ping 😄 Would you have time to review this ? Thanks in advance! |
There are also Rubocop failures: https://github.com/omniauth/omniauth_openid_connect/actions/runs/9779345570/job/27001862614?pr=179 |
124d76d
to
94c649a
Compare
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.
Looks good, just one little docs suggestion. Feel free to ignore it.
Co-authored-by: Roger Meier <[email protected]>
I've come across an issue where the
identifier
wasn't equal to theaudience
in the token. This resulted in verification errors because currently it will verify theaud
against theidentifier
if noaudience
is specified.In this PR, I introduced the
audience
asclient_options
and will pass this along in theverify!
of thedecoded_id_token
so the openid_connect gem can handle the expected audience