diff --git a/passbook/providers/saml/exceptions.py b/passbook/providers/saml/exceptions.py index 98fea6dec..a78218ca6 100644 --- a/passbook/providers/saml/exceptions.py +++ b/passbook/providers/saml/exceptions.py @@ -3,7 +3,3 @@ class CannotHandleAssertion(Exception): """This processor does not handle this assertion.""" - - -class UserNotAuthorized(Exception): - """User not authorized for SAML 2.0 authentication.""" diff --git a/passbook/providers/saml/models.py b/passbook/providers/saml/models.py index d69c2d189..4a1668b77 100644 --- a/passbook/providers/saml/models.py +++ b/passbook/providers/saml/models.py @@ -82,7 +82,7 @@ class SAMLProvider(Provider): self._meta.get_field("processor_path").choices = get_provider_choices() @property - def processor(self): + def processor(self) -> Processor: """Return selected processor as instance""" if not self._processor: try: diff --git a/passbook/providers/saml/processors/base.py b/passbook/providers/saml/processors/base.py index 13d51b4ba..c407d44d0 100644 --- a/passbook/providers/saml/processors/base.py +++ b/passbook/providers/saml/processors/base.py @@ -7,6 +7,7 @@ from structlog import get_logger from passbook.core.exceptions import PropertyMappingExpressionException from passbook.providers.saml.exceptions import CannotHandleAssertion +from passbook.providers.saml.processors.types import SAMLResponseParams from passbook.providers.saml.utils import get_random_id from passbook.providers.saml.utils.encoding import decode_base64_and_inflate, nice64 from passbook.providers.saml.utils.time import get_time_string, timedelta_from_string @@ -133,14 +134,13 @@ class Processor: self._response_params, saml_provider=self._remote, assertion_id=assertion_id ) - def _get_django_response_params(self) -> Dict[str, str]: + def _get_saml_response_params(self) -> SAMLResponseParams: """Returns a dictionary of parameters for the response template.""" - return { - "acs_url": self._request_params["ACS_URL"], - "saml_response": self._saml_response, - "relay_state": self._relay_state, - "autosubmit": self._remote.application.skip_authorization, - } + return SAMLResponseParams( + acs_url=self._request_params["ACS_URL"], + saml_response=self._saml_response, + relay_state=self._relay_state, + ) def _decode_and_parse_request(self): """Parses various parameters from _request_xml into _request_params.""" @@ -183,7 +183,7 @@ class Processor: # Read the request. try: self._extract_saml_request() - except Exception as exc: + except KeyError as exc: raise CannotHandleAssertion( f"can't find SAML request in user session: {exc}" ) from exc @@ -196,7 +196,7 @@ class Processor: self._validate_request() return True - def generate_response(self) -> Dict[str, str]: + def generate_response(self) -> SAMLResponseParams: """Processes request and returns template variables suitable for a response.""" # Build the assertion and response. # Only call can_handle if SP initiated Request, otherwise we have no Request @@ -210,9 +210,9 @@ class Processor: self._encode_response() # Return proper template params. - return self._get_django_response_params() + return self._get_saml_response_params() - def init_deep_link(self, request: HttpRequest, url: str): + def init_deep_link(self, request: HttpRequest): """Initialize this Processor to make an IdP-initiated call to the SP's deep-linked URL.""" self._http_request = request @@ -227,4 +227,4 @@ class Processor: "DESTINATION": "", "PROVIDER_NAME": "", } - self._relay_state = url + self._relay_state = "" diff --git a/passbook/providers/saml/processors/types.py b/passbook/providers/saml/processors/types.py new file mode 100644 index 000000000..a283ba06e --- /dev/null +++ b/passbook/providers/saml/processors/types.py @@ -0,0 +1,11 @@ +"""passbook saml provider types""" +from dataclasses import dataclass + + +@dataclass +class SAMLResponseParams: + """Class to keep track of SAML Response Parameters""" + + acs_url: str + saml_response: str + relay_state: str diff --git a/passbook/providers/saml/templates/saml/idp/autosubmit_form.html b/passbook/providers/saml/templates/saml/idp/autosubmit_form.html new file mode 100644 index 000000000..99166d5e9 --- /dev/null +++ b/passbook/providers/saml/templates/saml/idp/autosubmit_form.html @@ -0,0 +1,39 @@ +{% extends "login/base.html" %} + +{% load utils %} +{% load i18n %} + +{% block title %} +{% title 'Redirecting...' %} +{% endblock %} + +{% block card %} +
+

