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

Refresh JWT when required to stop it expiring #114

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ianbarker
Copy link

I've noticed that a long running app's token will expire. The API docs state that the token lasts for 24h so this PR should allow a refresh every 12 hours thus stopping it expiring.

@edwellbrook
Copy link
Owner

Hey Ian! Thanks for taking the time to put together this PR. I think this will make a great addition to the library! 😄

Could we implement this a slightly different way?

  • Offer a public option to enable/disable refreshing automatically — I think having this enabled by default would be sensible :)
  • Instead of generating our own expiration timestamp, can we just read the expiration date from the JWT? (see below)
  • Instead of refreshing every 12 hours, only refresh when needed, based off the expiration date on the JWT.

You can get the expiration date from the JWT by:

  • Splitting the token into three parts by the "." character. The 2nd part contains our JWT payload.
  • Base64 decode the 2nd part and JSON.parse to get the payload object
  • The "exp" key contains the token expiration, which can be stored and checked against.

Cheers, and let me know if there's anything I can help with!

@ianbarker
Copy link
Author

Ok, good idea. I'll make those changes and update the PR

@ianbarker ianbarker changed the title Refresh JWT every 12 hours to stop it expiring Refresh JWT when required to stop it expiring Jan 2, 2019
@ianbarker
Copy link
Author

Ok, I've updated the PR so that the token will refresh using the exp property of the JWT payload.

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.

3 participants