Don't verify token expiration on token refresh. #348
Conversation
|
Can you add a test for this, please? |
|
Can we use https://github.com/GetBlimp/django-rest-framework-jwt/pull/274/files#diff-a4f5c4d64d4811b15e1405573c00f3f4 for the test here? |
|
I've not looked into the details much, but #92 (comment) seems to discuss this well. |
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
|
@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 #92 (comment) describes well this |
…, inside of the refresh delta interval.
|
@blueyed Well, it took me a while 😆 |
|
Any news on this? Will this be merged? |
|
I wonder the same... lol |
|
Thanks for the test / completing it - but I cannot merge it myself. |
|
@jpadilla José, can you review this PR please? Gracias! |
|
Can you please merge and release this PR? |
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. |
|
@Alex3917 I'm bit confused. Then why refresh mechanism exists? I totally agree with @edelvalle. |
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 ( 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 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. |
|
Image we have the following use case of a generic API and as @Alex3917 mentioned we have configured:
The question is: How can I refresh the token?
|
|
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. |
|
The commits which came from #274 should have @jorrit-wehelp as the author in the commit metadata. |
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