From 20e971f5ce166fba837c21907f66f6aa5c4d7690 Mon Sep 17 00:00:00 2001 From: Jens L Date: Tue, 28 Feb 2023 15:18:29 +0100 Subject: [PATCH] flows: planner error handling (#4812) * handle FlowNonApplicableException everywhere Signed-off-by: Jens Langhammer * make flow planner check authentication when no pending user is in planning context Signed-off-by: Jens Langhammer * add mailhog to e2e test services, remove local docker requirement Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/core/api/users.py | 17 ++++++--- authentik/core/views/apps.py | 22 ++++++----- authentik/flows/planner.py | 7 +++- authentik/flows/views/executor.py | 10 +++-- authentik/providers/oauth2/views/authorize.py | 30 ++++++++------- .../providers/oauth2/views/device_init.py | 37 ++++++++++++------- authentik/providers/saml/views/sso.py | 26 +++++++------ authentik/sources/saml/views.py | 6 ++- authentik/stages/email/tests/test_stage.py | 4 +- tests/e2e/docker-compose.yml | 11 +++++- tests/e2e/test_flows_authenticators.py | 3 -- tests/e2e/test_flows_enroll.py | 18 --------- tests/e2e/test_flows_login.py | 4 -- tests/e2e/test_flows_recovery.py | 18 --------- tests/e2e/test_flows_stage_setup.py | 4 -- 15 files changed, 109 insertions(+), 108 deletions(-) diff --git a/authentik/core/api/users.py b/authentik/core/api/users.py index 6765641f6..28d66ac71 100644 --- a/authentik/core/api/users.py +++ b/authentik/core/api/users.py @@ -68,6 +68,7 @@ from authentik.core.models import ( User, ) from authentik.events.models import EventAction +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import FlowToken from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlanner from authentik.flows.views.executor import QS_KEY_TOKEN @@ -326,12 +327,16 @@ class UserViewSet(UsedByMixin, ModelViewSet): user: User = self.get_object() planner = FlowPlanner(flow) planner.allow_empty_flows = True - plan = planner.plan( - self.request._request, - { - PLAN_CONTEXT_PENDING_USER: user, - }, - ) + try: + plan = planner.plan( + self.request._request, + { + PLAN_CONTEXT_PENDING_USER: user, + }, + ) + except FlowNonApplicableException: + LOGGER.warning("Recovery flow not applicable to user") + return None, None token, __ = FlowToken.objects.update_or_create( identifier=f"{user.uid}-password-reset", defaults={ diff --git a/authentik/core/views/apps.py b/authentik/core/views/apps.py index e558009cf..d55bcef70 100644 --- a/authentik/core/views/apps.py +++ b/authentik/core/views/apps.py @@ -11,6 +11,7 @@ from authentik.flows.challenge import ( HttpChallengeResponse, RedirectChallenge, ) +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import in_memory_stage from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, FlowPlanner from authentik.flows.stage import ChallengeStageView @@ -41,15 +42,18 @@ class RedirectToAppLaunch(View): flow = tenant.flow_authentication planner = FlowPlanner(flow) planner.allow_empty_flows = True - plan = planner.plan( - request, - { - PLAN_CONTEXT_APPLICATION: app, - PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") - % {"application": app.name}, - PLAN_CONTEXT_CONSENT_PERMISSIONS: [], - }, - ) + try: + plan = planner.plan( + request, + { + PLAN_CONTEXT_APPLICATION: app, + PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") + % {"application": app.name}, + PLAN_CONTEXT_CONSENT_PERMISSIONS: [], + }, + ) + except FlowNonApplicableException: + raise Http404 plan.insert_stage(in_memory_stage(RedirectToAppStage)) request.session[SESSION_KEY_PLAN] = plan return redirect_with_qs("authentik_core:if-flow", request.GET, flow_slug=flow.slug) diff --git a/authentik/flows/planner.py b/authentik/flows/planner.py index 708ca0d22..2269837d8 100644 --- a/authentik/flows/planner.py +++ b/authentik/flows/planner.py @@ -147,7 +147,6 @@ class FlowPlanner: ) -> FlowPlan: """Check each of the flows' policies, check policies for each stage with PolicyBinding and return ordered list""" - self._check_authentication(request) with Hub.current.start_span( op="authentik.flow.planner.plan", description=self.flow.slug ) as span: @@ -165,6 +164,12 @@ class FlowPlanner: user = default_context[PLAN_CONTEXT_PENDING_USER] else: user = request.user + # We only need to check the flow authentication if it's planned without a user + # in the context, as a user in the context can only be set via the explicit code API + # or if a flow is restarted due to `invalid_response_action` being set to + # `restart_with_context`, which can only happen if the user was already authorized + # to use the flow + self._check_authentication(request) # First off, check the flow's direct policy bindings # to make sure the user even has access to the flow engine = PolicyEngine(self.flow, user, request) diff --git a/authentik/flows/views/executor.py b/authentik/flows/views/executor.py index 167f7b5c7..1505b0ae1 100644 --- a/authentik/flows/views/executor.py +++ b/authentik/flows/views/executor.py @@ -561,9 +561,13 @@ class ConfigureFlowInitView(LoginRequiredMixin, View): LOGGER.debug("Stage has no configure_flow set", stage=stage) raise Http404 - plan = FlowPlanner(stage.configure_flow).plan( - request, {PLAN_CONTEXT_PENDING_USER: request.user} - ) + try: + plan = FlowPlanner(stage.configure_flow).plan( + request, {PLAN_CONTEXT_PENDING_USER: request.user} + ) + except FlowNonApplicableException: + LOGGER.warning("Flow not applicable to user") + raise Http404 request.session[SESSION_KEY_PLAN] = plan return redirect_with_qs( "authentik_core:if-flow", diff --git a/authentik/providers/oauth2/views/authorize.py b/authentik/providers/oauth2/views/authorize.py index 9743c773a..5c7497732 100644 --- a/authentik/providers/oauth2/views/authorize.py +++ b/authentik/providers/oauth2/views/authorize.py @@ -24,6 +24,7 @@ from authentik.flows.challenge import ( ChallengeTypes, HttpChallengeResponse, ) +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import in_memory_stage from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, PLAN_CONTEXT_SSO, FlowPlanner from authentik.flows.stage import StageView @@ -373,19 +374,22 @@ class AuthorizationFlowInitView(PolicyAccessView): # Regardless, we start the planner and return to it planner = FlowPlanner(self.provider.authorization_flow) planner.allow_empty_flows = True - plan = planner.plan( - self.request, - { - PLAN_CONTEXT_SSO: True, - PLAN_CONTEXT_APPLICATION: self.application, - # OAuth2 related params - PLAN_CONTEXT_PARAMS: self.params, - # Consent related params - PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") - % {"application": self.application.name}, - PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions, - }, - ) + try: + plan = planner.plan( + self.request, + { + PLAN_CONTEXT_SSO: True, + PLAN_CONTEXT_APPLICATION: self.application, + # OAuth2 related params + PLAN_CONTEXT_PARAMS: self.params, + # Consent related params + PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") + % {"application": self.application.name}, + PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions, + }, + ) + except FlowNonApplicableException: + return self.handle_no_permission_authenticated() # OpenID clients can specify a `prompt` parameter, and if its set to consent we # need to inject a consent stage if PROMPT_CONSENT in self.params.prompt: diff --git a/authentik/providers/oauth2/views/device_init.py b/authentik/providers/oauth2/views/device_init.py index c1be90370..c9f240661 100644 --- a/authentik/providers/oauth2/views/device_init.py +++ b/authentik/providers/oauth2/views/device_init.py @@ -10,6 +10,7 @@ from structlog.stdlib import get_logger from authentik.core.models import Application from authentik.flows.challenge import Challenge, ChallengeResponse, ChallengeTypes +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import in_memory_stage from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, PLAN_CONTEXT_SSO, FlowPlanner from authentik.flows.stage import ChallengeStageView @@ -57,19 +58,23 @@ def validate_code(code: int, request: HttpRequest) -> Optional[HttpResponse]: scope_descriptions = UserInfoView().get_scope_descriptions(token.scope) planner = FlowPlanner(token.provider.authorization_flow) planner.allow_empty_flows = True - plan = planner.plan( - request, - { - PLAN_CONTEXT_SSO: True, - PLAN_CONTEXT_APPLICATION: app, - # OAuth2 related params - PLAN_CONTEXT_DEVICE: token, - # Consent related params - PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") - % {"application": app.name}, - PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions, - }, - ) + try: + plan = planner.plan( + request, + { + PLAN_CONTEXT_SSO: True, + PLAN_CONTEXT_APPLICATION: app, + # OAuth2 related params + PLAN_CONTEXT_DEVICE: token, + # Consent related params + PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") + % {"application": app.name}, + PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions, + }, + ) + except FlowNonApplicableException: + LOGGER.warning("Flow not applicable to user") + return None plan.insert_stage(in_memory_stage(OAuthDeviceCodeFinishStage)) request.session[SESSION_KEY_PLAN] = plan return redirect_with_qs( @@ -97,7 +102,11 @@ class DeviceEntryView(View): # Regardless, we start the planner and return to it planner = FlowPlanner(device_flow) planner.allow_empty_flows = True - plan = planner.plan(self.request) + try: + plan = planner.plan(self.request) + except FlowNonApplicableException: + LOGGER.warning("Flow not applicable to user") + return HttpResponse(status=404) plan.append_stage(in_memory_stage(OAuthDeviceCodeStage)) self.request.session[SESSION_KEY_PLAN] = plan diff --git a/authentik/providers/saml/views/sso.py b/authentik/providers/saml/views/sso.py index 8ddd9e1b7..99844150a 100644 --- a/authentik/providers/saml/views/sso.py +++ b/authentik/providers/saml/views/sso.py @@ -1,7 +1,7 @@ """authentik SAML IDP Views""" from typing import Optional -from django.http import HttpRequest, HttpResponse +from django.http import Http404, HttpRequest, HttpResponse from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator from django.utils.translation import gettext as _ @@ -11,6 +11,7 @@ from structlog.stdlib import get_logger from authentik.core.models import Application from authentik.events.models import Event, EventAction +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import in_memory_stage from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, PLAN_CONTEXT_SSO, FlowPlanner from authentik.flows.views.executor import SESSION_KEY_PLAN, SESSION_KEY_POST @@ -60,16 +61,19 @@ class SAMLSSOView(PolicyAccessView): # Regardless, we start the planner and return to it planner = FlowPlanner(self.provider.authorization_flow) planner.allow_empty_flows = True - plan = planner.plan( - request, - { - PLAN_CONTEXT_SSO: True, - PLAN_CONTEXT_APPLICATION: self.application, - PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") - % {"application": self.application.name}, - PLAN_CONTEXT_CONSENT_PERMISSIONS: [], - }, - ) + try: + plan = planner.plan( + request, + { + PLAN_CONTEXT_SSO: True, + PLAN_CONTEXT_APPLICATION: self.application, + PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.") + % {"application": self.application.name}, + PLAN_CONTEXT_CONSENT_PERMISSIONS: [], + }, + ) + except FlowNonApplicableException: + raise Http404 plan.append_stage(in_memory_stage(SAMLFlowFinalView)) request.session[SESSION_KEY_PLAN] = plan return redirect_with_qs( diff --git a/authentik/sources/saml/views.py b/authentik/sources/saml/views.py index ed13f6917..374d67fcb 100644 --- a/authentik/sources/saml/views.py +++ b/authentik/sources/saml/views.py @@ -22,6 +22,7 @@ from authentik.flows.challenge import ( ChallengeResponse, ChallengeTypes, ) +from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import in_memory_stage from authentik.flows.planner import ( PLAN_CONTEXT_REDIRECT, @@ -87,7 +88,10 @@ class InitiateView(View): # We run the Flow planner here so we can pass the Pending user in the context planner = FlowPlanner(source.pre_authentication_flow) planner.allow_empty_flows = True - plan = planner.plan(self.request, kwargs) + try: + plan = planner.plan(self.request, kwargs) + except FlowNonApplicableException: + raise Http404 for stage in stages_to_append: plan.append_stage(stage) self.request.session[SESSION_KEY_PLAN] = plan diff --git a/authentik/stages/email/tests/test_stage.py b/authentik/stages/email/tests/test_stage.py index 9164bf85e..45023a9cb 100644 --- a/authentik/stages/email/tests/test_stage.py +++ b/authentik/stages/email/tests/test_stage.py @@ -13,9 +13,9 @@ from authentik.flows.markers import StageMarker from authentik.flows.models import FlowDesignation, FlowStageBinding from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from authentik.flows.tests import FlowTestCase -from authentik.flows.views.executor import SESSION_KEY_PLAN +from authentik.flows.views.executor import QS_KEY_TOKEN, SESSION_KEY_PLAN from authentik.stages.email.models import EmailStage -from authentik.stages.email.stage import PLAN_CONTEXT_EMAIL_OVERRIDE, QS_KEY_TOKEN +from authentik.stages.email.stage import PLAN_CONTEXT_EMAIL_OVERRIDE class TestEmailStage(FlowTestCase): diff --git a/tests/e2e/docker-compose.yml b/tests/e2e/docker-compose.yml index 068331eec..c7146b83c 100644 --- a/tests/e2e/docker-compose.yml +++ b/tests/e2e/docker-compose.yml @@ -2,8 +2,17 @@ version: '3.7' services: chrome: - image: selenium/standalone-chrome:103.0-chromedriver-103.0 + image: selenium/standalone-chrome:110.0 volumes: - /dev/shm:/dev/shm network_mode: host restart: always + mailhog: + image: mailhog/mailhog:v1.0.1 + ports: + - 1025:1025 + - 8025:8025 + healthcheck: + test: ["CMD", "wget", "--spider", "http://localhost:8025"] + interval: 5s + start_period: 1s diff --git a/tests/e2e/test_flows_authenticators.py b/tests/e2e/test_flows_authenticators.py index 58754b791..100b489c9 100644 --- a/tests/e2e/test_flows_authenticators.py +++ b/tests/e2e/test_flows_authenticators.py @@ -1,8 +1,6 @@ """test flow with otp stages""" from base64 import b32decode -from sys import platform from time import sleep -from unittest.case import skipUnless from urllib.parse import parse_qs, urlparse from django_otp.oath import TOTP @@ -20,7 +18,6 @@ from authentik.stages.authenticator_totp.models import AuthenticatorTOTPStage from tests.e2e.utils import SeleniumTestCase, retry -@skipUnless(platform.startswith("linux"), "requires local docker") class TestFlowsAuthenticator(SeleniumTestCase): """test flow with otp stages""" diff --git a/tests/e2e/test_flows_enroll.py b/tests/e2e/test_flows_enroll.py index 5f9865c5e..d138c7824 100644 --- a/tests/e2e/test_flows_enroll.py +++ b/tests/e2e/test_flows_enroll.py @@ -1,11 +1,7 @@ """Test Enroll flow""" -from sys import platform from time import sleep -from typing import Any, Optional -from unittest.case import skipUnless from django.test import override_settings -from docker.types import Healthcheck from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as ec from selenium.webdriver.support.wait import WebDriverWait @@ -17,23 +13,9 @@ from authentik.stages.identification.models import IdentificationStage from tests.e2e.utils import SeleniumTestCase, retry -@skipUnless(platform.startswith("linux"), "requires local docker") class TestFlowsEnroll(SeleniumTestCase): """Test Enroll flow""" - def get_container_specs(self) -> Optional[dict[str, Any]]: - return { - "image": "mailhog/mailhog:v1.0.1", - "detach": True, - "network_mode": "host", - "auto_remove": True, - "healthcheck": Healthcheck( - test=["CMD", "wget", "--spider", "http://localhost:8025"], - interval=5 * 100 * 1000000, - start_period=1 * 100 * 1000000, - ), - } - @retry() @apply_blueprint( "default/flow-default-authentication-flow.yaml", diff --git a/tests/e2e/test_flows_login.py b/tests/e2e/test_flows_login.py index 194c27b53..aa477a519 100644 --- a/tests/e2e/test_flows_login.py +++ b/tests/e2e/test_flows_login.py @@ -1,12 +1,8 @@ """test default login flow""" -from sys import platform -from unittest.case import skipUnless - from authentik.blueprints.tests import apply_blueprint from tests.e2e.utils import SeleniumTestCase, retry -@skipUnless(platform.startswith("linux"), "requires local docker") class TestFlowsLogin(SeleniumTestCase): """test default login flow""" diff --git a/tests/e2e/test_flows_recovery.py b/tests/e2e/test_flows_recovery.py index f548b3a28..19d9b4eba 100644 --- a/tests/e2e/test_flows_recovery.py +++ b/tests/e2e/test_flows_recovery.py @@ -1,11 +1,7 @@ """Test recovery flow""" -from sys import platform from time import sleep -from typing import Any, Optional -from unittest.case import skipUnless from django.test import override_settings -from docker.types import Healthcheck from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as ec from selenium.webdriver.support.wait import WebDriverWait @@ -19,23 +15,9 @@ from authentik.stages.identification.models import IdentificationStage from tests.e2e.utils import SeleniumTestCase, retry -@skipUnless(platform.startswith("linux"), "requires local docker") class TestFlowsRecovery(SeleniumTestCase): """Test Recovery flow""" - def get_container_specs(self) -> Optional[dict[str, Any]]: - return { - "image": "mailhog/mailhog:v1.0.1", - "detach": True, - "network_mode": "host", - "auto_remove": True, - "healthcheck": Healthcheck( - test=["CMD", "wget", "--spider", "http://localhost:8025"], - interval=5 * 100 * 1000000, - start_period=1 * 100 * 1000000, - ), - } - def initial_stages(self, user: User): """Fill out initial stages""" # Identification stage, click recovery diff --git a/tests/e2e/test_flows_stage_setup.py b/tests/e2e/test_flows_stage_setup.py index b33630dab..f0c791ffe 100644 --- a/tests/e2e/test_flows_stage_setup.py +++ b/tests/e2e/test_flows_stage_setup.py @@ -1,7 +1,4 @@ """test stage setup flows (password change)""" -from sys import platform -from unittest.case import skipUnless - from selenium.webdriver.common.by import By from selenium.webdriver.common.keys import Keys @@ -13,7 +10,6 @@ from authentik.stages.password.models import PasswordStage from tests.e2e.utils import SeleniumTestCase, retry -@skipUnless(platform.startswith("linux"), "requires local docker") class TestFlowsStageSetup(SeleniumTestCase): """test stage setup flows"""