flows: revert to sever-side redirects for security, pass querystring from client during flow plan
Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
This commit is contained in:
parent
d44dc00757
commit
5b8b973345
|
@ -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:
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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"),
|
||||
]
|
|
@ -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)
|
|
@ -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
|
||||
|
|
|
@ -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")}>
|
||||
|
|
|
@ -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;
|
||||
}
|
Reference in New Issue