[PM-1859] Refactor to credentialId (#6034)

* PM-1859 Refactor to credentialId

* PM-1859 Minor changes

* PM-1859 Fix credentialId initialization logic

* PM-1859 Added missing logic

* PM-1859 Fixed logic to use credentialID instead of cipher.id

* [PM-1859] fix: missing renames

---------

Co-authored-by: Andreas Coroiu <andreas.coroiu@gmail.com>

libs/common/src/models/export/fido2key.export.ts

@@ -6,7 +6,7 @@ import { Fido2Key as Fido2KeyDomain } from "./../../vault/models/domain/fido2-ke
 export class Fido2KeyExport {
   static template(): Fido2KeyExport {
     const req = new Fido2KeyExport();
-    req.nonDiscoverableId = "keyId";
+    req.credentialId = "keyId";
     req.keyType = "keyType";
     req.keyAlgorithm = "keyAlgorithm";
     req.keyCurve = "keyCurve";
@@ -20,7 +20,7 @@ export class Fido2KeyExport {
   }
 
   static toView(req: Fido2KeyExport, view = new Fido2KeyView()) {
-    view.nonDiscoverableId = req.nonDiscoverableId;
+    view.credentialId = req.credentialId;
     view.keyType = req.keyType as "public-key";
     view.keyAlgorithm = req.keyAlgorithm as "ECDSA";
     view.keyCurve = req.keyCurve as "P-256";
@@ -34,8 +34,7 @@ export class Fido2KeyExport {
   }
 
   static toDomain(req: Fido2KeyExport, domain = new Fido2KeyDomain()) {
-    domain.nonDiscoverableId =
+    domain.credentialId = req.credentialId != null ? new EncString(req.credentialId) : null;
-      req.nonDiscoverableId != null ? new EncString(req.nonDiscoverableId) : null;
     domain.keyType = req.keyType != null ? new EncString(req.keyType) : null;
     domain.keyAlgorithm = req.keyAlgorithm != null ? new EncString(req.keyAlgorithm) : null;
     domain.keyCurve = req.keyCurve != null ? new EncString(req.keyCurve) : null;
@@ -49,7 +48,7 @@ export class Fido2KeyExport {
     return domain;
   }
 
-  nonDiscoverableId: string;
+  credentialId: string;
   keyType: string;
   keyAlgorithm: string;
   keyCurve: string;
@@ -66,7 +65,7 @@ export class Fido2KeyExport {
     }
 
     if (o instanceof Fido2KeyView) {
-      this.nonDiscoverableId = o.nonDiscoverableId;
+      this.credentialId = o.credentialId;
       this.keyType = o.keyType;
       this.keyAlgorithm = o.keyAlgorithm;
       this.keyCurve = o.keyCurve;
@@ -77,7 +76,7 @@ export class Fido2KeyExport {
       this.rpName = o.rpName;
       this.userDisplayName = o.userDisplayName;
     } else {
-      this.nonDiscoverableId = o.nonDiscoverableId?.encryptedString;
+      this.credentialId = o.credentialId?.encryptedString;
       this.keyType = o.keyType?.encryptedString;
       this.keyAlgorithm = o.keyAlgorithm?.encryptedString;
       this.keyCurve = o.keyCurve?.encryptedString;

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";
@@ -18,7 +18,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");

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";
@@ -17,7 +17,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;

libs/common/src/vault/models/domain/fido2-key.spec.ts

@@ -12,7 +12,7 @@ describe("Fido2Key", () => {
       const fido2Key = new Fido2Key(data);
 
       expect(fido2Key).toEqual({
-        nonDiscoverableId: null,
+        credentialId: null,
         keyType: null,
         keyAlgorithm: null,
         keyCurve: null,
@@ -27,7 +27,7 @@ describe("Fido2Key", () => {
 
     it("returns all fields as EncStrings when given full Fido2KeyData", () => {
       const data: Fido2KeyData = {
-        nonDiscoverableId: "nonDiscoverableId",
+        credentialId: "credentialId",
         keyType: "public-key",
         keyAlgorithm: "ECDSA",
         keyCurve: "P-256",
@@ -41,7 +41,7 @@ describe("Fido2Key", () => {
       const fido2Key = new Fido2Key(data);
 
       expect(fido2Key).toEqual({
-        nonDiscoverableId: { encryptedString: "nonDiscoverableId", encryptionType: 0 },
+        credentialId: { encryptedString: "credentialId", encryptionType: 0 },
         keyType: { encryptedString: "public-key", encryptionType: 0 },
         keyAlgorithm: { encryptedString: "ECDSA", encryptionType: 0 },
         keyCurve: { encryptedString: "P-256", encryptionType: 0 },
@@ -58,7 +58,7 @@ describe("Fido2Key", () => {
       const fido2Key = new Fido2Key();
 
       expect(fido2Key).toEqual({
-        nonDiscoverableId: null,
+        credentialId: null,
       });
     });
   });
@@ -66,7 +66,7 @@ describe("Fido2Key", () => {
   describe("decrypt", () => {
     it("decrypts and populates all fields when populated with EncStrings", async () => {
       const fido2Key = new Fido2Key();
-      fido2Key.nonDiscoverableId = mockEnc("nonDiscoverableId");
+      fido2Key.credentialId = mockEnc("credentialId");
       fido2Key.keyType = mockEnc("keyType");
       fido2Key.keyAlgorithm = mockEnc("keyAlgorithm");
       fido2Key.keyCurve = mockEnc("keyCurve");
@@ -80,7 +80,7 @@ describe("Fido2Key", () => {
       const fido2KeyView = await fido2Key.decrypt(null);
 
       expect(fido2KeyView).toEqual({
-        nonDiscoverableId: "nonDiscoverableId",
+        credentialId: "credentialId",
         keyType: "keyType",
         keyAlgorithm: "keyAlgorithm",
         keyCurve: "keyCurve",
@@ -97,7 +97,7 @@ describe("Fido2Key", () => {
   describe("toFido2KeyData", () => {
     it("encodes to data object when converted from Fido2KeyData and back", () => {
       const data: Fido2KeyData = {
-        nonDiscoverableId: "nonDiscoverableId",
+        credentialId: "credentialId",
         keyType: "public-key",
         keyAlgorithm: "ECDSA",
         keyCurve: "P-256",
@@ -119,7 +119,7 @@ describe("Fido2Key", () => {
   describe("fromJSON", () => {
     it("recreates equivalent object when converted to JSON and back", () => {
       const fido2Key = new Fido2Key();
-      fido2Key.nonDiscoverableId = createEncryptedEncString("nonDiscoverableId");
+      fido2Key.credentialId = createEncryptedEncString("credentialId");
       fido2Key.keyType = createEncryptedEncString("keyType");
       fido2Key.keyAlgorithm = createEncryptedEncString("keyAlgorithm");
       fido2Key.keyCurve = createEncryptedEncString("keyCurve");

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;
@@ -28,7 +28,7 @@ export class Fido2Key extends Domain {
       this,
       obj,
       {
-        nonDiscoverableId: null,
+        credentialId: null,
         keyType: null,
         keyAlgorithm: null,
         keyCurve: null,
@@ -47,7 +47,7 @@ export class Fido2Key extends Domain {
     const view = await this.decryptObj(
       new Fido2KeyView(),
       {
-        nonDiscoverableId: null,
+        credentialId: null,
         keyType: null,
         keyAlgorithm: null,
         keyCurve: null,
@@ -78,7 +78,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,
@@ -97,7 +97,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);
@@ -109,7 +109,7 @@ export class Fido2Key extends Domain {
     const userDisplayName = EncString.fromJSON(obj.userDisplayName);
 
     return Object.assign(new Fido2Key(), obj, {
-      nonDiscoverableId,
+      credentialId,
       keyType,
       keyAlgorithm,
       keyCurve,

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,
@@ -133,7 +133,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",

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 =
+          this.login.fido2Key.credentialId =
-            cipher.login.fido2Key.nonDiscoverableId != null
+            cipher.login.fido2Key.credentialId != null
-              ? cipher.login.fido2Key.nonDiscoverableId.encryptedString
+              ? 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 =
+        this.fido2Key.credentialId =
-          cipher.fido2Key.nonDiscoverableId != null
+          cipher.fido2Key.credentialId != null
-            ? cipher.fido2Key.nonDiscoverableId.encryptedString
+            ? cipher.fido2Key.credentialId.encryptedString
             : null;
         this.fido2Key.keyType =
           cipher.fido2Key.keyType != null

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";

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,

libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts

@@ -108,12 +108,12 @@ describe("FidoAuthenticatorService", () => {
       beforeEach(async () => {
         excludedCipher = createCipherView(
           { type: CipherType.Login },
-          { nonDiscoverableId: Utils.newGuid() }
+          { credentialId: Utils.newGuid() }
         );
         params = await createParams({
           excludeCredentialDescriptorList: [
             {
-              id: guidToRawFormat(excludedCipher.login.fido2Key.nonDiscoverableId),
+              id: guidToRawFormat(excludedCipher.login.fido2Key.credentialId),
               type: "public-key",
             },
           ],
@@ -187,7 +187,10 @@ describe("FidoAuthenticatorService", () => {
         excludedCipherView = createCipherView();
         params = await createParams({
           excludeCredentialDescriptorList: [
-            { id: guidToRawFormat(excludedCipherView.id), type: "public-key" },
+            {
+              id: guidToRawFormat(excludedCipherView.fido2Key.credentialId),
+              type: "public-key",
+            },
           ],
         });
         cipherService.get.mockImplementation(async (id) =>
@@ -313,7 +316,7 @@ describe("FidoAuthenticatorService", () => {
             name: params.rpEntity.name,
 
             fido2Key: expect.objectContaining({
-              nonDiscoverableId: null,
+              credentialId: expect.anything(),
               keyType: "public-key",
               keyAlgorithm: "ECDSA",
               keyCurve: "P-256",
@@ -412,7 +415,7 @@ describe("FidoAuthenticatorService", () => {
 
             login: expect.objectContaining({
               fido2Key: expect.objectContaining({
-                nonDiscoverableId: expect.anything(),
+                credentialId: expect.anything(),
                 keyType: "public-key",
                 keyAlgorithm: "ECDSA",
                 keyCurve: "P-256",
@@ -462,12 +465,8 @@ describe("FidoAuthenticatorService", () => {
         requireResidentKey ? "discoverable" : "non-discoverable"
       } credential`, () => {
         const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b";
-        const cipherIdBytes = new Uint8Array([
+        const credentialId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa";
-          0x75, 0x28, 0x0e, 0x7e, 0xa7, 0x2e, 0x4d, 0x6c, 0xbf, 0x1e, 0xd3, 0x72, 0x38, 0x35, 0x2f,
+        const credentialIdBytes = new Uint8Array([
-          0x9b,
-        ]);
-        const nonDiscoverableId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa";
-        const nonDiscoverableIdBytes = new Uint8Array([
           0x52, 0x21, 0x7b, 0x91, 0x73, 0xf1, 0x4f, 0xea, 0xb3, 0xf2, 0x54, 0xa7, 0x95, 0x9f, 0xd5,
           0xaa,
         ]);
@@ -490,7 +489,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;
           });
@@ -535,11 +536,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(credentialIdBytes);
-            expect(credentialId).toEqual(cipherIdBytes);
-          } else {
-            expect(credentialId).toEqual(nonDiscoverableIdBytes);
-          }
         });
       });
     }
@@ -658,7 +655,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();
@@ -701,11 +698,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] },
+            { type: CipherType.Fido2Key },
-            { rpId: RpId }
+            { credentialId: credentialIds[1], rpId: RpId }
           ),
         ];
         params = await createParams({
@@ -770,11 +767,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,
@@ -783,7 +780,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);
@@ -938,7 +935,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));

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: guidToRawFormat(credentialId),
@@ -279,10 +279,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
           selectedCipher.type === CipherType.Login
             ? selectedCipher.login.fido2Key
             : selectedCipher.fido2Key;
-        const selectedCredentialId =
+        const selectedCredentialId = selectedFido2Key.credentialId;
-          selectedCipher.type === CipherType.Login
-            ? selectedFido2Key.nonDiscoverableId
-            : selectedCipher.id;
 
         ++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";