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

jwt.sign() changes order of header keys #552

Closed
Biszkopt91 opened this issue Nov 8, 2018 · 2 comments
Closed

jwt.sign() changes order of header keys #552

Biszkopt91 opened this issue Nov 8, 2018 · 2 comments

Comments

@Biszkopt91
Copy link

This piece of the code in sign.js have control over keys order in header option. It causes an unexpected result in generated jwt. Generally, the token is valid but we cannot generate the same token across different technologies.
Current implementation:

Object.assign({
 alg: options.algorithm || 'HS256',
 typ: isObjectPayload ? 'JWT' : undefined,
 kid: options.keyid
}, options.header);

My proposal:

var header = options.header || {};
header.alg = header.alg ? header.alg : options.algorithm || 'HS256',
header.typ = header.typ ? header.typ : isObjectPayload ? 'JWT' : undefined,
header.kid = header.kid ? header.kid : options.keyid;

I'm aware of how the order of keys in js objects works but now it can act in a more predictable way until we use some numeric keys.

@ziluvatar
Copy link
Contributor

While I was reading this issue, this other issue came to my mind: #404

In both cases I think there is no problem from JWT concept around the order of the keys. Although as it is mentioned in the other issue:

In any event, you may want to provide a large warning saying that the key ordering isn't deterministic, and you may generate different signatures for the same payload and secret key pair.

Other than that, what is the issue? Consumer provide header and payload, get a signed string, any other system with that signed string and the proper key/secret will be able to verify it.

@ziluvatar
Copy link
Contributor

I will close this issue and its PR. I'll keep open the #404 to follow what is decided on jws project.

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

No branches or pull requests

2 participants