stages/authenticator_validate: autoselect last used 2fa device (#11087)

* 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 <[email protected]> Signed-off-by: Simonyi Gergő <[email protected]> * fix punctuation --------- Signed-off-by: Simonyi Gergő <[email protected]> Co-authored-by: Ken Sternberg <[email protected]> Co-authored-by: Tana M Berry <[email protected]>
goauthentik · Oct 24, 2024 · 70075e6 · 70075e6
1 parent dc670da
commit 70075e6
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 91 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<this>) {
+        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`<ak-empty-state loading> </ak-empty-state>`;
-        }
-        // 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`<header class="pf-c-login__main-header">
-                <h1 class="pf-c-title pf-m-3xl">${this.challenge.flowInfo?.title}</h1>
-            </header>
-            ${this.selectedDeviceChallenge
-                ? this.renderDeviceChallenge()
-                : html`<div class="pf-c-login__main-body">
-                          <form class="pf-c-form">
-                              ${this.renderUserInfo()}
-                              ${this.selectedDeviceChallenge
-                                  ? ""
-                                  : html`<p>${msg("Select an authentication method.")}</p>`}
-                              ${this.challenge.configurationStages.length > 0
-                                  ? this.renderStagePicker()
-                                  : html``}
-                          </form>
-                          ${this.renderDevicePicker()}
-                      </div>
-                      <footer class="pf-c-login__main-footer">
-                          <ul class="pf-c-login__main-footer-links"></ul>
-                      </footer>`}`;
+        return this.challenge
+            ? html`<header class="pf-c-login__main-header">
+                      <h1 class="pf-c-title pf-m-3xl">${this.challenge.flowInfo?.title}</h1>
+                  </header>
+                  ${this.selectedDeviceChallenge
+                      ? this.renderDeviceChallenge()
+                      : html`<div class="pf-c-login__main-body">
+                                <form class="pf-c-form">
+                                    ${this.renderUserInfo()}
+                                    ${this.selectedDeviceChallenge
+                                        ? ""
+                                        : html`<p>${msg("Select an authentication method.")}</p>`}
+                                    ${this.challenge.configurationStages.length > 0
+                                        ? this.renderStagePicker()
+                                        : html``}
+                                </form>
+                                ${this.renderDevicePicker()}
+                            </div>
+                            <footer class="pf-c-login__main-footer">
+                                <ul class="pf-c-login__main-footer-links"></ul>
+                            </footer>`}`
+            : html`<ak-empty-state loading> </ak-empty-state>`;
     }
 }
 

diff --git 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`<ak-empty-state loading> </ak-empty-state>`;
@@ -44,19 +72,8 @@ export class AuthenticatorValidateStageWebCode extends BaseDeviceStage<
             >
                 ${this.renderUserInfo()}
                 <div class="icon-description">
-                    <i
-                        class="fa ${this.deviceChallenge?.deviceClass == DeviceClassesEnum.Sms
-                            ? "fa-key"
-                            : "fa-mobile-alt"}"
-                        aria-hidden="true"
-                    ></i>
-                    ${this.deviceChallenge?.deviceClass == DeviceClassesEnum.Sms
-                        ? html`<p>${msg("A code has been sent to you via SMS.")}</p>`
-                        : html`<p>
-                              ${msg(
-                                  "Open your two-factor authenticator app to view your authentication code.",
-                              )}
-                          </p>`}
+                    <i class="fa ${this.deviceIcon()}" aria-hidden="true"></i>
+                    <p>${this.deviceMessage()}</p>
                 </div>
                 <ak-form-element
                     label="${this.deviceChallenge?.deviceClass === DeviceClassesEnum.Static

diff --git a/web/src/flow/stages/authenticator_validate/base.ts b/web/src/flow/stages/authenticator_validate/base.ts
@@ -59,7 +59,7 @@ export class BaseDeviceStage<
                 (this.host as AuthenticatorValidateStage).selectedDeviceChallenge = undefined;
             }}
         >
-            ${msg("Return to device picker")}
+            ${msg("Select another authentication method")}
         </button>`;
     }
 }