From a10ec34aec7a5de727e042dc5f53ad5918db44e3 Mon Sep 17 00:00:00 2001 From: Oluwatobi Mustapha Date: Thu, 19 Mar 2026 15:49:49 +0100 Subject: [PATCH] web/flow: reset stale authenticator selection between consecutive validate stages (#20802) * fix(web): reset stale MFA challenge selection across stages * Surface API errors in plucked details. * Clean up error messages, lifecycle, cancel states. * Address review feedback on base host property and tag resolver Fix lint and typing for authenticator component resolver Format authenticator resolver signature chore: trigger CI rerun after transient npm network failure * Tidy return value. --------- Co-authored-by: Teffen Ellis <592134+GirlBossRush@users.noreply.github.com> --- web/src/common/errors/network.ts | 44 ++++++- web/src/common/helpers/webauthn.ts | 15 ++- .../AuthenticatorValidateStage.ts | 115 +++++++++++------- .../AuthenticatorValidateStageWebAuthn.ts | 106 +++++++++------- .../stages/authenticator_validate/base.ts | 2 +- .../challenge-selection.ts | 15 +++ .../WebAuthnAuthenticatorRegisterStage.ts | 102 ++++++++++------ web/src/flow/stages/base.ts | 3 +- ...cator-validate-challenge-selection.test.ts | 43 +++++++ 9 files changed, 306 insertions(+), 139 deletions(-) create mode 100644 web/src/flow/stages/authenticator_validate/challenge-selection.ts create mode 100644 web/unit/authenticator-validate-challenge-selection.test.ts diff --git a/web/src/common/errors/network.ts b/web/src/common/errors/network.ts index e7364f1cfb..60715a174b 100644 --- a/web/src/common/errors/network.ts +++ b/web/src/common/errors/network.ts @@ -32,6 +32,32 @@ export const HTTPStatusCodeTransformer: Record //#region Type Predicates +/** + * A function that determines the specific type of error. + */ +export type ErrorPredicate = (error: unknown) => error is T; + +/** + * Recursively checks if an error or any of its causes satisfies a given predicate. + * + * This is useful for unwrapping errors that may be wrapped in multiple layers of `Error` objects with causes. + * + * @param error The error to check. + * @param predicate The type predicate to apply to the error and its causes. + * @returns The first error in the chain that satisfies the predicate, or `null` if none do. + */ +export function findCause(error: unknown, predicate: ErrorPredicate): T | null { + if (predicate(error)) { + return error; + } + + if (error instanceof Error && error.cause) { + return findCause(error.cause, predicate); + } + + return null; +} + /** * Type predicate to check if a response contains a JSON body. * @@ -205,15 +231,25 @@ export function pluckErrorDetail(errorLike: unknown, fallback?: string): string * Given API error, parses the response body and transforms it into a {@linkcode APIError}. */ export async function parseAPIResponseError( - error: unknown, + source: unknown, ): Promise { - if (!isResponseErrorLike(error)) { - const message = error instanceof Error ? error.message : String(error); + const apiError = findCause(source, isResponseErrorLike); + + if (!apiError) { + const message = source instanceof Error ? source.message : String(apiError); return createSyntheticGenericError(message) as T; } + const { response } = apiError; + let message: string | undefined; - const { response, message } = error; + if (apiError && apiError !== source) { + // The API error is wrapped in another error. + const wrapperMessage = pluckErrorDetail(source); + message = wrapperMessage ? `${wrapperMessage}: ${message}` : message; + } else { + message = apiError.message; + } if (!isJSONResponse(response)) { return createSyntheticGenericError(message || response.statusText) as T; diff --git a/web/src/common/helpers/webauthn.ts b/web/src/common/helpers/webauthn.ts index 52f5af49fe..b4921a01bb 100644 --- a/web/src/common/helpers/webauthn.ts +++ b/web/src/common/helpers/webauthn.ts @@ -16,16 +16,25 @@ export function u8arr(input: string): Uint8Array { ); } -export function checkWebAuthnSupport() { - if ("credentials" in navigator) { +export function assertWebAuthnSupported(scope = window): void { + if ("credentials" in scope.navigator) { return; } - if (window.location.protocol === "http:" && window.location.hostname !== "localhost") { + + if (scope.location.protocol === "http:" && scope.location.hostname !== "localhost") { throw new Error(msg("WebAuthn requires this page to be accessed via HTTPS.")); } + throw new Error(msg("WebAuthn not supported by browser.")); } +/** + * Predicate to determine if a given error originates from a user cancellation or timeout of a WebAuthn authentication ceremony. + */ +export function isWebAuthnNotAllowedError(error: unknown): error is DOMException { + return error instanceof DOMException && (error.name === "NotAllowedError" || error.code === 0); +} + /** * Check if the browser supports WebAuthn conditional UI (passkey autofill) */ diff --git a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts index 3d9187527a..9e822a5ffb 100644 --- a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts +++ b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts @@ -7,6 +7,10 @@ import Styles from "./AuthenticatorValidateStage.css"; import { DEFAULT_CONFIG } from "#common/api/config"; +import { SlottedTemplateResult } from "#elements/types"; +import { StrictUnsafe } from "#elements/utils/unsafe"; + +import { shouldResetSelectedChallenge } from "#flow/stages/authenticator_validate/challenge-selection"; import { BaseStage } from "#flow/stages/base"; import { PasswordManagerPrefill } from "#flow/stages/identification/IdentificationStage"; import type { StageHost, SubmitOptions } from "#flow/types"; @@ -77,6 +81,24 @@ const createDevicePickerPropMap = () => }, }) as const satisfies Record; +export function resolveAuthenticatorComponentTag( + deviceClass: DeviceClassesEnum | null | undefined, +) { + switch (deviceClass) { + case DeviceClassesEnum.Static: + case DeviceClassesEnum.Totp: + case DeviceClassesEnum.Email: + case DeviceClassesEnum.Sms: + return "ak-stage-authenticator-validate-code"; + case DeviceClassesEnum.Webauthn: + return "ak-stage-authenticator-validate-webauthn"; + case DeviceClassesEnum.Duo: + return "ak-stage-authenticator-validate-duo"; + default: + return null; + } +} + @customElement("ak-stage-authenticator-validate") export class AuthenticatorValidateStage extends BaseStage< @@ -85,9 +107,19 @@ export class AuthenticatorValidateStage > implements StageHost { - static styles: CSSResult[] = [PFLogin, PFForm, PFFormControl, PFTitle, PFButton, Styles]; + static styles: CSSResult[] = [ + // --- + PFLogin, + PFForm, + PFFormControl, + PFTitle, + PFButton, + Styles, + ]; - flowSlug = ""; + #api = new FlowsApi(DEFAULT_CONFIG); + + public flowSlug = ""; set loading(value: boolean) { this.host.loading = value; @@ -102,7 +134,7 @@ export class AuthenticatorValidateStage } @state() - _firstInitialized: boolean = false; + protected initialized = false; #selectedDeviceChallenge: DeviceChallenge | null = null; @@ -127,7 +159,7 @@ export class AuthenticatorValidateStage // We don't use this.submit here, as we don't want to advance the flow. // We just want to notify the backend which challenge has been selected. - new FlowsApi(DEFAULT_CONFIG).flowsExecutorSolve({ + this.#api.flowsExecutorSolve({ flowSlug: this.host?.flowSlug || "", query: window.location.search.substring(1), flowChallengeResponseRequest, @@ -149,12 +181,23 @@ export class AuthenticatorValidateStage this.selectedDeviceChallenge = null; } - willUpdate(_changed: PropertyValues) { - if (this._firstInitialized || !this.challenge) { + protected override willUpdate(changed: PropertyValues) { + // When moving between multiple authenticator-validate stages in one flow, the element + // instance is reused. Reset selection if it is no longer valid in the new challenge. + if (changed.has("challenge")) { + const allowedChallenges = this.challenge?.deviceChallenges ?? []; + + if (shouldResetSelectedChallenge(this.selectedDeviceChallenge, allowedChallenges)) { + this.selectedDeviceChallenge = null; + this.initialized = false; + } + } + + if (this.initialized || !this.challenge) { return; } - this._firstInitialized = true; + this.initialized = true; // If user only has a single device, autoselect that device. if (this.challenge.deviceChallenges.length === 1) { @@ -168,10 +211,9 @@ export class AuthenticatorValidateStage (challenge) => challenge.deviceClass === DeviceClassesEnum.Totp, ); if (PasswordManagerPrefill.totp && totpChallenge) { - console.debug( - "authentik/stages/authenticator_validate: found prefill totp code, selecting totp challenge", - ); + this.logger.debug("Found prefill TOTP code to select"); this.selectedDeviceChallenge = totpChallenge; + return; } @@ -185,10 +227,10 @@ export class AuthenticatorValidateStage } } - renderDevicePicker() { + protected renderDevicePicker(): SlottedTemplateResult { const { deviceChallenges } = this.challenge || {}; - if (this.selectedDeviceChallenge || !deviceChallenges?.length) { + if (!deviceChallenges?.length) { return nothing; } @@ -231,7 +273,7 @@ export class AuthenticatorValidateStage `; } - renderStagePicker() { + protected renderStagePicker(): SlottedTemplateResult { if (!this.challenge?.configurationStages.length) { return nothing; } @@ -264,40 +306,22 @@ export class AuthenticatorValidateStage `; } - renderDeviceChallenge() { + protected renderDeviceChallenge() { if (!this.selectedDeviceChallenge) { return nothing; } - switch (this.selectedDeviceChallenge?.deviceClass) { - case DeviceClassesEnum.Static: - case DeviceClassesEnum.Totp: - case DeviceClassesEnum.Email: - case DeviceClassesEnum.Sms: - return html` 1} - > - `; - case DeviceClassesEnum.Webauthn: - return html` 1} - > - `; - case DeviceClassesEnum.Duo: - return html` 1} - > - `; - } - return nothing; + + const tag = resolveAuthenticatorComponentTag(this.selectedDeviceChallenge.deviceClass); + if (!tag) return null; + + const showBackButton = (this.challenge?.deviceChallenges || []).length > 1; + + return StrictUnsafe(tag, { + host: this, + challenge: this.challenge, + deviceChallenge: this.selectedDeviceChallenge, + showBackButton, + }); } protected renderAuthenticatorSelection(): TemplateResult { @@ -305,7 +329,8 @@ export class AuthenticatorValidateStage ${this.renderUserInfo()}${this.renderStagePicker()}${this.renderDevicePicker()} `; } - render(): TemplateResult { + + protected override render(): TemplateResult { return html` ${this.selectedDeviceChallenge ? this.renderDeviceChallenge() diff --git a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageWebAuthn.ts b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageWebAuthn.ts index 6ced0f3697..96938e46f1 100644 --- a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageWebAuthn.ts +++ b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageWebAuthn.ts @@ -1,11 +1,15 @@ import "#elements/EmptyState"; +import { parseAPIResponseError, pluckErrorDetail } from "#common/errors/network"; import { - checkWebAuthnSupport, + assertWebAuthnSupported, + isWebAuthnNotAllowedError, transformAssertionForServer, transformCredentialRequestOptions, } from "#common/helpers/webauthn"; +import { SlottedTemplateResult } from "#elements/types"; + import { BaseDeviceStage } from "#flow/stages/authenticator_validate/base"; import { @@ -15,7 +19,7 @@ import { } from "@goauthentik/api"; import { msg } from "@lit/localize"; -import { html, nothing, PropertyValues, TemplateResult } from "lit"; +import { html, nothing, PropertyValues } from "lit"; import { customElement, property, state } from "lit/decorators.js"; @customElement("ak-stage-authenticator-validate-webauthn") @@ -24,57 +28,65 @@ export class AuthenticatorValidateStageWebAuthn extends BaseDeviceStage< AuthenticatorValidationChallengeResponseRequest > { @property({ attribute: false }) - deviceChallenge?: DeviceChallenge; + public deviceChallenge?: DeviceChallenge; - @property() - errorMessage?: string; + @property({ attribute: false }) + public errorMessage?: string; @property({ type: Boolean }) - showBackButton = false; + public showBackButton = false; @state() - authenticating = false; + protected authenticating = false; transformedCredentialRequestOptions?: PublicKeyCredentialRequestOptions; - async authenticate(): Promise { + protected async authenticate(): Promise { + assertWebAuthnSupported(); + // request the authenticator to create an assertion signature using the // credential private key - let assertion; - checkWebAuthnSupport(); - try { - assertion = await navigator.credentials.get({ + + const assertion = await navigator.credentials + .get({ publicKey: this.transformedCredentialRequestOptions, + }) + .then((assertion) => { + if (!assertion) { + throw new Error(msg("No assertion was returned by the authenticator")); + } + + return assertion as PublicKeyCredential; + }) + .catch((cause) => { + if (isWebAuthnNotAllowedError(cause)) { + throw new Error(msg("Authentication was cancelled or timed out"), { cause }); + } + + throw new Error("Error creating credential", { cause }); }); - if (!assertion) { - throw new Error("Assertions is empty"); - } - } catch (err) { - throw new Error(`Error when creating credential: ${err}`); - } - // we now have an authentication assertion! encode the byte arrays contained + // We now have an authentication assertion! encode the byte arrays contained // in the assertion data as strings for posting to the server - const transformedAssertionForServer = transformAssertionForServer( - assertion as PublicKeyCredential, - ); - // post the assertion to the server for verification. - try { - await this.host?.submit( + const transformedAssertionForServer = transformAssertionForServer(assertion); + + // Post the assertion to the server for verification. + return this.host + ?.submit( { webauthn: transformedAssertionForServer, }, { invisible: true, }, - ); - } catch (err) { - throw new Error(`Error when validating assertion on server: ${err}`); - } + ) + .catch((cause) => { + throw new Error(`Error when validating assertion on server`, { cause }); + }); } - updated(changedProperties: PropertyValues) { + public override updated(changedProperties: PropertyValues): void { super.updated(changedProperties); if (changedProperties.has("challenge") && this.challenge) { @@ -84,30 +96,34 @@ export class AuthenticatorValidateStageWebAuthn extends BaseDeviceStage< ?.challenge as PublicKeyCredentialRequestOptions; this.transformedCredentialRequestOptions = transformCredentialRequestOptions(credentialRequestOptions); - this.authenticateWrapper(); + + this.tryAuthenticating(); } } - async authenticateWrapper(): Promise { + protected tryAuthenticating = async (): Promise => { if (this.authenticating) { return; } + this.authenticating = true; - this.authenticate() - .catch((error: unknown) => { - console.warn( - "authentik/flows/authenticator_validate/webauthn: failed to auth", - error, - ); - this.errorMessage = msg("Authentication failed. Please try again."); + + return this.authenticate() + .catch(async (error: unknown) => { + const reason = msg("Failed to authenticate"); + this.logger.warn(reason, error); + + const parsedError = await parseAPIResponseError(error); + + this.errorMessage = pluckErrorDetail(parsedError, reason); }) .finally(() => { this.authenticating = false; }); - } + }; - render(): TemplateResult { - return html`
+ protected override render(): SlottedTemplateResult { + return html` ${this.renderUserInfo()} ${msg("Form actions")} ${!this.authenticating - ? html`