Skip to content
This repository has been archived by the owner on May 26, 2020. It is now read-only.

Fix refresh tokens #274

Closed
wants to merge 3 commits into from
Closed

Conversation

jor-rit
Copy link
Contributor

@jor-rit jor-rit commented Oct 24, 2016

Trying to get an new token, with an 'expired' token results in an
signature error. The token is decoded with verification of 'exp' claim.
Only the orig_iat claim should be verified in this case.

Problem is the JWT_DECODE_HANDLER does not have an option to disable exp verification.
Changing the default handler would be an backwards incompatible change... not sure how you guys would want to handle that.
So in this pull it has it's own call to jwt.encode...

Fixes #249

Trying to get an new token, with an 'expired' token results in an
signature error. The token is decoded with verification of 'exp' claim.
Only the orig_iat claim should be verified in this case.

Fixes jpadilla#249
@kodeine
Copy link

kodeine commented Nov 8, 2016

+1 for this

@kodeine
Copy link

kodeine commented Nov 11, 2016

any idea when this will be merged?

@kodeine
Copy link

kodeine commented Nov 14, 2016

@jorrit-wehelp i've tried your repo with this pull request. but i am still getting Refresh has expired. error.
Following is my setting.

'JWT_EXPIRATION_DELTA': datetime.timedelta(seconds=7200),
'JWT_REFRESH_EXPIRATION_DELTA': datetime.timedelta(days=7),

@tvanhouten
Copy link

👍

@jor-rit
Copy link
Contributor Author

jor-rit commented Nov 16, 2016

@kodeine This pull only avoids the 'Error decoding signature.' error.
With this pull request the 'Refresh has expired.' is only thrown when 'orig_iat' field (original issued at timestamp) is expired.
But do note that the 'orig_iat' field is not updated/changed when a new token is generated during refresh...
so orig_iat field functions as a kind of maximum reuse bound for the session...
Not what many people would expect from 'refresh tokens functionality', but it's another issue that I left out of this pull request.

@wlwg
Copy link

wlwg commented May 25, 2017

This would solve issue #92

@edelvalle
Copy link

👍

@blueyed
Copy link
Contributor

blueyed commented Sep 15, 2017

#348 is meant to update this.

Do you agree?

@jor-rit
Copy link
Contributor Author

jor-rit commented Sep 19, 2017

Yeah, agree. This pull is old and now has merge conflicts. Superseded by #348

@jor-rit jor-rit closed this Sep 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants