From a3a3dde1c8bef04fd32a19da2b7836cbc7b1d608 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 10 May 2020 17:02:01 +0200 Subject: [PATCH] stages/dummy: add unittests stages/password: improve coverage stages/user_login: improve coverage --- passbook/api/v2/urls.py | 8 ++--- passbook/core/views/user.py | 7 ++-- passbook/root/settings.py | 1 + passbook/stages/dummy/tests.py | 50 ++++++++++++++++++++++++++ passbook/stages/password/exceptions.py | 12 ------- passbook/stages/password/models.py | 18 ---------- passbook/stages/prompt/models.py | 2 +- passbook/stages/user_login/tests.py | 6 ++++ 8 files changed, 65 insertions(+), 39 deletions(-) create mode 100644 passbook/stages/dummy/tests.py delete mode 100644 passbook/stages/password/exceptions.py diff --git a/passbook/api/v2/urls.py b/passbook/api/v2/urls.py index b9fd4e23c..23b3b1ab1 100644 --- a/passbook/api/v2/urls.py +++ b/passbook/api/v2/urls.py @@ -60,12 +60,12 @@ router.register("sources/ldap", LDAPSourceViewSet) router.register("sources/oauth", OAuthSourceViewSet) router.register("policies/all", PolicyViewSet) -router.register("policies/passwordexpiry", PasswordExpiryPolicyViewSet) +router.register("policies/expression", ExpressionPolicyViewSet) router.register("policies/haveibeenpwned", HaveIBeenPwendPolicyViewSet) router.register("policies/password", PasswordPolicyViewSet) +router.register("policies/passwordexpiry", PasswordExpiryPolicyViewSet) router.register("policies/reputation", ReputationPolicyViewSet) router.register("policies/webhook", WebhookPolicyViewSet) -router.register("policies/expression", ExpressionPolicyViewSet) router.register("providers/all", ProviderViewSet) router.register("providers/applicationgateway", ApplicationGatewayProviderViewSet) @@ -80,12 +80,12 @@ router.register("propertymappings/saml", SAMLPropertyMappingViewSet) router.register("stages/all", StageViewSet) router.register("stages/captcha", CaptchaStageViewSet) router.register("stages/email", EmailStageViewSet) +router.register("stages/identification", IdentificationStageViewSet) router.register("stages/otp", OTPStageViewSet) router.register("stages/password", PasswordStageViewSet) -router.register("stages/identification", IdentificationStageViewSet) -router.register("stages/user_login", UserLoginStageViewSet) router.register("stages/prompt", PromptStageViewSet) router.register("stages/prompt/prompts", PromptViewSet) +router.register("stages/user_login", UserLoginStageViewSet) router.register("flows", FlowViewSet) router.register("flows/bindings", FlowStageBindingViewSet) diff --git a/passbook/core/views/user.py b/passbook/core/views/user.py index 0127266a9..215efb2f5 100644 --- a/passbook/core/views/user.py +++ b/passbook/core/views/user.py @@ -11,7 +11,6 @@ from django.views.generic import DeleteView, FormView, UpdateView from passbook.core.forms.users import PasswordChangeForm, UserDetailForm from passbook.lib.config import CONFIG -from passbook.stages.password.exceptions import PasswordPolicyInvalid class UserSettingsView(SuccessMessageMixin, LoginRequiredMixin, UpdateView): @@ -48,20 +47,20 @@ class UserChangePasswordView(LoginRequiredMixin, FormView): template_name = "login/form_with_user.html" def form_valid(self, form: PasswordChangeForm): + # TODO: Rewrite to flow try: # user.set_password checks against Policies so we don't need to manually do it here self.request.user.set_password(form.cleaned_data.get("password")) self.request.user.save() update_session_auth_hash(self.request, self.request.user) messages.success(self.request, _("Successfully changed password")) - except PasswordPolicyInvalid as exc: + except ValueError: # Manually inject error into form # pylint: disable=protected-access errors = form._errors.setdefault("password_repeat", ErrorList("")) # pylint: disable=protected-access errors = form._errors.setdefault("password", ErrorList()) - for error in exc.messages: - errors.append(error) + errors.append("foo") return self.form_invalid(form) return redirect("passbook_core:overview") diff --git a/passbook/root/settings.py b/passbook/root/settings.py index c0405bd42..ac8534c3e 100644 --- a/passbook/root/settings.py +++ b/passbook/root/settings.py @@ -352,6 +352,7 @@ for handler_name, level in _LOGGING_HANDLER_MAP.items(): TEST = False TEST_RUNNER = "xmlrunner.extra.djangotestrunner.XMLTestRunner" +TEST_OUTPUT_VERBOSE = 2 TEST_OUTPUT_FILE_NAME = "unittest.xml" diff --git a/passbook/stages/dummy/tests.py b/passbook/stages/dummy/tests.py new file mode 100644 index 000000000..390f47aa8 --- /dev/null +++ b/passbook/stages/dummy/tests.py @@ -0,0 +1,50 @@ +"""dummy tests""" +from django.shortcuts import reverse +from django.test import Client, TestCase + +from passbook.core.models import User +from passbook.flows.models import Flow, FlowDesignation, FlowStageBinding +from passbook.stages.dummy.forms import DummyStageForm +from passbook.stages.dummy.models import DummyStage + + +class TestDummyStage(TestCase): + """Dummy tests""" + + def setUp(self): + super().setUp() + self.user = User.objects.create(username="unittest", email="test@beryju.org") + self.client = Client() + + self.flow = Flow.objects.create( + name="test-dummy", + slug="test-dummy", + designation=FlowDesignation.AUTHENTICATION, + ) + self.stage = DummyStage.objects.create(name="dummy",) + FlowStageBinding.objects.create( + flow=self.flow, stage=self.stage, order=0, + ) + + def test_valid_render(self): + """Test that View renders correctly""" + response = self.client.get( + reverse( + "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} + ) + ) + self.assertEqual(response.status_code, 200) + + def test_post(self): + """Test with valid email, check that URL redirects back to itself""" + url = reverse( + "passbook_flows:flow-executor", kwargs={"flow_slug": self.flow.slug} + ) + response = self.client.post(url, {}) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse("passbook_core:overview")) + + def test_form(self): + """Test Form""" + data = {"name": "test"} + self.assertEqual(DummyStageForm(data).is_valid(), True) diff --git a/passbook/stages/password/exceptions.py b/passbook/stages/password/exceptions.py deleted file mode 100644 index ca56c03c0..000000000 --- a/passbook/stages/password/exceptions.py +++ /dev/null @@ -1,12 +0,0 @@ -"""passbook password policy exceptions""" -from passbook.lib.sentry import SentryIgnoredException - - -class PasswordPolicyInvalid(SentryIgnoredException): - """Exception raised when a Password Policy fails""" - - messages = [] - - def __init__(self, *messages): - super().__init__() - self.messages = messages diff --git a/passbook/stages/password/models.py b/passbook/stages/password/models.py index 74359a876..8d585cfb6 100644 --- a/passbook/stages/password/models.py +++ b/passbook/stages/password/models.py @@ -3,8 +3,6 @@ from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.translation import gettext_lazy as _ -from passbook.core.models import Policy, User -from passbook.core.types import UIUserSettings from passbook.flows.models import Stage @@ -15,26 +13,10 @@ class PasswordStage(Stage): models.TextField(), help_text=_("Selection of backends to test the password against."), ) - password_policies = models.ManyToManyField(Policy, blank=True) type = "passbook.stages.password.stage.PasswordStage" form = "passbook.stages.password.forms.PasswordStageForm" - @property - def ui_user_settings(self) -> UIUserSettings: - return UIUserSettings( - name="Change Password", - icon="pficon-key", - view_name="passbook_core:user-change-password", - ) - - def password_passes(self, user: User) -> bool: - """Return true if user's password passes, otherwise False or raise Exception""" - for policy in self.policies.all(): - if not policy.passes(user): - return False - return True - def __str__(self): return f"Password Stage {self.name}" diff --git a/passbook/stages/prompt/models.py b/passbook/stages/prompt/models.py index 268e88741..f0c85b6f9 100644 --- a/passbook/stages/prompt/models.py +++ b/passbook/stages/prompt/models.py @@ -53,7 +53,7 @@ class Prompt(UUIDModel): return forms.IntegerField( label=_(self.label), widget=forms.NumberInput(attrs=attrs), - requred=self.required, + required=self.required, ) raise ValueError diff --git a/passbook/stages/user_login/tests.py b/passbook/stages/user_login/tests.py index 93b77f85e..83bb38b74 100644 --- a/passbook/stages/user_login/tests.py +++ b/passbook/stages/user_login/tests.py @@ -7,6 +7,7 @@ from passbook.flows.models import Flow, FlowDesignation, FlowStageBinding from passbook.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from passbook.flows.views import SESSION_KEY_PLAN from passbook.stages.password.stage import PLAN_CONTEXT_AUTHENTICATION_BACKEND +from passbook.stages.user_login.forms import UserLoginStageForm from passbook.stages.user_login.models import UserLoginStage @@ -75,3 +76,8 @@ class TestUserLoginStage(TestCase): ) self.assertEqual(response.status_code, 302) self.assertEqual(response.url, reverse("passbook_flows:denied")) + + def test_form(self): + """Test Form""" + data = {"name": "test"} + self.assertEqual(UserLoginStageForm(data).is_valid(), True)