diff --git a/passbook/stages/password/stage.py b/passbook/stages/password/stage.py index 2d721f0eb..67c30990b 100644 --- a/passbook/stages/password/stage.py +++ b/passbook/stages/password/stage.py @@ -1,5 +1,4 @@ """passbook password stage""" -from inspect import Signature from typing import Any, Dict, List, Optional from django.contrib.auth import _clean_credentials @@ -23,32 +22,21 @@ PLAN_CONTEXT_AUTHENTICATION_BACKEND = "user_backend" def authenticate( - request: HttpRequest, backends: List[BaseBackend], **credentials: Dict[str, Any] + request: HttpRequest, backends: List[str], **credentials: Dict[str, Any] ) -> Optional[User]: """If the given credentials are valid, return a User object. Customized version of django's authenticate, which accepts a list of backends""" for backend_path in backends: - backend = path_to_class(backend_path)() - try: - signature = Signature.from_callable(backend.authenticate) - signature.bind(request, **credentials) - except TypeError: - LOGGER.warning("Backend doesn't accept our arguments", backend=backend) - # This backend doesn't accept these credentials as arguments. Try the next one. - continue + backend: BaseBackend = path_to_class(backend_path)() LOGGER.debug("Attempting authentication...", backend=backend) - try: - user = backend.authenticate(request, **credentials) - except PermissionDenied: - LOGGER.debug("Backend threw PermissionDenied", backend=backend) - # This backend says to stop in our tracks - this user should not be allowed in at all. - break + user = backend.authenticate(request, **credentials) if user is None: LOGGER.debug("Backend returned nothing, continuing") continue # Annotate the user object with the path of the backend. user.backend = backend_path + LOGGER.debug("Successful authentication", user=user, backend=backend) return user # The credentials supplied are invalid to all backends, fire signal @@ -78,22 +66,23 @@ class PasswordStage(FormView, AuthenticationStage): user = authenticate( self.request, self.executor.current_stage.backends, **auth_kwargs ) - if user: - # User instance returned from authenticate() has .backend property set - self.executor.plan.context[PLAN_CONTEXT_PENDING_USER] = user - self.executor.plan.context[ - PLAN_CONTEXT_AUTHENTICATION_BACKEND - ] = user.backend - return self.executor.stage_ok() - # No user was found -> invalid credentials - LOGGER.debug("Invalid credentials") - # Manually inject error into form - # pylint: disable=protected-access - errors = form._errors.setdefault("password", ErrorList()) - errors.append(_("Invalid password")) - return self.form_invalid(form) except PermissionDenied: del auth_kwargs["password"] # User was found, but permission was denied (i.e. user is not active) LOGGER.debug("Denied access", **auth_kwargs) return self.executor.stage_invalid() + else: + if not user: + # No user was found -> invalid credentials + LOGGER.debug("Invalid credentials") + # Manually inject error into form + # pylint: disable=protected-access + errors = form._errors.setdefault("password", ErrorList()) + errors.append(_("Invalid password")) + return self.form_invalid(form) + # User instance returned from authenticate() has .backend property set + self.executor.plan.context[PLAN_CONTEXT_PENDING_USER] = user + self.executor.plan.context[ + PLAN_CONTEXT_AUTHENTICATION_BACKEND + ] = user.backend + return self.executor.stage_ok() diff --git a/passbook/stages/password/tests.py b/passbook/stages/password/tests.py new file mode 100644 index 000000000..1a98333e4 --- /dev/null +++ b/passbook/stages/password/tests.py @@ -0,0 +1,118 @@ +"""password tests""" +import string +from random import SystemRandom +from unittest.mock import MagicMock, patch + +from django.core.exceptions import PermissionDenied +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.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan +from passbook.flows.views import SESSION_KEY_PLAN +from passbook.stages.password.models import PasswordStage + +MOCK_BACKEND_AUTHENTICATE = MagicMock(side_effect=PermissionDenied("test")) + + +class TestPasswordStage(TestCase): + """Password tests""" + + def setUp(self): + super().setUp() + self.password = "".join( + SystemRandom().choice(string.ascii_uppercase + string.digits) + for _ in range(8) + ) + self.user = User.objects.create_user( + username="unittest", email="test@beryju.org", password=self.password + ) + self.client = Client() + + self.flow = Flow.objects.create( + name="test-password", + slug="test-password", + designation=FlowDesignation.AUTHENTICATION, + ) + self.stage = PasswordStage.objects.create( + name="password", backends=["django.contrib.auth.backends.ModelBackend"] + ) + FlowStageBinding.objects.create(flow=self.flow, stage=self.stage, order=2) + + def test_without_user(self): + """Test without user""" + plan = FlowPlan(stages=[self.stage]) + 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} + ), + # Still have to send the password so the form is valid + {"password": self.password}, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse("passbook_flows:denied")) + + def test_valid_password(self): + """Test with a valid pending user and valid password""" + plan = FlowPlan(stages=[self.stage]) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + 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} + ), + # Form data + {"password": self.password}, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse("passbook_core:overview")) + + def test_invalid_password(self): + """Test with a valid pending user and invalid password""" + plan = FlowPlan(stages=[self.stage]) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + 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} + ), + # Form data + {"password": self.password + "test"}, + ) + self.assertEqual(response.status_code, 200) + + @patch( + "django.contrib.auth.backends.ModelBackend.authenticate", + MOCK_BACKEND_AUTHENTICATE, + ) + def test_permission_denied(self): + """Test with a valid pending user and valid password. + Backend is patched to return PermissionError""" + # from django.contrib.auth.backends import ModelBackend + # ModelBackend().authenticate() + plan = FlowPlan(stages=[self.stage]) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + 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} + ), + # Form data + {"password": self.password + "test"}, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse("passbook_flows:denied"))