Skip to content

Commit 3b8d0d6

Browse files
committed
Add additional RelayState URL validation
Also, some additional debug logging. Also fixed an issue which would cause logout requests to redirect to the fallback login redirect url
1 parent f8e035b commit 3b8d0d6

File tree

3 files changed

+53
-3
lines changed

3 files changed

+53
-3
lines changed

djangosaml2/utils.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from django.core.exceptions import ImproperlyConfigured
2323
from django.http import HttpResponse, HttpResponseRedirect
2424
from django.shortcuts import resolve_url
25+
from django.urls import NoReverseMatch
2526
from django.utils.http import url_has_allowed_host_and_scheme
2627

2728
from saml2.config import SPConfig
@@ -99,6 +100,24 @@ def get_fallback_login_redirect_url():
99100

100101

101102
def validate_referral_url(request, url):
103+
# Ensure the url is even a valid URL; sometimes the given url is a
104+
# RelayState containing PySAML data.
105+
# Some technically-valid urls will be fail this check, so the
106+
# SAML_STRICT_URL_VALIDATION setting can be used to turn off this check.
107+
# This should only happen if there is no slash, host and/or protocol in the
108+
# given URL. A better fix would be to add those to the RelayState.
109+
saml_strict_url_validation = getattr(
110+
settings,
111+
"SAML_STRICT_URL_VALIDATION",
112+
True
113+
)
114+
try:
115+
if saml_strict_url_validation:
116+
url = resolve_url(url)
117+
except NoReverseMatch:
118+
logger.debug("Could not validate given referral url is a valid URL")
119+
return None
120+
102121
# Ensure the user-originating redirection url is safe.
103122
# By setting SAML_ALLOWED_HOSTS in settings.py the user may provide a list of "allowed"
104123
# hostnames for post-login redirects, much like one would specify ALLOWED_HOSTS .
@@ -109,7 +128,10 @@ def validate_referral_url(request, url):
109128
)
110129

111130
if not url_has_allowed_host_and_scheme(url=url, allowed_hosts=saml_allowed_hosts):
112-
return get_fallback_login_redirect_url()
131+
logger.debug("Referral URL not in SAML_ALLOWED_HOSTS or of the origin "
132+
"host.")
133+
return None
134+
113135
return url
114136

115137

djangosaml2/views.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ def _get_next_path(request: HttpRequest) -> Optional[str]:
9999
return None
100100

101101
next_path = validate_referral_url(request, next_path)
102+
102103
return next_path
103104

104105

@@ -572,7 +573,15 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
572573
custom_redirect_url = self.custom_redirect(user, relay_state, session_info)
573574
if custom_redirect_url:
574575
return HttpResponseRedirect(custom_redirect_url)
576+
575577
relay_state = validate_referral_url(request, relay_state)
578+
if relay_state is None:
579+
logger.debug(
580+
"RelayState is not a valid URL, redirecting to fallback: %s",
581+
relay_state
582+
)
583+
return HttpResponseRedirect(get_fallback_login_redirect_url())
584+
576585
logger.debug("Redirecting to the RelayState: %s", relay_state)
577586
return HttpResponseRedirect(relay_state)
578587

@@ -825,12 +834,17 @@ def finish_logout(request, response):
825834

826835
next_path = _get_next_path(request)
827836
if next_path is not None:
837+
logger.debug("Redirecting to the RelayState: %s", next_path)
828838
return HttpResponseRedirect(next_path)
829839
elif settings.LOGOUT_REDIRECT_URL is not None:
830840
fallback_url = resolve_url(settings.LOGOUT_REDIRECT_URL)
841+
logger.debug("No valid RelayState found; Redirecting to "
842+
"LOGOUT_REDIRECT_URL")
831843
return HttpResponseRedirect(fallback_url)
832844
else:
833845
current_site = get_current_site(request)
846+
logger.debug("No valid RelayState or LOGOUT_REDIRECT_URL found, "
847+
"rendering fallback template.")
834848
return render(
835849
request,
836850
"registration/logged_out.html",

docs/source/contents/setup.rst

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ view to djangosaml2 wb path, like ``/saml2/login/``.
122122
Handling Post-Login Redirects
123123
=============================
124124

125-
It is often desireable for the client to maintain the URL state (or at least manage it) so that
125+
It is often desirable for the client to maintain the URL state (or at least manage it) so that
126126
the URL once authentication has completed is consistent with the desired application state (such
127127
as retaining query parameters, etc.) By default, the HttpRequest objects get_host() method is used
128128
to determine the hostname of the server, and redirect URL's are allowed so long as the destination
129-
host matches the output of get_host(). However, in some cases it becomes desireable for additional
129+
host matches the output of get_host(). However, in some cases it becomes desirable for additional
130130
hostnames to be used for the post-login redirect. In such cases, the setting::
131131

132132
SAML_ALLOWED_HOSTS = []
@@ -138,6 +138,20 @@ In the absence of a ``?next=parameter``, the ``ACS_DEFAULT_REDIRECT_URL`` or ``L
138138
be used (assuming the destination hostname either matches the output of get_host() or is included in the
139139
``SAML_ALLOWED_HOSTS`` setting)
140140

141+
Redirect URL validation
142+
=======================
143+
144+
Djangosaml2 will validate the redirect URL before redirecting to its value. In
145+
some edge-cases, valid redirect targets will fail to pass this check. This is
146+
limited to URLs that are a single 'word' without slashes. (For example, 'home'
147+
but also 'page-with-dashes').
148+
149+
In this situation, the best solution would be to add a slash to the URL. For
150+
example: 'home' could be '/home' or 'home/'.
151+
If this is unfeasible, this strict validation can be turned off by setting
152+
``SAML_STRICT_URL_VALIDATION`` to ``False`` in settings.py.
153+
154+
141155
Preferred sso binding
142156
=====================
143157

0 commit comments

Comments
 (0)