From 5007a6befe8c7ab5b77154000853c0468977f09c Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 20 Sep 2020 21:27:34 +0200 Subject: [PATCH] stages/prompt: integrate password comparison when multiple password fields are given --- .../enrollment-email-verification.json | 12 ----- .../examples/recovery-email-verification.json | 13 ----- e2e/test_flows_enroll.py | 15 ------ passbook/flows/tests/test_transfer.py | 5 -- .../0002_passwordstage_change_flow.py | 15 ------ passbook/stages/prompt/forms.py | 49 +++++++++++++++++-- .../migrations/0002_auto_20200920_1859.py | 42 ++++++++++++++++ passbook/stages/prompt/models.py | 11 ++++- passbook/stages/prompt/signals.py | 4 ++ 9 files changed, 101 insertions(+), 65 deletions(-) create mode 100644 passbook/stages/prompt/migrations/0002_auto_20200920_1859.py create mode 100644 passbook/stages/prompt/signals.py diff --git a/docs/flow/examples/enrollment-email-verification.json b/docs/flow/examples/enrollment-email-verification.json index 9c7092a32..299ad2d49 100644 --- a/docs/flow/examples/enrollment-email-verification.json +++ b/docs/flow/examples/enrollment-email-verification.json @@ -84,15 +84,6 @@ } }, { - "identifiers": { - "pk": "9922212c-47a2-475a-9905-abeb5e621652" - }, - "model": "passbook_policies_expression.expressionpolicy", - "attrs": { - "name": "policy-enrollment-password-equals", - "expression": "# Verifies that the passwords are equal\r\nreturn request.context['password'] == request.context['password_repeat']" - } - },{ "identifiers": { "pk": "096e6282-6b30-4695-bd03-3b143eab5580", "name": "default-enrollment-email-verficiation" @@ -135,9 +126,6 @@ "cb954fd4-65a5-4ad9-b1ee-180ee9559cf4", "7db91ee8-4290-4e08-8d39-63f132402515", "d30b5eb4-7787-4072-b1ba-65b46e928920" - ], - "validation_policies": [ - "9922212c-47a2-475a-9905-abeb5e621652" ] } }, diff --git a/docs/flow/examples/recovery-email-verification.json b/docs/flow/examples/recovery-email-verification.json index c4bc4ed4b..f3843d1bb 100644 --- a/docs/flow/examples/recovery-email-verification.json +++ b/docs/flow/examples/recovery-email-verification.json @@ -55,16 +55,6 @@ "order": 1 } }, - { - "identifiers": { - "pk": "cd042fc6-cc92-4b98-b7e6-f4729df798d8" - }, - "model": "passbook_policies_expression.expressionpolicy", - "attrs": { - "name": "default-password-change-password-equal", - "expression": "# Check that both passwords are equal.\nreturn request.context['password'] == request.context['password_repeat']" - } - }, { "identifiers": { "pk": "e54045a7-6ecb-4ad9-ad37-28e72d8e565e", @@ -118,9 +108,6 @@ "fields": [ "7db91ee8-4290-4e08-8d39-63f132402515", "d30b5eb4-7787-4072-b1ba-65b46e928920" - ], - "validation_policies": [ - "cd042fc6-cc92-4b98-b7e6-f4729df798d8" ] } }, diff --git a/e2e/test_flows_enroll.py b/e2e/test_flows_enroll.py index 9af3647fa..aee1884c1 100644 --- a/e2e/test_flows_enroll.py +++ b/e2e/test_flows_enroll.py @@ -10,7 +10,6 @@ from selenium.webdriver.support import expected_conditions as ec from e2e.utils import USER, SeleniumTestCase from passbook.flows.models import Flow, FlowDesignation, FlowStageBinding -from passbook.policies.expression.models import ExpressionPolicy from passbook.stages.email.models import EmailStage, EmailTemplates from passbook.stages.identification.models import IdentificationStage from passbook.stages.prompt.models import FieldTypes, Prompt, PromptStage @@ -59,16 +58,9 @@ class TestFlowsEnroll(SeleniumTestCase): field_key="email", label="E-Mail", order=1, type=FieldTypes.EMAIL ) - # Password checking policy - password_policy = ExpressionPolicy.objects.create( - name="policy-enrollment-password-equals", - expression="return request.context['password'] == request.context['password_repeat']", - ) - # Stages first_stage = PromptStage.objects.create(name="prompt-stage-first") first_stage.fields.set([username_prompt, password, password_repeat]) - first_stage.validation_policies.set([password_policy]) first_stage.save() second_stage = PromptStage.objects.create(name="prompt-stage-second") second_stage.fields.set([name_field, email]) @@ -152,16 +144,9 @@ class TestFlowsEnroll(SeleniumTestCase): field_key="email", label="E-Mail", order=1, type=FieldTypes.EMAIL ) - # Password checking policy - password_policy = ExpressionPolicy.objects.create( - name="policy-enrollment-password-equals", - expression="return request.context['password'] == request.context['password_repeat']", - ) - # Stages first_stage = PromptStage.objects.create(name="prompt-stage-first") first_stage.fields.set([username_prompt, password, password_repeat]) - first_stage.validation_policies.set([password_policy]) first_stage.save() second_stage = PromptStage.objects.create(name="prompt-stage-second") second_stage.fields.set([name_field, email]) diff --git a/passbook/flows/tests/test_transfer.py b/passbook/flows/tests/test_transfer.py index e71ef1959..f0945e016 100644 --- a/passbook/flows/tests/test_transfer.py +++ b/passbook/flows/tests/test_transfer.py @@ -105,15 +105,10 @@ class TestFlowTransfer(TransactionTestCase): order=2, type=FieldTypes.PASSWORD, ) - # Password checking policy - password_policy = ExpressionPolicy.objects.create( - name=generate_client_id(), expression="return True", - ) # Stages first_stage = PromptStage.objects.create(name=generate_client_id()) first_stage.fields.set([username_prompt, password, password_repeat]) - first_stage.validation_policies.set([password_policy]) first_stage.save() flow = Flow.objects.create( diff --git a/passbook/stages/password/migrations/0002_passwordstage_change_flow.py b/passbook/stages/password/migrations/0002_passwordstage_change_flow.py index 482e54428..670d8910f 100644 --- a/passbook/stages/password/migrations/0002_passwordstage_change_flow.py +++ b/passbook/stages/password/migrations/0002_passwordstage_change_flow.py @@ -8,18 +8,11 @@ from django.db.backends.base.schema import BaseDatabaseSchemaEditor from passbook.flows.models import FlowDesignation from passbook.stages.prompt.models import FieldTypes -PROMPT_POLICY_EXPRESSION = """# Check that both passwords are equal. -return request.context['password'] == request.context['password_repeat']""" - def create_default_password_change(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): Flow = apps.get_model("passbook_flows", "Flow") FlowStageBinding = apps.get_model("passbook_flows", "FlowStageBinding") - ExpressionPolicy = apps.get_model( - "passbook_policies_expression", "ExpressionPolicy" - ) - PromptStage = apps.get_model("passbook_stages_prompt", "PromptStage") Prompt = apps.get_model("passbook_stages_prompt", "Prompt") @@ -57,15 +50,8 @@ def create_default_password_change(apps: Apps, schema_editor: BaseDatabaseSchema }, ) - # Policy to only trigger prompt when no username is given - prompt_policy, _ = ExpressionPolicy.objects.using(db_alias).update_or_create( - name="default-password-change-password-equal", - defaults={"expression": PROMPT_POLICY_EXPRESSION}, - ) - prompt_stage.fields.add(password_prompt) prompt_stage.fields.add(password_rep_prompt) - prompt_stage.validation_policies.add(prompt_policy) prompt_stage.save() user_write, _ = UserWriteStage.objects.using(db_alias).update_or_create( @@ -100,7 +86,6 @@ class Migration(migrations.Migration): dependencies = [ ("passbook_flows", "0006_auto_20200629_0857"), - ("passbook_policies_expression", "0001_initial"), ("passbook_stages_password", "0001_initial"), ("passbook_stages_prompt", "0001_initial"), ("passbook_stages_user_write", "0001_initial"), diff --git a/passbook/stages/prompt/forms.py b/passbook/stages/prompt/forms.py index 154c7fdf6..34b0196d6 100644 --- a/passbook/stages/prompt/forms.py +++ b/passbook/stages/prompt/forms.py @@ -1,9 +1,11 @@ """Prompt forms""" from email.policy import Policy -from typing import Callable, Iterator, List +from types import MethodType +from typing import Any, Callable, Iterator, List from django import forms from django.contrib.admin.widgets import FilteredSelectMultiple +from django.db.models.query import QuerySet from django.http import HttpRequest from django.utils.translation import gettext_lazy as _ from guardian.shortcuts import get_anonymous_user @@ -13,6 +15,7 @@ from passbook.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from passbook.policies.engine import PolicyEngine from passbook.policies.models import PolicyBinding, PolicyBindingModel from passbook.stages.prompt.models import FieldTypes, Prompt, PromptStage +from passbook.stages.prompt.signals import password_validate class PromptStageForm(forms.ModelForm): @@ -86,12 +89,35 @@ class PromptForm(forms.Form): setattr( self, f"clean_{field.field_key}", - username_field_cleaner_generator(field), + MethodType(username_field_cleaner_factory(field), self), ) + # Check if we have a password field, add a handler that sends a signal + # to validate it + if field.type == FieldTypes.PASSWORD: + setattr( + self, + f"clean_{field.field_key}", + MethodType(password_single_cleaner_factory(field), self), + ) + self.field_order = sorted(fields, key=lambda x: x.order) + def _clean_password_fields(self, *field_names): + """Check if the value of all password fields match by merging them into a set + and checking the length""" + all_passwords = {self.cleaned_data[x] for x in field_names} + if len(all_passwords) > 1: + raise forms.ValidationError(_("Passwords don't match.")) + def clean(self): cleaned_data = super().clean() + # Check if we have two password fields, and make sure they are the same + password_fields: QuerySet[Prompt] = self.stage.fields.filter( + type=FieldTypes.PASSWORD + ) + if password_fields.exists() and password_fields.count() == 2: + self._clean_password_fields(*[field.field_key for field in password_fields]) + user = self.plan.context.get(PLAN_CONTEXT_PENDING_USER, get_anonymous_user()) engine = ListPolicyEngine(self.stage.validation_policies.all(), user) engine.request.context = cleaned_data @@ -101,13 +127,28 @@ class PromptForm(forms.Form): raise forms.ValidationError(list(result.messages)) -def username_field_cleaner_generator(field: Prompt) -> Callable: +def username_field_cleaner_factory(field: Prompt) -> Callable: """Return a `clean_` method for `field`. Clean method checks if username is taken already.""" - def username_field_cleaner(self: PromptForm): + def username_field_cleaner(self: PromptForm) -> Any: """Check for duplicate usernames""" username = self.cleaned_data.get(field.field_key) if User.objects.filter(username=username).exists(): raise forms.ValidationError("Username is already taken.") + return username return username_field_cleaner + + +def password_single_cleaner_factory(field: Prompt) -> Callable[[PromptForm], Any]: + """Return a `clean_` method for `field`. Clean method checks if username is taken already.""" + + def password_single_clean(self: PromptForm) -> Any: + """Send password validation signals for e.g. LDAP Source""" + password = self.cleaned_data[field.field_key] + password_validate.send( + sender=self, password=password, plan_context=self.plan.context + ) + return password + + return password_single_clean diff --git a/passbook/stages/prompt/migrations/0002_auto_20200920_1859.py b/passbook/stages/prompt/migrations/0002_auto_20200920_1859.py new file mode 100644 index 000000000..13b7fdd07 --- /dev/null +++ b/passbook/stages/prompt/migrations/0002_auto_20200920_1859.py @@ -0,0 +1,42 @@ +# Generated by Django 3.1.1 on 2020-09-20 18:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_stages_prompt", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="prompt", + name="type", + field=models.CharField( + choices=[ + ("text", "Text: Simple Text input"), + ( + "username", + "Username: Same as Text input, but checks for and prevents duplicate usernames.", + ), + ("email", "Email: Text field with Email type."), + ( + "password", + "Password: Masked input, password is validated against sources. Policies still have to be applied to this Stage. If two of these are used in the same stage, they are ensured to be identical.", + ), + ("number", "Number"), + ("checkbox", "Checkbox"), + ("data", "Date"), + ("data-time", "Date Time"), + ("separator", "Separator: Static Separator Line"), + ( + "hidden", + "Hidden: Hidden field, can be used to insert data into form.", + ), + ("static", "Static: Static value, displayed as-is."), + ], + max_length=100, + ), + ), + ] diff --git a/passbook/stages/prompt/models.py b/passbook/stages/prompt/models.py index 3b7fbdeaa..70b55b652 100644 --- a/passbook/stages/prompt/models.py +++ b/passbook/stages/prompt/models.py @@ -31,7 +31,16 @@ class FieldTypes(models.TextChoices): ), ) EMAIL = "email", _("Email: Text field with Email type.") - PASSWORD = "password" # noqa # nosec + PASSWORD = ( + "password", # noqa # nosec + _( + ( + "Password: Masked input, password is validated against sources. Policies still " + "have to be applied to this Stage. If two of these are used in the same stage, " + "they are ensured to be identical." + ) + ), + ) NUMBER = "number" CHECKBOX = "checkbox" DATE = "data" diff --git a/passbook/stages/prompt/signals.py b/passbook/stages/prompt/signals.py new file mode 100644 index 000000000..ad4b6bd11 --- /dev/null +++ b/passbook/stages/prompt/signals.py @@ -0,0 +1,4 @@ +"""passbook prompt stage signals""" +from django.core.signals import Signal + +password_validate = Signal(providing_args=["password", "plan_context"])