{% trans 'Redirecting...' %}

+
+
+ {% csrf_token %} + {% for key, value in attrs.items %} + + {% endfor %} +
+

+ {% trans "Redirecting..." %} +

+

+ {% blocktrans with user=user %} + You are logged in as {{ user }}. + {% endblocktrans %} + {% trans 'Not you?' %} +

+ +
+
+{% endblock %} + +{% block scripts %} +{{ block.super }} + +{% endblock %} diff --git a/passbook/providers/saml/templates/saml/idp/invalid_user.html b/passbook/providers/saml/templates/saml/idp/invalid_user.html deleted file mode 100644 index 16ba1f4e6..000000000 --- a/passbook/providers/saml/templates/saml/idp/invalid_user.html +++ /dev/null @@ -1,5 +0,0 @@ -{% extends "saml/idp/base.html" %} -{% load i18n %} -{% block content %} -{% trans "You have logged in, but your user account is not enabled for SAML 2.0." %} -{% endblock %} diff --git a/passbook/providers/saml/templates/saml/idp/logged_out.html b/passbook/providers/saml/templates/saml/idp/logged_out.html index 2d973d442..4b2a98b74 100644 --- a/passbook/providers/saml/templates/saml/idp/logged_out.html +++ b/passbook/providers/saml/templates/saml/idp/logged_out.html @@ -1,5 +1,9 @@ -{% extends "saml/idp/base.html" %} +{% extends "login/base.html" %} + {% load i18n %} -{% block content %} -{% trans "You have successfully logged out of the Identity Provider." %} + +{% block card %} +

+ {% trans "You have successfully logged out of the Identity Provider." %} +

{% endblock %} diff --git a/passbook/providers/saml/templates/saml/idp/login.html b/passbook/providers/saml/templates/saml/idp/login.html index 13fc082b3..6c0f5e80e 100644 --- a/passbook/providers/saml/templates/saml/idp/login.html +++ b/passbook/providers/saml/templates/saml/idp/login.html @@ -11,15 +11,15 @@

{% trans 'Authorize Application' %}

-
+ {% csrf_token %} - - - + + +

- {% blocktrans with remote=remote.application.name %} - You're about to sign into {{ remote }} + {% blocktrans with provider=provider.application.name %} + You're about to sign into {{ provider }} {% endblocktrans %}

diff --git a/passbook/providers/saml/templates/saml/idp/settings.html b/passbook/providers/saml/templates/saml/idp/settings.html deleted file mode 100644 index 336df4e9d..000000000 --- a/passbook/providers/saml/templates/saml/idp/settings.html +++ /dev/null @@ -1,47 +0,0 @@ -{% extends "_admin/module_default.html" %} - -{% load i18n %} -{% load utils %} - -{% block title %} -{% title "Overview" %} -{% endblock %} - -{% block module_content %} -

{% trans 'SAML2 IDP' %}

-
-
-
-
-

{% trans 'Settings' %}

-
- -
- {% include 'partials/form.html' with form=form %} -
- - -
-
-
-
-
-
-
-

{% trans 'Metadata' %}

-
-
-

{% trans 'Cert Fingerprint (SHA1):' %}

{{ fingerprint }}

-
-
{{ metadata }}
-
-
- -
-
-
-{% endblock %} diff --git a/passbook/providers/saml/urls.py b/passbook/providers/saml/urls.py index a8777ba79..c48edc1df 100644 --- a/passbook/providers/saml/urls.py +++ b/passbook/providers/saml/urls.py @@ -4,14 +4,17 @@ from django.urls import path from passbook.providers.saml import views urlpatterns = [ - path( - "/login/", views.LoginBeginView.as_view(), name="saml-login" - ), + # This view is used to initiate a Login-flow from the IDP path( "/login/initiate/", views.InitiateLoginView.as_view(), name="saml-login-initiate", ), + # This view is the endpoint a SP would redirect to, and saves data into the session + # this is required as the process view which it redirects to might have to login first. + path( + "/login/", views.LoginProcessView.as_view(), name="saml-login" + ), path( "/login/process/", views.LoginProcessView.as_view(), diff --git a/passbook/providers/saml/views.py b/passbook/providers/saml/views.py index 5add8e22f..17f2a2f15 100644 --- a/passbook/providers/saml/views.py +++ b/passbook/providers/saml/views.py @@ -5,10 +5,11 @@ from django.contrib.auth import logout from django.contrib.auth.mixins import AccessMixin from django.core.exceptions import ValidationError from django.core.validators import URLValidator -from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest +from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, render, reverse from django.utils.datastructures import MultiValueDictKeyError from django.utils.decorators import method_decorator +from django.utils.html import mark_safe from django.utils.translation import gettext as _ from django.views import View from django.views.decorators.csrf import csrf_exempt @@ -19,28 +20,16 @@ from passbook.audit.models import Event, EventAction from passbook.core.models import Application from passbook.lib.mixins import CSRFExemptMixin from passbook.lib.utils.template import render_to_string +from passbook.lib.views import bad_request_message from passbook.policies.engine import PolicyEngine from passbook.providers.saml import exceptions from passbook.providers.saml.models import SAMLProvider +from passbook.providers.saml.processors.types import SAMLResponseParams LOGGER = get_logger() URL_VALIDATOR = URLValidator(schemes=("http", "https")) -def _generate_response(request: HttpRequest, provider: SAMLProvider) -> HttpResponse: - """Generate a SAML response using processor_instance and return it in the proper Django - response.""" - try: - provider.processor.init_deep_link(request, "") - ctx = provider.processor.generate_response() - ctx["remote"] = provider - ctx["is_login"] = True - except exceptions.UserNotAuthorized: - return render(request, "saml/idp/invalid_user.html") - - return render(request, "saml/idp/login.html", ctx) - - class AccessRequiredView(AccessMixin, View): """Mixin class for Views using a provider instance""" @@ -97,7 +86,7 @@ class LoginBeginView(AccessRequiredView): try: request.session["SAMLRequest"] = source["SAMLRequest"] except (KeyError, MultiValueDictKeyError): - return HttpResponseBadRequest("the SAML request payload is missing") + return bad_request_message(request, "The SAML request payload is missing.") request.session["RelayState"] = source.get("RelayState", "") return redirect( @@ -108,73 +97,84 @@ class LoginBeginView(AccessRequiredView): ) -class RedirectToSPView(AccessRequiredView): - """Return autosubmit form""" - - def get( - self, request: HttpRequest, acs_url: str, saml_response: str, relay_state: str - ) -> HttpResponse: - """Return autosubmit form""" - return render( - request, - "core/autosubmit_form.html", - { - "url": acs_url, - "attrs": {"SAMLResponse": saml_response, "RelayState": relay_state}, - }, - ) - - class LoginProcessView(AccessRequiredView): """Processor-based login continuation. Presents a SAML 2.0 Assertion for POSTing back to the Service Provider.""" + def handle_redirect( + self, params: SAMLResponseParams, skipped_authorization: bool + ) -> HttpResponse: + """Handle direct redirect to SP""" + # Log Application Authorization + Event.new( + EventAction.AUTHORIZE_APPLICATION, + authorized_application=self.provider.application, + skipped_authorization=skipped_authorization, + ).from_http(self.request) + return render( + self.request, + "saml/idp/autosubmit_form.html", + { + "url": params.acs_url, + "attrs": { + "SAMLResponse": params.saml_response, + "RelayState": params.relay_state, + }, + }, + ) + # pylint: disable=unused-argument def get(self, request: HttpRequest, application: str) -> HttpResponse: """Handle get request, i.e. render form""" # User access gets checked in dispatch - if self.provider.application.skip_authorization: - ctx = self.provider.processor.generate_response() - # Log Application Authorization - Event.new( - EventAction.AUTHORIZE_APPLICATION, - authorized_application=self.provider.application, - skipped_authorization=True, - ).from_http(request) - return RedirectToSPView.as_view()( - request=request, - acs_url=ctx["acs_url"], - saml_response=ctx["saml_response"], - relay_state=ctx["relay_state"], - ) + + # Otherwise we generate the IdP initiated session try: - return _generate_response(request, self.provider) + # application.skip_authorization is set so we directly redirect the user + if self.provider.application.skip_authorization: + self.provider.processor.can_handle(request) + saml_params = self.provider.processor.generate_response() + return self.handle_redirect(saml_params, True) + + self.provider.processor.init_deep_link(request) + params = self.provider.processor.generate_response() + + return render( + request, + "saml/idp/login.html", + { + "saml_params": params, + "provider": self.provider, + # This is only needed to for the template to render correctly + "is_login": True, + }, + ) + except exceptions.CannotHandleAssertion as exc: - LOGGER.debug(exc) - return HttpResponseBadRequest() + LOGGER.error(exc) + did_you_mean_link = request.build_absolute_uri( + reverse( + "passbook_providers_saml:saml-login-initiate", + kwargs={"application": application}, + ) + ) + did_you_mean_message = ( + f" Did you mean to go here?" + ) + return bad_request_message( + request, mark_safe(str(exc) + did_you_mean_message) + ) # pylint: disable=unused-argument def post(self, request: HttpRequest, application: str) -> HttpResponse: """Handle post request, return back to ACS""" # User access gets checked in dispatch - if request.POST.get("ACSUrl", None): - # User accepted request - Event.new( - EventAction.AUTHORIZE_APPLICATION, - authorized_application=self.provider.application, - skipped_authorization=False, - ).from_http(request) - return RedirectToSPView.as_view()( - request=request, - acs_url=request.POST.get("ACSUrl"), - saml_response=request.POST.get("SAMLResponse"), - relay_state=request.POST.get("RelayState"), - ) - try: - return _generate_response(request, self.provider) - except exceptions.CannotHandleAssertion as exc: - LOGGER.debug(exc) - return HttpResponseBadRequest() + + # we get here when skip_authorization is False, and after the user accepted + # the authorization form + self.provider.processor.can_handle(request) + saml_params = self.provider.processor.generate_response() + return self.handle_redirect(saml_params, True) class LogoutView(CSRFExemptMixin, AccessRequiredView): @@ -254,9 +254,46 @@ class DescriptorDownloadView(AccessRequiredView): class InitiateLoginView(AccessRequiredView): """IdP-initiated Login""" + def handle_redirect( + self, params: SAMLResponseParams, skipped_authorization: bool + ) -> HttpResponse: + """Handle direct redirect to SP""" + # Log Application Authorization + Event.new( + EventAction.AUTHORIZE_APPLICATION, + authorized_application=self.provider.application, + skipped_authorization=skipped_authorization, + ).from_http(self.request) + return render( + self.request, + "saml/idp/autosubmit_form.html", + { + "url": params.acs_url, + "attrs": { + "SAMLResponse": params.saml_response, + "RelayState": params.relay_state, + }, + }, + ) + # pylint: disable=unused-argument def get(self, request: HttpRequest, application: str) -> HttpResponse: """Initiates an IdP-initiated link to a simple SP resource/target URL.""" - self.provider.processor.init_deep_link(request, "") self.provider.processor.is_idp_initiated = True - return _generate_response(request, self.provider) + self.provider.processor.init_deep_link(request) + params = self.provider.processor.generate_response() + + # IdP-initiated Login Flow + if self.provider.application.skip_authorization: + return self.handle_redirect(params, True) + + return render( + request, + "saml/idp/login.html", + { + "saml_params": params, + "provider": self.provider, + # This is only needed to for the template to render correctly + "is_login": True, + }, + ) diff --git a/passbook/root/settings.py b/passbook/root/settings.py index 39b066c19..d3222e3be 100644 --- a/passbook/root/settings.py +++ b/passbook/root/settings.py @@ -276,7 +276,7 @@ structlog.configure_once( structlog.stdlib.PositionalArgumentsFormatter(), structlog.processors.TimeStamper(), structlog.processors.StackInfoRenderer(), - # structlog.processors.format_exc_info, + structlog.processors.format_exc_info, structlog.stdlib.ProcessorFormatter.wrap_for_formatter, ], context_class=structlog.threadlocal.wrap_dict(dict),