From 2707811de8145ce00ea7d064efb49f4507a1faf8 Mon Sep 17 00:00:00 2001 From: Dave <3836813+enmande@users.noreply.github.com> Date: Mon, 29 Dec 2025 12:19:37 -0500 Subject: [PATCH] feat(2fa-webauthn) [PM-20109]: Increase 2FA WebAuthn Security Key Limit (#18040) * feat(2fa-webauthn) [PM-20109]: Update WebAuthN credential handling. * feat(messages) [PM-20109]: Add 'Unnamed key' translation. * refactor(2fa-webauthn) [PM-20109]: Refactor nextId for type safety. * refactor(2fa-webauthn) [PM-20109]: Clean up template comments. * fix(webauthn-2fa) [PM-3611]: Key name is required. --- .../two-factor-setup-webauthn.component.html | 43 ++++++------ .../two-factor-setup-webauthn.component.ts | 68 +++++++++++++------ apps/web/src/locales/en/messages.json | 3 + 3 files changed, 72 insertions(+), 42 deletions(-) diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html index c272a8e5b70..8a538cb961c 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html @@ -16,27 +16,26 @@ FIDO2 WebAuthn logo @@ -60,7 +59,9 @@ type="button" [bitAction]="readKey" buttonType="secondary" - [disabled]="$any(readKeyBtn).loading() || webAuthnListening || !keyIdAvailable" + [disabled]=" + $any(readKeyBtn).loading() || webAuthnListening || !keyIdAvailable || formGroup.invalid + " class="tw-mr-2" #readKeyBtn > diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.ts b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.ts index 11ba5955902..57001acc4d2 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.ts +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.ts @@ -1,6 +1,6 @@ import { CommonModule } from "@angular/common"; import { Component, Inject, NgZone } from "@angular/core"; -import { FormControl, FormGroup, ReactiveFormsModule } from "@angular/forms"; +import { FormControl, FormGroup, ReactiveFormsModule, Validators } from "@angular/forms"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; @@ -99,7 +99,7 @@ export class TwoFactorSetupWebAuthnComponent extends TwoFactorSetupMethodBaseCom toastService, ); this.formGroup = new FormGroup({ - name: new FormControl({ value: "", disabled: false }), + name: new FormControl({ value: "", disabled: false }, Validators.required), }); this.auth(data); } @@ -213,7 +213,22 @@ export class TwoFactorSetupWebAuthnComponent extends TwoFactorSetupMethodBaseCom this.webAuthnListening = listening; } + private findNextAvailableKeyId(existingIds: Set): number { + // Search for first gap, bounded by current key count + 1 + for (let i = 1; i <= existingIds.size + 1; i++) { + if (!existingIds.has(i)) { + return i; + } + } + + // This should never be reached due to loop bounds, but TypeScript requires a return + throw new Error("Unable to find next available key ID"); + } + private processResponse(response: TwoFactorWebAuthnResponse) { + if (!response.keys || response.keys.length === 0) { + response.keys = []; + } this.resetWebAuthn(); this.keys = []; this.keyIdAvailable = null; @@ -223,26 +238,37 @@ export class TwoFactorSetupWebAuthnComponent extends TwoFactorSetupMethodBaseCom nameControl.setValue(""); } this.keysConfiguredCount = 0; - for (let i = 1; i <= 5; i++) { - if (response.keys != null) { - const key = response.keys.filter((k) => k.id === i); - if (key.length > 0) { - this.keysConfiguredCount++; - this.keys.push({ - id: i, - name: key[0].name, - configured: true, - migrated: key[0].migrated, - removePromise: null, - }); - continue; - } - } - this.keys.push({ id: i, name: "", configured: false, removePromise: null }); - if (this.keyIdAvailable == null) { - this.keyIdAvailable = i; - } + + // Build configured keys + for (const key of response.keys) { + this.keysConfiguredCount++; + this.keys.push({ + id: key.id, + name: key.name, + configured: true, + migrated: key.migrated, + removePromise: null, + }); } + + // [PM-20109]: To accommodate the existing form logic with minimal changes, + // we need to have at least one unconfigured key slot available to the collection. + // Prior to PM-20109, both client and server had hard checks for IDs <= 5. + // While we don't have any technical constraints _at this time_, we should avoid + // unbounded growth of key IDs over time as users add/remove keys; + // this strategy gap-fills key IDs. + const existingIds = new Set(response.keys.map((k) => k.id)); + const nextId = this.findNextAvailableKeyId(existingIds); + + // Add unconfigured slot, which can be used to add a new key + this.keys.push({ + id: nextId, + name: "", + configured: false, + removePromise: null, + }); + this.keyIdAvailable = nextId; + this.enabled = response.enabled; this.onUpdated.emit(this.enabled); } diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index ac40f78e43f..4721c971dcc 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -2634,6 +2634,9 @@ "key": { "message": "Key" }, + "unnamedKey": { + "message": "Unnamed key" + }, "twoStepAuthenticatorEnterCodeV2": { "message": "Verification code" },