From 5af907db0cae692795cd1836e0c24a901a274c6f Mon Sep 17 00:00:00 2001 From: "Jens L." Date: Fri, 28 Mar 2025 14:16:13 +0100 Subject: [PATCH] stages/identification: refresh captcha on failure (#13697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor cleanup behavior after stage form submit * refresh captcha on failing Identification stage * Revert "stages/identification: check captcha after checking authentication (#13533)" This reverts commit b7beac67954803365d1250424b43551f9875d270. Including a Captcha stage in an Identification stage is partially to prevent password spraying attacks. The reverted commit negated this feature to fix a UX bug. After 6fde42a9170, the functionality can now be reinstated. --------- Co-authored-by: Simonyi Gergő --- authentik/stages/identification/stage.py | 49 ++++++----- web/src/flow/stages/base.ts | 10 ++- web/src/flow/stages/captcha/CaptchaStage.ts | 83 +++++++++++++++---- .../identification/IdentificationStage.ts | 9 +- 4 files changed, 108 insertions(+), 43 deletions(-) diff --git a/authentik/stages/identification/stage.py b/authentik/stages/identification/stage.py index 02546a0c2d..e8841bcd58 100644 --- a/authentik/stages/identification/stage.py +++ b/authentik/stages/identification/stage.py @@ -142,35 +142,38 @@ class IdentificationChallengeResponse(ChallengeResponse): raise ValidationError("Failed to authenticate.") self.pre_user = pre_user - # Password check - if current_stage.password_stage: - password = attrs.get("password", None) - if not password: - self.stage.logger.warning("Password not set for ident+auth attempt") - try: - with start_span( - op="authentik.stages.identification.authenticate", - name="User authenticate call (combo stage)", - ): - user = authenticate( - self.stage.request, - current_stage.password_stage.backends, - current_stage, - username=self.pre_user.username, - password=password, - ) - if not user: - raise ValidationError("Failed to authenticate.") - self.pre_user = user - except PermissionDenied as exc: - raise ValidationError(str(exc)) from exc - # Captcha check if captcha_stage := current_stage.captcha_stage: captcha_token = attrs.get("captcha_token", None) if not captcha_token: self.stage.logger.warning("Token not set for captcha attempt") verify_captcha_token(captcha_stage, captcha_token, client_ip) + + # Password check + if not current_stage.password_stage: + # No password stage select, don't validate the password + return attrs + + password = attrs.get("password", None) + if not password: + self.stage.logger.warning("Password not set for ident+auth attempt") + try: + with start_span( + op="authentik.stages.identification.authenticate", + name="User authenticate call (combo stage)", + ): + user = authenticate( + self.stage.request, + current_stage.password_stage.backends, + current_stage, + username=self.pre_user.username, + password=password, + ) + if not user: + raise ValidationError("Failed to authenticate.") + self.pre_user = user + except PermissionDenied as exc: + raise ValidationError(str(exc)) from exc return attrs diff --git a/web/src/flow/stages/base.ts b/web/src/flow/stages/base.ts index 35ae392ab3..921f5a2a93 100644 --- a/web/src/flow/stages/base.ts +++ b/web/src/flow/stages/base.ts @@ -72,7 +72,9 @@ export class BaseStage< } return this.host?.submit(object as unknown as Tout).then((successful) => { if (successful) { - this.cleanup(); + this.onSubmitSuccess(); + } else { + this.onSubmitFailure(); } return successful; }); @@ -124,7 +126,11 @@ export class BaseStage< `; } - cleanup(): void { + onSubmitSuccess(): void { + // Method that can be overridden by stages + return; + } + onSubmitFailure(): void { // Method that can be overridden by stages return; } diff --git a/web/src/flow/stages/captcha/CaptchaStage.ts b/web/src/flow/stages/captcha/CaptchaStage.ts index 14c7f6ee7f..ec29889a95 100644 --- a/web/src/flow/stages/captcha/CaptchaStage.ts +++ b/web/src/flow/stages/captcha/CaptchaStage.ts @@ -9,7 +9,7 @@ import { randomId } from "@goauthentik/elements/utils/randomId"; import "@goauthentik/flow/FormStatic"; import { BaseStage } from "@goauthentik/flow/stages/base"; import { P, match } from "ts-pattern"; -import type { TurnstileObject } from "turnstile-types"; +import type * as _ from "turnstile-types"; import { msg } from "@lit/localize"; import { CSSResult, PropertyValues, TemplateResult, css, html, nothing } from "lit"; @@ -24,10 +24,6 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css"; import { CaptchaChallenge, CaptchaChallengeResponseRequest } from "@goauthentik/api"; -interface TurnstileWindow extends Window { - turnstile: TurnstileObject; -} - type TokenHandler = (token: string) => void; type Dims = { height: number }; @@ -52,6 +48,8 @@ type CaptchaHandler = { name: string; interactive: () => Promise; execute: () => Promise; + refreshInteractive: () => Promise; + refresh: () => Promise; }; // A container iframe for a hosted Captcha, with an event emitter to monitor when the Captcha forces @@ -119,6 +117,12 @@ export class CaptchaStage extends BaseStage Object.hasOwn(window, name)); let lastError = undefined; let found = false; - for (const { name, interactive, execute } of handlers) { - console.debug(`authentik/stages/captcha: trying handler ${name}`); + for (const handler of handlers) { + console.debug(`authentik/stages/captcha: trying handler ${handler.name}`); try { - const runner = this.challenge.interactive ? interactive : execute; + const runner = this.challenge.interactive + ? handler.interactive + : handler.execute; await runner.apply(this); - console.debug(`authentik/stages/captcha[${name}]: handler succeeded`); + console.debug(`authentik/stages/captcha[${handler.name}]: handler succeeded`); found = true; + this.activeHandler = handler; break; } catch (exc) { - console.debug(`authentik/stages/captcha[${name}]: handler failed`); + console.debug(`authentik/stages/captcha[${handler.name}]: handler failed`); console.debug(exc); lastError = exc; } @@ -370,6 +406,19 @@ export class CaptchaStage extends BaseStage) { + if (!changedProperties.has("refreshedAt") || !this.challenge) { + return; + } + + console.debug("authentik/stages/captcha: refresh triggered"); + if (this.challenge.interactive) { + this.activeHandler?.refreshInteractive.apply(this); + } else { + this.activeHandler?.refresh.apply(this); + } + } } declare global { diff --git a/web/src/flow/stages/identification/IdentificationStage.ts b/web/src/flow/stages/identification/IdentificationStage.ts index 8fa21810fb..8f13a2bf90 100644 --- a/web/src/flow/stages/identification/IdentificationStage.ts +++ b/web/src/flow/stages/identification/IdentificationStage.ts @@ -49,6 +49,8 @@ export class IdentificationStage extends BaseStage< @state() captchaToken = ""; + @state() + captchaRefreshedAt = new Date(); static get styles(): CSSResult[] { return [ @@ -179,12 +181,16 @@ export class IdentificationStage extends BaseStage< this.form.appendChild(totp); } - cleanup(): void { + onSubmitSuccess(): void { if (this.form) { this.form.remove(); } } + onSubmitFailure(): void { + this.captchaRefreshedAt = new Date(); + } + renderSource(source: LoginSource): TemplateResult { const icon = renderSourceIcon(source.name, source.iconUrl); return html`