RelayState, fixes and cleanup (#218)

Author: mhindery

djangosaml2/middleware.py

@@ -67,8 +67,8 @@ def process_response(self, request, response):
                         max_age=max_age,
                         expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
                         path=settings.SESSION_COOKIE_PATH,
-                        secure=settings.SESSION_COOKIE_SECURE or None,
-                        httponly=settings.SESSION_COOKIE_HTTPONLY or None,
-                        samesite=None
+                        secure=settings.SESSION_COOKIE_SECURE,
+                        httponly=settings.SESSION_COOKIE_HTTPONLY,
+                        samesite=settings.SESSION_COOKIE_SAMESITE
                     )
         return response

djangosaml2/tests/__init__.py

@@ -18,16 +18,15 @@
 import re
 import sys
 from importlib import import_module
-from unittest import mock, skip
+from unittest import mock
 from urllib.parse import parse_qs, urlparse
 
 from django.conf import settings
 from django.contrib.auth import SESSION_KEY, get_user_model
 from django.contrib.auth.models import AnonymousUser
 from django.core.exceptions import ImproperlyConfigured
-from django.http.request import HttpRequest
 from django.template import Context, Template
-from django.test import Client, TestCase
+from django.test import Client, TestCase, override_settings
 from django.test.client import RequestFactory
 from django.urls import reverse
 from django.utils.encoding import force_text
@@ -36,8 +35,7 @@
 from djangosaml2.conf import get_config
 from djangosaml2.middleware import SamlSessionMiddleware
 from djangosaml2.tests import conf
