providers/saml: transition to dataclass from dict, cleanup unused templates, add missing autosubmit_form

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

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:

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 {
+        return SAMLResponseParams(
-            "acs_url": self._request_params["ACS_URL"],
+            acs_url=self._request_params["ACS_URL"],
-            "saml_response": self._saml_response,
+            saml_response=self._saml_response,
-            "relay_state": self._relay_state,
+            relay_state=self._relay_state,
-            "autosubmit": self._remote.application.skip_authorization,
+        )
-        }
 
     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 = ""

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

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 %}
+<header class="login-pf-header">
+    <h1>{% trans 'Redirecting...' %}</h1>
+</header>
+<form method="POST" action="{{ url }}">
+    {% csrf_token %}
+    {% for key, value in attrs.items %}
+    <input type="hidden" name="{{ key }}" value="{{ value }}">
+    {% endfor %}
+    <div class="login-group">
+        <h3>
+            {% trans "Redirecting..." %}
+        </h3>
+        <p>
+            {% blocktrans with user=user %}
+            You are logged in as {{ user }}.
+            {% endblocktrans %}
+            <a href="{% url 'passbook_core:auth-logout' %}">{% trans 'Not you?' %}</a>
+        </p>
+        <input class="btn btn-primary btn-block btn-lg" type="submit" value="{% trans 'Continue' %}" />
+    </div>
+</form>
+{% endblock %}
+
+{% block scripts %}
+{{ block.super }}
+<script>
+    $('form').submit();
+</script>
+{% endblock %}

passbook/providers/saml/templates/saml/idp/invalid_user.html

@@ -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 %}

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 %}
+<p>
+    {% trans "You have successfully logged out of the Identity Provider." %}
+</p>
 {% endblock %}

passbook/providers/saml/templates/saml/idp/login.html

@@ -11,15 +11,15 @@
 <header class="login-pf-header">
     <h1>{% trans 'Authorize Application' %}</h1>
 </header>
-<form method="POST" action="{{ acs_url }}">
+<form method="POST" action="{{ saml_params.acs_url }}">
     {% csrf_token %}
-    <input type="hidden" name="ACSUrl" value="{{ acs_url }}">
+    <input type="hidden" name="ACSUrl" value="{{ saml_params.acs_url }}">
-    <input type="hidden" name="RelayState" value="{{ relay_state }}" />
+    <input type="hidden" name="RelayState" value="{{ saml_params.relay_state }}" />
-    <input type="hidden" name="SAMLResponse" value="{{ saml_response }}" />
+    <input type="hidden" name="SAMLResponse" value="{{ saml_params.saml_response }}" />
     <div class="login-group">
         <h3>
-            {% blocktrans with remote=remote.application.name %}
+            {% blocktrans with provider=provider.application.name %}
-            You're about to sign into {{ remote }}
+            You're about to sign into {{ provider }}
             {% endblocktrans %}
         </h3>
         <p>

passbook/providers/saml/templates/saml/idp/settings.html

@@ -1,47 +0,0 @@
-{% extends "_admin/module_default.html" %}
-
-{% load i18n %}
-{% load utils %}
-
-{% block title %}
-{% title "Overview" %}
-{% endblock %}
-
-{% block module_content %}
-<h2><clr-icon shape="application" size="32"></clr-icon>{% trans 'SAML2 IDP' %}</h2>
-<div class="row">
-  <div class="col-md-12">
-    <div class="card">
-      <div class="card-header">
-        <h2><clr-icon shape="settings" size="32"></clr-icon>{% trans 'Settings' %}</h2>
-      </div>
-      <form role="form" method="POST">
-        <div class="card-block">
-          {% include 'partials/form.html' with form=form %}
-        </div>
-        <div class="card-footer">
-          <button type="submit" class="btn btn-primary">{% trans 'Update' %}</button>
-        </div>
-      </form>
-    </div>
-  </div>
-</div>
-<div class="row">
-  <div class="col-md-12">
-    <div class="card">
-      <div class="card-header">
-        <h2><clr-icon shape="bank" size="32"></clr-icon>{% trans 'Metadata' %}</h2>
-      </div>
-      <div class="card-block">
-        <p>{% trans 'Cert Fingerprint (SHA1):' %} <pre>{{ fingerprint }}</pre></p>
-        <section class="form-block">
-          <pre lang="xml" >{{ metadata }}</pre>
-        </section>
-      </div>
-      <div class="card-footer">
-        <a href="{% url 'passbook_providers_saml:saml-metadata' %}" class="btn btn-primary"><clr-icon shape="download"></clr-icon>{% trans 'Download Metadata' %}</a>
-      </div>
-    </div>
-  </div>
-</div>
-{% endblock %}

passbook/providers/saml/urls.py

@@ -4,14 +4,17 @@ from django.urls import path
 from passbook.providers.saml import views
 
 urlpatterns = [
-    path(
+    # This view is used to initiate a Login-flow from the IDP
-        "<slug:application>/login/", views.LoginBeginView.as_view(), name="saml-login"
-    ),
     path(
         "<slug:application>/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(
+        "<slug:application>/login/", views.LoginProcessView.as_view(), name="saml-login"
+    ),
     path(
         "<slug:application>/login/process/",
         views.LoginProcessView.as_view(),

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()
+        # Otherwise we generate the IdP initiated session
-            # 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"],
-            )
         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)
+            LOGGER.error(exc)
-        return HttpResponseBadRequest()
+            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 <a href='{did_you_mean_link}'>here</a>?"
+            )
+            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
+        # we get here when skip_authorization is False, and after the user accepted
-            Event.new(
+        # the authorization form
-                EventAction.AUTHORIZE_APPLICATION,
+        self.provider.processor.can_handle(request)
-                authorized_application=self.provider.application,
+        saml_params = self.provider.processor.generate_response()
-                skipped_authorization=False,
+        return self.handle_redirect(saml_params, True)
-            ).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()
 
 
 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,
+            },
+        )

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