-
Notifications
You must be signed in to change notification settings - Fork 185
Add RefreshOIDCAccessToken middleware #377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,18 @@ | ||
import json | ||
import logging | ||
import time | ||
|
||
from django.contrib.auth import BACKEND_SESSION_KEY | ||
from django.contrib import auth | ||
from django.http import HttpResponseRedirect, JsonResponse | ||
from django.urls import reverse | ||
from django.utils.crypto import get_random_string | ||
from django.utils.deprecation import MiddlewareMixin | ||
from django.utils.functional import cached_property | ||
from django.utils.module_loading import import_string | ||
import requests | ||
from requests.auth import HTTPBasicAuth | ||
|
||
from mozilla_django_oidc.auth import OIDCAuthenticationBackend | ||
from mozilla_django_oidc.auth import OIDCAuthenticationBackend, store_tokens | ||
from mozilla_django_oidc.utils import (absolutify, | ||
add_state_and_nonce_to_session, | ||
import_from_settings) | ||
|
@@ -95,6 +98,11 @@ def exempt_url_patterns(self): | |
exempt_patterns.add(url_pattern) | ||
return exempt_patterns | ||
|
||
@property | ||
def logout_redirect_url(self): | ||
"""Return the logout url defined in settings.""" | ||
return self.get_settings('LOGOUT_REDIRECT_URL', '/') | ||
|
||
def is_refreshable_url(self, request): | ||
"""Takes a request and returns whether it triggers a refresh examination | ||
|
@@ -104,7 +112,7 @@ def is_refreshable_url(self, request): | |
""" | ||
# Do not attempt to refresh the session if the OIDC backend is not used | ||
backend_session = request.session.get(BACKEND_SESSION_KEY) | ||
backend_session = request.session.get(auth.BACKEND_SESSION_KEY) | ||
is_oidc_enabled = True | ||
if backend_session: | ||
auth_backend = import_string(backend_session) | ||
|
@@ -118,27 +126,71 @@ def is_refreshable_url(self, request): | |
not any(pat.match(request.path) for pat in self.exempt_url_patterns) | ||
) | ||
|
||
def process_request(self, request): | ||
def is_expired(self, request): | ||
if not self.is_refreshable_url(request): | ||
LOGGER.debug('request is not refreshable') | ||
return | ||
return False | ||
|
||
expiration = request.session.get('oidc_id_token_expiration', 0) | ||
expiration = request.session.get('oidc_token_expiration', 0) | ||
now = time.time() | ||
if expiration > now: | ||
# The id_token is still valid, so we don't have to do anything. | ||
LOGGER.debug('id token is still valid (%s > %s)', expiration, now) | ||
return False | ||
|
||
return True | ||
|
||
def process_request(self, request): | ||
if not self.is_expired(request): | ||
return | ||
|
||
LOGGER.debug('id token has expired') | ||
# The id_token has expired, so we have to re-authenticate silently. | ||
return self.finish(request, prompt_reauth=True) | ||
|
||
def finish(self, request, prompt_reauth=True): | ||
"""Finish request handling and handle sending downstream responses for XHR. | ||
This function should only be run if the session is determind to | ||
be expired. | ||
Almost all XHR request handling in client-side code struggles | ||
with redirects since redirecting to a page where the user | ||
is supposed to do something is extremely unlikely to work | ||
in an XHR request. Make a special response for these kinds | ||
of requests. | ||
The use of 403 Forbidden is to match the fact that this | ||
middleware doesn't really want the user in if they don't | ||
refresh their session. | ||
""" | ||
default_response = None | ||
xhr_response_json = {'error': 'the authentication session has expired'} | ||
if prompt_reauth: | ||
# The id_token has expired, so we have to re-authenticate silently. | ||
refresh_url = self._prepare_reauthorization(request) | ||
default_response = HttpResponseRedirect(refresh_url) | ||
xhr_response_json['refresh_url'] = refresh_url | ||
|
||
if request.headers.get('x-requested-with') == 'XMLHttpRequest': | ||
xhr_response = JsonResponse(xhr_response_json, status=403) | ||
if 'refresh_url' in xhr_response_json: | ||
xhr_response['refresh_url'] = xhr_response_json['refresh_url'] | ||
return xhr_response | ||
else: | ||
return default_response | ||
|
||
def _prepare_reauthorization(self, request): | ||
# Constructs a new authorization grant request to refresh the session. | ||
# Besides constructing the request, the state and nonce included in the | ||
# request are registered in the current session in preparation for the | ||
# client following through with the authorization flow. | ||
auth_url = self.OIDC_OP_AUTHORIZATION_ENDPOINT | ||
client_id = self.OIDC_RP_CLIENT_ID | ||
state = get_random_string(self.OIDC_STATE_SIZE) | ||
|
||
# Build the parameters as if we were doing a real auth handoff, except | ||
# we also include prompt=none. | ||
params = { | ||
auth_params = { | ||
'response_type': 'code', | ||
'client_id': client_id, | ||
'redirect_uri': absolutify( | ||
|
@@ -152,26 +204,85 @@ def process_request(self, request): | |
|
||
if self.OIDC_USE_NONCE: | ||
nonce = get_random_string(self.OIDC_NONCE_SIZE) | ||
params.update({ | ||
auth_params.update({ | ||
'nonce': nonce | ||
}) | ||
|
||
add_state_and_nonce_to_session(request, state, params) | ||
|
||
# Register the one-time parameters in the session | ||
add_state_and_nonce_to_session(request, state, auth_params) | ||
request.session['oidc_login_next'] = request.get_full_path() | ||
|
||
query = urlencode(params, quote_via=quote) | ||
redirect_url = '{url}?{query}'.format(url=auth_url, query=query) | ||
if request.headers.get('x-requested-with') == 'XMLHttpRequest': | ||
# Almost all XHR request handling in client-side code struggles | ||
# with redirects since redirecting to a page where the user | ||
# is supposed to do something is extremely unlikely to work | ||
# in an XHR request. Make a special response for these kinds | ||
# of requests. | ||
# The use of 403 Forbidden is to match the fact that this | ||
# middleware doesn't really want the user in if they don't | ||
# refresh their session. | ||
response = JsonResponse({'refresh_url': redirect_url}, status=403) | ||
response['refresh_url'] = redirect_url | ||
return response | ||
return HttpResponseRedirect(redirect_url) | ||
query = urlencode(auth_params, quote_via=quote) | ||
return '{auth_url}?{query}'.format(auth_url=auth_url, query=query) | ||
|
||
|
||
class RefreshOIDCAccessToken(SessionRefresh): | ||
""" | ||
A middleware that will refresh the access token following proper OIDC protocol: | ||
https://auth0.com/docs/tokens/refresh-token/current | ||
""" | ||
def process_request(self, request): | ||
if not self.is_expired(request): | ||
return | ||
|
||
token_url = import_from_settings('OIDC_OP_TOKEN_ENDPOINT') | ||
client_id = import_from_settings('OIDC_RP_CLIENT_ID') | ||
client_secret = import_from_settings('OIDC_RP_CLIENT_SECRET') | ||
refresh_token = request.session.get('oidc_refresh_token') | ||
|
||
if not refresh_token: | ||
LOGGER.debug('no refresh token stored') | ||
return self.finish(request, prompt_reauth=True) | ||
|
||
token_payload = { | ||
'grant_type': 'refresh_token', | ||
'client_id': client_id, | ||
'client_secret': client_secret, | ||
'refresh_token': refresh_token, | ||
} | ||
|
||
req_auth = None | ||
if self.get_settings('OIDC_TOKEN_USE_BASIC_AUTH', False): | ||
# When Basic auth is defined, create the Auth Header and remove secret from payload. | ||
user = token_payload.get('client_id') | ||
pw = token_payload.get('client_secret') | ||
|
||
req_auth = HTTPBasicAuth(user, pw) | ||
del token_payload['client_secret'] | ||
|
||
try: | ||
response = requests.post( | ||
diurnalist marked this conversation as resolved.
Show resolved
Hide resolved
|
||
token_url, | ||
auth=req_auth, | ||
data=token_payload, | ||
verify=import_from_settings('OIDC_VERIFY_SSL', True) | ||
) | ||
response.raise_for_status() | ||
token_info = response.json() | ||
except requests.exceptions.Timeout: | ||
LOGGER.debug('timed out refreshing access token') | ||
# Don't prompt for reauth as this could be a temporary problem | ||
return self.finish(request, prompt_reauth=False) | ||
except requests.exceptions.HTTPError as exc: | ||
status_code = exc.response.status_code | ||
LOGGER.debug('http error %s when refreshing access token', status_code) | ||
# OAuth error response will be a 400 for various situations, including | ||
# an expired token. https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 | ||
return self.finish(request, prompt_reauth=(status_code == 400)) | ||
except json.JSONDecodeError: | ||
LOGGER.debug('malformed response when refreshing access token') | ||
# Don't prompt for reauth as this could be a temporary problem | ||
return self.finish(request, prompt_reauth=False) | ||
except Exception as exc: | ||
LOGGER.debug( | ||
'unknown error occurred when refreshing access token: %s', exc) | ||
# Don't prompt for reauth as this could be a temporary problem | ||
return self.finish(request, prompt_reauth=False) | ||
|
||
# Until we can properly validate an ID token on the refresh response | ||
# per the spec[1], we intentionally drop the id_token. | ||
# [1]: https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on cursory reading of the spec linked here, it seems like it could be possible to fulfill the specified behavior by decoding the old and new ID tokens, and comparing the props between them, in addition to using the information from settings. Is that correct? In that case, someone could relatively trivially make a follow-up PR implementing update of ID token too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my understanding, what we could do is first verify the token's integrity analogously to here and then additionally check |
||
id_token = None | ||
access_token = token_info.get('access_token') | ||
refresh_token = token_info.get('refresh_token') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The refresh token is optional. So this doesn't work, for example, for amazon cognito, as they don't return new refresh token. I think thee refresh_token should only be stored when it's contained in the response, otherwise keep the old one. |
||
store_tokens(request.session, access_token, id_token, refresh_token) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -49,9 +49,13 @@ def login_success(self): | |||||
auth.login(self.request, self.user) | ||||||
|
||||||
# Figure out when this id_token will expire. This is ignored unless you're | ||||||
# using the RenewIDToken middleware. | ||||||
expiration_interval = self.get_settings('OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS', 60 * 15) | ||||||
self.request.session['oidc_id_token_expiration'] = time.time() + expiration_interval | ||||||
# using the SessionRefresh or RefreshOIDCAccessToken middlewares. | ||||||
expiration_interval = self.get_settings( | ||||||
'OIDC_RENEW_TOKEN_EXPIRY_SECONDS', | ||||||
# 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
return HttpResponseRedirect(self.success_url) | ||||||
|
||||||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently it doesn't seem like self.request.session['oidc_token_expiration'] is ever updated when updating tokens using refresh-token, only in the login flow view. This seems to lead to refresh-token being used to refresh tokens upon every new request after the initial expiry is hit.
Should perhaps oidc_token_expiration be set here in store_tokens in addition to in the login view?
Ideally perhaps from the access or id tokens exp field (minus some small margin) - perhaps in a future PR