Don't verify token expiration on token refresh.

[edelvalle]

When a token is expired and you try to refresh it inside of the refresh interval. It does not refresh, because the refresh endpoints checks if the token is not expired, and that's kind of silly.

This PR duplicates this one that is outdated: #274

Fixes: #249 and #92

[blueyed]

Can you add a test for this, please?

[blueyed]

Can we use https://github.com/GetBlimp/django-rest-framework-jwt/pull/274/files#diff-a4f5c4d64d4811b15e1405573c00f3f4 for the test here?

[blueyed]

I've not looked into the details much, but https://github.com/GetBlimp/django-rest-framework-jwt/issues/92#issuecomment-327746574 seems to discuss this well.

[codecov[bot]]

Codecov Report

Merging #348 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #348   +/-   ##
=======================================
  Coverage   90.67%   90.67%           
=======================================
  Files          14       14           
  Lines         847      847           
  Branches       29       29           
=======================================
  Hits          768      768           
  Misses         66       66           
  Partials       13       13

Flag	Coverage Δ
#codecov	90.67% <100%> (ø)	:arrow_up:
#dj110	87.48% <100%> (ø)	:arrow_up:
#dj111	87.48% <100%> (ø)	:arrow_up:
#dj18	89.84% <100%> (ø)	:arrow_up:
#dj19	89.84% <100%> (ø)	:arrow_up:
#drf31	89.84% <100%> (ø)	:arrow_up:
#drf32	89.84% <100%> (ø)	:arrow_up:
#drf33	89.84% <100%> (ø)	:arrow_up:
#drf34	90.67% <100%> (ø)	:arrow_up:
#drf35	90.31% <100%> (ø)	:arrow_up:
#drf36	90.31% <100%> (ø)	:arrow_up:
#py27	90.67% <100%> (ø)	:arrow_up:
#py33	89.49% <100%> (ø)	:arrow_up:
#py34	89.49% <100%> (ø)	:arrow_up:

Impacted Files	Coverage Δ
rest_framework_jwt/utils.py	100% <ø> (ø)	:arrow_up:
rest_framework_jwt/serializers.py	82.41% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0a0bd40...82520b7. Read the comment docs.

[codecov[bot]]

Codecov Report

Merging #348 into master will decrease coverage by 0.26%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #348      +/-   ##
==========================================
- Coverage   90.67%   90.41%   -0.27%     
==========================================
  Files          14       12       -2     
  Lines         847      824      -23     
  Branches       29       29              
==========================================
- Hits          768      745      -23     
  Misses         66       66              
  Partials       13       13

Flag	Coverage Δ
#codecov	90.41% <100%> (-0.27%)	:arrow_down:
#dj110	87.13% <100%> (-0.35%)	:arrow_down:
#dj111	87.13% <100%> (-0.35%)	:arrow_down:
#dj18	89.56% <100%> (-0.29%)	:arrow_down:
#dj19	89.56% <100%> (-0.29%)	:arrow_down:
#drf31	89.56% <100%> (-0.29%)	:arrow_down:
#drf32	89.56% <100%> (-0.29%)	:arrow_down:
#drf33	89.56% <100%> (-0.29%)	:arrow_down:
#drf34	90.41% <100%> (-0.27%)	:arrow_down:
#drf35	90.04% <100%> (-0.28%)	:arrow_down:
#drf36	90.04% <100%> (-0.28%)	:arrow_down:
#py27	90.41% <100%> (-0.27%)	:arrow_down:
#py33	89.19% <100%> (-0.3%)	:arrow_down:
#py34	90.04% <100%> (+0.55%)	:arrow_up:
#py35	87.13% <100%> (?)
#py36	87.13% <100%> (?)

Impacted Files	Coverage Δ
rest_framework_jwt/serializers.py	82.41% <100%> (ø)	:arrow_up:
tests/test_views.py	100% <100%> (ø)	:arrow_up:
rest_framework_jwt/models.py

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0a0bd40...7f40b13. Read the comment docs.

[edelvalle]

@blueyed sorry, I had been away for a while... I will write some tests for this; right now I have them in the project where I use this package.

