new release: 0.7.17-beta

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.7.16-beta
+current_version = 0.7.17-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.7.16-beta
+          -t beryju/passbook:0.7.17-beta
           -t beryju/passbook:latest
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook:0.7.16-beta
+        run: docker push beryju/passbook:0.7.17-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.7.16-beta \
+          -t beryju/passbook-gatekeeper:0.7.17-beta \
           -t beryju/passbook-gatekeeper:latest \
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-gatekeeper:0.7.16-beta
+        run: docker push beryju/passbook-gatekeeper:0.7.17-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.7.16-beta
+          -t beryju/passbook-static:0.7.17-beta
           -t beryju/passbook-static:latest
           -f static.Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-static:0.7.16-beta
+        run: docker push beryju/passbook-static:0.7.17-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook-static:latest
   test-release:

docs/integrations/services/gitlab/index.md

@@ -4,9 +4,8 @@
 
 From https://about.gitlab.com/what-is-gitlab/
 
-```
-GitLab is a complete DevOps platform, delivered as a single application. This makes GitLab unique and makes Concurrent DevOps possible, unlocking your organization from the constraints of a pieced together toolchain. Join us for a live Q&A to learn how GitLab can give you unmatched visibility and higher levels of efficiency in a single application across the DevOps lifecycle.
-```
+!!! note ""
+    GitLab is a complete DevOps platform, delivered as a single application. This makes GitLab unique and makes Concurrent DevOps possible, unlocking your organization from the constraints of a pieced together toolchain. Join us for a live Q&A to learn how GitLab can give you unmatched visibility and higher levels of efficiency in a single application across the DevOps lifecycle.
 
 ## Preparation
 

docs/integrations/services/harbor/index.md

@@ -4,9 +4,8 @@
 
 From https://goharbor.io
 
-```
-Harbor is an open source container image registry that secures images with role-based access control, scans images for vulnerabilities, and signs images as trusted. A CNCF Incubating project, Harbor delivers compliance, performance, and interoperability to help you consistently and securely manage images across cloud native compute platforms like Kubernetes and Docker.
-```
+!!! note ""
+    Harbor is an open source container image registry that secures images with role-based access control, scans images for vulnerabilities, and signs images as trusted. A CNCF Incubating project, Harbor delivers compliance, performance, and interoperability to help you consistently and securely manage images across cloud native compute platforms like Kubernetes and Docker.
 
 ## Preparation
 

docs/integrations/services/rancher/index.md

@@ -4,10 +4,9 @@
 
 From https://rancher.com/products/rancher
 
-```
-An Enterprise Platform for Managing Kubernetes Everywhere
-Rancher is a platform built to address the needs of the DevOps teams deploying applications with Kubernetes, and the IT staff responsible for delivering an enterprise-critical service.
-```
+!!! note ""
+    An Enterprise Platform for Managing Kubernetes Everywhere
+    Rancher is a platform built to address the needs of the DevOps teams deploying applications with Kubernetes, and the IT staff responsible for delivering an enterprise-critical service.
 
 ## Preparation
 

docs/integrations/services/sentry/index.md

@@ -4,13 +4,12 @@
 
 From https://sentry.io
 
-```
-Sentry provides self-hosted and cloud-based error monitoring that helps all software
-teams discover, triage, and prioritize errors in real-time.
+!!! note ""
+    Sentry provides self-hosted and cloud-based error monitoring that helps all software
+    teams discover, triage, and prioritize errors in real-time.
 
-One million developers at over fifty thousand companies already ship
-better software faster with Sentry. Won’t you join them?
-```
+    One million developers at over fifty thousand companies already ship
+    better software faster with Sentry. Won’t you join them?
 
 ## Preparation
 

helm/Chart.yaml

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

helm/values.yaml

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

mkdocs.yml

@@ -32,3 +32,4 @@ theme:
 markdown_extensions:
   - toc:
       permalink: "¶"
+  - admonition

passbook/__init__.py

@@ -1,2 +1,2 @@
 """passbook"""
-__version__ = "0.7.16-beta"
+__version__ = "0.7.17-beta"

passbook/core/exceptions.py

