Add refresh token feature

[alvinchow86]

Added an optional endpoint that can refresh a token. This can be useful for a webapp to try to keep a user logged in without having to re-enter their password constantly. Got the idea from this post https://auth0.com/blog/2014/01/27/ten-things-you-should-know-about-tokens-and-cookies/

Basically, if JWT_ALLOW_TOKEN_RENEWAL is True, then an extra field orig_iat (stands for original issue at time) is stored on the returned JWT token. You can then pass the token to the RefreshJSONWebToken view, and if orig_iat hasn't yet expired, you get a token with a refreshed expiration time (but with the same orig_iat, so you can only do refreshes up to some time limit, specified in JWT_TOKEN_RENEWAL_LIMIT). Getting a brand new token (by passing in username/password) will return a completely new orig_iat though.

I also added a new handler, jwt_get_user_id_from_payload, that lets you specify how the user_id field is stored. Since it's possible for the custom payload handler to change how the user id is stored, it seems to make sense to have a way to retrieve it too.

Feedback welcome (there are some things that could probably be cleaned up). I didn't add tests yet but if there is interest in this PR I will write some (I did write a number of tests in my application that uses this, so I am reasonably confident it works).

[jpadilla]

Hey @alvinchow86 this seems great! Still have to take a look at it closely, but really appreciate the work you've put into this and no doubt we'll get this merged.

First thing, do you mind updating your fork with the latest from master which fixes failing tests from a recent change in DRF's compat module?

Second, we'd definitely need some tests for this before merging.

[alvinchow86]

Cool, I'll definitely work on some tests.

[jpadilla]

@simonluijk before merging this PR it needs some tests.
@alvinchow86 any status on these?

[alvinchow86]

sorry about the delay, I will try to push out some tests this week

[prantaaho]

Any news on this? If there is need for extra pair of hands, I could help with this as I need the functionality...

[alvinchow86]

In the middle of writing tests, will push soon

[simonluijk]

I just looked over the code, LGTM.

A small nitpick is why are you using timegm(datetime.utcnow().utctimetuple()) to get a timestamp int(time.time()) should be fine.

[alvinchow86]

Added tests, mainly for the refresh token API endpoint. I didn't write ones for the serializer (some of which would be redundant with the API tests, but let me know if they are needed).

Added a simtime.py module to help with mocking/fast-forwarding the current time. Basically I patch the datetime class with a custom class (however this needs to be done everywhere the datetime class is imported and used).

FYI I used timegm(datetime.utcnow().utctimetuple()) because that's what python-jwt uses =), I agree it's pretty clunky though.. I have no problem with int(time.time()) but I'll have to mock out the time class in addition to datetime for my simtime stuff to work.

[simonluijk]

Nice work. I didn't realize python-jwt used that. Ignore my nitpick then.

[alvinchow86]

Ah tests are failing on Python 2.6 and 3.2 but passing on 2.7.. guess I got to make some compatibility fixes

[jpadilla]

@alvinchow86 very excited for this, you've done a tremendous job on it. I haven't checked the failing tests yet, but shouldn't be hard to get fixed.

At a quick glance, I'm thinking there should be something simpler we could do regarding the simtime.py, but I might be wrong. You could check out the tests on pyjwt to see if we're using anything there that could help. As for tests, I think what you covered should be fine for now.

Will try to take a better look when tests are passing and give any additional feedback if any.

[alvinchow86]

let me see if I can rewrite the tests without forcing the time

[jpadilla]

@alvinchow86 where are you with this? I've got some time and I'd like to help however I can.

[alvinchow86]

was planning to wrap this up by today or tomorrow (sorry again for the delay)

[jpadilla]

@alvinchow86 no worries, if there's anything you need help with or would like to discuss, let me know.

[alvinchow86]

I integrated @liamlin's Python 2.6 fix for total_seconds. The Python 3 issue had something to do with my overriding datetime with SimulationDateTime; I guess the isinstance() override doesn't work in Python 3. Anyway I rewrote the tests to not use the simtime stuff so this isn't a problem anymore.

Travis now passes, I think this PR is ready. If you want I can squash the commits a bit.

[jpadilla]

@alvinchow86 again, great work on this. Definitely looks good to me. I think before merging this PR we need to document the new settings as well as a usage example for it.

[alvinchow86]

added documentation in README.

one thing I just noticed is I mix terminology (refresh in the code and renew in the settings), is this OK or should I just pick one?

[jpadilla]

@alvinchow86 we definitely should stick with only one, either refresh or renew. I'm more inclined towards probably using refresh. It'd also help to add a "scenario" in the Refresh Token part of the documentations. When would you want to use this and how might it fight into a real world scenario?

I'd also love to know how you're using this.

[alvinchow86]

Fixed wording and added use case notes. The basic idea is to try to keep a user "logged in" while they are actively using a site, without having to necessarily make the token expiration too long either. You can have a web-app that periodically checks if the current token is close to expiring, and if it is, refresh the token to push out the "session"'s expiration

[slat]

Thanks @alvinchow86
I'm thinking about using this shortly in an SPA. A combination of a timer that elapses close to when the token will expire and refresh if the application hasn't been idle past a threshold.
ie.
http://stackoverflow.com/questions/667555/detecting-idle-time-in-javascript-elegantly

I understand as the previous token will still be valid it is not necessary to suspend all requests while the new token is being fetched - the old token will still be valid (unless expired) after the new token is issued.

[jpadilla]

@alvinchow86 I think we have another wording problem with the JWT_TOKEN_REFRESH_LIMIT setting. It wasn't clear to me what exactly it meant after seeing that it's value was a time delta.

Would it help if it was called JWT_REFRESH_EXPIRATION_DELTA?

@gcollazo @slat do you have any input on this?

[gcollazo]

@jpadilla I agree the proposed name is better

👍 for JWT_REFRESH_EXPIRATION_DELTA

[alvinchow86]

makes sense, I renamed to JWT_REFRESH_EXPIRATION_DELTA. Along those lines, should JWT_ALLOW_TOKEN_REFRESH just be JWT_ALLOW_REFRESH or something

[jpadilla]

@alvinchow86 I like JWT_ALLOW_REFRESH

[alvinchow86]

done

[jpadilla]

@alvinchow86 Thanks again! This has been this project's biggest PR so far. I'll give it a try once again.

I'll also work on a Changelog and Contributors llist. Will update here when I release it to PyPI.

[gcollazo]

Great work everyone 👍

[jpadilla]

Just released v1.0.0 which includes this and a couple other things. Thanks again to everyone that made this happen, specially @alvinchow86! 👍

[jpadilla]

@alvinchow86 any reason why we used orig_iat here instead of iat? This seems to might have slipped by.

[alvinchow86]

iat time is a standard field in the JWT spec and means the time this particular token was issued. I didn't want to override that definition (and it's still useful to know), so I added a new field orig_iat to track the `iat of the original token in the refresh chain (it could be renamed anything else of course. is that what you were wondering about?

[jpadilla]

@alvinchow86 got it, thought we had incorrectly named it and thought it was meant to just be iat. Thanks for the clarification.