Skip to content

Commit fa6ea1b

Browse files
authored
Merge branch 'IdentityPython:master' into issue-391-add-csp
2 parents 93b7c09 + d815b5c commit fa6ea1b

File tree

11 files changed

+195
-64
lines changed

11 files changed

+195
-64
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: Publish Python distribution to PyPI
22
on:
33
release:
44
types:
5-
- created
5+
- published
66

77
jobs:
88
build-n-publish:

.github/workflows/python-package.yml

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,33 @@ jobs:
1515
runs-on: ubuntu-latest
1616
strategy:
1717
matrix:
18-
python-version: ["3.8", "3.9", "3.10"]
19-
django-version: ["3.2", "4.0", "4.1"]
18+
python-version: ["3.8", "3.9", "3.10", "3.11"]
19+
django-version: ["3.2", "4.1", "4.2", "5.0"]
2020
include:
21-
- python-version: "3.7"
21+
- python-version: "3.12"
22+
django-version: "4.2"
23+
- python-version: "3.12"
24+
django-version: "5.0"
25+
exclude:
26+
- python-version: "3.11"
2227
django-version: "3.2"
28+
- python-version: "3.8"
29+
django-version: "5.0"
30+
- python-version: "3.9"
31+
django-version: "5.0"
2332

2433
steps:
25-
- uses: actions/checkout@v2
34+
- uses: actions/checkout@v4
2635
- name: Set up Python ${{ matrix.python-version }}
27-
uses: actions/setup-python@v1
36+
uses: actions/setup-python@v4
2837
with:
2938
python-version: ${{ matrix.python-version }}
39+
allow-prereleases: true
3040
- name: Install dependencies and testing utilities
3141
run: |
3242
sudo apt-get update && sudo apt-get install xmlsec1
33-
python -m pip install --upgrade pip tox rstcheck setuptools codecov
43+
python -m pip install --upgrade pip
44+
python -m pip install --upgrade tox rstcheck setuptools codecov
3445
#- name: Readme check
3546
#if: ${{ matrix.python-version }} == 3.8 && ${{ matrix.django-version }} == "3.0"
3647
#run: rstcheck README.rst

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ djangosaml2
44
![CI build](https://github.com/peppelinux/djangosaml2/workflows/djangosaml2/badge.svg)
55
![pypi](https://img.shields.io/pypi/v/djangosaml2.svg)
66
[![Downloads](https://pepy.tech/badge/djangosaml2/month)](https://pepy.tech/project/djangosaml2)
7-
![Python version](https://img.shields.io/badge/license-Apache%202-blue.svg)
8-
![Django versions](https://img.shields.io/pypi/djversions/djangosaml2)
97
![Documentation Status](https://readthedocs.org/projects/djangosaml2/badge/?version=latest)
10-
![License](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)
8+
![License](https://img.shields.io/badge/license-Apache%202-blue.svg)
9+
![Python versions](https://img.shields.io/pypi/pyversions/djangosaml2)
10+
![Django versions](https://img.shields.io/pypi/djversions/djangosaml2)
1111

1212

1313
A Django application that builds a Fully Compliant SAML2 Service Provider on top of PySAML2 library.

djangosaml2/middleware.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ def process_response(self, request, response):
2626
session every time, save the changes and set a session cookie or delete
2727
the session cookie if the session has been emptied.
2828
"""
29+
SAMESITE = getattr(settings, "SAML_SESSION_COOKIE_SAMESITE", SAMESITE_NONE)
30+
2931
try:
3032
accessed = request.saml_session.accessed
3133
modified = request.saml_session.modified
@@ -39,7 +41,7 @@ def process_response(self, request, response):
3941
self.cookie_name,
4042
path=settings.SESSION_COOKIE_PATH,
4143
domain=settings.SESSION_COOKIE_DOMAIN,
42-
samesite=SAMESITE_NONE,
44+
samesite=SAMESITE,
4345
)
4446
patch_vary_headers(response, ("Cookie",))
4547
else:
@@ -74,6 +76,6 @@ def process_response(self, request, response):
7476
path=settings.SESSION_COOKIE_PATH,
7577
secure=settings.SESSION_COOKIE_SECURE or None,
7678
httponly=settings.SESSION_COOKIE_HTTPONLY or None,
77-
samesite=SAMESITE_NONE,
79+
samesite=SAMESITE,
7880
)
7981
return response

djangosaml2/tests/__init__.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,3 +1030,31 @@ def test_middleware_cookie_with_expiry(self):
10301030
self.assertIsNotNone(cookie["expires"])
10311031
self.assertNotEqual(cookie["expires"], "")
10321032
self.assertNotEqual(cookie["max-age"], "")
1033+
1034+
def test_middleware_cookie_samesite(self):
1035+
with override_settings(SAML_SESSION_COOKIE_SAMESITE="Lax"):
1036+
session = self.get_session()
1037+
session.save()
1038+
self.set_session_cookies(session)
1039+
1040+
config_loader_path = "djangosaml2.tests.test_config_loader_with_real_conf"
1041+
request = RequestFactory().get("/login/")
1042+
request.user = AnonymousUser()
1043+
request.session = session
1044+
middleware = SamlSessionMiddleware(dummy_get_response)
1045+
middleware.process_request(request)
1046+
1047+
saml_session_name = getattr(
1048+
settings, "SAML_SESSION_COOKIE_NAME", "saml_session"
1049+
)
1050+
getattr(request, saml_session_name).save()
1051+
1052+
response = views.LoginView.as_view(config_loader_path=config_loader_path)(
1053+
request
1054+
)
1055+
1056+
response = middleware.process_response(request, response)
1057+
1058+
cookie = response.cookies[saml_session_name]
1059+
1060+
self.assertEqual(cookie["samesite"], "Lax")

djangosaml2/utils.py

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

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

100101

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

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

115138

djangosaml2/views.py

Lines changed: 82 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import base64
1717
import logging
18+
from typing import Optional
1819
from urllib.parse import quote
1920

2021
from django.conf import settings
@@ -104,6 +105,19 @@ def _get_subject_id(session):
104105
return None
105106

106107

108+
def _get_next_path(request: HttpRequest) -> Optional[str]:
109+
if "next" in request.GET:
110+
next_path = request.GET["next"]
111+
elif "RelayState" in request.GET:
112+
next_path = request.GET["RelayState"]
113+
else:
114+
return None
115+
116+
next_path = validate_referral_url(request, next_path)
117+
118+
return next_path
119+
120+
107121
class SPConfigMixin:
108122
"""Mixin for some of the SAML views with re-usable methods."""
109123

@@ -154,20 +168,6 @@ class LoginView(SPConfigMixin, View):
154168
"djangosaml2/post_binding_form.html",
155169
)
156170

157-
def get_next_path(self, request: HttpRequest) -> str:
158-
"""Returns the path to put in the RelayState to redirect the user to after having logged in.
159-
If the user is already logged in (and if allowed), he will redirect to there immediately.
160-
"""
161-
162-
next_path = get_fallback_login_redirect_url()
163-
if "next" in request.GET:
164-
next_path = request.GET["next"]
165-
elif "RelayState" in request.GET:
166-
next_path = request.GET["RelayState"]
167-
168-
next_path = validate_referral_url(request, next_path)
169-
return next_path
170-
171171
def unknown_idp(self, request, idp):
172172
msg = f"Error: IdP EntityID {escape(idp)} was not found in metadata"
173173
logger.error(msg)
@@ -190,21 +190,25 @@ def load_sso_kwargs(self, sso_kwargs):
190190
def add_idp_hinting(self, http_response):
191191
return add_idp_hinting(self.request, http_response) or http_response
192192

193-
def get(self, request, *args, **kwargs):
194-
logger.debug("Login process started")
195-
next_path = self.get_next_path(request)
196-
197-
# if the user is already authenticated that maybe because of two reasons:
193+
def should_prevent_auth(self, request) -> bool:
194+
# If the user is already authenticated that maybe because of two reasons:
198195
# A) He has this URL in two browser windows and in the other one he
199196
# has already initiated the authenticated session.
200197
# B) He comes from a view that (incorrectly) send him here because
201198
# he does not have enough permissions. That view should have shown
202199
# an authorization error in the first place.
203-
# We can only make one thing here and that is configurable with the
204-
# SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN setting. If that setting
205-
# is True (default value) we will redirect him to the next_path path.
206-
# Otherwise, we will show an (configurable) authorization error.
207-
if request.user.is_authenticated:
200+
return request.user.is_authenticated
201+
202+
def get(self, request, *args, **kwargs):
203+
logger.debug("Login process started")
204+
next_path = _get_next_path(request)
205+
if next_path is None:
206+
next_path = get_fallback_login_redirect_url()
207+
208+
if self.should_prevent_auth(request):
209+
# If the SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN setting is True
210+
# (default value), redirect to the next_path. Otherwise, show a
211+
# configurable authorization error.
208212
if get_custom_setting("SAML_IGNORE_AUTHENTICATED_USERS_ON_LOGIN", True):
209213
return HttpResponseRedirect(next_path)
210214
logger.debug("User is already logged in")
@@ -566,7 +570,48 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
566570
if callable(create_unknown_user):
567571
create_unknown_user = create_unknown_user()
568572

573+
try:
574+
user = self.authenticate_user(
575+
request,
576+
session_info,
577+
attribute_mapping,
578+
create_unknown_user,
579+
assertion_info
580+
)
581+
except PermissionDenied as e:
582+
return self.handle_acs_failure(
583+
request,
584+
exception=e,
585+
session_info=session_info,
586+
)
587+
588+
relay_state = self.build_relay_state()
589+
custom_redirect_url = self.custom_redirect(user, relay_state, session_info)
590+
if custom_redirect_url:
591+
return HttpResponseRedirect(custom_redirect_url)
592+
593+
relay_state = validate_referral_url(request, relay_state)
594+
if not relay_state:
595+
logger.debug(
596+
"RelayState is not a valid URL, redirecting to fallback: %s",
597+
relay_state
598+
)
599+
return HttpResponseRedirect(get_fallback_login_redirect_url())
600+
601+
logger.debug("Redirecting to the RelayState: %s", relay_state)
602+
return HttpResponseRedirect(relay_state)
603+
604+
def authenticate_user(
605+
self,
606+
request,
607+
session_info,
608+
attribute_mapping,
609+
create_unknown_user,
610+
assertion_info
611+
):
612+
"""Calls Django's authenticate method after the SAML response is verified"""
569613
logger.debug("Trying to authenticate the user. Session info: %s", session_info)
614+
570615
user = auth.authenticate(
571616
request=request,
572617
session_info=session_info,
@@ -579,11 +624,7 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
579624
"Could not authenticate user received in SAML Assertion. Session info: %s",
580625
session_info,
581626
)
582-
return self.handle_acs_failure(
583-
request,
584-
exception=PermissionDenied("No user could be authenticated."),
585-
session_info=session_info,
586-
)
627+
raise PermissionDenied("No user could be authenticated.")
587628

588629
auth.login(self.request, user)
589630
_set_subject_id(request.saml_session, session_info["name_id"])
@@ -592,13 +633,7 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
592633
self.post_login_hook(request, user, session_info)
593634
self.customize_session(user, session_info)
594635

595-
relay_state = self.build_relay_state()
596-
custom_redirect_url = self.custom_redirect(user, relay_state, session_info)
597-
if custom_redirect_url:
598-
return HttpResponseRedirect(custom_redirect_url)
599-
relay_state = validate_referral_url(request, relay_state)
600-
logger.debug("Redirecting to the RelayState: %s", relay_state)
601-
return HttpResponseRedirect(relay_state)
636+
return user
602637

603638
def post_login_hook(
604639
self, request: HttpRequest, user: settings.AUTH_USER_MODEL, session_info: dict
@@ -814,10 +849,19 @@ def finish_logout(request, response):
814849

815850
auth.logout(request)
816851

817-
if settings.LOGOUT_REDIRECT_URL is not None:
818-
return HttpResponseRedirect(resolve_url(settings.LOGOUT_REDIRECT_URL))
852+
next_path = _get_next_path(request)
853+
if next_path is not None:
854+
logger.debug("Redirecting to the RelayState: %s", next_path)
855+
return HttpResponseRedirect(next_path)
856+
elif settings.LOGOUT_REDIRECT_URL is not None:
857+
fallback_url = resolve_url(settings.LOGOUT_REDIRECT_URL)
858+
logger.debug("No valid RelayState found; Redirecting to "
859+
"LOGOUT_REDIRECT_URL")
860+
return HttpResponseRedirect(fallback_url)
819861
else:
820862
current_site = get_current_site(request)
863+
logger.debug("No valid RelayState or LOGOUT_REDIRECT_URL found, "
864+
"rendering fallback template.")
821865
return render(
822866
request,
823867
"registration/logged_out.html",

docs/source/_templates/pplnx_template/layout.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
{# Not strictly valid HTML, but it's the only way to display/scale
1414
it properly, without weird scripting or heaps of work
1515
#}
16-
<a href="{{ pathto(master_doc) }}" id="logo_main"><img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="Logo" /></a>
16+
<a href="{{ pathto(master_doc) }}" id="logo_main"><img src="{{ pathto('_static/logo.jpg', 1) }}" class="logo" alt="Logo" /></a>
1717
{% endif %}
1818

1919
{% if logo and theme_logo_only %}
@@ -45,7 +45,7 @@
4545
{% block mobile_nav %}
4646
<i data-toggle="wy-nav-top" class="fa fa-bars"></i>
4747
<a href="{{ pathto(master_doc) }}">
48-
<img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="Logo"/>
48+
<img src="{{ pathto('_static/logo.jpg', 1) }}" class="logo" alt="Logo"/>
4949
</a>
5050
<a href="{{ pathto(master_doc) }}">
5151
{{ project }}

0 commit comments

Comments
 (0)