Merge pull request #388 from tymees/fix/invalid-redirect

Add additional RelayState URL validation

Author: Giuseppe De Marco

djangosaml2/utils.py

@@ -22,6 +22,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import resolve_url
+from django.urls import NoReverseMatch
 from django.utils.http import url_has_allowed_host_and_scheme
 
 from saml2.config import SPConfig
@@ -99,6 +100,25 @@ def get_fallback_login_redirect_url():
 
 
 def validate_referral_url(request, url):
+    # Ensure the url is even a valid URL; sometimes the given url is a
+    # RelayState containing PySAML data.
+    # Some technically-valid urls will be fail this check, so the
+    # SAML_STRICT_URL_VALIDATION setting can be used to turn off this check.
+    # This should only happen if there is no slash, host and/or protocol in the
+    # given URL. A better fix would be to add those to the RelayState.
+    saml_strict_url_validation = getattr(
+        settings,
+        "SAML_STRICT_URL_VALIDATION",
+        True
+    )
+    try:
+        if saml_strict_url_validation:
+            # This will also resolve Django URL pattern names
+            url = resolve_url(url)
+    except NoReverseMatch:
+        logger.debug("Could not validate given referral url is a valid URL")
+        return None
+
     # Ensure the user-originating redirection url is safe.
     # By setting SAML_ALLOWED_HOSTS in settings.py the user may provide a list of "allowed"
     # hostnames for post-login redirects, much like one would specify ALLOWED_HOSTS .
@@ -109,7 +129,10 @@ def validate_referral_url(request, url):
     )
 
     if not url_has_allowed_host_and_scheme(url=url, allowed_hosts=saml_allowed_hosts):
-        return get_fallback_login_redirect_url()
+        logger.debug("Referral URL not in SAML_ALLOWED_HOSTS or of the origin "
+                     "host.")
+        return None
+
     return url
 
 

djangosaml2/views.py

@@ -99,6 +99,7 @@ def _get_next_path(request: HttpRequest) -> Optional[str]:
         return None
 
     next_path = validate_referral_url(request, next_path)
+
     return next_path
 
 
@@ -572,7 +573,15 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
         custom_redirect_url = self.custom_redirect(user, relay_state, session_info)
         if custom_redirect_url:
             return HttpResponseRedirect(custom_redirect_url)
+
         relay_state = validate_referral_url(request, relay_state)
+        if not relay_state:
+            logger.debug(
+                "RelayState is not a valid URL, redirecting to fallback: %s",
+                relay_state
+            )
+            return HttpResponseRedirect(get_fallback_login_redirect_url())
+
         logger.debug("Redirecting to the RelayState: %s", relay_state)
         return HttpResponseRedirect(relay_state)
 
@@ -825,12 +834,17 @@ def finish_logout(request, response):
 
         next_path = _get_next_path(request)
         if next_path is not None:
+            logger.debug("Redirecting to the RelayState: %s", next_path)
             return HttpResponseRedirect(next_path)
         elif settings.LOGOUT_REDIRECT_URL is not None:
             fallback_url = resolve_url(settings.LOGOUT_REDIRECT_URL)
+            logger.debug("No valid RelayState found; Redirecting to "
+                         "LOGOUT_REDIRECT_URL")
             return HttpResponseRedirect(fallback_url)
         else:
             current_site = get_current_site(request)
+            logger.debug("No valid RelayState or LOGOUT_REDIRECT_URL found, "
+                         "rendering fallback template.")
             return render(
                 request,
                 "registration/logged_out.html",

docs/source/contents/setup.rst

@@ -122,11 +122,11 @@ view to djangosaml2 wb path, like ``/saml2/login/``.
 Handling Post-Login Redirects
 =============================
 
-It is often desireable for the client to maintain the URL state (or at least manage it) so that
+It is often desirable for the client to maintain the URL state (or at least manage it) so that
 the URL once authentication has completed is consistent with the desired application state (such
 as retaining query parameters, etc.)  By default, the HttpRequest objects get_host() method is used
 to determine the hostname of the server, and redirect URL's are allowed so long as the destination
-host matches the output of get_host().  However, in some cases it becomes desireable for additional
+host matches the output of get_host().  However, in some cases it becomes desirable for additional
 hostnames to be used for the post-login redirect.  In such cases, the setting::
 
   SAML_ALLOWED_HOSTS = []
@@ -138,6 +138,22 @@ In the absence of a ``?next=parameter``, the ``ACS_DEFAULT_REDIRECT_URL`` or ``L
 be used (assuming the destination hostname either matches the output of get_host() or is included in the
 ``SAML_ALLOWED_HOSTS`` setting)
 
+Redirect URL validation
+=======================
+
+Djangosaml2 will validate the redirect URL before redirecting to its value. In
+some edge-cases, valid redirect targets will fail to pass this check. This is
+limited to URLs that are a single 'word' without slashes. (For example, 'home'
+but also 'page-with-dashes').
+
+In this situation, the best solution would be to add a slash to the URL. For
+example: 'home' could be '/home' or 'home/'.
+If this is unfeasible, this strict validation can be turned off by setting
+``SAML_STRICT_URL_VALIDATION`` to ``False`` in settings.py.
+
+During validation, `Django named URL patterns<https://docs.djangoproject.com/en/dev/topics/http/urls/#naming-url-patterns>`_
+will also be resolved. Turning off strict validation will prevent this from happening.
+
 Preferred sso binding
 =====================
 

setup.py

@@ -27,7 +27,7 @@ def read(*rnames):
 
 setup(
     name="djangosaml2",
-    version="1.7.0",
+    version="1.8.0",
     description="pysaml2 integration for Django",
     long_description=read("README.md"),
     long_description_content_type="text/markdown",