Merge branch 'master' into dev

Author: Giuseppe De Marco

.github/workflows/pypy.yml renamed to .github/workflows/pypi.yml

@@ -2,7 +2,7 @@ name: Publish Python distribution to PyPI
 on:
   release:
     types:
-      - created
+      - published
 
 jobs:
   build-n-publish:

.github/workflows/python-package.yml

@@ -15,22 +15,33 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ["3.8", "3.9", "3.10"]
-        django-version: ["3.2", "4.0", "4.1"]
+        python-version: ["3.8", "3.9", "3.10", "3.11"]
+        django-version: ["3.2", "4.1", "4.2", "5.0"]
         include:
-        - python-version: "3.7"
+        - 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"
+        - python-version: "3.8"
+          django-version: "5.0"
+        - python-version: "3.9"
+          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
-        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

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.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-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.

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

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")

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

@@ -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,19 @@ 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 +152,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)
@@ -174,21 +174,25 @@ 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 = _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
+            # (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")
@@ -550,7 +554,48 @@ 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)
+        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)
+
+    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 +608,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,13 +617,7 @@ 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)
+        return user
 
     def post_login_hook(
         self, request: HttpRequest, user: settings.AUTH_USER_MODEL, session_info: dict
@@ -797,10 +832,19 @@ 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:
+            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/_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
     #}
-    <a href="{{ pathto(master_doc) }}" id="logo_main"><img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="Logo" /></a>
+    <a href="{{ pathto(master_doc) }}" id="logo_main"><img src="{{ pathto('_static/logo.jpg', 1) }}" class="logo" alt="Logo" /></a>
 {% endif %}
 
 {% if logo and theme_logo_only %}