From 8b4558fcd0ad09e731bd5ae72a268d4c4c7f4a71 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 29 May 2020 08:53:41 +0200 Subject: [PATCH 01/34] build(deps): bump boto3 from 1.13.18 to 1.13.19 (#41) Bumps [boto3](https://github.com/boto/boto3) from 1.13.18 to 1.13.19. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.18...1.13.19) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 3f4fd3420..7cb97bd17 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,18 +53,17 @@ }, "boto3": { "hashes": [ - "sha256:1bdab4f87ff39d5aab59b0aae69965bf604fa5608984c673877f4c62c1f16240", - "sha256:2b4924ccc1603d562969b9f3c8c74ff4a1f3bdbafe857c990422c73d8e2e229e" + "sha256:c774003dc13d6de74b5e19d2b84d625da4456e64bd97f44baa1fcf40d808d29a" ], "index": "pypi", - "version": "==1.13.18" + "version": "==1.13.19" }, "botocore": { "hashes": [ - "sha256:93574cf95a64c71d35c12c93a23f6214cf2f4b461be3bda3a436381cbe126a84", - "sha256:e65eb27cae262a510e335bc0c0e286e9e42381b1da0aafaa79fa13c1d8d74a95" + "sha256:5cb537e7a4cf2d59a2a8dfbbc8e14ec3bc5b640eb81a1bf3bb0523c0a75e6b1b", + "sha256:7b8b1f082665c8670b9aa70143ee527c5d04939fe027a63ac5958359be20ccb0" ], - "version": "==1.16.18" + "version": "==1.16.19" }, "celery": { "hashes": [ From 4d1658b35ee9f80e3f29c44c13a64225ce038a82 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 31 May 2020 23:01:08 +0200 Subject: [PATCH 02/34] stages/identification: explicitly define enrollment and recovery --- passbook/flows/tests/test_planner.py | 25 ++++++++++- passbook/flows/views.py | 2 +- passbook/stages/identification/api.py | 2 + .../migrations/0002_auto_20200530_2204.py | 41 +++++++++++++++++++ passbook/stages/identification/models.py | 25 ++++++++++- passbook/stages/identification/stage.py | 12 +++--- passbook/stages/identification/tests.py | 16 ++++++-- swagger.yaml | 14 +++++++ 8 files changed, 123 insertions(+), 14 deletions(-) create mode 100644 passbook/stages/identification/migrations/0002_auto_20200530_2204.py diff --git a/passbook/flows/tests/test_planner.py b/passbook/flows/tests/test_planner.py index 25f2bd1a7..df0dc5a7e 100644 --- a/passbook/flows/tests/test_planner.py +++ b/passbook/flows/tests/test_planner.py @@ -1,13 +1,15 @@ """flow planner tests""" from unittest.mock import MagicMock, patch +from django.core.cache import cache from django.shortcuts import reverse from django.test import RequestFactory, TestCase from guardian.shortcuts import get_anonymous_user +from passbook.core.models import User from passbook.flows.exceptions import EmptyFlowException, FlowNonApplicableException from passbook.flows.models import Flow, FlowDesignation, FlowStageBinding -from passbook.flows.planner import FlowPlanner +from passbook.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlanner, cache_key from passbook.policies.types import PolicyResult from passbook.stages.dummy.models import DummyStage @@ -81,3 +83,24 @@ class TestFlowPlanner(TestCase): self.assertEqual( TIME_NOW_MOCK.call_count, 2 ) # When taking from cache, time is not measured + + def test_planner_default_context(self): + """Test planner with default_context""" + flow = Flow.objects.create( + name="test-default-context", + slug="test-default-context", + designation=FlowDesignation.AUTHENTICATION, + ) + FlowStageBinding.objects.create( + flow=flow, stage=DummyStage.objects.create(name="dummy"), order=0 + ) + + user = User.objects.create(username="test-user") + request = self.request_factory.get( + reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), + ) + request.user = user + planner = FlowPlanner(flow) + planner.plan(request, default_context={PLAN_CONTEXT_PENDING_USER: user}) + key = cache_key(flow, user) + self.assertTrue(cache.get(key) is not None) diff --git a/passbook/flows/views.py b/passbook/flows/views.py index 37380a90c..f955106db 100644 --- a/passbook/flows/views.py +++ b/passbook/flows/views.py @@ -34,7 +34,7 @@ class FlowExecutorView(View): def setup(self, request: HttpRequest, flow_slug: str): super().setup(request, flow_slug=flow_slug) - self.flow = get_object_or_404(Flow, slug=flow_slug) + self.flow = get_object_or_404(Flow.objects.select_related(), slug=flow_slug) def handle_invalid_flow(self, exc: BaseException) -> HttpResponse: """When a flow is non-applicable check if user is on the correct domain""" diff --git a/passbook/stages/identification/api.py b/passbook/stages/identification/api.py index bd56a0a61..f40c329e3 100644 --- a/passbook/stages/identification/api.py +++ b/passbook/stages/identification/api.py @@ -16,6 +16,8 @@ class IdentificationStageSerializer(ModelSerializer): "name", "user_fields", "template", + "enrollment_flow", + "recovery_flow", ] diff --git a/passbook/stages/identification/migrations/0002_auto_20200530_2204.py b/passbook/stages/identification/migrations/0002_auto_20200530_2204.py new file mode 100644 index 000000000..8a554f1ee --- /dev/null +++ b/passbook/stages/identification/migrations/0002_auto_20200530_2204.py @@ -0,0 +1,41 @@ +# Generated by Django 3.0.6 on 2020-05-30 22:04 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0002_default_flows"), + ("passbook_stages_identification", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="identificationstage", + name="enrollment_flow", + field=models.ForeignKey( + blank=True, + default=None, + help_text="Optional enrollment flow, which is linked at the bottom of the page.", + null=True, + on_delete=django.db.models.deletion.SET_DEFAULT, + related_name="+", + to="passbook_flows.Flow", + ), + ), + migrations.AddField( + model_name="identificationstage", + name="recovery_flow", + field=models.ForeignKey( + blank=True, + default=None, + help_text="Optional enrollment flow, which is linked at the bottom of the page.", + null=True, + on_delete=django.db.models.deletion.SET_DEFAULT, + related_name="+", + to="passbook_flows.Flow", + ), + ), + ] diff --git a/passbook/stages/identification/models.py b/passbook/stages/identification/models.py index d43b6b15c..94bf0ec48 100644 --- a/passbook/stages/identification/models.py +++ b/passbook/stages/identification/models.py @@ -3,7 +3,7 @@ from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.translation import gettext_lazy as _ -from passbook.flows.models import Stage +from passbook.flows.models import Flow, Stage class UserFields(models.TextChoices): @@ -29,6 +29,29 @@ class IdentificationStage(Stage): ) template = models.TextField(choices=Templates.choices) + enrollment_flow = models.ForeignKey( + Flow, + on_delete=models.SET_DEFAULT, + null=True, + blank=True, + related_name="+", + default=None, + help_text=_( + "Optional enrollment flow, which is linked at the bottom of the page." + ), + ) + recovery_flow = models.ForeignKey( + Flow, + on_delete=models.SET_DEFAULT, + null=True, + blank=True, + related_name="+", + default=None, + help_text=_( + "Optional enrollment flow, which is linked at the bottom of the page." + ), + ) + type = "passbook.stages.identification.stage.IdentificationStageView" form = "passbook.stages.identification.forms.IdentificationStageForm" diff --git a/passbook/stages/identification/stage.py b/passbook/stages/identification/stage.py index c6abe62b6..38189bf1b 100644 --- a/passbook/stages/identification/stage.py +++ b/passbook/stages/identification/stage.py @@ -10,7 +10,6 @@ from django.views.generic import FormView from structlog import get_logger from passbook.core.models import Source, User -from passbook.flows.models import FlowDesignation from passbook.flows.planner import PLAN_CONTEXT_PENDING_USER from passbook.flows.stage import StageView from passbook.stages.identification.forms import IdentificationForm @@ -34,18 +33,17 @@ class IdentificationStageView(FormView, StageView): return [current_stage.template] def get_context_data(self, **kwargs): + current_stage: IdentificationStage = self.executor.current_stage # Check for related enrollment and recovery flow, add URL to view - enrollment_flow = self.executor.flow.related_flow(FlowDesignation.ENROLLMENT) - if enrollment_flow: + if current_stage.enrollment_flow: kwargs["enroll_url"] = reverse( "passbook_flows:flow-executor-shell", - kwargs={"flow_slug": enrollment_flow.slug}, + kwargs={"flow_slug": current_stage.enrollment_flow.slug}, ) - recovery_flow = self.executor.flow.related_flow(FlowDesignation.RECOVERY) - if recovery_flow: + if current_stage.recovery_flow: kwargs["recovery_url"] = reverse( "passbook_flows:flow-executor-shell", - kwargs={"flow_slug": recovery_flow.slug}, + kwargs={"flow_slug": current_stage.recovery_flow.slug}, ) kwargs["primary_action"] = _("Log in") diff --git a/passbook/stages/identification/tests.py b/passbook/stages/identification/tests.py index 4ecf255e9..fa8c3919d 100644 --- a/passbook/stages/identification/tests.py +++ b/passbook/stages/identification/tests.py @@ -85,15 +85,19 @@ class TestIdentificationStage(TestCase): slug="unique-enrollment-string", designation=FlowDesignation.ENROLLMENT, ) + self.stage.enrollment_flow = flow + self.stage.save() FlowStageBinding.objects.create( flow=flow, stage=self.stage, order=0, ) response = self.client.get( - reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), + reverse( + "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} + ), ) self.assertEqual(response.status_code, 200) - self.assertIn(flow.name, response.rendered_content) + self.assertIn(flow.slug, response.rendered_content) def test_recovery_flow(self): """Test that recovery flow is linked correctly""" @@ -102,12 +106,16 @@ class TestIdentificationStage(TestCase): slug="unique-recovery-string", designation=FlowDesignation.RECOVERY, ) + self.stage.recovery_flow = flow + self.stage.save() FlowStageBinding.objects.create( flow=flow, stage=self.stage, order=0, ) response = self.client.get( - reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), + reverse( + "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} + ), ) self.assertEqual(response.status_code, 200) - self.assertIn(flow.name, response.rendered_content) + self.assertIn(flow.slug, response.rendered_content) diff --git a/swagger.yaml b/swagger.yaml index d999299c8..ac38448f0 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -5919,6 +5919,20 @@ definitions: enum: - stages/identification/login.html - stages/identification/recovery.html + enrollment_flow: + title: Enrollment flow + description: Optional enrollment flow, which is linked at the bottom of the + page. + type: string + format: uuid + x-nullable: true + recovery_flow: + title: Recovery flow + description: Optional enrollment flow, which is linked at the bottom of the + page. + type: string + format: uuid + x-nullable: true InvitationStage: required: - name From 6ed822fa38a9f6e6b99ef85b642db217e7ac8bef Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 12:40:08 +0200 Subject: [PATCH 03/34] build(deps-dev): bump autopep8 from 1.5.2 to 1.5.3 (#44) Bumps [autopep8](https://github.com/hhatto/autopep8) from 1.5.2 to 1.5.3. - [Release notes](https://github.com/hhatto/autopep8/releases) - [Commits](https://github.com/hhatto/autopep8/compare/v1.5.2...v1.5.3) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 7cb97bd17..b77c29ade 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,6 +53,7 @@ }, "boto3": { "hashes": [ + "sha256:2e58368d32171f42100353007fb48fe583ea9deafd0ec556aa2bc1ce1582d06d", "sha256:c774003dc13d6de74b5e19d2b84d625da4456e64bd97f44baa1fcf40d808d29a" ], "index": "pypi", @@ -857,10 +858,10 @@ }, "autopep8": { "hashes": [ - "sha256:152fd8fe47d02082be86e05001ec23d6f420086db56b17fc883f3f965fb34954" + "sha256:60fd8c4341bab59963dafd5d2a566e94f547e660b9b396f772afe67d8481dbf0" ], "index": "pypi", - "version": "==1.5.2" + "version": "==1.5.3" }, "bandit": { "hashes": [ @@ -970,10 +971,10 @@ }, "gitpython": { "hashes": [ - "sha256:864a47472548f3ba716ca202e034c1900f197c0fb3a08f641c20c3cafd15ed94", - "sha256:da3b2cf819974789da34f95ac218ef99f515a928685db141327c09b73dd69c09" + "sha256:e107af4d873daed64648b4f4beb89f89f0cfbe3ef558fc7821ed2331c2f8da1a", + "sha256:ef1d60b01b5ce0040ad3ec20bc64f783362d41fa0822a2742d3586e1f49bb8ac" ], - "version": "==3.1.2" + "version": "==3.1.3" }, "isort": { "hashes": [ From 82d12ecfdfdb45e7f933e23a7cd4f6da24dd6430 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 1 Jun 2020 15:25:38 +0200 Subject: [PATCH 04/34] policies/expression: use pb_message() for messages instead of returning a tuple --- passbook/core/models.py | 12 ++- passbook/policies/expression/evaluator.py | 94 ++++++++++++------- passbook/policies/expression/models.py | 4 +- .../expression/tests/test_evaluator.py | 14 ++- 4 files changed, 80 insertions(+), 44 deletions(-) diff --git a/passbook/core/models.py b/passbook/core/models.py index 08ad322dc..13517ea0e 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -13,7 +13,6 @@ from django.utils.translation import gettext_lazy as _ from guardian.mixins import GuardianUserMixin from jinja2 import Undefined from jinja2.exceptions import TemplateSyntaxError, UndefinedError -from jinja2.nativetypes import NativeEnvironment from model_utils.managers import InheritanceManager from structlog import get_logger @@ -24,7 +23,6 @@ from passbook.lib.models import CreatedUpdatedModel from passbook.policies.models import PolicyBindingModel LOGGER = get_logger() -NATIVE_ENVIRONMENT = NativeEnvironment() def default_token_duration(): @@ -208,8 +206,11 @@ 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 + + evaluator = Evaluator() try: - expression = NATIVE_ENVIRONMENT.from_string(self.expression) + expression = evaluator.env.from_string(self.expression) except TemplateSyntaxError as exc: raise PropertyMappingExpressionException from exc try: @@ -221,8 +222,11 @@ class PropertyMapping(models.Model): raise PropertyMappingExpressionException from exc def save(self, *args, **kwargs): + from passbook.policies.expression.evaluator import Evaluator + + evaluator = Evaluator() try: - NATIVE_ENVIRONMENT.from_string(self.expression) + evaluator.env.from_string(self.expression) except TemplateSyntaxError as exc: raise ValidationError("Expression Syntax Error") from exc return super().save(*args, **kwargs) diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index b2120bb74..047c5f1ac 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -1,8 +1,9 @@ """passbook expression policy evaluator""" import re -from typing import TYPE_CHECKING, Any, Dict, Optional +from typing import TYPE_CHECKING, Any, Dict, List, Optional 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 @@ -25,12 +26,32 @@ class Evaluator: _env: NativeEnvironment + _context: Dict[str, Any] + _messages: List[str] + def __init__(self): - self._env = NativeEnvironment() + 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_logger": get_logger(), + "requests": Session(), + } + self._messages = [] + + @property + def env(self) -> NativeEnvironment: + """Access to our custom NativeEnvironment""" + return self._env @staticmethod def jinja2_filter_regex_match(value: Any, regex: str) -> bool: @@ -47,52 +68,57 @@ class Evaluator: """Check if `user` is member of group with name `group_name`""" return user.groups.filter(name=group_name).exists() - def _get_expression_context( - self, request: PolicyRequest, **kwargs - ) -> Dict[str, Any]: - """Return dictionary with additional global variables passed to expression""" + def jinja2_func_message(self, message: str): + """Wrapper to append to messages list, which is returned with PolicyResult""" + self._messages.append(message) + + def set_policy_request(self, request: PolicyRequest): + """Update context based on policy request (if http request is given, update that too)""" # update passbook/policies/expression/templates/policy/expression/form.html # update docs/policies/expression/index.md - kwargs["pb_is_group_member"] = Evaluator.jinja2_func_is_group_member - kwargs["pb_logger"] = get_logger() - kwargs["requests"] = Session() - kwargs["pb_is_sso_flow"] = request.context.get(PLAN_CONTEXT_SSO, False) + self._context["pb_is_sso_flow"] = request.context.get(PLAN_CONTEXT_SSO, False) + self._context["request"] = request if request.http_request: - kwargs["pb_client_ip"] = ( - get_client_ip(request.http_request) or "255.255.255.255" - ) - if SESSION_KEY_PLAN in request.http_request.session: - kwargs["pb_flow_plan"] = request.http_request.session[SESSION_KEY_PLAN] - return kwargs + self.set_http_request(request.http_request) - def evaluate(self, expression_source: str, request: PolicyRequest) -> PolicyResult: - """Parse and evaluate expression. - If the Expression evaluates to a list with 2 items, the first is used as passing bool and - the second as messages. - If the Expression evaluates to a truthy-object, it is used as passing bool.""" + 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["request"] = request + if SESSION_KEY_PLAN in request.http_request.session: + self._context["pb_flow_plan"] = request.http_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) + expression = self._env.from_string(expression_source.lstrip().rstrip()) except TemplateSyntaxError as exc: return PolicyResult(False, str(exc)) try: - result: Optional[Any] = expression.render( - request=request, **self._get_expression_context(request) - ) + 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): LOGGER.warning( "Expression policy returned undefined", src=expression_source, - req=request, + req=self._context, ) - return PolicyResult(False) - if isinstance(result, (list, tuple)) and len(result) == 2: - return PolicyResult(*result) + policy_result.passing = False if result: - return PolicyResult(bool(result)) - return PolicyResult(False) - except Exception as exc: # pylint: disable=broad-except - LOGGER.warning("Expression error", exc=exc) - return PolicyResult(False, str(exc)) + policy_result.passing = bool(result) + return policy_result def validate(self, expression: str): """Validate expression's syntax, raise ValidationError if Syntax is invalid""" diff --git a/passbook/policies/expression/models.py b/passbook/policies/expression/models.py index edf9b629b..e43f17560 100644 --- a/passbook/policies/expression/models.py +++ b/passbook/policies/expression/models.py @@ -16,7 +16,9 @@ class ExpressionPolicy(Policy): def passes(self, request: PolicyRequest) -> PolicyResult: """Evaluate and render expression. Returns PolicyResult(false) on error.""" - return Evaluator().evaluate(self.expression, request) + evaluator = Evaluator() + evaluator.set_policy_request(request) + return evaluator.evaluate(self.expression) def save(self, *args, **kwargs): Evaluator().validate(self.expression) diff --git a/passbook/policies/expression/tests/test_evaluator.py b/passbook/policies/expression/tests/test_evaluator.py index ca22e86ec..e39cdfec9 100644 --- a/passbook/policies/expression/tests/test_evaluator.py +++ b/passbook/policies/expression/tests/test_evaluator.py @@ -17,13 +17,15 @@ class TestEvaluator(TestCase): """test simple value expression""" template = "True" evaluator = Evaluator() - self.assertEqual(evaluator.evaluate(template, self.request).passing, True) + evaluator.set_policy_request(self.request) + self.assertEqual(evaluator.evaluate(template).passing, True) def test_messages(self): """test expression with message return""" - template = "False, 'some message'" + template = '{% do pb_message("some message") %}False' evaluator = Evaluator() - result = evaluator.evaluate(template, self.request) + evaluator.set_policy_request(self.request) + result = evaluator.evaluate(template) self.assertEqual(result.passing, False) self.assertEqual(result.messages, ("some message",)) @@ -31,7 +33,8 @@ class TestEvaluator(TestCase): """test invalid syntax""" template = "{%" evaluator = Evaluator() - result = evaluator.evaluate(template, self.request) + evaluator.set_policy_request(self.request) + result = evaluator.evaluate(template) self.assertEqual(result.passing, False) self.assertEqual(result.messages, ("tag name expected",)) @@ -39,7 +42,8 @@ class TestEvaluator(TestCase): """test undefined result""" template = "{{ foo.bar }}" evaluator = Evaluator() - result = evaluator.evaluate(template, self.request) + evaluator.set_policy_request(self.request) + result = evaluator.evaluate(template) self.assertEqual(result.passing, False) self.assertEqual(result.messages, ("'foo' is undefined",)) From fe1ff7fc7644d414b4c6d489e47bca5440259da8 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 1 Jun 2020 19:07:10 +0200 Subject: [PATCH 05/34] core: fix form not showing general errors --- passbook/core/templates/partials/form.html | 7 +++++++ passbook/policies/expression/evaluator.py | 19 +++++++++++++------ passbook/policies/types.py | 4 +++- passbook/stages/prompt/forms.py | 2 +- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/passbook/core/templates/partials/form.html b/passbook/core/templates/partials/form.html index 83a6e8ae3..efd249d08 100644 --- a/passbook/core/templates/partials/form.html +++ b/passbook/core/templates/partials/form.html @@ -2,6 +2,13 @@ {% load i18n %} {% csrf_token %} +{% if form.non_field_errors %} +
+

+ {{ form.non_field_errors }} +

+
+{% endif %} {% for field in form %}
{% if field.field.widget|fieldtype == 'RadioSelect' %} diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 047c5f1ac..2a0fa8904 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -1,6 +1,6 @@ """passbook expression policy evaluator""" import re -from typing import TYPE_CHECKING, Any, Dict, List, Optional +from typing import Any, Dict, List, Optional from django.core.exceptions import ValidationError from django.http import HttpRequest @@ -10,14 +10,12 @@ 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.utils.http import get_client_ip from passbook.policies.types import PolicyRequest, PolicyResult -if TYPE_CHECKING: - from passbook.core.models import User - LOGGER = get_logger() @@ -43,6 +41,7 @@ class Evaluator: 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(), } @@ -64,7 +63,15 @@ class Evaluator: return re.sub(regex, repl, value) @staticmethod - def jinja2_func_is_group_member(user: "User", group_name: str) -> bool: + 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() @@ -126,4 +133,4 @@ class Evaluator: self._env.from_string(expression) return True except TemplateSyntaxError as exc: - raise ValidationError("Expression Syntax Error") from exc + raise ValidationError(f"Expression Syntax Error: {str(exc)}") from exc diff --git a/passbook/policies/types.py b/passbook/policies/types.py index 1fd65e63b..29bf02932 100644 --- a/passbook/policies/types.py +++ b/passbook/policies/types.py @@ -39,4 +39,6 @@ class PolicyResult: self.messages = messages def __str__(self): - return f"" + if self.messages: + return f"PolicyResult passing={self.passing} messages={self.messages}" + return f"PolicyResult passing={self.passing}" diff --git a/passbook/stages/prompt/forms.py b/passbook/stages/prompt/forms.py index 7347e9053..e9157d198 100644 --- a/passbook/stages/prompt/forms.py +++ b/passbook/stages/prompt/forms.py @@ -64,4 +64,4 @@ class PromptForm(forms.Form): engine.build() result = engine.result if not result.passing: - raise forms.ValidationError(result.messages) + raise forms.ValidationError(list(result.messages)) From c961327d27c2db1c9a5803c2b79e12baa520fe33 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 1 Jun 2020 19:08:14 +0200 Subject: [PATCH 06/34] stages/identification: fix recovery template --- .../stages/identification/recovery.html | 95 +++++-------------- 1 file changed, 26 insertions(+), 69 deletions(-) diff --git a/passbook/stages/identification/templates/stages/identification/recovery.html b/passbook/stages/identification/templates/stages/identification/recovery.html index 1dab0ae77..4c9d07c18 100644 --- a/passbook/stages/identification/templates/stages/identification/recovery.html +++ b/passbook/stages/identification/templates/stages/identification/recovery.html @@ -1,72 +1,29 @@ -{% extends 'base/skeleton.html' %} - -{% load static %} {% load i18n %} +{% load static %} -{% block body %} -
- - - - - - - - - - - + + -{% include 'partials/messages.html' %} - -{% endblock %} +
+ {% if config.login.subtext %} +

