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

Explicitly set header in JWS::load #31

Merged
merged 2 commits into from
Apr 13, 2015
Merged

Explicitly set header in JWS::load #31

merged 2 commits into from
Apr 13, 2015

Conversation

mikesir87
Copy link
Contributor

When using the constructor, the header is ordered in its own order, not according to how the provided token may have be ordered. This then causes signature validation to fail because the header has been reordered from what the original token provided.

Example: My token is created by another service that puts the header in order of [typ, alg]. When JWS::load is used, when the JWS token's constructor is used, it always orders it [alg, typ]. This, now reordered header, is then later json encoded and used in the signature verification, which of course will now fail.

By setting the header to what the token provided, signature verification can now succeed, regardless of the order of the json fields.

When using the constructor, the header is ordered in its own order,
not according to how the provided token may ahve be ordered. This
then causes signature validation to fail because the header has
been reordered from what the original token provided.
@odino
Copy link
Contributor

odino commented Apr 12, 2015

Thanks dude! :)

Would you be able to add a test case as well?

@mikesir87
Copy link
Contributor Author

Alrighty. There ya go! I tried to match it up with the other test cases that were there, but feel free to modify it however needed.

@odino
Copy link
Contributor

odino commented Apr 13, 2015

yay! thanks man!

odino added a commit that referenced this pull request Apr 13, 2015
Explicitly set header in JWS::load
@odino odino merged commit fbaecbc into namshi:master Apr 13, 2015
@odino
Copy link
Contributor

odino commented Apr 13, 2015

there in 2.2.1

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.

2 participants