From 70075e6f0a47bf81f21888500527e2fdaf93fb94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simonyi=20Gerg=C5=91?= <28359278+gergosimonyi@users.noreply.github.com> Date: Thu, 24 Oct 2024 09:04:40 +0200 Subject: [PATCH] stages/authenticator_validate: autoselect last used 2fa device (#11087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * authenticator_validate: autoselect last used device class * improve usability of `AuthenticatorValidationStage` * don't automatically offer the recovery key authenticator validation I believe this could confuse users more than help them * web: move mutator block into the `willUpdate` override Removed the section of code from the renderer that updates the state of the component; Mutating in the middle of a render is strongly discouraged. This block contains an algorithm for determining if the selectedDeviceChallenge should be set and how; since `selectedDeviceChallenge` is a state, we don't want to be changing it outside of those lifecycle methods that do not trigger a rerender. * web: move styles() to top of class, extract custom CSS to a named block. * lint: collapse multiple early returns, missing curly brace. * autoselect device only once even if the user only has 1 device * make `DeviceChallenge.last_used` nullable instead of optional * clarify button text * fix typo * add docs for automatic device selection * update docs Co-authored-by: Tana M Berry Signed-off-by: Simonyi Gergő <28359278+gergosimonyi@users.noreply.github.com> * fix punctuation --------- Signed-off-by: Simonyi Gergő <28359278+gergosimonyi@users.noreply.github.com> Co-authored-by: Ken Sternberg Co-authored-by: Tana M Berry --- .../authenticator_validate/challenge.py | 3 +- .../stages/authenticator_validate/stage.py | 2 + .../authenticator_validate/tests/test_sms.py | 1 + .../tests/test_stage.py | 2 + .../tests/test_webauthn.py | 3 + schema.yml | 10 ++ .../AuthenticatorValidateStage.ts | 170 ++++++++++-------- .../AuthenticatorValidateStageCode.ts | 43 +++-- .../stages/authenticator_validate/base.ts | 2 +- .../stages/authenticator_validate/index.md | 10 +- 10 files changed, 155 insertions(+), 91 deletions(-) diff --git a/authentik/stages/authenticator_validate/challenge.py b/authentik/stages/authenticator_validate/challenge.py index c11439684fef..1f9a656a383d 100644 --- a/authentik/stages/authenticator_validate/challenge.py +++ b/authentik/stages/authenticator_validate/challenge.py @@ -8,7 +8,7 @@ from django.shortcuts import get_object_or_404 from django.utils.translation import gettext as __ from django.utils.translation import gettext_lazy as _ -from rest_framework.fields import CharField +from rest_framework.fields import CharField, DateTimeField from rest_framework.serializers import ValidationError from structlog.stdlib import get_logger from webauthn import options_to_json @@ -45,6 +45,7 @@ class DeviceChallenge(PassiveSerializer): device_class = CharField() device_uid = CharField() challenge = JSONDictField() + last_used = DateTimeField(allow_null=True) def get_challenge_for_device( diff --git a/authentik/stages/authenticator_validate/stage.py b/authentik/stages/authenticator_validate/stage.py index 96ae7e621582..bde76d37db40 100644 --- a/authentik/stages/authenticator_validate/stage.py +++ b/authentik/stages/authenticator_validate/stage.py @@ -217,6 +217,7 @@ def get_device_challenges(self) -> list[dict]: "device_class": device_class, "device_uid": device.pk, "challenge": get_challenge_for_device(self.request, stage, device), + "last_used": device.last_used, } ) challenge.is_valid() @@ -237,6 +238,7 @@ def get_webauthn_challenge_without_user(self) -> list[dict]: self.request, self.executor.current_stage, ), + "last_used": None, } ) challenge.is_valid() diff --git a/authentik/stages/authenticator_validate/tests/test_sms.py b/authentik/stages/authenticator_validate/tests/test_sms.py index 5cce79620711..850854a892bb 100644 --- a/authentik/stages/authenticator_validate/tests/test_sms.py +++ b/authentik/stages/authenticator_validate/tests/test_sms.py @@ -107,6 +107,7 @@ def test_last_auth_threshold_valid(self): "device_class": "sms", "device_uid": str(device.pk), "challenge": {}, + "last_used": None, }, }, ) diff --git a/authentik/stages/authenticator_validate/tests/test_stage.py b/authentik/stages/authenticator_validate/tests/test_stage.py index 98fe5d2fe429..82a2f5132251 100644 --- a/authentik/stages/authenticator_validate/tests/test_stage.py +++ b/authentik/stages/authenticator_validate/tests/test_stage.py @@ -169,6 +169,7 @@ def test_validate_selected_challenge(self): "device_class": "baz", "device_uid": "quox", "challenge": {}, + "last_used": None, } }, ) @@ -188,6 +189,7 @@ def test_validate_selected_challenge(self): "device_class": "static", "device_uid": "1", "challenge": {}, + "last_used": None, }, }, ) diff --git a/authentik/stages/authenticator_validate/tests/test_webauthn.py b/authentik/stages/authenticator_validate/tests/test_webauthn.py index e4247b221ab1..05fe216081df 100644 --- a/authentik/stages/authenticator_validate/tests/test_webauthn.py +++ b/authentik/stages/authenticator_validate/tests/test_webauthn.py @@ -274,6 +274,7 @@ def test_validate_challenge_unrestricted(self): "device_class": device.__class__.__name__.lower().replace("device", ""), "device_uid": device.pk, "challenge": {}, + "last_used": None, } ] session[SESSION_KEY_PLAN] = plan @@ -352,6 +353,7 @@ def test_validate_challenge_restricted(self): "device_class": device.__class__.__name__.lower().replace("device", ""), "device_uid": device.pk, "challenge": {}, + "last_used": None, } ] session[SESSION_KEY_PLAN] = plan @@ -432,6 +434,7 @@ def test_validate_challenge_userless(self): "device_class": device.__class__.__name__.lower().replace("device", ""), "device_uid": device.pk, "challenge": {}, + "last_used": None, } ] session[SESSION_KEY_PLAN] = plan diff --git a/schema.yml b/schema.yml index d4f3eb78ac7b..f0c8447abbc4 100644 --- a/schema.yml +++ b/schema.yml @@ -40204,10 +40204,15 @@ components: challenge: type: object additionalProperties: {} + last_used: + type: string + format: date-time + nullable: true required: - challenge - device_class - device_uid + - last_used DeviceChallengeRequest: type: object description: Single device challenge @@ -40221,10 +40226,15 @@ components: challenge: type: object additionalProperties: {} + last_used: + type: string + format: date-time + nullable: true required: - challenge - device_class - device_uid + - last_used DeviceClassesEnum: enum: - static diff --git a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts index a80ec9402278..3bfad7def077 100644 --- a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts +++ b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStage.ts @@ -6,7 +6,7 @@ import { BaseStage, StageHost, SubmitOptions } from "@goauthentik/flow/stages/ba import { PasswordManagerPrefill } from "@goauthentik/flow/stages/identification/IdentificationStage"; import { msg } from "@lit/localize"; -import { CSSResult, TemplateResult, css, html, nothing } from "lit"; +import { CSSResult, PropertyValues, TemplateResult, css, html, nothing } from "lit"; import { customElement, state } from "lit/decorators.js"; import PFButton from "@patternfly/patternfly/components/Button/button.css"; @@ -25,6 +25,37 @@ import { FlowsApi, } from "@goauthentik/api"; +const customCSS = css` + ul { + padding-top: 1rem; + } + ul > li:not(:last-child) { + padding-bottom: 1rem; + } + .authenticator-button { + display: flex; + align-items: center; + } + :host([theme="dark"]) .authenticator-button { + color: var(--ak-dark-foreground) !important; + } + i { + font-size: 1.5rem; + padding: 1rem 0; + width: 3rem; + } + .right { + display: flex; + flex-direction: column; + justify-content: space-between; + height: 100%; + text-align: left; + } + .right > * { + height: 50%; + } +`; + @customElement("ak-stage-authenticator-validate") export class AuthenticatorValidateStage extends BaseStage< @@ -33,6 +64,10 @@ export class AuthenticatorValidateStage > implements StageHost { + static get styles(): CSSResult[] { + return [PFBase, PFLogin, PFForm, PFFormControl, PFTitle, PFButton, customCSS]; + } + flowSlug = ""; set loading(value: boolean) { @@ -47,14 +82,18 @@ export class AuthenticatorValidateStage return this.host.brand; } + @state() + _firstInitialized: boolean = false; + @state() _selectedDeviceChallenge?: DeviceChallenge; set selectedDeviceChallenge(value: DeviceChallenge | undefined) { const previousChallenge = this._selectedDeviceChallenge; this._selectedDeviceChallenge = value; - if (!value) return; - if (value === previousChallenge) return; + if (value === undefined || value === previousChallenge) { + return; + } // 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({ @@ -79,37 +118,39 @@ export class AuthenticatorValidateStage return this.host?.submit(payload, options) || Promise.resolve(); } - static get styles(): CSSResult[] { - return [PFBase, PFLogin, PFForm, PFFormControl, PFTitle, PFButton].concat(css` - ul { - padding-top: 1rem; - } - ul > li:not(:last-child) { - padding-bottom: 1rem; - } - .authenticator-button { - display: flex; - align-items: center; - } - :host([theme="dark"]) .authenticator-button { - color: var(--ak-dark-foreground) !important; - } - i { - font-size: 1.5rem; - padding: 1rem 0; - width: 3rem; - } - .right { - display: flex; - flex-direction: column; - justify-content: space-between; - height: 100%; - text-align: left; - } - .right > * { - height: 50%; - } - `); + willUpdate(_changed: PropertyValues) { + if (this._firstInitialized || !this.challenge) { + return; + } + + this._firstInitialized = true; + + // If user only has a single device, autoselect that device. + if (this.challenge.deviceChallenges.length === 1) { + this.selectedDeviceChallenge = this.challenge.deviceChallenges[0]; + return; + } + + // If TOTP is allowed from the backend and we have a pre-filled value + // from the password manager, autoselect TOTP. + const totpChallenge = this.challenge.deviceChallenges.find( + (challenge) => challenge.deviceClass === DeviceClassesEnum.Totp, + ); + if (PasswordManagerPrefill.totp && totpChallenge) { + console.debug( + "authentik/stages/authenticator_validate: found prefill totp code, selecting totp challenge", + ); + this.selectedDeviceChallenge = totpChallenge; + return; + } + + // If the last used device is not Static, autoselect that device. + const lastUsedChallenge = this.challenge.deviceChallenges + .filter((deviceChallenge) => deviceChallenge.lastUsed) + .sort((a, b) => b.lastUsed!.valueOf() - a.lastUsed!.valueOf())[0]; + if (lastUsedChallenge && lastUsedChallenge.deviceClass !== DeviceClassesEnum.Static) { + this.selectedDeviceChallenge = lastUsedChallenge; + } } renderDevicePickerSingle(deviceChallenge: DeviceChallenge) { @@ -228,45 +269,28 @@ export class AuthenticatorValidateStage } render(): TemplateResult { - if (!this.challenge) { - return html` `; - } - // User only has a single device class, so we don't show a picker - if (this.challenge?.deviceChallenges.length === 1) { - this.selectedDeviceChallenge = this.challenge.deviceChallenges[0]; - } - // TOTP is a bit special, assuming that TOTP is allowed from the backend, - // and we have a pre-filled value from the password manager, - // directly set the the TOTP device Challenge as active. - const totpChallenge = this.challenge.deviceChallenges.find( - (challenge) => challenge.deviceClass === DeviceClassesEnum.Totp, - ); - if (PasswordManagerPrefill.totp && totpChallenge) { - console.debug( - "authentik/stages/authenticator_validate: found prefill totp code, selecting totp challenge", - ); - this.selectedDeviceChallenge = totpChallenge; - } - return html` - ${this.selectedDeviceChallenge - ? this.renderDeviceChallenge() - : html` -
- -
`}`; + return this.challenge + ? html` + ${this.selectedDeviceChallenge + ? this.renderDeviceChallenge() + : html` +
+ +
`}` + : html` `; } } diff --git a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageCode.ts b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageCode.ts index 9065f2e4d864..01fcd4ef6819 100644 --- a/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageCode.ts +++ b/web/src/flow/stages/authenticator_validate/AuthenticatorValidateStageCode.ts @@ -31,6 +31,34 @@ export class AuthenticatorValidateStageWebCode extends BaseDeviceStage< `); } + deviceMessage(): string { + switch (this.deviceChallenge?.deviceClass) { + case DeviceClassesEnum.Sms: + return msg("A code has been sent to you via SMS."); + case DeviceClassesEnum.Totp: + return msg( + "Open your two-factor authenticator app to view your authentication code.", + ); + case DeviceClassesEnum.Static: + return msg("Enter a one-time recovery code for this user."); + } + + return msg("Enter the code from your authenticator device."); + } + + deviceIcon(): string { + switch (this.deviceChallenge?.deviceClass) { + case DeviceClassesEnum.Sms: + return "fa-key"; + case DeviceClassesEnum.Totp: + return "fa-mobile-alt"; + case DeviceClassesEnum.Static: + return "fa-sticky-note"; + } + + return "fa-mobile-alt"; + } + render(): TemplateResult { if (!this.challenge) { return html` `; @@ -44,19 +72,8 @@ export class AuthenticatorValidateStageWebCode extends BaseDeviceStage< > ${this.renderUserInfo()}
- - ${this.deviceChallenge?.deviceClass == DeviceClassesEnum.Sms - ? html`

