flows: revert to sever-side redirects for security, pass querystring from client during flow plan

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>

authentik/flows/views.py

@@ -4,11 +4,13 @@ from typing import Any, Optional
 
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.http import Http404, HttpRequest, HttpResponse, HttpResponseRedirect
+from django.http.request import QueryDict
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
 from django.views.decorators.clickjacking import xframe_options_sameorigin
 from django.views.generic import View
+from drf_yasg2 import openapi
 from drf_yasg2.utils import no_body, swagger_auto_schema
 from rest_framework.permissions import AllowAny
 from rest_framework.views import APIView
@@ -101,6 +103,8 @@ class FlowExecutorView(APIView):
                 # To match behaviour with loading an empty flow plan from cache,
                 # we don't show an error message here, but rather call _flow_done()
                 return self._flow_done()
+            # Initial flow request, check if we have an upstream query string passed in
+            request.session[SESSION_KEY_GET] = QueryDict(request.GET.get("query", ""))
         # We don't save the Plan after getting the next stage
         # as it hasn't been successfully passed yet
         next_stage = self.plan.next(self.request)
@@ -121,8 +125,19 @@ class FlowExecutorView(APIView):
         return super().dispatch(request)
 
     @swagger_auto_schema(
-        responses={200: Challenge()},
+        responses={
+            200: Challenge(),
+        },
         request_body=no_body,
+        manual_parameters=[
+            openapi.Parameter(
+                "query",
+                openapi.IN_QUERY,
+                required=True,
+                description="Querystring as received",
+                type=openapi.TYPE_STRING,
+            )
+        ],
         operation_id="flows_executor_get",
     )
     def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:

authentik/stages/email/apps.py

@@ -16,7 +16,6 @@ class AuthentikStageEmailConfig(AppConfig):
     name = "authentik.stages.email"
     label = "authentik_stages_email"
     verbose_name = "authentik Stages.Email"
-    mountpoint = "stages/email/"
 
     def ready(self):
         import_module("authentik.stages.email.tasks")

authentik/stages/email/stage.py

@@ -45,7 +45,7 @@ class EmailStageView(ChallengeStageView):
     def get_full_url(self, **kwargs) -> str:
         """Get full URL to be used in template"""
         base_url = reverse(
-            "authentik_stages_email:from-email",
+            "authentik_core:if-flow",
             kwargs={"flow_slug": self.executor.flow.slug},
         )
         relative_url = f"{base_url}?{urlencode(kwargs)}"
@@ -66,7 +66,7 @@ class EmailStageView(ChallengeStageView):
             template_name=current_stage.template,
             to=[pending_user.email],
             template_context={
-                "url": self.get_full_url(**{QS_KEY_TOKEN: token.pk.hex}),
+                "url": self.get_full_url(**{QS_KEY_TOKEN: token.key}),
                 "user": pending_user,
                 "expires": token.expires,
             },
@@ -77,7 +77,7 @@ class EmailStageView(ChallengeStageView):
         # Check if the user came back from the email link to verify
         if QS_KEY_TOKEN in request.session.get(SESSION_KEY_GET, {}):
             token = get_object_or_404(
-                Token, pk=request.session[SESSION_KEY_GET][QS_KEY_TOKEN]
+                Token, key=request.session[SESSION_KEY_GET][QS_KEY_TOKEN]
             )
             self.executor.plan.context[PLAN_CONTEXT_PENDING_USER] = token.user
             token.delete()

authentik/stages/email/tests/test_stage.py

@@ -10,7 +10,7 @@ from authentik.core.models import Token, User
 from authentik.flows.markers import StageMarker
 from authentik.flows.models import Flow, FlowDesignation, FlowStageBinding
 from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan
-from authentik.flows.views import SESSION_KEY_PLAN
+from authentik.flows.views import SESSION_KEY_GET, SESSION_KEY_PLAN
 from authentik.stages.email.models import EmailStage
 from authentik.stages.email.stage import QS_KEY_TOKEN
 
@@ -104,17 +104,11 @@ class TestEmailStage(TestCase):
         )
         session = self.client.session
         session[SESSION_KEY_PLAN] = plan
+        token: Token = Token.objects.get(user=self.user)
+        session[SESSION_KEY_GET] = {QS_KEY_TOKEN: token.key}
         session.save()
 
         with patch("authentik.flows.views.FlowExecutorView.cancel", MagicMock()):
