bitwarden · trmartin4 · Sep 7, 2023 · Aug 15, 2023 · Aug 17, 2023 · Aug 18, 2023
diff --git a/libs/common/src/vault/api/fido2-key.api.ts b/libs/common/src/vault/api/fido2-key.api.ts
@@ -1,7 +1,7 @@
 import { BaseResponse } from "../../models/response/base.response";
 
 export class Fido2KeyApi extends BaseResponse {
-  nonDiscoverableId: string;
+  credentialId: string;
   keyType: "public-key";
   keyAlgorithm: "ECDSA";
   keyCurve: "P-256";
@@ -20,7 +20,7 @@ export class Fido2KeyApi extends BaseResponse {
       return;
     }
 
-    this.nonDiscoverableId = this.getResponseProperty("NonDiscoverableId");
+    this.credentialId = this.getResponseProperty("CredentialId");
     this.keyType = this.getResponseProperty("KeyType");
     this.keyAlgorithm = this.getResponseProperty("KeyAlgorithm");
     this.keyCurve = this.getResponseProperty("KeyCurve");

diff --git a/libs/common/src/vault/models/data/fido2-key.data.ts b/libs/common/src/vault/models/data/fido2-key.data.ts
@@ -1,7 +1,7 @@
 import { Fido2KeyApi } from "../../api/fido2-key.api";
 
 export class Fido2KeyData {
-  nonDiscoverableId: string;
+  credentialId: string;
   keyType: "public-key";
   keyAlgorithm: "ECDSA";
   keyCurve: "P-256";
@@ -19,7 +19,7 @@ export class Fido2KeyData {
       return;
     }
 
-    this.nonDiscoverableId = data.nonDiscoverableId;
+    this.credentialId = data.credentialId;
     this.keyType = data.keyType;
     this.keyAlgorithm = data.keyAlgorithm;
     this.keyCurve = data.keyCurve;

diff --git a/libs/common/src/vault/models/domain/fido2-key.ts b/libs/common/src/vault/models/domain/fido2-key.ts
@@ -7,7 +7,7 @@ import { Fido2KeyData } from "../data/fido2-key.data";
 import { Fido2KeyView } from "../view/fido2-key.view";
 
 export class Fido2Key extends Domain {
-  nonDiscoverableId: EncString | null = null;
+  credentialId: EncString | null = null;
   keyType: EncString;
   keyAlgorithm: EncString;
   keyCurve: EncString;
@@ -31,7 +31,7 @@ export class Fido2Key extends Domain {
       this,
       obj,
       {
-        nonDiscoverableId: null,
+        credentialId: null,
         keyType: null,
         keyAlgorithm: null,
         keyCurve: null,
@@ -51,7 +51,7 @@ export class Fido2Key extends Domain {
     const view = await this.decryptObj(
       new Fido2KeyView(),
       {
-        nonDiscoverableId: null,
+        credentialId: null,
         keyType: null,
         keyAlgorithm: null,
         keyCurve: null,
@@ -83,7 +83,7 @@ export class Fido2Key extends Domain {
   toFido2KeyData(): Fido2KeyData {
     const i = new Fido2KeyData();
     this.buildDataModel(this, i, {
-      nonDiscoverableId: null,
+      credentialId: null,
       keyType: null,
       keyAlgorithm: null,
       keyCurve: null,
@@ -103,7 +103,7 @@ export class Fido2Key extends Domain {
       return null;
     }
 
-    const nonDiscoverableId = EncString.fromJSON(obj.nonDiscoverableId);
+    const credentialId = EncString.fromJSON(obj.credentialId);
     const keyType = EncString.fromJSON(obj.keyType);
     const keyAlgorithm = EncString.fromJSON(obj.keyAlgorithm);
     const keyCurve = EncString.fromJSON(obj.keyCurve);
@@ -116,7 +116,7 @@ export class Fido2Key extends Domain {
     const origin = EncString.fromJSON(obj.origin);
 
     return Object.assign(new Fido2Key(), obj, {
-      nonDiscoverableId,
+      credentialId,
       keyType,
       keyAlgorithm,
       keyCurve,

diff --git a/libs/common/src/vault/models/domain/login.spec.ts b/libs/common/src/vault/models/domain/login.spec.ts
@@ -113,7 +113,7 @@ describe("Login DTO", () => {
         passwordRevisionDate: passwordRevisionDate.toISOString(),
         totp: "myTotp" as EncryptedString,
         fido2Key: {
-          nonDiscoverableId: "keyId" as EncryptedString,
+          credentialId: "keyId" as EncryptedString,
           keyType: "keyType" as EncryptedString,
           keyAlgorithm: "keyAlgorithm" as EncryptedString,
           keyCurve: "keyCurve" as EncryptedString,
@@ -134,7 +134,7 @@ describe("Login DTO", () => {
         passwordRevisionDate: passwordRevisionDate,
         totp: "myTotp_fromJSON",
         fido2Key: {
-          nonDiscoverableId: "keyId_fromJSON",
+          credentialId: "keyId_fromJSON",
           keyType: "keyType_fromJSON",
           keyAlgorithm: "keyAlgorithm_fromJSON",
           keyCurve: "keyCurve_fromJSON",

diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts
@@ -66,9 +66,9 @@ export class CipherRequest {
 
         if (cipher.login.fido2Key != null) {
           this.login.fido2Key = new Fido2KeyApi();
-          this.login.fido2Key.nonDiscoverableId =
-            cipher.login.fido2Key.nonDiscoverableId != null
-              ? cipher.login.fido2Key.nonDiscoverableId.encryptedString
+          this.login.fido2Key.credentialId =
+            cipher.login.fido2Key.credentialId != null
+              ? cipher.login.fido2Key.credentialId.encryptedString
               : null;
           this.login.fido2Key.keyType =
             cipher.login.fido2Key.keyType != null
@@ -167,9 +167,9 @@ export class CipherRequest {
         break;
       case CipherType.Fido2Key:
         this.fido2Key = new Fido2KeyApi();
-        this.fido2Key.nonDiscoverableId =
-          cipher.fido2Key.nonDiscoverableId != null
-            ? cipher.fido2Key.nonDiscoverableId.encryptedString
+        this.fido2Key.credentialId =
+          cipher.fido2Key.credentialId != null
+            ? cipher.fido2Key.credentialId.encryptedString
             : null;
         this.fido2Key.keyType =
           cipher.fido2Key.keyType != null

diff --git a/libs/common/src/vault/models/view/fido2-key.view.ts b/libs/common/src/vault/models/view/fido2-key.view.ts
@@ -3,7 +3,7 @@ import { Jsonify } from "type-fest";
 import { ItemView } from "./item.view";
 
 export class Fido2KeyView extends ItemView {
-  nonDiscoverableId: string;
+  credentialId: string;
   keyType: "public-key";
   keyAlgorithm: "ECDSA";
   keyCurve: "P-256";

diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts
@@ -1095,7 +1095,7 @@ export class CipherService implements CipherServiceAbstraction {
             model.login.fido2Key,
             cipher.login.fido2Key,
             {
-              nonDiscoverableId: null,
+              credentialId: null,
               keyType: null,
               keyAlgorithm: null,
               keyCurve: null,
@@ -1168,6 +1168,7 @@ export class CipherService implements CipherServiceAbstraction {
           model.fido2Key,
           cipher.fido2Key,
           {
+            credentialId: null,
             keyType: null,
             keyAlgorithm: null,
             keyCurve: null,

diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts
@@ -107,12 +107,12 @@ describe("FidoAuthenticatorService", () => {
       beforeEach(async () => {
         excludedCipher = createCipherView(
           { type: CipherType.Login },
-          { nonDiscoverableId: Utils.newGuid() }
+          { credentialId: Utils.newGuid() }
         );
         params = await createParams({
           excludeCredentialDescriptorList: [
             {
-              id: Utils.guidToRawFormat(excludedCipher.login.fido2Key.nonDiscoverableId),
+              id: Utils.guidToRawFormat(excludedCipher.login.fido2Key.credentialId),
               type: "public-key",
             },
           ],
@@ -186,7 +186,10 @@ describe("FidoAuthenticatorService", () => {
         excludedCipherView = createCipherView();
         params = await createParams({
           excludeCredentialDescriptorList: [
-            { id: Utils.guidToRawFormat(excludedCipherView.id), type: "public-key" },
+            {
+              id: Utils.guidToRawFormat(excludedCipherView.fido2Key.credentialId),
+              type: "public-key",
+            },
           ],
         });
         cipherService.get.mockImplementation(async (id) =>
@@ -312,7 +315,7 @@ describe("FidoAuthenticatorService", () => {
             name: params.rpEntity.name,
 
             fido2Key: expect.objectContaining({
-              nonDiscoverableId: null,
+              credentialId: expect.anything(),
               keyType: "public-key",
               keyAlgorithm: "ECDSA",
               keyCurve: "P-256",
@@ -411,7 +414,7 @@ describe("FidoAuthenticatorService", () => {
 
             login: expect.objectContaining({
               fido2Key: expect.objectContaining({
-                nonDiscoverableId: expect.anything(),
+                credentialId: expect.anything(),
                 keyType: "public-key",
                 keyAlgorithm: "ECDSA",
                 keyCurve: "P-256",
@@ -461,12 +464,8 @@ describe("FidoAuthenticatorService", () => {
         requireResidentKey ? "discoverable" : "non-discoverable"
       } credential`, () => {
         const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b";
-        const cipherIdBytes = new Uint8Array([
-          0x75, 0x28, 0x0e, 0x7e, 0xa7, 0x2e, 0x4d, 0x6c, 0xbf, 0x1e, 0xd3, 0x72, 0x38, 0x35, 0x2f,
-          0x9b,
-        ]);
-        const nonDiscoverableId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa";
-        const nonDiscoverableIdBytes = new Uint8Array([
+        const credentialId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa";
+        const credentialIdBytes = new Uint8Array([
           0x52, 0x21, 0x7b, 0x91, 0x73, 0xf1, 0x4f, 0xea, 0xb3, 0xf2, 0x54, 0xa7, 0x95, 0x9f, 0xd5,
           0xaa,
         ]);
@@ -489,7 +488,9 @@ describe("FidoAuthenticatorService", () => {
           cipherService.getAllDecrypted.mockResolvedValue([await cipher]);
           cipherService.encrypt.mockImplementation(async (cipher) => {
             if (!requireResidentKey) {
-              cipher.login.fido2Key.nonDiscoverableId = nonDiscoverableId; // Replace id for testability
+              cipher.login.fido2Key.credentialId = credentialId; // Replace id for testability
+            } else {
+              cipher.fido2Key.credentialId = credentialId;
             }
             return {} as any;
           });
@@ -534,11 +535,7 @@ describe("FidoAuthenticatorService", () => {
           expect(counter).toEqual(new Uint8Array([0, 0, 0, 0])); // 0 because of new counter
           expect(aaguid).toEqual(AAGUID);
           expect(credentialIdLength).toEqual(new Uint8Array([0, 16])); // 16 bytes because we're using GUIDs
-          if (requireResidentKey) {
-            expect(credentialId).toEqual(cipherIdBytes);
-          } else {
-            expect(credentialId).toEqual(nonDiscoverableIdBytes);
-          }
+          expect(credentialId).toEqual(credentialIdBytes);
         });
       });
     }
@@ -657,7 +654,7 @@ describe("FidoAuthenticatorService", () => {
 
       it("should inform user if credential exists but rpId does not match", async () => {
         const cipher = await createCipherView({ type: CipherType.Login });
-        cipher.login.fido2Key.nonDiscoverableId = credentialId;
+        cipher.login.fido2Key.credentialId = credentialId;
         cipher.login.fido2Key.rpId = "mismatch-rpid";
         cipherService.getAllDecrypted.mockResolvedValue([cipher]);
         userInterfaceSession.informCredentialNotFound.mockResolvedValue();
@@ -700,11 +697,11 @@ describe("FidoAuthenticatorService", () => {
         ciphers = [
           await createCipherView(
             { type: CipherType.Login },
-            { nonDiscoverableId: credentialIds[0], rpId: RpId }
+            { credentialId: credentialIds[0], rpId: RpId }
           ),
           await createCipherView(
-            { type: CipherType.Fido2Key, id: credentialIds[1] },
-            { rpId: RpId }
+            { type: CipherType.Fido2Key },
+            { credentialId: credentialIds[1], rpId: RpId }
           ),
         ];
         params = await createParams({
@@ -769,11 +766,11 @@ describe("FidoAuthenticatorService", () => {
             ciphers = credentialIds.map((id) =>
               createCipherView(
                 { type: CipherType.Fido2Key },
-                { rpId: RpId, counter: 9000, keyValue }
+                { credentialId: id, rpId: RpId, counter: 9000, keyValue }
               )
             );
             fido2Keys = ciphers.map((c) => c.fido2Key);
-            selectedCredentialId = ciphers[0].id;
+            selectedCredentialId = credentialIds[0];
             params = await createParams({
               allowCredentialDescriptorList: undefined,
               rpId: RpId,
@@ -782,7 +779,7 @@ describe("FidoAuthenticatorService", () => {
             ciphers = credentialIds.map((id) =>
               createCipherView(
                 { type: CipherType.Login },
-                { nonDiscoverableId: id, rpId: RpId, counter: 9000 }
+                { credentialId: id, rpId: RpId, counter: 9000 }
               )
             );
             fido2Keys = ciphers.map((c) => c.login.fido2Key);
@@ -937,7 +934,7 @@ function createCipherView(
   cipher.localData = {};
 
   const fido2KeyView = new Fido2KeyView();
-  fido2KeyView.nonDiscoverableId = fido2Key.nonDiscoverableId;
+  fido2KeyView.credentialId = fido2Key.credentialId ?? Utils.newGuid();
   fido2KeyView.rpId = fido2Key.rpId ?? RpId;
   fido2KeyView.counter = fido2Key.counter ?? 0;
   fido2KeyView.userHandle = fido2Key.userHandle ?? Fido2Utils.bufferToString(randomBytes(16));

diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts
@@ -96,6 +96,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
       let fido2Key: Fido2KeyView;
       let keyPair: CryptoKeyPair;
       let userVerified = false;
+      let credentialId: string;
       if (params.requireResidentKey) {
         const response = await userInterfaceSession.confirmNewCredential(
           {
@@ -132,6 +133,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
           const encrypted = await this.cipherService.encrypt(cipher);
           await this.cipherService.createWithServer(encrypted); // encrypted.id is assigned inside here
           cipher.id = encrypted.id;
+          credentialId = cipher.fido2Key.credentialId;
         } catch (error) {
           this.logService?.error(
             `[Fido2Authenticator] Aborting because of unknown error when creating discoverable credential: ${error}`
@@ -172,6 +174,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
           cipher.login.fido2Key = fido2Key = await createKeyView(params, keyPair.privateKey);
           const reencrypted = await this.cipherService.encrypt(cipher);
           await this.cipherService.updateWithServer(reencrypted);
+          credentialId = cipher.login.fido2Key.credentialId;
         } catch (error) {
           this.logService?.error(
             `[Fido2Authenticator] Aborting because of unknown error when creating non-discoverable credential: ${error}`
@@ -180,9 +183,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
         }
       }
 
-      const credentialId =
-        cipher.type === CipherType.Fido2Key ? cipher.id : cipher.login.fido2Key.nonDiscoverableId;
-
       const authData = await generateAuthData({
         rpId: params.rpEntity.id,
         credentialId: Utils.guidToRawFormat(credentialId),
@@ -279,10 +279,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
           selectedCipher.type === CipherType.Login
             ? selectedCipher.login.fido2Key
             : selectedCipher.fido2Key;
-        const selectedCredentialId =
-          selectedCipher.type === CipherType.Login
-            ? selectedFido2Key.nonDiscoverableId
-            : selectedCipher.id;
+        const selectedCredentialId = selectedFido2Key.credentialId;
 
         ++selectedFido2Key.counter;
 
@@ -349,10 +346,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
         (cipher) =>
           !cipher.isDeleted &&
           cipher.organizationId == undefined &&
-          ((cipher.type === CipherType.Fido2Key && ids.includes(cipher.id)) ||
+          ((cipher.type === CipherType.Fido2Key && ids.includes(cipher.fido2Key.credentialId)) ||
             (cipher.type === CipherType.Login &&
               cipher.login.fido2Key != undefined &&
-              ids.includes(cipher.login.fido2Key.nonDiscoverableId)))
+              ids.includes(cipher.login.fido2Key.credentialId)))
       )
       .map((cipher) => cipher.id);
   }
@@ -381,10 +378,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
           cipher.type === CipherType.Login &&
           cipher.login.fido2Key != undefined &&
           cipher.login.fido2Key.rpId === rpId &&
-          ids.includes(cipher.login.fido2Key.nonDiscoverableId)) ||
+          ids.includes(cipher.login.fido2Key.credentialId)) ||
         (cipher.type === CipherType.Fido2Key &&
           cipher.fido2Key.rpId === rpId &&
-          ids.includes(cipher.id))
+          ids.includes(cipher.fido2Key.credentialId))
     );
   }
 
@@ -418,7 +415,7 @@ async function createKeyView(
 
   const pkcs8Key = await crypto.subtle.exportKey("pkcs8", keyValue);
   const fido2Key = new Fido2KeyView();
-  fido2Key.nonDiscoverableId = params.requireResidentKey ? null : Utils.newGuid();
+  fido2Key.credentialId = Utils.newGuid();
   fido2Key.keyType = "public-key";
   fido2Key.keyAlgorithm = "ECDSA";
   fido2Key.keyCurve = "P-256";