-from djangosaml2.utils import (get_custom_setting,
-                               get_idp_sso_supported_bindings,
+from djangosaml2.utils import (get_idp_sso_supported_bindings,
                                get_session_id_from_saml2,
                                get_subject_id_from_saml2,
                                saml2_from_httpredirect_request)
@@ -81,35 +79,6 @@ class SAML2Tests(TestCase):
 
     urls = 'djangosaml2.tests.urls'
 
-    def setUp(self):
-        if hasattr(settings, 'SAML_ATTRIBUTE_MAPPING'):
-            self.actual_attribute_mapping = settings.SAML_ATTRIBUTE_MAPPING
-            del settings.SAML_ATTRIBUTE_MAPPING
-        if hasattr(settings, 'SAML_CONFIG_LOADER'):
-            self.actual_conf_loader = settings.SAML_CONFIG_LOADER
-            del settings.SAML_CONFIG_LOADER
-
-    def tearDown(self):
-        if hasattr(self, 'actual_attribute_mapping'):
-            settings.SAML_ATTRIBUTE_MAPPING = self.actual_attribute_mapping
-        if hasattr(self, 'actual_conf_loader'):
-            settings.SAML_CONFIG_LOADER = self.actual_conf_loader
-
-    def assertSAMLRequestsEquals(self, real_xml, expected_xmls):
-
-        def remove_variable_attributes(xml_string):
-            xml_string = re.sub(r' ID=".*?" ', ' ', xml_string)
-            xml_string = re.sub(r' IssueInstant=".*?" ', ' ', xml_string)
-            xml_string = re.sub(
-                r'<saml:NameID(.*)>.*</saml:NameID>',
-                r'<saml:NameID\1></saml:NameID>',
-                xml_string)
-
-            return xml_string
-
-        self.assertEqual(remove_variable_attributes(real_xml),
-                         remove_variable_attributes(expected_xmls))
-
     def init_cookies(self):
         self.client.cookies[settings.SESSION_COOKIE_NAME] = 'testing'
 
@@ -120,7 +89,7 @@ def add_outstanding_query(self, session_id, came_from):
         self.saml_session.save()
         self.oq_cache = OutstandingQueriesCache(self.saml_session)
 
-        self.oq_cache.set(session_id \
+        self.oq_cache.set(session_id
                           if isinstance(session_id, str) else session_id.decode(),
                           came_from)
         self.saml_session.save()
@@ -181,8 +150,7 @@ def test_unsigned_post_authn_request(self):
         saml_request = response_parser.saml_request_value
 
         self.assertIsNotNone(saml_request)
-        if 'AuthnRequest xmlns' not in base64.b64decode(saml_request).decode('utf-8'):
-            raise Exception('test_unsigned_post_authn_request: Not a valid AuthnRequest')
+        self.assertIn('AuthnRequest xmlns', base64.b64decode(saml_request).decode('utf-8'))
 
     def test_login_evil_redirect(self):
         """
@@ -241,8 +209,7 @@ def test_login_one_idp(self):
         self.assertIn('RelayState', params)
 
         saml_request = params['SAMLRequest'][0]
-        if 'AuthnRequest xmlns' not in decode_base64_and_inflate(saml_request).decode('utf-8'):
-            raise Exception('Not a valid AuthnRequest')
+        self.assertIn('AuthnRequest xmlns', decode_base64_and_inflate(saml_request).decode('utf-8'))
 
         # if we set a next arg in the login view, it is preserverd
         # in the RelayState argument
@@ -292,8 +259,7 @@ def test_login_several_idps(self):
         self.assertIn('RelayState', params)
 
         saml_request = params['SAMLRequest'][0]
-        if 'AuthnRequest xmlns' not in decode_base64_and_inflate(saml_request).decode('utf-8'):
-            raise Exception('Not a valid AuthnRequest')
+        self.assertIn('AuthnRequest xmlns', decode_base64_and_inflate(saml_request).decode('utf-8'))
 
     def test_assertion_consumer_service(self):
         # Get initial number of users
@@ -440,7 +406,6 @@ def do_login(self):
         self.assertEqual(response.status_code, 302)
         return subject_id
 
-    @skip("This is a known issue caused by pysaml2. Needs more investigation. Fixes are welcome.")
     def test_logout(self):
         settings.SAML_CONFIG = conf.create_conf(
             sp_host='sp.example.com',
@@ -466,8 +431,6 @@ def test_logout(self):
         if 'LogoutRequest xmlns' not in decode_base64_and_inflate(saml_request).decode('utf-8'):
             raise Exception('Not a valid LogoutRequest')
 
-
-
     def test_logout_service_local(self):
         settings.SAML_CONFIG = conf.create_conf(
             sp_host='sp.example.com',
@@ -562,43 +525,6 @@ def test_finish_logout_renders_error_template(self):
         response = finish_logout(request, None)
         self.assertContains(response, "<h1>Logout error</h1>", status_code=200)
 
-    def _test_metadata(self):
-        settings.SAML_CONFIG = conf.create_conf(
-            sp_host='sp.example.com',
-            idp_hosts=['idp.example.com'],
-            metadata_file='remote_metadata_one_idp.xml',
-        )
-        valid_until = datetime.datetime.utcnow() + datetime.timedelta(hours=24)
-        valid_until = valid_until.strftime("%Y-%m-%dT%H:%M:%SZ")
-        expected_metadata = """<?xml version='1.0' encoding='UTF-8'?>
-<md:EntityDescriptor entityID="http://sp.example.com/saml2/metadata/" validUntil="%s" xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"><md:SPSSODescriptor AuthnRequestsSigned="false" WantAssertionsSigned="true" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol"><md:KeyDescriptor><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:X509Data><ds:X509Certificate>MIIDPjCCAiYCCQCkHjPQlll+mzANBgkqhkiG9w0BAQUFADBhMQswCQYDVQQGEwJF
-UzEQMA4GA1UECBMHU2V2aWxsYTEbMBkGA1UEChMSWWFjbyBTaXN0ZW1hcyBTLkwu
-MRAwDgYDVQQHEwdTZXZpbGxhMREwDwYDVQQDEwh0aWNvdGljbzAeFw0wOTEyMDQx
-OTQzNTJaFw0xMDEyMDQxOTQzNTJaMGExCzAJBgNVBAYTAkVTMRAwDgYDVQQIEwdT
-ZXZpbGxhMRswGQYDVQQKExJZYWNvIFNpc3RlbWFzIFMuTC4xEDAOBgNVBAcTB1Nl
-dmlsbGExETAPBgNVBAMTCHRpY290aWNvMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
-MIIBCgKCAQEA7rMOMOaIZ/YYD5hYS6Hpjpovcu4k8gaIY+om9zCxLV5F8BLEfkxo
-Pk9IA3cRQNRxf7AXCFxEOH3nKy56AIi1gU7X6fCT30JBT8NQlYdgOVMLlR+tjy1b
-YV07tDa9U8gzjTyKQHgVwH0436+rmSPnacGj3fMwfySTMhtmrJmax0bIa8EB+gY1
-77DBtvf8dIZIXLlGMQFloZeUspvHOrgNoEA9xU4E9AanGnV9HeV37zv3mLDUOQLx
-4tk9sMQmylCpij7WZmcOV07DyJ/cEmnvHSalBTcyIgkcwlhmjtSgfCy6o5zuWxYd
-T9ia80SZbWzn8N6B0q+nq23+Oee9H0lvcwIDAQABMA0GCSqGSIb3DQEBBQUAA4IB
-AQCQBhKOqucJZAqGHx4ybDXNzpPethszonLNVg5deISSpWagy55KlGCi5laio/xq
-hHRx18eTzeCeLHQYvTQxw0IjZOezJ1X30DD9lEqPr6C+IrmZc6bn/pF76xsvdaRS
-gduNQPT1B25SV2HrEmbf8wafSlRARmBsyUHh860TqX7yFVjhYIAUF/El9rLca51j
-ljCIqqvT+klPdjQoZwODWPFHgute2oNRmoIcMjSnoy1+mxOC2Q/j7kcD8/etulg2
-XDxB3zD81gfdtT8VBFP+G4UrBa+5zFk6fT6U8a7ZqVsyH+rCXAdCyVlEC4Y5fZri
-ID4zT0FcZASGuthM56rRJJSx
-</ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="http://sp.example.com/saml2/ls/" /><md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="http://sp.example.com/saml2/acs/" index="1" /><md:AttributeConsumingService index="1"><md:ServiceName xml:lang="en">Test SP</md:ServiceName><md:RequestedAttribute FriendlyName="uid" Name="urn:oid:0.9.2342.19200300.100.1.1" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" isRequired="true" /><md:RequestedAttribute FriendlyName="eduPersonAffiliation" Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.1" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri" isRequired="false" /></md:AttributeConsumingService></md:SPSSODescriptor><md:Organization><md:OrganizationName xml:lang="es">Ejemplo S.A.</md:OrganizationName><md:OrganizationName xml:lang="en">Example Inc.</md:OrganizationName><md:OrganizationDisplayName xml:lang="es">Ejemplo</md:OrganizationDisplayName><md:OrganizationDisplayName xml:lang="en">Example</md:OrganizationDisplayName><md:OrganizationURL xml:lang="es">http://www.example.es</md:OrganizationURL><md:OrganizationURL xml:lang="en">http://www.example.com</md:OrganizationURL></md:Organization><md:ContactPerson contactType="technical"><md:Company>Example Inc.</md:Company><md:GivenName>Technical givenname</md:GivenName><md:SurName>Technical surname</md:SurName><md:EmailAddress>[email protected]</md:EmailAddress></md:ContactPerson><md:ContactPerson contactType="administrative"><md:Company>Example Inc.</md:Company><md:GivenName>Administrative givenname</md:GivenName><md:SurName>Administrative surname</md:SurName><md:EmailAddress>[email protected]</md:EmailAddress></md:ContactPerson></md:EntityDescriptor>"""
-
-        expected_metadata = expected_metadata % valid_until
-
-        response = self.client.get(reverse('saml2_metadata'))
-        self.assertEqual(response['Content-type'], 'text/xml; charset=utf8')
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.content, expected_metadata)
-
-
     def test_sigalg_not_passed_when_not_signing_request(self):
         # monkey patch SAML configuration
         settings.SAML_CONFIG = conf.create_conf(
@@ -690,3 +616,79 @@ def test_custom_conf_loader_from_view(self):
         url = urlparse(location)
         self.assertEqual(url.hostname, 'idp.example.com')
         self.assertEqual(url.path, '/simplesaml/saml2/idp/SSOService.php')
+
+
+class SessionEnabledTestCase(TestCase):
+    def get_session(self):
+        if self.client.session:
+            session = self.client.session
+        else:
+            engine = import_module(settings.SESSION_ENGINE)
+            session = engine.SessionStore()
+        return session
+
+    def set_session_cookies(self, session):
+        # Set the cookie to represent the session
+        session_cookie = settings.SESSION_COOKIE_NAME
+        self.client.cookies[session_cookie] = session.session_key
+        cookie_data = {
+            'max-age': None,
+            'path': '/',
+            'domain': settings.SESSION_COOKIE_DOMAIN,
+            'secure': settings.SESSION_COOKIE_SECURE or None,
+            'expires': None}
+        self.client.cookies[session_cookie].update(cookie_data)
+
+
+class MiddlewareTests(SessionEnabledTestCase):
+    def test_middleware_cookie_expireatbrowserclose(self):
+        with override_settings(SESSION_EXPIRE_AT_BROWSER_CLOSE=True):
+            session = self.get_session()
+            session.save()
+            self.set_session_cookies(session)
+
+            config_loader_path = 'djangosaml2.tests.test_config_loader_with_real_conf'
+            request = RequestFactory().get('/login/')
+            request.user = AnonymousUser()
+            request.session = session
+            middleware = SamlSessionMiddleware()
+            middleware.process_request(request)
+
+            saml_session_name = getattr(settings, 'SAML_SESSION_COOKIE_NAME', 'saml_session')
+            getattr(request, saml_session_name).save()
+
+            response = views.LoginView.as_view(config_loader_path=config_loader_path)(request)
+
+            response = middleware.process_response(request, response)
+
+            cookie = response.cookies[saml_session_name]
+
+            self.assertEqual(cookie['expires'], '')
+            self.assertEqual(cookie['max-age'], '')
+
+    def test_middleware_cookie_with_expiry(self):
+        with override_settings(SESSION_EXPIRE_AT_BROWSER_CLOSE=False):
+            session = self.get_session()
+            session.save()
+            self.set_session_cookies(session)
+
+            config_loader_path = 'djangosaml2.tests.test_config_loader_with_real_conf'
+            request = RequestFactory().get('/login/')
+            request.user = AnonymousUser()
+            request.session = session
+            middleware = SamlSessionMiddleware()
+            middleware.process_request(request)
+
+            saml_session_name = getattr(settings, 'SAML_SESSION_COOKIE_NAME', 'saml_session')
+            getattr(request, saml_session_name).save()
+
+            response = views.LoginView.as_view(config_loader_path=config_loader_path)(request)
+
+            response = middleware.process_response(request, response)
+
+            cookie = response.cookies[saml_session_name]
+
+            self.assertIsNotNone(cookie['expires'])
+
+            self.assertNotEqual(cookie['expires'], '')
+            self.assertNotEqual(cookie['max-age'], '')

djangosaml2/tests/urls.py

@@ -108,7 +108,13 @@ def get_next_path(self, request: HttpRequest) -> str:
         ''' Returns the path to put in the RelayState to redirect the user to after having logged in.
             If the user is already logged in (and if allowed), he will redirect to there immediately.
         '''
-        next_path = request.GET.get('next', settings.LOGIN_REDIRECT_URL) or settings.LOGIN_REDIRECT_URL
+
+        next_path = settings.LOGIN_REDIRECT_URL
+        if 'next' in request.GET:
+            next_path = request.GET['next']
+        elif 'RelayState' in request.GET:
+            next_path = request.GET['RelayState']
+
         next_path = validate_referral_url(request, next_path)
         return next_path
 
@@ -399,7 +405,7 @@ def get(self, request, *args, **kwargs):
         except AttributeError:
             return HttpResponse("No active SAML identity found. Are you sure you have logged in via SAML?")
 
-        return render(request, template, {'attributes': identity[0]})
+        return render(request, 'djangosaml2/echo_attributes.html', {'attributes': identity[0]})
 
 
 class LogoutInitView(LoginRequiredMixin, SPConfigMixin, View):

djangosaml2/views.py

@@ -24,7 +24,7 @@ def read(*rnames):
 
 setup(
     name='djangosaml2',
-    version='0.40.0',
+    version='1.0.0',
     description='pysaml2 integration for Django',
     long_description=read('README.rst'),
     classifiers=[

setup.py

@@ -379,7 +379,6 @@ def test_clean_attributes(self):
     def test_clean_user_main_attribute(self):
         self.assertEqual(self.backend.clean_user_main_attribute('va--l__ u -e'), 'va__l___u__e')
 
-
     def test_authenticate(self):
         attribute_mapping = {
             'uid': ('username', ),
@@ -413,6 +412,14 @@ def test_authenticate(self):
         )
         self.assertIsNone(user)
 
+        with override_settings(SAML_USE_NAME_ID_AS_USERNAME=True):
+            user = self.backend.authenticate(
+                None,
+                session_info={'ava': attributes, 'issuer': 'dummy_entity_id'},
+                attribute_mapping=attribute_mapping,
+            )
+            self.assertIsNone(user)
+
         attributes['is_staff'] = (False, )
         user = self.backend.authenticate(
             None,