policies/expression: migrate to raw python instead of jinja2 (#49)

* policies/expression: migrate to raw python instead of jinja2

* lib/expression: create base evaluator, custom subclass for policies

* core: rewrite propertymappings to use python

* providers/saml: update to new PropertyMappings

* sources/ldap: update to new PropertyMappings

* docs: update docs for new propertymappings

* root: remove jinja2

* root: re-add jinja to lock file as its implicitly required

Pipfile

@@ -40,7 +40,6 @@ signxml = "*"
 structlog = "*"
 swagger-spec-validator = "*"
 urllib3 = {extras = ["secure"],version = "*"}
-jinja2 = "*"
 
 [requires]
 python_version = "3.8"

Pipfile.lock

@@ -345,7 +345,6 @@
                 "sha256:c10142f819c2d22bdcd17548c46fa9b77cf4fda45097854c689666bf425e7484",
                 "sha256:c922560ac46888d47384de1dbdc3daaa2ea993af4b26a436dec31fa2c19ec668"
             ],
-            "index": "pypi",
             "version": "==3.0.0a1"
         },
         "jmespath": {

docs/expressions/index.md

@@ -0,0 +1,55 @@
+# Expressions
+
+Expressions allow you to write custom Logic using Python code.
+
+Expressions are used in different places throughout passbook, and can do different things.
+
+!!! info
+    These functions/objects are available wherever expressions are used. For more specific information, see [Expression Policies](../policies/expression.md) and [Property Mappings](../property-mappings/expression.md)
+
+## Global objects
+
+- `pb_logger`: structlog BoundLogger. ([ref](https://www.structlog.org/en/stable/api.html#structlog.BoundLogger))
+- `requests`: requests Session object. ([ref](https://requests.readthedocs.io/en/master/user/advanced/))
+
+## Generally available functions
+
+### `regex_match(value: Any, regex: str) -> bool`
+
+Check if `value` matches Regular Expression `regex`.
+
+Example:
+
+```python
+return regex_match(request.user.username, '.*admin.*')
+```
+
+### `regex_replace(value: Any, regex: str, repl: str) -> str`
+
+Replace anything matching `regex` within `value` with `repl` and return it.
+
+Example:
+
+```python
+user_email_local = regex_replace(request.user.email, '(.+)@.+', '')
+```
+
+### `pb_is_group_member(user: User, **group_filters) -> bool`
+
+Check if `user` is member of a group matching `**group_filters`.
+
+Example:
+
+```python
+return pb_is_group_member(request.user, name="test_group")
+```
+
+### `pb_user_by(**filters) -> Optional[User]`
+
+Fetch a user matching `**filters`. Returns None if no user was found.
+
+Example:
+
+```python
+other_user = pb_user_by(username="other_user")
+```

docs/expressions/reference/user-object.md

@@ -15,6 +15,7 @@ The User object has the following attributes:
 
 List all the User's Group Names
 
-```jinja2
-[{% for group in user.groups.all() %}'{{ group.name }}',{% endfor %}]
+```python
+for group in user.groups.all():
+    yield group.name
 ```

docs/policies/expression.md

@@ -0,0 +1,27 @@
+# Expression Policies
+
+The passing of the policy is determined by the return value of the code. Use `return True` to pass a policy and `return False` to fail it.
+
+### Available Functions
+
+#### `pb_message(message: str)`
+
+Add a message, visible by the end user. This can be used to show the reason why they were denied.
+
+Example:
+
+```python
+pb_message("Access denied")
+return False
+```
+
+### Context variables
+
+- `request`: A PolicyRequest object, which has the following properties:
+    - `request.user`: The current User, which the Policy is applied against. ([ref](../expressions/reference/user-object.md))
+    - `request.http_request`: The Django HTTP Request. ([ref](https://docs.djangoproject.com/en/3.0/ref/request-response/#httprequest-objects))
+    - `request.obj`: A Django Model instance. This is only set if the Policy is ran against an object.
+    - `request.context`: A dictionary with dynamic data. This depends on the origin of the execution.
+- `pb_is_sso_flow`: Boolean which is true if request was initiated by authenticating through an external Provider.
+- `pb_client_ip`: Client's IP Address or '255.255.255.255' if no IP Address could be extracted.
+- `pb_flow_plan`: Current Plan if Policy is called from the Flow Planner.

docs/policies/expression/index.md

@@ -1,22 +0,0 @@
-# Expression Policy
-
-Expression Policies allows you to write custom Policy Logic using Jinja2 Templating language.
-
-For a language reference, see [here](https://jinja.palletsprojects.com/en/2.11.x/templates/).
-
-The following objects are passed into the variable:
-
-- `request`: A PolicyRequest object, which has the following properties:
-    - `request.user`: The current User, which the Policy is applied against. ([ref](../../property-mappings/reference/user-object.md))
-    - `request.http_request`: The Django HTTP Request, as documented [here](https://docs.djangoproject.com/en/3.0/ref/request-response/#httprequest-objects).
-    - `request.obj`: A Django Model instance. This is only set if the Policy is ran against an object.
-- `pb_flow_plan`: Current Plan if Policy is called while a flow is active.
-- `pb_is_sso_flow`: Boolean which is true if request was initiated by authenticating through an external Provider.
-- `pb_is_group_member(user, group_name)`: Function which checks if `user` is member of a Group with Name `gorup_name`.
-- `pb_logger`: Standard Python Logger Object, which can be used to debug expressions.
-- `pb_client_ip`: Client's IP Address.
-
-There are also the following custom filters available:
-
-- `regex_match(regex)`: Return True if value matches `regex`
-- `regex_replace(regex, repl)`: Replace string matched by `regex` with `repl`

docs/policies/index.md

@@ -8,10 +8,6 @@ There are two different Kind of policies, a Standard Policy and a Password Polic
 
 ---
 
-### Group-Membership Policy
-
-This policy evaluates to True if the current user is a Member of the selected group.
-
 ### Reputation Policy
 
 passbook keeps track of failed login attempts by Source IP and Attempted Username. These values are saved as scores. Each failed login decreases the Score for the Client IP as well as the targeted Username by one.
@@ -20,11 +16,7 @@ This policy can be used to for example prompt Clients with a low score to pass a
 
 ## Expression Policy
 
-See [Expression Policy](expression/index.md).
-
-### Webhook Policy
-
-This policy allows you to send an arbitrary HTTP Request to any URL. You can then use JSONPath to extract the result you need.
+See [Expression Policy](expression.md).
 
 ## Password Policies
 

docs/property-mappings/expression.md

@@ -0,0 +1,9 @@
+# Property Mapping Expressions
+
+The property mapping should return a value that is expected by the Provider/Source. What types are supported, is documented in the individual Provider/Source. Returning `None` is always accepted, this simply skips this mapping.
+
+### Context Variables
+
+- `user`: The current user, this might be `None` if there is no contextual user. ([ref](../expression/reference/user-object.md))
+- `request`: The current request, this might be `None` if there is no contextual request. ([ref](https://docs.djangoproject.com/en/3.0/ref/request-response/#httprequest-objects))
+- Arbitrary other arguments given by the provider, this is documented on the Provider/Source.

docs/property-mappings/index.md

@@ -12,10 +12,10 @@ You can find examples [here](integrations/)
 
 LDAP Property Mappings are used when you define a LDAP Source. These Mappings define which LDAP Property maps to which passbook Property. By default, these mappings are created:
 
--   Autogenerated LDAP Mapping: givenName -> first_name
--   Autogenerated LDAP Mapping: mail -> email
--   Autogenerated LDAP Mapping: name -> name
--   Autogenerated LDAP Mapping: sAMAccountName -> username
--   Autogenerated LDAP Mapping: sn -> last_name
+- Autogenerated LDAP Mapping: givenName -> first_name
+- Autogenerated LDAP Mapping: mail -> email
+- Autogenerated LDAP Mapping: name -> name
+- Autogenerated LDAP Mapping: sAMAccountName -> username
+- Autogenerated LDAP Mapping: sn -> last_name
 
 These are configured for the most common LDAP Setups.

mkdocs.yml

@@ -10,14 +10,17 @@ nav:
     - Kubernetes: installation/kubernetes.md
   - Sources: sources.md
   - Providers: providers.md
+  - Expressions:
+    - Overview: expressions/index.md
+    - Reference:
+      - User Object: expressions/reference/user-object.md
   - Property Mappings:
     - Overview: property-mappings/index.md
-    - Reference:
-      - User Object: property-mappings/reference/user-object.md
+    - Expressions: property-mappings/expression.md
   - Factors: factors.md
   - Policies:
     - Overview: policies/index.md
-    - Expression: policies/expression/index.md
+    - Expression: policies/expression.md
   - Integrations:
     - as Provider:
         - Amazon Web Services: integrations/services/aws/index.md
@@ -38,3 +41,11 @@ markdown_extensions:
   - toc:
       permalink: "¶"
   - admonition
+  - codehilite
+  - pymdownx.betterem:
+      smart_enable: all
+  - pymdownx.inlinehilite
+  - pymdownx.magiclink
+
+plugins:
+  - search

passbook/admin/templates/administration/base.html

@@ -14,7 +14,7 @@
 <link rel="stylesheet" href="{% static 'node_modules/codemirror/theme/monokai.css' %}">
 <script src="{% static 'node_modules/codemirror/mode/xml/xml.js' %}"></script>
 <script src="{% static 'node_modules/codemirror/mode/yaml/yaml.js' %}"></script>
-<script src="{% static 'node_modules/codemirror/mode/jinja2/jinja2.js' %}"></script>
+<script src="{% static 'node_modules/codemirror/mode/python/python.js' %}"></script>
 {% endblock %}
 
 {% block page_content %}

passbook/core/expression.py

@@ -0,0 +1,21 @@
+"""Property Mapping Evaluator"""
+from typing import Optional
+
+from django.http import HttpRequest
+
+from passbook.core.models import User
+from passbook.lib.expression.evaluator import BaseEvaluator
+
+
+class PropertyMappingEvaluator(BaseEvaluator):
+    """Custom Evalautor that adds some different context variables."""
+
+    def set_context(
+        self, user: Optional[User], request: Optional[HttpRequest], **kwargs
+    ):
+        """Update context with context from PropertyMapping's evaluate"""
+        if user:
+            self._context["user"] = user
+        if request:
+            self._context["request"] = request
+        self._context.update(**kwargs)

passbook/core/migrations/0002_default_user.py

@@ -14,6 +14,7 @@ def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor):
     )
     pbadmin.set_password("pbadmin")  # nosec
     pbadmin.is_superuser = True
+    pbadmin.is_staff = True
     pbadmin.save()
 
 

passbook/core/models.py

@@ -5,14 +5,11 @@ from uuid import uuid4
 
 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.http import HttpRequest
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from guardian.mixins import GuardianUserMixin
-from jinja2 import Undefined
-from jinja2.exceptions import TemplateSyntaxError, UndefinedError
 from model_utils.managers import InheritanceManager
 from structlog import get_logger
 
@@ -206,30 +203,14 @@ class PropertyMapping(models.Model):
         self, user: Optional[User], request: Optional[HttpRequest], **kwargs
     ) -> Any:
         """Evaluate `self.expression` using `**kwargs` as Context."""
-        from passbook.policies.expression.evaluator import Evaluator
+        from passbook.core.expression import PropertyMappingEvaluator
 
-        evaluator = Evaluator()
+        evaluator = PropertyMappingEvaluator()
+        evaluator.set_context(user, request, **kwargs)
         try:
-            expression = evaluator.env.from_string(self.expression)
-        except TemplateSyntaxError as exc:
+            return evaluator.evaluate(self.expression)
+        except (ValueError, SyntaxError) as exc:
             raise PropertyMappingExpressionException from exc
-        try:
-            response = expression.render(user=user, request=request, **kwargs)
-            if isinstance(response, Undefined):
-                raise PropertyMappingExpressionException("Response was 'Undefined'")
-            return response
-        except UndefinedError as exc:
-            raise PropertyMappingExpressionException from exc
-
-    def save(self, *args, **kwargs):
-        from passbook.policies.expression.evaluator import Evaluator
-
-        evaluator = Evaluator()
-        try:
-            evaluator.env.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/lib/expression/evaluator.py

@@ -0,0 +1,101 @@
+"""passbook expression policy evaluator"""
+import re
+from textwrap import indent
+from typing import Any, Dict, Iterable, Optional
+
+from django.core.exceptions import ValidationError
+from requests import Session
+from structlog import get_logger
+
+from passbook.core.models import User
+
+LOGGER = get_logger()
+
+
+class BaseEvaluator:
+    """Validate and evaluate python-based expressions"""
+
+    # Globals that can be used by function
+    _globals: Dict[str, Any]
+    # Context passed as locals to exec()
+    _context: Dict[str, Any]
+
+    # Filename used for exec
+    _filename: str
+
+    def __init__(self):
+        # update passbook/policies/expression/templates/policy/expression/form.html
+        # update docs/policies/expression/index.md
+        self._globals = {
+            "regex_match": BaseEvaluator.expr_filter_regex_match,
+            "regex_replace": BaseEvaluator.expr_filter_regex_replace,
+            "pb_is_group_member": BaseEvaluator.expr_func_is_group_member,
+            "pb_user_by": BaseEvaluator.expr_func_user_by,
+            "pb_logger": get_logger(),
+            "requests": Session(),
+        }
+        self._context = {}
+        self._filename = "BaseEvalautor"
+
+    @staticmethod
+    def expr_filter_regex_match(value: Any, regex: str) -> bool:
+        """Expression Filter to run re.search"""
+        return re.search(regex, value) is None
+
+    @staticmethod
+    def expr_filter_regex_replace(value: Any, regex: str, repl: str) -> str:
+        """Expression Filter to run re.sub"""
+        return re.sub(regex, repl, value)
+
+    @staticmethod
+    def expr_func_user_by(**filters) -> Optional[User]:
+        """Get user by filters"""
+        users = User.objects.filter(**filters)
+        if users:
+            return users.first()
+        return None
+
+    @staticmethod
+    def expr_func_is_group_member(user: User, **group_filters) -> bool:
+        """Check if `user` is member of group with name `group_name`"""
+        return user.groups.filter(**group_filters).exists()
+
+    def wrap_expression(self, expression: str, params: Iterable[str]) -> str:
+        """Wrap expression in a function, call it, and save the result as `result`"""
+        handler_signature = ",".join(params)
+        full_expression = f"def handler({handler_signature}):\n"
+        full_expression += indent(expression, "    ")
+        full_expression += f"\nresult = handler({handler_signature})"
+        return full_expression
+
+    def evaluate(self, expression_source: str) -> Any:
+        """Parse and evaluate expression. If the syntax is incorrect, a SyntaxError is raised.
+        If any exception is raised during execution, it is raised.
+        The result is returned without any type-checking."""
+        param_keys = self._context.keys()
+        ast_obj = compile(
+            self.wrap_expression(expression_source, param_keys), self._filename, "exec",
+        )
+        try:
+            _locals = self._context
+            # Yes this is an exec, yes it is potentially bad. Since we limit what variables are
+            # available here, and these policies can only be edited by admins, this is a risk
+            # we're willing to take.
+            # pylint: disable=exec-used
+            exec(ast_obj, self._globals, _locals)  # nosec # noqa
+            result = _locals["result"]
+        except Exception as exc:
+            LOGGER.warning("Expression error", exc=exc)
+            raise
+        return result
+
+    def validate(self, expression: str) -> bool:
+        """Validate expression's syntax, raise ValidationError if Syntax is invalid"""
+        param_keys = self._context.keys()
+        try:
+            compile(
+                self.wrap_expression(expression, param_keys), self._filename, "exec",
+            )
+            return True
+        except (ValueError, SyntaxError) as exc:
+            raise ValidationError(f"Expression Syntax Error: {str(exc)}") from exc

passbook/policies/expression/evaluator.py

@@ -1,81 +1,30 @@
 """passbook expression policy evaluator"""
-import re
-from typing import Any, Dict, List, Optional
+from typing import List
 
-from django.core.exceptions import ValidationError
 from django.http import HttpRequest
-from jinja2 import Undefined
-from jinja2.exceptions import TemplateSyntaxError
-from jinja2.nativetypes import NativeEnvironment
-from requests import Session
 from structlog import get_logger
 
-from passbook.core.models import User
 from passbook.flows.planner import PLAN_CONTEXT_SSO
 from passbook.flows.views import SESSION_KEY_PLAN
+from passbook.lib.expression.evaluator import BaseEvaluator
 from passbook.lib.utils.http import get_client_ip
 from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 
 
-class Evaluator:
-    """Validate and evaluate jinja2-based expressions"""
+class PolicyEvaluator(BaseEvaluator):
+    """Validate and evaluate python-based expressions"""
 
-    _env: NativeEnvironment
-
-    _context: Dict[str, Any]
     _messages: List[str]
 
-    def __init__(self):
-        self._env = NativeEnvironment(
-            extensions=["jinja2.ext.do"],
-            trim_blocks=True,
-            lstrip_blocks=True,
-            line_statement_prefix=">",
-        )
-        # update passbook/policies/expression/templates/policy/expression/form.html
-        # update docs/policies/expression/index.md
-        self._env.filters["regex_match"] = Evaluator.jinja2_filter_regex_match
-        self._env.filters["regex_replace"] = Evaluator.jinja2_filter_regex_replace
-        self._env.globals["pb_message"] = self.jinja2_func_message
-        self._context = {
-            "pb_is_group_member": Evaluator.jinja2_func_is_group_member,
-            "pb_user_by": Evaluator.jinja2_func_user_by,
-            "pb_logger": get_logger(),
-            "requests": Session(),
-        }
+    def __init__(self, policy_name: str):
+        super().__init__()
         self._messages = []
+        self._context["pb_message"] = self.expr_func_message
+        self._filename = policy_name
 
-    @property
-    def env(self) -> NativeEnvironment:
-        """Access to our custom NativeEnvironment"""
-        return self._env
-
-    @staticmethod
-    def jinja2_filter_regex_match(value: Any, regex: str) -> bool:
-        """Jinja2 Filter to run re.search"""
-        return re.search(regex, value) is None
-
-    @staticmethod
-    def jinja2_filter_regex_replace(value: Any, regex: str, repl: str) -> str:
-        """Jinja2 Filter to run re.sub"""
-        return re.sub(regex, repl, value)
-
-    @staticmethod
-    def jinja2_func_user_by(**filters) -> Optional[User]:
-        """Get user by filters"""
-        users = User.objects.filter(**filters)
-        if users:
-            return users.first()
-        return None
-
-    @staticmethod
-    def jinja2_func_is_group_member(user: User, group_name: str) -> bool:
-        """Check if `user` is member of group with name `group_name`"""
-        return user.groups.filter(name=group_name).exists()
-
-    def jinja2_func_message(self, message: str):
+    def expr_func_message(self, message: str):
         """Wrapper to append to messages list, which is returned with PolicyResult"""
         self._messages.append(message)
 
@@ -84,41 +33,35 @@ class Evaluator:
         # update passbook/policies/expression/templates/policy/expression/form.html
         # update docs/policies/expression/index.md
         self._context["pb_is_sso_flow"] = request.context.get(PLAN_CONTEXT_SSO, False)
-        self._context["request"] = request
         if request.http_request:
             self.set_http_request(request.http_request)
+        self._context["request"] = request
 
     def set_http_request(self, request: HttpRequest):
         """Update context based on http request"""
         # update passbook/policies/expression/templates/policy/expression/form.html
         # update docs/policies/expression/index.md
-        self._context["pb_client_ip"] = (
-            get_client_ip(request.http_request) or "255.255.255.255"
-        )
+        self._context["pb_client_ip"] = get_client_ip(request) or "255.255.255.255"
         self._context["request"] = request
-        if SESSION_KEY_PLAN in request.http_request.session:
-            self._context["pb_flow_plan"] = request.http_request.session[
-                SESSION_KEY_PLAN
-            ]
+        if SESSION_KEY_PLAN in request.session:
+            self._context["pb_flow_plan"] = request.session[SESSION_KEY_PLAN]
 
     def evaluate(self, expression_source: str) -> PolicyResult:
         """Parse and evaluate expression. Policy is expected to return a truthy object.
         Messages can be added using 'do pb_message()'."""
         try:
-            expression = self._env.from_string(expression_source.lstrip().rstrip())
-        except TemplateSyntaxError as exc:
+            result = super().evaluate(expression_source)
+        except (ValueError, SyntaxError) as exc:
             return PolicyResult(False, str(exc))
-        try:
-            result: Optional[Any] = expression.render(self._context)
         except Exception as exc:  # pylint: disable=broad-except
             LOGGER.warning("Expression error", exc=exc)
             return PolicyResult(False, str(exc))
         else:
             policy_result = PolicyResult(False)
             policy_result.messages = tuple(self._messages)
-            if isinstance(result, Undefined):
+            if result is None:
                 LOGGER.warning(
-                    "Expression policy returned undefined",
+                    "Expression policy returned None",
                     src=expression_source,
                     req=self._context,
                 )
@@ -126,11 +69,3 @@ class Evaluator:
             if result:
                 policy_result.passing = bool(result)
             return policy_result
-
-    def validate(self, expression: str):
-        """Validate expression's syntax, raise ValidationError if Syntax is invalid"""
-        try:
-            self._env.from_string(expression)
-            return True
-        except TemplateSyntaxError as exc:
-            raise ValidationError(f"Expression Syntax Error: {str(exc)}") from exc

passbook/policies/expression/forms.py

@@ -3,7 +3,7 @@
 from django import forms
 
 from passbook.admin.fields import CodeMirrorWidget
-from passbook.policies.expression.evaluator import Evaluator
+from passbook.policies.expression.evaluator import PolicyEvaluator
 from passbook.policies.expression.models import ExpressionPolicy
 from passbook.policies.forms import GENERAL_FIELDS
 
@@ -14,9 +14,9 @@ class ExpressionPolicyForm(forms.ModelForm):
     template_name = "policy/expression/form.html"
 
     def clean_expression(self):
-        """Test Jinja2 Syntax"""
+        """Test Syntax"""
         expression = self.cleaned_data.get("expression")
-        Evaluator().validate(expression)
+        PolicyEvaluator(self.instance.name).validate(expression)
         return expression
 
     class Meta:
@@ -27,5 +27,5 @@ class ExpressionPolicyForm(forms.ModelForm):
         ]
         widgets = {
             "name": forms.TextInput(),
-            "expression": CodeMirrorWidget(mode="jinja2"),
+            "expression": CodeMirrorWidget(mode="python"),
         }

passbook/policies/expression/models.py

@@ -2,13 +2,13 @@
 from django.db import models
 from django.utils.translation import gettext as _
 
-from passbook.policies.expression.evaluator import Evaluator
+from passbook.policies.expression.evaluator import PolicyEvaluator
 from passbook.policies.models import Policy
 from passbook.policies.types import PolicyRequest, PolicyResult
 
 
 class ExpressionPolicy(Policy):
-    """Jinja2-based Expression policy that allows Admins to write their own logic"""
+    """Implement custom logic using python."""
 
     expression = models.TextField()
 
@@ -16,12 +16,12 @@ class ExpressionPolicy(Policy):
 
     def passes(self, request: PolicyRequest) -> PolicyResult:
         """Evaluate and render expression. Returns PolicyResult(false) on error."""
-        evaluator = Evaluator()
+        evaluator = PolicyEvaluator(self.name)
         evaluator.set_policy_request(request)
         return evaluator.evaluate(self.expression)
 
     def save(self, *args, **kwargs):
-        Evaluator().validate(self.expression)
+        PolicyEvaluator(self.name).validate(self.expression)
         return super().save(*args, **kwargs)
 
     class Meta:

passbook/policies/expression/tests/test_evaluator.py

@@ -3,7 +3,7 @@ from django.core.exceptions import ValidationError
 from django.test import TestCase
 from guardian.shortcuts import get_anonymous_user
 
-from passbook.policies.expression.evaluator import Evaluator
+from passbook.policies.expression.evaluator import PolicyEvaluator
 from passbook.policies.types import PolicyRequest
 
 
@@ -15,15 +15,15 @@ class TestEvaluator(TestCase):
 
     def test_valid(self):
         """test simple value expression"""
-        template = "True"
-        evaluator = Evaluator()
+        template = "return True"
+        evaluator = PolicyEvaluator("test")
         evaluator.set_policy_request(self.request)
         self.assertEqual(evaluator.evaluate(template).passing, True)
 
     def test_messages(self):
         """test expression with message return"""
-        template = '{% do pb_message("some message") %}False'
-        evaluator = Evaluator()
+        template = 'pb_message("some message");return False'
+        evaluator = PolicyEvaluator("test")
         evaluator.set_policy_request(self.request)
         result = evaluator.evaluate(template)
         self.assertEqual(result.passing, False)
@@ -31,32 +31,32 @@ class TestEvaluator(TestCase):
 
     def test_invalid_syntax(self):
         """test invalid syntax"""
-        template = "{%"
-        evaluator = Evaluator()
+        template = ";"
+        evaluator = PolicyEvaluator("test")
         evaluator.set_policy_request(self.request)
         result = evaluator.evaluate(template)
         self.assertEqual(result.passing, False)
-        self.assertEqual(result.messages, ("tag name expected",))
+        self.assertEqual(result.messages, ("invalid syntax (test, line 2)",))
 
     def test_undefined(self):
         """test undefined result"""
         template = "{{ foo.bar }}"
-        evaluator = Evaluator()
+        evaluator = PolicyEvaluator("test")
         evaluator.set_policy_request(self.request)
         result = evaluator.evaluate(template)
         self.assertEqual(result.passing, False)
-        self.assertEqual(result.messages, ("'foo' is undefined",))
+        self.assertEqual(result.messages, ("name 'foo' is not defined",))
 
     def test_validate(self):
         """test validate"""
         template = "True"
-        evaluator = Evaluator()
+        evaluator = PolicyEvaluator("test")
         result = evaluator.validate(template)
         self.assertEqual(result, True)
 
     def test_validate_invalid(self):
         """test validate"""
-        template = "{%"
-        evaluator = Evaluator()
+        template = ";"
+        evaluator = PolicyEvaluator("test")
         with self.assertRaises(ValidationError):
             evaluator.validate(template)

passbook/policies/process.py

@@ -39,6 +39,8 @@ class PolicyProcess(Process):
         super().__init__()
         self.binding = binding
         self.request = request
+        if not isinstance(self.request, PolicyRequest):
+            raise ValueError(f"{self.request} is not a Policy Request.")
         if connection:
             self.connection = connection
 

passbook/providers/saml/forms.py

@@ -4,6 +4,7 @@ from django import forms
 from django.contrib.admin.widgets import FilteredSelectMultiple
 from django.utils.translation import gettext as _
 
+from passbook.core.expression import PropertyMappingEvaluator
 from passbook.providers.saml.models import (
     SAMLPropertyMapping,
     SAMLProvider,
@@ -52,6 +53,13 @@ class SAMLPropertyMappingForm(forms.ModelForm):
 
     template_name = "saml/idp/property_mapping_form.html"
 
+    def clean_expression(self):
+        """Test Syntax"""
+        expression = self.cleaned_data.get("expression")
+        evaluator = PropertyMappingEvaluator()
+        evaluator.validate(expression)
+        return expression
+
     class Meta:
 
         model = SAMLPropertyMapping

passbook/providers/saml/migrations/0002_default_saml_property_mappings.py

@@ -13,27 +13,27 @@ def create_default_property_mappings(apps, schema_editor):
         {
             "FriendlyName": "eduPersonPrincipalName",
             "Name": "urn:oid:1.3.6.1.4.1.5923.1.1.1.6",
-            "Expression": "{{ user.email }}",
+            "Expression": "return user.get('email')",
         },
         {
             "FriendlyName": "cn",
             "Name": "urn:oid:2.5.4.3",
-            "Expression": "{{ user.name }}",
+            "Expression": "return user.get('name')",
         },
         {
             "FriendlyName": "mail",
             "Name": "urn:oid:0.9.2342.19200300.100.1.3",
-            "Expression": "{{ user.email }}",
+            "Expression": "return user.get('email')",
         },
         {
             "FriendlyName": "displayName",
             "Name": "urn:oid:2.16.840.1.113730.3.1.241",
-            "Expression": "{{ user.username }}",
+            "Expression": "return user.get('username')",
         },
         {
             "FriendlyName": "uid",
             "Name": "urn:oid:0.9.2342.19200300.100.1.1",
-            "Expression": "{{ user.pk }}",
+            "Expression": "return user.get('pk')",
         },
         {
             "FriendlyName": "member-of",

passbook/providers/saml/processors/base.py

@@ -1,4 +1,5 @@
 """Basic SAML Processor"""
+from types import GeneratorType
 from typing import TYPE_CHECKING, Dict, List, Union
 
 from cryptography.exceptions import InvalidSignature
@@ -21,7 +22,7 @@ if TYPE_CHECKING:
 
 # pylint: disable=too-many-instance-attributes
 class Processor:
-    """Base SAML 2.0 AuthnRequest to Response Processor.
+    """Base SAML 2.0 Auth-N-Request to Response Processor.
     Sub-classes should provide Service Provider-specific functionality."""
 
     is_idp_initiated = False
@@ -111,6 +112,8 @@ class Processor:
                     request=self._http_request,
                     provider=self._remote,
                 )
+                if value is None:
+                    continue
                 mapping_payload = {
                     "Name": mapping.saml_name,
                     "FriendlyName": mapping.friendly_name,
@@ -119,6 +122,8 @@ class Processor:
                 # differently in the template
                 if isinstance(value, list):
                     mapping_payload["ValueArray"] = value
+                elif isinstance(value, GeneratorType):
+                    mapping_payload["ValueArray"] = list(value)
                 else:
                     mapping_payload["Value"] = value
                 attributes.append(mapping_payload)

passbook/sources/ldap/connector.py

@@ -164,9 +164,10 @@ class Connector:
                 continue
             mapping: LDAPPropertyMapping
             try:
-                properties[mapping.object_field] = mapping.evaluate(
-                    user=None, request=None, ldap=attributes
-                )
+                value = mapping.evaluate(user=None, request=None, ldap=attributes)
+                if value is None:
+                    continue
+                properties[mapping.object_field] = value
             except PropertyMappingExpressionException as exc:
                 LOGGER.warning("Mapping failed to evaluate", exc=exc, mapping=mapping)
                 continue

passbook/sources/ldap/forms.py

@@ -5,6 +5,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple
 from django.utils.translation import gettext_lazy as _
 
 from passbook.admin.forms.source import SOURCE_FORM_FIELDS
+from passbook.core.expression import PropertyMappingEvaluator
 from passbook.sources.ldap.models import LDAPPropertyMapping, LDAPSource
 
 
@@ -52,6 +53,13 @@ class LDAPPropertyMappingForm(forms.ModelForm):
 
     template_name = "ldap/property_mapping_form.html"
 
+    def clean_expression(self):
+        """Test Syntax"""
+        expression = self.cleaned_data.get("expression")
+        evaluator = PropertyMappingEvaluator()
+        evaluator.validate(expression)
+        return expression
+
     class Meta:
 
         model = LDAPPropertyMapping

passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py

@@ -7,11 +7,11 @@ from django.db import migrations
 def create_default_ad_property_mappings(apps: Apps, schema_editor):
     LDAPPropertyMapping = apps.get_model("passbook_sources_ldap", "LDAPPropertyMapping")
     mapping = {
-        "name": "{{ ldap.name }}",
-        "first_name": "{{ ldap.givenName }}",
-        "last_name": "{{ ldap.sn }}",
-        "username": "{{ ldap.sAMAccountName }}",
-        "email": "{{ ldap.mail }}",
+        "name": "return ldap.get('name')",
+        "first_name": "return ldap.get('givenName')",
+        "last_name": "return ldap.get('sn')",
+        "username": "return ldap.get('sAMAccountName')",
+        "email": "return ldap.get('mail')",
     }
     db_alias = schema_editor.connection.alias
     for object_field, expression in mapping.items():

passbook/stages/prompt/tests.py

@@ -111,12 +111,10 @@ class TestPromptStage(TestCase):
             self.assertIn(prompt.label, response.rendered_content)
             self.assertIn(prompt.placeholder, response.rendered_content)
 
-    def test_valid_form(self) -> PromptForm:
+    def test_valid_form_with_policy(self) -> PromptForm:
         """Test form validation"""
         plan = FlowPlan(flow_pk=self.flow.pk.hex, stages=[self.stage])
-        expr = (
-            "{{ request.context.password_prompt == request.context.password2_prompt }}"
-        )
+        expr = "return request.context['password_prompt'] == request.context['password2_prompt']"
         expr_policy = ExpressionPolicy.objects.create(
             name="validate-form", expression=expr
         )
@@ -144,7 +142,7 @@ class TestPromptStage(TestCase):
         session[SESSION_KEY_PLAN] = plan
         session.save()
 
-        form = self.test_valid_form()
+        form = self.test_valid_form_with_policy()
 
         with patch("passbook.flows.views.FlowExecutorView.cancel", MagicMock()):
             response = self.client.post(