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.

apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html

@@ -16,27 +16,26 @@
       <img class="tw-float-right tw-ml-5 mfaType7" alt="FIDO2 WebAuthn logo" />
       <ul class="bwi-ul">
         <li *ngFor="let k of keys; let i = index" #removeKeyBtn [appApiAction]="k.removePromise">
-          <i class="bwi bwi-li bwi-key"></i>
-          <span *ngIf="!k.configured || !k.name" bitTypography="body1" class="tw-font-medium">
-            {{ "webAuthnkeyX" | i18n: (i + 1).toString() }}
-          </span>
-          <span *ngIf="k.configured && k.name" bitTypography="body1" class="tw-font-medium">
-            {{ k.name }}
-          </span>
-          <ng-container *ngIf="k.configured && !$any(removeKeyBtn).loading">
-            <ng-container *ngIf="k.migrated">
-              <span>{{ "webAuthnMigrated" | i18n }}</span>
+          <ng-container *ngIf="k.configured">
+            <i class="bwi bwi-li bwi-key"></i>
+            <span *ngIf="k.configured" bitTypography="body1" class="tw-font-medium">
+              {{ k.name || ("unnamedKey" | i18n) }}
+            </span>
+            <ng-container *ngIf="k.configured && !$any(removeKeyBtn).loading">
+              <ng-container *ngIf="k.migrated">
+                <span>{{ "webAuthnMigrated" | i18n }}</span>
+              </ng-container>
+            </ng-container>
+            <ng-container *ngIf="keysConfiguredCount > 1 && k.configured">
+              <i
+                class="bwi bwi-spin bwi-spinner tw-text-muted bwi-fw"
+                title="{{ 'loading' | i18n }}"
+                *ngIf="$any(removeKeyBtn).loading"
+                aria-hidden="true"
+              ></i>
+              -
+              <a bitLink href="#" appStopClick (click)="remove(k)">{{ "remove" | i18n }}</a>
             </ng-container>
-          </ng-container>
-          <ng-container *ngIf="keysConfiguredCount > 1 && k.configured">
-            <i
-              class="bwi bwi-spin bwi-spinner tw-text-muted bwi-fw"
-              title="{{ 'loading' | i18n }}"
-              *ngIf="$any(removeKeyBtn).loading"
-              aria-hidden="true"
-            ></i>
-            -
-            <a bitLink href="#" appStopClick (click)="remove(k)">{{ "remove" | i18n }}</a>
           </ng-container>
         </li>
       </ul>
@@ -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
       >

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>): 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);
   }

apps/web/src/locales/en/messages.json

@@ -2634,6 +2634,9 @@
   "key": {
     "message": "Key"
   },
+  "unnamedKey": {
+    "message": "Unnamed key"
+  },
   "twoStepAuthenticatorEnterCodeV2": {
     "message": "Verification code"
   },