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