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

fix: VerifyExpiresAt #246

Closed
wants to merge 1 commit into from
Closed

fix: VerifyExpiresAt #246

wants to merge 1 commit into from

Conversation

matteosilv
Copy link

VerifyExpiresAt does not do what is expected from its description

Signed-off-by: Matteo Silvestri <[email protected]>
@oxisto
Copy link
Collaborator

oxisto commented Oct 14, 2022

I am not quite sure if I understand you correctly. The description says VerifyExpiresAt compares the exp claim against cmp (cmp < exp). Which we do by only using Before.

@matteosilv
Copy link
Author

I am not quite sure if I understand you correctly. The description says VerifyExpiresAt compares the exp claim against cmp (cmp < exp). Which we do by only using Before.

It isn't correct documentation states <=

https://github.com/golang-jwt/jwt/blob/main/map_claims.go#L35

@oxisto
Copy link
Collaborator

oxisto commented Oct 14, 2022

I am not quite sure if I understand you correctly. The description says VerifyExpiresAt compares the exp claim against cmp (cmp < exp). Which we do by only using Before.

It isn't correct documentation states <=

https://github.com/golang-jwt/jwt/blob/main/map_claims.go#L35

Ah, it seams the documentation for MapClaims says <= and for StandardClaims it says <.

From a logical point of view I would say that < is correct because at the time of exp it is already expired. The additional leeway we can grant is another story. I will double check with the RFC later.

@matteosilv
Copy link
Author

VerifyExpiresAt should verify that at the provided time the token is going to be expired. Is this assumption right?

@oxisto
Copy link
Collaborator

oxisto commented Oct 14, 2022

VerifyExpiresAt should verify that at the provided time the token is going to be expired. Is this assumption right?

No. It checks whether the token is valid, so it must be checked if it is not yet expired. I agree that we should probably make the documentation of these functions clearer.

@matteosilv
Copy link
Author

In this case, I agree that strict < is correct. The name of the method and its description however are not clear enough imho

@oxisto
Copy link
Collaborator

oxisto commented Feb 21, 2023

Superseded by #234

@oxisto oxisto closed this Feb 21, 2023
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