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 everything but expiration #1130

Open
tonsV2 opened this issue May 13, 2024 · 7 comments
Open

Validate everything but expiration #1130

tonsV2 opened this issue May 13, 2024 · 7 comments
Assignees
Labels

Comments

@tonsV2
Copy link

tonsV2 commented May 13, 2024

I'm trying to parse a HTTP request which might contain an expired token. Even if the token is expired I'd still like to ensure everything else validates and parse the payload. (It's for logging users out and ensuring some cookies are deleted client side.)

I'm trying to do the following

token, err := jwt.ParseRequest(
	request,
	jwt.WithKey(jwa.RS256, h.publicKey),
	jwt.WithHeaderKey("Authorization"),
	jwt.WithCookieKey("accessToken"),
	jwt.WithCookieKey("refreshToken"),
	jwt.WithTypedClaim("user", model.User{}),
)
//	if err != nil && !errors.Is(err, jwt.ErrTokenExpired()) { // This doesn't work
if err != nil && !strings.Contains(err.Error(), "error: \"\\\"exp\\\" not satisfied") { // This also doesn't work because if the token is expired and invalid we'll move on
	return nil, err
}

userData, ok := token.Get("user")
if !ok {
	return nil, errors.New("user not found in claims")
}

But if the token is expired, then token.Get("user") results in panic. I could parse the request twice and do some hacky error handling but I was hoping there was a better way.

@lestrrat
Copy link
Collaborator

lestrrat commented May 14, 2024

But if the token is expired, then token.Get("user") results in panic.

Yes, because token is nil.

// if err != nil && !errors.Is(err, jwt.ErrTokenExpired()) { // This doesn't work

Please provide more details. I can't debug unless I have a reproducible test case.


To do what you want, you should delay validation by calling a Parse with WithValidation(false):

token, err := jwt.ParseRequest(
	request,
	jwt.WithValidate(false),  // **THIS**
	// other options....
)

This will give you a un-validated (but verified) token. Then you can run validation manually:

err := jwt.Validate(token, .... options....)

This way token is not nil, and you should be able to look into its contents

@tonsV2
Copy link
Author

tonsV2 commented May 16, 2024

This is how I now parse and validate.
These are my tests related to signing out and this one specifically submits an expired token.

Everything seems to work for my use case but I'm a bit surprised about the following. Maybe it is the expected behavior but it might be a bug, I'll let you decide :-)

token, err := jwt.ParseRequest(
	request,
	jwt.WithKey(jwa.RS256, h.publicKey),
	jwt.WithHeaderKey("Authorization"),
	jwt.WithCookieKey("accessToken"),
	jwt.WithCookieKey("refreshToken"),
	jwt.WithTypedClaim("user", model.User{}),
	//		jwt.WithValidate(false),
)
if err != nil && errors.Is(err, jwt.ErrTokenExpired()) {
	log.Println("expired token")
}

With the above code, and the request carrying an expired token, I would expect the log statement to be reached. I do get an error with the below text

failed to find a valid token in any location of the request (tried: [header keys: "Authorization"]). Additionally, errors were encountered during attempts to parse headers: ([header key: "Authorization", error: "\"exp\" not satisfied"]) cookies: ([cookie key: "accessToken", error: "http: named cookie not present"], [cookie key: "refreshToken", error: "http: named cookie not present"]

But it doesn't seem to be an errTokenExpired error.

@lestrrat
Copy link
Collaborator

@tonsV2 I couldn't immediately find where you set the "exp" field on the JWT. Are you by any chance trying to ask this library to tell you that the COOKIE has expired? If that's the case, this library isn't going to catch that.

Copy link
Contributor

This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 31, 2024
@tonsV2
Copy link
Author

tonsV2 commented May 31, 2024

Sorry for the late reply.

No, this isn't about the cookie expiration. I'm surprised that with an expired token the error message does contain ""exp" not satisfied" however the actual error doesn't seem to be a errTokenExpired. So the if statement in the example code I include in my post doesn't evaluate to true. Specifically errors.Is(err, jwt.ErrTokenExpired()) isn't true despite the token being expired.

@lestrrat
Copy link
Collaborator

@tonsV2 Sorry, I admit I haven't dug deep into the test code, but from the links you gave me I can't find the token contents. Can you please show me exactly what the tokens contain that should error with token expired?

@github-actions github-actions bot removed the Stale label Jun 1, 2024
Copy link
Contributor

This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants