auth_oidc: verify and store id_token nonce (fixes #29009)

This commit is contained in:
Benjamin Dauvergne 2018-12-14 10:42:14 +01:00
parent 0e34001537
commit b4110b3b3c
4 changed files with 34 additions and 17 deletions

View File

@ -341,7 +341,7 @@ def get_nonce(request):
return nonce
def record_authentication_event(request, how):
def record_authentication_event(request, how, nonce=None):
'''Record an authentication event in the session and in the database, in
later version the database persistence can be removed'''
from . import models
@ -362,7 +362,7 @@ def record_authentication_event(request, how):
'who': unicode(request.user)[:80],
'how': how,
}
nonce = get_nonce(request)
nonce = nonce or get_nonce(request)
if nonce:
kwargs['nonce'] = nonce
event['nonce'] = nonce
@ -388,7 +388,7 @@ def last_authentication_event(session):
return None
def login(request, user, how, service_slug=None, **kwargs):
def login(request, user, how, service_slug=None, nonce=None, **kwargs):
'''Login a user model, record the authentication event and redirect to next
URL or settings.LOGIN_REDIRECT_URL.'''
from . import hooks
@ -400,7 +400,7 @@ def login(request, user, how, service_slug=None, **kwargs):
if constants.LAST_LOGIN_SESSION_KEY not in request.session:
request.session[constants.LAST_LOGIN_SESSION_KEY] = \
localize(to_current_timezone(last_login), True)
record_authentication_event(request, how)
record_authentication_event(request, how, nonce=nonce)
hooks.call_hooks('event', name='login', user=user, how=how, service=service_slug)
return continue_to_next_url(request, **kwargs)

View File

@ -6,7 +6,6 @@ import requests
from jwcrypto.jwt import JWT
from jwcrypto.jwk import JWK
from django.core.exceptions import MultipleObjectsReturned
from django.utils.timezone import now
from django.contrib.auth import get_user_model
from django.contrib.auth.backends import ModelBackend
@ -20,7 +19,7 @@ from . import models, utils
class OIDCBackend(ModelBackend):
def authenticate(self, access_token=None, id_token=None, **kwargs):
def authenticate(self, access_token=None, id_token=None, nonce=None, **kwargs):
logger = logging.getLogger(__name__)
if id_token is None:
return
@ -96,6 +95,12 @@ class OIDCBackend(ModelBackend):
duration)
return None
id_token_nonce = getattr(id_token, 'nonce', None)
if nonce and nonce != id_token_nonce:
logger.warning('auth_oidc: id_token nonce %r != expected nonce %r',
id_token_nonce, nonce)
return None
User = get_user_model()
user = None
if provider.strategy == models.OIDCProvider.STRATEGY_FIND_UUID:

View File

@ -92,6 +92,9 @@ class LoginCallback(View):
messages.warning(request, _('Login with OpenIDConnect failed, state lost.'))
logger.warning('auth_oidc: state lost')
return redirect(request, settings.LOGIN_REDIRECT_URL)
oidc_request = oidc_state.get('request')
assert isinstance(oidc_request, dict), 'state is not properly initialized'
nonce = oidc_request.get('nonce')
try:
issuer = oidc_state.get('issuer')
provider = get_provider_by_issuer(issuer)
@ -176,7 +179,7 @@ class LoginCallback(View):
return self.continue_to_next_url()
logger.info(u'got token response %s', result)
access_token = result.get('access_token')
user = authenticate(access_token=access_token, id_token=result['id_token'])
user = authenticate(access_token=access_token, nonce=nonce, id_token=result['id_token'])
if user:
# remember last tokens for logout
tokens = request.session.setdefault('auth_oidc', {}).setdefault('tokens', [])
@ -185,7 +188,7 @@ class LoginCallback(View):
'provider_pk': provider.pk,
})
request.session.modified = True
login(request, user, 'oidc')
login(request, user, 'oidc', nonce=nonce)
else:
messages.warning(request, _('No user found'))
return self.continue_to_next_url()

View File

