providers/saml: better handle PropertyMapping evaluation errors

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

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

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