${msg("A code has been sent to you via SMS.")}

` - : html`

- ${msg( - "Open your two-factor authenticator app to view your authentication code.", - )} -

`} + +

${this.deviceMessage()}

`; } } diff --git a/website/docs/add-secure-apps/flows-stages/stages/authenticator_validate/index.md b/website/docs/add-secure-apps/flows-stages/stages/authenticator_validate/index.md index 907a967e1907..bbe7a1e7bffb 100644 --- a/website/docs/add-secure-apps/flows-stages/stages/authenticator_validate/index.md +++ b/website/docs/add-secure-apps/flows-stages/stages/authenticator_validate/index.md @@ -5,10 +5,10 @@ title: Authenticator validation stage This stage validates an already configured Authenticator Device. This device has to be configured using any of the other authenticator stages: - [Duo authenticator stage](../authenticator_duo/index.md) -- [SMS authenticator stage](../authenticator_sms/index.md). -- [Static authenticator stage](../authenticator_static/index.md). +- [SMS authenticator stage](../authenticator_sms/index.md) +- [Static authenticator stage](../authenticator_static/index.md) - [TOTP authenticator stage](../authenticator_totp/index.md) -- [WebAuth authenticator stage](../authenticator_webauthn/index.md). +- [WebAuthn authenticator stage](../authenticator_webauthn/index.md) You can select which type of device classes are allowed. @@ -75,3 +75,7 @@ Optionally restrict which WebAuthn device types can be used to authenticate. When no restriction is set, all WebAuthn devices a user has registered are allowed. These restrictions only apply to WebAuthn devices created with authentik 2024.4 or later. + +#### Automatic device selection + +If the user has more than one device, the user is prompted to select which device they want to use for validation. After the user successfully authenticates with a certain device, that device is marked as "last used". In subsequent prompts by the Authenticator validation stage, the last used device is automatically selected for the user. Should they wish to use another device, the user can return to the device selection screen.