Skip to content

Commit

Permalink
Merge pull request #359 from akatsoulas/get-settings-nonce
Browse files Browse the repository at this point in the history 
Move nonce outside of add_state_and_noce_to_session method.
  • Loading branch information
@akatsoulas
akatsoulas committed Jul 21, 2020
2 parents 0b3c756 + 73f85f1 commit 56773c1
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 56 deletions.
6 changes: 6 additions & 0 deletions mozilla_django_oidc/middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ def process_request(self, request):
'prompt': 'none',
}

if self.get_settings('OIDC_USE_NONCE', True):
nonce = get_random_string(self.get_settings('OIDC_NONCE_SIZE', 32))
params.update({
'nonce': nonce
})

add_state_and_nonce_to_session(request, state, params)

request.session['oidc_login_next'] = request.get_full_path()
Expand Down
8 changes: 1 addition & 7 deletions mozilla_django_oidc/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.utils.crypto import get_random_string

try:
from urllib.request import parse_http_list, parse_keqv_list
Expand Down Expand Up @@ -67,12 +66,7 @@ def add_state_and_nonce_to_session(request, state, params):
To keep the session space to a reasonable size, the dictionary is kept at 50 state/nonce
combinations maximum.
"""
nonce = None
if import_from_settings('OIDC_USE_NONCE', True):
nonce = get_random_string(import_from_settings('OIDC_NONCE_SIZE', 32))
params.update({
'nonce': nonce
})
nonce = params.get('nonce')

# Store Nonce with the State parameter in the session "oidc_states" dictionary.
# The dictionary can store multiple State/Nonce combinations to allow parallel
Expand Down
6 changes: 6 additions & 0 deletions mozilla_django_oidc/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ def get(self, request):

params.update(self.get_extra_params(request))

if self.get_settings('OIDC_USE_NONCE', True):
nonce = get_random_string(self.get_settings('OIDC_NONCE_SIZE', 32))
params.update({
'nonce': nonce
})

add_state_and_nonce_to_session(request, state, params)

request.session['oidc_login_next'] = get_next_url(request, redirect_field_name)
Expand Down
25 changes: 8 additions & 17 deletions tests/test_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,8 @@ def test_is_POST(self):
@override_settings(OIDC_RP_CLIENT_ID='foo')
@override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
@patch('mozilla_django_oidc.middleware.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_is_ajax(self, mock_utils_random, mock_middleware_random):
def test_is_ajax(self, mock_middleware_random):
mock_middleware_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'

request = self.factory.get(
'/foo',
Expand All @@ -80,7 +78,7 @@ def test_is_ajax(self, mock_utils_random, mock_middleware_random):
'response_type': ['code'],
'redirect_uri': ['http://testserver/callback/'],
'client_id': ['foo'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'prompt': ['none'],
'scope': ['openid email'],
'state': ['examplestring'],
Expand All @@ -93,11 +91,8 @@ def test_is_ajax(self, mock_utils_random, mock_middleware_random):
@override_settings(OIDC_RP_CLIENT_ID='foo')
@override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
@patch('mozilla_django_oidc.middleware.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_no_oidc_token_expiration_forces_renewal(self, mock_utils_random,
mock_middleware_random):
def test_no_oidc_token_expiration_forces_renewal(self, mock_middleware_random):
mock_middleware_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'

request = self.factory.get('/foo')
request.user = self.user
Expand All @@ -112,7 +107,7 @@ def test_no_oidc_token_expiration_forces_renewal(self, mock_utils_random,
'response_type': ['code'],
'redirect_uri': ['http://testserver/callback/'],
'client_id': ['foo'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'prompt': ['none'],
'scope': ['openid email'],
'state': ['examplestring'],
Expand All @@ -123,10 +118,8 @@ def test_no_oidc_token_expiration_forces_renewal(self, mock_utils_random,
@override_settings(OIDC_RP_CLIENT_ID='foo')
@override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
@patch('mozilla_django_oidc.middleware.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_expired_token_forces_renewal(self, mock_utils_random, mock_middleware_random):
def test_expired_token_forces_renewal(self, mock_middleware_random):
mock_middleware_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'

request = self.factory.get('/foo')
request.user = self.user
Expand All @@ -143,7 +136,7 @@ def test_expired_token_forces_renewal(self, mock_utils_random, mock_middleware_r
'response_type': ['code'],
'redirect_uri': ['http://testserver/callback/'],
'client_id': ['foo'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'prompt': ['none'],
'scope': ['openid email'],
'state': ['examplestring'],
Expand Down Expand Up @@ -263,10 +256,8 @@ def test_authenticated_user(self):
@override_settings(OIDC_RP_CLIENT_ID='foo')
@override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
@patch('mozilla_django_oidc.middleware.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_expired_token_redirects_to_sso(self, mock_utils_random, mock_middleware_random):
def test_expired_token_redirects_to_sso(self, mock_middleware_random):
mock_middleware_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'

client = ClientWithUser()
client.login(username=self.user.username, password='password')
Expand All @@ -286,7 +277,7 @@ def test_expired_token_redirects_to_sso(self, mock_utils_random, mock_middleware
'response_type': ['code'],
'redirect_uri': ['http://testserver/callback/'],
'client_id': ['foo'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'prompt': ['none'],
'scope': ['openid email'],
'state': ['examplestring'],
Expand Down
18 changes: 3 additions & 15 deletions tests/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,10 @@ def test_add_state_to_session(self):

add_state_and_nonce_to_session(self.request, state, params)

self.assertIn('nonce', params)
self.assertIn('oidc_states', self.request.session)
self.assertEqual(1, len(self.request.session['oidc_states']))
self.assertIn(state, self.request.session['oidc_states'].keys())

def test_existing_params(self):
state = 'example_state'

param_key = 'example_param'
params = {
param_key: 'example',
}

add_state_and_nonce_to_session(self.request, state, params)

self.assertIn('nonce', params)
self.assertIn(param_key, params)

def test_multiple_states(self):
state1 = 'example_state_1'
state2 = 'example_state_2'
Expand Down Expand Up @@ -143,7 +129,9 @@ def test_state_dictionary_without_nonce_format(self):

def test_state_dictionary_with_nonce_format(self):
state = 'example_state'
params = {}
params = {
'nonce': 'example_nonce'
}

add_state_and_nonce_to_session(self.request, state, params)

Expand Down
25 changes: 8 additions & 17 deletions tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,9 @@ def setUp(self):
@override_settings(OIDC_OP_AUTHORIZATION_ENDPOINT='https://server.example.com/auth')
@override_settings(OIDC_RP_CLIENT_ID='example_id')
@patch('mozilla_django_oidc.views.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_get(self, mock_utils_random, mock_views_random):
def test_get(self, mock_views_random):
"""Test initiation of a successful OIDC attempt."""
mock_views_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'
url = reverse('oidc_authentication_init')
request = self.factory.get(url)
request.session = dict()
Expand All @@ -404,7 +402,7 @@ def test_get(self, mock_utils_random, mock_views_random):
'client_id': ['example_id'],
'redirect_uri': ['http://testserver/callback/'],
'state': ['examplestring'],
'nonce': ['examplenonce']
'nonce': ['examplestring']
}
self.assertDictEqual(parse_qs(o.query), expected_query)
self.assertEqual(o.hostname, 'server.example.com')
Expand All @@ -415,11 +413,9 @@ def test_get(self, mock_utils_random, mock_views_random):
@override_settings(OIDC_RP_CLIENT_ID='example_id')
@override_settings(OIDC_AUTHENTICATION_CALLBACK_URL='namespace:oidc_authentication_callback')
@patch('mozilla_django_oidc.views.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_get_namespaced(self, mock_utils_random, mock_views_random):
def test_get_namespaced(self, mock_views_random):
"""Test initiation of a successful OIDC attempt with namespaced redirect_uri."""
mock_views_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'
url = reverse('namespace:oidc_authentication_init')
request = self.factory.get(url)
request.session = dict()
Expand All @@ -434,7 +430,7 @@ def test_get_namespaced(self, mock_utils_random, mock_views_random):
'client_id': ['example_id'],
'redirect_uri': ['http://testserver/namespace/callback/'],
'state': ['examplestring'],
'nonce': ['examplenonce']
'nonce': ['examplestring']
}
self.assertDictEqual(parse_qs(o.query), expected_query)
self.assertEqual(o.hostname, 'server.example.com')
Expand All @@ -444,11 +440,9 @@ def test_get_namespaced(self, mock_utils_random, mock_views_random):
@override_settings(OIDC_RP_CLIENT_ID='example_id')
@override_settings(OIDC_AUTH_REQUEST_EXTRA_PARAMS={'audience': 'some-api.example.com'})
@patch('mozilla_django_oidc.views.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
def test_get_with_audience(self, mock_utils_random, mock_views_random):
def test_get_with_audience(self, mock_views_random):
"""Test initiation of a successful OIDC attempt."""
mock_views_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'
url = reverse('oidc_authentication_init')
request = self.factory.get(url)
request.session = dict()
Expand All @@ -463,7 +457,7 @@ def test_get_with_audience(self, mock_utils_random, mock_views_random):
'client_id': ['example_id'],
'redirect_uri': ['http://testserver/callback/'],
'state': ['examplestring'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'audience': ['some-api.example.com'],
}
self.assertDictEqual(parse_qs(o.query), expected_query)
Expand All @@ -473,13 +467,10 @@ def test_get_with_audience(self, mock_utils_random, mock_views_random):
@override_settings(OIDC_OP_AUTHORIZATION_ENDPOINT='https://server.example.com/auth')
@override_settings(OIDC_RP_CLIENT_ID='example_id')
@patch('mozilla_django_oidc.views.get_random_string')
@patch('mozilla_django_oidc.utils.get_random_string')
@patch('mozilla_django_oidc.views.OIDCAuthenticationRequestView.get_extra_params')
def test_get_with_overridden_extra_params(self, mock_extra_params, mock_utils_random,
mock_views_random):
def test_get_with_overridden_extra_params(self, mock_extra_params, mock_views_random):
"""Test overriding OIDCAuthenticationRequestView.get_extra_params()."""
mock_views_random.return_value = 'examplestring'
mock_utils_random.return_value = 'examplenonce'

mock_extra_params.return_value = {
'connection': 'foo'
Expand All @@ -499,7 +490,7 @@ def test_get_with_overridden_extra_params(self, mock_extra_params, mock_utils_ra
'client_id': ['example_id'],
'redirect_uri': ['http://testserver/callback/'],
'state': ['examplestring'],
'nonce': ['examplenonce'],
'nonce': ['examplestring'],
'connection': ['foo'],
}
self.assertDictEqual(parse_qs(o.query), expected_query)
Expand Down

0 comments on commit 56773c1

Please sign in to comment.