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

Confused about API #35

Closed
wvh opened this issue Feb 13, 2018 · 4 comments · Fixed by #36
Closed

Confused about API #35

wvh opened this issue Feb 13, 2018 · 4 comments · Fixed by #36

Comments

@wvh
Copy link
Contributor

wvh commented Feb 13, 2018

Hello,

I'm trying to verify a HS256-signed jwt token.

In the jwt module, there's a jwt.Parse() function with the comment that it returns an error if the signature fails, but it doesn't take a key or algorithm as parameter when calling jws.Parse(); so it doesn't actually check the signature.

If I call jws.Verify(), I get back a []byte payload, but there is no clear way to feed this to anything in the jwt module. Feeding it to jwt.ParseBytes panics with an invalid address or nil pointer at github.com/lestrrat/go-jwx/jws/jws.go:422:

	plain.payload, err = base64.RawURLEncoding.DecodeString(wrapper.Payload)

... I assume wrapper.Payload is a nil pointer.

If I call jws.Verify(), throw away the []byte but keep the verification status and then call jwt.ParseString() on the original token to get a jwt struct, I think I have both a valid signature and token with claims, but I guess with double parsing.

I'm a bit confused about the way to go here and would appreciate some insight. Thanks!

@lestrrat
Copy link
Collaborator

Ah, that's a recent change, and a bad omission... will do a fix. thanks!

@lestrrat
Copy link
Collaborator

@wvh the underlying API underwent a huge refactor recently, and I think intent that was put on the documentation was not properly implemented at the time. I have addressed this as #36.

If you checkout topic/jwt-parse-verify branch, you should now be able to do this:

// Parse JWT without caring for signatures
t1, err := jwt.Parse(source)

// Parse JWT and verify using signatures
t2, err := jwt.Parse(srouce, jwt.WithVerify(algorithm, key))

Can you please confirm this makes sense + fixes your issue?

@wvh
Copy link
Contributor Author

wvh commented Feb 14, 2018

I've tried the code and this fixes my issue. Thanks!

If people forget to provide the optional argument though, they'll get a token without signature check. Maybe it would be safer to also add some sort of "VerifyThenParse" method that doesn't return a parsed token without a valid signature, to make it hard to use a token without valid signature.

@lestrrat
Copy link
Collaborator

I agree in principle, but I think we can do that in the same API, and also not break compatibility:

jwt.AllowParseWithoutVerify = false
jwt.Parse(...) // always errors

either way, let's do that on a separate thread. Please file a new issue if you feel strongly about it :)

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