Add RefreshOIDCAccessToken middleware #377
Conversation
|
@akatsoulas , I think the other PR had been sitting for quite some time, if you get a chance to review this (and maybe @GermanoGuerrini as well?), maybe it can move forward this time. Of course, feel free to close this and address #301 instead. I just wanted this functionality for my own deploy, saw it was already done (woo!) and then took the liberty of cleaning it up in hopes that it could make a merge soon. 👋 🚪 🚶♂️ |
diurnalist
force-pushed
the
master
branch
3 times, most recently
from
fbb7235
to
c304785
Compare
|
I've also added error handling for when the refresh fails... prior, it would bomb the entire request, which to me is not expected. |
|
@akatsoulas Any update on that? |
|
Can we have a look at this PR? This is a much needed feature.. |
|
What is needed for this PR to be looked at? It's kind of a crucial piece of any integration of OIDC into django unless you like constantly logging in over and over, and it's just been sitting here. It would be nice to get this merged so I can stop maintaining my own branch- what exactly needs done here? |
|
Hey all, saw there was some chatter on this and decided to freshen up the PR. I did a few things:
Hopefully now that it is rebased and the tests (should) pass, it will be in better shape for merging. @akatsoulas, I would be thankful for your time in review. |
diurnalist
force-pushed
the
master
branch
2 times, most recently
from
956fed2
to
075e232
Compare
The OP can provide a refresh_token to the client on authentication. This can later be used to get a new access_token. Typically refresh_tokens have a longer TTL than access_tokens and represent the total allowed session length. As a bonus, the refresh happens in the background and does not require taking the user to a new location (which also makes it more compatible with e.g., XHR). If there is no refresh token stored, making refreshing impossible, OR the refresh request fails with a 401, indicating the OP session has terminated, the user is taken through a refresh flow similar to the SessionRefresh middleware. If any error occurs during refresh, the middleware aborts, but does not perform any cleanup on the session. Co-Authored-By: Jason Anderson <jasonanderson@uchicago.edu>
|
For those interested in (or even using) this patch, I pushed a fix for an issue we found, where it was not prompting for reauth if the refresh token request returned a 400 error. I had assumed incorrectly that a 401 would be returned, but that is not in the OAuth 2.0 spec. |
+1, what can we help with maintainers? |
Any progress? Would love to see this merged! |
| # [1]: https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse | ||
| id_token = None | ||
| access_token = token_info.get('access_token') | ||
| refresh_token = token_info.get('refresh_token') |
There was a problem hiding this comment.
The refresh token is optional.
https://www.ietf.org/rfc/rfc6749.txt
So this doesn't work, for example, for amazon cognito, as they don't return new refresh token.
https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html
I think thee refresh_token should only be stored when it's contained in the response, otherwise keep the old one.
| # Handle old configuration value | ||
| self.get_settings('OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS', 60 * 15) | ||
| ) | ||
| self.request.session['oidc_token_expiration'] = time.time() + expiration_interval |
There was a problem hiding this comment.
| self.request.session['oidc_token_expiration'] = time.time() + expiration_interval | |
| self.request.session['oidc_id_token_expiration'] = time.time() + expiration_interval |
Note: this was originally proposed as #301. I took the nice work by @GermanoGuerrini and compressed it into a single commit, removed the HISTORY.rst change, and also changed one thing: now, the id_token is dropped from the session, because it cannot be verified securely. This is called out in the documentation. In my use-case, and perhaps many others, this is not a problem, as the access_token is the most important thing to preserve, as it's used to authenticate on behalf of the user to other services.
The OP can provide a refresh_token to the client on authentication. This
can later be used to get a new access_token. Typically refresh_tokens
have a longer TTL than access_tokens and represent the total allowed
session length. As a bonus, the refresh happens in the background and
does not require taking the user to a new location (which also makes it
more compatible with e.g., XHR).
This adds a new middleware to perform the refreshing of the access token
in the background.