Merge pull request #358 from akatsoulas/support-multiple-sessions

Support multiple sessions

docs/settings.rst

@@ -99,6 +99,13 @@ of ``mozilla-django-oidc``.
 
    Sets the length of the random string used for OpenID Connect nonce verification
 
+.. py:attribute:: OIDC_MAX_STATES
+
+   :default: ``50``
+
+   Sets the maximum number of State / Nonce combinations stored in the session.
+   Multiple combinations are used when the user does multiple concurrent login sessions.
+
 .. py:attribute:: OIDC_REDIRECT_FIELD_NAME
 
    :default: ``next``

mozilla_django_oidc/middleware.py

@@ -1,21 +1,24 @@
 import logging
 import time
-try:
-    from urllib.parse import urlencode
-except ImportError:
-    # Python < 3
-    from urllib import urlencode
 
-from django.urls import reverse
 from django.contrib.auth import BACKEND_SESSION_KEY
 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
 
 from mozilla_django_oidc.auth import OIDCAuthenticationBackend
-from mozilla_django_oidc.utils import absolutify, import_from_settings
+from mozilla_django_oidc.utils import (absolutify,
+                                       add_state_and_nonce_to_session,
+                                       import_from_settings)
+
+try:
+    from urllib.parse import urlencode
+except ImportError:
+    # Python < 3
+    from urllib import urlencode
 
 
 LOGGER = logging.getLogger(__name__)
@@ -111,14 +114,8 @@ 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
-            })
-            request.session['oidc_nonce'] = nonce
+        add_state_and_nonce_to_session(request, state, params)
 
-        request.session['oidc_state'] = state
         request.session['oidc_login_next'] = request.get_full_path()
 
         query = urlencode(params)

mozilla_django_oidc/utils.py

@@ -1,13 +1,19 @@
+import logging
+import time
 import warnings
 
+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
 except ImportError:
     # python < 3
     from urllib2 import parse_http_list, parse_keqv_list
 
-from django.conf import settings
-from django.core.exceptions import ImproperlyConfigured
+
+LOGGER = logging.getLogger(__name__)
 
 
 def parse_www_authenticate_header(header):
@@ -51,3 +57,53 @@ def is_authenticated(user):
     msg = '`is_authenticated()` is going to be removed in mozilla-django-oidc v 2.x'
     warnings.warn(msg, DeprecationWarning)
     return user.is_authenticated
