From bbdf8c054bf81d42ee5574fcff32b401168897e0 Mon Sep 17 00:00:00 2001 From: Jens L Date: Tue, 5 Sep 2023 22:55:33 +0200 Subject: [PATCH] stages/password: move password validation to serializer (#6766) * handle non-applicable when restarting flow Signed-off-by: Jens Langhammer * flows: add StageInvalidException error to be used in challenge/response serializer validation to return a stage_invalid error Signed-off-by: Jens Langhammer * rework password stage Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/flows/exceptions.py | 5 ++ authentik/flows/stage.py | 11 +++- authentik/flows/views/executor.py | 7 ++- authentik/stages/password/stage.py | 87 ++++++++++++++++-------------- 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/authentik/flows/exceptions.py b/authentik/flows/exceptions.py index cf38d9a50..7f903633e 100644 --- a/authentik/flows/exceptions.py +++ b/authentik/flows/exceptions.py @@ -26,3 +26,8 @@ class EmptyFlowException(SentryIgnoredException): class FlowSkipStageException(SentryIgnoredException): """Exception to skip a stage""" + + +class StageInvalidException(SentryIgnoredException): + """Exception can be thrown in a `Challenge` or `ChallengeResponse` serializer's + validation to trigger a `executor.stage_invalid()` response""" diff --git a/authentik/flows/stage.py b/authentik/flows/stage.py index 135d54f4d..d9fa75893 100644 --- a/authentik/flows/stage.py +++ b/authentik/flows/stage.py @@ -23,6 +23,7 @@ from authentik.flows.challenge import ( RedirectChallenge, WithUserInfoChallenge, ) +from authentik.flows.exceptions import StageInvalidException from authentik.flows.models import InvalidResponseAction from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, PLAN_CONTEXT_PENDING_USER from authentik.lib.avatars import DEFAULT_AVATAR @@ -100,8 +101,14 @@ class ChallengeStageView(StageView): def post(self, request: Request, *args, **kwargs) -> HttpResponse: """Handle challenge response""" - challenge: ChallengeResponse = self.get_response_instance(data=request.data) - if not challenge.is_valid(): + valid = False + try: + challenge: ChallengeResponse = self.get_response_instance(data=request.data) + valid = challenge.is_valid() + except StageInvalidException as exc: + self.logger.debug("Got StageInvalidException", exc=exc) + return self.executor.stage_invalid() + if not valid: if self.executor.current_binding.invalid_response_action in [ InvalidResponseAction.RESTART, InvalidResponseAction.RESTART_WITH_CONTEXT, diff --git a/authentik/flows/views/executor.py b/authentik/flows/views/executor.py index 0133ed5c1..3dea776ae 100644 --- a/authentik/flows/views/executor.py +++ b/authentik/flows/views/executor.py @@ -362,10 +362,15 @@ class FlowExecutorView(APIView): def restart_flow(self, keep_context=False) -> HttpResponse: """Restart the currently active flow, optionally keeping the current context""" planner = FlowPlanner(self.flow) + planner.use_cache = False default_context = None if keep_context: default_context = self.plan.context - plan = planner.plan(self.request, default_context) + try: + plan = planner.plan(self.request, default_context) + except FlowNonApplicableException as exc: + self._logger.warning("f(exec): Flow restart not applicable to current user", exc=exc) + return self.handle_invalid_flow(exc) self.request.session[SESSION_KEY_PLAN] = plan kwargs = self.kwargs kwargs.update({"flow_slug": self.flow.slug}) diff --git a/authentik/stages/password/stage.py b/authentik/stages/password/stage.py index bdf3650bf..de80c6725 100644 --- a/authentik/stages/password/stage.py +++ b/authentik/stages/password/stage.py @@ -7,7 +7,7 @@ from django.core.exceptions import PermissionDenied from django.http import HttpRequest, HttpResponse from django.urls import reverse from django.utils.translation import gettext as _ -from rest_framework.exceptions import ErrorDetail, ValidationError +from rest_framework.exceptions import ValidationError from rest_framework.fields import CharField from sentry_sdk.hub import Hub from structlog.stdlib import get_logger @@ -20,6 +20,7 @@ from authentik.flows.challenge import ( ChallengeTypes, WithUserInfoChallenge, ) +from authentik.flows.exceptions import StageInvalidException from authentik.flows.models import Flow, FlowDesignation, Stage from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER from authentik.flows.stage import ChallengeStageView @@ -79,9 +80,52 @@ class PasswordChallenge(WithUserInfoChallenge): class PasswordChallengeResponse(ChallengeResponse): """Password challenge response""" + component = CharField(default="ak-stage-password") + password = CharField(trim_whitespace=False) - component = CharField(default="ak-stage-password") + def validate_password(self, password: str) -> str | None: + """Validate password and authenticate user""" + executor = self.stage.executor + if PLAN_CONTEXT_PENDING_USER not in executor.plan.context: + raise StageInvalidException("No pending user") + # Get the pending user's username, which is used as + # an Identifier by most authentication backends + pending_user: User = executor.plan.context[PLAN_CONTEXT_PENDING_USER] + auth_kwargs = { + "password": password, + "username": pending_user.username, + } + try: + with Hub.current.start_span( + op="authentik.stages.password.authenticate", + description="User authenticate call", + ): + user = authenticate( + self.stage.request, + executor.current_stage.backends, + executor.current_stage, + **auth_kwargs, + ) + except PermissionDenied as exc: + del auth_kwargs["password"] + # User was found, but permission was denied (i.e. user is not active) + self.stage.logger.debug("Denied access", **auth_kwargs) + raise StageInvalidException("Denied access") from exc + except ValidationError as exc: + del auth_kwargs["password"] + # User was found, authentication succeeded, but another signal raised an error + # (most likely LDAP) + self.stage.logger.debug("Validation error from signal", exc=exc, **auth_kwargs) + raise StageInvalidException("Validation error") from exc + if not user: + # No user was found -> invalid credentials + self.stage.logger.info("Invalid credentials") + raise ValidationError(_("Invalid password"), "invalid") + # User instance returned from authenticate() has .backend property set + executor.plan.context[PLAN_CONTEXT_PENDING_USER] = user + executor.plan.context[PLAN_CONTEXT_AUTHENTICATION_BACKEND] = user.backend + return password class PasswordStageView(ChallengeStageView): @@ -122,43 +166,4 @@ class PasswordStageView(ChallengeStageView): """Authenticate against django's authentication backend""" if PLAN_CONTEXT_PENDING_USER not in self.executor.plan.context: return self.executor.stage_invalid() - # Get the pending user's username, which is used as - # an Identifier by most authentication backends - pending_user: User = self.executor.plan.context[PLAN_CONTEXT_PENDING_USER] - auth_kwargs = { - "password": response.validated_data.get("password", None), - "username": pending_user.username, - } - try: - with Hub.current.start_span( - op="authentik.stages.password.authenticate", - description="User authenticate call", - ): - user = authenticate( - self.request, - self.executor.current_stage.backends, - self.executor.current_stage, - **auth_kwargs, - ) - except PermissionDenied: - del auth_kwargs["password"] - # User was found, but permission was denied (i.e. user is not active) - self.logger.debug("Denied access", **auth_kwargs) - return self.executor.stage_invalid() - except ValidationError as exc: - del auth_kwargs["password"] - # User was found, authentication succeeded, but another signal raised an error - # (most likely LDAP) - self.logger.debug("Validation error from signal", exc=exc, **auth_kwargs) - return self.executor.stage_invalid() - if not user: - # No user was found -> invalid credentials - self.logger.info("Invalid credentials") - # Manually inject error into form - response._errors.setdefault("password", []) - response._errors["password"].append(ErrorDetail(_("Invalid password"), "invalid")) - return self.challenge_invalid(response) - # 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()