From 984179d76c19b6674b4e96545e23bc3a042e92a1 Mon Sep 17 00:00:00 2001 From: Marcel Kornblum Date: Fri, 28 Apr 2023 14:01:47 +0100 Subject: [PATCH 01/22] split authenticate into separate overridable function (#1) * split authenticate into separate overridable function * cleaner exception * basic unit test * remove new test * space to revert change --- djangosaml2/views.py | 47 ++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 9b2d9e93..86ea19d1 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -550,7 +550,40 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None): if callable(create_unknown_user): create_unknown_user = create_unknown_user() + try: + user = self.authenticate_user( + request, + session_info, + attribute_mapping, + create_unknown_user, + assertion_info + ) + except PermissionDenied as e: + return self.handle_acs_failure( + request, + exception=e, + session_info=session_info, + ) + + relay_state = self.build_relay_state() + 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) + logger.debug("Redirecting to the RelayState: %s", relay_state) + return HttpResponseRedirect(relay_state) + + def authenticate_user( + self, + request, + session_info, + attribute_mapping, + create_unknown_user, + assertion_info + ): + """Calls Django's authenticate method after the SAML response is verified""" logger.debug("Trying to authenticate the user. Session info: %s", session_info) + user = auth.authenticate( request=request, session_info=session_info, @@ -563,11 +596,7 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None): "Could not authenticate user received in SAML Assertion. Session info: %s", session_info, ) - return self.handle_acs_failure( - request, - exception=PermissionDenied("No user could be authenticated."), - session_info=session_info, - ) + raise PermissionDenied("No user could be authenticated.") auth.login(self.request, user) _set_subject_id(request.saml_session, session_info["name_id"]) @@ -576,14 +605,6 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None): self.post_login_hook(request, user, session_info) self.customize_session(user, session_info) - relay_state = self.build_relay_state() - 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) - logger.debug("Redirecting to the RelayState: %s", relay_state) - return HttpResponseRedirect(relay_state) - def post_login_hook( self, request: HttpRequest, user: settings.AUTH_USER_MODEL, session_info: dict ) -> None: From 09b3801052ecf6c31fbd218d1668c84806cf2571 Mon Sep 17 00:00:00 2001 From: Marcel Kornblum Date: Fri, 28 Apr 2023 14:26:25 +0100 Subject: [PATCH 02/22] fix for split (#2) * split authenticate into separate overridable function * cleaner exception * basic unit test * remove new test * space to revert change * return the user --- djangosaml2/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 86ea19d1..9aa42f46 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -605,6 +605,8 @@ def authenticate_user( self.post_login_hook(request, user, session_info) self.customize_session(user, session_info) + return user + def post_login_hook( self, request: HttpRequest, user: settings.AUTH_USER_MODEL, session_info: dict ) -> None: From 24343221c3d7dafed89e2f56f5afca43ca348624 Mon Sep 17 00:00:00 2001 From: Marcel Kornblum Date: Fri, 28 Apr 2023 15:45:27 +0100 Subject: [PATCH 03/22] bump patch version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 7576e96c..ce7e18d8 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ def read(*rnames): setup( name="djangosaml2", - version="1.5.6", + version="1.5.7", description="pysaml2 integration for Django", long_description=read("README.md"), long_description_content_type="text/markdown", From e2e06d668689feede3a06c7d4a75885b863c13e8 Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Mon, 1 May 2023 11:57:47 +0200 Subject: [PATCH 04/22] Update python-package.yml --- .github/workflows/python-package.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 1b9c6f63..d2b3a51c 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -30,7 +30,8 @@ jobs: - name: Install dependencies and testing utilities run: | sudo apt-get update && sudo apt-get install xmlsec1 - python -m pip install --upgrade pip tox rstcheck setuptools codecov + python -m pip install --upgrade pip + python -m pip install --upgrade tox rstcheck setuptools codecov #- name: Readme check #if: ${{ matrix.python-version }} == 3.8 && ${{ matrix.django-version }} == "3.0" #run: rstcheck README.rst From b4aeb0353f003e9285d01d6039d6c4903d94d4d0 Mon Sep 17 00:00:00 2001 From: Cameron Lamb Date: Fri, 12 May 2023 12:04:50 +0100 Subject: [PATCH 05/22] Separate out the condition for skipping login --- djangosaml2/views.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 9aa42f46..7c73940b 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -174,21 +174,23 @@ def load_sso_kwargs(self, sso_kwargs): def add_idp_hinting(self, http_response): return add_idp_hinting(self.request, http_response) or http_response - def get(self, request, *args, **kwargs): - logger.debug("Login process started") - next_path = self.get_next_path(request) - - # if the user is already authenticated that maybe because of two reasons: + def should_prevent_auth(self, request) -> bool: + # If the user is already authenticated that maybe because of two reasons: # A) He has this URL in two browser windows and in the other one he # has already initiated the authenticated session. # B) He comes from a view that (incorrectly) send him here because # he does not have enough permissions. That view should have shown # an authorization error in the first place. - # We can only make one thing here and that is configurable with the - # SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN setting. If that setting - # is True (default value) we will redirect him to the next_path path. - # Otherwise, we will show an (configurable) authorization error. - if request.user.is_authenticated: + return request.user.is_authenticated + + def get(self, request, *args, **kwargs): + logger.debug("Login process started") + next_path = self.get_next_path(request) + + if self.should_prevent_auth(request): + # If the SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN setting is True + # (default value), redirect to the next_path. Otherwise, show a + # configurable authorization error. if get_custom_setting("SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN", True): return HttpResponseRedirect(next_path) logger.debug("User is already logged in") From 2366a927fa9bb36d1ee49009eda2513d553e7896 Mon Sep 17 00:00:00 2001 From: Cameron Lamb Date: Mon, 15 May 2023 12:15:41 +0100 Subject: [PATCH 06/22] Bump package version to 1.5.8 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ce7e18d8..a97c3313 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ def read(*rnames): setup( name="djangosaml2", - version="1.5.7", + version="1.5.8", description="pysaml2 integration for Django", long_description=read("README.md"), long_description_content_type="text/markdown", From 767b48336a75d33bae028457effd461d85eadf96 Mon Sep 17 00:00:00 2001 From: Kelvin Chan Date: Wed, 14 Jun 2023 11:34:02 -0400 Subject: [PATCH 07/22] Allow overidding samesite value for session cookie --- djangosaml2/middleware.py | 6 ++++-- djangosaml2/tests/__init__.py | 28 ++++++++++++++++++++++++++++ docs/source/contents/setup.rst | 6 +++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/djangosaml2/middleware.py b/djangosaml2/middleware.py index 6418c473..7528c782 100644 --- a/djangosaml2/middleware.py +++ b/djangosaml2/middleware.py @@ -26,6 +26,8 @@ def process_response(self, request, response): session every time, save the changes and set a session cookie or delete the session cookie if the session has been emptied. """ + SAMESITE = getattr(settings, "SAML_SESSION_COOKIE_SAMESITE", SAMESITE_NONE) + try: accessed = request.saml_session.accessed modified = request.saml_session.modified @@ -39,7 +41,7 @@ def process_response(self, request, response): self.cookie_name, path=settings.SESSION_COOKIE_PATH, domain=settings.SESSION_COOKIE_DOMAIN, - samesite=SAMESITE_NONE, + samesite=SAMESITE, ) patch_vary_headers(response, ("Cookie",)) else: @@ -74,6 +76,6 @@ def process_response(self, request, response): path=settings.SESSION_COOKIE_PATH, secure=settings.SESSION_COOKIE_SECURE or None, httponly=settings.SESSION_COOKIE_HTTPONLY or None, - samesite=SAMESITE_NONE, + samesite=SAMESITE, ) return response diff --git a/djangosaml2/tests/__init__.py b/djangosaml2/tests/__init__.py index 251c3549..59223410 100644 --- a/djangosaml2/tests/__init__.py +++ b/djangosaml2/tests/__init__.py @@ -1030,3 +1030,31 @@ def test_middleware_cookie_with_expiry(self): self.assertIsNotNone(cookie["expires"]) self.assertNotEqual(cookie["expires"], "") self.assertNotEqual(cookie["max-age"], "") + + def test_middleware_cookie_samesite(self): + with override_settings(SAML_SESSION_COOKIE_SAMESITE="Lax"): + 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(dummy_get_response) + 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["samesite"], "Lax") diff --git a/docs/source/contents/setup.rst b/docs/source/contents/setup.rst index 89f9d295..571b2211 100644 --- a/docs/source/contents/setup.rst +++ b/docs/source/contents/setup.rst @@ -62,6 +62,10 @@ You can even configure the SAML cookie name as follows:: SAML_SESSION_COOKIE_NAME = 'saml_session' +By default, djangosaml2 will set "SameSite=None" for the SAML session cookie. This value can be configured as follows:: + + SAML_SESSION_COOKIE_SAMESITE = 'Lax' + Remember that in your browser "SameSite=None" attribute MUST also have the "Secure" attribute, which is required in order to use "SameSite=None", otherwise the cookie will be blocked, so you must also set:: @@ -69,7 +73,7 @@ have the "Secure" attribute, which is required in order to use "SameSite=None", .. Note:: - djangosaml2 will attempt to set the ``SameSite`` attribute of the SAML session cookie to ``None`` so that it can be + djangosaml2 will by default attempt to set the ``SameSite`` attribute of the SAML session cookie to ``None`` so that it can be used in cross-site requests, but this is only possible with Django 3.1 or higher. If you are experiencing issues with unsolicited requests or cookies not being sent (particularly when using the HTTP-POST binding), consider upgrading to Django 3.1 or higher. If you can't do that, configure "allow_unsolicited" to True in pySAML2 configuration. From 4fe6cc3c02415f3f0201eca9d61515b5dcbd2e4b Mon Sep 17 00:00:00 2001 From: Kelvin Chan Date: Thu, 15 Jun 2023 09:00:36 -0400 Subject: [PATCH 08/22] Bump version to 1.6.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a97c3313..ab7446c5 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ def read(*rnames): setup( name="djangosaml2", - version="1.5.8", + version="1.6.0", description="pysaml2 integration for Django", long_description=read("README.md"), long_description_content_type="text/markdown", From 456b1b7005dbe8492dec3ecb6ec2383e1cc2329a Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Mon, 19 Jun 2023 18:51:23 +0200 Subject: [PATCH 09/22] fix: doc heading --- docs/source/contents/setup.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/contents/setup.rst b/docs/source/contents/setup.rst index 571b2211..a7fda926 100644 --- a/docs/source/contents/setup.rst +++ b/docs/source/contents/setup.rst @@ -247,7 +247,7 @@ setting:: SAML_CONFIG_LOADER = 'python.path.to.your.callable' Bearer Assertion Replay Attack Prevention -================================== +========================================= In SAML standard doc, section 4.1.4.5 it states The service provider MUST ensure that bearer assertions are not replayed, by maintaining the set of used ID values for the length of time for which the assertion would be considered valid based on the NotOnOrAfter attribute in the From 855a31dbe7ac56aa56fa6707933f525ac084b8f9 Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Mon, 19 Jun 2023 18:54:55 +0200 Subject: [PATCH 10/22] fix: docs logo --- docs/source/_templates/pplnx_template/layout.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/_templates/pplnx_template/layout.html b/docs/source/_templates/pplnx_template/layout.html index 1bca49b8..c3d81e8e 100644 --- a/docs/source/_templates/pplnx_template/layout.html +++ b/docs/source/_templates/pplnx_template/layout.html @@ -13,7 +13,7 @@ {# Not strictly valid HTML, but it's the only way to display/scale it properly, without weird scripting or heaps of work #} - + {% endif %} {% if logo and theme_logo_only %} @@ -45,7 +45,7 @@ {% block mobile_nav %} - + {{ project }} From b58e471594eba8152c550e85c309f99d5b87bfbe Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Mon, 19 Jun 2023 18:58:55 +0200 Subject: [PATCH 11/22] fix: pypy.yml to pypi.yml --- .github/workflows/{pypy.yml => pypi.yml} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .github/workflows/{pypy.yml => pypi.yml} (97%) diff --git a/.github/workflows/pypy.yml b/.github/workflows/pypi.yml similarity index 97% rename from .github/workflows/pypy.yml rename to .github/workflows/pypi.yml index 4a916526..bb9da865 100644 --- a/.github/workflows/pypy.yml +++ b/.github/workflows/pypi.yml @@ -2,7 +2,7 @@ name: Publish Python distribution to PyPI on: release: types: - - created + - published jobs: build-n-publish: From c0fb589f7dd56ccabe4ba5c961ae24562464dffc Mon Sep 17 00:00:00 2001 From: Yon Ploj Date: Tue, 20 Jun 2023 11:49:19 +0200 Subject: [PATCH 12/22] Support next and RelayState on logout Fixes #379 --- djangosaml2/views.py | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 7c73940b..483b77ca 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -15,6 +15,7 @@ import base64 import logging +from typing import Optional from urllib.parse import quote from django.conf import settings @@ -89,6 +90,18 @@ def _get_subject_id(session): return None +def _get_next_path(request: HttpRequest) -> Optional[str]: + if "next" in request.GET: + next_path = request.GET["next"] + elif "RelayState" in request.GET: + next_path = request.GET["RelayState"] + else: + return None + + next_path = validate_referral_url(request, next_path) + return next_path + + class SPConfigMixin: """Mixin for some of the SAML views with re-usable methods.""" @@ -138,20 +151,6 @@ class LoginView(SPConfigMixin, View): "djangosaml2/post_binding_form.html", ) - 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 = get_fallback_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 - def unknown_idp(self, request, idp): msg = f"Error: IdP EntityID {escape(idp)} was not found in metadata" logger.error(msg) @@ -185,7 +184,9 @@ def should_prevent_auth(self, request) -> bool: def get(self, request, *args, **kwargs): logger.debug("Login process started") - next_path = self.get_next_path(request) + next_path = _get_next_path(request) + if next_path is None: + next_path = get_fallback_login_redirect_url() if self.should_prevent_auth(request): # If the SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN setting is True @@ -822,8 +823,12 @@ def finish_logout(request, response): auth.logout(request) - if settings.LOGOUT_REDIRECT_URL is not None: - return HttpResponseRedirect(resolve_url(settings.LOGOUT_REDIRECT_URL)) + next_path = _get_next_path(request) + if next_path is not None: + return HttpResponseRedirect(next_path) + elif settings.LOGOUT_REDIRECT_URL is not None: + fallback_url = resolve_url(settings.LOGOUT_REDIRECT_URL) + return HttpResponseRedirect(fallback_url) else: current_site = get_current_site(request) return render( From 5351bd9dc532aae975ca37e5701b1e8ea07311bb Mon Sep 17 00:00:00 2001 From: Yon Ploj Date: Thu, 22 Jun 2023 12:41:09 +0200 Subject: [PATCH 13/22] Bump version to 1.7.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ab7446c5..b4a21e57 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ def read(*rnames): setup( name="djangosaml2", - version="1.6.0", + version="1.7.0", description="pysaml2 integration for Django", long_description=read("README.md"), long_description_content_type="text/markdown", From e6d905c26bf0a76b59528fc06e8e37c1baa69917 Mon Sep 17 00:00:00 2001 From: Guillaume Andreu Sabater Date: Mon, 25 Sep 2023 12:10:32 +0200 Subject: [PATCH 14/22] removed python3.7 --- .github/workflows/python-package.yml | 3 --- README.md | 2 +- setup.py | 1 - tox.ini | 2 +- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index d2b3a51c..e9dfb036 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -17,9 +17,6 @@ jobs: matrix: python-version: ["3.8", "3.9", "3.10"] django-version: ["3.2", "4.0", "4.1"] - include: - - python-version: "3.7" - django-version: "3.2" steps: - uses: actions/checkout@v2 diff --git a/README.md b/README.md index 0e1d36b5..e4bd10ae 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ djangosaml2 ![Python version](https://img.shields.io/badge/license-Apache%202-blue.svg) ![Django versions](https://img.shields.io/pypi/djversions/djangosaml2) ![Documentation Status](https://readthedocs.org/projects/djangosaml2/badge/?version=latest) -![License](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg) +![License](https://img.shields.io/badge/python-3.8%20%7C%203.9%20%7C%203.10-blue.svg) A Django application that builds a Fully Compliant SAML2 Service Provider on top of PySAML2 library. diff --git a/setup.py b/setup.py index b4a21e57..d4afe4d3 100644 --- a/setup.py +++ b/setup.py @@ -42,7 +42,6 @@ def read(*rnames): "License :: OSI Approved :: Apache Software License", "Operating System :: OS Independent", "Programming Language :: Python", - "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", diff --git a/tox.ini b/tox.ini index 5507db5a..0c33dcf9 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] envlist = - py{3.7,3.8,3.9,3.10}-django{3.2,4.0,4.1} + py{3.8,3.9,3.10}-django{3.2,4.0,4.1} [testenv] commands = From 837445c5b501afccce2d34d9bf051299ce1be4be Mon Sep 17 00:00:00 2001 From: Guillaume Andreu Sabater Date: Mon, 25 Sep 2023 12:11:48 +0200 Subject: [PATCH 15/22] added python3.11 --- .github/workflows/python-package.yml | 5 ++++- README.md | 2 +- setup.py | 1 + tox.ini | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index e9dfb036..5315f019 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -15,8 +15,11 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.8", "3.9", "3.10"] + python-version: ["3.8", "3.9", "3.10", "3.11"] django-version: ["3.2", "4.0", "4.1"] + exclude: + - python-version: "3.11" + django-version: "3.2" steps: - uses: actions/checkout@v2 diff --git a/README.md b/README.md index e4bd10ae..3d688405 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ djangosaml2 ![Python version](https://img.shields.io/badge/license-Apache%202-blue.svg) ![Django versions](https://img.shields.io/pypi/djversions/djangosaml2) ![Documentation Status](https://readthedocs.org/projects/djangosaml2/badge/?version=latest) -![License](https://img.shields.io/badge/python-3.8%20%7C%203.9%20%7C%203.10-blue.svg) +![License](https://img.shields.io/badge/python-3.8%20%7C%203.9%20%7C%203.10%20%7C%203.11-blue.svg) A Django application that builds a Fully Compliant SAML2 Service Provider on top of PySAML2 library. diff --git a/setup.py b/setup.py index d4afe4d3..5cbb79c7 100644 --- a/setup.py +++ b/setup.py @@ -45,6 +45,7 @@ def read(*rnames): "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", "Topic :: Internet :: WWW/HTTP", "Topic :: Internet :: WWW/HTTP :: WSGI", "Topic :: Security", diff --git a/tox.ini b/tox.ini index 0c33dcf9..343844ae 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] envlist = - py{3.8,3.9,3.10}-django{3.2,4.0,4.1} + py{3.8,3.9,3.10,3.11}-django{3.2,4.0,4.1} [testenv] commands = From d384877760385bb09e37b696c4e128b44750191e Mon Sep 17 00:00:00 2001 From: Guillaume Andreu Sabater Date: Mon, 25 Sep 2023 12:17:29 +0200 Subject: [PATCH 16/22] added django5.0 --- .github/workflows/python-package.yml | 6 +++++- setup.py | 5 +++-- tox.ini | 7 ++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 5315f019..54495729 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -16,10 +16,14 @@ jobs: strategy: matrix: python-version: ["3.8", "3.9", "3.10", "3.11"] - django-version: ["3.2", "4.0", "4.1"] + django-version: ["3.2", "4.1", "4.2", "5.0"] exclude: - python-version: "3.11" django-version: "3.2" + - python-version: "3.8" + django-version: "5.0" + - python-version: "3.9" + django-version: "5.0" steps: - uses: actions/checkout@v2 diff --git a/setup.py b/setup.py index 5cbb79c7..2d33cdee 100644 --- a/setup.py +++ b/setup.py @@ -36,8 +36,9 @@ def read(*rnames): "Environment :: Web Environment", "Framework :: Django", "Framework :: Django :: 3.2", - "Framework :: Django :: 4.0", "Framework :: Django :: 4.1", + "Framework :: Django :: 4.2", + "Framework :: Django :: 5.0", "Intended Audience :: Developers", "License :: OSI Approved :: Apache Software License", "Operating System :: OS Independent", @@ -61,5 +62,5 @@ def read(*rnames): packages=find_packages(exclude=["tests", "tests.*"]), include_package_data=True, zip_safe=False, - install_requires=["defusedxml>=0.4.1", "Django>=2.2,<5", "pysaml2>=6.5.1"], + install_requires=["defusedxml>=0.4.1", "Django>=3.2", "pysaml2>=6.5.1"], ) diff --git a/tox.ini b/tox.ini index 343844ae..8830d972 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] envlist = - py{3.8,3.9,3.10,3.11}-django{3.2,4.0,4.1} + py{3.8,3.9,3.10,3.11}-django{3.2,4.1,4.2,5.0} [testenv] commands = @@ -9,8 +9,9 @@ commands = deps = django3.2: django~=3.2 - django4.0: django~=4.0 - django4.1: django==4.1b1 + django4.1: django~=4.1 + django4.2: django~=4.2 + django5.0: django==5.0a1 djangomaster: https://github.com/django/django/archive/master.tar.gz . From f23fe232801d47cb4a8b5406844336bf8f87959f Mon Sep 17 00:00:00 2001 From: Guillaume Andreu Sabater Date: Wed, 27 Sep 2023 16:46:07 +0200 Subject: [PATCH 17/22] .github: bumped actions --- .github/workflows/python-package.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 54495729..b7902a2e 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -26,11 +26,12 @@ jobs: django-version: "5.0" steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v1 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} + allow-prereleases: true - name: Install dependencies and testing utilities run: | sudo apt-get update && sudo apt-get install xmlsec1 From 57fec3cca050cb422ed41dcaa1a829701ba97557 Mon Sep 17 00:00:00 2001 From: Guillaume Andreu Sabater Date: Wed, 27 Sep 2023 16:37:55 +0200 Subject: [PATCH 18/22] added python3.12 compat --- .github/workflows/python-package.yml | 5 +++++ README.md | 6 +++--- setup.py | 1 + tox.ini | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index b7902a2e..ef280e58 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -17,6 +17,11 @@ jobs: matrix: python-version: ["3.8", "3.9", "3.10", "3.11"] django-version: ["3.2", "4.1", "4.2", "5.0"] + include: + - python-version: "3.12" + django-version: "4.2" + - python-version: "3.12" + django-version: "5.0" exclude: - python-version: "3.11" django-version: "3.2" diff --git a/README.md b/README.md index 3d688405..9af17f6c 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,10 @@ djangosaml2 ![CI build](https://github.com/peppelinux/djangosaml2/workflows/djangosaml2/badge.svg) ![pypi](https://img.shields.io/pypi/v/djangosaml2.svg) [![Downloads](https://pepy.tech/badge/djangosaml2/month)](https://pepy.tech/project/djangosaml2) -![Python version](https://img.shields.io/badge/license-Apache%202-blue.svg) -![Django versions](https://img.shields.io/pypi/djversions/djangosaml2) ![Documentation Status](https://readthedocs.org/projects/djangosaml2/badge/?version=latest) -![License](https://img.shields.io/badge/python-3.8%20%7C%203.9%20%7C%203.10%20%7C%203.11-blue.svg) +![License](https://img.shields.io/badge/license-Apache%202-blue.svg) +![Python versions](https://img.shields.io/pypi/pyversions/djangosaml2) +![Django versions](https://img.shields.io/pypi/djversions/djangosaml2) A Django application that builds a Fully Compliant SAML2 Service Provider on top of PySAML2 library. diff --git a/setup.py b/setup.py index 2d33cdee..ab0d7e0d 100644 --- a/setup.py +++ b/setup.py @@ -47,6 +47,7 @@ def read(*rnames): "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", "Topic :: Internet :: WWW/HTTP", "Topic :: Internet :: WWW/HTTP :: WSGI", "Topic :: Security", diff --git a/tox.ini b/tox.ini index 8830d972..06f57b11 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] envlist = - py{3.8,3.9,3.10,3.11}-django{3.2,4.1,4.2,5.0} + py{3.8,3.9,3.10,3.11,3.12}-django{3.2,4.1,4.2,5.0} [testenv] commands = From 3b8d0d64a283d6337a7dc3f9aa2f7554a5c9cc13 Mon Sep 17 00:00:00 2001 From: "Mees, T.D. (Ty)" Date: Mon, 2 Oct 2023 13:32:29 +0200 Subject: [PATCH 19/22] 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 --- djangosaml2/utils.py | 24 +++++++++++++++++++++++- djangosaml2/views.py | 14 ++++++++++++++ docs/source/contents/setup.rst | 18 ++++++++++++++++-- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/djangosaml2/utils.py b/djangosaml2/utils.py index 1b4e824a..1ab50d7a 100644 --- a/djangosaml2/utils.py +++ b/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,24 @@ 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: + 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 +128,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 diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 483b77ca..402eec4d 100644 --- a/djangosaml2/views.py +++ b/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 relay_state is None: + 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", diff --git a/docs/source/contents/setup.rst b/docs/source/contents/setup.rst index a7fda926..c9831b7e 100644 --- a/docs/source/contents/setup.rst +++ b/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,20 @@ 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. + + Preferred sso binding ===================== From 9bfd3df73853053910bf4e3f2871badb0e25e3e4 Mon Sep 17 00:00:00 2001 From: Ty Mees Date: Mon, 2 Oct 2023 15:26:32 +0200 Subject: [PATCH 20/22] Use more generic False-y check This will prevent redirect loops with the strict validation disabled Co-authored-by: Giuseppe De Marco --- djangosaml2/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 402eec4d..44effaab 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -575,7 +575,7 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None): return HttpResponseRedirect(custom_redirect_url) relay_state = validate_referral_url(request, relay_state) - if relay_state is None: + if not relay_state: logger.debug( "RelayState is not a valid URL, redirecting to fallback: %s", relay_state From 64f6bcee47bb6dfecf9d50fdc17cbd1652d97fe2 Mon Sep 17 00:00:00 2001 From: "Mees, T.D. (Ty)" Date: Mon, 2 Oct 2023 15:43:53 +0200 Subject: [PATCH 21/22] Document named URL patterns also being resolved --- djangosaml2/utils.py | 1 + docs/source/contents/setup.rst | 2 ++ 2 files changed, 3 insertions(+) diff --git a/djangosaml2/utils.py b/djangosaml2/utils.py index 1ab50d7a..b5e24b79 100644 --- a/djangosaml2/utils.py +++ b/djangosaml2/utils.py @@ -113,6 +113,7 @@ def validate_referral_url(request, url): ) 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") diff --git a/docs/source/contents/setup.rst b/docs/source/contents/setup.rst index c9831b7e..886b4128 100644 --- a/docs/source/contents/setup.rst +++ b/docs/source/contents/setup.rst @@ -151,6 +151,8 @@ 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`_ +will also be resolved. Turning off strict validation will prevent this from happening. Preferred sso binding ===================== From add2fe56b9aa82bfacbbcda97450f2dff6607a2c Mon Sep 17 00:00:00 2001 From: "Mees, T.D. (Ty)" Date: Mon, 2 Oct 2023 15:44:13 +0200 Subject: [PATCH 22/22] Bump version to 1.8.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ab0d7e0d..db40249c 100644 --- a/setup.py +++ b/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",