+
+
+def add_state_and_nonce_to_session(request, state, params):
+    """
+    Stores the `state` and `nonce` parameters in a session dictionary including the time when it
+    was added. The dictionary can contain multiple state/nonce combinations to allow parallel
+    logins with multiple browser sessions.
+    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
+        })
+
+    # Store Nonce with the State parameter in the session "oidc_states" dictionary.
+    # The dictionary can store multiple State/Nonce combinations to allow parallel
+    # authentication flows which would otherwise overwrite State/Nonce values!
+    # The "oidc_states" dictionary uses the state as key and as value a dictionary with "nonce"
+    # and "added_on". "added_on" contains the time when the state was added to the session.
+    # With this value, the oldest element can be found and deleted from the session.
+    if 'oidc_states' not in request.session or \
+            not isinstance(request.session['oidc_states'], dict):
+        request.session['oidc_states'] = {}
+
+    # Make sure that the State/Nonce dictionary in the session does not get too big.
+    # If the number of State/Nonce combinations reaches a certain threshold, remove the oldest
+    # state by finding out
+    # which element has the oldest "add_on" time.
+    limit = import_from_settings('OIDC_MAX_STATES', 50)
+    if len(request.session['oidc_states']) >= limit:
+        LOGGER.info(
+            'User has more than {} "oidc_states" in his session, '
+            'deleting the oldest one!'.format(limit)
+        )
+        oldest_state = None
+        oldest_added_on = time.time()
+        for item_state, item in request.session['oidc_states'].items():
+            if item['added_on'] < oldest_added_on:
+                oldest_state = item_state
+                oldest_added_on = item['added_on']
+        if oldest_state:
+            del request.session['oidc_states'][oldest_state]
+
+    request.session['oidc_states'][state] = {
+        'nonce': nonce,
+        'added_on': time.time(),
+    }

mozilla_django_oidc/views.py

@@ -1,20 +1,23 @@
 import time
-try:
-    from urllib.parse import urlencode
-except ImportError:
-    # Python < 3
-    from urllib import urlencode
 
-from django.core.exceptions import SuspiciousOperation
-from django.urls import reverse
 from django.contrib import auth
+from django.core.exceptions import SuspiciousOperation
 from django.http import HttpResponseRedirect
+from django.urls import reverse
 from django.utils.crypto import get_random_string
 from django.utils.http import is_safe_url
 from django.utils.module_loading import import_string
 from django.views.generic import View
 
-from mozilla_django_oidc.utils import absolutify, import_from_settings
+from mozilla_django_oidc.utils import (absolutify,
+                                       add_state_and_nonce_to_session,
+                                       import_from_settings)
+
+try:
+    from urllib.parse import urlencode
+except ImportError:
+    # Python < 3
+    from urllib import urlencode
 
 
 class OIDCAuthenticationCallbackView(View):
@@ -53,11 +56,6 @@ 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 request.GET.get('error'):
             # Ouch! Something important failed.
             # Make sure the user doesn't get to continue to be logged in
@@ -68,18 +66,36 @@ def get(self, request):
                 auth.logout(request)
             assert not request.user.is_authenticated
         elif 'code' in request.GET and 'state' in request.GET:
-            kwargs = {
-                'request': request,
-                'nonce': nonce,
-            }
 
-            if 'oidc_state' not in request.session:
+            # Check instead of "oidc_state" check if the "oidc_states" session key exists!
+            if 'oidc_states' not in request.session:
                 return self.login_failure()
 
-            if request.GET['state'] != request.session['oidc_state']:
-                msg = 'Session `oidc_state` does not match the OIDC callback state'
+            # State and Nonce are stored in the session "oidc_states" dictionary.
+            # State is the key, the value is a dictionary with the Nonce in the "nonce" field.
+            state = request.GET.get('state')
+            if state not in request.session['oidc_states']:
+                msg = 'OIDC callback state not found in session `oidc_states`!'
                 raise SuspiciousOperation(msg)
 
+            # Get the nonce from the dictionary for further processing and delete the entry to
+            # prevent replay attacks.
+            nonce = request.session['oidc_states'][state]['nonce']
+            del request.session['oidc_states'][state]
+
+            # Authenticating is slow, so save the updated oidc_states.
+            request.session.save()
+            # Reset the session. This forces the session to get reloaded from the database after
+            # fetching the token from the OpenID connect provider.
+            # Without this step we would overwrite items that are being added/removed from the
+            # session in parallel browser tabs.
+            request.session = request.session.__class__(request.session.session_key)
+
+            kwargs = {
+                'request': request,
+                'nonce': nonce,
+            }
+
             self.user = auth.authenticate(**kwargs)
 
             if self.user and self.user.is_active:
@@ -152,14 +168,8 @@ 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
-            })
-            request.session['oidc_nonce'] = nonce
+        add_state_and_nonce_to_session(request, state, params)
 
-        request.session['oidc_state'] = state
         request.session['oidc_login_next'] = get_next_url(request, redirect_field_name)
 
         query = urlencode(params)

tests/test_middleware.py

@@ -57,8 +57,10 @@ 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')
-    def test_is_ajax(self, mock_random_string):
-        mock_random_string.return_value = 'examplestring'
+    @patch('mozilla_django_oidc.utils.get_random_string')
+    def test_is_ajax(self, mock_utils_random, mock_middleware_random):
+        mock_middleware_random.return_value = 'examplestring'
+        mock_utils_random.return_value = 'examplenonce'
 
         request = self.factory.get(
             '/foo',
@@ -78,7 +80,7 @@ def test_is_ajax(self, mock_random_string):
             'response_type': ['code'],
             'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
-            'nonce': ['examplestring'],
+            'nonce': ['examplenonce'],
             'prompt': ['none'],
             'scope': ['openid email'],
             'state': ['examplestring'],
@@ -91,8 +93,11 @@ def test_is_ajax(self, mock_random_string):
     @override_settings(OIDC_RP_CLIENT_ID='foo')
     @override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
     @patch('mozilla_django_oidc.middleware.get_random_string')
-    def test_no_oidc_token_expiration_forces_renewal(self, mock_random_string):
-        mock_random_string.return_value = 'examplestring'
+    @patch('mozilla_django_oidc.utils.get_random_string')
+    def test_no_oidc_token_expiration_forces_renewal(self, mock_utils_random,
+                                                     mock_middleware_random):
+        mock_middleware_random.return_value = 'examplestring'
+        mock_utils_random.return_value = 'examplenonce'
 
         request = self.factory.get('/foo')
         request.user = self.user
@@ -107,7 +112,7 @@ def test_no_oidc_token_expiration_forces_renewal(self, mock_random_string):
             'response_type': ['code'],
             'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
-            'nonce': ['examplestring'],
+            'nonce': ['examplenonce'],
             'prompt': ['none'],
             'scope': ['openid email'],
             'state': ['examplestring'],
@@ -118,8 +123,10 @@ def test_no_oidc_token_expiration_forces_renewal(self, mock_random_string):
     @override_settings(OIDC_RP_CLIENT_ID='foo')
     @override_settings(OIDC_RENEW_ID_TOKEN_EXPIRY_SECONDS=120)
     @patch('mozilla_django_oidc.middleware.get_random_string')
-    def test_expired_token_forces_renewal(self, mock_random_string):
-        mock_random_string.return_value = 'examplestring'
+    @patch('mozilla_django_oidc.utils.get_random_string')
+    def test_expired_token_forces_renewal(self, mock_utils_random, mock_middleware_random):
+        mock_middleware_random.return_value = 'examplestring'
+        mock_utils_random.return_value = 'examplenonce'
 
         request = self.factory.get('/foo')
         request.user = self.user
@@ -136,7 +143,7 @@ def test_expired_token_forces_renewal(self, mock_random_string):
             'response_type': ['code'],
             'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
-            'nonce': ['examplestring'],
+            'nonce': ['examplenonce'],
             'prompt': ['none'],
             'scope': ['openid email'],
             'state': ['examplestring'],
@@ -256,8 +263,10 @@ 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')
-    def test_expired_token_redirects_to_sso(self, mock_random_string):
-        mock_random_string.return_value = 'examplestring'
+    @patch('mozilla_django_oidc.utils.get_random_string')
+    def test_expired_token_redirects_to_sso(self, mock_utils_random, 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')
@@ -277,7 +286,7 @@ def test_expired_token_redirects_to_sso(self, mock_random_string):
             'response_type': ['code'],
             'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
-            'nonce': ['examplestring'],
+            'nonce': ['examplenonce'],
             'prompt': ['none'],
             'scope': ['openid email'],
             'state': ['examplestring'],