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

Validate audience when payload is a scalar and options is an array #183

Merged
merged 1 commit into from
Feb 2, 2017

Conversation

steti
Copy link
Contributor

@steti steti commented Jan 4, 2017

I recently updated from gem version 1.5.1 to 1.5.6 in one of my applications and I found that this behavior has changed. In my application, the JWT issuer creates the JWT with a scalar (string) aud claim. The JWT consumer verifies the aud claim against an array of possible values. The consumer has multiple valid names and a valid JWT could target any of those names as an aud. This works fine in v1.5.1, but the audience validation fails in v1.5.6. I'm not sure if this change was intentional. Based on my reading of the RFC this particular case seems like an implementation detail.

@excpt excpt added this to the Version 1.6.0 milestone Jan 4, 2017
@excpt excpt self-requested a review January 18, 2017 21:23
@excpt excpt self-assigned this Jan 18, 2017
@cgeers
Copy link

cgeers commented Feb 1, 2017

In addition to elegantly fixing the problem as described, the implementation here also addresses a separate problem I'm having validating the audience claim.

When both the JWT aud claim, and the aud options field are arrays, the current verification implementation in 1.5.6 requires that all aud options elements be present in the aud claim of the token, but this behavior doesn't make sense (at least in my use case).

The changes proposed here appear to behave such that aud verification will pass when any of the aud options are present in the aud claim of the token. This is a much more sensible behavior.

+1 for merger, LGTM

Copy link
Member

@excpt excpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this contribution. This fixes a lot of issues for the project.

@excpt excpt merged commit 6c0e93f into jwt:master Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants