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

(Feature Request) Easy way to get content of expired token #24

Closed
PaulAtST opened this issue Nov 18, 2021 · 8 comments
Closed

(Feature Request) Easy way to get content of expired token #24

PaulAtST opened this issue Nov 18, 2021 · 8 comments

Comments

@PaulAtST
Copy link

PaulAtST commented Nov 18, 2021

It would be great if there was a nicer way to get the payload of an expired token. Currently the only way I have found is by setting a long graceSeconds, and then checking the exp after.

What I am thinking is it would be nice to have a function that returns an object like this:

{
    isExpired: false, 
    payload: {}
}

To give a bit more insigt to my use case, when a token is expired, I want the server to confirm the user should still have access in a secondary permission system before asking the client to refresh, otherwise I want to return a different condition that will tell the client to fully log the user out.

Overall I am liking this library and look forwards to seeing where it goes.

@ottokruse
Copy link
Contributor

ottokruse commented Nov 22, 2021

Thanks for the feature request.

What is your pseudo code flow? We understand it like so now:

verify JWT
if error thrown:
  if error is because of JWT expiry:
    confirm user has access in secondary permission system
    xyz # And then what?
  else:
    re-raise error # API access is prevented
pass # Access granted

We'd like to fully understand your use case. Can you tell more about the secondary permission system and how your auth flow works (and why like that)?

What I am thinking is it would be nice to have a function that returns an object like this ...

Your proposal makes sense. We do wanna stick with throwing the Error though, to make it harder for users to misunderstand what is going on. Another (very well known) JWT lib out there once included a method decode in their lib, that some users then confused for verify, leading to insecure APIs, we don't want that to happen.

A thought we're playing with, is including the raw JWT in thrown Errors, e.g.:

try {
  verifier.verify(jwt);
} catch (err) {
  console.log(err.rawJwt.payload.exp);
}

Probably make that opt-in, behind a flag.

Overall I am liking this library and look forwards to seeing where it goes.

Thank you! Great to hear

@PaulAtST
Copy link
Author

Sure, here is sudo code with a bit more detail:

verify JWT
if error thrown:
  if error is because of JWT expiry:
    decode expired access token to get username
    confirm user has access in secondary permission system
    if user has access:
      raise error # API access is prevented, tell client to refresh session
    else: 
      raise error # API access is prevented, tell client to logout
  else:
    raise error # API access is prevented
pass # Access granted

I like your idea of having an option to have the decoded payload in the error object, that would work well for my use case, and I can see how that can help prevent insecure implementations.

@elorzafe
Copy link
Contributor

elorzafe commented Dec 2, 2021

@PaulAtST thanks for giving more details, I have one more question: why you need to tell the client the token is expired because that could be made locally as well.

Thanks again!

@PaulAtST
Copy link
Author

PaulAtST commented Dec 4, 2021

@elorzafe The taught was to make the REST API straight forwards to consume, by making the API explicit on why a request was rejected

For example:
Was the request missing credentials -> Client should login first
Was the access token expired -> Client should refresh before retrying the request
Was the user locked out by the secondary permission system -> Client should logout, and probably alert the user to contact there account administrator

You are correct though, technically we could make the clients responsible for keeping track of when they should refresh

@elorzafe
Copy link
Contributor

elorzafe commented Dec 7, 2021

@PaulAtST thanks for your response.

Is best security practice to not disclose information so attackers can benefit from that and potentially make a more specific attack (social engineering to a particular user) for example on the case you mention, if the token is expired but it has access to a specific resource. In case an expired token is compromised and an attacker tries to use it, then next time will know if the token is not expire it can access a certain resource.

@PaulAtST
Copy link
Author

PaulAtST commented Dec 7, 2021

@elorzafe Good point. Would an attacker actually be able to gain much if the API always checks for an unexpired and valid token before ever checking for permissions? In the current design the API will always return a 401, before a 403, the only way a 403 would be returned is if the token is valid.

Going down the social engineering path, I could see it possibly worth never returning a 403. This way if someone gets a valid token they won't be able to probe endpoint's that they might be able to use, if they figure out a way to get a token with elevated permissions. On the other hand I feel like an attacker is more likely to try to decompile the code to figure out what the endpoints are then randomly guessing them.

Granted if someones plan is to use social engineering to gain higher access levels, they could just compromise admin credentials and would have a full UI for messing with the system. No token interception or endpoint reverse engineering required. Which side note for anyone reading along, is why it's a good idea to have a second API for system maintnince, which is not exposed publicly (Should not be referenced in any public web / mobile apps, and should ideally be behind a vpn as an extra level of security).

@ottokruse
Copy link
Contributor

Solution merged into main: #27
Will release to NPM shortly.
Let us know what you think @PaulAtST !

@PaulAtST
Copy link
Author

@ottokruse Thank you for the update! It's working well for me.

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

No branches or pull requests

3 participants