@ -21,7 +21,7 @@ from authentic2_auth_oidc.utils import (base64url_decode, parse_id_token, IDToke
has_providers)
from authentic2_auth_oidc.models import OIDCProvider, OIDCClaimMapping
from authentic2.models import AttributeValue
from authentic2.utils import timestamp_from_datetime
from authentic2.utils import timestamp_from_datetime, last_authentication_event
from authentic2.a2_rbac.utils import get_default_ou
from authentic2.crypto import base64url_encode
@ -166,7 +166,7 @@ def code():
def oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, extra_id_token=None,
extra_user_info=None, sub='john.doe'):
extra_user_info=None, sub='john.doe', nonce=None):
token_endpoint = urlparse.urlparse(oidc_provider.token_endpoint)
userinfo_endpoint = urlparse.urlparse(oidc_provider.userinfo_endpoint)
token_revocation_endpoint = urlparse.urlparse(oidc_provider.token_revocation_endpoint)
@ -181,6 +181,8 @@ def oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, extra_id_token
'aud': str(oidc_provider.client_id),
'exp': timestamp_from_datetime(now() + datetime.timedelta(seconds=10)),
}
if nonce:
id_token['nonce'] = nonce
if extra_id_token:
id_token.update(extra_id_token)
@ -277,6 +279,8 @@ def test_sso(app, caplog, code, oidc_provider, oidc_provider_jwkset, login_url,
assert query['client_id'] == str(oidc_provider.client_id)
assert query['scope'] == 'openid'
assert query['redirect_uri'] == 'http://testserver' + reverse('oidc-login-callback')
# get the nonce
nonce = app.session['auth_oidc'][query['state']]['request']['nonce']
if oidc_provider.claims_parameter_supported:
claims = json.loads(query['claims'])
@ -312,9 +316,12 @@ def test_sso(app, caplog, code, oidc_provider, oidc_provider_jwkset, login_url,
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code,
extra_id_token={'aud': 'zz'}):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
with utils.check_log(caplog, 'expected nonce'):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert not hooks.auth_oidc_backend_modify_user
with utils.check_log(caplog, 'created user'):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert len(hooks.auth_oidc_backend_modify_user) == 1
assert set(hooks.auth_oidc_backend_modify_user[0]['kwargs']) >= set(['user', 'provider', 'user_info', 'id_token', 'access_token'])
@ -330,21 +337,22 @@ def test_sso(app, caplog, code, oidc_provider, oidc_provider_jwkset, login_url,
assert user.attributes.last_name == 'Doe'
assert AttributeValue.objects.filter(content='John', verified=True).count() == 1
assert AttributeValue.objects.filter(content='Doe', verified=False).count() == 1
assert last_authentication_event(app.session)['nonce'] == nonce
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code,
extra_user_info={'family_name_verified': True}):
extra_user_info={'family_name_verified': True}, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert AttributeValue.objects.filter(content='Doe', verified=False).count() == 0
assert AttributeValue.objects.filter(content='Doe', verified=True).count() == 1
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code,
extra_user_info={'ou': 'cassis'}):
extra_user_info={'ou': 'cassis'}, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert User.objects.count() == 1
user = User.objects.get()
assert user.ou == cassis
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert User.objects.count() == 1
user = User.objects.get()
@ -403,15 +411,16 @@ def test_strategy_find_uuid(app, caplog, code, oidc_provider, oidc_provider_jwks
response = response.click(oidc_provider.name)
location = urlparse.urlparse(response.location)
query = check_simple_qs(urlparse.parse_qs(location.query))
nonce = app.session['auth_oidc'][query['state']]['request']['nonce']
# sub=john.doe, MUST not work
with utils.check_log(caplog, 'cannot create user'):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
# sub=simple_user.uuid MUST work
with utils.check_log(caplog, 'found user using UUID'):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, sub=simple_user.uuid):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, sub=simple_user.uuid, nonce=nonce):
response = app.get(login_callback_url, params={'code': code, 'state': query['state']})
assert urlparse.urlparse(response['Location']).path == '/'
@ -427,6 +436,6 @@ def test_strategy_find_uuid(app, caplog, code, oidc_provider, oidc_provider_jwks
response = app.get(reverse('account_management'))
with utils.check_log(caplog, 'revoked token from OIDC'):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code):
with oidc_provider_mock(oidc_provider, oidc_provider_jwkset, code, nonce=nonce):
response = response.click(href='logout')
assert 'https://idp.example.com/logout' in response.content