diff --git a/apps/cli/src/vault/models/cipher.response.ts b/apps/cli/src/vault/models/cipher.response.ts index 87e158eb23d3..373139ae7f70 100644 --- a/apps/cli/src/vault/models/cipher.response.ts +++ b/apps/cli/src/vault/models/cipher.response.ts @@ -22,6 +22,8 @@ export class CipherResponse extends CipherWithIdExport implements BaseResponse { super(); this.object = "item"; this.build(o); + // Redact the decrypted item key to prevent logging it to the CLI output. + this.key = undefined; if (o.attachments != null) { this.attachments = o.attachments.map((a) => new AttachmentResponse(a)); } diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/v2-key-rotation-migration.spec.ts b/libs/common/src/key-management/encrypted-migrator/migrations/v2-key-rotation-migration.spec.ts index 8b0dd1645522..09bdf43effbf 100644 --- a/libs/common/src/key-management/encrypted-migrator/migrations/v2-key-rotation-migration.spec.ts +++ b/libs/common/src/key-management/encrypted-migrator/migrations/v2-key-rotation-migration.spec.ts @@ -16,7 +16,6 @@ import { UserKey } from "../../../types/key"; import { CipherService } from "../../../vault/abstractions/cipher.service"; import { AttachmentView } from "../../../vault/models/view/attachment.view"; import { CipherView } from "../../../vault/models/view/cipher.view"; -import { EncString } from "../../crypto/models/enc-string"; import { MasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; import { V2KeyRotationMigration } from "./v2-key-rotation-migration"; @@ -44,11 +43,9 @@ describe("V2KeyRotationMigration", () => { return cipher; }; - const makeAttachment = (hasEncryptedKey: boolean): AttachmentView => { + const makeAttachment = (hasKey: boolean): AttachmentView => { const a = new AttachmentView(); - a.encryptedKey = hasEncryptedKey - ? new EncString("2.abc|def|ghi") - : (undefined as unknown as EncString); + a.key = hasKey ? mock() : undefined; return a; }; diff --git a/libs/common/src/models/export/cipher.export.ts b/libs/common/src/models/export/cipher.export.ts index 7ab7173ea5d0..364da49f6be0 100644 --- a/libs/common/src/models/export/cipher.export.ts +++ b/libs/common/src/models/export/cipher.export.ts @@ -1,4 +1,5 @@ import { EncString } from "../../key-management/crypto/models/enc-string"; +import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; import { CipherRepromptType } from "../../vault/enums/cipher-reprompt-type"; import { CipherType } from "../../vault/enums/cipher-type"; import { Cipher as CipherDomain } from "../../vault/models/domain/cipher"; @@ -43,7 +44,12 @@ export class CipherExport { view.notes = req.notes; view.favorite = req.favorite; view.reprompt = req.reprompt ?? CipherRepromptType.None; - view.key = req.key != null ? new EncString(req.key) : undefined; + try { + view.key = req.key != null ? SymmetricCryptoKey.fromString(req.key) : undefined; + } catch { + // Old exports stored the wrapped EncString key which cannot be used on import + view.key = undefined; + } if (req.fields != null) { view.fields = req.fields.map((f) => FieldExport.toView(f)); @@ -208,7 +214,10 @@ export class CipherExport { this.name = safeGetString(o.name) ?? ""; this.notes = safeGetString(o.notes); if ("key" in o) { - this.key = o.key?.encryptedString; + this.key = + o.key instanceof SymmetricCryptoKey + ? o.key.toBase64() + : (o.key as EncString | undefined)?.encryptedString; } this.favorite = o.favorite; diff --git a/libs/common/src/vault/models/domain/attachment.ts b/libs/common/src/vault/models/domain/attachment.ts index b9bcaad8cea9..23d23a8c5a5e 100644 --- a/libs/common/src/vault/models/domain/attachment.ts +++ b/libs/common/src/vault/models/domain/attachment.ts @@ -46,7 +46,6 @@ export class Attachment extends Domain { if (this.key != null) { view.key = await this.decryptAttachmentKey(decryptionKey); - view.encryptedKey = this.key; // Keep the encrypted key for the view // When the attachment key couldn't be decrypted, mark a decryption error // The file won't be able to be downloaded in these cases diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index 8602d2a7adc0..48ebb2c71a06 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -162,6 +162,7 @@ export class Cipher extends Domain implements Decryptable { try { const cipherKey = await encryptService.unwrapSymmetricKey(this.key, userKeyOrOrgKey); cipherDecryptionKey = cipherKey; + model.key = cipherKey; bypassValidation = false; } catch { model.name = "[error: cannot decrypt]"; diff --git a/libs/common/src/vault/models/view/attachment.view.spec.ts b/libs/common/src/vault/models/view/attachment.view.spec.ts index 31815ac9cac3..3db683a162fb 100644 --- a/libs/common/src/vault/models/view/attachment.view.spec.ts +++ b/libs/common/src/vault/models/view/attachment.view.spec.ts @@ -1,7 +1,6 @@ import { AttachmentView as SdkAttachmentView } from "@bitwarden/sdk-internal"; import { mockFromJson } from "../../../../spec"; -import { EncString } from "../../../key-management/crypto/models/enc-string"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { AttachmentView } from "./attachment.view"; @@ -34,8 +33,7 @@ describe("AttachmentView", () => { size: "size", sizeName: "sizeName", fileName: "fileName", - key: "encKeyB64_fromString", - decryptedKey: "decryptedKey_B64", + key: "decryptedKey_B64", } as SdkAttachmentView; const result = AttachmentView.fromSdkAttachmentView(sdkAttachmentView); @@ -47,7 +45,6 @@ describe("AttachmentView", () => { sizeName: "sizeName", fileName: "fileName", key: "mockKey", - encryptedKey: new EncString(sdkAttachmentView.key as string), }); expect(SymmetricCryptoKey.fromString).toHaveBeenCalledWith("decryptedKey_B64"); @@ -57,7 +54,7 @@ describe("AttachmentView", () => { describe("toSdkAttachmentView", () => { it("should convert AttachmentView to SdkAttachmentView", () => { const mockKey = { - toBase64: jest.fn().mockReturnValue("keyB64"), + toSdk: jest.fn().mockReturnValue("keyB64"), } as any; const attachmentView = new AttachmentView(); @@ -66,7 +63,6 @@ describe("AttachmentView", () => { attachmentView.size = "size"; attachmentView.sizeName = "sizeName"; attachmentView.fileName = "fileName"; - attachmentView.encryptedKey = new EncString("encKeyB64"); attachmentView.key = mockKey; const result = attachmentView.toSdkAttachmentView(); @@ -77,8 +73,7 @@ describe("AttachmentView", () => { size: "size", sizeName: "sizeName", fileName: "fileName", - key: "encKeyB64", - decryptedKey: "keyB64", + key: "keyB64", }); }); }); diff --git a/libs/common/src/vault/models/view/attachment.view.ts b/libs/common/src/vault/models/view/attachment.view.ts index 724e9304c6c7..199e9f651991 100644 --- a/libs/common/src/vault/models/view/attachment.view.ts +++ b/libs/common/src/vault/models/view/attachment.view.ts @@ -2,7 +2,7 @@ import { Jsonify } from "type-fest"; import { AttachmentView as SdkAttachmentView } from "@bitwarden/sdk-internal"; -import { DECRYPT_ERROR, EncString } from "../../../key-management/crypto/models/enc-string"; +import { DECRYPT_ERROR } from "../../../key-management/crypto/models/enc-string"; import { View } from "../../../models/view/view"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { Attachment } from "../domain/attachment"; @@ -14,10 +14,6 @@ export class AttachmentView implements View { sizeName?: string; fileName?: string; key?: SymmetricCryptoKey; - /** - * The SDK returns an encrypted key for the attachment. - */ - encryptedKey: EncString | undefined; private _hasDecryptionError?: boolean; constructor(a?: Attachment) { @@ -52,18 +48,7 @@ export class AttachmentView implements View { static fromJSON(obj: Partial>): AttachmentView { const key = obj.key == null ? null : SymmetricCryptoKey.fromJSON(obj.key); - - let encryptedKey: EncString | undefined; - if (obj.encryptedKey != null) { - if (typeof obj.encryptedKey === "string") { - // If the key is a string, we need to parse it as EncString - encryptedKey = EncString.fromJSON(obj.encryptedKey); - } else if ((obj.encryptedKey as any) instanceof EncString) { - // If the key is already an EncString instance, we can use it directly - encryptedKey = obj.encryptedKey; - } - } - return Object.assign(new AttachmentView(), obj, { key: key, encryptedKey: encryptedKey }); + return Object.assign(new AttachmentView(), obj, { key: key }); } /** @@ -76,9 +61,7 @@ export class AttachmentView implements View { size: this.size, sizeName: this.sizeName, fileName: this.fileName, - key: this.encryptedKey?.toSdk(), - // TODO: PM-23005 - Temporary field, should be removed when encrypted migration is complete - decryptedKey: this.key ? this.key.toBase64() : undefined, + key: this.key?.toSdk(), }; } @@ -99,9 +82,7 @@ export class AttachmentView implements View { view.size = obj.size; view.sizeName = obj.sizeName; view.fileName = obj.fileName; - // TODO: PM-23005 - Temporary field, should be removed when encrypted migration is complete - view.key = obj.decryptedKey ? SymmetricCryptoKey.fromString(obj.decryptedKey) : undefined; - view.encryptedKey = obj.key ? new EncString(obj.key) : undefined; + view.key = obj.key ? SymmetricCryptoKey.fromString(obj.key) : undefined; view._hasDecryptionError = failure; return view; @@ -112,6 +93,6 @@ export class AttachmentView implements View { * In this case, the attachment is encrypted with the user's user-key */ isLegacyAttachment(): boolean { - return this.key == null && this.encryptedKey == null; + return this.key == null && !this.hasDecryptionError; } } diff --git a/libs/common/src/vault/models/view/cipher.view.spec.ts b/libs/common/src/vault/models/view/cipher.view.spec.ts index bcebdf1136be..a9973748ad8c 100644 --- a/libs/common/src/vault/models/view/cipher.view.spec.ts +++ b/libs/common/src/vault/models/view/cipher.view.spec.ts @@ -1,5 +1,3 @@ -import { Jsonify } from "type-fest"; - import { CiphersClient, CipherView as SdkCipherView, @@ -13,8 +11,8 @@ import { } from "@bitwarden/sdk-internal"; import { mockFromJson, mockFromSdk } from "../../../../spec"; -import { EncString } from "../../../key-management/crypto/models/enc-string"; import { asUuid } from "../../../platform/abstractions/sdk/sdk.service"; +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { CipherRepromptType } from "../../enums"; import { CipherType } from "../../enums/cipher-type"; import { CipherPermissionsApi } from "../api/cipher-permissions.api"; @@ -94,23 +92,13 @@ describe("CipherView", () => { expect(actual).toMatchObject(expected); }); - it("handle both string and object inputs for the cipher key", () => { - const cipherKeyString = "cipherKeyString"; - const cipherKeyObject = new EncString("cipherKeyObject"); + it("deserializes the cipher key to a SymmetricCryptoKey", () => { + const mockKey = { keyB64: "c29tZS1iYXNlNjQta2V5" }; + jest.spyOn(SymmetricCryptoKey, "fromJSON").mockReturnValue(mockKey as any); - // Test with string input - let actual = CipherView.fromJSON({ - key: cipherKeyString, - }); - expect(actual.key).toBeInstanceOf(EncString); - expect(actual.key?.toJSON()).toBe(cipherKeyString); - - // Test with object input (which can happen when cipher view is stored in an InMemory state provider) - actual = CipherView.fromJSON({ - key: cipherKeyObject, - } as Jsonify); - expect(actual.key).toBeInstanceOf(EncString); - expect(actual.key?.toJSON()).toBe(cipherKeyObject.toJSON()); + const actual = CipherView.fromJSON({ key: mockKey as any }); + expect(actual.key).toBe(mockKey); + expect(SymmetricCryptoKey.fromJSON).toHaveBeenCalledWith(mockKey); }); it("fromJSON should always restore top-level CipherView properties", () => { @@ -143,7 +131,7 @@ describe("CipherView", () => { original.deletedDate = new Date("2022-01-03"); original.archivedDate = new Date("2022-01-04"); original.reprompt = CipherRepromptType.Password; - original.key = new EncString("test-key"); + original.key = new SymmetricCryptoKey(new Uint8Array(64)); original.decryptionFailure = true; // Serialize and deserialize @@ -304,7 +292,7 @@ describe("CipherView", () => { cipherView.organizationId = "000f2a6e-da5e-4726-87ed-1c5c77322c3c"; cipherView.folderId = "41b22db4-8e2a-4ed2-b568-f1186c72922f"; cipherView.collectionIds = ["b0473506-3c3c-4260-a734-dfaaf833ab6f"]; - cipherView.key = new EncString("some-key"); + cipherView.key = { toSdk: () => "some-key-b64" } as any; cipherView.name = "name"; cipherView.notes = "notes"; cipherView.type = CipherType.Login; @@ -334,7 +322,7 @@ describe("CipherView", () => { organizationId: asUuid("000f2a6e-da5e-4726-87ed-1c5c77322c3c"), folderId: asUuid("41b22db4-8e2a-4ed2-b568-f1186c72922f"), collectionIds: [asUuid("b0473506-3c3c-4260-a734-dfaaf833ab6f")], - key: "some-key" as any, + key: "some-key-b64" as any, name: "name", notes: "notes", type: SdkCipherType.Login, @@ -516,7 +504,7 @@ describe("CipherView", () => { cipherView.reprompt = CipherRepromptType.Password; cipherView.revisionDate = new Date("2022-01-02T12:00:00.000Z"); cipherView.archivedDate = new Date("2022-01-03T12:00:00.000Z"); - cipherView.key = new EncString("cipher-key"); + cipherView.key = { toSdk: () => "cipher-key-b64" } as any; const mockField = new RealFieldView(); mockField.name = "testField"; diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index e914e5b0d3e2..e7ce001791a5 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -7,10 +7,10 @@ import { CipherView as SdkCipherView, } from "@bitwarden/sdk-internal"; -import { EncString } from "../../../key-management/crypto/models/enc-string"; import { View } from "../../../models/view/view"; import { asUuid, uuidAsString } from "../../../platform/abstractions/sdk/sdk.service"; import { InitializerMetadata } from "../../../platform/interfaces/initializer-metadata.interface"; +import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { InitializerKey } from "../../../platform/services/cryptography/initializer-key"; import { DeepJsonify } from "../../../types/deep-jsonify"; import { CipherType, LinkedIdType } from "../../enums"; @@ -23,7 +23,6 @@ import { AttachmentView } from "./attachment.view"; import { BankAccountView } from "./bank-account.view"; import { CardView } from "./card.view"; import { DriversLicenseView } from "./drivers-license.view"; -import { Fido2CredentialView } from "./fido2-credential.view"; import { FieldView } from "./field.view"; import { IdentityView } from "./identity.view"; import { ItemView } from "./item.view"; @@ -65,9 +64,7 @@ export class CipherView implements View, InitializerMetadata { deletedDate?: Date; archivedDate?: Date; reprompt: CipherRepromptType = CipherRepromptType.None; - // We need a copy of the encrypted key so we can pass it to - // the SdkCipherView during encryption - key?: EncString; + key?: SymmetricCryptoKey; /** * Flag to indicate if the cipher decryption failed. @@ -97,7 +94,6 @@ export class CipherView implements View, InitializerMetadata { this.archivedDate = c.archivedDate; // Old locally stored ciphers might have reprompt == null. If so set it to None. this.reprompt = c.reprompt ?? CipherRepromptType.None; - this.key = c.key; } private get item(): ItemView | undefined { @@ -252,17 +248,7 @@ export class CipherView implements View, InitializerMetadata { view.passwordHistory = obj.passwordHistory?.map((ph: any) => PasswordHistoryView.fromJSON(ph)) ?? []; - if (obj.key != null) { - let key: EncString | undefined; - if (typeof obj.key === "string") { - // If the key is a string, we need to parse it as EncString - key = EncString.fromJSON(obj.key); - } else if ((obj.key as any) instanceof EncString) { - // If the key is already an EncString instance, we can use it directly - key = obj.key; - } - view.key = key; - } + view.key = obj.key != null ? SymmetricCryptoKey.fromJSON(obj.key) : undefined; switch (obj.type) { case CipherType.Card: @@ -299,7 +285,7 @@ export class CipherView implements View, InitializerMetadata { /** * Creates a CipherView from the SDK CipherView. */ - static fromSdkCipherView(obj: SdkCipherView, sdk?: CiphersClient): CipherView | undefined { + static fromSdkCipherView(obj: SdkCipherView): CipherView | undefined { if (obj == null) { return undefined; } @@ -340,7 +326,7 @@ export class CipherView implements View, InitializerMetadata { cipherView.deletedDate = obj.deletedDate == null ? undefined : new Date(obj.deletedDate); cipherView.archivedDate = obj.archivedDate == null ? undefined : new Date(obj.archivedDate); cipherView.reprompt = obj.reprompt ?? CipherRepromptType.None; - cipherView.key = obj.key ? EncString.fromJSON(obj.key) : undefined; + cipherView.key = obj.key ? SymmetricCryptoKey.fromString(obj.key) : undefined; switch (obj.type) { case CipherType.Card: @@ -353,19 +339,6 @@ export class CipherView implements View, InitializerMetadata { break; case CipherType.Login: cipherView.login = obj.login ? LoginView.fromSdkLoginView(obj.login) : new LoginView(); - if (sdk && obj.login?.fido2Credentials?.length) { - const fido2CredentialViews = sdk.decrypt_fido2_credentials(obj); - const decryptedKeyValue = sdk.decrypt_fido2_private_key(obj); - cipherView.login.fido2Credentials = fido2CredentialViews - .map((cred) => { - const view = Fido2CredentialView.fromSdkFido2CredentialView(cred); - if (view) { - view.keyValue = decryptedKeyValue; - } - return view; - }) - .filter((cred): cred is Fido2CredentialView => !!cred); - } break; case CipherType.SecureNote: cipherView.secureNote = obj.secureNote diff --git a/libs/common/src/vault/models/view/login.view.ts b/libs/common/src/vault/models/view/login.view.ts index 63f6332b3e6b..21440618af6a 100644 --- a/libs/common/src/vault/models/view/login.view.ts +++ b/libs/common/src/vault/models/view/login.view.ts @@ -111,10 +111,7 @@ export class LoginView extends ItemView { /** * Converts the SDK LoginView to a LoginView. * - * Note: FIDO2 credentials remain encrypted at this stage. - * Unlike other fields that are decrypted as part of the LoginView, the SDK maintains - * the FIDO2 credentials in encrypted form. We can decrypt them later using a separate - * call to client.vault().ciphers().decrypt_fido2_credentials(). + * FIDO2 credentials are eagerly decrypted by the SDK and mapped here directly. */ static fromSdkLoginView(obj: SdkLoginView): LoginView { const loginView = new LoginView(); @@ -129,8 +126,10 @@ export class LoginView extends ItemView { obj.uris ?.filter((uri) => uri.uri != null && uri.uri !== "") .map((uri) => LoginUriView.fromSdkLoginUriView(uri)!) || []; - // FIDO2 credentials are not decrypted here, they remain encrypted - loginView.fido2Credentials = []; + loginView.fido2Credentials = + obj.fido2Credentials + ?.map((cred) => Fido2CredentialView.fromSdkFido2CredentialView(cred)) + .filter((cred): cred is Fido2CredentialView => !!cred) ?? []; return loginView; } @@ -138,7 +137,8 @@ export class LoginView extends ItemView { /** * Converts the LoginView to an SDK LoginView. * - * Note: FIDO2 credentials remain encrypted in the SDK view so they are not included here. + * Note: FIDO2 credentials are intentionally excluded on the write path — they are + * handled separately via toSdkCipherView when the cipher has passkeys. */ toSdkLoginView(): SdkLoginView { return { @@ -148,7 +148,7 @@ export class LoginView extends ItemView { totp: this.hasTotp ? this.totp : undefined, autofillOnPageLoad: this.autofillOnPageLoad ?? undefined, uris: this.uris?.map((uri) => uri.toSdkLoginUriView()), - fido2Credentials: undefined, // FIDO2 credentials are handled separately and remain encrypted + fido2Credentials: undefined, }; } } diff --git a/libs/common/src/vault/services/cipher-sdk.service.spec.ts b/libs/common/src/vault/services/cipher-sdk.service.spec.ts index d5ba0188ae58..1683fe5b408f 100644 --- a/libs/common/src/vault/services/cipher-sdk.service.spec.ts +++ b/libs/common/src/vault/services/cipher-sdk.service.spec.ts @@ -66,7 +66,6 @@ describe("DefaultCipherSdkService", () => { share_cipher: jest.fn(), share_ciphers_bulk: jest.fn(), decrypt_fido2_credentials: jest.fn(), - decrypt_fido2_private_key: jest.fn(), get_all: jest.fn().mockResolvedValue({ successes: [], failures: [] }), update_collection: jest.fn(), delete_attachment: jest.fn(), @@ -192,14 +191,13 @@ describe("DefaultCipherSdkService", () => { expect(result?.name).toBe(cipherView.name); }); - it("should decrypt FIDO2 credentials from create response", async () => { + it("should pass sdkCipherView with FIDO2 credentials to fromSdkCipherView", async () => { const cipherView = new CipherView(); cipherView.id = cipherId; cipherView.type = CipherType.Login; cipherView.name = "Test Cipher"; cipherView.organizationId = orgId; - // Build an SDK response that includes encrypted FIDO2 credentials const mockSdkResponse = { ...cipherView.toSdkCipherView(), login: { @@ -209,22 +207,14 @@ describe("DefaultCipherSdkService", () => { } as unknown as SdkCipherView; mockCiphersSdk.create.mockResolvedValue(mockSdkResponse); - // Mock FIDO2 decryption - const mockDecryptedFido2 = [{ credentialId: "decrypted-cred-id" }]; - mockCiphersSdk.decrypt_fido2_credentials.mockReturnValue(mockDecryptedFido2); - mockCiphersSdk.decrypt_fido2_private_key.mockReturnValue("decrypted-key-value"); - const mockFido2View = new Fido2CredentialView(); - mockFido2View.credentialId = "decrypted-cred-id"; + mockFido2View.credentialId = "encrypted-cred-id"; jest.spyOn(Fido2CredentialView, "fromSdkFido2CredentialView").mockReturnValue(mockFido2View); const result = await cipherSdkService.createWithServer(cipherView, userId, false); - expect(mockCiphersSdk.decrypt_fido2_credentials).toHaveBeenCalledWith(mockSdkResponse); - expect(mockCiphersSdk.decrypt_fido2_private_key).toHaveBeenCalledWith(mockSdkResponse); expect(result?.login?.fido2Credentials).toHaveLength(1); - expect(result?.login?.fido2Credentials?.[0].credentialId).toBe("decrypted-cred-id"); - expect(result?.login?.fido2Credentials?.[0].keyValue).toBe("decrypted-key-value"); + expect(result?.login?.fido2Credentials?.[0].credentialId).toBe("encrypted-cred-id"); }); it("should throw error and log when SDK throws an error", async () => { @@ -373,7 +363,7 @@ describe("DefaultCipherSdkService", () => { expect(result.name).toBe(cipherView.name); }); - it("should decrypt FIDO2 credentials from edit response", async () => { + it("should pass sdkCipherView with FIDO2 credentials to fromSdkCipherView", async () => { const cipherView = new CipherView(); cipherView.id = cipherId; cipherView.type = CipherType.Login; @@ -381,7 +371,6 @@ describe("DefaultCipherSdkService", () => { cipherView.organizationId = orgId; cipherView.edit = true; - // Build an SDK response that includes encrypted FIDO2 credentials const mockSdkResponse = { ...cipherView.toSdkCipherView(), login: { @@ -391,22 +380,14 @@ describe("DefaultCipherSdkService", () => { } as unknown as SdkCipherView; mockCiphersSdk.edit.mockResolvedValue(mockSdkResponse); - // Mock FIDO2 decryption - const mockDecryptedFido2 = [{ credentialId: "decrypted-cred-id" }]; - mockCiphersSdk.decrypt_fido2_credentials.mockReturnValue(mockDecryptedFido2); - mockCiphersSdk.decrypt_fido2_private_key.mockReturnValue("decrypted-key-value"); - const mockFido2View = new Fido2CredentialView(); - mockFido2View.credentialId = "decrypted-cred-id"; + mockFido2View.credentialId = "encrypted-cred-id"; jest.spyOn(Fido2CredentialView, "fromSdkFido2CredentialView").mockReturnValue(mockFido2View); const result = await cipherSdkService.updateWithServer(cipherView, userId, undefined, false); - expect(mockCiphersSdk.decrypt_fido2_credentials).toHaveBeenCalledWith(mockSdkResponse); - expect(mockCiphersSdk.decrypt_fido2_private_key).toHaveBeenCalledWith(mockSdkResponse); expect(result?.login?.fido2Credentials).toHaveLength(1); - expect(result?.login?.fido2Credentials?.[0].credentialId).toBe("decrypted-cred-id"); - expect(result?.login?.fido2Credentials?.[0].keyValue).toBe("decrypted-key-value"); + expect(result?.login?.fido2Credentials?.[0].credentialId).toBe("encrypted-cred-id"); }); it("should throw error and log when SDK throws an error", async () => { diff --git a/libs/common/src/vault/services/cipher-sdk.service.ts b/libs/common/src/vault/services/cipher-sdk.service.ts index 7705bfb484c6..2ae4aee8e288 100644 --- a/libs/common/src/vault/services/cipher-sdk.service.ts +++ b/libs/common/src/vault/services/cipher-sdk.service.ts @@ -47,7 +47,7 @@ export class DefaultCipherSdkService implements CipherSdkService { result = await sdkCiphersClient.create(sdkCreateRequest); } - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to create cipher: ${error}`); @@ -87,7 +87,7 @@ export class DefaultCipherSdkService implements CipherSdkService { result = await sdkCiphersClient.edit_partial(sdkPartialUpdateRequest); } - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to update cipher: ${error}`); @@ -282,7 +282,7 @@ export class DefaultCipherSdkService implements CipherSdkService { originalCipherView?.toSdkCipherView(sdkCiphersClient), ); - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to share cipher: ${error}`); @@ -313,7 +313,7 @@ export class DefaultCipherSdkService implements CipherSdkService { ); return results - .map((c) => CipherView.fromSdkCipherView(c, sdkCiphersClient)) + .map((c) => CipherView.fromSdkCipherView(c)) .filter((c): c is CipherView => c !== undefined); }), catchError((error: unknown) => { @@ -441,12 +441,11 @@ export class DefaultCipherSdkService implements CipherSdkService { this.sdkService.userClient$(userId).pipe( switchMap(async (sdk) => { using ref = sdk.take(); - const sdkCiphersClient = ref.value.vault().ciphers(); const result = await ref.value .vault() .attachments() .upgrade_attachment(asUuid(cipherId), attachmentId); - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to upgrade attachment: ${error}`); @@ -466,9 +465,7 @@ export class DefaultCipherSdkService implements CipherSdkService { const decryptResult = await sdkCiphersClient.get_all(); const successes = [...(decryptResult.successes ?? [])] - .map((sdkCipherView: any) => - CipherView.fromSdkCipherView(sdkCipherView, sdkCiphersClient), - ) + .map((sdkCipherView: any) => CipherView.fromSdkCipherView(sdkCipherView)) .filter((v): v is CipherView => v !== undefined); const failures: CipherView[] = [...(decryptResult.failures ?? [])].map((failure: any) => { @@ -611,7 +608,7 @@ export class DefaultCipherSdkService implements CipherSdkService { asUuid(cipherId), collectionIds.map((id) => asUuid(id)), ); - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to update cipher collections as admin: ${error}`); @@ -636,7 +633,7 @@ export class DefaultCipherSdkService implements CipherSdkService { collectionIds.map((id) => asUuid(id)), false, ); - return CipherView.fromSdkCipherView(result, sdkCiphersClient); + return CipherView.fromSdkCipherView(result); }), catchError((error: unknown) => { this.logService.error(`Failed to update cipher collections: ${error}`); diff --git a/libs/common/src/vault/services/default-cipher-encryption.service.spec.ts b/libs/common/src/vault/services/default-cipher-encryption.service.spec.ts index 78e017b5c45b..0202b2247f70 100644 --- a/libs/common/src/vault/services/default-cipher-encryption.service.spec.ts +++ b/libs/common/src/vault/services/default-cipher-encryption.service.spec.ts @@ -11,7 +11,6 @@ import { Fido2CredentialFullView, } from "@bitwarden/sdk-internal"; -import { mockEnc } from "../../../spec"; import { UriMatchStrategy } from "../../models/domain/domain-service"; import { LogService } from "../../platform/abstractions/log.service"; import { SdkService } from "../../platform/abstractions/sdk/sdk.service"; @@ -101,7 +100,6 @@ describe("DefaultCipherEncryptionService", () => { decrypt: jest.fn(), decrypt_list: jest.fn(), decrypt_list_with_failures: jest.fn(), - decrypt_fido2_credentials: jest.fn(), move_to_organization: jest.fn(), }), attachments: jest.fn().mockReturnValue({ @@ -466,79 +464,28 @@ describe("DefaultCipherEncryptionService", () => { expect(cipherObj.toSdkCipher).toHaveBeenCalledTimes(1); expect(mockSdkClient.vault().ciphers().decrypt).toHaveBeenCalledWith({ id: cipherData.id }); expect(CipherView.fromSdkCipherView).toHaveBeenCalledWith(sdkCipherView); - expect(mockSdkClient.vault().ciphers().decrypt_fido2_credentials).not.toHaveBeenCalled(); }); - it("should decrypt FIDO2 credentials if present", async () => { + it("should pass sdkCipherView with FIDO2 credentials to fromSdkCipherView", async () => { const fido2Credentials = [ - { - credentialId: mockEnc("credentialId"), - keyType: mockEnc("keyType"), - keyAlgorithm: mockEnc("keyAlgorithm"), - keyCurve: mockEnc("keyCurve"), - keyValue: mockEnc("keyValue"), - rpId: mockEnc("rpId"), - userHandle: mockEnc("userHandle"), - userName: mockEnc("userName"), - counter: mockEnc("2"), - rpName: mockEnc("rpName"), - userDisplayName: mockEnc("userDisplayName"), - discoverable: mockEnc("true"), - creationDate: new Date("2023-01-01T12:00:00.000Z"), - }, + { credentialId: "credentialId" }, ] as unknown as SdkFido2Credential[]; - sdkCipherView.login!.fido2Credentials = fido2Credentials; const expectedCipherView: CipherView = { id: cipherId, type: CipherType.Login, name: "test-name", - login: { - username: "test-username", - password: "test-password", - fido2Credentials: [], - }, + login: { username: "test-username", fido2Credentials: [] }, } as unknown as CipherView; - const fido2CredentialView: Fido2CredentialView = { - credentialId: "credentialId", - keyType: "keyType", - keyAlgorithm: "keyAlgorithm", - keyCurve: "keyCurve", - keyValue: "decrypted-key-value", - rpId: "rpId", - userHandle: "userHandle", - userName: "userName", - counter: 2, - rpName: "rpName", - userDisplayName: "userDisplayName", - discoverable: true, - creationDate: new Date("2023-01-01T12:00:00.000Z"), - } as unknown as Fido2CredentialView; - mockSdkClient.vault().ciphers().decrypt.mockReturnValue(sdkCipherView); - mockSdkClient.vault().ciphers().decrypt_fido2_credentials.mockReturnValue(fido2Credentials); - mockSdkClient.vault().ciphers().decrypt_fido2_private_key = jest - .fn() - .mockReturnValue("decrypted-key-value"); - jest.spyOn(CipherView, "fromSdkCipherView").mockReturnValue(expectedCipherView); - jest - .spyOn(Fido2CredentialView, "fromSdkFido2CredentialView") - .mockReturnValueOnce(fido2CredentialView); const result = await cipherEncryptionService.decrypt(cipherObj, userId); expect(result).toBe(expectedCipherView); - expect(result.login?.fido2Credentials).toEqual([fido2CredentialView]); - expect(mockSdkClient.vault().ciphers().decrypt_fido2_credentials).toHaveBeenCalledWith( - sdkCipherView, - ); - expect(mockSdkClient.vault().ciphers().decrypt_fido2_private_key).toHaveBeenCalledWith( - sdkCipherView, - ); - expect(Fido2CredentialView.fromSdkFido2CredentialView).toHaveBeenCalledTimes(1); + expect(CipherView.fromSdkCipherView).toHaveBeenCalledWith(sdkCipherView); }); }); diff --git a/libs/common/src/vault/services/default-cipher-encryption.service.ts b/libs/common/src/vault/services/default-cipher-encryption.service.ts index 5458c65c0e1b..65b0bf39fd25 100644 --- a/libs/common/src/vault/services/default-cipher-encryption.service.ts +++ b/libs/common/src/vault/services/default-cipher-encryption.service.ts @@ -8,11 +8,9 @@ import { UserId, OrganizationId } from "../../types/guid"; import { UserKey } from "../../types/key"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; import { EncryptionContext } from "../abstractions/cipher.service"; -import { CipherType } from "../enums"; import { Cipher } from "../models/domain/cipher"; import { AttachmentView } from "../models/view/attachment.view"; import { CipherView } from "../models/view/cipher.view"; -import { Fido2CredentialView } from "../models/view/fido2-credential.view"; export class DefaultCipherEncryptionService implements CipherEncryptionService { constructor( @@ -142,32 +140,6 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService { const clientCipherView = CipherView.fromSdkCipherView(sdkCipherView)!; - // Decrypt Fido2 credentials if available - if ( - clientCipherView.type === CipherType.Login && - sdkCipherView.login?.fido2Credentials?.length - ) { - const fido2CredentialViews = ref.value - .vault() - .ciphers() - .decrypt_fido2_credentials(sdkCipherView); - - // TEMPORARY: Manually decrypt the keyValue for Fido2 credentials - // since we don't currently use the SDK for Fido2 Authentication. - const decryptedKeyValue = ref.value - .vault() - .ciphers() - .decrypt_fido2_private_key(sdkCipherView); - - clientCipherView.login.fido2Credentials = fido2CredentialViews - .map((f) => { - const view = Fido2CredentialView.fromSdkFido2CredentialView(f)!; - view.keyValue = decryptedKeyValue; - return view; - }) - .filter((view): view is Fido2CredentialView => view !== undefined); - } - return clientCipherView; }), catchError((error: unknown) => { @@ -192,30 +164,6 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService { const sdkCipherView = await ref.value.vault().ciphers().decrypt(cipher.toSdkCipher()); const clientCipherView = CipherView.fromSdkCipherView(sdkCipherView)!; - // Handle FIDO2 credentials if present - if ( - clientCipherView.type === CipherType.Login && - sdkCipherView.login?.fido2Credentials?.length - ) { - const fido2CredentialViews = ref.value - .vault() - .ciphers() - .decrypt_fido2_credentials(sdkCipherView); - - const decryptedKeyValue = ref.value - .vault() - .ciphers() - .decrypt_fido2_private_key(sdkCipherView); - - clientCipherView.login.fido2Credentials = fido2CredentialViews - .map((f) => { - const view = Fido2CredentialView.fromSdkFido2CredentialView(f)!; - view.keyValue = decryptedKeyValue; - return view; - }) - .filter((view): view is Fido2CredentialView => view !== undefined); - } - successful.push(clientCipherView); } catch (error) { this.logService.error(`Failed to decrypt cipher ${cipher.id}: ${error}`);