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

Allow to Check expiration of JWT Token #1226

Open
aryanmehrotra opened this issue Nov 19, 2024 · 9 comments
Open

Allow to Check expiration of JWT Token #1226

aryanmehrotra opened this issue Nov 19, 2024 · 9 comments
Assignees
Labels
Service Related to external http, grpc communciation , middlewares etc.

Comments

@aryanmehrotra
Copy link
Member

aryanmehrotra commented Nov 19, 2024

Currently, Oauth middleware validates if the JWT token received is valid or not but it does not check if it is expired.

As per, RFC-7519 it is optional, but how would user configure the middleware to check? Right Now, user would need to check in handler or write their own middleware - which shouldn't be the case

There should be an option while user set the JWKS url to enable these feature to validate the token expiry.

Similar to expiry the follwing should also be checked:

  1. Issuer
  2. Audience
  3. Not Before Claim
  • JTI may be can be used to manage the user sessions from the middleware itself will create a separate ticket for this.
@aryanmehrotra aryanmehrotra added the Service Related to external http, grpc communciation , middlewares etc. label Nov 19, 2024
@RahulMohanK
Copy link
Contributor

@aryanmehrotra I am interested to work on this issue. Could you assign this to me ?

@aryanmehrotra
Copy link
Member Author

Sure, assigning to you, before implementing can you please share the proposed solution and how user would be using it.

@Umang01-hash
Copy link
Contributor

Umang01-hash commented Nov 25, 2024

@RahulMohanK have you thought of any approach on how can we make it configurable to check if JWT token expiration check is needed? Please feel free to share it here.

@RahulMohanK
Copy link
Contributor

@aryanmehrotra @Umang01-hash I have added the possible design change in oauth.go and gofr.go here. Please check the same.

@aryanmehrotra
Copy link
Member Author

@RahulMohanK changing the signature of EnableOauth introduces a breaking change, we need to figure out a different approach which does not break it for existing users.

@RahulMohanK
Copy link
Contributor

@aryanmehrotra To minimize the impact of breaking change the next best option is to introduce a new configuration that users can use to explicitly validate the exp of the jwt.

Users would only need to set this config, gofr will validate exp if and only if the config is set by the end user.

Name of the configuration could be JWKS_VALIDATE_EXP which can be set to true or false. Absence of the config key implies that the value is false by default.

We would only need to introduce the change in Oauth.go parseToken function.

@aryanmehrotra
Copy link
Member Author

aryanmehrotra commented Dec 12, 2024

@RahulMohanK this seems to be a nice option but as user would anyway need to enable oauth using app.EnableOauth it would be better if we take variadic parameters such that it neither breaks nor allows to manage things from multiple places that is configs and the main.go - anyway if the user wished to use the configs to fetch the values they may read the values from the env file using app.Config.

Moreover, I am updating the ticket with more use cases of the same.

Should we accept an option for all these claims, or it would be better if take some map or something such that users can also provide their own claim if needed.

@danysz
Copy link
Contributor

danysz commented Dec 18, 2024

Just take in consideration that if you implement the expiration claim than it should be done including how much time is allowed after the expiration.

   The "exp" (expiration time) claim identifies the expiration time on
   or after which the JWT MUST NOT be accepted for processing.  The
   processing of the "exp" claim requires that the current date/time
   MUST be before the expiration date/time listed in the "exp" claim.
   Implementers MAY provide for some small leeway, usually no more than
   a few minutes, to account for clock skew.  Its value MUST be a number
   containing a NumericDate value.  Use of this claim is OPTIONAL.

@Umang01-hash
Copy link
Contributor

@RahulMohanK are you still working on this issue?
If yes can you please give an update regarding the status of it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Service Related to external http, grpc communciation , middlewares etc.
Projects
None yet
Development

No branches or pull requests

4 participants