From 5400882d78858265eabb2ee4b43372be0158a56a Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 7 May 2020 21:30:52 +0200 Subject: [PATCH] flows/: more migration progress, consolidate views --- .../core/templates/login/factors/backend.html | 2 +- .../core/tests/test_views_authentication.py | 2 +- passbook/core/urls.py | 15 ------------- passbook/core/views/authentication.py | 4 ++-- passbook/factors/captcha/factor.py | 2 +- passbook/factors/captcha/forms.py | 2 +- passbook/factors/dummy/factor.py | 2 +- passbook/factors/dummy/forms.py | 2 +- passbook/factors/email/factor.py | 2 +- passbook/factors/email/forms.py | 2 +- passbook/factors/otp/factors.py | 2 +- passbook/factors/otp/forms.py | 2 +- passbook/factors/password/factor.py | 4 ++-- passbook/factors/password/forms.py | 2 +- .../{factors/base.py => flows/factor_base.py} | 2 +- passbook/{factors => flows}/forms.py | 0 ..._flowfactorbinding_re_evaluate_policies.py | 21 ++++++++++++++++++ passbook/flows/models.py | 7 ++++++ passbook/{factors => flows}/tests.py | 22 +++++++++---------- passbook/flows/urls.py | 18 ++++++++++++++- passbook/{factors => flows}/view.py | 4 ++-- passbook/policies/expression/evaluator.py | 2 +- passbook/sources/oauth/views/core.py | 4 ++-- 23 files changed, 77 insertions(+), 48 deletions(-) rename passbook/{factors/base.py => flows/factor_base.py} (95%) rename passbook/{factors => flows}/forms.py (100%) create mode 100644 passbook/flows/migrations/0002_flowfactorbinding_re_evaluate_policies.py rename passbook/{factors => flows}/tests.py (86%) rename passbook/{factors => flows}/view.py (98%) diff --git a/passbook/core/templates/login/factors/backend.html b/passbook/core/templates/login/factors/backend.html index a88dd6b0a..2115e9682 100644 --- a/passbook/core/templates/login/factors/backend.html +++ b/passbook/core/templates/login/factors/backend.html @@ -4,6 +4,6 @@ {% block beneath_form %} {% if show_password_forget_notice %} -{% trans 'Forgot password?' %} +{% trans 'Forgot password?' %} {% endif %} {% endblock %} diff --git a/passbook/core/tests/test_views_authentication.py b/passbook/core/tests/test_views_authentication.py index 4dd0b0954..870484223 100644 --- a/passbook/core/tests/test_views_authentication.py +++ b/passbook/core/tests/test_views_authentication.py @@ -77,7 +77,7 @@ class TestAuthenticationViews(TestCase): reverse("passbook_core:auth-login"), data=self.login_data ) self.assertEqual(login_response.status_code, 302) - self.assertEqual(login_response.url, reverse("passbook_core:auth-process")) + self.assertEqual(login_response.url, reverse("passbook_flows:auth-process")) def test_sign_up_view_post(self): """Test account.sign_up view POST (Anonymous)""" diff --git a/passbook/core/urls.py b/passbook/core/urls.py index 154cc12f1..3e3ce066f 100644 --- a/passbook/core/urls.py +++ b/passbook/core/urls.py @@ -1,11 +1,7 @@ """passbook URL Configuration""" from django.urls import path -from structlog import get_logger from passbook.core.views import authentication, overview, user -from passbook.factors import view - -LOGGER = get_logger() urlpatterns = [ # Authentication views @@ -17,22 +13,11 @@ urlpatterns = [ authentication.SignUpConfirmView.as_view(), name="auth-sign-up-confirm", ), - path( - "auth/process/denied/", - view.FactorPermissionDeniedView.as_view(), - name="auth-denied", - ), path( "auth/password/reset//", authentication.PasswordResetView.as_view(), name="auth-password-reset", ), - path("auth/process/", view.AuthenticationView.as_view(), name="auth-process"), - path( - "auth/process//", - view.AuthenticationView.as_view(), - name="auth-process", - ), # User views path("_/user/", user.UserSettingsView.as_view(), name="user-settings"), path("_/user/delete/", user.UserDeleteView.as_view(), name="user-delete"), diff --git a/passbook/core/views/authentication.py b/passbook/core/views/authentication.py index 4749beeda..2c34a3fec 100644 --- a/passbook/core/views/authentication.py +++ b/passbook/core/views/authentication.py @@ -16,7 +16,7 @@ from passbook.core.forms.authentication import LoginForm, SignUpForm from passbook.core.models import Invitation, Nonce, Source, User from passbook.core.signals import invitation_used, user_signed_up from passbook.factors.password.exceptions import PasswordPolicyInvalid -from passbook.factors.view import AuthenticationView, _redirect_with_qs +from passbook.flows.view import AuthenticationView, _redirect_with_qs from passbook.lib.config import CONFIG LOGGER = get_logger() @@ -72,7 +72,7 @@ class LoginView(UserPassesTestMixin, FormView): # No user found return self.invalid_login(self.request) self.request.session[AuthenticationView.SESSION_PENDING_USER] = pre_user.pk - return _redirect_with_qs("passbook_core:auth-process", self.request.GET) + return _redirect_with_qs("passbook_flows:auth-process", self.request.GET) def invalid_login( self, request: HttpRequest, disabled_user: User = None diff --git a/passbook/factors/captcha/factor.py b/passbook/factors/captcha/factor.py index 88fdd4c15..67e2970ba 100644 --- a/passbook/factors/captcha/factor.py +++ b/passbook/factors/captcha/factor.py @@ -2,8 +2,8 @@ from django.views.generic import FormView -from passbook.factors.base import AuthenticationFactor from passbook.factors.captcha.forms import CaptchaForm +from passbook.flows.factor_base import AuthenticationFactor class CaptchaFactor(FormView, AuthenticationFactor): diff --git a/passbook/factors/captcha/forms.py b/passbook/factors/captcha/forms.py index 9ac94ddeb..c19a3eeb4 100644 --- a/passbook/factors/captcha/forms.py +++ b/passbook/factors/captcha/forms.py @@ -5,7 +5,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext_lazy as _ from passbook.factors.captcha.models import CaptchaFactor -from passbook.factors.forms import GENERAL_FIELDS +from passbook.flows.forms import GENERAL_FIELDS class CaptchaForm(forms.Form): diff --git a/passbook/factors/dummy/factor.py b/passbook/factors/dummy/factor.py index 63fce66c1..9b3320d2f 100644 --- a/passbook/factors/dummy/factor.py +++ b/passbook/factors/dummy/factor.py @@ -1,7 +1,7 @@ """passbook multi-factor authentication engine""" from django.http import HttpRequest -from passbook.factors.base import AuthenticationFactor +from passbook.flows.factor_base import AuthenticationFactor class DummyFactor(AuthenticationFactor): diff --git a/passbook/factors/dummy/forms.py b/passbook/factors/dummy/forms.py index c3a0bdf7f..72de002c9 100644 --- a/passbook/factors/dummy/forms.py +++ b/passbook/factors/dummy/forms.py @@ -4,7 +4,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext as _ from passbook.factors.dummy.models import DummyFactor -from passbook.factors.forms import GENERAL_FIELDS +from passbook.flows.forms import GENERAL_FIELDS class DummyFactorForm(forms.ModelForm): diff --git a/passbook/factors/email/factor.py b/passbook/factors/email/factor.py index 082d7ef8a..07401a90d 100644 --- a/passbook/factors/email/factor.py +++ b/passbook/factors/email/factor.py @@ -6,9 +6,9 @@ from django.utils.translation import gettext as _ from structlog import get_logger from passbook.core.models import Nonce -from passbook.factors.base import AuthenticationFactor from passbook.factors.email.tasks import send_mails from passbook.factors.email.utils import TemplateEmailMessage +from passbook.flows.factor_base import AuthenticationFactor from passbook.lib.config import CONFIG LOGGER = get_logger() diff --git a/passbook/factors/email/forms.py b/passbook/factors/email/forms.py index 862e90959..8429a0870 100644 --- a/passbook/factors/email/forms.py +++ b/passbook/factors/email/forms.py @@ -4,7 +4,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext_lazy as _ from passbook.factors.email.models import EmailFactor -from passbook.factors.forms import GENERAL_FIELDS +from passbook.flows.forms import GENERAL_FIELDS class EmailFactorForm(forms.ModelForm): diff --git a/passbook/factors/otp/factors.py b/passbook/factors/otp/factors.py index e16d8111b..c7053731b 100644 --- a/passbook/factors/otp/factors.py +++ b/passbook/factors/otp/factors.py @@ -5,9 +5,9 @@ from django.views.generic import FormView from django_otp import match_token, user_has_device from structlog import get_logger -from passbook.factors.base import AuthenticationFactor from passbook.factors.otp.forms import OTPVerifyForm from passbook.factors.otp.views import OTP_SETTING_UP_KEY, EnableView +from passbook.flows.factor_base import AuthenticationFactor LOGGER = get_logger() diff --git a/passbook/factors/otp/forms.py b/passbook/factors/otp/forms.py index b71198a80..c6c6816a5 100644 --- a/passbook/factors/otp/forms.py +++ b/passbook/factors/otp/forms.py @@ -7,8 +7,8 @@ from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ from django_otp.models import Device -from passbook.factors.forms import GENERAL_FIELDS from passbook.factors.otp.models import OTPFactor +from passbook.flows.forms import GENERAL_FIELDS OTP_CODE_VALIDATOR = RegexValidator( r"^[0-9a-z]{6,8}$", _("Only alpha-numeric characters are allowed.") diff --git a/passbook/factors/password/factor.py b/passbook/factors/password/factor.py index 0aaf7c7d0..d71600b99 100644 --- a/passbook/factors/password/factor.py +++ b/passbook/factors/password/factor.py @@ -11,9 +11,9 @@ from django.views.generic import FormView from structlog import get_logger from passbook.core.models import User -from passbook.factors.base import AuthenticationFactor from passbook.factors.password.forms import PasswordForm -from passbook.factors.view import AuthenticationView +from passbook.flows.factor_base import AuthenticationFactor +from passbook.flows.view import AuthenticationView from passbook.lib.config import CONFIG from passbook.lib.utils.reflection import path_to_class diff --git a/passbook/factors/password/forms.py b/passbook/factors/password/forms.py index 1594ab876..50b48e5c0 100644 --- a/passbook/factors/password/forms.py +++ b/passbook/factors/password/forms.py @@ -4,8 +4,8 @@ from django.conf import settings from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext_lazy as _ -from passbook.factors.forms import GENERAL_FIELDS from passbook.factors.password.models import PasswordFactor +from passbook.flows.forms import GENERAL_FIELDS from passbook.lib.utils.reflection import path_to_class diff --git a/passbook/factors/base.py b/passbook/flows/factor_base.py similarity index 95% rename from passbook/factors/base.py rename to passbook/flows/factor_base.py index ec44b213d..05766cfd1 100644 --- a/passbook/factors/base.py +++ b/passbook/flows/factor_base.py @@ -5,7 +5,7 @@ from django.utils.translation import gettext as _ from django.views.generic import TemplateView from passbook.core.models import User -from passbook.factors.view import AuthenticationView +from passbook.flows.view import AuthenticationView from passbook.lib.config import CONFIG diff --git a/passbook/factors/forms.py b/passbook/flows/forms.py similarity index 100% rename from passbook/factors/forms.py rename to passbook/flows/forms.py diff --git a/passbook/flows/migrations/0002_flowfactorbinding_re_evaluate_policies.py b/passbook/flows/migrations/0002_flowfactorbinding_re_evaluate_policies.py new file mode 100644 index 000000000..bbd5bf15c --- /dev/null +++ b/passbook/flows/migrations/0002_flowfactorbinding_re_evaluate_policies.py @@ -0,0 +1,21 @@ +# Generated by Django 3.0.3 on 2020-05-07 19:18 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="flowfactorbinding", + name="re_evaluate_policies", + field=models.BooleanField( + default=False, + help_text="When this option is enabled, the planner will re-evaluate policies bound to this.", + ), + ), + ] diff --git a/passbook/flows/models.py b/passbook/flows/models.py index a2d9a1a3a..7fa595ef5 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -59,6 +59,13 @@ class FlowFactorBinding(PolicyBindingModel, UUIDModel): flow = models.ForeignKey("Flow", on_delete=models.CASCADE) factor = models.ForeignKey(Factor, on_delete=models.CASCADE) + re_evaluate_policies = models.BooleanField( + default=False, + help_text=_( + "When this option is enabled, the planner will re-evaluate policies bound to this." + ), + ) + order = models.IntegerField() def __str__(self) -> str: diff --git a/passbook/factors/tests.py b/passbook/flows/tests.py similarity index 86% rename from passbook/factors/tests.py rename to passbook/flows/tests.py index 3311081a3..ec1f444bf 100644 --- a/passbook/factors/tests.py +++ b/passbook/flows/tests.py @@ -10,7 +10,7 @@ from django.urls import reverse from passbook.core.models import User from passbook.factors.dummy.models import DummyFactor from passbook.factors.password.models import PasswordFactor -from passbook.factors.view import AuthenticationView +from passbook.flows.view import AuthenticationView class TestFactorAuthentication(TestCase): @@ -37,13 +37,13 @@ class TestFactorAuthentication(TestCase): def test_unauthenticated_raw(self): """test direct call to AuthenticationView""" - response = self.client.get(reverse("passbook_core:auth-process")) + response = self.client.get(reverse("passbook_flows:auth-process")) # Response should be 400 since no pending user is set self.assertEqual(response.status_code, 400) def test_unauthenticated_prepared(self): """test direct call but with pending_uesr in session""" - request = RequestFactory().get(reverse("passbook_core:auth-process")) + request = RequestFactory().get(reverse("passbook_flows:auth-process")) request.user = AnonymousUser() request.session = {} request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk @@ -55,21 +55,21 @@ class TestFactorAuthentication(TestCase): """Test with all factors disabled""" self.factor.enabled = False self.factor.save() - request = RequestFactory().get(reverse("passbook_core:auth-process")) + request = RequestFactory().get(reverse("passbook_flows:auth-process")) request.user = AnonymousUser() request.session = {} request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk response = AuthenticationView.as_view()(request) self.assertEqual(response.status_code, 302) - self.assertEqual(response.url, reverse("passbook_core:auth-denied")) + self.assertEqual(response.url, reverse("passbook_flows:auth-denied")) self.factor.enabled = True self.factor.save() def test_authenticated(self): """Test with already logged in user""" self.client.force_login(self.user) - response = self.client.get(reverse("passbook_core:auth-process")) + response = self.client.get(reverse("passbook_flows:auth-process")) # Response should be 400 since no pending user is set self.assertEqual(response.status_code, 400) self.client.logout() @@ -77,7 +77,7 @@ class TestFactorAuthentication(TestCase): def test_unauthenticated_post(self): """Test post request as unauthenticated user""" request = RequestFactory().post( - reverse("passbook_core:auth-process"), data={"password": self.password} + reverse("passbook_flows:auth-process"), data={"password": self.password} ) request.user = AnonymousUser() middleware = SessionMiddleware() @@ -93,7 +93,7 @@ class TestFactorAuthentication(TestCase): def test_unauthenticated_post_invalid(self): """Test post request as unauthenticated user""" request = RequestFactory().post( - reverse("passbook_core:auth-process"), + reverse("passbook_flows:auth-process"), data={"password": self.password + "a"}, ) request.user = AnonymousUser() @@ -110,7 +110,7 @@ class TestFactorAuthentication(TestCase): """Test view with multiple active factors""" DummyFactor.objects.get_or_create(name="dummy", slug="dummy", order=1) request = RequestFactory().post( - reverse("passbook_core:auth-process"), data={"password": self.password} + reverse("passbook_flows:auth-process"), data={"password": self.password} ) request.user = AnonymousUser() middleware = SessionMiddleware() @@ -122,10 +122,10 @@ class TestFactorAuthentication(TestCase): session_copy = request.session.items() self.assertEqual(response.status_code, 302) # Verify view redirects to itself after auth - self.assertEqual(response.url, reverse("passbook_core:auth-process")) + self.assertEqual(response.url, reverse("passbook_flows:auth-process")) # Run another request with same session which should result in a logged in user - request = RequestFactory().post(reverse("passbook_core:auth-process")) + request = RequestFactory().post(reverse("passbook_flows:auth-process")) request.user = AnonymousUser() middleware = SessionMiddleware() middleware.process_request(request) diff --git a/passbook/flows/urls.py b/passbook/flows/urls.py index a55895878..a4b507bde 100644 --- a/passbook/flows/urls.py +++ b/passbook/flows/urls.py @@ -1,2 +1,18 @@ """flow urls""" -urlpatterns = [] +from django.urls import path + +from passbook.flows.view import AuthenticationView, FactorPermissionDeniedView + +urlpatterns = [ + path("auth/process/", AuthenticationView.as_view(), name="auth-process"), + path( + "auth/process//", + AuthenticationView.as_view(), + name="auth-process", + ), + path( + "auth/process/denied/", + FactorPermissionDeniedView.as_view(), + name="auth-denied", + ), +] diff --git a/passbook/factors/view.py b/passbook/flows/view.py similarity index 98% rename from passbook/factors/view.py rename to passbook/flows/view.py index a26877edc..d99387408 100644 --- a/passbook/factors/view.py +++ b/passbook/flows/view.py @@ -178,7 +178,7 @@ class AuthenticationView(UserPassesTestMixin, View): ] = self.pending_factors self.request.session[AuthenticationView.SESSION_FACTOR] = next_factor LOGGER.debug("Rendering Factor", next_factor=next_factor) - return _redirect_with_qs("passbook_core:auth-process", self.request.GET) + return _redirect_with_qs("passbook_flows:auth-process", self.request.GET) # User passed all factors LOGGER.debug("User passed all factors, logging in", user=self.pending_user) return self._user_passed() @@ -188,7 +188,7 @@ class AuthenticationView(UserPassesTestMixin, View): This should only be shown if user authenticated successfully, but is disabled/locked/etc""" LOGGER.debug("User invalid") self.cleanup() - return _redirect_with_qs("passbook_core:auth-denied", self.request.GET) + return _redirect_with_qs("passbook_flows:auth-denied", self.request.GET) def _user_passed(self) -> HttpResponse: """User Successfully passed all factors""" diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index a2c4d1b3a..fb97e8a29 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -8,7 +8,7 @@ from jinja2.exceptions import TemplateSyntaxError, UndefinedError from jinja2.nativetypes import NativeEnvironment from structlog import get_logger -from passbook.factors.view import AuthenticationView +from passbook.flows.view import AuthenticationView from passbook.lib.utils.http import get_client_ip from passbook.policies.types import PolicyRequest, PolicyResult diff --git a/passbook/sources/oauth/views/core.py b/passbook/sources/oauth/views/core.py index 5c0508c0a..f5d822dae 100644 --- a/passbook/sources/oauth/views/core.py +++ b/passbook/sources/oauth/views/core.py @@ -13,7 +13,7 @@ from django.views.generic import RedirectView, View from structlog import get_logger from passbook.audit.models import Event, EventAction -from passbook.factors.view import AuthenticationView, _redirect_with_qs +from passbook.flows.view import AuthenticationView, _redirect_with_qs from passbook.sources.oauth.clients import get_client from passbook.sources.oauth.models import OAuthSource, UserOAuthSourceConnection @@ -168,7 +168,7 @@ class OAuthCallback(OAuthClientMixin, View): self.request.session[AuthenticationView.SESSION_PENDING_USER] = user.pk self.request.session[AuthenticationView.SESSION_USER_BACKEND] = user.backend self.request.session[AuthenticationView.SESSION_IS_SSO_LOGIN] = True - return _redirect_with_qs("passbook_core:auth-process", self.request.GET) + return _redirect_with_qs("passbook_flows:auth-process", self.request.GET) # pylint: disable=unused-argument def handle_existing_user(self, source, user, access, info):