Skip to content

Commit

Permalink
Merge pull request #52 from akatsoulas/add-nonce
Browse files Browse the repository at this point in the history 
Add Nonce support.
  • Loading branch information
@akatsoulas
akatsoulas committed Nov 16, 2016
2 parents 72ed275 + 698c12a commit f71f9b6
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 15 deletions.
23 changes: 17 additions & 6 deletions mozilla_django_oidc/auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
except ImportError:
from django.utils.encoding import smart_str as smart_bytes
from django.contrib.auth import get_user_model
from django.core.exceptions import SuspiciousOperation
from django.core.urlresolvers import reverse

from mozilla_django_oidc.utils import absolutify, import_from_settings
Expand All @@ -18,6 +19,7 @@


def default_username_algo(email):
"""Generate username for the Django user."""
# bluntly stolen from django-browserid
# store the username as a base64 encoded sha224 of the email address
# this protects against data leakage because usernames are often
Expand Down Expand Up @@ -65,21 +67,30 @@ def create_user(self, claims):

def verify_token(self, token, **kwargs):
"""Validate the token signature."""

nonce = kwargs.get('nonce')
# Get JWT audience without signature verification
audience = jwt.decode(token, verify=False)['aud']

secret = self.OIDC_RP_CLIENT_SECRET
if import_from_settings('OIDC_RP_CLIENT_SECRET_ENCODED', False):
secret = base64.urlsafe_b64decode(self.OIDC_RP_CLIENT_SECRET)

return jwt.decode(token, secret,
verify=import_from_settings('OIDC_VERIFY_JWT', True),
audience=audience)
id_token = jwt.decode(token, secret,
verify=import_from_settings('OIDC_VERIFY_JWT', True),
audience=audience)

if import_from_settings('OIDC_USE_NONCE', True) and nonce != id_token['nonce']:
msg = 'JWT Nonce verification failed.'
raise SuspiciousOperation(msg)
return id_token

def authenticate(self, code=None, state=None):
def authenticate(self, **kwargs):
"""Authenticates a user based on the OIDC code flow."""

code = kwargs.pop('code', None)
state = kwargs.pop('state', None)
nonce = kwargs.pop('nonce', None)

if not code or not state:
return None

Expand All @@ -99,7 +110,7 @@ def authenticate(self, code=None, state=None):

# Validate the token
token_response = response.json()
payload = self.verify_token(token_response.get('id_token'))
payload = self.verify_token(token_response.get('id_token'), nonce=nonce)

if payload:
access_token = token_response.get('access_token')
Expand Down
15 changes: 14 additions & 1 deletion mozilla_django_oidc/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,16 @@ def login_success(self):
def get(self, request):
"""Callback handler for OIDC authorization code flow"""

nonce = request.session.get('oidc_nonce')
if nonce:
# Make sure that nonce is not used twice
del request.session['oidc_nonce']

if 'code' in request.GET and 'state' in request.GET:
kwargs = {
'code': request.GET['code'],
'state': request.GET['state']
'state': request.GET['state'],
'nonce': nonce
}

if 'oidc_state' not in request.session:
Expand Down Expand Up @@ -81,6 +87,13 @@ def get(self, request):
'state': state,
}

if import_from_settings('OIDC_USE_NONCE', True):
nonce = get_random_string(import_from_settings('OIDC_NONCE_SIZE', 32))
params.update({
'nonce': nonce
})
request.session['oidc_nonce'] = nonce

request.session['oidc_state'] = state
request.session['oidc_login_next'] = request.GET.get(redirect_field_name)

Expand Down
31 changes: 28 additions & 3 deletions tests/test_auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.exceptions import SuspiciousOperation
from django.test import TestCase, override_settings

from mozilla_django_oidc.auth import OIDCAuthenticationBackend
Expand Down Expand Up @@ -81,7 +82,7 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
'redirect_uri': 'http://site-url.com/callback/'
}
self.assertEqual(self.backend.authenticate(code='foo', state='bar'), user)
token_mock.assert_called_once_with('id_token')
token_mock.assert_called_once_with('id_token', nonce=None)
request_mock.post.assert_called_once_with('https://server.example.com/token',
data=post_data,
verify=True)
Expand Down Expand Up @@ -125,7 +126,7 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
self.assertEquals(user.email, 'email@example.com')
self.assertEquals(user.username, 'username_algo')

token_mock.assert_called_once_with('id_token')
token_mock.assert_called_once_with('id_token', nonce=None)
request_mock.post.assert_called_once_with('https://server.example.com/token',
data=post_data,
verify=True)
Expand All @@ -139,6 +140,7 @@ def test_authenticate_no_code_no_state(self):

self.assertEqual(self.backend.authenticate(code='', state=''), None)

@override_settings(OIDC_USE_NONCE=False)
@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
def test_jwt_decode_params(self, request_mock, jwt_mock):
Expand Down Expand Up @@ -167,6 +169,7 @@ def test_jwt_decode_params(self, request_mock, jwt_mock):
jwt_mock.decode.assert_has_calls(calls)

@override_settings(OIDC_VERIFY_JWT=False)
@override_settings(OIDC_USE_NONCE=False)
@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
def test_jwt_decode_params_verify_false(self, request_mock, jwt_mock):
Expand Down Expand Up @@ -195,7 +198,22 @@ def test_jwt_decode_params_verify_false(self, request_mock, jwt_mock):
self.backend.authenticate(code='foo', state='bar')
jwt_mock.decode.assert_has_calls(calls)

@override_settings(OIDC_USE_NONCE=True)
@patch('mozilla_django_oidc.auth.jwt')
def test_jwt_failed_nonce(self, jwt_mock):
"""Test Nonce verification."""

jwt_mock.decode.return_value = {
'nonce': 'foobar',
'aud': 'aud'
}
id_token = 'my_token'
with self.assertRaises(SuspiciousOperation) as context:
self.backend.verify_token(id_token, **{'nonce': 'foo'})
self.assertEqual('JWT Nonce verification failed.', str(context.exception))

@override_settings(OIDC_CREATE_USER=False)
@override_settings(OIDC_USE_NONCE=False)
@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
def test_create_user_disabled(self, request_mock, jwt_mock):
Expand All @@ -218,6 +236,7 @@ def test_create_user_disabled(self, request_mock, jwt_mock):

@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
@override_settings(OIDC_USE_NONCE=False)
def test_create_user_enabled(self, request_mock, jwt_mock):
"""Test with user creation enabled and no user found."""

Expand All @@ -239,6 +258,7 @@ def test_create_user_enabled(self, request_mock, jwt_mock):
User.objects.get(email='email@example.com'))

@patch.object(settings, 'OIDC_USERNAME_ALGO')
@override_settings(OIDC_USE_NONCE=False)
@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
def test_custom_username_algo(self, request_mock, jwt_mock, algo_mock):
Expand All @@ -262,6 +282,7 @@ def test_custom_username_algo(self, request_mock, jwt_mock, algo_mock):
self.assertEqual(self.backend.authenticate(code='foo', state='bar'),
User.objects.get(username='username_algo'))

@override_settings(OIDC_USE_NONCE=False)
@patch('mozilla_django_oidc.auth.jwt')
@patch('mozilla_django_oidc.auth.requests')
def test_duplicate_emails(self, request_mock, jwt_mock):
Expand All @@ -282,4 +303,8 @@ def test_duplicate_emails(self, request_mock, jwt_mock):
'access_token': 'access_granted'
}
request_mock.post.return_value = post_json_mock
self.assertEqual(self.backend.authenticate(code='foo', state='bar'), None)
auth_kwargs = {
'code': 'foo',
'state': 'bar'
}
self.assertEqual(self.backend.authenticate(**auth_kwargs), None)
52 changes: 47 additions & 5 deletions tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ def test_get_auth_success(self):
mock_auth.return_value = user
response = callback_view(request)

mock_auth.assert_called_once_with(code='example_code', state='example_state')
mock_auth.assert_called_once_with(code='example_code',
state='example_state',
nonce=None)
mock_login.assert_called_once_with(request, user)

self.assertEqual(response.status_code, 302)
Expand Down Expand Up @@ -69,7 +71,9 @@ def test_get_auth_success_next_url(self):
mock_auth.return_value = user
response = callback_view(request)

mock_auth.assert_called_once_with(code='example_code', state='example_state')
mock_auth.assert_called_once_with(code='example_code',
state='example_state',
nonce=None)
mock_login.assert_called_once_with(request, user)

self.assertEqual(response.status_code, 302)
Expand All @@ -94,7 +98,9 @@ def test_get_auth_failure_nonexisting_user(self):
mock_auth.return_value = None
response = callback_view(request)

mock_auth.assert_called_once_with(code='example_code', state='example_state')
mock_auth.assert_called_once_with(code='example_code',
state='example_state',
nonce=None)

self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, '/failure')
Expand Down Expand Up @@ -122,11 +128,14 @@ def test_get_auth_failure_inactive_user(self):
mock_auth.return_value = user
response = callback_view(request)

mock_auth.assert_called_once_with(code='example_code', state='example_state')
mock_auth.assert_called_once_with(code='example_code',
state='example_state',
nonce=None)

self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, '/failure')

@override_settings(OIDC_USE_NONCE=False)
@override_settings(LOGIN_REDIRECT_URL_FAILURE='/failure')
def test_get_auth_dirty_data(self):
"""Test authentication attempt with wrong get data."""
Expand All @@ -136,6 +145,7 @@ def test_get_auth_dirty_data(self):

url = reverse('oidc_authentication_callback')
request = self.factory.get(url, get_data)
request.session = {}
callback_view = views.OIDCAuthenticationCallbackView.as_view()
response = callback_view(request)
self.assertEqual(response.status_code, 302)
Expand Down Expand Up @@ -189,6 +199,37 @@ def test_get_auth_failure_tampered_session_state(self):
expected_error_message = 'Session `oidc_state` does not match the OIDC callback state'
self.assertEqual(context.exception.args, (expected_error_message,))

@override_settings(LOGIN_REDIRECT_URL='/success')
def test_nonce_is_deleted(self):
"""Test Nonce is not in session."""
user = User.objects.create_user('example_username')

get_data = {
'code': 'example_code',
'state': 'example_state'
}
url = reverse('oidc_authentication_callback')
request = self.factory.get(url, get_data)
request.session = {
'oidc_state': 'example_state',
'oidc_nonce': 'example_nonce'
}
callback_view = views.OIDCAuthenticationCallbackView.as_view()

with patch('mozilla_django_oidc.views.auth.authenticate') as mock_auth:
with patch('mozilla_django_oidc.views.auth.login') as mock_login:
mock_auth.return_value = user
response = callback_view(request)

mock_auth.assert_called_once_with(code='example_code',
state='example_state',
nonce='example_nonce')
mock_login.assert_called_once_with(request, user)

self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, '/success')
self.assertTrue('oidc_nonce' not in request.session)


class OIDCAuthorizationRequestViewTestCase(TestCase):
def setUp(self):
Expand All @@ -214,7 +255,8 @@ def test_get(self, mock_random_string):
'scope': ['openid'],
'client_id': ['example_id'],
'redirect_uri': ['http://site-url.com/callback/'],
'state': ['examplestring']
'state': ['examplestring'],
'nonce': ['examplestring']
}
self.assertDictEqual(parse_qs(o.query), expected_query)
self.assertEqual(o.hostname, 'server.example.com')
Expand Down

0 comments on commit f71f9b6

Please sign in to comment.