Fix #277: Resolve LOGIN_REDIRECT_URL (#279)

Author: jaap3

djangosaml2/tests/__init__.py

@@ -167,12 +167,16 @@ def test_login_evil_redirect(self):
             idp_hosts=['idp.example.com'],
             metadata_file='remote_metadata_one_idp.xml',
         )
-        response = self.client.get(
-            reverse('saml2_login') + '?next=http://evil.com')
-        url = urlparse(response['Location'])
-        params = parse_qs(url.query)
 
-        self.assertEqual(params['RelayState'], [settings.LOGIN_REDIRECT_URL, ])
+        for redirect_url in ['/dashboard/', 'testprofiles:dashboard']:
+            with self.subTest(LOGIN_REDIRECT_URL=redirect_url):
+                with override_settings(LOGIN_REDIRECT_URL=redirect_url):
+                    response = self.client.get(
+                        reverse('saml2_login') + '?next=http://evil.com')
+                    url = urlparse(response['Location'])
+                    params = parse_qs(url.query)
+
+                    self.assertEqual(params['RelayState'], ['/dashboard/'])
 
     def test_no_redirect(self):
         """
@@ -186,11 +190,30 @@ def test_no_redirect(self):
             idp_hosts=['idp.example.com'],
             metadata_file='remote_metadata_one_idp.xml',
         )
-        response = self.client.get(reverse('saml2_login') + '?next=')
-        url = urlparse(response['Location'])
-        params = parse_qs(url.query)
 
-        self.assertEqual(params['RelayState'], [settings.LOGIN_REDIRECT_URL, ])
+        for redirect_url in ['/dashboard/', 'testprofiles:dashboard']:
+            with self.subTest(LOGIN_REDIRECT_URL=redirect_url):
+                with override_settings(LOGIN_REDIRECT_URL=redirect_url):
+                    response = self.client.get(reverse('saml2_login') + '?next=')
+                    url = urlparse(response['Location'])
+                    params = parse_qs(url.query)
+
+                    self.assertEqual(params['RelayState'], ['/dashboard/'])
+
+    @override_settings(SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN=True)
+    def test_login_already_logged(self):
+        self.client.force_login(User.objects.create(username='user', password='pass'))
+
+        for redirect_url in ['/dashboard/', 'testprofiles:dashboard']:
+            with self.subTest(LOGIN_REDIRECT_URL=redirect_url):
+                with override_settings(LOGIN_REDIRECT_URL=redirect_url):
+                    with self.subTest('no next url'):
+                        response = self.client.get(reverse('saml2_login'))
+                        self.assertRedirects(response, '/dashboard/')
+
+                    with self.subTest('evil next url'):
+                        response = self.client.get(reverse('saml2_login') + '?next=http://evil.com')
+                        self.assertRedirects(response, '/dashboard/')
 
     def test_unknown_idp(self):
         # monkey patch SAML configuration
@@ -277,6 +300,7 @@ def test_login_several_idps(self):
         self.assertIn('AuthnRequest xmlns', decode_base64_and_inflate(
             saml_request).decode('utf-8'))
 
+    @override_settings(LOGIN_REDIRECT_URL='testprofiles:dashboard')
     def test_assertion_consumer_service(self):
         # Get initial number of users
         initial_user_count = User.objects.count()
@@ -325,14 +349,36 @@ def test_assertion_consumer_service(self):
             'SAMLResponse': self.b64_for_post(saml_response),
             'RelayState': came_from,
         })
-        self.assertEqual(response.status_code, 302)
-        location = response['Location']
 
-        url = urlparse(location)
         # as the RelayState is empty we have redirect to LOGIN_REDIRECT_URL
-        self.assertEqual(url.path, settings.LOGIN_REDIRECT_URL)
+        self.assertRedirects(response, '/dashboard/')
         self.assertEqual(force_text(new_user.id), client.session[SESSION_KEY])
 
+    @override_settings(LOGIN_REDIRECT_URL='testprofiles:dashboard')
+    def test_assertion_consumer_service_default_relay_state(self):
+        settings.SAML_CONFIG = conf.create_conf(
+            sp_host='sp.example.com',
+            idp_hosts=['idp.example.com'],
+            metadata_file='remote_metadata_one_idp.xml',
+        )
+
+        new_user = User.objects.create(username='teacher', password='not-used')
+
+        response = self.client.get(reverse('saml2_login'))
+        saml2_req = saml2_from_httpredirect_request(response.url)
+        session_id = get_session_id_from_saml2(saml2_req)
+
+        saml_response = auth_response(session_id, 'teacher')
+        self.add_outstanding_query(session_id, '/')
+        response = self.client.post(reverse('saml2_acs'), {
+            'SAMLResponse': self.b64_for_post(saml_response),
+        })
+        self.assertEqual(response.status_code, 302)
+
+        # The RelayState is missing, redirect to LOGIN_REDIRECT_URL
+        self.assertRedirects(response, '/dashboard/')
+        self.assertEqual(force_text(new_user.id), self.client.session[SESSION_KEY])
+
     def test_assertion_consumer_service_already_logged_in_allowed(self):
         self.client.force_login(User.objects.create(
             username='user', password='pass'))

djangosaml2/utils.py

@@ -21,6 +21,7 @@
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.http import HttpResponse, HttpResponseRedirect
+from django.shortcuts import resolve_url
 from django.utils.http import is_safe_url
 from saml2.config import SPConfig
 from saml2.s_utils import UnknownSystemEntity
@@ -91,7 +92,7 @@ def validate_referral_url(request, url):
         getattr(settings, 'SAML_ALLOWED_HOSTS', [request.get_host()]))
 
     if not is_safe_url(url=url, allowed_hosts=saml_allowed_hosts):
-        return settings.LOGIN_REDIRECT_URL
+        return resolve_url(settings.LOGIN_REDIRECT_URL)
     return url
 
 

djangosaml2/views.py

@@ -24,7 +24,7 @@
 from django.core.exceptions import PermissionDenied, SuspiciousOperation
 from django.http import (HttpRequest, HttpResponse, HttpResponseBadRequest,
                          HttpResponseRedirect, HttpResponseServerError)
-from django.shortcuts import render
+from django.shortcuts import render, resolve_url
 from django.template import TemplateDoesNotExist
 from django.urls import reverse
 from django.utils.decorators import method_decorator
@@ -116,7 +116,7 @@ def get_next_path(self, request: HttpRequest) -> str:
             If the user is already logged in (and if allowed), he will redirect to there immediately.
         '''
 
-        next_path = settings.LOGIN_REDIRECT_URL
+        next_path = resolve_url(settings.LOGIN_REDIRECT_URL)
         if 'next' in request.GET:
             next_path = request.GET['next']
         elif 'RelayState' in request.GET:
@@ -501,8 +501,8 @@ def build_relay_state(self) -> str:
         """ The relay state is a URL used to redirect the user to the view where they came from.
         """
         login_redirect_url = get_custom_setting('LOGIN_REDIRECT_URL', '/')
-        default_relay_state = get_custom_setting(
-            'ACS_DEFAULT_REDIRECT_URL', login_redirect_url)
+        default_relay_state = resolve_url(
+            get_custom_setting('ACS_DEFAULT_REDIRECT_URL', login_redirect_url))
         relay_state = self.request.POST.get('RelayState', default_relay_state)
         relay_state = self.customize_relay_state(relay_state)
         if not relay_state:

tests/testprofiles/urls.py

@@ -1,9 +1,16 @@
-from django.conf.urls import include, url
+from django.urls import include, path
 from django.contrib import admin
+from django.http import HttpResponse
 
-app_name='testprofiles'
+testpatterns = (
+    [
+        path('dashboard/', lambda request: HttpResponse(''), name='dashboard')
+    ],
+    'testprofiles'  # app_name
+)
 
 urlpatterns = [
-    url(r'^saml2/', include('djangosaml2.urls')),
-    url(r'^admin/', admin.site.urls),
+    path('saml2/', include('djangosaml2.urls')),
+    path('admin/', admin.site.urls),
+    path('', include(testpatterns))
 ]