@@ -0,0 +1,5 @@
+"""passbook core exceptions"""
+
+
+class PropertyMappingExpressionException(Exception):
+    """Error when a PropertyMapping Exception expression could not be parsed or evaluated."""

passbook/core/models.py

@@ -2,22 +2,25 @@
 from datetime import timedelta
 from random import SystemRandom
 from time import sleep
-from typing import Optional, Any
+from typing import Any, Optional
 from uuid import uuid4
 
-from jinja2.nativetypes import NativeEnvironment
 from django.contrib.auth.models import AbstractUser
 from django.contrib.postgres.fields import JSONField
+from django.core.exceptions import ValidationError
 from django.db import models
-from django.urls import reverse_lazy
 from django.http import HttpRequest
+from django.urls import reverse_lazy
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from django_prometheus.models import ExportModelOperationsMixin
 from guardian.mixins import GuardianUserMixin
+from jinja2.exceptions import TemplateSyntaxError, UndefinedError
+from jinja2.nativetypes import NativeEnvironment
 from model_utils.managers import InheritanceManager
 from structlog import get_logger
 
+from passbook.core.exceptions import PropertyMappingExpressionException
 from passbook.core.signals import password_changed
 from passbook.lib.models import CreatedUpdatedModel, UUIDModel
 from passbook.policies.exceptions import PolicyException
@@ -303,8 +306,21 @@ class PropertyMapping(UUIDModel):
 
     def evaluate(self, user: User, request: HttpRequest, **kwargs) -> Any:
         """Evaluate `self.expression` using `**kwargs` as Context."""
-        expression = NATIVE_ENVIRONMENT.from_string(self.expression)
-        return expression.render(user=user, request=request, **kwargs)
+        try:
+            expression = NATIVE_ENVIRONMENT.from_string(self.expression)
+        except TemplateSyntaxError as exc:
+            raise PropertyMappingExpressionException from exc
+        try:
+            return expression.render(user=user, request=request, **kwargs)
+        except UndefinedError as exc:
+            raise PropertyMappingExpressionException from exc
+
+    def save(self, *args, **kwargs):
+        try:
+            NATIVE_ENVIRONMENT.from_string(self.expression)
+        except TemplateSyntaxError as exc:
+            raise ValidationError("Expression Syntax Error") from exc
+        return super().save(*args, **kwargs)
 
     def __str__(self):
         return f"Property Mapping {self.name}"

passbook/core/templates/error/400.html

@@ -19,6 +19,9 @@
     <h1>{% trans 'Bad Request' %}</h1>
 </header>
 <form>
+    {% if message %}
+    <h3>{% trans message %}</h3>
+    {% endif %}
     {% if 'back' in request.GET %}
     <a href="{% back %}" class="btn btn-primary btn-block btn-lg">{% trans 'Back' %}</a>
     {% endif %}

passbook/factors/tests.py

@@ -38,9 +38,8 @@ class TestFactorAuthentication(TestCase):
     def test_unauthenticated_raw(self):
         """test direct call to AuthenticationView"""
         response = self.client.get(reverse("passbook_core:auth-process"))
-        # Response should be 302 since no pending user is set
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(response.url, reverse("passbook_core:auth-login"))
+        # Response should be 400 since no pending user is set
+        self.assertEqual(response.status_code, 400)
 
     def test_unauthenticated_prepared(self):
         """test direct call but with pending_uesr in session"""
@@ -71,9 +70,8 @@ class TestFactorAuthentication(TestCase):
         """Test with already logged in user"""
         self.client.force_login(self.user)
         response = self.client.get(reverse("passbook_core:auth-process"))
-        # Response should be 302 since no pending user is set
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(response.url, reverse("passbook_core:overview"))
+        # Response should be 400 since no pending user is set
+        self.assertEqual(response.status_code, 400)
         self.client.logout()
 
     def test_unauthenticated_post(self):

passbook/factors/view.py

@@ -1,15 +1,17 @@
 """passbook multi-factor authentication engine"""
-from typing import List, Tuple
+from typing import List, Optional, Tuple
 
 from django.contrib.auth import login
 from django.contrib.auth.mixins import UserPassesTestMixin
