From 14a4047bdd30d15cda1553883c92146d50e0b022 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 3 Jul 2022 21:36:13 +0200 Subject: [PATCH] flows: show messages from ak_message when flow is denied fallback to same generic message closes #3197 Signed-off-by: Jens Langhammer --- authentik/core/sources/flow_manager.py | 2 +- authentik/flows/api/flows.py | 2 +- authentik/flows/exceptions.py | 8 ++++++++ authentik/flows/planner.py | 2 +- authentik/flows/tests/test_executor.py | 5 ++--- authentik/flows/views/executor.py | 5 ++--- authentik/providers/oauth2/models.py | 5 ++++- 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/authentik/core/sources/flow_manager.py b/authentik/core/sources/flow_manager.py index e0a6b4ff2..d22ad533d 100644 --- a/authentik/core/sources/flow_manager.py +++ b/authentik/core/sources/flow_manager.py @@ -165,7 +165,7 @@ class SourceFlowManager: self._logger.debug("Handling enrollment of new user") return self.handle_enroll(connection) except FlowNonApplicableException as exc: - self._logger.warning("Flow non applicable", exc=exc) + self._logger.warning("Flow non applicable", exc=exc, result=exc.policy_result) return self.error_handler(exc, exc.policy_result) # Default case, assume deny error = ( diff --git a/authentik/flows/api/flows.py b/authentik/flows/api/flows.py index bbb0e49c0..296a6976f 100644 --- a/authentik/flows/api/flows.py +++ b/authentik/flows/api/flows.py @@ -372,7 +372,7 @@ class FlowViewSet(UsedByMixin, ModelViewSet): request, _( "Flow not applicable to current user/request: %(messages)s" - % {"messages": str(exc)} + % {"messages": exc.messages} ), ) return Response( diff --git a/authentik/flows/exceptions.py b/authentik/flows/exceptions.py index ddb8b4412..6ea03429e 100644 --- a/authentik/flows/exceptions.py +++ b/authentik/flows/exceptions.py @@ -1,4 +1,5 @@ """flow exceptions""" +from django.utils.translation import gettext_lazy as _ from authentik.lib.sentry import SentryIgnoredException from authentik.policies.types import PolicyResult @@ -9,6 +10,13 @@ class FlowNonApplicableException(SentryIgnoredException): policy_result: PolicyResult + @property + def messages(self) -> str: + """Get messages from policy result, fallback to generic reason""" + if len(self.policy_result.messages) < 1: + return _("Flow does not apply to current user (denied by policy).") + return "\n".join(self.policy_result.messages) + class EmptyFlowException(SentryIgnoredException): """Flow has no stages.""" diff --git a/authentik/flows/planner.py b/authentik/flows/planner.py index e64681575..4a120b418 100644 --- a/authentik/flows/planner.py +++ b/authentik/flows/planner.py @@ -147,7 +147,7 @@ class FlowPlanner: engine.build() result = engine.result if not result.passing: - exc = FlowNonApplicableException(",".join(result.messages)) + exc = FlowNonApplicableException() exc.policy_result = result raise exc # User is passing so far, check if we have a cached plan diff --git a/authentik/flows/tests/test_executor.py b/authentik/flows/tests/test_executor.py index 4387e9d28..5ac07cd75 100644 --- a/authentik/flows/tests/test_executor.py +++ b/authentik/flows/tests/test_executor.py @@ -7,7 +7,6 @@ from django.urls import reverse from authentik.core.models import User from authentik.core.tests.utils import create_test_flow -from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.markers import ReevaluateMarker, StageMarker from authentik.flows.models import ( FlowDeniedAction, @@ -29,7 +28,7 @@ from authentik.stages.deny.models import DenyStage from authentik.stages.dummy.models import DummyStage from authentik.stages.identification.models import IdentificationStage, UserFields -POLICY_RETURN_FALSE = PropertyMock(return_value=PolicyResult(False)) +POLICY_RETURN_FALSE = PropertyMock(return_value=PolicyResult(False, "foo")) POLICY_RETURN_TRUE = MagicMock(return_value=PolicyResult(True)) @@ -93,7 +92,7 @@ class TestFlowExecutor(FlowTestCase): self.assertStageResponse( response, flow=flow, - error_message=FlowNonApplicableException.__doc__, + error_message="foo", component="ak-stage-access-denied", ) diff --git a/authentik/flows/views/executor.py b/authentik/flows/views/executor.py index a75568bae..908af4491 100644 --- a/authentik/flows/views/executor.py +++ b/authentik/flows/views/executor.py @@ -132,7 +132,7 @@ class FlowExecutorView(APIView): self._logger = get_logger().bind(flow_slug=flow_slug) set_tag("authentik.flow", self.flow.slug) - def handle_invalid_flow(self, exc: BaseException) -> HttpResponse: + def handle_invalid_flow(self, exc: FlowNonApplicableException) -> HttpResponse: """When a flow is non-applicable check if user is on the correct domain""" if self.flow.denied_action in [ FlowDeniedAction.CONTINUE, @@ -146,8 +146,7 @@ class FlowExecutorView(APIView): return to_stage_response( self.request, redirect(reverse("authentik_core:root-redirect")) ) - message = exc.__doc__ if exc.__doc__ else str(exc) - return to_stage_response(self.request, self.stage_invalid(error_message=message)) + return to_stage_response(self.request, self.stage_invalid(error_message=exc.messages)) def _check_flow_token(self, key: str) -> Optional[FlowPlan]: """Check if the user is using a flow token to restore a plan""" diff --git a/authentik/providers/oauth2/models.py b/authentik/providers/oauth2/models.py index 577cd72fb..43b80754e 100644 --- a/authentik/providers/oauth2/models.py +++ b/authentik/providers/oauth2/models.py @@ -143,7 +143,10 @@ class OAuth2Provider(Provider): choices=ClientTypes.choices, default=ClientTypes.CONFIDENTIAL, verbose_name=_("Client Type"), - help_text=_(ClientTypes.__doc__), + help_text=_( + "Confidential clients are capable of maintaining the confidentiality " + "of their credentials. Public clients are incapable" + ), ) client_id = models.CharField( max_length=255,