From e50e52492093af48828e3ed6ac315b752690134d Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Wed, 5 Jul 2023 15:06:09 -0400 Subject: [PATCH] =?UTF-8?q?Tweak=20device=20trust=20crypto=20service=20imp?= =?UTF-8?q?lementation=20to=20match=20mobile=20late=E2=80=A6=20(#5744)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Tweak device trust crypto service implementation to match mobile latest which results in more single responsibility methods * Update tests to match device trust crypto service implementation changes --- ...ice-trust-crypto.service.implementation.ts | 22 +++++------ .../device-trust-crypto.service.spec.ts | 39 ++++++++++--------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts b/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts index ab3880d1d0..5297fe2b21 100644 --- a/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts @@ -69,23 +69,24 @@ export class DeviceTrustCryptoService implements DeviceTrustCryptoServiceAbstrac // Send encrypted keys to server const deviceIdentifier = await this.appIdService.getAppId(); - return this.devicesApiService.updateTrustedDeviceKeys( + const deviceResponse = await this.devicesApiService.updateTrustedDeviceKeys( deviceIdentifier, devicePublicKeyEncryptedUserKey.encryptedString, userKeyEncryptedDevicePublicKey.encryptedString, deviceKeyEncryptedDevicePrivateKey.encryptedString ); + + // store device key in local/secure storage if enc keys posted to server successfully + await this.setDeviceKey(deviceKey); + return deviceResponse; } async getDeviceKey(): Promise { - // Check if device key is already stored - const existingDeviceKey = await this.stateService.getDeviceKey(); + return await this.stateService.getDeviceKey(); + } - if (existingDeviceKey != null) { - return existingDeviceKey; - } else { - return this.makeDeviceKey(); - } + private async setDeviceKey(deviceKey: DeviceKey): Promise { + await this.stateService.setDeviceKey(deviceKey); } private async makeDeviceKey(): Promise { @@ -93,9 +94,6 @@ export class DeviceTrustCryptoService implements DeviceTrustCryptoServiceAbstrac const randomBytes: CsprngArray = await this.cryptoFunctionService.randomBytes(64); const deviceKey = new SymmetricCryptoKey(randomBytes) as DeviceKey; - // Save device key in secure storage - await this.stateService.setDeviceKey(deviceKey); - return deviceKey; } @@ -106,7 +104,7 @@ export class DeviceTrustCryptoService implements DeviceTrustCryptoServiceAbstrac encryptedUserKey: any ): Promise { // get device key - const existingDeviceKey = await this.stateService.getDeviceKey(); + const existingDeviceKey = await this.getDeviceKey(); if (!existingDeviceKey) { // TODO: not sure what to do here diff --git a/libs/common/src/auth/services/device-trust-crypto.service.spec.ts b/libs/common/src/auth/services/device-trust-crypto.service.spec.ts index 685c1ee799..2b1b4be97a 100644 --- a/libs/common/src/auth/services/device-trust-crypto.service.spec.ts +++ b/libs/common/src/auth/services/device-trust-crypto.service.spec.ts @@ -87,44 +87,33 @@ describe("deviceTrustCryptoService", () => { const userKeyBytesLength = 64; describe("getDeviceKey", () => { - let mockRandomBytes: CsprngArray; - let mockDeviceKey: SymmetricCryptoKey; let existingDeviceKey: DeviceKey; let stateSvcGetDeviceKeySpy: jest.SpyInstance; - let makeDeviceKeySpy: jest.SpyInstance; beforeEach(() => { - mockRandomBytes = new Uint8Array(deviceKeyBytesLength).buffer as CsprngArray; - mockDeviceKey = new SymmetricCryptoKey(mockRandomBytes); existingDeviceKey = new SymmetricCryptoKey( new Uint8Array(deviceKeyBytesLength).buffer as CsprngArray ) as DeviceKey; stateSvcGetDeviceKeySpy = jest.spyOn(stateService, "getDeviceKey"); - makeDeviceKeySpy = jest.spyOn(deviceTrustCryptoService as any, "makeDeviceKey"); }); - it("gets a device key when there is not an existing device key", async () => { + it("returns null when there is not an existing device key", async () => { stateSvcGetDeviceKeySpy.mockResolvedValue(null); - makeDeviceKeySpy.mockResolvedValue(mockDeviceKey); const deviceKey = await deviceTrustCryptoService.getDeviceKey(); expect(stateSvcGetDeviceKeySpy).toHaveBeenCalledTimes(1); - expect(makeDeviceKeySpy).toHaveBeenCalledTimes(1); - expect(deviceKey).not.toBeNull(); - expect(deviceKey).toBeInstanceOf(SymmetricCryptoKey); - expect(deviceKey).toEqual(mockDeviceKey); + expect(deviceKey).toBeNull(); }); - it("returns the existing device key without creating a new one when there is an existing device key", async () => { + it("returns the device key when there is an existing device key", async () => { stateSvcGetDeviceKeySpy.mockResolvedValue(existingDeviceKey); const deviceKey = await deviceTrustCryptoService.getDeviceKey(); expect(stateSvcGetDeviceKeySpy).toHaveBeenCalledTimes(1); - expect(makeDeviceKeySpy).not.toHaveBeenCalled(); expect(deviceKey).not.toBeNull(); expect(deviceKey).toBeInstanceOf(SymmetricCryptoKey); @@ -132,6 +121,23 @@ describe("deviceTrustCryptoService", () => { }); }); + describe("setDeviceKey", () => { + it("sets the device key in the state service", async () => { + const stateSvcSetDeviceKeySpy = jest.spyOn(stateService, "setDeviceKey"); + + const deviceKey = new SymmetricCryptoKey( + new Uint8Array(deviceKeyBytesLength).buffer as CsprngArray + ) as DeviceKey; + + // TypeScript will allow calling private methods if the object is of type 'any' + // This is a hacky workaround, but it allows for cleaner tests + await (deviceTrustCryptoService as any).setDeviceKey(deviceKey); + + expect(stateSvcSetDeviceKeySpy).toHaveBeenCalledTimes(1); + expect(stateSvcSetDeviceKeySpy).toHaveBeenCalledWith(deviceKey); + }); + }); + describe("makeDeviceKey", () => { it("creates a new non-null 64 byte device key, securely stores it, and returns it", async () => { const mockRandomBytes = new Uint8Array(deviceKeyBytesLength).buffer as CsprngArray; @@ -140,8 +146,6 @@ describe("deviceTrustCryptoService", () => { .spyOn(cryptoFunctionService, "randomBytes") .mockResolvedValue(mockRandomBytes); - const stateSvcSetDeviceKeySpy = jest.spyOn(stateService, "setDeviceKey"); - // TypeScript will allow calling private methods if the object is of type 'any' // This is a hacky workaround, but it allows for cleaner tests const deviceKey = await (deviceTrustCryptoService as any).makeDeviceKey(); @@ -151,9 +155,6 @@ describe("deviceTrustCryptoService", () => { expect(deviceKey).not.toBeNull(); expect(deviceKey).toBeInstanceOf(SymmetricCryptoKey); - - expect(stateSvcSetDeviceKeySpy).toHaveBeenCalledTimes(1); - expect(stateSvcSetDeviceKeySpy).toHaveBeenCalledWith(deviceKey); }); });