new release: 0.8.11-beta

move can_handle calls to binding endpoints (/login/ and /login/initiate/), so that /login/authorize/ works either way, can clean up the session and audit

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.8.9-beta
+current_version = 0.8.11-beta
 tag = True
 commit = True
 parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)\-(?P<release>.*)

.github/workflows/release.yml

@@ -16,11 +16,11 @@ jobs:
       - name: Building Docker Image
         run: docker build
           --no-cache
-          -t beryju/passbook:0.8.9-beta
+          -t beryju/passbook:0.8.11-beta
           -t beryju/passbook:latest
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook:0.8.9-beta
+        run: docker push beryju/passbook:0.8.11-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook:latest
   build-gatekeeper:
@@ -37,11 +37,11 @@ jobs:
           cd gatekeeper
           docker build \
           --no-cache \
-          -t beryju/passbook-gatekeeper:0.8.9-beta \
+          -t beryju/passbook-gatekeeper:0.8.11-beta \
           -t beryju/passbook-gatekeeper:latest \
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-gatekeeper:0.8.9-beta
+        run: docker push beryju/passbook-gatekeeper:0.8.11-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook-gatekeeper:latest
   build-static:
@@ -66,11 +66,11 @@ jobs:
         run: docker build
           --no-cache
           --network=$(docker network ls | grep github | awk '{print $1}')
-          -t beryju/passbook-static:0.8.9-beta
+          -t beryju/passbook-static:0.8.11-beta
           -t beryju/passbook-static:latest
           -f static.Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-static:0.8.9-beta
+        run: docker push beryju/passbook-static:0.8.11-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook-static:latest
   test-release:

helm/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
-appVersion: "0.8.9-beta"
+appVersion: "0.8.11-beta"
 description: A Helm chart for passbook.
 name: passbook
-version: "0.8.9-beta"
+version: "0.8.11-beta"
 icon: https://git.beryju.org/uploads/-/system/project/avatar/108/logo.png

helm/templates/web-deployment.yaml

@@ -18,7 +18,7 @@ spec:
       labels:
         app.kubernetes.io/name: {{ include "passbook.name" . }}
         app.kubernetes.io/instance: {{ .Release.Name }}
-        passbook.io/component: web
+        k8s.passbook.io/component: web
     spec:
       volumes:
         - name: config-volume

helm/templates/web-service.yaml

@@ -18,4 +18,4 @@ spec:
   selector:
     app.kubernetes.io/name: {{ include "passbook.name" . }}
     app.kubernetes.io/instance: {{ .Release.Name }}
-    passbook.io/component: web
+    k8s.passbook.io/component: web

helm/templates/worker-deployment.yaml

@@ -18,7 +18,7 @@ spec:
       labels:
         app.kubernetes.io/name: {{ include "passbook.name" . }}
         app.kubernetes.io/instance: {{ .Release.Name }}
-        passbook.io/component: worker
+        k8s.passbook.io/component: worker
     spec:
       volumes:
         - name: config-volume

helm/values.yaml

@@ -2,7 +2,7 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 image:
-  tag: 0.8.9-beta
+  tag: 0.8.11-beta
 
 nameOverride: ""
 

passbook/__init__.py

@@ -1,2 +1,2 @@
 """passbook"""
-__version__ = "0.8.9-beta"
+__version__ = "0.8.11-beta"

passbook/audit/models.py

@@ -137,6 +137,7 @@ class Event(UUIDModel):
             action=self.action,
             context=self.context,
             client_ip=self.client_ip,
+            user=self.user,
         )
         return super().save(*args, **kwargs)
 

passbook/core/forms/applications.py

@@ -10,7 +10,8 @@ class ApplicationForm(forms.ModelForm):
     """Application Form"""
 
     provider = forms.ModelChoiceField(
-        queryset=Provider.objects.all().select_subclasses(), required=False
+        queryset=Provider.objects.all().order_by("pk").select_subclasses(),
+        required=False,
     )
 
     class Meta:

passbook/core/views/authentication.py

@@ -231,7 +231,6 @@ class PasswordResetView(View):
         login(request, nonce.user)
         nonce.delete()
         messages.success(
-            request,
-            _(("Temporarily authenticated with Nonce, " "please change your password")),
+            request, _(("Temporarily authenticated, please change your password")),
         )
         return redirect("passbook_core:user-change-password")

passbook/lib/logging.py

@@ -5,5 +5,5 @@ from os import getpid
 # pylint: disable=unused-argument
 def add_process_id(logger, method_name, event_dict):
     """Add the current process ID"""
-    event_dict["pdi"] = getpid()
+    event_dict["pid"] = getpid()
     return event_dict

passbook/lib/sentry.py

@@ -12,7 +12,7 @@ LOGGER = get_logger()
 
 
 class SentryIgnoredException(Exception):
-    """Base Class for all errors that are supressed, and not sent to sentry."""
+    """Base Class for all errors that are suppressed, and not sent to sentry."""
 
 
 def before_send(event, hint):

passbook/providers/oidc/templates/oidc_provider/error.html

@@ -0,0 +1,18 @@
+{% extends 'login/base.html' %}
+
+{% load static %}
+{% load i18n %}
+{% load utils %}
+
+{% block card_title %}
+{% trans error %}
+{% endblock %}
+
+{% block card %}
+<form>
+    <h3>{% trans description %}</h3>
+    {% if 'back' in request.GET %}
+    <a href="{% back %}" class="btn btn-primary btn-block btn-lg">{% trans 'Back' %}</a>
+    {% endif %}
+</form>
+{% endblock %}

passbook/providers/saml/processors/base.py

@@ -184,7 +184,7 @@ class Processor:
         try:
             self._extract_saml_request()
         except KeyError:
-            raise CannotHandleAssertion(f"Couldn't find SAML request in user session:")
+            raise CannotHandleAssertion(f"Couldn't find SAML request in user session")
 
         try:
             self._decode_and_parse_request()

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

@@ -4,11 +4,8 @@
 {% load i18n %}
 
 {% block card %}
-<form method="POST" class="pf-c-form" action="{{ saml_params.acs_url }}">
+<form method="POST" class="pf-c-form">
     {% csrf_token %}
-    <input type="hidden" name="ACSUrl" value="{{ saml_params.acs_url }}">
-    <input type="hidden" name="RelayState" value="{{ saml_params.relay_state }}" />
-    <input type="hidden" name="SAMLResponse" value="{{ saml_params.saml_response }}" />
     <div class="pf-c-form__group">
         <h3>
             {% blocktrans with provider=provider.application.name %}

passbook/providers/saml/urls.py

