mirror of
https://github.com/goauthentik/authentik
synced 2026-04-25 17:15:26 +02:00
flows: add warning message for expired password reset links (#21395)
* flows: add warning message for expired password reset links Fixes #21306 * Replace token expiry check with REQUIRE_TOKEN authentication requirement Incorporate review comments to move expired/invalid token handling from executor-level check to flow planner authentication requirement. This avoids disclosing whether a token ever existed and handles already-cleaned-up tokens. * The fix was changing gettext_lazy to gettext * remove unneeded migration Signed-off-by: Jens Langhammer <jens@goauthentik.io> * update form Signed-off-by: Jens Langhammer <jens@goauthentik.io> --------- Signed-off-by: Jens Langhammer <jens@goauthentik.io> Co-authored-by: Jens Langhammer <jens@goauthentik.io>
This commit is contained in:
@@ -11,6 +11,10 @@ class FlowNonApplicableException(SentryIgnoredException):
|
||||
|
||||
policy_result: PolicyResult | None = None
|
||||
|
||||
def __init__(self, policy_result: PolicyResult | None = None, *args):
|
||||
super().__init__(*args)
|
||||
self.policy_result = policy_result
|
||||
|
||||
@property
|
||||
def messages(self) -> str:
|
||||
"""Get messages from policy result, fallback to generic reason"""
|
||||
|
||||
@@ -42,6 +42,7 @@ class Migration(migrations.Migration):
|
||||
("require_superuser", "Require Superuser"),
|
||||
("require_redirect", "Require Redirect"),
|
||||
("require_outpost", "Require Outpost"),
|
||||
("require_token", "Require Token"),
|
||||
],
|
||||
default="none",
|
||||
help_text="Required level of authentication and authorization to access a flow.",
|
||||
|
||||
@@ -40,6 +40,7 @@ class FlowAuthenticationRequirement(models.TextChoices):
|
||||
REQUIRE_SUPERUSER = "require_superuser"
|
||||
REQUIRE_REDIRECT = "require_redirect"
|
||||
REQUIRE_OUTPOST = "require_outpost"
|
||||
REQUIRE_TOKEN = "require_token"
|
||||
|
||||
|
||||
class NotConfiguredAction(models.TextChoices):
|
||||
|
||||
@@ -5,6 +5,7 @@ from typing import TYPE_CHECKING, Any
|
||||
|
||||
from django.core.cache import cache
|
||||
from django.http import HttpRequest, HttpResponse
|
||||
from django.utils.translation import gettext as _
|
||||
from sentry_sdk import start_span
|
||||
from sentry_sdk.tracing import Span
|
||||
from structlog.stdlib import BoundLogger, get_logger
|
||||
@@ -26,6 +27,7 @@ from authentik.lib.config import CONFIG
|
||||
from authentik.lib.utils.urls import redirect_with_qs
|
||||
from authentik.outposts.models import Outpost
|
||||
from authentik.policies.engine import PolicyEngine
|
||||
from authentik.policies.types import PolicyResult
|
||||
from authentik.root.middleware import ClientIPMiddleware
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -226,6 +228,15 @@ class FlowPlanner:
|
||||
and context.get(PLAN_CONTEXT_IS_REDIRECTED) is None
|
||||
):
|
||||
raise FlowNonApplicableException()
|
||||
if (
|
||||
self.flow.authentication == FlowAuthenticationRequirement.REQUIRE_TOKEN
|
||||
and context.get(PLAN_CONTEXT_IS_RESTORED) is None
|
||||
):
|
||||
raise FlowNonApplicableException(
|
||||
PolicyResult(
|
||||
False, _("This link is invalid or has expired. Please request a new one.")
|
||||
)
|
||||
)
|
||||
outpost_user = ClientIPMiddleware.get_outpost_user(request)
|
||||
if self.flow.authentication == FlowAuthenticationRequirement.REQUIRE_OUTPOST:
|
||||
if not outpost_user:
|
||||
@@ -273,9 +284,7 @@ class FlowPlanner:
|
||||
engine.build()
|
||||
result = engine.result
|
||||
if not result.passing:
|
||||
exc = FlowNonApplicableException()
|
||||
exc.policy_result = result
|
||||
raise exc
|
||||
raise FlowNonApplicableException(result)
|
||||
# User is passing so far, check if we have a cached plan
|
||||
cached_plan_key = cache_key(self.flow, user)
|
||||
cached_plan = cache.get(cached_plan_key, None)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""flow views tests"""
|
||||
|
||||
from datetime import timedelta
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
from urllib.parse import urlencode
|
||||
|
||||
@@ -7,6 +8,7 @@ from django.http import HttpRequest, HttpResponse
|
||||
from django.test import override_settings
|
||||
from django.test.client import RequestFactory
|
||||
from django.urls import reverse
|
||||
from django.utils.timezone import now
|
||||
from rest_framework.exceptions import ParseError
|
||||
|
||||
from authentik.core.models import Group, User
|
||||
@@ -17,6 +19,7 @@ from authentik.flows.models import (
|
||||
FlowDeniedAction,
|
||||
FlowDesignation,
|
||||
FlowStageBinding,
|
||||
FlowToken,
|
||||
InvalidResponseAction,
|
||||
)
|
||||
from authentik.flows.planner import FlowPlan, FlowPlanner
|
||||
@@ -24,6 +27,7 @@ from authentik.flows.stage import PLAN_CONTEXT_PENDING_USER_IDENTIFIER, StageVie
|
||||
from authentik.flows.tests import FlowTestCase
|
||||
from authentik.flows.views.executor import (
|
||||
NEXT_ARG_NAME,
|
||||
QS_KEY_TOKEN,
|
||||
QS_QUERY,
|
||||
SESSION_KEY_PLAN,
|
||||
FlowExecutorView,
|
||||
@@ -740,3 +744,77 @@ class TestFlowExecutor(FlowTestCase):
|
||||
"title": flow.title,
|
||||
},
|
||||
)
|
||||
|
||||
@patch(
|
||||
"authentik.flows.views.executor.to_stage_response",
|
||||
TO_STAGE_RESPONSE_MOCK,
|
||||
)
|
||||
def test_expired_flow_token(self):
|
||||
"""Test that an expired flow token shows an appropriate error message"""
|
||||
flow = create_test_flow(
|
||||
FlowDesignation.RECOVERY,
|
||||
authentication=FlowAuthenticationRequirement.REQUIRE_TOKEN,
|
||||
)
|
||||
user = create_test_user()
|
||||
plan = FlowPlan(flow_pk=flow.pk.hex, bindings=[], markers=[])
|
||||
|
||||
token = FlowToken.objects.create(
|
||||
user=user,
|
||||
identifier=generate_id(),
|
||||
flow=flow,
|
||||
_plan=FlowToken.pickle(plan),
|
||||
expires=now() - timedelta(hours=1),
|
||||
)
|
||||
|
||||
url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug})
|
||||
response = self.client.get(
|
||||
url + f"?{urlencode({QS_QUERY: urlencode({QS_KEY_TOKEN: token.key})})}"
|
||||
)
|
||||
self.assertStageResponse(
|
||||
response,
|
||||
flow,
|
||||
component="ak-stage-access-denied",
|
||||
error_message="This link is invalid or has expired. Please request a new one.",
|
||||
)
|
||||
|
||||
@patch(
|
||||
"authentik.flows.views.executor.to_stage_response",
|
||||
TO_STAGE_RESPONSE_MOCK,
|
||||
)
|
||||
def test_invalid_flow_token_require_token(self):
|
||||
"""Test that an invalid/nonexistent token on a REQUIRE_TOKEN flow shows error"""
|
||||
flow = create_test_flow(
|
||||
FlowDesignation.RECOVERY,
|
||||
authentication=FlowAuthenticationRequirement.REQUIRE_TOKEN,
|
||||
)
|
||||
|
||||
url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug})
|
||||
response = self.client.get(
|
||||
url + f"?{urlencode({QS_QUERY: urlencode({QS_KEY_TOKEN: 'invalid-token'})})}"
|
||||
)
|
||||
self.assertStageResponse(
|
||||
response,
|
||||
flow,
|
||||
component="ak-stage-access-denied",
|
||||
error_message="This link is invalid or has expired. Please request a new one.",
|
||||
)
|
||||
|
||||
@patch(
|
||||
"authentik.flows.views.executor.to_stage_response",
|
||||
TO_STAGE_RESPONSE_MOCK,
|
||||
)
|
||||
def test_no_token_require_token(self):
|
||||
"""Test that accessing a REQUIRE_TOKEN flow without any token shows error"""
|
||||
flow = create_test_flow(
|
||||
FlowDesignation.RECOVERY,
|
||||
authentication=FlowAuthenticationRequirement.REQUIRE_TOKEN,
|
||||
)
|
||||
|
||||
url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug})
|
||||
response = self.client.get(url)
|
||||
self.assertStageResponse(
|
||||
response,
|
||||
flow,
|
||||
component="ak-stage-access-denied",
|
||||
error_message="This link is invalid or has expired. Please request a new one.",
|
||||
)
|
||||
|
||||
@@ -26,6 +26,7 @@ from authentik.flows.models import (
|
||||
)
|
||||
from authentik.flows.planner import (
|
||||
PLAN_CONTEXT_IS_REDIRECTED,
|
||||
PLAN_CONTEXT_IS_RESTORED,
|
||||
PLAN_CONTEXT_PENDING_USER,
|
||||
FlowPlanner,
|
||||
cache_key,
|
||||
@@ -129,6 +130,22 @@ class TestFlowPlanner(TestCase):
|
||||
planner.allow_empty_flows = True
|
||||
planner.plan(request)
|
||||
|
||||
def test_authentication_require_token(self):
|
||||
"""Test flow authentication (require_token)"""
|
||||
flow = create_test_flow()
|
||||
flow.authentication = FlowAuthenticationRequirement.REQUIRE_TOKEN
|
||||
request = self.request_factory.get(
|
||||
reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}),
|
||||
)
|
||||
planner = FlowPlanner(flow)
|
||||
planner.allow_empty_flows = True
|
||||
|
||||
with self.assertRaises(FlowNonApplicableException):
|
||||
planner.plan(request)
|
||||
|
||||
context = {PLAN_CONTEXT_IS_RESTORED: True}
|
||||
planner.plan(request, context)
|
||||
|
||||
@patch(
|
||||
"authentik.policies.engine.PolicyEngine.result",
|
||||
POLICY_RETURN_FALSE,
|
||||
|
||||
@@ -62,6 +62,7 @@ from authentik.policies.engine import PolicyEngine
|
||||
LOGGER = get_logger()
|
||||
# Argument used to redirect user after login
|
||||
NEXT_ARG_NAME = "next"
|
||||
|
||||
SESSION_KEY_PLAN = "authentik/flows/plan"
|
||||
SESSION_KEY_GET = "authentik/flows/get"
|
||||
SESSION_KEY_POST = "authentik/flows/post"
|
||||
|
||||
@@ -8430,7 +8430,8 @@
|
||||
"require_unauthenticated",
|
||||
"require_superuser",
|
||||
"require_redirect",
|
||||
"require_outpost"
|
||||
"require_outpost",
|
||||
"require_token"
|
||||
],
|
||||
"title": "Authentication",
|
||||
"description": "Required level of authentication and authorization to access a flow."
|
||||
|
||||
@@ -23,6 +23,7 @@ export const AuthenticationEnum = {
|
||||
RequireSuperuser: "require_superuser",
|
||||
RequireRedirect: "require_redirect",
|
||||
RequireOutpost: "require_outpost",
|
||||
RequireToken: "require_token",
|
||||
UnknownDefaultOpenApi: "11184809",
|
||||
} as const;
|
||||
export type AuthenticationEnum = (typeof AuthenticationEnum)[keyof typeof AuthenticationEnum];
|
||||
|
||||
@@ -34355,6 +34355,7 @@ components:
|
||||
- require_superuser
|
||||
- require_redirect
|
||||
- require_outpost
|
||||
- require_token
|
||||
type: string
|
||||
AuthenticatorAttachmentEnum:
|
||||
enum:
|
||||
|
||||
@@ -218,6 +218,15 @@ export class FlowForm extends WithCapabilitiesConfig(ModelForm<Flow, string>) {
|
||||
>
|
||||
${msg("Require Outpost (flow can only be executed from an outpost)")}
|
||||
</option>
|
||||
<option
|
||||
value=${AuthenticationEnum.RequireToken}
|
||||
?selected=${this.instance?.authentication ===
|
||||
AuthenticationEnum.RequireToken}
|
||||
>
|
||||
${msg(
|
||||
"Require Flow token (flow can only be executed from a generated recovery link)",
|
||||
)}
|
||||
</option>
|
||||
</select>
|
||||
<p class="pf-c-form__helper-text">
|
||||
${msg("Required authentication level for this flow.")}
|
||||
|
||||
Reference in New Issue
Block a user