{{ config.login.subtext }}

+ {% endif %} +
From 1774e33c2489dda67eb851a5759c7a3b27f7bfae Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2020 09:37:53 +0200 Subject: [PATCH 07/34] build(deps): bump kombu from 4.6.8 to 4.6.9 (#46) Bumps [kombu](https://kombu.readthedocs.io) from 4.6.8 to 4.6.9. Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index b77c29ade..d1618b865 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -18,10 +18,10 @@ "default": { "amqp": { "hashes": [ - "sha256:6e649ca13a7df3faacdc8bbb280aa9a6602d22fd9d545336077e573a1f4ff3b8", - "sha256:77f1aef9410698d20eaeac5b73a87817365f457a507d82edf292e12cbb83b08d" + "sha256:24dbaff8ce4f30566bb88976b398e8c4e77637171af3af6f1b9650f48890e60b", + "sha256:bb68f8d2bced8f93ccfd07d96c689b716b3227720add971be980accfc2952139" ], - "version": "==2.5.2" + "version": "==2.6.0" }, "asgiref": { "hashes": [ @@ -61,10 +61,10 @@ }, "botocore": { "hashes": [ - "sha256:5cb537e7a4cf2d59a2a8dfbbc8e14ec3bc5b640eb81a1bf3bb0523c0a75e6b1b", - "sha256:7b8b1f082665c8670b9aa70143ee527c5d04939fe027a63ac5958359be20ccb0" + "sha256:990f3fc33dec746829740b1a9e1fe86183cdc96aedba6a632ccfcbae03e097cc", + "sha256:d4cc47ac989a7f1d2992ef7679fb423a7966f687becf623a291a555a2d7ce1c0" ], - "version": "==1.16.19" + "version": "==1.16.20" }, "celery": { "hashes": [ @@ -364,11 +364,11 @@ }, "kombu": { "hashes": [ - "sha256:2d1cda774126a044d91a7ff5fa6d09edf99f46924ab332a810760fe6740e9b76", - "sha256:598e7e749d6ab54f646b74b2d2df67755dee13894f73ab02a2a9feb8870c7cb2" + "sha256:ab0afaa5388dd2979cbc439d3623b86a4f7a58d41f621096bef7767c37bc2505", + "sha256:aece08f48706743aaa1b9d607fee300559481eafcc5ee56451aa0ef867a3be07" ], "index": "pypi", - "version": "==4.6.8" + "version": "==4.6.9" }, "ldap3": { "hashes": [ @@ -688,10 +688,10 @@ }, "redis": { "hashes": [ - "sha256:2ef11f489003f151777c064c5dbc6653dfb9f3eade159bcadc524619fddc2242", - "sha256:6d65e84bc58091140081ee9d9c187aab0480097750fac44239307a3bdf0b1251" + "sha256:0e7e0cfca8660dea8b7d5cd8c4f6c5e29e11f31158c0b0ae91a397f00e5a05a2", + "sha256:432b788c4530cfe16d8d943a09d40ca6c16149727e4afe8c2c9d5580c59d9f24" ], - "version": "==3.5.2" + "version": "==3.5.3" }, "requests": { "hashes": [ From 46410428d9a3e384e957d3c6c0c66c375311a173 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2020 09:38:17 +0200 Subject: [PATCH 08/34] build(deps): bump boto3 from 1.13.19 to 1.13.20 (#45) Bumps [boto3](https://github.com/boto/boto3) from 1.13.19 to 1.13.20. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.19...1.13.20) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index d1618b865..06a8d01b3 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,11 +53,11 @@ }, "boto3": { "hashes": [ - "sha256:2e58368d32171f42100353007fb48fe583ea9deafd0ec556aa2bc1ce1582d06d", - "sha256:c774003dc13d6de74b5e19d2b84d625da4456e64bd97f44baa1fcf40d808d29a" + "sha256:26f8564b46d009b8f4c6470a6d6cde147b282a197339c7e31cbb0fe9fd9e5f5d", + "sha256:f59d0bd230ed3a4b932c5c4e497a0e0ff3c93b46b7e8cde54efb6fe10c8266ba" ], "index": "pypi", - "version": "==1.13.19" + "version": "==1.13.20" }, "botocore": { "hashes": [ From 1912b29dc554bf78db8e3f1eb0c7e467b647f78a Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 2 Jun 2020 15:20:02 +0200 Subject: [PATCH 09/34] policies/expression: fix lint error --- passbook/policies/expression/evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 2a0fa8904..01ed3b7ae 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -29,7 +29,7 @@ class Evaluator: def __init__(self): self._env = NativeEnvironment( - extensions=["jinja2.ext.do",], + extensions=["jinja2.ext.do"], trim_blocks=True, lstrip_blocks=True, line_statement_prefix=">", From 07d047c887621309d8e5b6fbcf72625fc6a3b167 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 2 Jun 2020 16:39:55 +0200 Subject: [PATCH 10/34] stages/identification: fix *_flows missing in edit form --- passbook/stages/identification/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passbook/stages/identification/forms.py b/passbook/stages/identification/forms.py index 04217f7f8..882ce0f03 100644 --- a/passbook/stages/identification/forms.py +++ b/passbook/stages/identification/forms.py @@ -16,7 +16,7 @@ class IdentificationStageForm(forms.ModelForm): class Meta: model = IdentificationStage - fields = ["name", "user_fields", "template"] + fields = ["name", "user_fields", "template", "enrollment_flow", "recovery_flow"] widgets = { "name": forms.TextInput(), } From 052bf88c3d983ebb89352516aa1970e688d854b9 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 19:12:55 +0200 Subject: [PATCH 11/34] core: create default user # Conflicts: # README.md --- passbook/core/migrations/0003_default_user.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 passbook/core/migrations/0003_default_user.py diff --git a/passbook/core/migrations/0003_default_user.py b/passbook/core/migrations/0003_default_user.py new file mode 100644 index 000000000..d0363c07c --- /dev/null +++ b/passbook/core/migrations/0003_default_user.py @@ -0,0 +1,28 @@ +# Generated by Django 3.0.6 on 2020-05-23 16:40 + +from django.apps.registry import Apps +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + + +def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): + # User = apps.get_model("passbook_core", "User") + from passbook.core.models import User + + pbadmin = User.objects.create( + username="pbadmin", email="root@localhost", # password="pbadmin" + ) + pbadmin.set_password("pbadmin") # nosec + pbadmin.is_superuser = True + pbadmin.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_core", "0002_auto_20200523_1133"), + ] + + operations = [ + migrations.RunPython(create_default_user), + ] From d4fa60f5095c0c708e58d35d9dfdaa00e11c8c0d Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 20:22:23 +0200 Subject: [PATCH 12/34] core: only show user delete button if an unenrollment flow exists --- passbook/core/templates/user/settings.html | 2 ++ passbook/core/views/user.py | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/passbook/core/templates/user/settings.html b/passbook/core/templates/user/settings.html index edbcc4928..5e752f935 100644 --- a/passbook/core/templates/user/settings.html +++ b/passbook/core/templates/user/settings.html @@ -17,7 +17,9 @@
+ {% if unenrollment_enabled %} {% trans "Delete account" %} + {% endif %}
diff --git a/passbook/core/views/user.py b/passbook/core/views/user.py index bb575ee8e..4bc02b27f 100644 --- a/passbook/core/views/user.py +++ b/passbook/core/views/user.py @@ -1,4 +1,6 @@ """passbook core user views""" +from typing import Any, Dict + from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.messages.views import SuccessMessageMixin from django.urls import reverse_lazy @@ -6,6 +8,7 @@ from django.utils.translation import gettext as _ from django.views.generic import UpdateView from passbook.core.forms.users import UserDetailForm +from passbook.flows.models import Flow, FlowDesignation class UserSettingsView(SuccessMessageMixin, LoginRequiredMixin, UpdateView): @@ -19,3 +22,11 @@ class UserSettingsView(SuccessMessageMixin, LoginRequiredMixin, UpdateView): def get_object(self): return self.request.user + + def get_context_data(self, **kwargs: Dict[str, Any]) -> Dict[str, Any]: + kwargs = super().get_context_data(**kwargs) + unenrollment_flow = Flow.with_policy( + self.request, designation=FlowDesignation.UNRENOLLMENT + ) + kwargs["unenrollment_enabled"] = bool(unenrollment_flow) + return kwargs From 3f92d1c420ada6207216c452a1593d9bd05afcab Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 20:23:09 +0200 Subject: [PATCH 13/34] flows: Correctly check initial policies on flow with context # Conflicts: # passbook/flows/planner.py # passbook/flows/tests/test_planner.py # passbook/flows/tests/test_views.py # passbook/flows/views.py --- ...3_default_user.py => 0002_default_user.py} | 2 +- passbook/flows/models.py | 27 +++++++++++++++++-- passbook/flows/planner.py | 22 +++++++-------- passbook/flows/tests/test_planner.py | 7 +++-- passbook/flows/tests/test_views.py | 7 +++-- passbook/flows/views.py | 6 +++-- 6 files changed, 47 insertions(+), 24 deletions(-) rename passbook/core/migrations/{0003_default_user.py => 0002_default_user.py} (93%) diff --git a/passbook/core/migrations/0003_default_user.py b/passbook/core/migrations/0002_default_user.py similarity index 93% rename from passbook/core/migrations/0003_default_user.py rename to passbook/core/migrations/0002_default_user.py index d0363c07c..66e6a2d3e 100644 --- a/passbook/core/migrations/0003_default_user.py +++ b/passbook/core/migrations/0002_default_user.py @@ -20,7 +20,7 @@ def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): class Migration(migrations.Migration): dependencies = [ - ("passbook_core", "0002_auto_20200523_1133"), + ("passbook_core", "0001_initial"), ] operations = [ diff --git a/passbook/flows/models.py b/passbook/flows/models.py index de0147c99..ffb194386 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -3,12 +3,16 @@ from typing import Optional from uuid import uuid4 from django.db import models +from django.http import HttpRequest from django.utils.translation import gettext_lazy as _ from model_utils.managers import InheritanceManager +from structlog import get_logger from passbook.core.types import UIUserSettings from passbook.policies.models import PolicyBindingModel +LOGGER = get_logger() + class FlowDesignation(models.TextChoices): """Designation of what a Flow should be used for. At a later point, this @@ -62,10 +66,29 @@ class Flow(PolicyBindingModel): PolicyBindingModel, parent_link=True, on_delete=models.CASCADE, related_name="+" ) - def related_flow(self, designation: str) -> Optional["Flow"]: + @staticmethod + def with_policy(request: HttpRequest, **flow_filter) -> Optional["Flow"]: + """Get a Flow by `**flow_filter` and check if the request from `request` can access it.""" + from passbook.policies.engine import PolicyEngine + + flows = Flow.objects.filter(**flow_filter) + for flow in flows: + engine = PolicyEngine(flow, request.user, request) + engine.build() + result = engine.result + if result.passing: + LOGGER.debug("with_policy: flow passing", flow=flow) + return flow + LOGGER.warning( + "with_policy: flow not passing", flow=flow, messages=result.messages + ) + LOGGER.debug("with_policy: no flow found", filters=flow_filter) + return None + + def related_flow(self, designation: str, request: HttpRequest) -> Optional["Flow"]: """Get a related flow with `designation`. Currently this only queries Flows by `designation`, but will eventually use `self` for related lookups.""" - return Flow.objects.filter(designation=designation).first() + return Flow.with_policy(request, designation=designation) def __str__(self) -> str: return f"Flow {self.name} ({self.slug})" diff --git a/passbook/flows/planner.py b/passbook/flows/planner.py index e44e378ee..262279434 100644 --- a/passbook/flows/planner.py +++ b/passbook/flows/planner.py @@ -11,7 +11,6 @@ from passbook.core.models import User from passbook.flows.exceptions import EmptyFlowException, FlowNonApplicableException from passbook.flows.models import Flow, Stage from passbook.policies.engine import PolicyEngine -from passbook.policies.types import PolicyResult LOGGER = get_logger() @@ -52,22 +51,12 @@ class FlowPlanner: self.use_cache = True self.flow = flow - def _check_flow_root_policies(self, request: HttpRequest) -> PolicyResult: - engine = PolicyEngine(self.flow, request.user, request) - engine.build() - return engine.result - def plan( self, request: HttpRequest, default_context: Optional[Dict[str, Any]] = None ) -> FlowPlan: """Check each of the flows' policies, check policies for each stage with PolicyBinding and return ordered list""" LOGGER.debug("f(plan): Starting planning process", flow=self.flow) - # First off, check the flow's direct policy bindings - # to make sure the user even has access to the flow - root_result = self._check_flow_root_policies(request) - if not root_result.passing: - raise FlowNonApplicableException(*root_result.messages) # Bit of a workaround here, if there is a pending user set in the default context # we use that user for our cache key # to make sure they don't get the generic response @@ -75,6 +64,16 @@ class FlowPlanner: user = default_context[PLAN_CONTEXT_PENDING_USER] else: user = request.user + # First off, check the flow's direct policy bindings + # to make sure the user even has access to the flow + engine = PolicyEngine(self.flow, user, request) + if default_context: + engine.request.context = default_context + engine.build() + result = engine.result + if not result.passing: + raise FlowNonApplicableException(result.messages) + # User is passing so far, check if we have a cached plan cached_plan_key = cache_key(self.flow, user) cached_plan = cache.get(cached_plan_key, None) if cached_plan and self.use_cache: @@ -82,6 +81,7 @@ class FlowPlanner: "f(plan): Taking plan from cache", flow=self.flow, key=cached_plan_key ) return cached_plan + LOGGER.debug("f(plan): building plan", flow=self.flow) plan = self._build_plan(user, request, default_context) cache.set(cache_key(self.flow, user), plan) if not plan.stages: diff --git a/passbook/flows/tests/test_planner.py b/passbook/flows/tests/test_planner.py index df0dc5a7e..af6ae98fa 100644 --- a/passbook/flows/tests/test_planner.py +++ b/passbook/flows/tests/test_planner.py @@ -1,5 +1,5 @@ """flow planner tests""" -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, PropertyMock, patch from django.core.cache import cache from django.shortcuts import reverse @@ -13,7 +13,7 @@ from passbook.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlanner, cache from passbook.policies.types import PolicyResult from passbook.stages.dummy.models import DummyStage -POLICY_RESULT_MOCK = MagicMock(return_value=PolicyResult(False)) +POLICY_RESULT_MOCK = PropertyMock(return_value=PolicyResult(False)) TIME_NOW_MOCK = MagicMock(return_value=3) @@ -40,8 +40,7 @@ class TestFlowPlanner(TestCase): planner.plan(request) @patch( - "passbook.flows.planner.FlowPlanner._check_flow_root_policies", - POLICY_RESULT_MOCK, + "passbook.policies.engine.PolicyEngine.result", POLICY_RESULT_MOCK, ) def test_non_applicable_plan(self): """Test that empty plan raises exception""" diff --git a/passbook/flows/tests/test_views.py b/passbook/flows/tests/test_views.py index e6a2ad20c..cacbe2004 100644 --- a/passbook/flows/tests/test_views.py +++ b/passbook/flows/tests/test_views.py @@ -1,5 +1,5 @@ """flow views tests""" -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, PropertyMock, patch from django.shortcuts import reverse from django.test import Client, TestCase @@ -12,7 +12,7 @@ from passbook.lib.config import CONFIG from passbook.policies.types import PolicyResult from passbook.stages.dummy.models import DummyStage -POLICY_RESULT_MOCK = MagicMock(return_value=PolicyResult(False)) +POLICY_RESULT_MOCK = PropertyMock(return_value=PolicyResult(False)) class TestFlowExecutor(TestCase): @@ -45,8 +45,7 @@ class TestFlowExecutor(TestCase): self.assertEqual(cancel_mock.call_count, 1) @patch( - "passbook.flows.planner.FlowPlanner._check_flow_root_policies", - POLICY_RESULT_MOCK, + "passbook.policies.engine.PolicyEngine.result", POLICY_RESULT_MOCK, ) def test_invalid_non_applicable_flow(self): """Tests that a non-applicable flow returns the correct error message""" diff --git a/passbook/flows/views.py b/passbook/flows/views.py index f955106db..5c8c71f11 100644 --- a/passbook/flows/views.py +++ b/passbook/flows/views.py @@ -1,7 +1,7 @@ """passbook multi-stage authentication engine""" from typing import Any, Dict, Optional -from django.http import HttpRequest, HttpResponse +from django.http import Http404, HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, reverse from django.utils.decorators import method_decorator from django.views.decorators.clickjacking import xframe_options_sameorigin @@ -164,7 +164,9 @@ class ToDefaultFlow(View): designation: Optional[FlowDesignation] = None def dispatch(self, request: HttpRequest) -> HttpResponse: - flow = get_object_or_404(Flow, designation=self.designation) + flow = Flow.with_policy(request, designation=self.designation) + if not flow: + raise Http404 # If user already has a pending plan, clear it so we don't have to later. if SESSION_KEY_PLAN in self.request.session: plan: FlowPlan = self.request.session[SESSION_KEY_PLAN] From 7462d561825939c77f75555e1898a15f7b729bae Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 2 Jun 2020 16:56:58 +0200 Subject: [PATCH 14/34] policies/engine: add more verbosity --- passbook/policies/engine.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/passbook/policies/engine.py b/passbook/policies/engine.py index 143ad6473..c465b5ae5 100644 --- a/passbook/policies/engine.py +++ b/passbook/policies/engine.py @@ -74,9 +74,10 @@ class PolicyEngine: for binding in self._iter_bindings(): self._check_policy_type(binding.policy) policy = binding.policy - cached_policy = cache.get(cache_key(binding, self.request.user), None) + key = cache_key(binding, self.request.user) + cached_policy = cache.get(key, None) if cached_policy and self.use_cache: - LOGGER.debug("P_ENG: Taking result from cache", policy=policy) + LOGGER.debug("P_ENG: Taking result from cache", policy=policy, cache_key=key) self.__cached_policies.append(cached_policy) continue LOGGER.debug("P_ENG: Evaluating policy", policy=policy) @@ -103,7 +104,7 @@ class PolicyEngine: x.result for x in self.__processes if x.result ] for result in process_results + self.__cached_policies: - LOGGER.debug("P_ENG: result", passing=result.passing) + LOGGER.debug("P_ENG: result", passing=result.passing, messages=result.messages) if result.messages: messages += result.messages if not result.passing: From 5fc5e54f475c46784a06db653f0d38450a09a5dd Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 20:33:23 +0200 Subject: [PATCH 15/34] sources/oauth: fix typing errors # Conflicts: # passbook/sources/oauth/clients.py --- .github/workflows/ci.yml | 2 ++ passbook/sources/oauth/clients.py | 15 ++++++++++----- passbook/sources/oauth/views/core.py | 4 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b76df232..dbb0e4d62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,8 @@ jobs: python-version: '3.8' - name: Install pyright run: npm install -g pyright + - name: Show pyright version + run: pyright --version - name: Install dependencies run: sudo pip install -U wheel pipenv && pipenv install --dev - name: Lint with pyright diff --git a/passbook/sources/oauth/clients.py b/passbook/sources/oauth/clients.py index 06a9310fc..35c58d7ba 100644 --- a/passbook/sources/oauth/clients.py +++ b/passbook/sources/oauth/clients.py @@ -1,6 +1,6 @@ """OAuth Clients""" import json -from typing import Dict, Optional +from typing import TYPE_CHECKING, Any, Dict, Optional from urllib.parse import parse_qs, urlencode from django.http import HttpRequest @@ -14,24 +14,29 @@ from structlog import get_logger from passbook import __version__ LOGGER = get_logger() +if TYPE_CHECKING: + from passbook.sources.oauth.models import OAuthSource class BaseOAuthClient: """Base OAuth Client""" session: Session + source: "OAuthSource" - def __init__(self, source, token=""): # nosec + def __init__(self, source: "OAuthSource", token=""): # nosec self.source = source self.token = token self.session = Session() self.session.headers.update({"User-Agent": "passbook %s" % __version__}) - def get_access_token(self, request, callback=None): + def get_access_token( + self, request: HttpRequest, callback=None + ) -> Optional[Dict[str, Any]]: "Fetch access token from callback request." raise NotImplementedError("Defined in a sub-class") # pragma: no cover - def get_profile_info(self, token: Dict[str, str]): + def get_profile_info(self, token: Dict[str, str]) -> Optional[Dict[str, Any]]: "Fetch user profile information." try: headers = { @@ -45,7 +50,7 @@ class BaseOAuthClient: LOGGER.warning("Unable to fetch user profile", exc=exc) return None else: - return response.json() or response.text + return response.json() def get_redirect_args(self, request, callback) -> Dict[str, str]: "Get request parameters for redirect url." diff --git a/passbook/sources/oauth/views/core.py b/passbook/sources/oauth/views/core.py index 7e3249bc7..9166ad3b6 100644 --- a/passbook/sources/oauth/views/core.py +++ b/passbook/sources/oauth/views/core.py @@ -21,7 +21,7 @@ from passbook.flows.planner import ( ) from passbook.flows.views import SESSION_KEY_PLAN from passbook.lib.utils.urls import redirect_with_qs -from passbook.sources.oauth.clients import get_client +from passbook.sources.oauth.clients import BaseOAuthClient, get_client from passbook.sources.oauth.models import OAuthSource, UserOAuthSourceConnection from passbook.stages.password.stage import PLAN_CONTEXT_AUTHENTICATION_BACKEND @@ -34,7 +34,7 @@ class OAuthClientMixin: client_class: Optional[Callable] = None - def get_client(self, source): + def get_client(self, source: OAuthSource) -> BaseOAuthClient: "Get instance of the OAuth client for this source." if self.client_class is not None: # pylint: disable=not-callable From ddfa2abbaa5dcaf471185400bab8c94fb65c8884 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 22:02:12 +0200 Subject: [PATCH 16/34] sources/ldap: re-add default PropertyMappings --- .../0003_default_ldap_property_mappings.py | 35 +++++++++ passbook/sources/ldap/tests.py | 75 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py create mode 100644 passbook/sources/ldap/tests.py diff --git a/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py b/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py new file mode 100644 index 000000000..318952211 --- /dev/null +++ b/passbook/sources/ldap/migrations/0003_default_ldap_property_mappings.py @@ -0,0 +1,35 @@ +# Generated by Django 3.0.6 on 2020-05-23 19:30 + +from django.apps.registry import Apps +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 }}", + } + db_alias = schema_editor.connection.alias + for object_field, expression in mapping.items(): + LDAPPropertyMapping.objects.using(db_alias).get_or_create( + expression=expression, + object_field=object_field, + defaults={ + "name": f"Autogenerated LDAP Mapping: {expression} -> {object_field}" + }, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_sources_ldap", "0002_ldapsource_sync_users"), + ] + + operations = [ + migrations.RunPython(create_default_ad_property_mappings), + ] diff --git a/passbook/sources/ldap/tests.py b/passbook/sources/ldap/tests.py new file mode 100644 index 000000000..faa3f4177 --- /dev/null +++ b/passbook/sources/ldap/tests.py @@ -0,0 +1,75 @@ +"""LDAP Source tests""" +from unittest.mock import PropertyMock, patch + +from django.test import TestCase +from ldap3 import MOCK_SYNC, OFFLINE_AD_2012_R2, Connection, Server + +from passbook.core.models import User +from passbook.sources.ldap.connector import Connector +from passbook.sources.ldap.models import LDAPPropertyMapping, LDAPSource + + +def _build_mock_connection() -> Connection: + """Create mock connection""" + server = Server("my_fake_server", get_info=OFFLINE_AD_2012_R2) + _pass = "foo" # noqa # nosec + connection = Connection( + server, + user="cn=my_user,ou=test,o=lab", + password=_pass, + client_strategy=MOCK_SYNC, + ) + connection.strategy.add_entry( + "cn=user0,ou=test,o=lab", + { + "userPassword": "test0000", + "sAMAccountName": "user0_sn", + "revision": 0, + "objectSid": "unique-test0000", + "objectCategory": "Person", + }, + ) + connection.strategy.add_entry( + "cn=user1,ou=test,o=lab", + { + "userPassword": "test1111", + "sAMAccountName": "user1_sn", + "revision": 0, + "objectSid": "unique-test1111", + "objectCategory": "Person", + }, + ) + connection.strategy.add_entry( + "cn=user2,ou=test,o=lab", + { + "userPassword": "test2222", + "sAMAccountName": "user2_sn", + "revision": 0, + "objectSid": "unique-test2222", + "objectCategory": "Person", + }, + ) + connection.bind() + return connection + + +LDAP_CONNECTION_PATCH = PropertyMock(return_value=_build_mock_connection()) + + +class LDAPSourceTests(TestCase): + """LDAP Source tests""" + + def setUp(self): + self.source = LDAPSource.objects.create( + name="ldap", slug="ldap", base_dn="o=lab" + ) + self.source.property_mappings.set(LDAPPropertyMapping.objects.all()) + self.source.save() + + @patch("passbook.sources.ldap.models.LDAPSource.connection", LDAP_CONNECTION_PATCH) + def test_sync_users(self): + """Test user sync""" + connector = Connector(self.source) + connector.sync_users() + user = User.objects.filter(username="user2_sn") + self.assertTrue(user.exists()) From 8080b0380eb9944a36e50c4f51875454dec513d3 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 22:02:23 +0200 Subject: [PATCH 17/34] providers/saml: re-add default PropertyMappings --- .../0002_default_saml_property_mappings.py | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 passbook/providers/saml/migrations/0002_default_saml_property_mappings.py diff --git a/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py b/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py new file mode 100644 index 000000000..72575b6d6 --- /dev/null +++ b/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py @@ -0,0 +1,63 @@ +# Generated by Django 3.0.6 on 2020-05-23 19:32 + +from django.db import migrations + + +def create_default_property_mappings(apps, schema_editor): + """Create default SAML Property Mappings""" + SAMLPropertyMapping = apps.get_model( + "passbook_providers_saml", "SAMLPropertyMapping" + ) + db_alias = schema_editor.connection.alias + defaults = [ + { + "FriendlyName": "eduPersonPrincipalName", + "Name": "urn:oid:1.3.6.1.4.1.5923.1.1.1.6", + "Expression": "{{ user.email }}", + }, + { + "FriendlyName": "cn", + "Name": "urn:oid:2.5.4.3", + "Expression": "{{ user.name }}", + }, + { + "FriendlyName": "mail", + "Name": "urn:oid:0.9.2342.19200300.100.1.3", + "Expression": "{{ user.email }}", + }, + { + "FriendlyName": "displayName", + "Name": "urn:oid:2.16.840.1.113730.3.1.241", + "Expression": "{{ user.username }}", + }, + { + "FriendlyName": "uid", + "Name": "urn:oid:0.9.2342.19200300.100.1.1", + "Expression": "{{ user.pk }}", + }, + { + "FriendlyName": "member-of", + "Name": "member-of", + "Expression": "[{% for group in user.groups.all() %}'{{ group.name }}',{% endfor %}]", + }, + ] + for default in defaults: + SAMLPropertyMapping.objects.using(db_alias).get_or_create( + saml_name=default["Name"], + friendly_name=default["FriendlyName"], + expression=default["Expression"], + defaults={ + "name": f"Autogenerated SAML Mapping: {default['FriendlyName']} -> {default['Expression']}" + }, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_providers_saml", "0001_initial"), + ] + + operations = [ + migrations.RunPython(create_default_property_mappings), + ] From b0ddc6a8c028f02dbb8c87657f49d632577fb474 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 May 2020 01:17:11 +0200 Subject: [PATCH 18/34] admin: fix missing stage count --- .../templates/administration/overview.html | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/passbook/admin/templates/administration/overview.html b/passbook/admin/templates/administration/overview.html index d053f8c3d..21dcb8906 100644 --- a/passbook/admin/templates/administration/overview.html +++ b/passbook/admin/templates/administration/overview.html @@ -55,15 +55,26 @@
- {% if factor_count < 1 %} - {{ factor_count }} + {% if stage_count < 1 %} + {{ stage_count }}

{% trans 'No Stages configured. No Users will be able to login.' %}">

{% else %} - {{ factor_count }} + {{ stage_count }} {% endif %}
+ +
+
+ {% trans 'Flows' %} +
+
+
+ {{ flow_count }} +
+ +
From 2a78d2d0a01e59e720cb799ff7965b5f82d432e1 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 May 2020 01:17:38 +0200 Subject: [PATCH 19/34] crypto: fix being unable to save with private key --- passbook/crypto/forms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/passbook/crypto/forms.py b/passbook/crypto/forms.py index babf25919..79d5f7100 100644 --- a/passbook/crypto/forms.py +++ b/passbook/crypto/forms.py @@ -34,7 +34,6 @@ class CertificateKeyPairForm(forms.ModelForm): password=None, backend=default_backend(), ) - load_pem_x509_certificate(key_data.encode("utf-8"), default_backend()) except ValueError: raise forms.ValidationError("Unable to load private key.") return key_data From f22c89c99832ab37951ebacdf00fb4c3f4f1bfb0 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 May 2020 01:17:52 +0200 Subject: [PATCH 20/34] crypto: re-add default self-signed keypair --- .../migrations/0002_create_self_signed_kp.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 passbook/crypto/migrations/0002_create_self_signed_kp.py diff --git a/passbook/crypto/migrations/0002_create_self_signed_kp.py b/passbook/crypto/migrations/0002_create_self_signed_kp.py new file mode 100644 index 000000000..645db1dfe --- /dev/null +++ b/passbook/crypto/migrations/0002_create_self_signed_kp.py @@ -0,0 +1,28 @@ +# Generated by Django 3.0.6 on 2020-05-23 23:07 + +from django.db import migrations + + +def create_self_signed(apps, schema_editor): + CertificateKeyPair = apps.get_model("passbook_crypto", "CertificateKeyPair") + db_alias = schema_editor.connection.alias + from passbook.crypto.builder import CertificateBuilder + + builder = CertificateBuilder() + builder.build() + CertificateKeyPair.objects.using(db_alias).create( + name="passbook Self-signed Certificate", + certificate_data=builder.certificate, + key_data=builder.private_key, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_crypto', '0001_initial'), + ] + + operations = [ + migrations.RunPython(create_self_signed) + ] From 55fc5a60681ca7bc705d8868eddc75e2f4694995 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 May 2020 02:06:54 +0200 Subject: [PATCH 21/34] policies: rewrite cache_key to prevent wrong cache # Conflicts: # passbook/core/signals.py # passbook/policies/engine.py # passbook/policies/process.py --- passbook/core/signals.py | 24 ------------------ .../migrations/0002_create_self_signed_kp.py | 6 ++--- passbook/policies/apps.py | 6 +++++ passbook/policies/engine.py | 9 +++---- passbook/policies/process.py | 10 +++++--- passbook/policies/signals.py | 25 +++++++++++++++++++ passbook/stages/user_write/tests.py | 2 ++ 7 files changed, 45 insertions(+), 37 deletions(-) create mode 100644 passbook/policies/signals.py diff --git a/passbook/core/signals.py b/passbook/core/signals.py index 01299f90e..74b6b49f1 100644 --- a/passbook/core/signals.py +++ b/passbook/core/signals.py @@ -1,31 +1,7 @@ """passbook core signals""" -from django.core.cache import cache from django.core.signals import Signal -from django.db.models.signals import post_save -from django.dispatch import receiver -from structlog import get_logger - -LOGGER = get_logger() user_signed_up = Signal(providing_args=["request", "user"]) invitation_created = Signal(providing_args=["request", "invitation"]) invitation_used = Signal(providing_args=["request", "invitation", "user"]) password_changed = Signal(providing_args=["user", "password"]) - - -@receiver(post_save) -# pylint: disable=unused-argument -def invalidate_policy_cache(sender, instance, **_): - """Invalidate Policy cache when policy is updated""" - from passbook.policies.models import Policy, PolicyBinding - from passbook.policies.process import cache_key - - if isinstance(instance, Policy): - LOGGER.debug("Invalidating policy cache", policy=instance) - total = 0 - for binding in PolicyBinding.objects.filter(policy=instance): - prefix = cache_key(binding) + "*" - keys = cache.keys(prefix) - total += len(keys) - cache.delete_many(keys) - LOGGER.debug("Deleted keys", len=total) diff --git a/passbook/crypto/migrations/0002_create_self_signed_kp.py b/passbook/crypto/migrations/0002_create_self_signed_kp.py index 645db1dfe..66239b816 100644 --- a/passbook/crypto/migrations/0002_create_self_signed_kp.py +++ b/passbook/crypto/migrations/0002_create_self_signed_kp.py @@ -20,9 +20,7 @@ def create_self_signed(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('passbook_crypto', '0001_initial'), + ("passbook_crypto", "0001_initial"), ] - operations = [ - migrations.RunPython(create_self_signed) - ] + operations = [migrations.RunPython(create_self_signed)] diff --git a/passbook/policies/apps.py b/passbook/policies/apps.py index 5795355b6..946f84609 100644 --- a/passbook/policies/apps.py +++ b/passbook/policies/apps.py @@ -1,4 +1,6 @@ """passbook policies app config""" +from importlib import import_module + from django.apps import AppConfig @@ -8,3 +10,7 @@ class PassbookPoliciesConfig(AppConfig): name = "passbook.policies" label = "passbook_policies" verbose_name = "passbook Policies" + + def ready(self): + """Load source_types from config file""" + import_module("passbook.policies.signals") diff --git a/passbook/policies/engine.py b/passbook/policies/engine.py index c465b5ae5..34dc6d54d 100644 --- a/passbook/policies/engine.py +++ b/passbook/policies/engine.py @@ -73,17 +73,16 @@ class PolicyEngine: """Build task group""" for binding in self._iter_bindings(): self._check_policy_type(binding.policy) - policy = binding.policy - key = cache_key(binding, self.request.user) + key = cache_key(binding, self.request) cached_policy = cache.get(key, None) if cached_policy and self.use_cache: - LOGGER.debug("P_ENG: Taking result from cache", policy=policy, cache_key=key) + LOGGER.debug("P_ENG: Taking result from cache", policy=binding.policy, cache_key=key) self.__cached_policies.append(cached_policy) continue - LOGGER.debug("P_ENG: Evaluating policy", policy=policy) + LOGGER.debug("P_ENG: Evaluating policy", policy=binding.policy) our_end, task_end = Pipe(False) task = PolicyProcess(binding, self.request, task_end) - LOGGER.debug("P_ENG: Starting Process", policy=policy) + LOGGER.debug("P_ENG: Starting Process", policy=binding.policy) task.start() self.__processes.append( PolicyProcessInfo(process=task, connection=our_end, binding=binding) diff --git a/passbook/policies/process.py b/passbook/policies/process.py index 1fb906c9f..3d5ce2525 100644 --- a/passbook/policies/process.py +++ b/passbook/policies/process.py @@ -14,11 +14,13 @@ from passbook.policies.types import PolicyRequest, PolicyResult LOGGER = get_logger() -def cache_key(binding: PolicyBinding, user: Optional[User] = None) -> str: +def cache_key(binding: PolicyBinding, request: PolicyRequest) -> str: """Generate Cache key for policy""" prefix = f"policy_{binding.policy_binding_uuid.hex}_{binding.policy.pk.hex}" - if user: - prefix += f"#{user.pk}" + if request.http_request: + prefix += f"_{request.http_request.session.session_key}" + if request.user: + prefix += f"#{request.user.pk}" return prefix @@ -65,7 +67,7 @@ class PolicyProcess(Process): passing=policy_result.passing, user=self.request.user, ) - key = cache_key(self.binding, self.request.user) + key = cache_key(self.binding, self.request) cache.set(key, policy_result) LOGGER.debug("P_ENG(proc): Cached policy evaluation", key=key) return policy_result diff --git a/passbook/policies/signals.py b/passbook/policies/signals.py new file mode 100644 index 000000000..0f1b1a6d6 --- /dev/null +++ b/passbook/policies/signals.py @@ -0,0 +1,25 @@ +"""passbook policy signals""" +from django.core.cache import cache +from django.db.models.signals import post_save +from django.dispatch import receiver +from structlog import get_logger + +LOGGER = get_logger() + + +@receiver(post_save) +# pylint: disable=unused-argument +def invalidate_policy_cache(sender, instance, **_): + """Invalidate Policy cache when policy is updated""" + from passbook.policies.models import Policy, PolicyBinding + from passbook.policies.process import cache_key + + if isinstance(instance, Policy): + LOGGER.debug("Invalidating policy cache", policy=instance) + total = 0 + for binding in PolicyBinding.objects.filter(policy=instance): + prefix = f"policy_{binding.policy_binding_uuid.hex}_{binding.policy.pk.hex}" + "*" + keys = cache.keys(prefix) + total += len(keys) + cache.delete_many(keys) + LOGGER.debug("Deleted keys", len=total) diff --git a/passbook/stages/user_write/tests.py b/passbook/stages/user_write/tests.py index 5bad06809..d37012207 100644 --- a/passbook/stages/user_write/tests.py +++ b/passbook/stages/user_write/tests.py @@ -72,6 +72,7 @@ class TestUserWriteStage(TestCase): plan.context[PLAN_CONTEXT_PROMPT] = { "username": "test-user-new", "password": new_password, + "some-custom-attribute": "test", } session = self.client.session session[SESSION_KEY_PLAN] = plan @@ -88,6 +89,7 @@ class TestUserWriteStage(TestCase): ) self.assertTrue(user_qs.exists()) self.assertTrue(user_qs.first().check_password(new_password)) + self.assertEqual(user_qs.first().attributes["some-custom-attribute"], "test") def test_without_data(self): """Test without data results in error""" From ef913abc7addce5c92c03b6e32572b9ced9624f4 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 23 May 2020 22:01:38 +0200 Subject: [PATCH 22/34] sources/ldap: add option to disable user sync, move connection init to model --- passbook/policies/engine.py | 10 ++++- passbook/policies/process.py | 1 - passbook/policies/signals.py | 5 ++- passbook/sources/ldap/api.py | 1 + passbook/sources/ldap/connector.py | 41 ++++++++----------- passbook/sources/ldap/forms.py | 1 + .../migrations/0002_ldapsource_sync_users.py | 18 ++++++++ passbook/sources/ldap/models.py | 24 +++++++++++ passbook/sources/ldap/tasks.py | 3 -- swagger.yaml | 7 ++-- 10 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 passbook/sources/ldap/migrations/0002_ldapsource_sync_users.py diff --git a/passbook/policies/engine.py b/passbook/policies/engine.py index 34dc6d54d..5db4f8cfc 100644 --- a/passbook/policies/engine.py +++ b/passbook/policies/engine.py @@ -76,7 +76,11 @@ class PolicyEngine: key = cache_key(binding, self.request) cached_policy = cache.get(key, None) if cached_policy and self.use_cache: - LOGGER.debug("P_ENG: Taking result from cache", policy=binding.policy, cache_key=key) + LOGGER.debug( + "P_ENG: Taking result from cache", + policy=binding.policy, + cache_key=key, + ) self.__cached_policies.append(cached_policy) continue LOGGER.debug("P_ENG: Evaluating policy", policy=binding.policy) @@ -103,7 +107,9 @@ class PolicyEngine: x.result for x in self.__processes if x.result ] for result in process_results + self.__cached_policies: - LOGGER.debug("P_ENG: result", passing=result.passing, messages=result.messages) + LOGGER.debug( + "P_ENG: result", passing=result.passing, messages=result.messages + ) if result.messages: messages += result.messages if not result.passing: diff --git a/passbook/policies/process.py b/passbook/policies/process.py index 3d5ce2525..a187627a6 100644 --- a/passbook/policies/process.py +++ b/passbook/policies/process.py @@ -6,7 +6,6 @@ from typing import Optional from django.core.cache import cache from structlog import get_logger -from passbook.core.models import User from passbook.policies.exceptions import PolicyException from passbook.policies.models import PolicyBinding from passbook.policies.types import PolicyRequest, PolicyResult diff --git a/passbook/policies/signals.py b/passbook/policies/signals.py index 0f1b1a6d6..82e0b3d94 100644 --- a/passbook/policies/signals.py +++ b/passbook/policies/signals.py @@ -12,13 +12,14 @@ LOGGER = get_logger() def invalidate_policy_cache(sender, instance, **_): """Invalidate Policy cache when policy is updated""" from passbook.policies.models import Policy, PolicyBinding - from passbook.policies.process import cache_key if isinstance(instance, Policy): LOGGER.debug("Invalidating policy cache", policy=instance) total = 0 for binding in PolicyBinding.objects.filter(policy=instance): - prefix = f"policy_{binding.policy_binding_uuid.hex}_{binding.policy.pk.hex}" + "*" + prefix = ( + f"policy_{binding.policy_binding_uuid.hex}_{binding.policy.pk.hex}*" + ) keys = cache.keys(prefix) total += len(keys) cache.delete_many(keys) diff --git a/passbook/sources/ldap/api.py b/passbook/sources/ldap/api.py index a51a5ce12..e5ad2677c 100644 --- a/passbook/sources/ldap/api.py +++ b/passbook/sources/ldap/api.py @@ -23,6 +23,7 @@ class LDAPSourceSerializer(ModelSerializer): "group_object_filter", "user_group_membership_field", "object_uniqueness_field", + "sync_users", "sync_groups", "sync_parent_group", "property_mappings", diff --git a/passbook/sources/ldap/connector.py b/passbook/sources/ldap/connector.py index 064a6a628..748c25e9e 100644 --- a/passbook/sources/ldap/connector.py +++ b/passbook/sources/ldap/connector.py @@ -16,26 +16,10 @@ LOGGER = get_logger() class Connector: """Wrapper for ldap3 to easily manage user authentication and creation""" - _server: ldap3.Server - _connection = ldap3.Connection _source: LDAPSource def __init__(self, source: LDAPSource): self._source = source - self._server = ldap3.Server(source.server_uri) # Implement URI parsing - - def bind(self): - """Bind using Source's Credentials""" - self._connection = ldap3.Connection( - self._server, - raise_exceptions=True, - user=self._source.bind_cn, - password=self._source.bind_password, - ) - - self._connection.bind() - if self._source.start_tls: - self._connection.start_tls() @staticmethod def encode_pass(password: str) -> bytes: @@ -45,19 +29,23 @@ class Connector: @property def base_dn_users(self) -> str: """Shortcut to get full base_dn for user lookups""" - return ",".join([self._source.additional_user_dn, self._source.base_dn]) + if self._source.additional_user_dn: + return f"{self._source.additional_user_dn},{self._source.base_dn}" + return self._source.base_dn @property def base_dn_groups(self) -> str: """Shortcut to get full base_dn for group lookups""" - return ",".join([self._source.additional_group_dn, self._source.base_dn]) + if self._source.additional_group_dn: + return f"{self._source.additional_group_dn},{self._source.base_dn}" + return self._source.base_dn def sync_groups(self): """Iterate over all LDAP Groups and create passbook_core.Group instances""" if not self._source.sync_groups: - LOGGER.debug("Group syncing is disabled for this Source") + LOGGER.warning("Group syncing is disabled for this Source") return - groups = self._connection.extend.standard.paged_search( + groups = self._source.connection.extend.standard.paged_search( search_base=self.base_dn_groups, search_filter=self._source.group_object_filter, search_scope=ldap3.SUBTREE, @@ -87,7 +75,10 @@ class Connector: def sync_users(self): """Iterate over all LDAP Users and create passbook_core.User instances""" - users = self._connection.extend.standard.paged_search( + if not self._source.sync_users: + LOGGER.warning("User syncing is disabled for this Source") + return + users = self._source.connection.extend.standard.paged_search( search_base=self.base_dn_users, search_filter=self._source.user_object_filter, search_scope=ldap3.SUBTREE, @@ -101,9 +92,9 @@ class Connector: LOGGER.warning("Cannot find uniqueness Field in attributes") continue try: + defaults = self._build_object_properties(attributes) user, created = User.objects.update_or_create( - attributes__ldap_uniq=uniq, - defaults=self._build_object_properties(attributes), + attributes__ldap_uniq=uniq, defaults=defaults, ) except IntegrityError as exc: LOGGER.warning("Failed to create user", exc=exc) @@ -123,7 +114,7 @@ class Connector: def sync_membership(self): """Iterate over all Users and assign Groups using memberOf Field""" - users = self._connection.extend.standard.paged_search( + users = self._source.connection.extend.standard.paged_search( search_base=self.base_dn_users, search_filter=self._source.user_object_filter, search_scope=ldap3.SUBTREE, @@ -220,7 +211,7 @@ class Connector: LOGGER.debug("Attempting Binding as user", user=user) try: temp_connection = ldap3.Connection( - self._server, + self._source.connection.server, user=user.attributes.get("distinguishedName"), password=password, raise_exceptions=True, diff --git a/passbook/sources/ldap/forms.py b/passbook/sources/ldap/forms.py index 249ebd5af..48d71d48a 100644 --- a/passbook/sources/ldap/forms.py +++ b/passbook/sources/ldap/forms.py @@ -26,6 +26,7 @@ class LDAPSourceForm(forms.ModelForm): "group_object_filter", "user_group_membership_field", "object_uniqueness_field", + "sync_users", "sync_groups", "sync_parent_group", "property_mappings", diff --git a/passbook/sources/ldap/migrations/0002_ldapsource_sync_users.py b/passbook/sources/ldap/migrations/0002_ldapsource_sync_users.py new file mode 100644 index 000000000..27a0da2b3 --- /dev/null +++ b/passbook/sources/ldap/migrations/0002_ldapsource_sync_users.py @@ -0,0 +1,18 @@ +# Generated by Django 3.0.6 on 2020-05-23 19:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_sources_ldap", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="ldapsource", + name="sync_users", + field=models.BooleanField(default=True), + ), + ] diff --git a/passbook/sources/ldap/models.py b/passbook/sources/ldap/models.py index 393cccfa2..34fa96e56 100644 --- a/passbook/sources/ldap/models.py +++ b/passbook/sources/ldap/models.py @@ -1,8 +1,10 @@ """passbook LDAP Models""" +from typing import Optional from django.core.validators import URLValidator from django.db import models from django.utils.translation import gettext_lazy as _ +from ldap3 import Connection, Server from passbook.core.models import Group, PropertyMapping, Source @@ -22,10 +24,12 @@ class LDAPSource(Source): additional_user_dn = models.TextField( help_text=_("Prepended to Base DN for User-queries."), verbose_name=_("Addition User DN"), + blank=True, ) additional_group_dn = models.TextField( help_text=_("Prepended to Base DN for Group-queries."), verbose_name=_("Addition Group DN"), + blank=True, ) user_object_filter = models.TextField( @@ -43,6 +47,7 @@ class LDAPSource(Source): default="objectSid", help_text=_("Field which contains a unique Identifier.") ) + sync_users = models.BooleanField(default=True) sync_groups = models.BooleanField(default=True) sync_parent_group = models.ForeignKey( Group, blank=True, null=True, default=None, on_delete=models.SET_DEFAULT @@ -50,6 +55,25 @@ class LDAPSource(Source): form = "passbook.sources.ldap.forms.LDAPSourceForm" + _connection: Optional[Connection] + + @property + def connection(self) -> Connection: + """Get a fully connected and bound LDAP Connection""" + if not self._connection: + server = Server(self.server_uri) + self._connection = Connection( + server, + raise_exceptions=True, + user=self.bind_cn, + password=self.bind_password, + ) + + self._connection.bind() + if self.start_tls: + self._connection.start_tls() + return self._connection + class Meta: verbose_name = _("LDAP Source") diff --git a/passbook/sources/ldap/tasks.py b/passbook/sources/ldap/tasks.py index 581d27c7a..eeb1cb282 100644 --- a/passbook/sources/ldap/tasks.py +++ b/passbook/sources/ldap/tasks.py @@ -9,7 +9,6 @@ def sync_groups(source_pk: int): """Sync LDAP Groups on background worker""" source = LDAPSource.objects.get(pk=source_pk) connector = Connector(source) - connector.bind() connector.sync_groups() @@ -18,7 +17,6 @@ def sync_users(source_pk: int): """Sync LDAP Users on background worker""" source = LDAPSource.objects.get(pk=source_pk) connector = Connector(source) - connector.bind() connector.sync_users() @@ -27,7 +25,6 @@ def sync(): """Sync all sources""" for source in LDAPSource.objects.filter(enabled=True): connector = Connector(source) - connector.bind() connector.sync_users() connector.sync_groups() connector.sync_membership() diff --git a/swagger.yaml b/swagger.yaml index ac38448f0..31c87f5be 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -5606,8 +5606,6 @@ definitions: - bind_cn - bind_password - base_dn - - additional_user_dn - - additional_group_dn type: object properties: pk: @@ -5654,12 +5652,10 @@ definitions: title: Addition User DN description: Prepended to Base DN for User-queries. type: string - minLength: 1 additional_group_dn: title: Addition Group DN description: Prepended to Base DN for Group-queries. type: string - minLength: 1 user_object_filter: title: User object filter description: Consider Objects matching this filter to be Users. @@ -5680,6 +5676,9 @@ definitions: description: Field which contains a unique Identifier. type: string minLength: 1 + sync_users: + title: Sync users + type: boolean sync_groups: title: Sync groups type: boolean From 448ca62661942804b5103097a2fd9da14d7893eb Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 3 Jun 2020 08:55:23 +0200 Subject: [PATCH 23/34] build(deps): bump kombu from 4.6.9 to 4.6.10 (#48) Bumps [kombu](https://kombu.readthedocs.io) from 4.6.9 to 4.6.10. Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 06a8d01b3..0c7c920b4 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -61,10 +61,10 @@ }, "botocore": { "hashes": [ - "sha256:990f3fc33dec746829740b1a9e1fe86183cdc96aedba6a632ccfcbae03e097cc", - "sha256:d4cc47ac989a7f1d2992ef7679fb423a7966f687becf623a291a555a2d7ce1c0" + "sha256:3837f15e9ec9fd395beb0fb840a5976655c8ebb83121a4612e26f21aad8da908", + "sha256:7bd43e2fdf579875e3d3073e25699f5e524cc36a1748c4aee7c9c626e3760b2b" ], - "version": "==1.16.20" + "version": "==1.16.21" }, "celery": { "hashes": [ @@ -364,11 +364,11 @@ }, "kombu": { "hashes": [ - "sha256:ab0afaa5388dd2979cbc439d3623b86a4f7a58d41f621096bef7767c37bc2505", - "sha256:aece08f48706743aaa1b9d607fee300559481eafcc5ee56451aa0ef867a3be07" + "sha256:437b9cdea193cc2ed0b8044c85fd0f126bb3615ca2f4d4a35b39de7cacfa3c1a", + "sha256:dc282bb277197d723bccda1a9ba30a27a28c9672d0ab93e9e51bb05a37bd29c3" ], "index": "pypi", - "version": "==4.6.9" + "version": "==4.6.10" }, "ldap3": { "hashes": [ From f4be007803bd53b83e62bd8700af53331160b6ed Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 3 Jun 2020 08:56:13 +0200 Subject: [PATCH 24/34] build(deps): bump boto3 from 1.13.20 to 1.13.21 (#47) Bumps [boto3](https://github.com/boto/boto3) from 1.13.20 to 1.13.21. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.20...1.13.21) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 0c7c920b4..c262f51b7 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,11 +53,11 @@ }, "boto3": { "hashes": [ - "sha256:26f8564b46d009b8f4c6470a6d6cde147b282a197339c7e31cbb0fe9fd9e5f5d", - "sha256:f59d0bd230ed3a4b932c5c4e497a0e0ff3c93b46b7e8cde54efb6fe10c8266ba" + "sha256:45e851db4b4ea6cedc9e138f6aa93fedaa5a8e0ae6d8a3f893b3e27c6aff60c8", + "sha256:86de0882ba324ba4ced362b5c94491b8d126d378f2ee24d32486cb64b160a89f" ], "index": "pypi", - "version": "==1.13.20" + "version": "==1.13.21" }, "botocore": { "hashes": [ From 790139f8bc181e68a865841d6921e852f7b53f64 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 3 Jun 2020 21:00:04 +0200 Subject: [PATCH 25/34] stages/email: Add test to check if user is pending --- passbook/stages/email/tests.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/passbook/stages/email/tests.py b/passbook/stages/email/tests.py index 69fd683ac..919548ffa 100644 --- a/passbook/stages/email/tests.py +++ b/passbook/stages/email/tests.py @@ -45,6 +45,19 @@ class TestEmailStage(TestCase): response = self.client.get(url) self.assertEqual(response.status_code, 200) + def test_without_user(self): + """Test without pending user""" + plan = FlowPlan(flow_pk=self.flow.pk.hex, stages=[self.stage]) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + url = reverse( + "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + def test_pending_user(self): """Test with pending user""" plan = FlowPlan(flow_pk=self.flow.pk.hex, stages=[self.stage]) From 90ce704def9b69150fd93539aca8608344c3e9bc Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 4 Jun 2020 09:15:11 +0200 Subject: [PATCH 26/34] build(deps): bump django from 3.0.6 to 3.0.7 (#52) Bumps [django](https://github.com/django/django) from 3.0.6 to 3.0.7. - [Release notes](https://github.com/django/django/releases) - [Commits](https://github.com/django/django/compare/3.0.6...3.0.7) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index c262f51b7..9506a09ac 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -61,10 +61,10 @@ }, "botocore": { "hashes": [ - "sha256:3837f15e9ec9fd395beb0fb840a5976655c8ebb83121a4612e26f21aad8da908", - "sha256:7bd43e2fdf579875e3d3073e25699f5e524cc36a1748c4aee7c9c626e3760b2b" + "sha256:5df4dc49392922dc23042333187ca33686552ae5a901e91d0bc4140c4226995c", + "sha256:eb077e2da1654558c9d97888a2e0710b92cc3162a6641959545451e1a21eea8a" ], - "version": "==1.16.21" + "version": "==1.16.22" }, "celery": { "hashes": [ @@ -169,11 +169,11 @@ }, "django": { "hashes": [ - "sha256:051ba55d42daa3eeda3944a8e4df2bc96d4c62f94316dea217248a22563c3621", - "sha256:9aaa6a09678e1b8f0d98a948c56482eac3e3dd2ddbfb8de70a868135ef3b5e01" + "sha256:5052b34b34b3425233c682e0e11d658fd6efd587d11335a0203d827224ada8f2", + "sha256:e1630333248c9b3d4e38f02093a26f1e07b271ca896d73097457996e0fae12e8" ], "index": "pypi", - "version": "==3.0.6" + "version": "==3.0.7" }, "django-cors-middleware": { "hashes": [ @@ -948,11 +948,11 @@ }, "django": { "hashes": [ - "sha256:051ba55d42daa3eeda3944a8e4df2bc96d4c62f94316dea217248a22563c3621", - "sha256:9aaa6a09678e1b8f0d98a948c56482eac3e3dd2ddbfb8de70a868135ef3b5e01" + "sha256:5052b34b34b3425233c682e0e11d658fd6efd587d11335a0203d827224ada8f2", + "sha256:e1630333248c9b3d4e38f02093a26f1e07b271ca896d73097457996e0fae12e8" ], "index": "pypi", - "version": "==3.0.6" + "version": "==3.0.7" }, "django-debug-toolbar": { "hashes": [ From d18b76a47d05e59e0c2aa14deb7b2295b35d4f66 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 4 Jun 2020 09:15:31 +0200 Subject: [PATCH 27/34] build(deps): bump boto3 from 1.13.21 to 1.13.22 (#51) Bumps [boto3](https://github.com/boto/boto3) from 1.13.21 to 1.13.22. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.21...1.13.22) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 9506a09ac..8d8da73e4 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,11 +53,11 @@ }, "boto3": { "hashes": [ - "sha256:45e851db4b4ea6cedc9e138f6aa93fedaa5a8e0ae6d8a3f893b3e27c6aff60c8", - "sha256:86de0882ba324ba4ced362b5c94491b8d126d378f2ee24d32486cb64b160a89f" + "sha256:9f556ab7124aac29ce64d0dcc2219c14960c5faaee8fe7aef1fd9cf8577b6426", + "sha256:fd0800a3acda1dc15ac4675123f15efb160dc05467df3e19489b3245ea8f689d" ], "index": "pypi", - "version": "==1.13.21" + "version": "==1.13.22" }, "botocore": { "hashes": [ From 5502c319d476292d36a49e18ce3c0c77bdfa8767 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 4 Jun 2020 09:15:42 +0200 Subject: [PATCH 28/34] build(deps): bump celery from 4.4.2 to 4.4.4 (#50) Bumps [celery](https://github.com/celery/celery) from 4.4.2 to 4.4.4. - [Release notes](https://github.com/celery/celery/releases) - [Changelog](https://github.com/celery/celery/blob/master/Changelog.rst) - [Commits](https://github.com/celery/celery/compare/4.4.2...v4.4.4) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 8d8da73e4..1c15bae61 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -68,11 +68,11 @@ }, "celery": { "hashes": [ - "sha256:108a0bf9018a871620936c33a3ee9f6336a89f8ef0a0f567a9001f4aa361415f", - "sha256:5b4b37e276033fe47575107a2775469f0b721646a08c96ec2c61531e4fe45f2a" + "sha256:9ae2e73b93cc7d6b48b56aaf49a68c91752d0ffd7dfdcc47f842ca79a6f13eae", + "sha256:c2037b6a8463da43b19969a0fc13f9023ceca6352b4dd51be01c66fbbb13647e" ], "index": "pypi", - "version": "==4.4.2" + "version": "==4.4.4" }, "certifi": { "hashes": [ From 147212a5f9c5dd6e0ed7e49c3ab2c7966ef3bc53 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 5 Jun 2020 10:27:07 +0200 Subject: [PATCH 29/34] build(deps): bump boto3 from 1.13.22 to 1.13.23 (#53) Bumps [boto3](https://github.com/boto/boto3) from 1.13.22 to 1.13.23. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.22...1.13.23) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- Pipfile.lock | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 1c15bae61..d9cd8623d 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -53,18 +53,18 @@ }, "boto3": { "hashes": [ - "sha256:9f556ab7124aac29ce64d0dcc2219c14960c5faaee8fe7aef1fd9cf8577b6426", - "sha256:fd0800a3acda1dc15ac4675123f15efb160dc05467df3e19489b3245ea8f689d" + "sha256:bcaa88b2f81b88741c47da52f3414c876236700441df87b6198f860e6a200d6f", + "sha256:e974e7a3bbdbd6a73ffc07bea5fa0c0744a5a8b87dcca94702597176e3de465e" ], "index": "pypi", - "version": "==1.13.22" + "version": "==1.13.23" }, "botocore": { "hashes": [ - "sha256:5df4dc49392922dc23042333187ca33686552ae5a901e91d0bc4140c4226995c", - "sha256:eb077e2da1654558c9d97888a2e0710b92cc3162a6641959545451e1a21eea8a" + "sha256:5831068c9b49b4c91b0733e0ec784a7733d8732359d73c67a07a0b0868433cae", + "sha256:7778957bdc9a25dd33bb4383ebd6d45a8570a2cbff03d1edf430fdacec2b7437" ], - "version": "==1.16.22" + "version": "==1.16.23" }, "celery": { "hashes": [ @@ -1133,10 +1133,10 @@ }, "stevedore": { "hashes": [ - "sha256:18afaf1d623af5950cc0f7e75e70f917784c73b652a34a12d90b309451b5500b", - "sha256:a4e7dc759fb0f2e3e2f7d8ffe2358c19d45b9b8297f393ef1256858d82f69c9b" + "sha256:001e90cd704be6470d46cc9076434e2d0d566c1379187e7013eb296d3a6032d9", + "sha256:471c920412265cc809540ae6fb01f3f02aba89c79bbc7091372f4745a50f9691" ], - "version": "==1.32.0" + "version": "==2.0.0" }, "toml": { "hashes": [ From 73116b9d1a0030273c393961f3b633525ecdf434 Mon Sep 17 00:00:00 2001 From: Jens L Date: Fri, 5 Jun 2020 12:00:27 +0200 Subject: [PATCH 30/34] 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 | 1 - Pipfile.lock | 1 - docs/expressions/index.md | 55 ++++++++++ .../reference/user-object.md | 5 +- docs/policies/expression.md | 27 +++++ docs/policies/expression/index.md | 22 ---- docs/policies/index.md | 10 +- docs/property-mappings/expression.md | 9 ++ docs/property-mappings/index.md | 10 +- mkdocs.yml | 17 ++- .../admin/templates/administration/base.html | 2 +- passbook/core/expression.py | 21 ++++ passbook/core/migrations/0002_default_user.py | 1 + passbook/core/models.py | 29 +---- passbook/lib/expression/__init__.py | 0 passbook/lib/expression/evaluator.py | 101 ++++++++++++++++++ passbook/policies/expression/evaluator.py | 99 +++-------------- passbook/policies/expression/forms.py | 8 +- passbook/policies/expression/models.py | 8 +- .../expression/tests/test_evaluator.py | 26 ++--- passbook/policies/process.py | 2 + passbook/providers/saml/forms.py | 8 ++ .../0002_default_saml_property_mappings.py | 10 +- passbook/providers/saml/processors/base.py | 7 +- passbook/sources/ldap/connector.py | 7 +- passbook/sources/ldap/forms.py | 8 ++ .../0003_default_ldap_property_mappings.py | 10 +- passbook/stages/prompt/tests.py | 8 +- 28 files changed, 322 insertions(+), 190 deletions(-) create mode 100644 docs/expressions/index.md rename docs/{property-mappings => expressions}/reference/user-object.md (84%) create mode 100644 docs/policies/expression.md delete mode 100644 docs/policies/expression/index.md create mode 100644 docs/property-mappings/expression.md create mode 100644 passbook/core/expression.py create mode 100644 passbook/lib/expression/__init__.py create mode 100644 passbook/lib/expression/evaluator.py 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( From 7067d1f236f0837c5f9be104f2a51c84f48726fb Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Fri, 5 Jun 2020 12:10:28 +0200 Subject: [PATCH 31/34] docs: fix typo'd URL --- docs/property-mappings/expression.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/property-mappings/expression.md b/docs/property-mappings/expression.md index 9f43de0eb..a25ee2aad 100644 --- a/docs/property-mappings/expression.md +++ b/docs/property-mappings/expression.md @@ -4,6 +4,6 @@ The property mapping should return a value that is expected by the Provider/Sour ### Context Variables -- `user`: The current user, this might be `None` if there is no contextual user. ([ref](../expression/reference/user-object.md)) +- `user`: The current user, this might be `None` if there is no contextual user. ([ref](../expressions/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. From c62794c738d92d4d2456a30b9b4d8db4bdbe0a6f Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 May 2020 16:58:01 +0200 Subject: [PATCH 32/34] admin: fix PropertyMapping widget not rendering properly --- passbook/core/migrations/0002_default_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passbook/core/migrations/0002_default_user.py b/passbook/core/migrations/0002_default_user.py index 82a8d6d35..1dd6ecdd0 100644 --- a/passbook/core/migrations/0002_default_user.py +++ b/passbook/core/migrations/0002_default_user.py @@ -10,7 +10,7 @@ def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): from passbook.core.models import User pbadmin = User.objects.create( - username="pbadmin", email="root@localhost", # password="pbadmin" + username="pbadmin", email="root@localhost", name="passbook Default Admin" ) pbadmin.set_password("pbadmin") # nosec pbadmin.is_superuser = True From f91e02a0ec791b2d12ea14985441494f3b827020 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Fri, 5 Jun 2020 20:30:47 +0200 Subject: [PATCH 33/34] flows: allow username for default flow --- passbook/flows/migrations/0002_default_flows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/passbook/flows/migrations/0002_default_flows.py b/passbook/flows/migrations/0002_default_flows.py index 21d61d338..5977cc8ee 100644 --- a/passbook/flows/migrations/0002_default_flows.py +++ b/passbook/flows/migrations/0002_default_flows.py @@ -31,7 +31,7 @@ def create_default_authentication_flow( if not IdentificationStage.objects.using(db_alias).exists(): IdentificationStage.objects.using(db_alias).create( name="identification", - user_fields=[UserFields.E_MAIL], + user_fields=[UserFields.E_MAIL, UserFields.USERNAME], template=Templates.DEFAULT_LOGIN, ) @@ -44,7 +44,7 @@ def create_default_authentication_flow( UserLoginStage.objects.using(db_alias).create(name="authentication") flow = Flow.objects.using(db_alias).create( - name="default-authentication-flow", + name="Welcome to passbook!", slug="default-authentication-flow", designation=FlowDesignation.AUTHENTICATION, ) From 49152056785427ad3ba7cef05bfd466e0fd2d890 Mon Sep 17 00:00:00 2001 From: Jens L Date: Sun, 7 Jun 2020 16:35:08 +0200 Subject: [PATCH 34/34] WIP Use Flows for Sources and Providers (#32) * core: start migrating to flows for authorisation * sources/oauth: start type-hinting * core: create default user * core: only show user delete button if an unenrollment flow exists * flows: Correctly check initial policies on flow with context * policies: add more verbosity to engine * sources/oauth: migrate to flows * sources/oauth: fix typing errors * flows: add more tests * sources/oauth: start implementing unittests * sources/ldap: add option to disable user sync, move connection init to model * sources/ldap: re-add default PropertyMappings * providers/saml: re-add default PropertyMappings * admin: fix missing stage count * stages/identification: fix sources not being shown * crypto: fix being unable to save with private key * crypto: re-add default self-signed keypair * policies: rewrite cache_key to prevent wrong cache * sources/saml: migrate to flows for auth and enrollment * stages/consent: add new stage * admin: fix PropertyMapping widget not rendering properly * core: provider.authorization_flow is mandatory * flows: add support for "autosubmit" attribute on form * flows: add InMemoryStage for dynamic stages * flows: optionally allow empty flows from FlowPlanner * providers/saml: update to authorization_flow * sources/*: fix flow executor URL * flows: fix pylint error * flows: wrap responses in JSON object to easily handle redirects * flow: dont cache plan's context * providers/oauth: rewrite OAuth2 Provider to use flows * providers/*: update docstrings of models * core: fix forms not passing help_text through safe * flows: fix HttpResponses not being converted to JSON * providers/oidc: rewrite to use flows * flows: fix linting --- passbook/admin/forms/source.py | 17 +- .../administration/provider/list.html | 2 +- passbook/admin/templates/generic/create.html | 4 +- passbook/core/api/applications.py | 1 - passbook/core/api/providers.py | 2 +- passbook/core/forms/applications.py | 1 - .../migrations/0002_auto_20200523_1133.py | 52 +++ ...2_default_user.py => 0003_default_user.py} | 4 +- passbook/core/models.py | 28 +- passbook/core/templates/login/base.html | 37 ++ .../templates/partials/form_horizontal.html | 7 +- passbook/core/views/access.py | 2 +- passbook/core/views/overview.py | 4 +- passbook/crypto/builder.py | 8 +- .../migrations/0003_auto_20200523_1133.py | 29 ++ .../flows/migrations/0004_source_flows.py | 131 +++++++ .../flows/migrations/0005_provider_flows.py | 44 +++ passbook/flows/models.py | 12 +- passbook/flows/planner.py | 8 +- passbook/flows/templates/flows/shell.html | 51 ++- passbook/flows/views.py | 36 +- passbook/policies/types.py | 3 + passbook/providers/app_gw/forms.py | 2 +- passbook/providers/app_gw/models.py | 3 +- passbook/providers/oauth/forms.py | 11 + passbook/providers/oauth/models.py | 3 +- passbook/providers/oauth/settings.py | 1 + passbook/providers/oauth/urls.py | 12 +- passbook/providers/oauth/views/github.py | 39 +- passbook/providers/oauth/views/oauth2.py | 144 ++++--- passbook/providers/oidc/apps.py | 5 +- passbook/providers/oidc/auth.py | 7 +- passbook/providers/oidc/forms.py | 24 +- passbook/providers/oidc/models.py | 8 +- passbook/providers/oidc/signals.py | 15 - passbook/providers/oidc/urls.py | 13 + passbook/providers/oidc/views.py | 127 ++++++ passbook/providers/saml/forms.py | 6 + .../0002_default_saml_property_mappings.py | 12 +- .../0003_samlprovider_sp_binding.py | 20 + passbook/providers/saml/models.py | 14 +- .../templates/saml/idp/autosubmit_form.html | 30 +- .../saml/templates/saml/idp/login.html | 26 -- .../saml/templates/saml/xml/metadata.xml | 5 +- passbook/providers/saml/urls.py | 31 +- passbook/providers/saml/views.py | 371 ++++++++---------- passbook/root/settings.py | 1 + .../migrations/0004_auto_20200524_1146.py | 31 ++ passbook/sources/oauth/forms.py | 8 + passbook/sources/oauth/tests.py | 38 ++ passbook/sources/oauth/types/azure_ad.py | 14 +- passbook/sources/oauth/types/manager.py | 4 +- passbook/sources/oauth/types/oidc.py | 4 +- passbook/sources/oauth/views/core.py | 79 ++-- passbook/sources/saml/forms.py | 1 + .../migrations/0002_auto_20200523_2329.py | 30 ++ passbook/sources/saml/models.py | 20 +- passbook/sources/saml/processors/base.py | 58 ++- passbook/sources/saml/views.py | 35 +- passbook/stages/captcha/tests.py | 8 +- passbook/stages/consent/__init__.py | 0 passbook/stages/consent/api.py | 21 + passbook/stages/consent/apps.py | 10 + passbook/stages/consent/forms.py | 20 + .../stages/consent/migrations/0001_initial.py | 37 ++ .../stages/consent/migrations/__init__.py | 0 passbook/stages/consent/models.py | 19 + passbook/stages/consent/stage.py | 25 ++ passbook/stages/consent/tests.py | 47 +++ passbook/stages/dummy/tests.py | 8 +- passbook/stages/email/tests.py | 9 +- passbook/stages/identification/tests.py | 12 +- passbook/stages/invitation/tests.py | 25 +- passbook/stages/password/tests.py | 27 +- passbook/stages/prompt/tests.py | 14 +- passbook/stages/user_delete/tests.py | 15 +- passbook/stages/user_login/tests.py | 25 +- passbook/stages/user_logout/tests.py | 9 +- passbook/stages/user_write/tests.py | 23 +- pyproject.toml | 2 + swagger.yaml | 47 ++- 81 files changed, 1609 insertions(+), 529 deletions(-) create mode 100644 passbook/core/migrations/0002_auto_20200523_1133.py rename passbook/core/migrations/{0002_default_user.py => 0003_default_user.py} (87%) create mode 100644 passbook/flows/migrations/0003_auto_20200523_1133.py create mode 100644 passbook/flows/migrations/0004_source_flows.py create mode 100644 passbook/flows/migrations/0005_provider_flows.py delete mode 100644 passbook/providers/oidc/signals.py create mode 100644 passbook/providers/oidc/urls.py create mode 100644 passbook/providers/oidc/views.py create mode 100644 passbook/providers/saml/migrations/0003_samlprovider_sp_binding.py delete mode 100644 passbook/providers/saml/templates/saml/idp/login.html create mode 100644 passbook/sources/ldap/migrations/0004_auto_20200524_1146.py create mode 100644 passbook/sources/oauth/tests.py create mode 100644 passbook/sources/saml/migrations/0002_auto_20200523_2329.py create mode 100644 passbook/stages/consent/__init__.py create mode 100644 passbook/stages/consent/api.py create mode 100644 passbook/stages/consent/apps.py create mode 100644 passbook/stages/consent/forms.py create mode 100644 passbook/stages/consent/migrations/0001_initial.py create mode 100644 passbook/stages/consent/migrations/__init__.py create mode 100644 passbook/stages/consent/models.py create mode 100644 passbook/stages/consent/stage.py create mode 100644 passbook/stages/consent/tests.py create mode 100644 pyproject.toml diff --git a/passbook/admin/forms/source.py b/passbook/admin/forms/source.py index b18d7157a..2207dec1c 100644 --- a/passbook/admin/forms/source.py +++ b/passbook/admin/forms/source.py @@ -1,4 +1,17 @@ """passbook core source form fields""" -SOURCE_FORM_FIELDS = ["name", "slug", "enabled"] -SOURCE_SERIALIZER_FIELDS = ["pk", "name", "slug", "enabled"] +SOURCE_FORM_FIELDS = [ + "name", + "slug", + "enabled", + "authentication_flow", + "enrollment_flow", +] +SOURCE_SERIALIZER_FIELDS = [ + "pk", + "name", + "slug", + "enabled", + "authentication_flow", + "enrollment_flow", +] diff --git a/passbook/admin/templates/administration/provider/list.html b/passbook/admin/templates/administration/provider/list.html index 9ae3a7f20..5a804bfd6 100644 --- a/passbook/admin/templates/administration/provider/list.html +++ b/passbook/admin/templates/administration/provider/list.html @@ -29,7 +29,7 @@ {% for type, name in types.items %}
  • - {{ name|verbose_name }} + {{ name|verbose_name }}
    {{ name|doc }} diff --git a/passbook/admin/templates/generic/create.html b/passbook/admin/templates/generic/create.html index 3094c2da7..640ab3916 100644 --- a/passbook/admin/templates/generic/create.html +++ b/passbook/admin/templates/generic/create.html @@ -5,14 +5,14 @@ {% block above_form %}

    - {% blocktrans with type=form|form_verbose_name|title %} + {% blocktrans with type=form|form_verbose_name %} Create {{ type }} {% endblocktrans %}

    {% endblock %} {% block action %} -{% blocktrans with type=form|form_verbose_name|title %} +{% blocktrans with type=form|form_verbose_name %} Create {{ type }} {% endblocktrans %} {% endblock %} diff --git a/passbook/core/api/applications.py b/passbook/core/api/applications.py index f0b4c69e2..f2bcb5e9c 100644 --- a/passbook/core/api/applications.py +++ b/passbook/core/api/applications.py @@ -15,7 +15,6 @@ class ApplicationSerializer(ModelSerializer): "pk", "name", "slug", - "skip_authorization", "provider", "meta_launch_url", "meta_icon_url", diff --git a/passbook/core/api/providers.py b/passbook/core/api/providers.py index 2b54a5807..c2830c06e 100644 --- a/passbook/core/api/providers.py +++ b/passbook/core/api/providers.py @@ -17,7 +17,7 @@ class ProviderSerializer(ModelSerializer): class Meta: model = Provider - fields = ["pk", "property_mappings", "__type__"] + fields = ["pk", "authorization_flow", "property_mappings", "__type__"] class ProviderViewSet(ReadOnlyModelViewSet): diff --git a/passbook/core/forms/applications.py b/passbook/core/forms/applications.py index e63eeb06d..4eb071719 100644 --- a/passbook/core/forms/applications.py +++ b/passbook/core/forms/applications.py @@ -19,7 +19,6 @@ class ApplicationForm(forms.ModelForm): fields = [ "name", "slug", - "skip_authorization", "provider", "meta_launch_url", "meta_icon_url", diff --git a/passbook/core/migrations/0002_auto_20200523_1133.py b/passbook/core/migrations/0002_auto_20200523_1133.py new file mode 100644 index 000000000..393a9be75 --- /dev/null +++ b/passbook/core/migrations/0002_auto_20200523_1133.py @@ -0,0 +1,52 @@ +# Generated by Django 3.0.6 on 2020-05-23 11:33 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0003_auto_20200523_1133"), + ("passbook_core", "0001_initial"), + ] + + operations = [ + migrations.RemoveField(model_name="application", name="skip_authorization",), + migrations.AddField( + model_name="source", + name="authentication_flow", + field=models.ForeignKey( + blank=True, + default=None, + help_text="Flow to use when authenticating existing users.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="source_authentication", + to="passbook_flows.Flow", + ), + ), + migrations.AddField( + model_name="source", + name="enrollment_flow", + field=models.ForeignKey( + blank=True, + default=None, + help_text="Flow to use when enrolling new users.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="source_enrollment", + to="passbook_flows.Flow", + ), + ), + migrations.AddField( + model_name="provider", + name="authorization_flow", + field=models.ForeignKey( + help_text="Flow used when authorizing this provider.", + on_delete=django.db.models.deletion.CASCADE, + related_name="provider_authorization", + to="passbook_flows.Flow", + ), + ), + ] diff --git a/passbook/core/migrations/0002_default_user.py b/passbook/core/migrations/0003_default_user.py similarity index 87% rename from passbook/core/migrations/0002_default_user.py rename to passbook/core/migrations/0003_default_user.py index 1dd6ecdd0..e1d52f3b4 100644 --- a/passbook/core/migrations/0002_default_user.py +++ b/passbook/core/migrations/0003_default_user.py @@ -12,7 +12,7 @@ def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): pbadmin = User.objects.create( username="pbadmin", email="root@localhost", name="passbook Default Admin" ) - pbadmin.set_password("pbadmin") # nosec + pbadmin.set_password("pbadmin") # noqa # nosec pbadmin.is_superuser = True pbadmin.is_staff = True pbadmin.save() @@ -21,7 +21,7 @@ def create_default_user(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): class Migration(migrations.Migration): dependencies = [ - ("passbook_core", "0001_initial"), + ("passbook_core", "0002_auto_20200523_1133"), ] operations = [ diff --git a/passbook/core/models.py b/passbook/core/models.py index 4e42ba6e9..2c8a8930e 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -16,6 +16,7 @@ from structlog import get_logger from passbook.core.exceptions import PropertyMappingExpressionException from passbook.core.signals import password_changed from passbook.core.types import UILoginButton, UIUserSettings +from passbook.flows.models import Flow from passbook.lib.models import CreatedUpdatedModel from passbook.policies.models import PolicyBindingModel @@ -75,6 +76,13 @@ class User(GuardianUserMixin, AbstractUser): class Provider(models.Model): """Application-independent Provider instance. For example SAML2 Remote, OAuth2 Application""" + authorization_flow = models.ForeignKey( + Flow, + on_delete=models.CASCADE, + help_text=_("Flow used when authorizing this provider."), + related_name="provider_authorization", + ) + property_mappings = models.ManyToManyField( "PropertyMapping", default=None, blank=True ) @@ -95,7 +103,6 @@ class Application(PolicyBindingModel): name = models.TextField(help_text=_("Application's display Name.")) slug = models.SlugField(help_text=_("Internal application name, used in URLs.")) - skip_authorization = models.BooleanField(default=False) provider = models.OneToOneField( "Provider", null=True, blank=True, default=None, on_delete=models.SET_DEFAULT ) @@ -128,6 +135,25 @@ class Source(PolicyBindingModel): "PropertyMapping", default=None, blank=True ) + authentication_flow = models.ForeignKey( + Flow, + blank=True, + null=True, + default=None, + on_delete=models.SET_NULL, + help_text=_("Flow to use when authenticating existing users."), + related_name="source_authentication", + ) + enrollment_flow = models.ForeignKey( + Flow, + blank=True, + null=True, + default=None, + on_delete=models.SET_NULL, + help_text=_("Flow to use when enrolling new users."), + related_name="source_enrollment", + ) + form = "" # ModelForm-based class ued to create/edit instance objects = InheritanceManager() diff --git a/passbook/core/templates/login/base.html b/passbook/core/templates/login/base.html index b7234adaa..5c8000887 100644 --- a/passbook/core/templates/login/base.html +++ b/passbook/core/templates/login/base.html @@ -20,3 +20,40 @@ {% endblock %}
    • + diff --git a/passbook/core/templates/partials/form_horizontal.html b/passbook/core/templates/partials/form_horizontal.html index facce648e..1e2836e6e 100644 --- a/passbook/core/templates/partials/form_horizontal.html +++ b/passbook/core/templates/partials/form_horizontal.html @@ -28,6 +28,9 @@
    {{ field|css_class:"pf-c-form-control" }} + {% if field.help_text %} +

    {{ field.help_text|safe }}

    + {% endif %}
    {% elif field.field.widget|fieldtype == 'CheckboxInput' %}
    @@ -36,7 +39,7 @@
    {% if field.help_text %} -

    {{ field.help_text }}

    +

    {{ field.help_text|safe }}

    {% endif %}
    {% else %} @@ -49,7 +52,7 @@
    {{ field|css_class:'pf-c-form-control' }} {% if field.help_text %} -

    {{ field.help_text }}

    +

    {{ field.help_text|safe }}

    {% endif %}
    {% endif %} diff --git a/passbook/core/views/access.py b/passbook/core/views/access.py index c2e07dd19..287a12f80 100644 --- a/passbook/core/views/access.py +++ b/passbook/core/views/access.py @@ -35,6 +35,6 @@ class AccessMixin: def user_has_access(self, application: Application, user: User) -> PolicyResult: """Check if user has access to application.""" LOGGER.debug("Checking permissions", user=user, application=application) - policy_engine = PolicyEngine(application.policies.all(), user, self.request) + policy_engine = PolicyEngine(application, user, self.request) policy_engine.build() return policy_engine.result diff --git a/passbook/core/views/overview.py b/passbook/core/views/overview.py index 6185dce05..ef5186ac6 100644 --- a/passbook/core/views/overview.py +++ b/passbook/core/views/overview.py @@ -16,9 +16,7 @@ class OverviewView(LoginRequiredMixin, TemplateView): def get_context_data(self, **kwargs): kwargs["applications"] = [] for application in Application.objects.all().order_by("name"): - engine = PolicyEngine( - application.policies.all(), self.request.user, self.request - ) + engine = PolicyEngine(application, self.request.user, self.request) engine.build() if engine.passing: kwargs["applications"].append(application) diff --git a/passbook/crypto/builder.py b/passbook/crypto/builder.py index 67545f5bb..47722a463 100644 --- a/passbook/crypto/builder.py +++ b/passbook/crypto/builder.py @@ -36,11 +36,11 @@ class CertificateBuilder: x509.Name( [ x509.NameAttribute( - NameOID.COMMON_NAME, u"passbook Self-signed Certificate", + NameOID.COMMON_NAME, "passbook Self-signed Certificate", ), - x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"passbook"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "passbook"), x509.NameAttribute( - NameOID.ORGANIZATIONAL_UNIT_NAME, u"Self-signed" + NameOID.ORGANIZATIONAL_UNIT_NAME, "Self-signed" ), ] ) @@ -49,7 +49,7 @@ class CertificateBuilder: x509.Name( [ x509.NameAttribute( - NameOID.COMMON_NAME, u"passbook Self-signed Certificate", + NameOID.COMMON_NAME, "passbook Self-signed Certificate", ), ] ) diff --git a/passbook/flows/migrations/0003_auto_20200523_1133.py b/passbook/flows/migrations/0003_auto_20200523_1133.py new file mode 100644 index 000000000..951835239 --- /dev/null +++ b/passbook/flows/migrations/0003_auto_20200523_1133.py @@ -0,0 +1,29 @@ +# Generated by Django 3.0.6 on 2020-05-23 11:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0002_default_flows"), + ] + + operations = [ + migrations.AlterField( + model_name="flow", + name="designation", + field=models.CharField( + choices=[ + ("authentication", "Authentication"), + ("authorization", "Authorization"), + ("invalidation", "Invalidation"), + ("enrollment", "Enrollment"), + ("unenrollment", "Unrenollment"), + ("recovery", "Recovery"), + ("password_change", "Password Change"), + ], + max_length=100, + ), + ), + ] diff --git a/passbook/flows/migrations/0004_source_flows.py b/passbook/flows/migrations/0004_source_flows.py new file mode 100644 index 000000000..97db472d6 --- /dev/null +++ b/passbook/flows/migrations/0004_source_flows.py @@ -0,0 +1,131 @@ +# Generated by Django 3.0.6 on 2020-05-23 15:47 + +from django.apps.registry import Apps +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +from passbook.flows.models import FlowDesignation +from passbook.stages.prompt.models import FieldTypes + +FLOW_POLICY_EXPRESSION = """{{ pb_is_sso_flow }}""" + +PROMPT_POLICY_EXPRESSION = """ +{% if pb_flow_plan.context.prompt_data.username %} +False +{% else %} +True +{% endif %} +""" + + +def create_default_source_enrollment_flow( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +): + Flow = apps.get_model("passbook_flows", "Flow") + FlowStageBinding = apps.get_model("passbook_flows", "FlowStageBinding") + PolicyBinding = apps.get_model("passbook_policies", "PolicyBinding") + + ExpressionPolicy = apps.get_model( + "passbook_policies_expression", "ExpressionPolicy" + ) + + PromptStage = apps.get_model("passbook_stages_prompt", "PromptStage") + Prompt = apps.get_model("passbook_stages_prompt", "Prompt") + UserWriteStage = apps.get_model("passbook_stages_user_write", "UserWriteStage") + UserLoginStage = apps.get_model("passbook_stages_user_login", "UserLoginStage") + + db_alias = schema_editor.connection.alias + + # Create a policy that only allows this flow when doing an SSO Request + flow_policy = ExpressionPolicy.objects.create( + name="default-source-enrollment-if-sso", expression=FLOW_POLICY_EXPRESSION + ) + + # This creates a Flow used by sources to enroll users + # It makes sure that a username is set, and if not, prompts the user for a Username + flow = Flow.objects.create( + name="default-source-enrollment", + slug="default-source-enrollment", + designation=FlowDesignation.ENROLLMENT, + ) + PolicyBinding.objects.create(policy=flow_policy, target=flow, order=0) + + # PromptStage to ask user for their username + prompt_stage = PromptStage.objects.create( + name="default-source-enrollment-username-prompt", + ) + prompt_stage.fields.add( + Prompt.objects.create( + field_key="username", + label="Username", + type=FieldTypes.TEXT, + required=True, + placeholder="Username", + ) + ) + # Policy to only trigger prompt when no username is given + prompt_policy = ExpressionPolicy.objects.create( + name="default-source-enrollment-if-username", + expression=PROMPT_POLICY_EXPRESSION, + ) + + # UserWrite stage to create the user, and login stage to log user in + user_write = UserWriteStage.objects.create(name="default-source-enrollment-write") + user_login = UserLoginStage.objects.create(name="default-source-enrollment-login") + + binding = FlowStageBinding.objects.create(flow=flow, stage=prompt_stage, order=0) + PolicyBinding.objects.create(policy=prompt_policy, target=binding) + + FlowStageBinding.objects.create(flow=flow, stage=user_write, order=1) + FlowStageBinding.objects.create(flow=flow, stage=user_login, order=2) + + +def create_default_source_authentication_flow( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +): + Flow = apps.get_model("passbook_flows", "Flow") + FlowStageBinding = apps.get_model("passbook_flows", "FlowStageBinding") + PolicyBinding = apps.get_model("passbook_policies", "PolicyBinding") + + ExpressionPolicy = apps.get_model( + "passbook_policies_expression", "ExpressionPolicy" + ) + + UserLoginStage = apps.get_model("passbook_stages_user_login", "UserLoginStage") + + db_alias = schema_editor.connection.alias + + # Create a policy that only allows this flow when doing an SSO Request + flow_policy = ExpressionPolicy.objects.create( + name="default-source-authentication-if-sso", expression=FLOW_POLICY_EXPRESSION + ) + + # This creates a Flow used by sources to authenticate users + flow = Flow.objects.create( + name="default-source-authentication", + slug="default-source-authentication", + designation=FlowDesignation.AUTHENTICATION, + ) + PolicyBinding.objects.create(policy=flow_policy, target=flow, order=0) + + user_login = UserLoginStage.objects.create( + name="default-source-authentication-login" + ) + FlowStageBinding.objects.create(flow=flow, stage=user_login, order=0) + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0003_auto_20200523_1133"), + ("passbook_policies", "0001_initial"), + ("passbook_policies_expression", "0001_initial"), + ("passbook_stages_prompt", "0001_initial"), + ("passbook_stages_user_write", "0001_initial"), + ("passbook_stages_user_login", "0001_initial"), + ] + + operations = [ + migrations.RunPython(create_default_source_enrollment_flow), + migrations.RunPython(create_default_source_authentication_flow), + ] diff --git a/passbook/flows/migrations/0005_provider_flows.py b/passbook/flows/migrations/0005_provider_flows.py new file mode 100644 index 000000000..a197d0532 --- /dev/null +++ b/passbook/flows/migrations/0005_provider_flows.py @@ -0,0 +1,44 @@ +# Generated by Django 3.0.6 on 2020-05-24 11:34 + +from django.apps.registry import Apps +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +from passbook.flows.models import FlowDesignation + + +def create_default_provider_authz_flow( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +): + Flow = apps.get_model("passbook_flows", "Flow") + FlowStageBinding = apps.get_model("passbook_flows", "FlowStageBinding") + + ConsentStage = apps.get_model("passbook_stages_consent", "ConsentStage") + + db_alias = schema_editor.connection.alias + + # Empty flow for providers where no consent is needed + Flow.objects.create( + name="default-provider-authorization", + slug="default-provider-authorization", + designation=FlowDesignation.AUTHORIZATION, + ) + + # Flow with consent form to obtain user consent for authorization + flow = Flow.objects.create( + name="default-provider-authorization-consent", + slug="default-provider-authorization-consent", + designation=FlowDesignation.AUTHORIZATION, + ) + stage = ConsentStage.objects.create(name="default-provider-authorization-consent") + FlowStageBinding.objects.create(flow=flow, stage=stage, order=0) + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0004_source_flows"), + ("passbook_stages_consent", "0001_initial"), + ] + + operations = [migrations.RunPython(create_default_provider_authz_flow)] diff --git a/passbook/flows/models.py b/passbook/flows/models.py index ffb194386..d049a6885 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -1,5 +1,5 @@ """Flow models""" -from typing import Optional +from typing import Callable, Optional from uuid import uuid4 from django.db import models @@ -9,6 +9,7 @@ from model_utils.managers import InheritanceManager from structlog import get_logger from passbook.core.types import UIUserSettings +from passbook.lib.utils.reflection import class_to_path from passbook.policies.models import PolicyBindingModel LOGGER = get_logger() @@ -19,6 +20,7 @@ class FlowDesignation(models.TextChoices): should be replaced by a database entry.""" AUTHENTICATION = "authentication" + AUTHORIZATION = "authorization" INVALIDATION = "invalidation" ENROLLMENT = "enrollment" UNRENOLLMENT = "unenrollment" @@ -48,6 +50,14 @@ class Stage(models.Model): return f"Stage {self.name}" +def in_memory_stage(_type: Callable) -> Stage: + """Creates an in-memory stage instance, based on a `_type` as view.""" + class_path = class_to_path(_type) + stage = Stage() + stage.type = class_path + return stage + + class Flow(PolicyBindingModel): """Flow describes how a series of Stages should be executed to authenticate/enroll/recover a user. Additionally, policies can be applied, to specify which users diff --git a/passbook/flows/planner.py b/passbook/flows/planner.py index 262279434..ca8a81c5e 100644 --- a/passbook/flows/planner.py +++ b/passbook/flows/planner.py @@ -16,6 +16,7 @@ LOGGER = get_logger() PLAN_CONTEXT_PENDING_USER = "pending_user" PLAN_CONTEXT_SSO = "is_sso" +PLAN_CONTEXT_APPLICATION = "application" def cache_key(flow: Flow, user: Optional[User] = None) -> str: @@ -45,10 +46,13 @@ class FlowPlanner: that should be applied.""" use_cache: bool + allow_empty_flows: bool + flow: Flow def __init__(self, flow: Flow): self.use_cache = True + self.allow_empty_flows = False self.flow = flow def plan( @@ -80,11 +84,13 @@ class FlowPlanner: LOGGER.debug( "f(plan): Taking plan from cache", flow=self.flow, key=cached_plan_key ) + # Reset the context as this isn't factored into caching + cached_plan.context = default_context or {} return cached_plan LOGGER.debug("f(plan): building plan", flow=self.flow) plan = self._build_plan(user, request, default_context) cache.set(cache_key(self.flow, user), plan) - if not plan.stages: + if not plan.stages and not self.allow_empty_flows: raise EmptyFlowException() return plan diff --git a/passbook/flows/templates/flows/shell.html b/passbook/flows/templates/flows/shell.html index be6b22724..08190994d 100644 --- a/passbook/flows/templates/flows/shell.html +++ b/passbook/flows/templates/flows/shell.html @@ -113,19 +113,18 @@ const updateMessages = () => { }); }); }; -const updateCard = (response) => { - if (!response.ok) { - console.log("well"); - } - if (response.redirected && !response.url.endsWith(flowBodyUrl)) { - window.location = response.url; - } else { - response.text().then(text => { - flowBody.innerHTML = text; +const updateCard = (data) => { + switch (data.type) { + case "redirect": + window.location = data.to + break; + case "template": + flowBody.innerHTML = data.body; updateMessages(); loadFormCode(); setFormSubmitHandlers(); - }); + default: + break; } }; const showSpinner = () => { @@ -139,10 +138,28 @@ const loadFormCode = () => { document.head.appendChild(newScript); }); }; +const updateFormAction = (form) => { + for (let index = 0; index < form.elements.length; index++) { + const element = form.elements[index]; + if (element.value === form.action) { + console.log("Found Form action URL in form elements, not changing form action."); + return false; + } + } + form.action = flowBodyUrl; + return true; +}; +const checkAutosubmit = (form) => { + if ("autosubmit" in form.attributes) { + return form.submit(); + } +}; const setFormSubmitHandlers = () => { document.querySelectorAll("#flow-body form").forEach(form => { + console.log(`Checking for autosubmit attribute ${form}`); + checkAutosubmit(form); console.log(`Setting action for form ${form}`); - form.action = flowBodyUrl; + updateFormAction(form); console.log(`Adding handler for form ${form}`); form.addEventListener('submit', (e) => { e.preventDefault(); @@ -150,19 +167,13 @@ const setFormSubmitHandlers = () => { fetch(flowBodyUrl, { method: 'post', body: formData, - }).then((response) => { - showSpinner(); - if (!response.url.endsWith(flowBodyUrl)) { - window.location = response.url; - } else { - updateCard(response); - } + }).then(response => response.json()).then(data => { + updateCard(data); }); }); }); }; -fetch(flowBodyUrl).then(updateCard); - +fetch(flowBodyUrl).then(response => response.json()).then(data => updateCard(data)); {% endblock %} diff --git a/passbook/flows/views.py b/passbook/flows/views.py index 5c8c71f11..3613a7017 100644 --- a/passbook/flows/views.py +++ b/passbook/flows/views.py @@ -1,8 +1,15 @@ """passbook multi-stage authentication engine""" from typing import Any, Dict, Optional -from django.http import Http404, HttpRequest, HttpResponse +from django.http import ( + Http404, + HttpRequest, + HttpResponse, + HttpResponseRedirect, + JsonResponse, +) from django.shortcuts import get_object_or_404, redirect, reverse +from django.template.response import TemplateResponse from django.utils.decorators import method_decorator from django.views.decorators.clickjacking import xframe_options_sameorigin from django.views.generic import TemplateView, View @@ -81,6 +88,8 @@ class FlowExecutorView(View): ) stage_cls = path_to_class(self.current_stage.type) self.current_stage_view = stage_cls(self) + self.current_stage_view.args = self.args + self.current_stage_view.kwargs = self.kwargs self.current_stage_view.request = request return super().dispatch(request) @@ -91,7 +100,8 @@ class FlowExecutorView(View): view_class=class_to_path(self.current_stage_view.__class__), flow_slug=self.flow.slug, ) - return self.current_stage_view.get(request, *args, **kwargs) + stage_response = self.current_stage_view.get(request, *args, **kwargs) + return to_stage_response(request, stage_response) def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: """pass post request to current stage""" @@ -100,7 +110,8 @@ class FlowExecutorView(View): view_class=class_to_path(self.current_stage_view.__class__), flow_slug=self.flow.slug, ) - return self.current_stage_view.post(request, *args, **kwargs) + stage_response = self.current_stage_view.post(request, *args, **kwargs) + return to_stage_response(request, stage_response) def _initiate_plan(self) -> FlowPlan: planner = FlowPlanner(self.flow) @@ -191,3 +202,22 @@ class FlowExecutorShellView(TemplateView): kwargs["exec_url"] = reverse("passbook_flows:flow-executor", kwargs=self.kwargs) kwargs["msg_url"] = reverse("passbook_api:messages-list") return kwargs + + +def to_stage_response(request: HttpRequest, source: HttpResponse) -> HttpResponse: + """Convert normal HttpResponse into JSON Response""" + if isinstance(source, HttpResponseRedirect) or source.status_code == 302: + redirect_url = source["Location"] + if request.path != redirect_url: + return JsonResponse({"type": "redirect", "to": redirect_url}) + return source + if isinstance(source, TemplateResponse): + return JsonResponse( + {"type": "template", "body": source.render().content.decode("utf-8")} + ) + # Check for actual HttpResponse (without isinstance as we dont want to check inheritance) + if source.__class__ == HttpResponse: + return JsonResponse( + {"type": "template", "body": source.content.decode("utf-8")} + ) + return source diff --git a/passbook/policies/types.py b/passbook/policies/types.py index 29bf02932..29ab69146 100644 --- a/passbook/policies/types.py +++ b/passbook/policies/types.py @@ -38,6 +38,9 @@ class PolicyResult: self.passing = passing self.messages = messages + def __repr__(self): + return self.__str__() + def __str__(self): if self.messages: return f"PolicyResult passing={self.passing} messages={self.messages}" diff --git a/passbook/providers/app_gw/forms.py b/passbook/providers/app_gw/forms.py index ad4c7fc60..6dddc8927 100644 --- a/passbook/providers/app_gw/forms.py +++ b/passbook/providers/app_gw/forms.py @@ -32,7 +32,7 @@ class ApplicationGatewayProviderForm(forms.ModelForm): class Meta: model = ApplicationGatewayProvider - fields = ["name", "internal_host", "external_host"] + fields = ["name", "authorization_flow", "internal_host", "external_host"] widgets = { "name": forms.TextInput(), "internal_host": forms.TextInput(), diff --git a/passbook/providers/app_gw/models.py b/passbook/providers/app_gw/models.py index 664c75f8b..b85f759ee 100644 --- a/passbook/providers/app_gw/models.py +++ b/passbook/providers/app_gw/models.py @@ -14,7 +14,8 @@ from passbook.lib.utils.template import render_to_string class ApplicationGatewayProvider(Provider): - """This provider uses oauth2_proxy with the OIDC Provider.""" + """Protect applications that don't support any of the other + Protocols by using a Reverse-Proxy.""" name = models.TextField() internal_host = models.TextField() diff --git a/passbook/providers/oauth/forms.py b/passbook/providers/oauth/forms.py index 9c4acc4e0..3d3a7517e 100644 --- a/passbook/providers/oauth/forms.py +++ b/passbook/providers/oauth/forms.py @@ -1,21 +1,32 @@ """passbook OAuth2 Provider Forms""" from django import forms +from django.utils.translation import gettext_lazy as _ +from passbook.flows.models import Flow, FlowDesignation from passbook.providers.oauth.models import OAuth2Provider class OAuth2ProviderForm(forms.ModelForm): """OAuth2 Provider form""" + authorization_flow = forms.ModelChoiceField( + queryset=Flow.objects.filter(designation=FlowDesignation.AUTHORIZATION) + ) + class Meta: model = OAuth2Provider fields = [ "name", + "authorization_flow", "redirect_uris", "client_type", "authorization_grant_type", "client_id", "client_secret", ] + labels = { + "client_id": _("Client ID"), + "redirect_uris": _("Redirect URIs"), + } diff --git a/passbook/providers/oauth/models.py b/passbook/providers/oauth/models.py index b99ebaccf..c42d3bcb4 100644 --- a/passbook/providers/oauth/models.py +++ b/passbook/providers/oauth/models.py @@ -12,7 +12,8 @@ from passbook.lib.utils.template import render_to_string class OAuth2Provider(Provider, AbstractApplication): - """Associate an OAuth2 Application with a Product""" + """Generic OAuth2 Provider for applications not using OpenID-Connect. This Provider + also supports the GitHub-pretend mode.""" form = "passbook.providers.oauth.forms.OAuth2ProviderForm" diff --git a/passbook/providers/oauth/settings.py b/passbook/providers/oauth/settings.py index b8bd2e1a0..bd04dd43e 100644 --- a/passbook/providers/oauth/settings.py +++ b/passbook/providers/oauth/settings.py @@ -24,6 +24,7 @@ OAUTH2_PROVIDER = { "SCOPES": { "openid": "Access OpenID Userinfo", "openid:userinfo": "Access OpenID Userinfo", + "email": "Access OpenID E-Mail", # 'write': 'Write scope', # 'groups': 'Access to your groups', "user:email": "GitHub Compatibility: User E-Mail", diff --git a/passbook/providers/oauth/urls.py b/passbook/providers/oauth/urls.py index a8b68f603..4da8b3a07 100644 --- a/passbook/providers/oauth/urls.py +++ b/passbook/providers/oauth/urls.py @@ -6,17 +6,12 @@ from oauth2_provider import views from passbook.providers.oauth.views import github, oauth2 oauth_urlpatterns = [ - # Custom OAuth 2 Authorize View + # Custom OAuth2 Authorize View path( "authorize/", - oauth2.PassbookAuthorizationView.as_view(), + oauth2.AuthorizationFlowInitView.as_view(), name="oauth2-authorize", ), - path( - "authorize/permission_denied/", - oauth2.OAuthPermissionDenied.as_view(), - name="oauth2-permission-denied", - ), # OAuth API path("token/", views.TokenView.as_view(), name="token"), path("revoke_token/", views.RevokeTokenView.as_view(), name="revoke-token"), @@ -26,7 +21,7 @@ oauth_urlpatterns = [ github_urlpatterns = [ path( "login/oauth/authorize", - oauth2.PassbookAuthorizationView.as_view(), + oauth2.AuthorizationFlowInitView.as_view(), name="github-authorize", ), path( @@ -35,6 +30,7 @@ github_urlpatterns = [ name="github-access-token", ), path("user", github.GitHubUserView.as_view(), name="github-user"), + path("user/teams", github.GitHubUserTeamsView.as_view(), name="github-user-teams"), ] urlpatterns = [ diff --git a/passbook/providers/oauth/views/github.py b/passbook/providers/oauth/views/github.py index b52446ee9..11adb9e0a 100644 --- a/passbook/providers/oauth/views/github.py +++ b/passbook/providers/oauth/views/github.py @@ -1,21 +1,32 @@ """passbook pretend GitHub Views""" -from django.http import JsonResponse +from django.core.exceptions import PermissionDenied +from django.http import HttpRequest, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404 from django.views import View from oauth2_provider.models import AccessToken +from passbook.core.models import User -class GitHubUserView(View): + +class GitHubPretendView(View): + """Emulate GitHub's API Endpoints""" + + def verify_access_token(self) -> User: + """Verify access token manually since github uses /user?access_token=...""" + if "HTTP_AUTHORIZATION" in self.request.META: + full_token = self.request.META.get("HTTP_AUTHORIZATION") + _, token = full_token.split(" ") + elif "access_token" in self.request.GET: + token = self.request.GET.get("access_token", "") + else: + raise PermissionDenied("No access token passed.") + return get_object_or_404(AccessToken, token=token).user + + +class GitHubUserView(GitHubPretendView): """Emulate GitHub's /user API Endpoint""" - def verify_access_token(self): - """Verify access token manually since github uses /user?access_token=...""" - token = get_object_or_404( - AccessToken, token=self.request.GET.get("access_token", "") - ) - return token.user - - def get(self, request): + def get(self, request: HttpRequest) -> HttpResponse: """Emulate GitHub's /user API Endpoint""" user = self.verify_access_token() return JsonResponse( @@ -65,3 +76,11 @@ class GitHubUserView(View): }, } ) + + +class GitHubUserTeamsView(GitHubPretendView): + """Emulate GitHub's /user/teams API Endpoint""" + + def get(self, request: HttpRequest) -> HttpResponse: + """Emulate GitHub's /user/teams API Endpoint""" + return JsonResponse([], safe=False) diff --git a/passbook/providers/oauth/views/oauth2.py b/passbook/providers/oauth/views/oauth2.py index 208dd99ac..301d3e432 100644 --- a/passbook/providers/oauth/views/oauth2.py +++ b/passbook/providers/oauth/views/oauth2.py @@ -1,76 +1,124 @@ """passbook OAuth2 Views""" -from typing import Optional -from urllib.parse import urlencode - from django.contrib import messages -from django.forms import Form -from django.http import HttpRequest, HttpResponse -from django.shortcuts import get_object_or_404, redirect, reverse +from django.http import HttpRequest, HttpResponse, HttpResponseRedirect +from django.shortcuts import get_object_or_404, redirect +from django.views import View +from oauth2_provider.exceptions import OAuthToolkitError from oauth2_provider.views.base import AuthorizationView from structlog import get_logger from passbook.audit.models import Event, EventAction from passbook.core.models import Application from passbook.core.views.access import AccessMixin -from passbook.core.views.utils import PermissionDeniedView +from passbook.flows.models import in_memory_stage +from passbook.flows.planner import ( + PLAN_CONTEXT_APPLICATION, + PLAN_CONTEXT_SSO, + FlowPlanner, +) +from passbook.flows.stage import StageView +from passbook.flows.views import SESSION_KEY_PLAN +from passbook.lib.utils.urls import redirect_with_qs from passbook.providers.oauth.models import OAuth2Provider LOGGER = get_logger() +PLAN_CONTEXT_CLIENT_ID = "client_id" +PLAN_CONTEXT_REDIRECT_URI = "redirect_uri" +PLAN_CONTEXT_RESPONSE_TYPE = "response_type" +PLAN_CONTEXT_STATE = "state" -class OAuthPermissionDenied(PermissionDeniedView): - """Show permission denied view""" +PLAN_CONTEXT_CODE_CHALLENGE = "code_challenge" +PLAN_CONTEXT_CODE_CHALLENGE_METHOD = "code_challenge_method" +PLAN_CONTEXT_SCOPE = "scope" +PLAN_CONTEXT_NONCE = "nonce" -class PassbookAuthorizationView(AccessMixin, AuthorizationView): - """Custom OAuth2 Authorization View which checks policies, etc""" +class AuthorizationFlowInitView(AccessMixin, View): + """OAuth2 Flow initializer, checks access to application and starts flow""" - _application: Optional[Application] = None - - def _inject_response_type(self): - """Inject response_type into querystring if not set""" - LOGGER.debug("response_type not set, defaulting to 'code'") - querystring = urlencode(self.request.GET) - querystring += "&response_type=code" - return redirect( - reverse("passbook_providers_oauth:oauth2-ok-authorize") + "?" + querystring - ) - - def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: - """Update OAuth2Provider's skip_authorization state""" - # Get client_id to get provider, so we can update skip_authorization field + # pylint: disable=unused-argument + def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: + """Check access to application, start FlowPLanner, return to flow executor shell""" client_id = request.GET.get("client_id") provider = get_object_or_404(OAuth2Provider, client_id=client_id) try: application = self.provider_to_application(provider) except Application.DoesNotExist: return redirect("passbook_providers_oauth:oauth2-permission-denied") - # Update field here so oauth-toolkit does work for us - provider.skip_authorization = application.skip_authorization - provider.save() - self._application = application # Check permissions - result = self.user_has_access(self._application, request.user) + result = self.user_has_access(application, request.user) if not result.passing: for policy_message in result.messages: messages.error(request, policy_message) return redirect("passbook_providers_oauth:oauth2-permission-denied") - # Some clients don't pass response_type, so we default to code - if "response_type" not in request.GET: - return self._inject_response_type() - actual_response = AuthorizationView.dispatch(self, request, *args, **kwargs) - if actual_response.status_code == 400: - LOGGER.debug("Bad request", redirect_uri=request.GET.get("redirect_uri")) - return actual_response - - def form_valid(self, form: Form): - # User has clicked on "Authorize" - Event.new( - EventAction.AUTHORIZE_APPLICATION, authorized_application=self._application, - ).from_http(self.request) - LOGGER.debug( - "User authorized Application", - user=self.request.user, - application=self._application, + # Regardless, we start the planner and return to it + planner = FlowPlanner(provider.authorization_flow) + # planner.use_cache = False + planner.allow_empty_flows = True + plan = planner.plan( + self.request, + { + PLAN_CONTEXT_SSO: True, + PLAN_CONTEXT_APPLICATION: application, + PLAN_CONTEXT_CLIENT_ID: client_id, + PLAN_CONTEXT_REDIRECT_URI: request.GET.get(PLAN_CONTEXT_REDIRECT_URI), + PLAN_CONTEXT_RESPONSE_TYPE: request.GET.get(PLAN_CONTEXT_RESPONSE_TYPE), + PLAN_CONTEXT_STATE: request.GET.get(PLAN_CONTEXT_STATE), + PLAN_CONTEXT_SCOPE: request.GET.get(PLAN_CONTEXT_SCOPE), + PLAN_CONTEXT_NONCE: request.GET.get(PLAN_CONTEXT_NONCE), + }, ) - return AuthorizationView.form_valid(self, form) + plan.stages.append(in_memory_stage(OAuth2Stage)) + self.request.session[SESSION_KEY_PLAN] = plan + return redirect_with_qs( + "passbook_flows:flow-executor-shell", + self.request.GET, + flow_slug=provider.authorization_flow.slug, + ) + + +class OAuth2Stage(AuthorizationView, StageView): + """OAuth2 Stage, dynamically injected into the plan""" + + def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: + """Last stage in flow, finalizes OAuth Response and redirects to Client""" + application: Application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION] + provider: OAuth2Provider = application.provider + + Event.new( + EventAction.AUTHORIZE_APPLICATION, authorized_application=application, + ).from_http(self.request) + + credentials = { + "client_id": self.executor.plan.context[PLAN_CONTEXT_CLIENT_ID], + "redirect_uri": self.executor.plan.context[PLAN_CONTEXT_REDIRECT_URI], + "response_type": self.executor.plan.context.get( + PLAN_CONTEXT_RESPONSE_TYPE, None + ), + "state": self.executor.plan.context.get(PLAN_CONTEXT_STATE, None), + "nonce": self.executor.plan.context.get(PLAN_CONTEXT_NONCE, None), + } + if self.executor.plan.context.get(PLAN_CONTEXT_CODE_CHALLENGE, False): + credentials[PLAN_CONTEXT_CODE_CHALLENGE] = self.executor.plan.context.get( + PLAN_CONTEXT_CODE_CHALLENGE + ) + if self.executor.plan.context.get(PLAN_CONTEXT_CODE_CHALLENGE_METHOD, False): + credentials[ + PLAN_CONTEXT_CODE_CHALLENGE_METHOD + ] = self.executor.plan.context.get(PLAN_CONTEXT_CODE_CHALLENGE_METHOD) + scopes = self.executor.plan.context.get(PLAN_CONTEXT_SCOPE) + + try: + uri, _headers, _body, _status = self.create_authorization_response( + request=self.request, + scopes=scopes, + credentials=credentials, + allow=True, + ) + LOGGER.debug("Success url for the request: {0}".format(uri)) + except OAuthToolkitError as error: + return self.error_response(error, provider) + + self.executor.stage_ok() + return HttpResponseRedirect(self.redirect(uri, provider).url) diff --git a/passbook/providers/oidc/apps.py b/passbook/providers/oidc/apps.py index ee4f50191..363366e17 100644 --- a/passbook/providers/oidc/apps.py +++ b/passbook/providers/oidc/apps.py @@ -1,6 +1,4 @@ """passbook auth oidc provider app config""" -from importlib import import_module - from django.apps import AppConfig from django.db.utils import InternalError, OperationalError, ProgrammingError from django.urls import include, path @@ -15,6 +13,7 @@ class PassbookProviderOIDCConfig(AppConfig): name = "passbook.providers.oidc" label = "passbook_providers_oidc" verbose_name = "passbook Providers.OIDC" + mountpoint = "application/oidc/" def ready(self): try: @@ -36,5 +35,3 @@ class PassbookProviderOIDCConfig(AppConfig): include("oidc_provider.urls", namespace="oidc_provider"), ), ) - - import_module("passbook.providers.oidc.signals") diff --git a/passbook/providers/oidc/auth.py b/passbook/providers/oidc/auth.py index 91a0b9dcf..0050463bf 100644 --- a/passbook/providers/oidc/auth.py +++ b/passbook/providers/oidc/auth.py @@ -10,6 +10,8 @@ from structlog import get_logger from passbook.audit.models import Event, EventAction from passbook.core.models import Application, Provider, User +from passbook.flows.planner import FlowPlan +from passbook.flows.views import SESSION_KEY_PLAN from passbook.policies.engine import PolicyEngine LOGGER = get_logger() @@ -46,7 +48,7 @@ def check_permissions( LOGGER.debug( "Checking permissions for application", user=user, application=application ) - policy_engine = PolicyEngine(application.policies.all(), user, request) + policy_engine = PolicyEngine(application, user, request) policy_engine.build() # Check permissions @@ -56,9 +58,10 @@ def check_permissions( messages.error(request, policy_message) return redirect("passbook_providers_oauth:oauth2-permission-denied") + plan: FlowPlan = request.session[SESSION_KEY_PLAN] Event.new( EventAction.AUTHORIZE_APPLICATION, authorized_application=application, - skipped_authorization=False, + flow=plan.flow_pk, ).from_http(request) return None diff --git a/passbook/providers/oidc/forms.py b/passbook/providers/oidc/forms.py index 857adeef6..46e56d16b 100644 --- a/passbook/providers/oidc/forms.py +++ b/passbook/providers/oidc/forms.py @@ -4,12 +4,17 @@ from django import forms from oauth2_provider.generators import generate_client_id, generate_client_secret from oidc_provider.models import Client +from passbook.flows.models import Flow, FlowDesignation from passbook.providers.oidc.models import OpenIDProvider class OIDCProviderForm(forms.ModelForm): """OpenID Client form""" + authorization_flow = forms.ModelChoiceField( + queryset=Flow.objects.filter(designation=FlowDesignation.AUTHORIZATION) + ) + def __init__(self, *args, **kwargs): # Correctly load data from 1:1 rel if "instance" in kwargs and kwargs["instance"]: @@ -17,20 +22,35 @@ class OIDCProviderForm(forms.ModelForm): super().__init__(*args, **kwargs) self.fields["client_id"].initial = generate_client_id() self.fields["client_secret"].initial = generate_client_secret() + try: + self.fields[ + "authorization_flow" + ].initial = self.instance.openidprovider.authorization_flow + # pylint: disable=no-member + except Client.openidprovider.RelatedObjectDoesNotExist: + pass def save(self, *args, **kwargs): self.instance.reuse_consent = False # This is managed by passbook - self.instance.require_consent = True # This is managed by passbook + self.instance.require_consent = False # This is managed by passbook response = super().save(*args, **kwargs) # Check if openidprovider class instance exists if not OpenIDProvider.objects.filter(oidc_client=self.instance).exists(): - OpenIDProvider.objects.create(oidc_client=self.instance) + OpenIDProvider.objects.create( + oidc_client=self.instance, + authorization_flow=self.cleaned_data.get("authorization_flow"), + ) + self.instance.openidprovider.authorization_flow = self.cleaned_data.get( + "authorization_flow" + ) + self.instance.openidprovider.save() return response class Meta: model = Client fields = [ "name", + "authorization_flow", "client_type", "client_id", "client_secret", diff --git a/passbook/providers/oidc/models.py b/passbook/providers/oidc/models.py index 16c6d9d34..e0f3feaf0 100644 --- a/passbook/providers/oidc/models.py +++ b/passbook/providers/oidc/models.py @@ -12,7 +12,7 @@ from passbook.lib.utils.template import render_to_string class OpenIDProvider(Provider): - """Proxy model for OIDC Client""" + """OpenID Connect Provider for applications that support OIDC.""" # Since oidc_provider doesn't currently support swappable models # (https://github.com/juanifioren/django-oidc-provider/pull/305) @@ -28,7 +28,7 @@ class OpenIDProvider(Provider): return self.oidc_client.name def __str__(self): - return "OpenID Connect Provider %s" % self.oidc_client.__str__() + return f"OpenID Connect Provider {self.oidc_client.__str__()}" def html_setup_urls(self, request: HttpRequest) -> Optional[str]: """return template and context modal with URLs for authorize, token, openid-config, etc""" @@ -37,14 +37,14 @@ class OpenIDProvider(Provider): { "provider": self, "authorize": request.build_absolute_uri( - reverse("oidc_provider:authorize") + reverse("passbook_providers_oidc:authorize") ), "token": request.build_absolute_uri(reverse("oidc_provider:token")), "userinfo": request.build_absolute_uri( reverse("oidc_provider:userinfo") ), "provider_info": request.build_absolute_uri( - reverse("oidc_provider:provider-info") + reverse("passbook_providers_oidc:provider-info") ), }, ) diff --git a/passbook/providers/oidc/signals.py b/passbook/providers/oidc/signals.py deleted file mode 100644 index a74550388..000000000 --- a/passbook/providers/oidc/signals.py +++ /dev/null @@ -1,15 +0,0 @@ -"""OIDC Provider signals""" -from django.db.models.signals import post_save -from django.dispatch import receiver - -from passbook.core.models import Application -from passbook.providers.oidc.models import OpenIDProvider - - -@receiver(post_save, sender=Application) -# pylint: disable=unused-argument -def on_application_save(sender, instance: Application, **_): - """Synchronize application's skip_authorization with oidc_client's require_consent""" - if isinstance(instance.provider, OpenIDProvider): - instance.provider.oidc_client.require_consent = not instance.skip_authorization - instance.provider.oidc_client.save() diff --git a/passbook/providers/oidc/urls.py b/passbook/providers/oidc/urls.py new file mode 100644 index 000000000..a7f317709 --- /dev/null +++ b/passbook/providers/oidc/urls.py @@ -0,0 +1,13 @@ +"""oidc provider URLs""" +from django.conf.urls import url + +from passbook.providers.oidc.views import AuthorizationFlowInitView, ProviderInfoView + +urlpatterns = [ + url(r"^authorize/?$", AuthorizationFlowInitView.as_view(), name="authorize"), + url( + r"^\.well-known/openid-configuration/?$", + ProviderInfoView.as_view(), + name="provider-info", + ), +] diff --git a/passbook/providers/oidc/views.py b/passbook/providers/oidc/views.py new file mode 100644 index 000000000..cd81f9c59 --- /dev/null +++ b/passbook/providers/oidc/views.py @@ -0,0 +1,127 @@ +"""passbook OIDC Views""" +from django.contrib import messages +from django.http import HttpRequest, HttpResponse, JsonResponse +from django.shortcuts import get_object_or_404, redirect, reverse +from django.views import View +from oidc_provider.lib.endpoints.authorize import AuthorizeEndpoint +from oidc_provider.lib.utils.common import get_issuer, get_site_url +from oidc_provider.models import ResponseType +from oidc_provider.views import AuthorizeView +from structlog import get_logger + +from passbook.core.models import Application +from passbook.core.views.access import AccessMixin +from passbook.flows.models import in_memory_stage +from passbook.flows.planner import ( + PLAN_CONTEXT_APPLICATION, + PLAN_CONTEXT_SSO, + FlowPlan, + FlowPlanner, +) +from passbook.flows.stage import StageView +from passbook.flows.views import SESSION_KEY_PLAN +from passbook.lib.utils.urls import redirect_with_qs +from passbook.providers.oidc.models import OpenIDProvider + +LOGGER = get_logger() + +PLAN_CONTEXT_PARAMS = "params" + + +class AuthorizationFlowInitView(AccessMixin, View): + """OIDC Flow initializer, checks access to application and starts flow""" + + # pylint: disable=unused-argument + def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: + """Check access to application, start FlowPLanner, return to flow executor shell""" + client_id = request.GET.get("client_id") + provider = get_object_or_404(OpenIDProvider, oidc_client__client_id=client_id) + try: + application = self.provider_to_application(provider) + except Application.DoesNotExist: + return redirect("passbook_providers_oauth:oauth2-permission-denied") + # Check permissions + result = self.user_has_access(application, request.user) + if not result.passing: + for policy_message in result.messages: + messages.error(request, policy_message) + return redirect("passbook_providers_oauth:oauth2-permission-denied") + # Extract params so we can save them in the plan context + endpoint = AuthorizeEndpoint(request) + # Regardless, we start the planner and return to it + planner = FlowPlanner(provider.authorization_flow) + # planner.use_cache = False + planner.allow_empty_flows = True + plan = planner.plan( + self.request, + { + PLAN_CONTEXT_SSO: True, + PLAN_CONTEXT_APPLICATION: application, + PLAN_CONTEXT_PARAMS: endpoint.params, + }, + ) + plan.stages.append(in_memory_stage(OIDCStage)) + self.request.session[SESSION_KEY_PLAN] = plan + return redirect_with_qs( + "passbook_flows:flow-executor-shell", + self.request.GET, + flow_slug=provider.authorization_flow.slug, + ) + + +class FlowAuthorizeEndpoint(AuthorizeEndpoint): + """Restore params from flow context""" + + def _extract_params(self): + plan: FlowPlan = self.request.session[SESSION_KEY_PLAN] + self.params = plan.context[PLAN_CONTEXT_PARAMS] + + +class OIDCStage(AuthorizeView, StageView): + """Finall stage, restores params from Flow.""" + + authorize_endpoint_class = FlowAuthorizeEndpoint + + +class ProviderInfoView(View): + """Custom ProviderInfo View which shows our URLs instead""" + + # pylint: disable=unused-argument + def get(self, request, *args, **kwargs): + """Custom ProviderInfo View which shows our URLs instead""" + dic = dict() + + site_url = get_site_url(request=request) + dic["issuer"] = get_issuer(site_url=site_url, request=request) + + dic["authorization_endpoint"] = site_url + reverse( + "passbook_providers_oidc:authorize" + ) + dic["token_endpoint"] = site_url + reverse("oidc_provider:token") + dic["userinfo_endpoint"] = site_url + reverse("oidc_provider:userinfo") + dic["end_session_endpoint"] = site_url + reverse("oidc_provider:end-session") + dic["introspection_endpoint"] = site_url + reverse( + "oidc_provider:token-introspection" + ) + + types_supported = [ + response_type.value for response_type in ResponseType.objects.all() + ] + dic["response_types_supported"] = types_supported + + dic["jwks_uri"] = site_url + reverse("oidc_provider:jwks") + + dic["id_token_signing_alg_values_supported"] = ["HS256", "RS256"] + + # See: http://openid.net/specs/openid-connect-core-1_0.html#SubjectIDTypes + dic["subject_types_supported"] = ["public"] + + dic["token_endpoint_auth_methods_supported"] = [ + "client_secret_post", + "client_secret_basic", + ] + + response = JsonResponse(dic) + response["Access-Control-Allow-Origin"] = "*" + + return response diff --git a/passbook/providers/saml/forms.py b/passbook/providers/saml/forms.py index 3a5dd8904..864e36a74 100644 --- a/passbook/providers/saml/forms.py +++ b/passbook/providers/saml/forms.py @@ -5,6 +5,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext as _ from passbook.core.expression import PropertyMappingEvaluator +from passbook.flows.models import Flow, FlowDesignation from passbook.providers.saml.models import ( SAMLPropertyMapping, SAMLProvider, @@ -15,6 +16,9 @@ from passbook.providers.saml.models import ( class SAMLProviderForm(forms.ModelForm): """SAML Provider form""" + authorization_flow = forms.ModelChoiceField( + queryset=Flow.objects.filter(designation=FlowDesignation.AUTHORIZATION) + ) processor_path = forms.ChoiceField( choices=get_provider_choices(), label="Processor" ) @@ -24,10 +28,12 @@ class SAMLProviderForm(forms.ModelForm): model = SAMLProvider fields = [ "name", + "authorization_flow", "processor_path", "acs_url", "audience", "issuer", + "sp_binding", "assertion_valid_not_before", "assertion_valid_not_on_or_after", "session_valid_not_on_or_after", 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 38a67f5bd..9f1f13357 100644 --- a/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py +++ b/passbook/providers/saml/migrations/0002_default_saml_property_mappings.py @@ -13,32 +13,32 @@ 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": "return user.get('email')", + "Expression": "return user.email", }, { "FriendlyName": "cn", "Name": "urn:oid:2.5.4.3", - "Expression": "return user.get('name')", + "Expression": "return user.name", }, { "FriendlyName": "mail", "Name": "urn:oid:0.9.2342.19200300.100.1.3", - "Expression": "return user.get('email')", + "Expression": "return user.email", }, { "FriendlyName": "displayName", "Name": "urn:oid:2.16.840.1.113730.3.1.241", - "Expression": "return user.get('username')", + "Expression": "return user.username", }, { "FriendlyName": "uid", "Name": "urn:oid:0.9.2342.19200300.100.1.1", - "Expression": "return user.get('pk')", + "Expression": "return user.pk", }, { "FriendlyName": "member-of", "Name": "member-of", - "Expression": "[{% for group in user.groups.all() %}'{{ group.name }}',{% endfor %}]", + "Expression": "for group in user.groups.all():\n yield group.name", }, ] for default in defaults: diff --git a/passbook/providers/saml/migrations/0003_samlprovider_sp_binding.py b/passbook/providers/saml/migrations/0003_samlprovider_sp_binding.py new file mode 100644 index 000000000..20ffbbf06 --- /dev/null +++ b/passbook/providers/saml/migrations/0003_samlprovider_sp_binding.py @@ -0,0 +1,20 @@ +# Generated by Django 3.0.6 on 2020-06-06 13:25 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_providers_saml", "0002_default_saml_property_mappings"), + ] + + operations = [ + migrations.AddField( + model_name="samlprovider", + name="sp_binding", + field=models.TextField( + choices=[("redirect", "Redirect"), ("post", "Post")], default="redirect" + ), + ), + ] diff --git a/passbook/providers/saml/models.py b/passbook/providers/saml/models.py index 4472df8c7..26767fbbb 100644 --- a/passbook/providers/saml/models.py +++ b/passbook/providers/saml/models.py @@ -17,6 +17,13 @@ from passbook.providers.saml.utils.time import timedelta_string_validator LOGGER = get_logger() +class SAMLBindings(models.TextChoices): + """SAML Bindings supported by passbook""" + + REDIRECT = "redirect" + POST = "post" + + class SAMLProvider(Provider): """Model to save information about a Remote SAML Endpoint""" @@ -26,6 +33,9 @@ class SAMLProvider(Provider): acs_url = models.URLField(verbose_name=_("ACS URL")) audience = models.TextField(default="") issuer = models.TextField(help_text=_("Also known as EntityID")) + sp_binding = models.TextField( + choices=SAMLBindings.choices, default=SAMLBindings.REDIRECT + ) assertion_valid_not_before = models.TextField( default="minutes=-5", @@ -118,8 +128,8 @@ class SAMLProvider(Provider): try: # pylint: disable=no-member return reverse( - "passbook_providers_saml:saml-metadata", - kwargs={"application": self.application.slug}, + "passbook_providers_saml:metadata", + kwargs={"application_slug": self.application.slug}, ) except Provider.application.RelatedObjectDoesNotExist: return None diff --git a/passbook/providers/saml/templates/saml/idp/autosubmit_form.html b/passbook/providers/saml/templates/saml/idp/autosubmit_form.html index dff5c2dba..6202797b1 100644 --- a/passbook/providers/saml/templates/saml/idp/autosubmit_form.html +++ b/passbook/providers/saml/templates/saml/idp/autosubmit_form.html @@ -4,30 +4,26 @@ {% load i18n %} {% block card_title %} -{% trans 'Redirecting...' %} +{% blocktrans with app=application.name %} +Redirecting to {{ app }}... +{% endblocktrans %} {% endblock %} {% block card %} -
    + {% csrf_token %} {% for key, value in attrs.items %} {% endfor %} -