From 37b2400cdb0e0c52863c58077684ada0bf7cc8ce Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 20 Jul 2020 11:35:16 +0200 Subject: [PATCH 1/4] lib: move SAML timestring utils into lib --- passbook/lib/utils/time.py | 38 +++++++++++++++++++ .../providers/saml/migrations/0001_initial.py | 14 ++----- passbook/providers/saml/models.py | 2 +- .../providers/saml/processors/assertion.py | 3 +- .../providers/saml/tests/test_utils_time.py | 5 +-- passbook/providers/saml/utils/time.py | 36 ------------------ .../migrations/0003_auto_20200624_1957.py | 6 +-- passbook/sources/saml/models.py | 2 +- passbook/sources/saml/tasks.py | 2 +- 9 files changed, 50 insertions(+), 58 deletions(-) create mode 100644 passbook/lib/utils/time.py diff --git a/passbook/lib/utils/time.py b/passbook/lib/utils/time.py new file mode 100644 index 000000000..4ba41ddb5 --- /dev/null +++ b/passbook/lib/utils/time.py @@ -0,0 +1,38 @@ +"""Time utilities""" +import datetime + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + +ALLOWED_KEYS = ( + "days", + "seconds", + "microseconds", + "milliseconds", + "minutes", + "hours", + "weeks", +) + + +def timedelta_string_validator(value: str): + """Validator for Django that checks if value can be parsed with `timedelta_from_string`""" + try: + timedelta_from_string(value) + except ValueError as exc: + raise ValidationError( + _("%(value)s is not in the correct format of 'hours=3;minutes=1'."), + params={"value": value}, + ) from exc + + +def timedelta_from_string(expr: str) -> datetime.timedelta: + """Convert a string with the format of 'hours=1;minute=3;seconds=5' to a + `datetime.timedelta` Object with hours = 1, minutes = 3, seconds = 5""" + kwargs = {} + for duration_pair in expr.split(";"): + key, value = duration_pair.split("=") + if key.lower() not in ALLOWED_KEYS: + continue + kwargs[key.lower()] = float(value) + return datetime.timedelta(**kwargs) diff --git a/passbook/providers/saml/migrations/0001_initial.py b/passbook/providers/saml/migrations/0001_initial.py index 22ca56c21..c50faae78 100644 --- a/passbook/providers/saml/migrations/0001_initial.py +++ b/passbook/providers/saml/migrations/0001_initial.py @@ -3,7 +3,7 @@ import django.db.models.deletion from django.db import migrations, models -import passbook.providers.saml.utils.time +import passbook.lib.utils.time class Migration(migrations.Migration): @@ -66,9 +66,7 @@ class Migration(migrations.Migration): models.TextField( default="minutes=-5", help_text="Assertion valid not before current time + this value (Format: hours=-1;minutes=-2;seconds=-3).", - validators=[ - passbook.providers.saml.utils.time.timedelta_string_validator - ], + validators=[passbook.lib.utils.time.timedelta_string_validator], ), ), ( @@ -76,9 +74,7 @@ class Migration(migrations.Migration): models.TextField( default="minutes=5", help_text="Assertion not valid on or after current time + this value (Format: hours=1;minutes=2;seconds=3).", - validators=[ - passbook.providers.saml.utils.time.timedelta_string_validator - ], + validators=[passbook.lib.utils.time.timedelta_string_validator], ), ), ( @@ -86,9 +82,7 @@ class Migration(migrations.Migration): models.TextField( default="minutes=86400", help_text="Session not valid on or after current time + this value (Format: hours=1;minutes=2;seconds=3).", - validators=[ - passbook.providers.saml.utils.time.timedelta_string_validator - ], + validators=[passbook.lib.utils.time.timedelta_string_validator], ), ), ( diff --git a/passbook/providers/saml/models.py b/passbook/providers/saml/models.py index 56b6a8c74..6e2e02162 100644 --- a/passbook/providers/saml/models.py +++ b/passbook/providers/saml/models.py @@ -10,7 +10,7 @@ from structlog import get_logger from passbook.core.models import PropertyMapping, Provider from passbook.crypto.models import CertificateKeyPair from passbook.lib.utils.template import render_to_string -from passbook.providers.saml.utils.time import timedelta_string_validator +from passbook.lib.utils.time import timedelta_string_validator LOGGER = get_logger() diff --git a/passbook/providers/saml/processors/assertion.py b/passbook/providers/saml/processors/assertion.py index a36aa7d68..014817a88 100644 --- a/passbook/providers/saml/processors/assertion.py +++ b/passbook/providers/saml/processors/assertion.py @@ -9,10 +9,11 @@ from signxml import XMLSigner, XMLVerifier from structlog import get_logger from passbook.core.exceptions import PropertyMappingExpressionException +from passbook.lib.utils.time import timedelta_from_string from passbook.providers.saml.models import SAMLPropertyMapping, SAMLProvider from passbook.providers.saml.processors.request_parser import AuthNRequest from passbook.providers.saml.utils import get_random_id -from passbook.providers.saml.utils.time import get_time_string, timedelta_from_string +from passbook.providers.saml.utils.time import get_time_string from passbook.sources.saml.exceptions import UnsupportedNameIDFormat from passbook.sources.saml.processors.constants import ( NS_MAP, diff --git a/passbook/providers/saml/tests/test_utils_time.py b/passbook/providers/saml/tests/test_utils_time.py index 7a35ed8f9..a596481df 100644 --- a/passbook/providers/saml/tests/test_utils_time.py +++ b/passbook/providers/saml/tests/test_utils_time.py @@ -4,10 +4,7 @@ from datetime import timedelta from django.core.exceptions import ValidationError from django.test import TestCase -from passbook.providers.saml.utils.time import ( - timedelta_from_string, - timedelta_string_validator, -) +from passbook.lib.utils.time import timedelta_from_string, timedelta_string_validator class TestTimeUtils(TestCase): diff --git a/passbook/providers/saml/utils/time.py b/passbook/providers/saml/utils/time.py index 2fe490ba9..c807315a2 100644 --- a/passbook/providers/saml/utils/time.py +++ b/passbook/providers/saml/utils/time.py @@ -2,42 +2,6 @@ import datetime from typing import Optional -from django.core.exceptions import ValidationError -from django.utils.translation import gettext_lazy as _ - -ALLOWED_KEYS = ( - "days", - "seconds", - "microseconds", - "milliseconds", - "minutes", - "hours", - "weeks", -) - - -def timedelta_string_validator(value: str): - """Validator for Django that checks if value can be parsed with `timedelta_from_string`""" - try: - timedelta_from_string(value) - except ValueError as exc: - raise ValidationError( - _("%(value)s is not in the correct format of 'hours=3;minutes=1'."), - params={"value": value}, - ) from exc - - -def timedelta_from_string(expr: str) -> datetime.timedelta: - """Convert a string with the format of 'hours=1;minute=3;seconds=5' to a - `datetime.timedelta` Object with hours = 1, minutes = 3, seconds = 5""" - kwargs = {} - for duration_pair in expr.split(";"): - key, value = duration_pair.split("=") - if key.lower() not in ALLOWED_KEYS: - continue - kwargs[key.lower()] = float(value) - return datetime.timedelta(**kwargs) - def get_time_string(delta: Optional[datetime.timedelta] = None) -> str: """Get Data formatted in SAML format""" diff --git a/passbook/sources/saml/migrations/0003_auto_20200624_1957.py b/passbook/sources/saml/migrations/0003_auto_20200624_1957.py index 8164b5d6a..9f9f0fc90 100644 --- a/passbook/sources/saml/migrations/0003_auto_20200624_1957.py +++ b/passbook/sources/saml/migrations/0003_auto_20200624_1957.py @@ -3,7 +3,7 @@ import django.db.models.deletion from django.db import migrations, models -import passbook.providers.saml.utils.time +import passbook.lib.utils.time class Migration(migrations.Migration): @@ -27,9 +27,7 @@ class Migration(migrations.Migration): field=models.TextField( default="days=1", help_text="Time offset when temporary users should be deleted. This only applies if your IDP uses the NameID Format 'transient', and the user doesn't log out manually. (Format: hours=1;minutes=2;seconds=3).", - validators=[ - passbook.providers.saml.utils.time.timedelta_string_validator - ], + validators=[passbook.lib.utils.time.timedelta_string_validator], verbose_name="Delete temporary users after", ), ), diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index 33e1b3da9..975629b70 100644 --- a/passbook/sources/saml/models.py +++ b/passbook/sources/saml/models.py @@ -8,7 +8,7 @@ from django.utils.translation import gettext_lazy as _ from passbook.core.models import Source from passbook.core.types import UILoginButton from passbook.crypto.models import CertificateKeyPair -from passbook.providers.saml.utils.time import timedelta_string_validator +from passbook.lib.utils.time import timedelta_string_validator from passbook.sources.saml.processors.constants import ( SAML_NAME_ID_FORMAT_EMAIL, SAML_NAME_ID_FORMAT_PERSISTENT, diff --git a/passbook/sources/saml/tasks.py b/passbook/sources/saml/tasks.py index caad70dab..dde27b87d 100644 --- a/passbook/sources/saml/tasks.py +++ b/passbook/sources/saml/tasks.py @@ -3,7 +3,7 @@ from django.utils.timezone import now from structlog import get_logger from passbook.core.models import User -from passbook.providers.saml.utils.time import timedelta_from_string +from passbook.lib.utils.time import timedelta_from_string from passbook.root.celery import CELERY_APP from passbook.sources.saml.models import SAMLSource From 50612991fadcbe614cc9597ddc619a01b29a58dd Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 20 Jul 2020 13:19:58 +0200 Subject: [PATCH 2/4] stages/consent: start implementing user consent --- passbook/stages/consent/api.py | 2 +- passbook/stages/consent/forms.py | 2 +- .../migrations/0002_auto_20200720_0941.py | 83 +++++++++++++++++++ passbook/stages/consent/models.py | 45 +++++++++- passbook/stages/consent/stage.py | 44 +++++++++- swagger.yaml | 12 +++ 6 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 passbook/stages/consent/migrations/0002_auto_20200720_0941.py diff --git a/passbook/stages/consent/api.py b/passbook/stages/consent/api.py index 845f840e6..f0623dd78 100644 --- a/passbook/stages/consent/api.py +++ b/passbook/stages/consent/api.py @@ -11,7 +11,7 @@ class ConsentStageSerializer(ModelSerializer): class Meta: model = ConsentStage - fields = ["pk", "name"] + fields = ["pk", "name", "mode", "consent_expire_in"] class ConsentStageViewSet(ModelViewSet): diff --git a/passbook/stages/consent/forms.py b/passbook/stages/consent/forms.py index 1b7b34d2c..d716490ae 100644 --- a/passbook/stages/consent/forms.py +++ b/passbook/stages/consent/forms.py @@ -14,7 +14,7 @@ class ConsentStageForm(forms.ModelForm): class Meta: model = ConsentStage - fields = ["name"] + fields = ["name", "mode", "consent_expire_in"] widgets = { "name": forms.TextInput(), } diff --git a/passbook/stages/consent/migrations/0002_auto_20200720_0941.py b/passbook/stages/consent/migrations/0002_auto_20200720_0941.py new file mode 100644 index 000000000..4849be788 --- /dev/null +++ b/passbook/stages/consent/migrations/0002_auto_20200720_0941.py @@ -0,0 +1,83 @@ +# Generated by Django 3.0.8 on 2020-07-20 09:41 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import passbook.core.models +import passbook.lib.utils.time + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_core", "0006_auto_20200709_1608"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("passbook_stages_consent", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="consentstage", + name="consent_expire_in", + field=models.TextField( + default="weeks=4", + help_text="Offset after which consent expires. (Format: hours=1;minutes=2;seconds=3).", + validators=[passbook.lib.utils.time.timedelta_string_validator], + verbose_name="Consent expires in", + ), + ), + migrations.AddField( + model_name="consentstage", + name="mode", + field=models.TextField( + choices=[ + ("always_require", "Always Require"), + ("permanent", "Permanent"), + ("expiring", "Expiring"), + ], + default="always_require", + ), + ), + migrations.CreateModel( + name="UserConsent", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "expires", + models.DateTimeField( + default=passbook.core.models.default_token_duration + ), + ), + ("expiring", models.BooleanField(default=True)), + ( + "application", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="passbook_core.Application", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="pb_consent", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "User Consent", + "verbose_name_plural": "User Consents", + "unique_together": {("user", "application")}, + }, + ), + ] diff --git a/passbook/stages/consent/models.py b/passbook/stages/consent/models.py index 3132474c5..f75e8d739 100644 --- a/passbook/stages/consent/models.py +++ b/passbook/stages/consent/models.py @@ -1,13 +1,39 @@ """passbook consent stage""" +from django.db import models from django.utils.translation import gettext_lazy as _ +from passbook.core.models import Application, ExpiringModel, User from passbook.flows.models import Stage +from passbook.lib.utils.time import timedelta_string_validator + + +class ConsentMode(models.TextChoices): + """Modes a Consent Stage can operate in""" + + ALWAYS_REQUIRE = "always_require" + PERMANENT = "permanent" + EXPIRING = "expiring" class ConsentStage(Stage): """Prompt the user for confirmation.""" - type = "passbook.stages.consent.stage.ConsentStage" + mode = models.TextField( + choices=ConsentMode.choices, default=ConsentMode.ALWAYS_REQUIRE + ) + consent_expire_in = models.TextField( + validators=[timedelta_string_validator], + default="weeks=4", + verbose_name="Consent expires in", + help_text=_( + ( + "Offset after which consent expires. " + "(Format: hours=1;minutes=2;seconds=3)." + ) + ), + ) + + type = "passbook.stages.consent.stage.ConsentStageView" form = "passbook.stages.consent.forms.ConsentStageForm" def __str__(self): @@ -17,3 +43,20 @@ class ConsentStage(Stage): verbose_name = _("Consent Stage") verbose_name_plural = _("Consent Stages") + + +class UserConsent(ExpiringModel): + """Consent given by a user for an application""" + + # TODO: Remove related_name when oidc provider is v2 + user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="pb_consent") + application = models.ForeignKey(Application, on_delete=models.CASCADE) + + def __str__(self): + return f"User Consent {self.application} by {self.user}" + + class Meta: + + unique_together = (("user", "application"),) + verbose_name = _("User Consent") + verbose_name_plural = _("User Consents") diff --git a/passbook/stages/consent/stage.py b/passbook/stages/consent/stage.py index 3a1fa14a2..58894c15a 100644 --- a/passbook/stages/consent/stage.py +++ b/passbook/stages/consent/stage.py @@ -1,15 +1,20 @@ """passbook consent stage""" from typing import Any, Dict, List +from django.http import HttpRequest, HttpResponse +from django.utils.timezone import now from django.views.generic import FormView +from passbook.flows.planner import PLAN_CONTEXT_APPLICATION from passbook.flows.stage import StageView +from passbook.lib.utils.time import timedelta_from_string from passbook.stages.consent.forms import ConsentForm +from passbook.stages.consent.models import ConsentMode, ConsentStage, UserConsent PLAN_CONTEXT_CONSENT_TEMPLATE = "consent_template" -class ConsentStage(FormView, StageView): +class ConsentStageView(FormView, StageView): """Simple consent checker.""" form_class = ConsentForm @@ -26,5 +31,40 @@ class ConsentStage(FormView, StageView): return [template_name] return super().get_template_names() - def form_valid(self, form): + def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: + current_stage: ConsentStage = self.executor.current_stage + # For always require, we always show the form + if current_stage.mode == ConsentMode.ALWAYS_REQUIRE: + return super().get(request, *args, **kwargs) + # at this point we need to check consent from database + if PLAN_CONTEXT_APPLICATION not in self.executor.plan.context: + # No application in this plan, hence we can't check DB and require user consent + return super().get(request, *args, **kwargs) + + application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION] + # TODO: Check for user in plan? + if UserConsent.filter_not_expired( + user=self.request.user, application=application + ).exists(): + return self.executor.stage_ok() + + # No consent found, show form + return super().get(request, *args, **kwargs) + + def form_valid(self, form: ConsentForm) -> HttpResponse: + current_stage: ConsentStage = self.executor.current_stage + if PLAN_CONTEXT_APPLICATION not in self.executor.plan.context: + return self.executor.stage_ok() + application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION] + # Since we only get here when no consent exists, we can create it without update + if current_stage.mode == ConsentMode.PERMANENT: + UserConsent.objects.create( + user=self.request.user, application=application, expiring=False + ) + if current_stage.mode == ConsentMode.EXPIRING: + UserConsent.objects.create( + user=self.request.user, + application=application, + expires=now() + timedelta_from_string(current_stage.consent_expire_in), + ) return self.executor.stage_ok() diff --git a/swagger.yaml b/swagger.yaml index 2539f357c..073d2459c 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -6609,6 +6609,18 @@ definitions: title: Name type: string minLength: 1 + mode: + title: Mode + type: string + enum: + - always_require + - permanent + - expiring + consent_expire_in: + title: Consent expires in + description: 'Offset after which consent expires.(Format: hours=1;minutes=2;seconds=3).' + type: string + minLength: 1 DummyStage: required: - name From 37a432267d441d07ede9d01271da6a6725596053 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 20 Jul 2020 18:17:14 +0200 Subject: [PATCH 3/4] Squashed commit of the following: commit 88029a43355d73011b9fd078231fd932cfa1e2a6 Author: Jens Langhammer Date: Mon Jul 20 16:55:55 2020 +0200 admin: update to work with new form commit 4040eb9619a9beb8ae6bc93f49c0170e4f198f15 Author: Jens Langhammer Date: Mon Jul 20 16:43:30 2020 +0200 *: remove path-based import from all PropertyMappings commit c9663a08dade5b14fcc9a1ddb68637ccc2da8dc9 Author: Jens Langhammer Date: Mon Jul 20 16:33:34 2020 +0200 flows: update work with new stages commit a3d92ebc0a9744efbe3f3940d83e357fad0c63d5 Author: Jens Langhammer Date: Mon Jul 20 16:23:30 2020 +0200 stages/*: remove path-based import from all stages commit 6fa825e372942dbc9f3733ffdac888d44f862ec5 Author: Jens Langhammer Date: Mon Jul 20 16:03:55 2020 +0200 providers/*: remove path-based import from all providers commit 6aefd072c881870948f746992803a181eb65d1e1 Author: Jens Langhammer Date: Mon Jul 20 15:58:48 2020 +0200 policies/*: remove path-based import from all policies commit ac2dd3611fd70c880c2ce356651aee54c11014cf Author: Jens Langhammer Date: Mon Jul 20 15:11:27 2020 +0200 sources/*: remove path-based import from all sources commit 74e628ce9c4f3359e34b17eae922b76e47bc1a37 Author: Jens Langhammer Date: Mon Jul 20 14:43:38 2020 +0200 ui: allow overriding of verbose_name commit d4ee18ee32491c4b41090120b72986de3c7e5222 Author: Jens Langhammer Date: Mon Jul 20 14:08:27 2020 +0200 sources/oauth: migrate from discordapp.com to discord.com --- e2e/test_provider_oauth.py | 12 +++--- e2e/test_provider_oidc.py | 12 +++--- passbook/admin/views/utils.py | 8 ++-- passbook/core/models.py | 18 +++++++-- passbook/flows/models.py | 21 +++++++--- passbook/flows/views.py | 4 +- passbook/lib/templatetags/passbook_utils.py | 7 +++- passbook/policies/dummy/models.py | 7 +++- passbook/policies/expiry/models.py | 7 +++- passbook/policies/expression/models.py | 8 +++- passbook/policies/group_membership/models.py | 8 +++- passbook/policies/hibp/models.py | 7 +++- passbook/policies/models.py | 6 +++ passbook/policies/password/models.py | 7 +++- passbook/policies/reputation/models.py | 8 +++- passbook/providers/app_gw/models.py | 8 +++- passbook/providers/oauth/models.py | 8 +++- passbook/providers/oidc/models.py | 8 +++- passbook/providers/saml/models.py | 13 ++++-- passbook/root/settings.py | 2 + passbook/sources/ldap/models.py | 13 ++++-- passbook/sources/oauth/forms.py | 6 +-- passbook/sources/oauth/models.py | 42 +++++++++++++++----- passbook/sources/saml/models.py | 8 +++- passbook/stages/captcha/models.py | 15 ++++++- passbook/stages/captcha/stage.py | 2 +- passbook/stages/consent/models.py | 15 ++++++- passbook/stages/dummy/models.py | 15 ++++++- passbook/stages/dummy/stage.py | 2 +- passbook/stages/email/models.py | 15 ++++++- passbook/stages/identification/models.py | 15 ++++++- passbook/stages/invitation/models.py | 14 ++++++- passbook/stages/otp_static/models.py | 15 +++++-- passbook/stages/otp_time/models.py | 15 +++++-- passbook/stages/otp_validate/models.py | 15 ++++++- passbook/stages/password/models.py | 15 +++++-- passbook/stages/password/stage.py | 2 +- passbook/stages/prompt/models.py | 14 ++++++- passbook/stages/user_delete/models.py | 15 ++++++- passbook/stages/user_login/models.py | 15 ++++++- passbook/stages/user_logout/models.py | 15 ++++++- passbook/stages/user_write/models.py | 15 ++++++- 42 files changed, 379 insertions(+), 98 deletions(-) diff --git a/e2e/test_provider_oauth.py b/e2e/test_provider_oauth.py index 6b40bbfc7..8733a15e7 100644 --- a/e2e/test_provider_oauth.py +++ b/e2e/test_provider_oauth.py @@ -103,9 +103,9 @@ class TestProviderOAuth(SeleniumTestCase): USER().username, ) self.assertEqual( - self.driver.find_element( - By.CSS_SELECTOR, "input[name=name]" - ).get_attribute("value"), + self.driver.find_element(By.CSS_SELECTOR, "input[name=name]").get_attribute( + "value" + ), USER().username, ) self.assertEqual( @@ -172,9 +172,9 @@ class TestProviderOAuth(SeleniumTestCase): USER().username, ) self.assertEqual( - self.driver.find_element( - By.CSS_SELECTOR, "input[name=name]" - ).get_attribute("value"), + self.driver.find_element(By.CSS_SELECTOR, "input[name=name]").get_attribute( + "value" + ), USER().username, ) self.assertEqual( diff --git a/e2e/test_provider_oidc.py b/e2e/test_provider_oidc.py index c9ae3524c..779a048bf 100644 --- a/e2e/test_provider_oidc.py +++ b/e2e/test_provider_oidc.py @@ -153,9 +153,9 @@ class TestProviderOIDC(SeleniumTestCase): USER().name, ) self.assertEqual( - self.driver.find_element( - By.CSS_SELECTOR, "input[name=name]" - ).get_attribute("value"), + self.driver.find_element(By.CSS_SELECTOR, "input[name=name]").get_attribute( + "value" + ), USER().name, ) self.assertEqual( @@ -232,9 +232,9 @@ class TestProviderOIDC(SeleniumTestCase): USER().name, ) self.assertEqual( - self.driver.find_element( - By.CSS_SELECTOR, "input[name=name]" - ).get_attribute("value"), + self.driver.find_element(By.CSS_SELECTOR, "input[name=name]").get_attribute( + "value" + ), USER().name, ) self.assertEqual( diff --git a/passbook/admin/views/utils.py b/passbook/admin/views/utils.py index 1c2f5e74b..22e984a64 100644 --- a/passbook/admin/views/utils.py +++ b/passbook/admin/views/utils.py @@ -6,7 +6,7 @@ from django.contrib.messages.views import SuccessMessageMixin from django.http import Http404 from django.views.generic import DeleteView, ListView, UpdateView -from passbook.lib.utils.reflection import all_subclasses, path_to_class +from passbook.lib.utils.reflection import all_subclasses from passbook.lib.views import CreateAssignPermView @@ -40,7 +40,7 @@ class InheritanceCreateView(CreateAssignPermView): ) except StopIteration as exc: raise Http404 from exc - return path_to_class(model.form) + return model.form(model) def get_context_data(self, **kwargs: Any) -> Dict[str, Any]: kwargs = super().get_context_data(**kwargs) @@ -61,9 +61,7 @@ class InheritanceUpdateView(UpdateView): return kwargs def get_form_class(self): - form_class_path = self.get_object().form - form_class = path_to_class(form_class_path) - return form_class + return self.get_object().form() def get_object(self, queryset=None): return ( diff --git a/passbook/core/models.py b/passbook/core/models.py index 3febb463a..fb29c1c64 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -1,12 +1,13 @@ """passbook core models""" from datetime import timedelta -from typing import Any, Optional +from typing import Any, Optional, Type from uuid import uuid4 from django.contrib.auth.models import AbstractUser from django.contrib.postgres.fields import JSONField from django.db import models from django.db.models import Q, QuerySet +from django.forms import ModelForm from django.http import HttpRequest from django.utils.timezone import now from django.utils.translation import gettext_lazy as _ @@ -92,6 +93,10 @@ class Provider(models.Model): objects = InheritanceManager() + def form(self) -> Type[ModelForm]: + """Return Form class used to edit this object""" + raise NotImplementedError + # This class defines no field for easier inheritance def __str__(self): if hasattr(self, "name"): @@ -162,10 +167,12 @@ class Source(PolicyBindingModel): related_name="source_enrollment", ) - form = "" # ModelForm-based class ued to create/edit instance - objects = InheritanceManager() + def form(self) -> Type[ModelForm]: + """Return Form class used to edit this object""" + raise NotImplementedError + @property def ui_login_button(self) -> Optional[UILoginButton]: """If source uses a http-based flow, return UI Information about the login @@ -261,9 +268,12 @@ class PropertyMapping(models.Model): name = models.TextField() expression = models.TextField() - form = "" objects = InheritanceManager() + def form(self) -> Type[ModelForm]: + """Return Form class used to edit this object""" + raise NotImplementedError + def evaluate( self, user: Optional[User], request: Optional[HttpRequest], **kwargs ) -> Any: diff --git a/passbook/flows/models.py b/passbook/flows/models.py index aa1523722..06338e596 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -3,13 +3,13 @@ from typing import TYPE_CHECKING, Optional, Type from uuid import uuid4 from django.db import models +from django.forms import ModelForm 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.lib.utils.reflection import class_to_path from passbook.policies.models import PolicyBindingModel if TYPE_CHECKING: @@ -47,8 +47,17 @@ class Stage(models.Model): name = models.TextField() objects = InheritanceManager() - type = "" - form = "" + + def type(self) -> Type["StageView"]: + """Return StageView class that implements logic for this stage""" + # This is a bit of a workaround, since we can't set class methods with setattr + if hasattr(self, "__in_memory_type"): + return getattr(self, "__in_memory_type") + raise NotImplementedError + + def form(self) -> Type[ModelForm]: + """Return Form class used to edit this object""" + raise NotImplementedError @property def ui_user_settings(self) -> Optional[UIUserSettings]: @@ -62,9 +71,11 @@ class Stage(models.Model): def in_memory_stage(view: Type["StageView"]) -> Stage: """Creates an in-memory stage instance, based on a `_type` as view.""" - class_path = class_to_path(view) stage = Stage() - stage.type = class_path + # Because we can't pickle a locally generated function, + # we set the view as a separate property and reference a generic function + # that returns that member + setattr(stage, "__in_memory_type", view) return stage diff --git a/passbook/flows/views.py b/passbook/flows/views.py index c01ede83b..f2c5e1622 100644 --- a/passbook/flows/views.py +++ b/passbook/flows/views.py @@ -21,7 +21,7 @@ from passbook.core.views.utils import PermissionDeniedView from passbook.flows.exceptions import EmptyFlowException, FlowNonApplicableException from passbook.flows.models import Flow, FlowDesignation, Stage from passbook.flows.planner import FlowPlan, FlowPlanner -from passbook.lib.utils.reflection import class_to_path, path_to_class +from passbook.lib.utils.reflection import class_to_path from passbook.lib.utils.urls import is_url_absolute, redirect_with_qs from passbook.lib.views import bad_request_message @@ -94,7 +94,7 @@ class FlowExecutorView(View): if not self.current_stage: LOGGER.debug("f(exec): no more stages, flow is done.") return self._flow_done() - stage_cls = path_to_class(self.current_stage.type) + stage_cls = 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 diff --git a/passbook/lib/templatetags/passbook_utils.py b/passbook/lib/templatetags/passbook_utils.py index aa8cc062f..6acca985b 100644 --- a/passbook/lib/templatetags/passbook_utils.py +++ b/passbook/lib/templatetags/passbook_utils.py @@ -36,7 +36,7 @@ def back(context: Context) -> str: def fieldtype(field): """Return classname""" if isinstance(field.__class__, Model) or issubclass(field.__class__, Model): - return field._meta.verbose_name + return verbose_name(field) return field.__class__.__name__ @@ -84,6 +84,9 @@ def verbose_name(obj) -> str: """Return Object's Verbose Name""" if not obj: return "" + if hasattr(obj, "verbose_name"): + print(obj.verbose_name) + return obj.verbose_name return obj._meta.verbose_name @@ -92,7 +95,7 @@ def form_verbose_name(obj) -> str: """Return ModelForm's Object's Verbose Name""" if not obj: return "" - return obj._meta.model._meta.verbose_name + return verbose_name(obj._meta.model) @register.filter diff --git a/passbook/policies/dummy/models.py b/passbook/policies/dummy/models.py index a8172157a..497f7a0fb 100644 --- a/passbook/policies/dummy/models.py +++ b/passbook/policies/dummy/models.py @@ -1,8 +1,10 @@ """Dummy policy""" from random import SystemRandom from time import sleep +from typing import Type from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ from structlog import get_logger @@ -22,7 +24,10 @@ class DummyPolicy(Policy): wait_min = models.IntegerField(default=5) wait_max = models.IntegerField(default=30) - form = "passbook.policies.dummy.forms.DummyPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.dummy.forms import DummyPolicyForm + + return DummyPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: """Wait random time then return result""" diff --git a/passbook/policies/expiry/models.py b/passbook/policies/expiry/models.py index dfba3b4b9..f476c4dc0 100644 --- a/passbook/policies/expiry/models.py +++ b/passbook/policies/expiry/models.py @@ -1,7 +1,9 @@ """passbook password_expiry_policy Models""" from datetime import timedelta +from typing import Type from django.db import models +from django.forms import ModelForm from django.utils.timezone import now from django.utils.translation import gettext as _ from structlog import get_logger @@ -19,7 +21,10 @@ class PasswordExpiryPolicy(Policy): deny_only = models.BooleanField(default=False) days = models.IntegerField() - form = "passbook.policies.expiry.forms.PasswordExpiryPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.expiry.forms import PasswordExpiryPolicyForm + + return PasswordExpiryPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: """If password change date is more than x days in the past, call set_unusable_password diff --git a/passbook/policies/expression/models.py b/passbook/policies/expression/models.py index 67137d0b6..31c3c398e 100644 --- a/passbook/policies/expression/models.py +++ b/passbook/policies/expression/models.py @@ -1,5 +1,8 @@ """passbook expression Policy Models""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ from passbook.policies.expression.evaluator import PolicyEvaluator @@ -12,7 +15,10 @@ class ExpressionPolicy(Policy): expression = models.TextField() - form = "passbook.policies.expression.forms.ExpressionPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.expression.forms import ExpressionPolicyForm + + return ExpressionPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: """Evaluate and render expression. Returns PolicyResult(false) on error.""" diff --git a/passbook/policies/group_membership/models.py b/passbook/policies/group_membership/models.py index 730b49a9e..7f87a9a00 100644 --- a/passbook/policies/group_membership/models.py +++ b/passbook/policies/group_membership/models.py @@ -1,5 +1,8 @@ """user field matcher models""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ from passbook.core.models import Group @@ -12,7 +15,10 @@ class GroupMembershipPolicy(Policy): group = models.ForeignKey(Group, null=True, blank=True, on_delete=models.SET_NULL) - form = "passbook.policies.group_membership.forms.GroupMembershipPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.group_membership.forms import GroupMembershipPolicyForm + + return GroupMembershipPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: return PolicyResult(self.group.user_set.filter(pk=request.user.pk).exists()) diff --git a/passbook/policies/hibp/models.py b/passbook/policies/hibp/models.py index 90407321d..f9a27fd19 100644 --- a/passbook/policies/hibp/models.py +++ b/passbook/policies/hibp/models.py @@ -1,7 +1,9 @@ """passbook HIBP Models""" from hashlib import sha1 +from typing import Type from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ from requests import get from structlog import get_logger @@ -25,7 +27,10 @@ class HaveIBeenPwendPolicy(Policy): allowed_count = models.IntegerField(default=0) - form = "passbook.policies.hibp.forms.HaveIBeenPwnedPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.hibp.forms import HaveIBeenPwnedPolicyForm + + return HaveIBeenPwnedPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: """Check if password is in HIBP DB. Hashes given Password with SHA1, uses the first 5 diff --git a/passbook/policies/models.py b/passbook/policies/models.py index 826938f51..06c096ec7 100644 --- a/passbook/policies/models.py +++ b/passbook/policies/models.py @@ -1,7 +1,9 @@ """Policy base models""" +from typing import Type from uuid import uuid4 from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ from model_utils.managers import InheritanceManager @@ -73,6 +75,10 @@ class Policy(CreatedUpdatedModel): objects = InheritanceAutoManager() + def form(self) -> Type[ModelForm]: + """Return Form class used to edit this object""" + raise NotImplementedError + def __str__(self): return f"Policy {self.name}" diff --git a/passbook/policies/password/models.py b/passbook/policies/password/models.py index f0250a2f4..bdd40bd92 100644 --- a/passbook/policies/password/models.py +++ b/passbook/policies/password/models.py @@ -1,7 +1,9 @@ """user field matcher models""" import re +from typing import Type from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ from structlog import get_logger @@ -28,7 +30,10 @@ class PasswordPolicy(Policy): symbol_charset = models.TextField(default=r"!\"#$%&'()*+,-./:;<=>?@[\]^_`{|}~ ") error_message = models.TextField() - form = "passbook.policies.password.forms.PasswordPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.password.forms import PasswordPolicyForm + + return PasswordPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: if self.password_field not in request.context: diff --git a/passbook/policies/reputation/models.py b/passbook/policies/reputation/models.py index aa4326129..9ed0366a4 100644 --- a/passbook/policies/reputation/models.py +++ b/passbook/policies/reputation/models.py @@ -1,6 +1,9 @@ """passbook reputation request policy""" +from typing import Type + from django.core.cache import cache from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ from passbook.core.models import User @@ -19,7 +22,10 @@ class ReputationPolicy(Policy): check_username = models.BooleanField(default=True) threshold = models.IntegerField(default=-5) - form = "passbook.policies.reputation.forms.ReputationPolicyForm" + def form(self) -> Type[ModelForm]: + from passbook.policies.reputation.forms import ReputationPolicyForm + + return ReputationPolicyForm def passes(self, request: PolicyRequest) -> PolicyResult: remote_ip = get_client_ip(request.http_request) or "255.255.255.255" diff --git a/passbook/providers/app_gw/models.py b/passbook/providers/app_gw/models.py index efacbafb9..88771ae7c 100644 --- a/passbook/providers/app_gw/models.py +++ b/passbook/providers/app_gw/models.py @@ -1,9 +1,10 @@ """passbook app_gw models""" import string from random import SystemRandom -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.http import HttpRequest from django.utils.translation import gettext as _ from oidc_provider.models import Client @@ -23,7 +24,10 @@ class ApplicationGatewayProvider(Provider): client = models.ForeignKey(Client, on_delete=models.CASCADE) - form = "passbook.providers.app_gw.forms.ApplicationGatewayProviderForm" + def form(self) -> Type[ModelForm]: + from passbook.providers.app_gw.forms import ApplicationGatewayProviderForm + + return ApplicationGatewayProviderForm def html_setup_urls(self, request: HttpRequest) -> Optional[str]: """return template and context modal with URLs for authorize, token, openid-config, etc""" diff --git a/passbook/providers/oauth/models.py b/passbook/providers/oauth/models.py index fe0167d98..f1a9ad522 100644 --- a/passbook/providers/oauth/models.py +++ b/passbook/providers/oauth/models.py @@ -1,7 +1,8 @@ """Oauth2 provider product extension""" -from typing import Optional +from typing import Optional, Type +from django.forms import ModelForm from django.http import HttpRequest from django.shortcuts import reverse from django.utils.translation import gettext as _ @@ -16,7 +17,10 @@ class OAuth2Provider(Provider, AbstractApplication): This Provider also supports the GitHub-pretend mode for Applications that don't support generic OAuth.""" - form = "passbook.providers.oauth.forms.OAuth2ProviderForm" + def form(self) -> Type[ModelForm]: + from passbook.providers.oauth.forms import OAuth2ProviderForm + + return OAuth2ProviderForm def __str__(self): return self.name diff --git a/passbook/providers/oidc/models.py b/passbook/providers/oidc/models.py index 5774d7201..6f74ccb53 100644 --- a/passbook/providers/oidc/models.py +++ b/passbook/providers/oidc/models.py @@ -1,7 +1,8 @@ """oidc models""" -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.http import HttpRequest from django.shortcuts import reverse from django.utils.translation import gettext as _ @@ -20,7 +21,10 @@ class OpenIDProvider(Provider): oidc_client = models.OneToOneField(Client, on_delete=models.CASCADE) - form = "passbook.providers.oidc.forms.OIDCProviderForm" + def form(self) -> Type[ModelForm]: + from passbook.providers.oidc.forms import OIDCProviderForm + + return OIDCProviderForm @property def name(self): diff --git a/passbook/providers/saml/models.py b/passbook/providers/saml/models.py index 6e2e02162..bae2fa59d 100644 --- a/passbook/providers/saml/models.py +++ b/passbook/providers/saml/models.py @@ -1,7 +1,8 @@ """passbook saml_idp Models""" -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.http import HttpRequest from django.shortcuts import reverse from django.utils.translation import ugettext_lazy as _ @@ -101,7 +102,10 @@ class SAMLProvider(Provider): ), ) - form = "passbook.providers.saml.forms.SAMLProviderForm" + def form(self) -> Type[ModelForm]: + from passbook.providers.saml.forms import SAMLProviderForm + + return SAMLProviderForm def __str__(self): return self.name @@ -143,7 +147,10 @@ class SAMLPropertyMapping(PropertyMapping): saml_name = models.TextField(verbose_name="SAML Name") friendly_name = models.TextField(default=None, blank=True, null=True) - form = "passbook.providers.saml.forms.SAMLPropertyMappingForm" + def form(self) -> Type[ModelForm]: + from passbook.providers.saml.forms import SAMLPropertyMappingForm + + return SAMLPropertyMappingForm def __str__(self): return f"SAML Property Mapping {self.saml_name}" diff --git a/passbook/root/settings.py b/passbook/root/settings.py index 77e3aa9e9..0149abdd9 100644 --- a/passbook/root/settings.py +++ b/passbook/root/settings.py @@ -325,6 +325,8 @@ LOG_PRE_CHAIN = [ structlog.stdlib.add_log_level, structlog.stdlib.add_logger_name, structlog.processors.TimeStamper(), + structlog.processors.StackInfoRenderer(), + structlog.processors.format_exc_info, ] LOGGING = { diff --git a/passbook/sources/ldap/models.py b/passbook/sources/ldap/models.py index 0de975870..4fc1ffc3c 100644 --- a/passbook/sources/ldap/models.py +++ b/passbook/sources/ldap/models.py @@ -1,8 +1,9 @@ """passbook LDAP Models""" -from typing import Optional +from typing import Optional, Type from django.core.validators import URLValidator from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ from ldap3 import Connection, Server @@ -53,7 +54,10 @@ class LDAPSource(Source): Group, blank=True, null=True, default=None, on_delete=models.SET_DEFAULT ) - form = "passbook.sources.ldap.forms.LDAPSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.ldap.forms import LDAPSourceForm + + return LDAPSourceForm _connection: Optional[Connection] = None @@ -85,7 +89,10 @@ class LDAPPropertyMapping(PropertyMapping): object_field = models.TextField() - form = "passbook.sources.ldap.forms.LDAPPropertyMappingForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.ldap.forms import LDAPPropertyMappingForm + + return LDAPPropertyMappingForm def __str__(self): return f"LDAP Property Mapping {self.expression} -> {self.object_field}" diff --git a/passbook/sources/oauth/forms.py b/passbook/sources/oauth/forms.py index 509ace23d..5f1e6a65b 100644 --- a/passbook/sources/oauth/forms.py +++ b/passbook/sources/oauth/forms.py @@ -98,9 +98,9 @@ class DiscordOAuthSourceForm(OAuthSourceForm): overrides = { "provider_type": "discord", "request_token_url": "", - "authorization_url": "https://discordapp.com/api/oauth2/authorize", - "access_token_url": "https://discordapp.com/api/oauth2/token", - "profile_url": "https://discordapp.com/api/users/@me", + "authorization_url": "https://discord.com/api/oauth2/authorize", + "access_token_url": "https://discord.com/api/oauth2/token", + "profile_url": "https://discord.com/api/users/@me", } diff --git a/passbook/sources/oauth/models.py b/passbook/sources/oauth/models.py index 95262f32f..ff97d84f2 100644 --- a/passbook/sources/oauth/models.py +++ b/passbook/sources/oauth/models.py @@ -1,7 +1,8 @@ """OAuth Client models""" -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.urls import reverse, reverse_lazy from django.utils.translation import gettext_lazy as _ @@ -40,7 +41,10 @@ class OAuthSource(Source): consumer_key = models.TextField() consumer_secret = models.TextField() - form = "passbook.sources.oauth.forms.OAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import OAuthSourceForm + + return OAuthSourceForm @property def ui_login_button(self) -> UILoginButton: @@ -80,7 +84,10 @@ class OAuthSource(Source): class GitHubOAuthSource(OAuthSource): """Social Login using GitHub.com or a GitHub-Enterprise Instance.""" - form = "passbook.sources.oauth.forms.GitHubOAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import GitHubOAuthSourceForm + + return GitHubOAuthSourceForm class Meta: @@ -92,7 +99,9 @@ class GitHubOAuthSource(OAuthSource): # class TwitterOAuthSource(OAuthSource): # """Social Login using Twitter.com""" -# form = "passbook.sources.oauth.forms.TwitterOAuthSourceForm" +# def form(self) -> Type[ModelForm]: +# from passbook.sources.oauth.forms import TwitterOAuthSourceForm +# return TwitterOAuthSourceForm # class Meta: @@ -104,7 +113,10 @@ class GitHubOAuthSource(OAuthSource): class FacebookOAuthSource(OAuthSource): """Social Login using Facebook.com.""" - form = "passbook.sources.oauth.forms.FacebookOAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import FacebookOAuthSourceForm + + return FacebookOAuthSourceForm class Meta: @@ -116,7 +128,10 @@ class FacebookOAuthSource(OAuthSource): class DiscordOAuthSource(OAuthSource): """Social Login using Discord.""" - form = "passbook.sources.oauth.forms.DiscordOAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import DiscordOAuthSourceForm + + return DiscordOAuthSourceForm class Meta: @@ -128,7 +143,10 @@ class DiscordOAuthSource(OAuthSource): class GoogleOAuthSource(OAuthSource): """Social Login using Google or Gsuite.""" - form = "passbook.sources.oauth.forms.GoogleOAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import GoogleOAuthSourceForm + + return GoogleOAuthSourceForm class Meta: @@ -140,7 +158,10 @@ class GoogleOAuthSource(OAuthSource): class AzureADOAuthSource(OAuthSource): """Social Login using Azure AD.""" - form = "passbook.sources.oauth.forms.AzureADOAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import AzureADOAuthSourceForm + + return AzureADOAuthSourceForm class Meta: @@ -152,7 +173,10 @@ class AzureADOAuthSource(OAuthSource): class OpenIDOAuthSource(OAuthSource): """Login using a Generic OpenID-Connect compliant provider.""" - form = "passbook.sources.oauth.forms.OAuthSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.oauth.forms import OAuthSourceForm + + return OAuthSourceForm class Meta: diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index 975629b70..a5fa74128 100644 --- a/passbook/sources/saml/models.py +++ b/passbook/sources/saml/models.py @@ -1,5 +1,8 @@ """saml sp models""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.http import HttpRequest from django.shortcuts import reverse from django.urls import reverse_lazy @@ -93,7 +96,10 @@ class SAMLSource(Source): on_delete=models.PROTECT, ) - form = "passbook.sources.saml.forms.SAMLSourceForm" + def form(self) -> Type[ModelForm]: + from passbook.sources.saml.forms import SAMLSourceForm + + return SAMLSourceForm def get_issuer(self, request: HttpRequest) -> str: """Get Source's Issuer, falling back to our Metadata URL if none is set""" diff --git a/passbook/stages/captcha/models.py b/passbook/stages/captcha/models.py index db52e4722..584a5ca41 100644 --- a/passbook/stages/captcha/models.py +++ b/passbook/stages/captcha/models.py @@ -1,6 +1,10 @@ """passbook captcha stage""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage @@ -19,8 +23,15 @@ class CaptchaStage(Stage): ) ) - type = "passbook.stages.captcha.stage.CaptchaStage" - form = "passbook.stages.captcha.forms.CaptchaStageForm" + def type(self) -> Type[View]: + from passbook.stages.captcha.stage import CaptchaStageView + + return CaptchaStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.captcha.forms import CaptchaStageForm + + return CaptchaStageForm def __str__(self): return f"Captcha Stage {self.name}" diff --git a/passbook/stages/captcha/stage.py b/passbook/stages/captcha/stage.py index 69e789ac3..f758506e7 100644 --- a/passbook/stages/captcha/stage.py +++ b/passbook/stages/captcha/stage.py @@ -6,7 +6,7 @@ from passbook.flows.stage import StageView from passbook.stages.captcha.forms import CaptchaForm -class CaptchaStage(FormView, StageView): +class CaptchaStageView(FormView, StageView): """Simple captcha checker, logic is handeled in django-captcha module""" form_class = CaptchaForm diff --git a/passbook/stages/consent/models.py b/passbook/stages/consent/models.py index f75e8d739..809e168f6 100644 --- a/passbook/stages/consent/models.py +++ b/passbook/stages/consent/models.py @@ -1,6 +1,10 @@ """passbook consent stage""" from django.db import models +from typing import Type + +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.core.models import Application, ExpiringModel, User from passbook.flows.models import Stage @@ -33,8 +37,15 @@ class ConsentStage(Stage): ), ) - type = "passbook.stages.consent.stage.ConsentStageView" - form = "passbook.stages.consent.forms.ConsentStageForm" + def type(self) -> Type[View]: + from passbook.stages.consent.stage import ConsentStageView + + return ConsentStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.consent.forms import ConsentStageForm + + return ConsentStageForm def __str__(self): return f"Consent Stage {self.name}" diff --git a/passbook/stages/dummy/models.py b/passbook/stages/dummy/models.py index 3dfb1d891..14d77ba6a 100644 --- a/passbook/stages/dummy/models.py +++ b/passbook/stages/dummy/models.py @@ -1,5 +1,9 @@ """dummy stage models""" +from typing import Type + +from django.forms import ModelForm from django.utils.translation import gettext as _ +from django.views import View from passbook.flows.models import Stage @@ -9,8 +13,15 @@ class DummyStage(Stage): __debug_only__ = True - type = "passbook.stages.dummy.stage.DummyStage" - form = "passbook.stages.dummy.forms.DummyStageForm" + def type(self) -> Type[View]: + from passbook.stages.dummy.stage import DummyStageView + + return DummyStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.dummy.forms import DummyStageForm + + return DummyStageForm def __str__(self): return f"Dummy Stage {self.name}" diff --git a/passbook/stages/dummy/stage.py b/passbook/stages/dummy/stage.py index 07f3dacff..bb0620cef 100644 --- a/passbook/stages/dummy/stage.py +++ b/passbook/stages/dummy/stage.py @@ -6,7 +6,7 @@ from django.http import HttpRequest from passbook.flows.stage import StageView -class DummyStage(StageView): +class DummyStageView(StageView): """Dummy stage for testing with multiple stages""" def post(self, request: HttpRequest): diff --git a/passbook/stages/email/models.py b/passbook/stages/email/models.py index 6508704b3..3c575f692 100644 --- a/passbook/stages/email/models.py +++ b/passbook/stages/email/models.py @@ -1,8 +1,12 @@ """email stage models""" +from typing import Type + from django.core.mail import get_connection from django.core.mail.backends.base import BaseEmailBackend from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext as _ +from django.views import View from passbook.flows.models import Stage @@ -40,8 +44,15 @@ class EmailStage(Stage): choices=EmailTemplates.choices, default=EmailTemplates.PASSWORD_RESET ) - type = "passbook.stages.email.stage.EmailStageView" - form = "passbook.stages.email.forms.EmailStageForm" + def type(self) -> Type[View]: + from passbook.stages.email.stage import EmailStageView + + return EmailStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.email.forms import EmailStageForm + + return EmailStageForm @property def backend(self) -> BaseEmailBackend: diff --git a/passbook/stages/identification/models.py b/passbook/stages/identification/models.py index bc5c4cecc..86963d931 100644 --- a/passbook/stages/identification/models.py +++ b/passbook/stages/identification/models.py @@ -1,7 +1,11 @@ """identification stage models""" +from typing import Type + from django.contrib.postgres.fields import ArrayField from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Flow, Stage @@ -52,8 +56,15 @@ class IdentificationStage(Stage): ), ) - type = "passbook.stages.identification.stage.IdentificationStageView" - form = "passbook.stages.identification.forms.IdentificationStageForm" + def type(self) -> Type[View]: + from passbook.stages.identification.stage import IdentificationStageView + + return IdentificationStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.identification.forms import IdentificationStageForm + + return IdentificationStageForm def __str__(self): return f"Identification Stage {self.name}" diff --git a/passbook/stages/invitation/models.py b/passbook/stages/invitation/models.py index 5b9024aec..db7e47011 100644 --- a/passbook/stages/invitation/models.py +++ b/passbook/stages/invitation/models.py @@ -1,9 +1,12 @@ """invitation stage models""" +from typing import Type from uuid import uuid4 from django.contrib.postgres.fields import JSONField from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.core.models import User from passbook.flows.models import Stage @@ -24,8 +27,15 @@ class InvitationStage(Stage): ), ) - type = "passbook.stages.invitation.stage.InvitationStageView" - form = "passbook.stages.invitation.forms.InvitationStageForm" + def type(self) -> Type[View]: + from passbook.stages.invitation.stage import InvitationStageView + + return InvitationStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.invitation.forms import InvitationStageForm + + return InvitationStageForm def __str__(self): return f"Invitation Stage {self.name}" diff --git a/passbook/stages/otp_static/models.py b/passbook/stages/otp_static/models.py index 0d6b9a7a0..7005fed8c 100644 --- a/passbook/stages/otp_static/models.py +++ b/passbook/stages/otp_static/models.py @@ -1,9 +1,11 @@ """OTP Static models""" -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.shortcuts import reverse from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.core.types import UIUserSettings from passbook.flows.models import Stage @@ -14,8 +16,15 @@ class OTPStaticStage(Stage): token_count = models.IntegerField(default=6) - type = "passbook.stages.otp_static.stage.OTPStaticStageView" - form = "passbook.stages.otp_static.forms.OTPStaticStageForm" + def type(self) -> Type[View]: + from passbook.stages.otp_static.stage import OTPStaticStageView + + return OTPStaticStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.otp_static.forms import OTPStaticStageForm + + return OTPStaticStageForm @property def ui_user_settings(self) -> Optional[UIUserSettings]: diff --git a/passbook/stages/otp_time/models.py b/passbook/stages/otp_time/models.py index 73b6840e5..7b9460cc5 100644 --- a/passbook/stages/otp_time/models.py +++ b/passbook/stages/otp_time/models.py @@ -1,9 +1,11 @@ """OTP Time-based models""" -from typing import Optional +from typing import Optional, Type from django.db import models +from django.forms import ModelForm from django.shortcuts import reverse from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.core.types import UIUserSettings from passbook.flows.models import Stage @@ -21,8 +23,15 @@ class OTPTimeStage(Stage): digits = models.IntegerField(choices=TOTPDigits.choices) - type = "passbook.stages.otp_time.stage.OTPTimeStageView" - form = "passbook.stages.otp_time.forms.OTPTimeStageForm" + def type(self) -> Type[View]: + from passbook.stages.otp_time.stage import OTPTimeStageView + + return OTPTimeStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.otp_time.forms import OTPTimeStageForm + + return OTPTimeStageForm @property def ui_user_settings(self) -> Optional[UIUserSettings]: diff --git a/passbook/stages/otp_validate/models.py b/passbook/stages/otp_validate/models.py index 4015aa365..f89735025 100644 --- a/passbook/stages/otp_validate/models.py +++ b/passbook/stages/otp_validate/models.py @@ -1,6 +1,10 @@ """OTP Validation Stage""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import NotConfiguredAction, Stage @@ -12,8 +16,15 @@ class OTPValidateStage(Stage): choices=NotConfiguredAction.choices, default=NotConfiguredAction.SKIP ) - type = "passbook.stages.otp_validate.stage.OTPValidateStageView" - form = "passbook.stages.otp_validate.forms.OTPValidateStageForm" + def type(self) -> Type[View]: + from passbook.stages.otp_validate.stage import OTPValidateStageView + + return OTPValidateStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.otp_validate.forms import OTPValidateStageForm + + return OTPValidateStageForm def __str__(self) -> str: return f"OTP Validation Stage {self.name}" diff --git a/passbook/stages/password/models.py b/passbook/stages/password/models.py index 24275916f..c9d7fce3c 100644 --- a/passbook/stages/password/models.py +++ b/passbook/stages/password/models.py @@ -1,11 +1,13 @@ """password stage models""" -from typing import Optional +from typing import Optional, Type from django.contrib.postgres.fields import ArrayField from django.db import models +from django.forms import ModelForm from django.shortcuts import reverse from django.utils.http import urlencode from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.core.types import UIUserSettings from passbook.flows.models import Flow, Stage @@ -33,8 +35,15 @@ class PasswordStage(Stage): ), ) - type = "passbook.stages.password.stage.PasswordStage" - form = "passbook.stages.password.forms.PasswordStageForm" + def type(self) -> Type[View]: + from passbook.stages.password.stage import PasswordStageView + + return PasswordStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.password.forms import PasswordStageForm + + return PasswordStageForm @property def ui_user_settings(self) -> Optional[UIUserSettings]: diff --git a/passbook/stages/password/stage.py b/passbook/stages/password/stage.py index 8dd708f25..9a48fb3c0 100644 --- a/passbook/stages/password/stage.py +++ b/passbook/stages/password/stage.py @@ -46,7 +46,7 @@ def authenticate( ) -class PasswordStage(FormView, StageView): +class PasswordStageView(FormView, StageView): """Authentication stage which authenticates against django's AuthBackend""" form_class = PasswordForm diff --git a/passbook/stages/prompt/models.py b/passbook/stages/prompt/models.py index 9c44c0ed5..d641f7405 100644 --- a/passbook/stages/prompt/models.py +++ b/passbook/stages/prompt/models.py @@ -1,9 +1,12 @@ """prompt models""" +from typing import Type from uuid import uuid4 from django import forms from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage from passbook.policies.models import PolicyBindingModel @@ -117,8 +120,15 @@ class PromptStage(PolicyBindingModel, Stage): fields = models.ManyToManyField(Prompt) - type = "passbook.stages.prompt.stage.PromptStageView" - form = "passbook.stages.prompt.forms.PromptStageForm" + def type(self) -> Type[View]: + from passbook.stages.prompt.stage import PromptStageView + + return PromptStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.prompt.forms import PromptStageForm + + return PromptStageForm def __str__(self): return f"Prompt Stage {self.name}" diff --git a/passbook/stages/user_delete/models.py b/passbook/stages/user_delete/models.py index 1e9483ad3..667673c72 100644 --- a/passbook/stages/user_delete/models.py +++ b/passbook/stages/user_delete/models.py @@ -1,5 +1,9 @@ """delete stage models""" +from typing import Type + +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage @@ -8,8 +12,15 @@ class UserDeleteStage(Stage): """Deletes the currently pending user without confirmation. Use with caution.""" - type = "passbook.stages.user_delete.stage.UserDeleteStageView" - form = "passbook.stages.user_delete.forms.UserDeleteStageForm" + def type(self) -> Type[View]: + from passbook.stages.user_delete.stage import UserDeleteStageView + + return UserDeleteStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.user_delete.forms import UserDeleteStageForm + + return UserDeleteStageForm def __str__(self): return f"User Delete Stage {self.name}" diff --git a/passbook/stages/user_login/models.py b/passbook/stages/user_login/models.py index 08497983f..96c86661e 100644 --- a/passbook/stages/user_login/models.py +++ b/passbook/stages/user_login/models.py @@ -1,6 +1,10 @@ """login stage models""" +from typing import Type + from django.db import models +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage @@ -16,8 +20,15 @@ class UserLoginStage(Stage): ), ) - type = "passbook.stages.user_login.stage.UserLoginStageView" - form = "passbook.stages.user_login.forms.UserLoginStageForm" + def type(self) -> Type[View]: + from passbook.stages.user_login.stage import UserLoginStageView + + return UserLoginStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.user_login.forms import UserLoginStageForm + + return UserLoginStageForm def __str__(self): return f"User Login Stage {self.name}" diff --git a/passbook/stages/user_logout/models.py b/passbook/stages/user_logout/models.py index 51833bfe1..d85bf2252 100644 --- a/passbook/stages/user_logout/models.py +++ b/passbook/stages/user_logout/models.py @@ -1,5 +1,9 @@ """logout stage models""" +from typing import Type + +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage @@ -7,8 +11,15 @@ from passbook.flows.models import Stage class UserLogoutStage(Stage): """Resets the users current session.""" - type = "passbook.stages.user_logout.stage.UserLogoutStageView" - form = "passbook.stages.user_logout.forms.UserLogoutStageForm" + def type(self) -> Type[View]: + from passbook.stages.user_logout.stage import UserLogoutStageView + + return UserLogoutStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.user_logout.forms import UserLogoutStageForm + + return UserLogoutStageForm def __str__(self): return f"User Logout Stage {self.name}" diff --git a/passbook/stages/user_write/models.py b/passbook/stages/user_write/models.py index 140b5623d..bc7635b6d 100644 --- a/passbook/stages/user_write/models.py +++ b/passbook/stages/user_write/models.py @@ -1,5 +1,9 @@ """write stage models""" +from typing import Type + +from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ +from django.views import View from passbook.flows.models import Stage @@ -8,8 +12,15 @@ class UserWriteStage(Stage): """Writes currently pending data into the pending user, or if no user exists, creates a new user with the data.""" - type = "passbook.stages.user_write.stage.UserWriteStageView" - form = "passbook.stages.user_write.forms.UserWriteStageForm" + def type(self) -> Type[View]: + from passbook.stages.user_write.stage import UserWriteStageView + + return UserWriteStageView + + def form(self) -> Type[ModelForm]: + from passbook.stages.user_write.forms import UserWriteStageForm + + return UserWriteStageForm def __str__(self): return f"User Write Stage {self.name}" From ffff69ada0c85d54e118160f6c09532d3ce20498 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 20 Jul 2020 18:47:44 +0200 Subject: [PATCH 4/4] stages/consent: add unittests for new modes --- passbook/flows/planner.py | 3 +- passbook/stages/consent/models.py | 2 +- passbook/stages/consent/tests.py | 112 ++++++++++++++++++++++++++---- swagger.yaml | 2 +- 4 files changed, 102 insertions(+), 17 deletions(-) diff --git a/passbook/flows/planner.py b/passbook/flows/planner.py index 9e92e889a..546417bd3 100644 --- a/passbook/flows/planner.py +++ b/passbook/flows/planner.py @@ -52,7 +52,8 @@ class FlowPlan: stage = self.stages[0] marker = self.markers[0] - LOGGER.debug("f(plan_inst): stage has marker", stage=stage, marker=marker) + if marker.__class__ is not StageMarker: + LOGGER.debug("f(plan_inst): stage has marker", stage=stage, marker=marker) marked_stage = marker.process(self, stage) if not marked_stage: LOGGER.debug("f(plan_inst): marker returned none, next stage", stage=stage) diff --git a/passbook/stages/consent/models.py b/passbook/stages/consent/models.py index 809e168f6..b0b17e2a0 100644 --- a/passbook/stages/consent/models.py +++ b/passbook/stages/consent/models.py @@ -1,7 +1,7 @@ """passbook consent stage""" -from django.db import models from typing import Type +from django.db import models from django.forms import ModelForm from django.utils.translation import gettext_lazy as _ from django.views import View diff --git a/passbook/stages/consent/tests.py b/passbook/stages/consent/tests.py index 2af565dc9..536b41c6b 100644 --- a/passbook/stages/consent/tests.py +++ b/passbook/stages/consent/tests.py @@ -1,14 +1,17 @@ """consent tests""" +from time import sleep + from django.shortcuts import reverse from django.test import Client, TestCase from django.utils.encoding import force_text -from passbook.core.models import User +from passbook.core.models import Application, User +from passbook.core.tasks import clean_expired_models from passbook.flows.markers import StageMarker from passbook.flows.models import Flow, FlowDesignation, FlowStageBinding -from passbook.flows.planner import FlowPlan +from passbook.flows.planner import PLAN_CONTEXT_APPLICATION, FlowPlan from passbook.flows.views import SESSION_KEY_PLAN -from passbook.stages.consent.models import ConsentStage +from passbook.stages.consent.models import ConsentMode, ConsentStage, UserConsent class TestConsentStage(TestCase): @@ -19,28 +22,29 @@ class TestConsentStage(TestCase): self.user = User.objects.create_user( username="unittest", email="test@beryju.org" ) + self.application = Application.objects.create( + name="test-application", slug="test-application", + ) self.client = Client() - self.flow = Flow.objects.create( + def test_always_required(self): + """Test always required consent""" + flow = Flow.objects.create( name="test-consent", slug="test-consent", designation=FlowDesignation.AUTHENTICATION, ) - self.stage = ConsentStage.objects.create(name="consent",) - FlowStageBinding.objects.create(target=self.flow, stage=self.stage, order=2) - - def test_valid(self): - """Test valid consent""" - plan = FlowPlan( - flow_pk=self.flow.pk.hex, stages=[self.stage], markers=[StageMarker()] + stage = ConsentStage.objects.create( + name="consent", mode=ConsentMode.ALWAYS_REQUIRE ) + FlowStageBinding.objects.create(target=flow, stage=stage, order=2) + + plan = FlowPlan(flow_pk=flow.pk.hex, stages=[stage], markers=[StageMarker()]) session = self.client.session session[SESSION_KEY_PLAN] = plan session.save() response = self.client.post( - reverse( - "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} - ), + reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), {}, ) self.assertEqual(response.status_code, 200) @@ -48,3 +52,83 @@ class TestConsentStage(TestCase): force_text(response.content), {"type": "redirect", "to": reverse("passbook_core:overview")}, ) + self.assertFalse(UserConsent.objects.filter(user=self.user).exists()) + + def test_permanent(self): + """Test permanent consent from user""" + self.client.force_login(self.user) + flow = Flow.objects.create( + name="test-consent", + slug="test-consent", + designation=FlowDesignation.AUTHENTICATION, + ) + stage = ConsentStage.objects.create(name="consent", mode=ConsentMode.PERMANENT) + FlowStageBinding.objects.create(target=flow, stage=stage, order=2) + + plan = FlowPlan( + flow_pk=flow.pk.hex, + stages=[stage], + markers=[StageMarker()], + context={PLAN_CONTEXT_APPLICATION: self.application}, + ) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + response = self.client.post( + reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), + {}, + ) + self.assertEqual(response.status_code, 200) + self.assertJSONEqual( + force_text(response.content), + {"type": "redirect", "to": reverse("passbook_core:overview")}, + ) + self.assertTrue( + UserConsent.objects.filter( + user=self.user, application=self.application + ).exists() + ) + + def test_expire(self): + """Test expiring consent from user""" + self.client.force_login(self.user) + flow = Flow.objects.create( + name="test-consent", + slug="test-consent", + designation=FlowDesignation.AUTHENTICATION, + ) + stage = ConsentStage.objects.create( + name="consent", mode=ConsentMode.EXPIRING, consent_expire_in="seconds=1" + ) + FlowStageBinding.objects.create(target=flow, stage=stage, order=2) + + plan = FlowPlan( + flow_pk=flow.pk.hex, + stages=[stage], + markers=[StageMarker()], + context={PLAN_CONTEXT_APPLICATION: self.application}, + ) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + response = self.client.post( + reverse("passbook_flows:flow-executor", kwargs={"flow_slug": flow.slug}), + {}, + ) + self.assertEqual(response.status_code, 200) + self.assertJSONEqual( + force_text(response.content), + {"type": "redirect", "to": reverse("passbook_core:overview")}, + ) + self.assertTrue( + UserConsent.objects.filter( + user=self.user, application=self.application + ).exists() + ) + sleep(1) + clean_expired_models() + self.assertFalse( + UserConsent.objects.filter( + user=self.user, application=self.application + ).exists() + ) diff --git a/swagger.yaml b/swagger.yaml index 073d2459c..6637f4edd 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -6618,7 +6618,7 @@ definitions: - expiring consent_expire_in: title: Consent expires in - description: 'Offset after which consent expires.(Format: hours=1;minutes=2;seconds=3).' + description: 'Offset after which consent expires. (Format: hours=1;minutes=2;seconds=3).' type: string minLength: 1 DummyStage: