Users continue to be authenticated past access token expiry
daggaz opened this issue · 13 comments
I don't know if this is expected behaviour, but it seems wrong.
If you authenticate a user using this library, the user will retain a logged in session indefinitely (depending on your django session settings).
Should the user not be automatically logged out when the access token expires?
How long the token is valid for and how you decide to configure your sessions is in my opinion two different things in an MVC app. In restful (DRF) APIs, the token is validated on each request, but this is not true for sessions.
I can see an argument for that position, and you are right, in the DRF each request is authenticated.
However, if you're coming from the point of view of knowing that access tokens expire, it is somewhat surprising that authenticating with a JWT doesn't expire.
I wonder if there shouldn't be a middleware that's checking expiry and using refresh tokens to update the access token?
Alternatively, a middleware that redirects you back through login at/near the expiry.
Presumably either would be optional addons so that we preserve the current behaviour for existing installations.
I don't see why this has to be implemented? Set session time out to the same time as expiration of the token and everything is solved?
Typical access token expiry is very short. I think the default is 1 hour for ADFS.
This is what the refresh token is for, so you can transparently refresh your access token.
This also means that any claims/permissions/roles/whatever in the access token get automatically refreshed.
Any further thoughts on this issue?
Typical access token expiry is very short. I think the default is 1 hour for ADFS.
This is what the refresh token is for, so you can transparently refresh your access token.
This also means that any claims/permissions/roles/whatever in the access token get automatically refreshed.
@daggaz I'm still a bit confused here. Why does any of this relate to a session? I could see us benefiting from a docs change to let a user know to change their session timeout to slightly exceed their refresh token period, but I don't see the need for us to change something as I currently understand it.
So, there's these benefits:
- tokens are renewed, and groups therefor renewed, without the user having to log out and in again
- the user won't be redirected to ADFS once an hour (depending on company settings) if session is set to match token expiry
- sessions can match the token.
And on the other side:
- If you're signed in to ADFS, the redirect don't require you to log in again.
- You can just set the session to be as long as you'd like, but this would obviously mean the user has to log out and in to refresh groups, and won't match the expiry of the token.
How ever, normally, these things are handled by the frontend asynchronous in the background. I'm not sure how to implement this beautifully. You're suggesting if a request goes through a session, check the token, if expiring, refresh it?
I've added a PR with something I think works.
Lemme know what you think.
Tests will follow.
Updated PR with complete set of tests, and fixed up a few other things I found along the way.
Not updated docs. I'm not sure if you want to make this an optional addon, or add it to the standard setup instructions.
One thing to point out, is that if you are using the LoginRequiredMiddleware, the new adfs_refresh_middleware should go first. This is so that when the refresh token eventually expires (not the access token), and you are logged out, your will be correctly redirected to login.