@@ -16,9 +16,9 @@ urlpatterns = [
         "<slug:application>/login/", views.LoginBeginView.as_view(), name="saml-login"
     ),
     path(
-        "<slug:application>/login/process/",
-        views.LoginProcessView.as_view(),
-        name="saml-login-process",
+        "<slug:application>/login/authorize/",
+        views.AuthorizeView.as_view(),
+        name="saml-login-authorize",
     ),
     path("<slug:application>/logout/", views.LogoutView.as_view(), name="saml-logout"),
     path(

passbook/providers/saml/views.py

@@ -21,12 +21,16 @@ from passbook.core.models import Application, Provider
 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.exceptions import CannotHandleAssertion
 from passbook.providers.saml.models import SAMLProvider
 from passbook.providers.saml.processors.types import SAMLResponseParams
 
 LOGGER = get_logger()
 URL_VALIDATOR = URLValidator(schemes=("http", "https"))
+SESSION_KEY_SAML_REQUEST = "SAMLRequest"
+SESSION_KEY_SAML_RESPONSE = "SAMLResponse"
+SESSION_KEY_RELAY_STATE = "RelayState"
+SESSION_KEY_PARAMS = "SAMLParams"
 
 
 class AccessRequiredView(AccessMixin, View):
@@ -50,14 +54,18 @@ class AccessRequiredView(AccessMixin, View):
 
     def _has_access(self) -> bool:
         """Check if user has access to application"""
-        LOGGER.debug(
-            "_has_access", user=self.request.user, app=self.provider.application
-        )
         policy_engine = PolicyEngine(
             self.provider.application.policies.all(), self.request.user, self.request
         )
         policy_engine.build()
-        return policy_engine.passing
+        passing = policy_engine.passing
+        LOGGER.debug(
+            "saml_has_access",
+            user=self.request.user,
+            app=self.provider.application,
+            passing=passing,
+        )
+        return passing
 
     def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         if not request.user.is_authenticated:
@@ -75,80 +83,29 @@ class LoginBeginView(AccessRequiredView):
     """Receives a SAML 2.0 AuthnRequest from a Service Provider and
     stores it in the session prior to enforcing login."""
 
-    @method_decorator(csrf_exempt)
-    def dispatch(self, request: HttpRequest, application: str) -> HttpResponse:
-        if request.method == "POST":
-            source = request.POST
-        else:
-            source = request.GET
-
+    def handler(self, source, application: str) -> HttpResponse:
+        """Handle SAML Request whether its a POST or a Redirect binding"""
         # Store these values now, because Django's login cycle won't preserve them.
         try:
-            request.session["SAMLRequest"] = source["SAMLRequest"]
+            self.request.session[SESSION_KEY_SAML_REQUEST] = source[
+                SESSION_KEY_SAML_REQUEST
+            ]
         except (KeyError, MultiValueDictKeyError):
-            return bad_request_message(request, "The SAML request payload is missing.")
-
-        request.session["RelayState"] = source.get("RelayState", "")
-        return redirect(
-            reverse(
-                "passbook_providers_saml:saml-login-process",
-                kwargs={"application": application},
+            return bad_request_message(
+                self.request, "The SAML request payload is missing."
             )
+
+        self.request.session[SESSION_KEY_RELAY_STATE] = source.get(
+            SESSION_KEY_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,
-                },
-            },
-        )
-
-    def get(self, request: HttpRequest, application: str) -> HttpResponse:
-        """Handle get request, i.e. render form"""
-        # User access gets checked in dispatch
-
-        # Otherwise we generate the IdP initiated session
         try:
-            # application.skip_authorization is set so we directly redirect the user
-            if self.provider.application.skip_authorization:
-                return self.post(request, application)
-
-            self.provider.processor.init_deep_link(request)
+            self.provider.processor.can_handle(self.request)
             params = self.provider.processor.generate_response()
-
-            return render(
-                request,
-                "saml/idp/login.html",
-                {
-                    "saml_params": params,
-                    "provider": self.provider,
-                    "title": "Authorize Application",
-                },
-            )
-
-        except exceptions.CannotHandleAssertion as exc:
-            LOGGER.error(exc)
-            did_you_mean_link = request.build_absolute_uri(
+            self.request.session[SESSION_KEY_PARAMS] = params
+        except CannotHandleAssertion as exc:
+            LOGGER.info(exc)
+            did_you_mean_link = self.request.build_absolute_uri(
                 reverse(
                     "passbook_providers_saml:saml-login-initiate",
                     kwargs={"application": application},
@@ -158,19 +115,97 @@ class LoginProcessView(AccessRequiredView):
                 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)
+                self.request, mark_safe(str(exc) + did_you_mean_message)
             )
 
+        return redirect(
+            reverse(
+                "passbook_providers_saml:saml-login-authorize",
+                kwargs={"application": application},
+            )
+        )
+
+    @method_decorator(csrf_exempt)
+    def get(self, request: HttpRequest, application: str) -> HttpResponse:
+        """Handle REDIRECT bindings"""
+        return self.handler(request.GET, application)
+
+    @method_decorator(csrf_exempt)
+    def post(self, request: HttpRequest, application: str) -> HttpResponse:
+        """Handle POST Bindings"""
+        return self.handler(request.POST, application)
+
+
+class InitiateLoginView(AccessRequiredView):
+    """IdP-initiated Login"""
+
+    def get(self, request: HttpRequest, application: str) -> HttpResponse:
+        """Initiates an IdP-initiated link to a simple SP resource/target URL."""
+        self.provider.processor.is_idp_initiated = True
+        self.provider.processor.init_deep_link(request)
+        params = self.provider.processor.generate_response()
+        request.session[SESSION_KEY_PARAMS] = params
+        return redirect(
+            reverse(
+                "passbook_providers_saml:saml-login-authorize",
+                kwargs={"application": application},
+            )
+        )
+
+
+class AuthorizeView(AccessRequiredView):
+    """Ask the user for authorization to continue to the SP.
+    Presents a SAML 2.0 Assertion for POSTing back to the Service Provider."""
+
+    def get(self, request: HttpRequest, application: str) -> HttpResponse:
+        """Handle get request, i.e. render form"""
+        # User access gets checked in dispatch
+
+        # Otherwise we generate the IdP initiated session
+        try:
+            # application.skip_authorization is set so we directly redirect the user
+            if self.provider.application.skip_authorization:
+                LOGGER.debug("skipping authz", application=self.provider.application)
+                return self.post(request, application)
+
+            return render(
+                request,
+                "saml/idp/login.html",
+                {"provider": self.provider, "title": "Authorize Application",},
+            )
+
+        except KeyError:
+            return bad_request_message(request, "Missing SAML Payload")
+
     # 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
 
-        # we get here when skip_authorization is False, and after the user accepted
+        # we get here when skip_authorization is True, 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)
+        # Log Application Authorization
+        Event.new(
+            EventAction.AUTHORIZE_APPLICATION,
+            authorized_application=self.provider.application,
+            skipped_authorization=self.provider.application.skip_authorization,
+        ).from_http(self.request)
+        self.request.session.pop(SESSION_KEY_SAML_REQUEST, None)
+        self.request.session.pop(SESSION_KEY_SAML_RESPONSE, None)
+        self.request.session.pop(SESSION_KEY_RELAY_STATE, None)
+        response: SAMLResponseParams = self.request.session.pop(SESSION_KEY_PARAMS)
+        return render(
+            self.request,
+            "saml/idp/autosubmit_form.html",
+            {
+                "url": response.acs_url,
+                "attrs": {
+                    "ACSUrl": response.acs_url,
+                    SESSION_KEY_SAML_RESPONSE: response.saml_response,
+                    SESSION_KEY_RELAY_STATE: response.relay_state,
+                },
+            },
+        )
 
 
 @method_decorator(csrf_exempt, name="dispatch")
@@ -204,7 +239,9 @@ class SLOLogout(AccessRequiredView):
     # pylint: disable=unused-argument
     def post(self, request: HttpRequest, application: str) -> HttpResponse:
         """Perform logout"""
-        request.session["SAMLRequest"] = request.POST["SAMLRequest"]
+        request.session[SESSION_KEY_SAML_REQUEST] = request.POST[
+            SESSION_KEY_SAML_REQUEST
+        ]
         # TODO: Parse SAML LogoutRequest from POST data, similar to login_process().
         # TODO: Modify the base processor to handle logouts?
         # TODO: Combine this with login_process(), since they are so very similar?
@@ -259,54 +296,7 @@ class DescriptorDownloadView(AccessRequiredView):
             )
         else:
             response = HttpResponse(metadata, content_type="application/xml")
-            response["Content-Disposition"] = (
-                'attachment; filename="' '%s_passbook_meta.xml"' % self.provider.name
-            )
+            response[
+                "Content-Disposition"
+            ] = f'attachment; filename="{self.provider.name}_passbook_meta.xml"'
             return response
-
-
-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.is_idp_initiated = True
-        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,
-                "title": "Authorize Application",
-            },
-        )

passbook/root/settings.py

@@ -317,7 +317,7 @@ LOGGING = {
     },
     "handlers": {
         "console": {
-            "level": DEBUG,
+            "level": "DEBUG",
             "class": "logging.StreamHandler",
             "formatter": "colored" if DEBUG else "plain",
         },
@@ -325,6 +325,7 @@ LOGGING = {
     "loggers": {},
 }
 _LOGGING_HANDLER_MAP = {
+    "": "DEBUG",
     "passbook": "DEBUG",
     "django": "WARNING",
     "celery": "WARNING",
@@ -337,7 +338,7 @@ for handler_name, level in _LOGGING_HANDLER_MAP.items():
     LOGGING["loggers"][handler_name] = {
         "handlers": ["console"],
         "level": level,
-        "propagate": True,
+        "propagate": False,
     }
 
 TEST = False