diff --git a/Pipfile b/Pipfile index 5230760fc..5a6c688d9 100644 --- a/Pipfile +++ b/Pipfile @@ -40,7 +40,6 @@ signxml = "*" structlog = "*" swagger-spec-validator = "*" urllib3 = {extras = ["secure"],version = "*"} -jinja2 = "*" [requires] python_version = "3.8" diff --git a/Pipfile.lock b/Pipfile.lock index d9cd8623d..f09bc6204 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -345,7 +345,6 @@ "sha256:c10142f819c2d22bdcd17548c46fa9b77cf4fda45097854c689666bf425e7484", "sha256:c922560ac46888d47384de1dbdc3daaa2ea993af4b26a436dec31fa2c19ec668" ], - "index": "pypi", "version": "==3.0.0a1" }, "jmespath": { diff --git a/docs/expressions/index.md b/docs/expressions/index.md new file mode 100644 index 000000000..4355dfca3 --- /dev/null +++ b/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") +``` diff --git a/docs/property-mappings/reference/user-object.md b/docs/expressions/reference/user-object.md similarity index 84% rename from docs/property-mappings/reference/user-object.md rename to docs/expressions/reference/user-object.md index 8cc35c162..5cdb0780a 100644 --- a/docs/property-mappings/reference/user-object.md +++ b/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 ``` diff --git a/docs/policies/expression.md b/docs/policies/expression.md new file mode 100644 index 000000000..e3d0812cb --- /dev/null +++ b/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. diff --git a/docs/policies/expression/index.md b/docs/policies/expression/index.md deleted file mode 100644 index 2bf6b2fb0..000000000 --- a/docs/policies/expression/index.md +++ /dev/null @@ -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` diff --git a/docs/policies/index.md b/docs/policies/index.md index b413a7794..362f43d54 100644 --- a/docs/policies/index.md +++ b/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 diff --git a/docs/property-mappings/expression.md b/docs/property-mappings/expression.md new file mode 100644 index 000000000..9f43de0eb --- /dev/null +++ b/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. diff --git a/docs/property-mappings/index.md b/docs/property-mappings/index.md index 0eebcb0d7..7e7e21e17 100644 --- a/docs/property-mappings/index.md +++ b/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. diff --git a/mkdocs.yml b/mkdocs.yml index cb31d86a1..2fd537455 100644 --- a/mkdocs.yml +++ b/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 diff --git a/passbook/admin/templates/administration/base.html b/passbook/admin/templates/administration/base.html index 7c4ccc5f7..864c526c5 100644 --- a/passbook/admin/templates/administration/base.html +++ b/passbook/admin/templates/administration/base.html @@ -14,7 +14,7 @@ - + {% endblock %} {% block page_content %} diff --git a/passbook/core/expression.py b/passbook/core/expression.py new file mode 100644 index 000000000..37a397ca0 --- /dev/null +++ b/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) diff --git a/passbook/core/migrations/0002_default_user.py b/passbook/core/migrations/0002_default_user.py index 66e6a2d3e..82a8d6d35 100644 --- a/passbook/core/migrations/0002_default_user.py +++ b/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() diff --git a/passbook/core/models.py b/passbook/core/models.py index 13517ea0e..4e42ba6e9 100644 --- a/passbook/core/models.py +++ b/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}" diff --git a/passbook/lib/expression/__init__.py b/passbook/lib/expression/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/passbook/lib/expression/evaluator.py b/passbook/lib/expression/evaluator.py new file mode 100644 index 000000000..3a252f08b --- /dev/null +++ b/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 diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 01ed3b7ae..825c23ebf 100644 --- a/passbook/policies/expression/evaluator.py +++ b/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 diff --git a/passbook/policies/expression/forms.py b/passbook/policies/expression/forms.py index edc7a3545..23d2bc47f 100644 --- a/passbook/policies/expression/forms.py +++ b/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"), } diff --git a/passbook/policies/expression/models.py b/passbook/policies/expression/models.py index e43f17560..103be2b69 100644 --- a/passbook/policies/expression/models.py +++ b/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: diff --git a/passbook/policies/expression/tests/test_evaluator.py b/passbook/policies/expression/tests/test_evaluator.py index e39cdfec9..0526d6c9c 100644 --- a/passbook/policies/expression/tests/test_evaluator.py +++ b/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) diff --git a/passbook/policies/process.py b/passbook/policies/process.py index a187627a6..f4af4819e 100644 --- a/passbook/policies/process.py +++ b/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 diff --git a/passbook/providers/saml/forms.py b/passbook/providers/saml/forms.py index 76a56d66a..3a5dd8904 100644 --- a/passbook/providers/saml/forms.py +++ b/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 diff --git a/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py b/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py index 72575b6d6..38a67f5bd 100644 --- a/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py +++ b/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", diff --git a/passbook/providers/saml/processors/base.py b/passbook/providers/saml/processors/base.py index db65e37ff..966f687de 100644 --- a/passbook/providers/saml/processors/base.py +++ b/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) diff --git a/passbook/sources/ldap/connector.py b/passbook/sources/ldap/connector.py index 748c25e9e..98d243bc8 100644 --- a/passbook/sources/ldap/connector.py +++ b/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 diff --git a/passbook/sources/ldap/forms.py b/passbook/sources/ldap/forms.py index 48d71d48a..6536241d8 100644 --- a/passbook/sources/ldap/forms.py +++ b/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 diff --git a/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py b/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py index 318952211..414ca84ef 100644 --- a/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py +++ b/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(): diff --git a/passbook/stages/prompt/tests.py b/passbook/stages/prompt/tests.py index 85b66c1f8..831301acb 100644 --- a/passbook/stages/prompt/tests.py +++ b/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(