diff --git a/.github/workflows/pypy.yml b/.github/workflows/pypi.yml similarity index 90% rename from .github/workflows/pypy.yml rename to .github/workflows/pypi.yml index 4a916526..80c45871 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: @@ -10,10 +10,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@master - - name: Setup Python 3.8 + - name: Setup Python 3.10 uses: actions/setup-python@v1 with: - python-version: 3.8 + python-version: "3.10" - name: Install pypa/build run: >- python -m diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 1b9c6f63..0f9a1634 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -15,22 +15,26 @@ 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.10", "3.11", "3.12"] + django-version: ["4.2", "5.0", "5.1"] include: - - python-version: "3.7" - django-version: "3.2" + - python-version: "3.9" + django-version: "4.2" + - python-version: "3.13" + django-version: "5.1" 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 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a662b442..c3e6be30 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,7 +2,7 @@ exclude: 'docs|migrations' repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.3.0 + rev: v5.0.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -13,37 +13,37 @@ repos: - id: debug-statements - repo: https://github.com/asottile/pyupgrade - rev: v2.34.0 + rev: v3.19.0 hooks: - id: pyupgrade - args: [--py37-plus] + args: [--py39-plus] - repo: https://github.com/myint/autoflake - rev: 'v1.4' + rev: 'v2.3.1' hooks: - id: autoflake args: ['--in-place', '--remove-all-unused-imports', '--ignore-init-module-imports'] - repo: https://github.com/pycqa/isort - rev: 5.10.1 + rev: 5.13.2 hooks: - id: isort name: isort (python) args: ['--settings-path=pyproject.toml'] - repo: https://github.com/psf/black - rev: 22.6.0 + rev: 24.10.0 hooks: - id: black - repo: https://github.com/adamchainz/django-upgrade - rev: 1.7.0 + rev: 1.22.2 hooks: - id: django-upgrade - args: [--target-version, "3.2"] + args: [--target-version, "4.2"] - repo: https://github.com/pycqa/flake8 - rev: 4.0.1 + rev: 7.1.1 hooks: - id: flake8 args: ['--config=setup.cfg'] diff --git a/README.md b/README.md index 0e1d36b5..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.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. diff --git a/djangosaml2/backends.py b/djangosaml2/backends.py index e7f36fc2..ad4d5fa6 100644 --- a/djangosaml2/backends.py +++ b/djangosaml2/backends.py @@ -15,7 +15,7 @@ import logging import warnings -from typing import Any, Optional, Tuple +from typing import Any, Optional from django.apps import apps from django.conf import settings @@ -72,7 +72,7 @@ def _user_lookup_attribute(self) -> str: def _extract_user_identifier_params( self, session_info: dict, attributes: dict, attribute_mapping: dict - ) -> Tuple[str, Optional[Any]]: + ) -> tuple[str, Optional[Any]]: """Returns the attribute to perform a user lookup on, and the value to use for it. The value could be the name_id, or any other saml attribute from the request. """ @@ -262,8 +262,8 @@ def get_or_create_user( attributes: dict, attribute_mapping: dict, request, - ) -> Tuple[Optional[settings.AUTH_USER_MODEL], bool]: - """Look up the user to authenticate. If he doesn't exist, this method creates him (if so desired). + ) -> tuple[Optional[settings.AUTH_USER_MODEL], bool]: + """Look up the user to authenticate. If they doesn't exist, this method creates them (if so desired). The default implementation looks only at the user_identifier. Override this method in order to do more complex behaviour, e.g. customize this per IdP. """ @@ -277,25 +277,23 @@ def get_or_create_user( ): user_lookup_value } - # Lookup existing user # Lookup existing user user, created = None, False try: user = UserModel.objects.get(**user_query_args) except MultipleObjectsReturned: - logger.error( - "Multiple users match, model: %s, lookup: %s", - UserModel._meta, - user_query_args, + logger.exception( + f"Multiple users match, model: {UserModel._meta}, lookup: {user_query_args}", ) except UserModel.DoesNotExist: # Create new one if desired by settings if create_unknown_user: user = UserModel(**{user_lookup_key: user_lookup_value}) + user.set_unusable_password() created = True - logger.debug(f"New user created: {user}") + logger.debug(f"New user created: {user}", exc_info=True) else: - logger.error( + logger.exception( f"The user does not exist, model: {UserModel._meta}, lookup: {user_query_args}" ) @@ -323,6 +321,7 @@ def get_attribute_value(self, django_field, attributes, attribute_mapping): warnings.warn( "get_attribute_value() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return self._get_attribute_value(django_field, attributes, attribute_mapping) @@ -330,6 +329,7 @@ def get_django_user_main_attribute(self): warnings.warn( "get_django_user_main_attribute() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return self._user_lookup_attribute @@ -337,6 +337,7 @@ def get_django_user_main_attribute_lookup(self): warnings.warn( "get_django_user_main_attribute_lookup() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return getattr(settings, "SAML_DJANGO_USER_MAIN_ATTRIBUTE_LOOKUP", "") @@ -344,6 +345,7 @@ def get_user_query_args(self, main_attribute): warnings.warn( "get_user_query_args() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return { self.get_django_user_main_attribute() @@ -354,6 +356,7 @@ def configure_user(self, user, attributes, attribute_mapping): warnings.warn( "configure_user() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return self._update_user(user, attributes, attribute_mapping) @@ -361,6 +364,7 @@ def update_user(self, user, attributes, attribute_mapping, force_save=False): warnings.warn( "update_user() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return self._update_user(user, attributes, attribute_mapping) @@ -368,6 +372,7 @@ def _set_attribute(self, obj, attr, value): warnings.warn( "_set_attribute() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return set_attribute(obj, attr, value) @@ -376,5 +381,6 @@ def get_saml_user_model(): warnings.warn( "_set_attribute() is deprecated, look at the Saml2Backend on how to subclass it", DeprecationWarning, + stacklevel=2, ) return Saml2Backend()._user_model diff --git a/djangosaml2/cache.py b/djangosaml2/cache.py index b3d31fc4..546df556 100644 --- a/djangosaml2/cache.py +++ b/djangosaml2/cache.py @@ -72,7 +72,7 @@ class IdentityCache(Cache): logged in. This information is useful because when the user logs out we must - know where does he come from in order to notify such IdP/AA. + know where does they come from in order to notify such IdP/AA. The current implementation stores this information in the Django session. """ 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/overrides.py b/djangosaml2/overrides.py index f2b8ce34..8f6bc6bf 100644 --- a/djangosaml2/overrides.py +++ b/djangosaml2/overrides.py @@ -19,9 +19,9 @@ class Saml2Client(saml2.client.Saml2Client): def do_logout(self, *args, **kwargs): if not kwargs.get("expected_binding"): try: - kwargs[ - "expected_binding" - ] = settings.SAML_LOGOUT_REQUEST_PREFERRED_BINDING + kwargs["expected_binding"] = ( + settings.SAML_LOGOUT_REQUEST_PREFERRED_BINDING + ) except AttributeError: logger.warning( "SAML_LOGOUT_REQUEST_PREFERRED_BINDING setting is" diff --git a/djangosaml2/templates/djangosaml2/post_binding_form.html b/djangosaml2/templates/djangosaml2/post_binding_form.html new file mode 100644 index 00000000..e70c183a --- /dev/null +++ b/djangosaml2/templates/djangosaml2/post_binding_form.html @@ -0,0 +1,15 @@ + +

+You're being redirected to a SSO login page. +Please click the button below if you're not redirected automatically within a few seconds. +

+
+ {% for key, value in params.items %} + + {% endfor %} + +
diff --git a/djangosaml2/tests/__init__.py b/djangosaml2/tests/__init__.py index 251c3549..56dc7aeb 100644 --- a/djangosaml2/tests/__init__.py +++ b/djangosaml2/tests/__init__.py @@ -139,9 +139,9 @@ def add_outstanding_query(self, session_id, came_from): came_from, ) self.saml_session.save() - self.client.cookies[ - settings.SESSION_COOKIE_NAME - ] = self.saml_session.session_key + self.client.cookies[settings.SESSION_COOKIE_NAME] = ( + self.saml_session.session_key + ) def b64_for_post(self, xml_text, encoding="utf-8"): return base64.b64encode(xml_text.encode(encoding)).decode("ascii") @@ -308,8 +308,12 @@ def test_unknown_idp(self): metadata_file="remote_metadata_three_idps.xml", ) - response = self.client.get(reverse("saml2_login") + "?idp=https://unknown.org") - self.assertContains(response, "<b>https://unknown.org</b>", status_code=403) + response = self.client.get( + reverse("saml2_login") + "?idp=https://unknown.org" + ) + self.assertContains( + response, "<b>https://unknown.org</b>", status_code=403 + ) def test_login_authn_context(self): sp_kwargs = { @@ -462,6 +466,9 @@ def test_assertion_consumer_service(self): user_id = self.client.session[SESSION_KEY] user = User.objects.get(id=user_id) self.assertEqual(user.username, "student") + # Since a new user object is created, the password + # field is set to have an unusable password. + self.assertEqual(user.has_usable_password(), False) # let's create another user and log in with that one new_user = User.objects.create(username="teacher", password="not-used") @@ -486,6 +493,10 @@ def test_assertion_consumer_service(self): # as the RelayState is empty we have redirect to ACS_DEFAULT_REDIRECT_URL self.assertRedirects(response, "/dashboard/") self.assertEqual(str(new_user.id), client.session[SESSION_KEY]) + new_user.refresh_from_db() + # Since "new_user" already had a password, + # the password field will remain unchanged. + self.assertEqual(new_user.has_usable_password(), True) @override_settings(ACS_DEFAULT_REDIRECT_URL="testprofiles:dashboard") def test_assertion_consumer_service_default_relay_state(self): @@ -1030,3 +1041,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/djangosaml2/utils.py b/djangosaml2/utils.py index 1b4e824a..10d022dd 100644 --- a/djangosaml2/utils.py +++ b/djangosaml2/utils.py @@ -16,13 +16,17 @@ import re import urllib import zlib +from functools import lru_cache, wraps from typing import Optional +from importlib.metadata import version, PackageNotFoundError 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.urls import NoReverseMatch from django.utils.http import url_has_allowed_host_and_scheme +from django.utils.module_loading import import_string from saml2.config import SPConfig from saml2.mdstore import MetaDataMDX @@ -44,7 +48,7 @@ def available_idps(config: SPConfig, langpref=None, idp_to_check: str = None) -> for metadata in config.metadata.metadata.values(): # initiate a fetch to the selected idp when using MDQ, otherwise the MetaDataMDX is an empty database if isinstance(metadata, MetaDataMDX) and idp_to_check: - m = metadata[idp_to_check] + m = metadata[idp_to_check] # noqa: F841 result = metadata.any("idpsso_descriptor", "single_sign_on_service") if result: idps.update(result.keys()) @@ -79,7 +83,7 @@ def get_idp_sso_supported_bindings( except UnknownSystemEntity: raise UnknownSystemEntity except Exception as e: - logger.error(f"get_idp_sso_supported_bindings failed with: {e}") + logger.exception(f"get_idp_sso_supported_bindings failed with: {e}") def get_location(http_info): @@ -99,6 +103,23 @@ 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", exc_info=True + ) + 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 +130,9 @@ 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 @@ -181,3 +204,78 @@ def add_idp_hinting(request, http_response) -> bool: f"Idp hinting: cannot detect request type [{http_response.status_code}]" ) return False + + +@lru_cache +def get_csp_handler(): + """Returns a view decorator for CSP.""" + + def empty_view_decorator(view): + return view + + csp_handler_string = get_custom_setting("SAML_CSP_HANDLER", None) + + if csp_handler_string is None: + # No CSP handler configured, attempt to use django-csp + return _django_csp_update_decorator() or empty_view_decorator + + if csp_handler_string.strip() != "": + # Non empty string is configured, attempt to import it + csp_handler = import_string(csp_handler_string) + + def custom_csp_updater(f): + @wraps(f) + def wrapper(*args, **kwargs): + return csp_handler(f(*args, **kwargs)) + + return wrapper + + return custom_csp_updater + + # Fall back to empty decorator when csp_handler_string is empty + return empty_view_decorator + + +def _django_csp_update_decorator(): + """Returns a view CSP decorator if django-csp is available, otherwise None.""" + try: + from csp.decorators import csp_update + import csp + except ModuleNotFoundError: + # If csp is not installed, do not update fields as Content-Security-Policy + # is not used + logger.warning( + "django-csp could not be found, not updating Content-Security-Policy. Please " + "make sure CSP is configured. This can be done by your reverse proxy, " + "django-csp or a custom CSP handler via SAML_CSP_HANDLER. See " + "https://djangosaml2.readthedocs.io/contents/security.html#content-security-policy" + " for more information. " + "This warning can be disabled by setting `SAML_CSP_HANDLER=''` in your settings." + ) + return + else: + # autosubmit of forms uses nonce per default + # form-action https: to send data to IdPs + # Check django-csp version to determine the appropriate format + try: + csp_version = version('django-csp') + major_version = int(csp_version.split('.')[0]) + + # Version detection successful + if major_version >= 4: + # django-csp 4.0+ uses dict format with named 'config' parameter + return csp_update(config={"form-action": ["https:"]}) + # django-csp < 4.0 uses kwargs format + return csp_update(FORM_ACTION=["https:"]) + except (PackageNotFoundError, ValueError, RuntimeError, AttributeError, IndexError): + # Version detection failed, we need to try both formats + # Try v4.0+ style first because: + # 1. It has better error handling with clear messages + # 2. Newer versions are more likely to be supported in the future + # 3. If using kwargs with v4.0, it raises a specific RuntimeError we can catch + try: + return csp_update(config={"form-action": ["https:"]}) + except (TypeError, RuntimeError): + # TypeErrors could happen if config is not a recognized parameter (v3.x) + # RuntimeErrors could happen in v4.0+ if we try the wrong approach + return csp_update(FORM_ACTION=["https:"]) diff --git a/djangosaml2/views.py b/djangosaml2/views.py index 9b2d9e93..0d1180a3 100644 --- a/djangosaml2/views.py +++ b/djangosaml2/views.py @@ -15,6 +15,8 @@ import base64 import logging +from functools import wraps +from typing import Optional from urllib.parse import quote from django.conf import settings @@ -68,6 +70,7 @@ from .utils import ( add_idp_hinting, available_idps, + get_csp_handler, get_custom_setting, get_fallback_login_redirect_url, get_idp_sso_supported_bindings, @@ -78,6 +81,16 @@ logger = logging.getLogger("djangosaml2") +def saml2_csp_update(view): + csp_handler = get_csp_handler() + + @wraps(view) + def wrapper(*args, **kwargs): + return csp_handler(view)(*args, **kwargs) + + return wrapper + + def _set_subject_id(session, subject_id): session["_saml2_subject_id"] = code(subject_id) @@ -89,6 +102,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.""" @@ -109,6 +135,7 @@ def get_state_client(self, request: HttpRequest): return state, client +@method_decorator(saml2_csp_update, name="dispatch") class LoginView(SPConfigMixin, View): """SAML Authorization Request initiator. @@ -138,23 +165,9 @@ 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) + logger.exception(msg) return HttpResponse(msg, status=403) def load_sso_kwargs_scoping(self, sso_kwargs): @@ -174,21 +187,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 should_prevent_auth(self, request) -> bool: + # If the user is already authenticated that maybe because of two reasons: + # A) They have this URL in two browser windows and in the other one they + # have already initiated the authenticated session. + # B) They comes from a view that (incorrectly) sends them here because + # they do not have enough permissions. That view should have shown + # an authorization error in the first place. + return request.user.is_authenticated + 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: - # 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: + 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") @@ -333,7 +350,7 @@ def get(self, request, *args, **kwargs): **sso_kwargs, ) except TypeError as e: - logger.error(f"{_msg}: {e}") + logger.exception(f"{_msg}: {e}") return HttpResponse(_msg) else: http_response = HttpResponseRedirect(get_location(result)) @@ -344,7 +361,7 @@ def get(self, request, *args, **kwargs): try: location = client.sso_location(selected_idp, binding) except TypeError as e: - logger.error(f"{_msg}: {e}") + logger.exception(f"{_msg}: {e}") return HttpResponse(_msg) session_id, request_xml = client.create_authn_request( @@ -371,7 +388,8 @@ def get(self, request, *args, **kwargs): ) except TemplateDoesNotExist as e: logger.debug( - f"TemplateDoesNotExist: [{self.post_binding_form_template}] - {e}" + f"TemplateDoesNotExist: [{self.post_binding_form_template}] - {e}", + exc_info=True, ) if not http_response: @@ -385,7 +403,7 @@ def get(self, request, *args, **kwargs): ) except TypeError as e: _msg = f"Can't prepare the authentication for {selected_idp}" - logger.error(f"{_msg}: {e}") + logger.exception(f"{_msg}: {e}") return HttpResponse(_msg) else: http_response = HttpResponse(result["data"]) @@ -520,7 +538,7 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None): try: self.custom_validation(response) except Exception as e: - logger.warning(f"SAML Response validation error: {e}") + logger.warning(f"SAML Response validation error: {e}", exc_info=True) return self.handle_acs_failure( request, status=400, @@ -550,7 +568,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 +622,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 +631,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 @@ -636,6 +685,7 @@ def get(self, request, *args, **kwargs): ) +@method_decorator(saml2_csp_update, name="dispatch") class LogoutInitView(LoginRequiredMixin, SPConfigMixin, View): """SAML Logout Request initiator @@ -714,7 +764,7 @@ def handle_unsupported_slo_exception(self, request, exception, *args, **kwargs): return HttpResponseRedirect(getattr(settings, "LOGOUT_REDIRECT_URL", "/")) -@method_decorator(csrf_exempt, name="dispatch") +@method_decorator([saml2_csp_update, csrf_exempt], name="dispatch") class LogoutView(SPConfigMixin, View): """SAML Logout Response endpoint @@ -738,7 +788,7 @@ def post(self, request, *args, **kwargs): request, request.POST, saml2.BINDING_HTTP_POST, *args, **kwargs ) - def do_logout_service(self, request, data, binding): + def do_logout_service(self, request, data, binding, *args, **kwargs): logger.debug("Logout service started") state, client = self.get_state_client(request) @@ -751,7 +801,9 @@ def do_logout_service(self, request, data, binding): ) except StatusError as e: response = None - logger.warning("Error logging out from remote provider: " + str(e)) + logger.warning( + f"Error logging out from remote provider: {e}", exc_info=True + ) state.sync() return finish_logout(request, response) @@ -797,10 +849,22 @@ 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", diff --git a/docs/source/_templates/pplnx_template/layout.html b/docs/source/_templates/pplnx_template/layout.html index 1bca49b8..834c10db 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 %} @@ -44,9 +44,11 @@ {% block mobile_nav %} + {% if logo %} - - + + + {% endif %} {{ project }} diff --git a/docs/source/contents/security.md b/docs/source/contents/security.md new file mode 100644 index 00000000..4e0f7c49 --- /dev/null +++ b/docs/source/contents/security.md @@ -0,0 +1,45 @@ +Introduction +============ + +Authentication and Authorization are quite security relevant topics on its own. +Make sure you understand SAML2 and its implications, specifically the +separation of duties between Service Provider (SP) and Identity Provider (IdP): +this library aims to support a Service Provider in getting authenticated with +with one or more Identity Provider. + +Communication between SP and IdP is routed via the Browser, eliminating the +need for direct communication between SP and IdP. However, for security the use +of cryptographic signatures (both while sending and receiving messages) must be +examined and the private keys in use must be kept closely guarded. + +Content Security Policy +======================= + +When using POST-Bindings, the Browser is presented with a small HTML-Form for +every redirect (both Login and Logout), which is sent using JavaScript and +sends the Data to the selected IdP. If your application uses technices such as +Content Security Policy, this might affect the calls. Since Version 1.9.0 +djangosaml2 will detect if django-csp is installed and update the Content +Security Policy accordingly. + +[Content Security Policy](https://content-security-policy.com/) is an important +HTTP-Extension to prevent User Input or other harmful sources from manipulating +application data. Usage is strongly advised, see +[OWASP Control](https://owasp.org/www-community/controls/Content_Security_Policy). + +To enable CSP with [django-csp](https://django-csp.readthedocs.io/), simply +follow their [installation](https://django-csp.readthedocs.io/en/latest/installation.html) +and [configuration](https://django-csp.readthedocs.io/en/latest/configuration.html) +guides: djangosaml2 will automatically blend in and update the headers for +POST-bindings, so you must not include exceptions for djangosaml2 in your +global configuration. + +Note that to enable autosubmit of post-bindings inline-javascript is used. To +allow execution of this autosubmit-code a nonce is included, which works in +default configuration but may not work if you modify `CSP_INCLUDE_NONCE_IN` +to exclude `script-src`. + +You can specify a custom CSP handler via the `SAML_CSP_HANDLER` setting and the +warning can be disabled by setting `SAML_CSP_HANDLER=''`. See the +[djangosaml2](https://djangosaml2.readthedocs.io/) documentation for more +information. diff --git a/docs/source/contents/setup.rst b/docs/source/contents/setup.rst index 89f9d295..384979de 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. @@ -118,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 = [] @@ -134,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 `_ +will also be resolved. Turning off strict validation will prevent this from happening. + Preferred sso binding ===================== @@ -202,13 +222,13 @@ see AARC Blueprint specs `here @@ -268,6 +288,28 @@ djangosaml2 provides a hook 'is_authorized' for the SP to store assertion IDs an cache_storage.set(assertion_id, 'True', ex=time_delta) return True +CSP Configuration +================= +By default djangosaml2 will use `django-csp `_ +to configure CSP if available otherwise a warning will be logged. + +The warning can be disabled by setting:: + + SAML_CSP_HANDLER = '' + +A custom handler can similary be specified:: + + # Django settings + SAML_CSP_HANDLER = 'myapp.utils.csp_handler' + + # myapp/utils.py + def csp_handler(response): + response.headers['Content-Security-Policy'] = ... + return response + +A value of `None` is the default and will use `django-csp `_ if available. + + Users, attributes and account linking ------------------------------------- diff --git a/docs/source/index.rst b/docs/source/index.rst index df441c40..4868dcda 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -41,3 +41,9 @@ under the `Apache 2.0 `_. :caption: FAQ contents/faq.md + +.. toctree:: + :maxdepth: 2 + :caption: Security considerations + + contents/security.md \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 5d906c1e..28b8614c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.black] force-exclude = '''/(migrations)/''' -target-version = ["py36"] +target-version = ["py39"] [tool.isort] src_paths = ["djangosaml2", "tests"] diff --git a/setup.py b/setup.py index 7576e96c..6999dd87 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ def read(*rnames): setup( name="djangosaml2", - version="1.5.6", + version="1.11.1", description="pysaml2 integration for Django", long_description=read("README.md"), long_description_content_type="text/markdown", @@ -35,17 +35,18 @@ def read(*rnames): "Development Status :: 5 - Production/Stable", "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", + "Framework :: Django :: 5.1", "Intended Audience :: Developers", "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", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "Topic :: Internet :: WWW/HTTP", "Topic :: Internet :: WWW/HTTP :: WSGI", "Topic :: Security", @@ -61,5 +62,6 @@ 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>=4.2", "pysaml2>=6.5.1"], + python_requires=">=3.9", ) diff --git a/tests/settings.py b/tests/settings.py index 58a2e28c..ca741c77 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -104,8 +104,6 @@ USE_I18N = True -USE_L10N = True - USE_TZ = True diff --git a/tests/testprofiles/tests.py b/tests/testprofiles/tests.py index 9bb39e72..1904ba3b 100644 --- a/tests/testprofiles/tests.py +++ b/tests/testprofiles/tests.py @@ -16,12 +16,14 @@ from django.conf import settings from django.core.exceptions import ImproperlyConfigured -from django.test import TestCase, override_settings +from django.test import Client, TestCase, override_settings +from django.urls import reverse from django.contrib.auth import get_user_model from django.contrib.auth.models import User as DjangoUserModel from djangosaml2.backends import Saml2Backend, get_saml_user_model, set_attribute +from djangosaml2.utils import get_csp_handler from testprofiles.models import TestUser @@ -294,7 +296,7 @@ def test_get_or_create_user_duplicates(self): self.assertFalse(created) self.assertIn( "ERROR:djangosaml2:Multiple users match, model: testprofiles.testuser, lookup: {'age': ''}", - logs.output, + logs.output[0], ) def test_get_or_create_user_no_create(self): @@ -314,7 +316,7 @@ def test_get_or_create_user_no_create(self): self.assertFalse(created) self.assertIn( "ERROR:djangosaml2:The user does not exist, model: testprofiles.testuser, lookup: {'username': 'paul'}", - logs.output, + logs.output[0], ) def test_get_or_create_user_create(self): @@ -334,7 +336,7 @@ def test_get_or_create_user_create(self): self.assertTrue(created) self.assertIn( f"DEBUG:djangosaml2:New user created: {user}", - logs.output, + logs.output[0], ) def test_deprecations(self): @@ -559,3 +561,36 @@ def test_user_cleaned_main_attribute(self): self.user.refresh_from_db() self.assertEqual(user.username, "john") + + +class CSPHandlerTests(TestCase): + def test_get_csp_handler_none(self): + get_csp_handler.cache_clear() + with override_settings(SAML_CSP_HANDLER=None): + csp_handler = get_csp_handler() + self.assertIn( + csp_handler.__module__, ["csp.decorators", "djangosaml2.utils"] + ) + self.assertIn(csp_handler.__name__, ["decorator", "empty_view_decorator"]) + + def test_get_csp_handler_empty(self): + get_csp_handler.cache_clear() + with override_settings(SAML_CSP_HANDLER=""): + csp_handler = get_csp_handler() + self.assertEqual(csp_handler.__name__, "empty_view_decorator") + + def test_get_csp_handler_specified(self): + get_csp_handler.cache_clear() + with override_settings(SAML_CSP_HANDLER="testprofiles.utils.csp_handler"): + client = Client() + response = client.get(reverse("saml2_login")) + self.assertIn("Content-Security-Policy", response.headers) + self.assertEqual( + response.headers["Content-Security-Policy"], "testing CSP value" + ) + + def test_get_csp_handler_specified_missing(self): + get_csp_handler.cache_clear() + with override_settings(SAML_CSP_HANDLER="does.not.exist"): + with self.assertRaises(ImportError): + get_csp_handler() diff --git a/tests/testprofiles/utils.py b/tests/testprofiles/utils.py new file mode 100644 index 00000000..34421df5 --- /dev/null +++ b/tests/testprofiles/utils.py @@ -0,0 +1,3 @@ +def csp_handler(response): + response.headers["Content-Security-Policy"] = "testing CSP value" + return response diff --git a/tox.ini b/tox.ini index 5507db5a..ccfc3f7d 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.9,3.10,3.11,3.12,3.13}-django{4.2,5.0,5.1} [testenv] commands = @@ -8,9 +8,9 @@ commands = python tests/run_tests.py deps = - django3.2: django~=3.2 - django4.0: django~=4.0 - django4.1: django==4.1b1 + django4.2: django~=4.2 + django5.0: django~=5.0 + django5.1: django~=5.1 djangomaster: https://github.com/django/django/archive/master.tar.gz .