-            # Call the executor shell to preseed the session
-            url = reverse(
-                "authentik_stages_email:from-email",
-                kwargs={"flow_slug": self.flow.slug},
-            )
-            token = Token.objects.get(user=self.user)
-            url += f"?{QS_KEY_TOKEN}={token.pk.hex}"
-            self.client.get(url)
             # Call the actual executor to get the JSON Response
             response = self.client.get(
                 reverse(

authentik/stages/email/urls.py

@@ -1,8 +0,0 @@
-"""Email stage url patterns"""
-from django.urls import path
-
-from authentik.stages.email.views import FromEmailView
-
-urlpatterns = [
-    path("from-email/<slug:flow_slug>/", FromEmailView.as_view(), name="from-email"),
-]

authentik/stages/email/views.py

@@ -1,31 +0,0 @@
-"""Email stage views"""
-from django.http.request import HttpRequest
-from django.http.response import HttpResponse, HttpResponseBadRequest
-from django.shortcuts import get_object_or_404, redirect
-from django.views import View
-from structlog.stdlib import get_logger
-
-from authentik.core.models import Token
-from authentik.flows.views import SESSION_KEY_GET
-from authentik.stages.email.stage import QS_KEY_TOKEN
-
-LOGGER = get_logger()
-
-
-class FromEmailView(View):
-    """FromEmailView, this view is linked in the email confirmation link.
-    It is required because the flow executor does not pass query args to the API,
-    so this view gets called, checks for a Querystring and updates the plan
-    if everything is valid."""
-
-    def get(self, request: HttpRequest, flow_slug: str) -> HttpResponse:
-        """Check for ?token param and validate it."""
-        if QS_KEY_TOKEN not in request.GET:
-            LOGGER.debug("No token set")
-            return HttpResponseBadRequest()
-        # Lookup token here to quickly fail for invalid input
-        get_object_or_404(Token, pk=request.GET[QS_KEY_TOKEN])
-        if SESSION_KEY_GET not in request.session:
-            request.session[SESSION_KEY_GET] = {}
-        request.session[SESSION_KEY_GET][QS_KEY_TOKEN] = request.GET[QS_KEY_TOKEN]
-        return redirect("authentik_core:if-flow", flow_slug=flow_slug)

swagger.yaml

@@ -2744,7 +2744,12 @@ paths:
     get:
       operationId: flows_executor_get
       description: Get the next pending challenge from the currently active flow.
-      parameters: []
+      parameters:
+        - name: query
+          in: query
+          description: Querystring as received
+          required: true
+          type: string
       responses:
         '200':
           description: Challenge that gets sent to the client based on which stage

web/src/flows/FlowExecutor.ts

@@ -40,11 +40,8 @@ import { ifDefined } from "lit-html/directives/if-defined";
 import { until } from "lit-html/directives/until";
 import { TITLE_SUFFIX } from "../elements/router/RouterOutlet";
 import { AccessDeniedChallenge } from "./access_denied/FlowAccessDenied";
-import { getQueryVariables } from "./utils";
 import { SpinnerSize } from "../elements/Spinner";
 
-export const NEXT_ARG = "next";
-
 @customElement("ak-flow-executor")
 export class FlowExecutor extends LitElement implements StageHost {
 
@@ -129,7 +126,8 @@ export class FlowExecutor extends LitElement implements StageHost {
         });
         this.loading = true;
         new FlowsApi(DEFAULT_CONFIG).flowsExecutorGetRaw({
-            flowSlug: this.flowSlug
+            flowSlug: this.flowSlug,
+            query: window.location.search.substring(1),
         }).then((challengeRaw) => {
             return challengeRaw.raw.json();
         }).then((challenge) => {
@@ -168,29 +166,14 @@ export class FlowExecutor extends LitElement implements StageHost {
         </div>`;
     }
 
-    private redirect(challenge: RedirectChallenge): void {
-        // Check if there is a ?next arg and save it
-        // this is used for deep linking, if a user tries to access an application,
-        // but needs to authenticate first
-        const queryVars = getQueryVariables();
-        localStorage.clear();
-        if (NEXT_ARG in queryVars) {
-            const next = queryVars[NEXT_ARG];
-            console.debug("authentik/flows: redirecting to saved url", next);
-            window.location.assign(next);
-            return;
-        }
-        console.debug("authentik/flows: redirecting to url from server", challenge.to);
-        window.location.assign(challenge.to);
-    }
-
     renderChallenge(): TemplateResult {
         if (!this.challenge) {
             return html``;
         }
         switch (this.challenge.type) {
             case ChallengeTypeEnum.Redirect:
-                this.redirect(this.challenge as RedirectChallenge);
+                console.debug("authentik/flows: redirecting to url from server", (this.challenge as RedirectChallenge).to);
+                window.location.assign((this.challenge as RedirectChallenge).to);
                 return html`<ak-empty-state
                         ?loading=${true}
                         header=${gettext("Loading")}>

web/src/flows/utils.ts

@@ -1,10 +0,0 @@
-export function getQueryVariables(): Record<string, string> {
-    const query = window.location.search.substring(1);
-    const vars = query.split("&");
-    const entries: Record<string, string> = {};
-    for (let i = 0; i < vars.length; i++) {
-        const pair = vars[i].split("=");
-        entries[decodeURIComponent(pair[0])] = decodeURIComponent(pair[1]);
-    }
-    return entries;
-}