-from django.shortcuts import get_object_or_404, redirect, reverse
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import get_object_or_404, redirect, render, reverse
 from django.utils.http import urlencode
 from django.views.generic import View
 from structlog import get_logger
 
 from passbook.core.models import Factor, User
 from passbook.core.views.utils import PermissionDeniedView
+from passbook.lib.config import CONFIG
 from passbook.lib.utils.reflection import class_to_path, path_to_class
 from passbook.lib.utils.urls import is_url_absolute
 from passbook.policies.engine import PolicyEngine
@@ -44,10 +46,26 @@ class AuthenticationView(UserPassesTestMixin, View):
     current_factor: Factor
 
     # Allow only not authenticated users to login
-    def test_func(self):
+    def test_func(self) -> bool:
         return AuthenticationView.SESSION_PENDING_USER in self.request.session
 
-    def handle_no_permission(self):
+    def _check_config_domain(self) -> Optional[HttpResponse]:
+        """Checks if current request's domain matches configured Domain, and
+        adds a warning if not."""
+        current_domain = self.request.get_host()
+        config_domain = CONFIG.y("domain")
+        if current_domain != config_domain:
+            message = (
+                f"Current domain of '{current_domain}' doesn't "
+                f"match configured domain of '{config_domain}'."
+            )
+            LOGGER.warning(message)
+            return render(
+                self.request, "error/400.html", context={"message": message}, status=400
+            )
+        return None
+
+    def handle_no_permission(self) -> HttpResponse:
         # Function from UserPassesTestMixin
         if NEXT_ARG_NAME in self.request.GET:
             return redirect(self.request.GET.get(NEXT_ARG_NAME))
@@ -55,7 +73,7 @@ class AuthenticationView(UserPassesTestMixin, View):
             return _redirect_with_qs("passbook_core:overview", self.request.GET)
         return _redirect_with_qs("passbook_core:auth-login", self.request.GET)
 
-    def get_pending_factors(self):
+    def get_pending_factors(self) -> List[Tuple[str, str]]:
         """Loading pending factors from Database or load from session variable"""
         # Write pending factors to session
         if AuthenticationView.SESSION_PENDING_FACTORS in self.request.session:
@@ -67,6 +85,7 @@ class AuthenticationView(UserPassesTestMixin, View):
         )
         pending_factors = []
         for factor in _all_factors:
+            factor: Factor
             LOGGER.debug(
                 "Checking if factor applies to user",
                 factor=factor,
@@ -81,10 +100,13 @@ class AuthenticationView(UserPassesTestMixin, View):
                 LOGGER.debug("Factor applies", factor=factor, user=self.pending_user)
         return pending_factors
 
-    def dispatch(self, request, *args, **kwargs):
+    def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         # Check if user passes test (i.e. SESSION_PENDING_USER is set)
         user_test_result = self.get_test_func()()
         if not user_test_result:
+            incorrect_domain_message = self._check_config_domain()
+            if incorrect_domain_message:
+                return incorrect_domain_message
             return self.handle_no_permission()
         # Extract pending user from session (only remember uid)
         self.pending_user = get_object_or_404(
@@ -117,7 +139,7 @@ class AuthenticationView(UserPassesTestMixin, View):
         self._current_factor_class.request = request
         return super().dispatch(request, *args, **kwargs)
 
-    def get(self, request, *args, **kwargs):
+    def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         """pass get request to current factor"""
         LOGGER.debug(
             "Passing GET",
@@ -125,7 +147,7 @@ class AuthenticationView(UserPassesTestMixin, View):
         )
         return self._current_factor_class.get(request, *args, **kwargs)
 
-    def post(self, request, *args, **kwargs):
+    def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         """pass post request to current factor"""
         LOGGER.debug(
             "Passing POST",
@@ -133,7 +155,7 @@ class AuthenticationView(UserPassesTestMixin, View):
         )
         return self._current_factor_class.post(request, *args, **kwargs)
 
-    def user_ok(self):
+    def user_ok(self) -> HttpResponse:
         """Redirect to next Factor"""
         LOGGER.debug(
             "Factor passed",
@@ -160,14 +182,14 @@ class AuthenticationView(UserPassesTestMixin, View):
         LOGGER.debug("User passed all factors, logging in", user=self.pending_user)
         return self._user_passed()
 
-    def user_invalid(self):
+    def user_invalid(self) -> HttpResponse:
         """Show error message, user cannot login.
         This should only be shown if user authenticated successfully, but is disabled/locked/etc"""
         LOGGER.debug("User invalid")
         self.cleanup()
         return _redirect_with_qs("passbook_core:auth-denied", self.request.GET)
 
-    def _user_passed(self):
+    def _user_passed(self) -> HttpResponse:
         """User Successfully passed all factors"""
         backend = self.request.session[AuthenticationView.SESSION_USER_BACKEND]
         login(self.request, self.pending_user, backend=backend)

passbook/lib/views.py

@@ -1,5 +1,6 @@
 """passbook helper views"""
-
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import render
 from django.views.generic import CreateView
 from guardian.shortcuts import assign_perm
 
@@ -22,3 +23,8 @@ class CreateAssignPermView(CreateView):
             )
             assign_perm(full_permission, self.request.user, self.object)
         return response
+
+
+def bad_request_message(request: HttpRequest, message: str) -> HttpResponse:
+    """Return generic error page with message, with status code set to 400"""
+    return render(request, "error/400.html", {"message": message}, status=400)

passbook/policies/expression/admin.py

@@ -0,0 +1,5 @@
+"""Passbook passbook expression policy Admin"""
+
+from passbook.lib.admin import admin_autoregister
+
+admin_autoregister("passbook_policies_expression")

passbook/policies/expression/api.py

@@ -0,0 +1,21 @@
+"""Expression Policy API"""
+from rest_framework.serializers import ModelSerializer
+from rest_framework.viewsets import ModelViewSet
+
+from passbook.policies.expression.models import ExpressionPolicy
+from passbook.policies.forms import GENERAL_SERIALIZER_FIELDS
+
+
+class ExpressionPolicySerializer(ModelSerializer):
+    """Group Membership Policy Serializer"""
+
+    class Meta:
+        model = ExpressionPolicy
+        fields = GENERAL_SERIALIZER_FIELDS + ["expression"]
+
+
+class ExpressionPolicyViewSet(ModelViewSet):
+    """Source Viewset"""
+
+    queryset = ExpressionPolicy.objects.all()
+    serializer_class = ExpressionPolicySerializer

passbook/policies/expression/apps.py

@@ -0,0 +1,11 @@
+"""Passbook policy_expression app config"""
+
+from django.apps import AppConfig
+
+
+class PassbookPolicyExpressionConfig(AppConfig):
+    """Passbook policy_expression app config"""
+
+    name = "passbook.policies.expression"
+    label = "passbook_policies_expression"
+    verbose_name = "passbook Policies.Expression"

passbook/policies/expression/forms.py

@@ -0,0 +1,22 @@
+"""passbook Expression Policy forms"""
+
+from django import forms
+
+from passbook.policies.expression.models import ExpressionPolicy
+from passbook.policies.forms import GENERAL_FIELDS
+
+
+class ExpressionPolicyForm(forms.ModelForm):
+    """ExpressionPolicy Form"""
+
+    template_name = "policy/expression/form.html"
+
+    class Meta:
+
+        model = ExpressionPolicy
+        fields = GENERAL_FIELDS + [
+            "expression",
+        ]
+        widgets = {
+            "name": forms.TextInput(),
+        }

passbook/policies/expression/migrations/0001_initial.py

@@ -0,0 +1,38 @@
+# Generated by Django 3.0.3 on 2020-02-18 14:00
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    initial = True
+
+    dependencies = [
+        ("passbook_core", "0007_auto_20200217_1934"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="ExpressionPolicy",
+            fields=[
+                (
+                    "policy_ptr",
+                    models.OneToOneField(
+                        auto_created=True,
+                        on_delete=django.db.models.deletion.CASCADE,
+                        parent_link=True,
+                        primary_key=True,
+                        serialize=False,
+                        to="passbook_core.Policy",
+                    ),
+                ),
+                ("expression", models.TextField()),
+            ],
+            options={
+                "verbose_name": "Expression Policy",
+                "verbose_name_plural": "Expression Policies",
+            },
+            bases=("passbook_core.policy",),
+        ),
+    ]

passbook/policies/expression/models.py

@@ -0,0 +1,49 @@
+"""passbook expression Policy Models"""
+from django.core.exceptions import ValidationError
+from django.db import models
+from django.utils.translation import gettext as _
+from jinja2.exceptions import TemplateSyntaxError, UndefinedError
+from jinja2.nativetypes import NativeEnvironment
+from structlog import get_logger
+
+from passbook.core.models import Policy
+from passbook.policies.struct import PolicyRequest, PolicyResult
+
+LOGGER = get_logger()
+NATIVE_ENVIRONMENT = NativeEnvironment()
+
+
+class ExpressionPolicy(Policy):
+    """Jinja2-based Expression policy that allows Admins to write their own logic"""
+
+    expression = models.TextField()
+
+    form = "passbook.policies.expression.forms.ExpressionPolicyForm"
+
+    def passes(self, request: PolicyRequest) -> PolicyResult:
+        """Evaluate and render expression. Returns PolicyResult(false) on error."""
+        try:
+            expression = NATIVE_ENVIRONMENT.from_string(self.expression)
+        except TemplateSyntaxError as exc:
+            return PolicyResult(False, str(exc))
+        try:
+            result = expression.render(request=request)
+            if isinstance(result, list) and len(result) == 2:
+                return PolicyResult(*result)
+            if result:
+                return PolicyResult(result)
+            return PolicyResult(False)
+        except UndefinedError as exc:
+            return PolicyResult(False, str(exc))
+
+    def save(self, *args, **kwargs):
+        try:
+            NATIVE_ENVIRONMENT.from_string(self.expression)
+        except TemplateSyntaxError as exc:
+            raise ValidationError("Expression Syntax Error") from exc
+        return super().save(*args, **kwargs)
+
+    class Meta:
+
+        verbose_name = _("Expression Policy")
+        verbose_name_plural = _("Expression Policies")

passbook/policies/expression/templates/policy/expression/form.html

@@ -0,0 +1,20 @@
+{% extends "generic/form.html" %}
+
+{% load i18n %}
+
+{% block beneath_form %}
+<div class="form-group ">
+    <label class="col-sm-2 control-label" for="friendly_name-2">
+    </label>
+    <div class="col-sm-10">
+        <p>
+            Expression using <a href="https://jinja.palletsprojects.com/en/2.11.x/templates/">Jinja</a>. Following variables are available:
+            <ul>
+                <li><code>request.user</code>: Passbook User Object (<a href="https://beryju.github.io/passbook/reference/property-mappings/user-object/">Reference</a>)</li>
+                <li><code>request.http_request</code>: Django HTTP Request Object (<a href="https://docs.djangoproject.com/en/3.0/ref/request-response/#httprequest-objects">Reference</a>) </li>
+                <li><code>request.obj</code>: Model the Policy is run against. </li>
+            </ul>
+        </p>
+    </div>
+</div>
+{% endblock %}

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/migrations/0006_auto_20200217_2031.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.0.3 on 2020-02-17 20:31
+
+from django.db import migrations, models
+
+import passbook.providers.saml.utils.time
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("passbook_providers_saml", "0005_remove_samlpropertymapping_values"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="samlprovider",
+            name="assertion_valid_not_before",
+            field=models.TextField(
+                default="minutes=-5",
+                help_text="Assertion valid not before current time + this value (Format: hours=-1;minutes=-2;seconds=-3).",
+                validators=[
+                    passbook.providers.saml.utils.time.timedelta_string_validator
+                ],
+            ),
+        ),
+    ]

passbook/providers/saml/models.py

@@ -23,12 +23,12 @@ class SAMLProvider(Provider):
     issuer = models.TextField()
 
     assertion_valid_not_before = models.TextField(
-        default="minutes=5",
+        default="minutes=-5",
         validators=[timedelta_string_validator],
         help_text=_(
             (
-                "Assertion valid not before current time - this value "
-                "(Format: hours=1;minutes=2;seconds=3)."
+                "Assertion valid not before current time + this value "
+                "(Format: hours=-1;minutes=-2;seconds=-3)."
             )
         ),
     )
@@ -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:
@@ -106,6 +106,16 @@ class SAMLProvider(Provider):
         except Provider.application.RelatedObjectDoesNotExist:
             return None
 
+    def html_metadata_view(self, request):
+        """return template and context modal with to view Metadata without downloading it"""
+        from passbook.providers.saml.views import DescriptorDownloadView
+
+        metadata = DescriptorDownloadView.get_metadata(request, self)
+        return (
+            "saml/idp/admin_metadata_modal.html",
+            {"provider": self, "metadata": metadata,},
+        )
+
     class Meta:
 
         verbose_name = _("SAML Provider")

passbook/providers/saml/processors/base.py

@@ -5,7 +5,9 @@ from defusedxml import ElementTree
 from django.http import HttpRequest
 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
@@ -97,7 +99,10 @@ class Processor:
         from passbook.providers.saml.models import SAMLPropertyMapping
 
         for mapping in self._remote.property_mappings.all().select_subclasses():
-            if isinstance(mapping, SAMLPropertyMapping):
+            if not isinstance(mapping, SAMLPropertyMapping):
+                continue
+            try:
+                mapping: SAMLPropertyMapping
                 value = mapping.evaluate(
                     user=self._http_request.user,
                     request=self._http_request,
@@ -107,11 +112,16 @@ class Processor:
                     "Name": mapping.saml_name,
                     "FriendlyName": mapping.friendly_name,
                 }
+                # Normal values and arrays need different dict keys as they are handeled
+                # differently in the template
                 if isinstance(value, list):
                     mapping_payload["ValueArray"] = value
                 else:
                     mapping_payload["Value"] = value
                 attributes.append(mapping_payload)
+            except PropertyMappingExpressionException as exc:
+                self._logger.warning(exc)
+                continue
         self._assertion_params["ATTRIBUTES"] = attributes
         self._assertion_xml = get_assertion_xml(
             "saml/xml/assertions/generic.xml", self._assertion_params, signed=True
@@ -124,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."""
@@ -174,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
@@ -187,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
@@ -201,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
@@ -218,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/admin_metadata_modal.html

@@ -0,0 +1,41 @@
+{% load i18n %}
+{% load static %}
+
+<script src="{% static 'codemirror/lib/codemirror.js' %}"></script>
+<script src="{% static 'codemirror/addon/display/autorefresh.js' %}"></script>
+<link rel="stylesheet" href="{% static 'codemirror/lib/codemirror.css' %}">
+<link rel="stylesheet" href="{% static 'codemirror/theme/monokai.css' %}">
+<script src="{% static 'codemirror/mode/xml/xml.js' %}"></script>
+
+<button class="btn btn-default btn-sm" data-toggle="modal" data-target="#{{ provider.pk }}">{% trans 'View Metadata' %}</button>
+<div class="modal fade" id="{{ provider.pk }}" tabindex="-1" role="dialog" aria-labelledby="{{ provider.pk }}Label" aria-hidden="true">
+  <div class="modal-dialog modal-lg">
+    <div class="modal-content">
+      <div class="modal-header">
+        <button type="button" class="close" data-dismiss="modal" aria-hidden="true" aria-label="Close">
+          <span class="pficon pficon-close"></span>
+        </button>
+        <h4 class="modal-title" id="{{ provider.pk }}Label">{% trans 'Metadata' %}</h4>
+      </div>
+      <div class="modal-body">
+        <form class="form-horizontal">
+          <textarea class="codemirror" id="{{ provider.pk }}-textarea">
+{{ metadata }}
+          </textarea>
+        </form>
+      </div>
+      <div class="modal-footer">
+        <button type="button" class="btn btn-primary" data-dismiss="modal">{% trans 'Close' %}</button>
+      </div>
+    </div>
+  </div>
+</div>
+<script>
+  CodeMirror.fromTextArea(document.getElementById("{{ provider.pk }}-textarea"), {
+    mode: 'xml',
+    theme: 'monokai',
+    lineNumbers: false,
+    readOnly: true,
+    autoRefresh: true,
+  });
+</script>

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="RelayState" value="{{ relay_state }}" />
-    <input type="hidden" name="SAMLResponse" value="{{ saml_response }}" />
+    <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="login-group">
         <h3>
-            {% 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 %}
         </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(
-        "<slug:application>/login/", views.LoginBeginView.as_view(), name="saml-login"
-    ),
+    # This view is used to initiate a Login-flow from the IDP
     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,83 @@ 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."""
 
-    # pylint: disable=unused-argument
+    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
-        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 <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
-            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):
@@ -219,22 +218,23 @@ class SLOLogout(CSRFExemptMixin, AccessRequiredView):
 class DescriptorDownloadView(AccessRequiredView):
     """Replies with the XML Metadata IDSSODescriptor."""
 
-    def get(self, request: HttpRequest, application: str) -> HttpResponse:
-        """Replies with the XML Metadata IDSSODescriptor."""
-        entity_id = self.provider.issuer
+    @staticmethod
+    def get_metadata(request: HttpRequest, provider: SAMLProvider) -> str:
+        """Return rendered XML Metadata"""
+        entity_id = provider.issuer
         slo_url = request.build_absolute_uri(
             reverse(
                 "passbook_providers_saml:saml-logout",
-                kwargs={"application": application},
+                kwargs={"application": provider.application},
             )
         )
         sso_url = request.build_absolute_uri(
             reverse(
                 "passbook_providers_saml:saml-login",
-                kwargs={"application": application},
+                kwargs={"application": provider.application},
             )
         )
-        pubkey = strip_pem_header(self.provider.signing_cert.replace("\r", "")).replace(
+        pubkey = strip_pem_header(provider.signing_cert.replace("\r", "")).replace(
             "\n", ""
         )
         ctx = {
@@ -243,7 +243,12 @@ class DescriptorDownloadView(AccessRequiredView):
             "slo_url": slo_url,
             "sso_url": sso_url,
         }
-        metadata = render_to_string("saml/xml/metadata.xml", ctx)
+        return render_to_string("saml/xml/metadata.xml", ctx)
+
+    # pylint: disable=unused-argument
+    def get(self, request: HttpRequest, application: str) -> HttpResponse:
+        """Replies with the XML Metadata IDSSODescriptor."""
+        metadata = DescriptorDownloadView.get_metadata(request, self.provider)
         response = HttpResponse(metadata, content_type="application/xml")
         response["Content-Disposition"] = (
             'attachment; filename="' '%s_passbook_meta.xml"' % self.provider.name
@@ -254,9 +259,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

@@ -98,6 +98,7 @@ INSTALLED_APPS = [
     "passbook.policies.password.apps.PassbookPoliciesPasswordConfig",
     "passbook.policies.sso.apps.PassbookPoliciesSSOConfig",
     "passbook.policies.webhook.apps.PassbookPoliciesWebhookConfig",
+    "passbook.policies.expression.apps.PassbookPolicyExpressionConfig",
 ]
 
 GUARDIAN_MONKEY_PATCH = False
@@ -276,7 +277,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),

passbook/sources/ldap/connector.py

@@ -5,8 +5,9 @@ import ldap3
 import ldap3.core.exceptions
 from structlog import get_logger
 
+from passbook.core.exceptions import PropertyMappingExpressionException
 from passbook.core.models import Group, User
-from passbook.sources.ldap.models import LDAPSource, LDAPPropertyMapping
+from passbook.sources.ldap.models import LDAPPropertyMapping, LDAPSource
 
 LOGGER = get_logger()
 
@@ -155,9 +156,13 @@ class Connector:
         properties = {"attributes": {}}
         for mapping in self._source.property_mappings.all().select_subclasses():
             mapping: LDAPPropertyMapping
-            properties[mapping.object_field] = mapping.evaluate(
-                user=None, request=None, ldap=attributes
-            )
+            try:
+                properties[mapping.object_field] = mapping.evaluate(
+                    user=None, request=None, ldap=attributes
+                )
+            except PropertyMappingExpressionException as exc:
+                LOGGER.warning(exc)
+                continue
         if self._source.object_uniqueness_field in attributes:
             properties["attributes"]["ldap_uniq"] = attributes.get(
                 self._source.object_uniqueness_field