From 813b2676de767c0f2048960c9dbfff3a1149b575 Mon Sep 17 00:00:00 2001
From: Jens Langhammer <jens.langhammer@beryju.org>
Date: Tue, 18 Feb 2020 10:12:42 +0100
Subject: [PATCH] providers/saml: better handle PropertyMapping evaluation
 errors

---
 passbook/core/exceptions.py                |  5 +++++
 passbook/core/models.py                    | 12 ++++++++++--
 passbook/providers/saml/processors/base.py | 11 ++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 passbook/core/exceptions.py

diff --git a/passbook/core/exceptions.py b/passbook/core/exceptions.py
new file mode 100644
index 000000000..53961e631
--- /dev/null
+++ b/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."""
diff --git a/passbook/core/models.py b/passbook/core/models.py
index 7f1e26c8e..9372a1d98 100644
--- a/passbook/core/models.py
+++ b/passbook/core/models.py
@@ -5,6 +5,7 @@ from time import sleep
 from typing import Optional, Any
 from uuid import uuid4
 
+from jinja2.exceptions import TemplateSyntaxError, UndefinedError
 from jinja2.nativetypes import NativeEnvironment
 from django.contrib.auth.models import AbstractUser
 from django.contrib.postgres.fields import JSONField
@@ -18,6 +19,7 @@ from guardian.mixins import GuardianUserMixin
 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 +305,14 @@ 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 __str__(self):
         return f"Property Mapping {self.name}"
diff --git a/passbook/providers/saml/processors/base.py b/passbook/providers/saml/processors/base.py
index 12942f8b7..13d51b4ba 100644
--- a/passbook/providers/saml/processors/base.py
+++ b/passbook/providers/saml/processors/base.py
@@ -5,6 +5,7 @@ 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.utils import get_random_id
 from passbook.providers.saml.utils.encoding import decode_base64_and_inflate, nice64
@@ -97,7 +98,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 +111,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