And you are right https://github.com/GetBlimp/django-rest-framework-jwt/issues/92#issuecomment-327746574 describes well this

[edelvalle]

@blueyed Well, it took me a while 😆 But here is the test 🚀

[Emnalyeriar]

Any news on this? Will this be merged?

[edelvalle]

I wonder the same... lol

[blueyed]

Thanks for the test / completing it - but I cannot merge it myself. You will have to patiently wait for some maintainer to review and do it.

[edelvalle]

@jpadilla José, can you review this PR please? Gracias!

[pmferreira3]

Can you please merge and release this PR?

[Alex3917]

When a token is expired and you try to refresh it inside of the refresh interval. It does not refresh, because the refresh endpoints checks if the token is not expired

AFAIK this is the intended behavior. I could be wrong, but my understanding is that users are supposed to be required to re-authenticate every so often, so that if someone else has access to their token they can't just keep using it indefinitely without doing something like trying to change the user's password that would likely lead to them being detected.

[Koomook]

@Alex3917 I'm bit confused. Then why refresh mechanism exists? I totally agree with @edelvalle. I just decided to fork repo from @edelvalle . Thank u edelvalle!

[Alex3917]

@Koomook

Then why refresh mechanism exists?

Think about the way your bank's website works. Once you log in, you're probably allowed to keep using the site for up to 24 hours without logging in again, but only if you're continuously using the site and requesting a new page at least once every 15 minutes.

In this scenario the access token life (JWT_EXPIRATION_DELTA) would be 15 minutes, and the refresh token life (JWT_REFRESH_EXPIRATION_DELTA) would be 24 hours.

And that's the way it's supposed to be; your bank doesn't want you to be able to keep using your current session if you've already gotten logged out for inactivity, at least not unless you enter your username and password again. (And the reason all banks work like this isn't a coincidence, there are actually regulations that require it.) Allowing people to keep requesting a new access token after it's already expired would break a lot of financial apps (and similar) that rely on this behavior working as currently defined.

If you want people to be able to use their access tokens up until the end of the refresh delta, is there a reason or use case why just making JWT_EXPIRATION_DELTA the same as JWT_REFRESH_EXPIRATION_DELTA isn't a good solution?

I'm assuming you must have some custom logic beyond what's built into this library to decide whether or not to give the user a new access token, because with the out-of-the-box implementation this wouldn't really do anything.

[pmferreira3]

Image we have the following use case of a generic API and as @Alex3917 mentioned we have configured:

JWT_EXPIRATION_DELTA= 15min 
JWT_REFRESH_EXPIRATION_DELTA = 24h

1. I never accessed the API before, so start clean;
2. First action is login to receive the token;
3. For the next 15 min I invoked several endpoints the API passing the token;
4. 16 min later, the token is no longer valid.

The question is: How can I refresh the token? I am within the 24h period but over than 15 min since the login:

• Should I be able to invoke the refresh-token endpoint and get the new one?
• Should I login again receiving a new token?
• Should the behavior of the client be refresh the token e.g. every 10 min?

[Alex3917]

@pmferreira3

The way we do it is every time we make a request from the front end to the API, there is a request interceptor that checks how much time is left until the token expires. If there is less than a certain amount of time remaining, the request interceptor triggers a function to refresh the token and then store the new token in local storage, and then continues with the original request but with the new token.

So for example if the access token lasts 15 minutes, you could refresh the token before any request where there are less than 12 minutes remaining. The reason for this is that rendering any one page could easily be 10 or 15 network requests, and it would be a needless performance hit to refresh the token before each of those network requests. But this logic still allows the token to be refreshed on every page view, unless those page views are basically right after one another (depending on the time parameters you choose, this could easily be months instead of minutes if we're talking about an app like Facebook instead of a bank).

Having a background process to automatically refresh the token every few minutes regardless of whether there have been any network requests would make sense a scenario where you want to user to stay logged in as long as they still have the page open in their browser, regardless of whether or not they're interacting with it. Basically how it should work really depends on what you want in terms of behavior.

[jayvdb]

The commits which came from https://github.com/GetBlimp/django-rest-framework-jwt/pull/274 should have @jorrit-wehelp as the author in the commit metadata.