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.encode shouldn't hardcode header typ as 'JWT' #6

Closed
joeblackwaslike opened this issue Jul 18, 2023 · 2 comments · Fixed by #8
Closed

jwt.encode shouldn't hardcode header typ as 'JWT' #6

joeblackwaslike opened this issue Jul 18, 2023 · 2 comments · Fixed by #8

Comments

@joeblackwaslike
Copy link

It took me awhile to finally track down the source of a subtle bug that appeared when I started to use this library to generate and validate dpop proof tokens (a form of jwt).

The token header and claims look something like this

{
	"typ": "dpop+jwt",
	"alg": "ES256",
	"jwk": {
		"keys": [
			{
				"crv": "P-256",
				"x": "d_gYGcjbItA5JZ1yDo8Jrsx05NFZKiWbddM1a7GrqVE",
				"y": "jNHKYu6Lg9e9oxpHxs-0LydXD1XGmTV94g6IH78tMOI",
				"kty": "EC",
				"kid": "rma_Eq5FqSUgKkh8Fkd6uZw8Nv6hirCyH1iNsKet4pQ"
			}
		]
	}
}
.
{
    "htm": "POST",
    "htu": "/v1/auth/device/1234/review",
    "jti": "c618fcc6-a83f-4198-9c83-647e3aef6a00",
    "iat": 1689702506,
    "exp": 1689706106
}

When I attempt to encode this token with jwt.encode and then decode it, the header has been altered (in place too, an original reference to the header dict now has the 'typ' key changed to 'JWT').

It would seem this line at the top of encode is the culprit https://github.com/authlib/joserfc/blob/main/src/joserfc/jwt.py#L61

    # add ``typ`` in header
    header["typ"] = "JWT"

I can't tell you how excited I was to learn this library existed not long ago, Python has been sorely lacking a clean, no bs, pythonic jose implementation. But this hardcoded typ makes it impossible to use for one of my use cases (one of which is dpop validation) because I'm unable to generate test case material at runtime.

I understand not wanting to support a draft spec until its at least more mature, but I think at this point, dpop is here to stay and my ask would only be adding some flexibility in order to make such a use case possible. So my question is, how best can I scratch my own itch here? What kind of change would you accept in PR form to open up the typ value a bit? Initially, I was thinking 'typ' could be something that can be overrided in JWSRegistry kinda like registering custom header claims. What do you think?

Thanks for the great library and your time btw!

@lepture
Copy link
Member

lepture commented Jul 18, 2023

Oh, sorry for the trouble. I'll make a new release in this week.

@lepture
Copy link
Member

lepture commented Jul 20, 2023

@joeblackwaslike 0.6.0 is released.

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 a pull request may close this issue.

2 participants