From c02723d6a6ef1b61644f39babdeeec436632391f Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 9 Apr 2024 10:17:00 -0500 Subject: [PATCH] Specify clearOn options for platform services (#8584) * Use UserKeys in biometric state * Remove global clear todo. Answer is never * User UserKeys in crypto state * Clear userkey on both lock and logout via User Key Definitions * Use UserKeyDefinitions in environment service * Rely on userKeyDefinition to clear org keys * Rely on userKeyDefinition to clear provider keys * Rely on userKeyDefinition to clear user keys * Rely on userKeyDefinitions to clear user asym key pair --- libs/common/spec/fake-account-service.ts | 8 +- .../platform/abstractions/crypto.service.ts | 26 +- .../biometrics/biometric.state.spec.ts | 12 +- .../platform/biometrics/biometric.state.ts | 17 +- .../services/config/default-config.service.ts | 1 - .../platform/services/crypto.service.spec.ts | 263 +++--------------- .../src/platform/services/crypto.service.ts | 101 +++---- .../default-environment.service.spec.ts | 7 +- .../services/default-environment.service.ts | 49 +++- .../services/key-state/org-keys.state.ts | 5 +- .../services/key-state/provider-keys.state.ts | 5 +- .../services/key-state/user-key.state.ts | 14 +- .../src/platform/state/derive-definition.ts | 22 +- .../vault-timeout.service.spec.ts | 1 - .../vault-timeout/vault-timeout.service.ts | 3 - 15 files changed, 169 insertions(+), 365 deletions(-) diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index 2f33d9cf02..bd35d901c2 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -32,12 +32,8 @@ export class FakeAccountService implements AccountService { get activeUserId() { return this._activeUserId; } - get accounts$() { - return this.accountsSubject.asObservable(); - } - get activeAccount$() { - return this.activeAccountSubject.asObservable(); - } + accounts$ = this.accountsSubject.asObservable(); + activeAccount$ = this.activeAccountSubject.asObservable(); accountLock$: Observable; accountLogout$: Observable; diff --git a/libs/common/src/platform/abstractions/crypto.service.ts b/libs/common/src/platform/abstractions/crypto.service.ts index 85b2bfe82e..ed451fd896 100644 --- a/libs/common/src/platform/abstractions/crypto.service.ts +++ b/libs/common/src/platform/abstractions/crypto.service.ts @@ -26,7 +26,7 @@ export abstract class CryptoService { * any other necessary versions (such as auto, biometrics, * or pin) * - * @throws when key is null. Use {@link clearUserKey} instead + * @throws when key is null. Lock the account to clear a key * @param key The user key to set * @param userId The desired user */ @@ -93,13 +93,6 @@ export abstract class CryptoService { * @returns A new user key and the master key protected version of it */ abstract makeUserKey(key: MasterKey): Promise<[UserKey, EncString]>; - /** - * Clears the user key - * @param clearStoredKeys Clears all stored versions of the user keys as well, - * such as the biometrics key - * @param userId The desired user - */ - abstract clearUserKey(clearSecretStorage?: boolean, userId?: string): Promise; /** * Clears the user's stored version of the user key * @param keySuffix The desired version of the key to clear @@ -238,12 +231,6 @@ export abstract class CryptoService { abstract makeDataEncKey( key: T, ): Promise<[SymmetricCryptoKey, EncString]>; - /** - * Clears the user's stored organization keys - * @param memoryOnly Clear only the in-memory keys - * @param userId The desired user - */ - abstract clearOrgKeys(memoryOnly?: boolean, userId?: string): Promise; /** * Stores the encrypted provider keys and clears any decrypted * provider keys currently in memory @@ -260,11 +247,6 @@ export abstract class CryptoService { * @returns A record of the provider Ids to their symmetric keys */ abstract getProviderKeys(): Promise>; - /** - * @param memoryOnly Clear only the in-memory keys - * @param userId The desired user - */ - abstract clearProviderKeys(memoryOnly?: boolean, userId?: string): Promise; /** * Returns the public key from memory. If not available, extracts it * from the private key and stores it in memory @@ -304,12 +286,6 @@ export abstract class CryptoService { * @returns A new keypair: [publicKey in Base64, encrypted privateKey] */ abstract makeKeyPair(key?: SymmetricCryptoKey): Promise<[string, EncString]>; - /** - * Clears the user's key pair - * @param memoryOnly Clear only the in-memory keys - * @param userId The desired user - */ - abstract clearKeyPair(memoryOnly?: boolean, userId?: string): Promise; /** * @param pin The user's pin * @param salt The user's salt diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts index 420a0fb86e..7bcccd2ea9 100644 --- a/libs/common/src/platform/biometrics/biometric.state.spec.ts +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -1,5 +1,5 @@ import { EncryptedString } from "../models/domain/enc-string"; -import { KeyDefinition } from "../state"; +import { KeyDefinition, UserKeyDefinition } from "../state"; import { BIOMETRIC_UNLOCK_ENABLED, @@ -22,9 +22,15 @@ describe.each([ ])( "deserializes state %s", ( - ...args: [KeyDefinition, EncryptedString] | [KeyDefinition, boolean] + ...args: + | [UserKeyDefinition, EncryptedString] + | [UserKeyDefinition, boolean] + | [KeyDefinition, boolean] ) => { - function testDeserialization(keyDefinition: KeyDefinition, state: T) { + function testDeserialization( + keyDefinition: UserKeyDefinition | KeyDefinition, + state: T, + ) { const deserialized = keyDefinition.deserializer(JSON.parse(JSON.stringify(state))); expect(deserialized).toEqual(state); } diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts index aa16e14baa..bcefb7b215 100644 --- a/libs/common/src/platform/biometrics/biometric.state.ts +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -1,15 +1,16 @@ import { UserId } from "../../types/guid"; import { EncryptedString } from "../models/domain/enc-string"; -import { KeyDefinition, BIOMETRIC_SETTINGS_DISK } from "../state"; +import { KeyDefinition, BIOMETRIC_SETTINGS_DISK, UserKeyDefinition } from "../state"; /** * Indicates whether the user elected to store a biometric key to unlock their vault. */ -export const BIOMETRIC_UNLOCK_ENABLED = new KeyDefinition( +export const BIOMETRIC_UNLOCK_ENABLED = new UserKeyDefinition( BIOMETRIC_SETTINGS_DISK, "biometricUnlockEnabled", { deserializer: (obj) => obj, + clearOn: [], }, ); @@ -18,11 +19,12 @@ export const BIOMETRIC_UNLOCK_ENABLED = new KeyDefinition( * * A true setting controls whether {@link ENCRYPTED_CLIENT_KEY_HALF} is set. */ -export const REQUIRE_PASSWORD_ON_START = new KeyDefinition( +export const REQUIRE_PASSWORD_ON_START = new UserKeyDefinition( BIOMETRIC_SETTINGS_DISK, "requirePasswordOnStart", { deserializer: (value) => value, + clearOn: [], }, ); @@ -33,11 +35,12 @@ export const REQUIRE_PASSWORD_ON_START = new KeyDefinition( * For operating systems without application-level key storage, this key half is concatenated with a signature * provided by the OS and used to encrypt the biometric key prior to storage. */ -export const ENCRYPTED_CLIENT_KEY_HALF = new KeyDefinition( +export const ENCRYPTED_CLIENT_KEY_HALF = new UserKeyDefinition( BIOMETRIC_SETTINGS_DISK, "clientKeyHalf", { deserializer: (obj) => obj, + clearOn: ["logout"], }, ); @@ -45,11 +48,12 @@ export const ENCRYPTED_CLIENT_KEY_HALF = new KeyDefinition( * Indicates the user has been warned about the security implications of using biometrics and, depending on the OS, * recommended to require a password on first unlock of an application instance. */ -export const DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT = new KeyDefinition( +export const DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT = new UserKeyDefinition( BIOMETRIC_SETTINGS_DISK, "dismissedBiometricRequirePasswordOnStartCallout", { deserializer: (obj) => obj, + clearOn: [], }, ); @@ -68,11 +72,12 @@ export const PROMPT_CANCELLED = KeyDefinition.record( /** * Stores whether the user has elected to automatically prompt for biometric unlock on application start. */ -export const PROMPT_AUTOMATICALLY = new KeyDefinition( +export const PROMPT_AUTOMATICALLY = new UserKeyDefinition( BIOMETRIC_SETTINGS_DISK, "promptAutomatically", { deserializer: (obj) => obj, + clearOn: [], }, ); diff --git a/libs/common/src/platform/services/config/default-config.service.ts b/libs/common/src/platform/services/config/default-config.service.ts index 9532b903d3..e124deccf8 100644 --- a/libs/common/src/platform/services/config/default-config.service.ts +++ b/libs/common/src/platform/services/config/default-config.service.ts @@ -32,7 +32,6 @@ export const USER_SERVER_CONFIG = new UserKeyDefinition(CONFIG_DIS clearOn: ["logout"], }); -// TODO MDG: When to clean these up? export const GLOBAL_SERVER_CONFIGURATIONS = KeyDefinition.record( CONFIG_DISK, "byServer", diff --git a/libs/common/src/platform/services/crypto.service.spec.ts b/libs/common/src/platform/services/crypto.service.spec.ts index 9160664aa5..c17d3f97d2 100644 --- a/libs/common/src/platform/services/crypto.service.spec.ts +++ b/libs/common/src/platform/services/crypto.service.spec.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { firstValueFrom, of } from "rxjs"; +import { firstValueFrom, of, tap } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { FakeActiveUserState, FakeSingleUserState } from "../../../spec/fake-state"; @@ -18,6 +18,7 @@ import { Utils } from "../misc/utils"; import { EncString } from "../models/domain/enc-string"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; import { CryptoService } from "../services/crypto.service"; +import { UserKeyDefinition } from "../state"; import { USER_ENCRYPTED_ORGANIZATION_KEYS } from "./key-state/org-keys.state"; import { USER_ENCRYPTED_PROVIDER_KEYS } from "./key-state/provider-keys.state"; @@ -336,231 +337,22 @@ describe("cryptoService", () => { }); }); - describe("clearUserKey", () => { - it.each([mockUserId, null])("should clear the User Key for id %2", async (userId) => { - await cryptoService.clearUserKey(false, userId); - - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(USER_KEY, null, userId); - }); - - it("should update status to locked", async () => { - await cryptoService.clearUserKey(false, mockUserId); - - expect(accountService.mock.setMaxAccountStatus).toHaveBeenCalledWith( - mockUserId, - AuthenticationStatus.Locked, + describe("clearKeys", () => { + it("resolves active user id when called with no user id", async () => { + let callCount = 0; + accountService.activeAccount$ = accountService.activeAccountSubject.pipe( + tap(() => callCount++), ); - }); - it.each([true, false])( - "should clear stored user keys if clearAll is true (%s)", - async (clear) => { - const clearSpy = (cryptoService["clearAllStoredUserKeys"] = jest.fn()); - await cryptoService.clearUserKey(clear, mockUserId); + await cryptoService.clearKeys(null); + expect(callCount).toBe(1); - if (clear) { - expect(clearSpy).toHaveBeenCalledWith(mockUserId); - expect(clearSpy).toHaveBeenCalledTimes(1); - } else { - expect(clearSpy).not.toHaveBeenCalled(); - } - }, - ); - }); - - describe("clearOrgKeys", () => { - let forceMemorySpy: jest.Mock; - beforeEach(() => { - forceMemorySpy = cryptoService["activeUserOrgKeysState"].forceValue = jest.fn(); - }); - it("clears in memory org keys when called with memoryOnly", async () => { - await cryptoService.clearOrgKeys(true); - - expect(forceMemorySpy).toHaveBeenCalledWith({}); - }); - - it("does not clear memory when called with the non active user and memory only", async () => { - await cryptoService.clearOrgKeys(true, "someOtherUser" as UserId); - - expect(forceMemorySpy).not.toHaveBeenCalled(); - }); - - it("does not write to disk state if called with memory only", async () => { - await cryptoService.clearOrgKeys(true); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalled(); - }); - - it("clears disk state when called with diskOnly", async () => { - await cryptoService.clearOrgKeys(false); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_ORGANIZATION_KEYS, - ); - expect( - stateProvider.singleUser.getFake(mockUserId, USER_ENCRYPTED_ORGANIZATION_KEYS).nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("clears another user's disk state when called with diskOnly and that user", async () => { - await cryptoService.clearOrgKeys(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - "someOtherUser" as UserId, - USER_ENCRYPTED_ORGANIZATION_KEYS, - ); - expect( - stateProvider.singleUser.getFake( - "someOtherUser" as UserId, - USER_ENCRYPTED_ORGANIZATION_KEYS, - ).nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("does not clear active user disk state when called with diskOnly and a different specified user", async () => { - await cryptoService.clearOrgKeys(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_ORGANIZATION_KEYS, - ); - }); - }); - - describe("clearProviderKeys", () => { - let forceMemorySpy: jest.Mock; - beforeEach(() => { - forceMemorySpy = cryptoService["activeUserProviderKeysState"].forceValue = jest.fn(); - }); - it("clears in memory org keys when called with memoryOnly", async () => { - await cryptoService.clearProviderKeys(true); - - expect(forceMemorySpy).toHaveBeenCalledWith({}); - }); - - it("does not clear memory when called with the non active user and memory only", async () => { - await cryptoService.clearProviderKeys(true, "someOtherUser" as UserId); - - expect(forceMemorySpy).not.toHaveBeenCalled(); - }); - - it("does not write to disk state if called with memory only", async () => { - await cryptoService.clearProviderKeys(true); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalled(); - }); - - it("clears disk state when called with diskOnly", async () => { - await cryptoService.clearProviderKeys(false); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_PROVIDER_KEYS, - ); - expect( - stateProvider.singleUser.getFake(mockUserId, USER_ENCRYPTED_PROVIDER_KEYS).nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("clears another user's disk state when called with diskOnly and that user", async () => { - await cryptoService.clearProviderKeys(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - "someOtherUser" as UserId, - USER_ENCRYPTED_PROVIDER_KEYS, - ); - expect( - stateProvider.singleUser.getFake("someOtherUser" as UserId, USER_ENCRYPTED_PROVIDER_KEYS) - .nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("does not clear active user disk state when called with diskOnly and a different specified user", async () => { - await cryptoService.clearProviderKeys(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_PROVIDER_KEYS, - ); - }); - }); - - describe("clearKeyPair", () => { - let forceMemoryPrivateKeySpy: jest.Mock; - let forceMemoryPublicKeySpy: jest.Mock; - beforeEach(() => { - forceMemoryPrivateKeySpy = cryptoService["activeUserPrivateKeyState"].forceValue = jest.fn(); - forceMemoryPublicKeySpy = cryptoService["activeUserPublicKeyState"].forceValue = jest.fn(); - }); - it("clears in memory org keys when called with memoryOnly", async () => { - await cryptoService.clearKeyPair(true); - - expect(forceMemoryPrivateKeySpy).toHaveBeenCalledWith(null); - expect(forceMemoryPublicKeySpy).toHaveBeenCalledWith(null); - }); - - it("does not clear memory when called with the non active user and memory only", async () => { - await cryptoService.clearKeyPair(true, "someOtherUser" as UserId); - - expect(forceMemoryPrivateKeySpy).not.toHaveBeenCalled(); - expect(forceMemoryPublicKeySpy).not.toHaveBeenCalled(); - }); - - it("does not write to disk state if called with memory only", async () => { - await cryptoService.clearKeyPair(true); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalled(); - }); - - it("clears disk state when called with diskOnly", async () => { - await cryptoService.clearKeyPair(false); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_PRIVATE_KEY, - ); - expect( - stateProvider.singleUser.getFake(mockUserId, USER_ENCRYPTED_PRIVATE_KEY).nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("clears another user's disk state when called with diskOnly and that user", async () => { - await cryptoService.clearKeyPair(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).toHaveBeenCalledWith( - "someOtherUser" as UserId, - USER_ENCRYPTED_PRIVATE_KEY, - ); - expect( - stateProvider.singleUser.getFake("someOtherUser" as UserId, USER_ENCRYPTED_PRIVATE_KEY) - .nextMock, - ).toHaveBeenCalledWith(null); - }); - - it("does not clear active user disk state when called with diskOnly and a different specified user", async () => { - await cryptoService.clearKeyPair(false, "someOtherUser" as UserId); - - expect(stateProvider.singleUser.mock.get).not.toHaveBeenCalledWith( - mockUserId, - USER_ENCRYPTED_PRIVATE_KEY, - ); - }); - }); - - describe("clearUserKey", () => { - it("clears the user key for the active user when no userId is specified", async () => { - await cryptoService.clearUserKey(false); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(USER_KEY, null, undefined); - }); - - it("clears the user key for the specified user when a userId is specified", async () => { - await cryptoService.clearUserKey(false, "someOtherUser" as UserId); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith(USER_KEY, null, "someOtherUser"); + // revert to the original state + accountService.activeAccount$ = accountService.activeAccountSubject.asObservable(); }); it("sets the maximum account status of the active user id to locked when user id is not specified", async () => { - await cryptoService.clearUserKey(false); + await cryptoService.clearKeys(); expect(accountService.mock.setMaxAccountStatus).toHaveBeenCalledWith( mockUserId, AuthenticationStatus.Locked, @@ -568,17 +360,36 @@ describe("cryptoService", () => { }); it("sets the maximum account status of the specified user id to locked when user id is specified", async () => { - await cryptoService.clearUserKey(false, "someOtherUser" as UserId); + const userId = "someOtherUser" as UserId; + await cryptoService.clearKeys(userId); expect(accountService.mock.setMaxAccountStatus).toHaveBeenCalledWith( - "someOtherUser" as UserId, + userId, AuthenticationStatus.Locked, ); }); - it("clears all stored user keys when clearAll is true", async () => { - const clearAllSpy = (cryptoService["clearAllStoredUserKeys"] = jest.fn()); - await cryptoService.clearUserKey(true); - expect(clearAllSpy).toHaveBeenCalledWith(mockUserId); + describe.each([ + USER_ENCRYPTED_ORGANIZATION_KEYS, + USER_ENCRYPTED_PROVIDER_KEYS, + USER_ENCRYPTED_PRIVATE_KEY, + USER_KEY, + ])("key removal", (key: UserKeyDefinition) => { + it(`clears ${key.key} for active user when unspecified`, async () => { + await cryptoService.clearKeys(null); + + const encryptedOrgKeyState = stateProvider.singleUser.getFake(mockUserId, key); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledTimes(1); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledWith(null); + }); + + it(`clears ${key.key} for the specified user when specified`, async () => { + const userId = "someOtherUser" as UserId; + await cryptoService.clearKeys(userId); + + const encryptedOrgKeyState = stateProvider.singleUser.getFake(userId, key); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledTimes(1); + expect(encryptedOrgKeyState.nextMock).toHaveBeenCalledWith(null); + }); }); }); }); diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index dd3c497470..df7528b13c 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -144,7 +144,7 @@ export class CryptoService implements CryptoServiceAbstraction { async setUserKey(key: UserKey, userId?: UserId): Promise { if (key == null) { - throw new Error("No key provided. Use ClearUserKey to clear the key"); + throw new Error("No key provided. Lock the user to clear the key"); } // Set userId to ensure we have one for the account status update [userId, key] = await this.stateProvider.setUserState(USER_KEY, key, userId); @@ -242,13 +242,19 @@ export class CryptoService implements CryptoServiceAbstraction { return this.buildProtectedSymmetricKey(masterKey, newUserKey.key); } - async clearUserKey(clearStoredKeys = true, userId?: UserId): Promise { - // Set userId to ensure we have one for the account status update - [userId] = await this.stateProvider.setUserState(USER_KEY, null, userId); - await this.accountService.setMaxAccountStatus(userId, AuthenticationStatus.Locked); - if (clearStoredKeys) { - await this.clearAllStoredUserKeys(userId); + /** + * Clears the user key. Clears all stored versions of the user keys as well, such as the biometrics key + * @param userId The desired user + */ + async clearUserKey(userId: UserId): Promise { + if (userId == null) { + // nothing to do + return; } + // Set userId to ensure we have one for the account status update + await this.stateProvider.setUserState(USER_KEY, null, userId); + await this.accountService.setMaxAccountStatus(userId, AuthenticationStatus.Locked); + await this.clearAllStoredUserKeys(userId); } async clearStoredUserKey(keySuffix: KeySuffixOptions, userId?: UserId): Promise { @@ -480,25 +486,12 @@ export class CryptoService implements CryptoServiceAbstraction { return this.buildProtectedSymmetricKey(key, newSymKey.key); } - async clearOrgKeys(memoryOnly?: boolean, userId?: UserId): Promise { - const activeUserId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const userIdIsActive = userId == null || userId === activeUserId; - - if (!memoryOnly) { - if (userId == null && activeUserId == null) { - // nothing to do - return; - } - await this.stateProvider - .getUser(userId ?? activeUserId, USER_ENCRYPTED_ORGANIZATION_KEYS) - .update(() => null); + private async clearOrgKeys(userId: UserId): Promise { + if (userId == null) { + // nothing to do return; } - - // org keys are only cached for active users - if (userIdIsActive) { - await this.activeUserOrgKeysState.forceValue({}); - } + await this.stateProvider.setUserState(USER_ENCRYPTED_ORGANIZATION_KEYS, null, userId); } async setProviderKeys(providers: ProfileProviderResponse[]): Promise { @@ -526,25 +519,12 @@ export class CryptoService implements CryptoServiceAbstraction { return await firstValueFrom(this.activeUserProviderKeys$); } - async clearProviderKeys(memoryOnly?: boolean, userId?: UserId): Promise { - const activeUserId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const userIdIsActive = userId == null || userId === activeUserId; - - if (!memoryOnly) { - if (userId == null && activeUserId == null) { - // nothing to do - return; - } - await this.stateProvider - .getUser(userId ?? activeUserId, USER_ENCRYPTED_PROVIDER_KEYS) - .update(() => null); + private async clearProviderKeys(userId: UserId): Promise { + if (userId == null) { + // nothing to do return; } - - // provider keys are only cached for active users - if (userIdIsActive) { - await this.activeUserProviderKeysState.forceValue({}); - } + await this.stateProvider.setUserState(USER_ENCRYPTED_PROVIDER_KEYS, null, userId); } async getPublicKey(): Promise { @@ -597,26 +577,17 @@ export class CryptoService implements CryptoServiceAbstraction { return [publicB64, privateEnc]; } - async clearKeyPair(memoryOnly?: boolean, userId?: UserId): Promise { - const activeUserId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const userIdIsActive = userId == null || userId === activeUserId; - - if (!memoryOnly) { - if (userId == null && activeUserId == null) { - // nothing to do - return; - } - await this.stateProvider - .getUser(userId ?? activeUserId, USER_ENCRYPTED_PRIVATE_KEY) - .update(() => null); + /** + * Clears the user's key pair + * @param userId The desired user + */ + private async clearKeyPair(userId: UserId): Promise { + if (userId == null) { + // nothing to do return; } - // decrypted key pair is only cached for active users - if (userIdIsActive) { - await this.activeUserPrivateKeyState.forceValue(null); - await this.activeUserPublicKeyState.forceValue(null); - } + await this.stateProvider.setUserState(USER_ENCRYPTED_PRIVATE_KEY, null, userId); } async makePinKey(pin: string, salt: string, kdf: KdfType, kdfConfig: KdfConfig): Promise { @@ -681,11 +652,17 @@ export class CryptoService implements CryptoServiceAbstraction { } async clearKeys(userId?: UserId): Promise { - await this.clearUserKey(true, userId); + userId ||= (await firstValueFrom(this.accountService.activeAccount$))?.id; + + if (userId == null) { + throw new Error("Cannot clear keys, no user Id resolved."); + } + + await this.clearUserKey(userId); await this.clearMasterKeyHash(userId); - await this.clearOrgKeys(false, userId); - await this.clearProviderKeys(false, userId); - await this.clearKeyPair(false, userId); + await this.clearOrgKeys(userId); + await this.clearProviderKeys(userId); + await this.clearKeyPair(userId); await this.clearPinKeys(userId); await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, null, userId); } diff --git a/libs/common/src/platform/services/default-environment.service.spec.ts b/libs/common/src/platform/services/default-environment.service.spec.ts index 66bc1acfda..a70ab3d179 100644 --- a/libs/common/src/platform/services/default-environment.service.spec.ts +++ b/libs/common/src/platform/services/default-environment.service.spec.ts @@ -7,9 +7,10 @@ import { UserId } from "../../types/guid"; import { CloudRegion, Region } from "../abstractions/environment.service"; import { - ENVIRONMENT_KEY, + GLOBAL_ENVIRONMENT_KEY, DefaultEnvironmentService, EnvironmentUrls, + USER_ENVIRONMENT_KEY, } from "./default-environment.service"; // There are a few main states EnvironmentService could be in when first used @@ -55,7 +56,7 @@ describe("EnvironmentService", () => { }; const setGlobalData = (region: Region, environmentUrls: EnvironmentUrls) => { - stateProvider.global.getFake(ENVIRONMENT_KEY).stateSubject.next({ + stateProvider.global.getFake(GLOBAL_ENVIRONMENT_KEY).stateSubject.next({ region: region, urls: environmentUrls, }); @@ -66,7 +67,7 @@ describe("EnvironmentService", () => { environmentUrls: EnvironmentUrls, userId: UserId = testUser, ) => { - stateProvider.singleUser.getFake(userId, ENVIRONMENT_KEY).nextState({ + stateProvider.singleUser.getFake(userId, USER_ENVIRONMENT_KEY).nextState({ region: region, urls: environmentUrls, }); diff --git a/libs/common/src/platform/services/default-environment.service.ts b/libs/common/src/platform/services/default-environment.service.ts index d074ff43f8..59956ede7a 100644 --- a/libs/common/src/platform/services/default-environment.service.ts +++ b/libs/common/src/platform/services/default-environment.service.ts @@ -18,6 +18,7 @@ import { GlobalState, KeyDefinition, StateProvider, + UserKeyDefinition, } from "../state"; export class EnvironmentUrls { @@ -40,7 +41,7 @@ class EnvironmentState { } } -export const ENVIRONMENT_KEY = new KeyDefinition( +export const GLOBAL_ENVIRONMENT_KEY = new KeyDefinition( ENVIRONMENT_DISK, "environment", { @@ -48,9 +49,31 @@ export const ENVIRONMENT_KEY = new KeyDefinition( }, ); -export const CLOUD_REGION_KEY = new KeyDefinition(ENVIRONMENT_MEMORY, "cloudRegion", { - deserializer: (b) => b, -}); +export const USER_ENVIRONMENT_KEY = new UserKeyDefinition( + ENVIRONMENT_DISK, + "environment", + { + deserializer: EnvironmentState.fromJSON, + clearOn: ["logout"], + }, +); + +export const GLOBAL_CLOUD_REGION_KEY = new KeyDefinition( + ENVIRONMENT_MEMORY, + "cloudRegion", + { + deserializer: (b) => b, + }, +); + +export const USER_CLOUD_REGION_KEY = new UserKeyDefinition( + ENVIRONMENT_MEMORY, + "cloudRegion", + { + deserializer: (b) => b, + clearOn: ["logout"], + }, +); /** * The production regions available for selection. @@ -114,8 +137,8 @@ export class DefaultEnvironmentService implements EnvironmentService { private stateProvider: StateProvider, private accountService: AccountService, ) { - this.globalState = this.stateProvider.getGlobal(ENVIRONMENT_KEY); - this.globalCloudRegionState = this.stateProvider.getGlobal(CLOUD_REGION_KEY); + this.globalState = this.stateProvider.getGlobal(GLOBAL_ENVIRONMENT_KEY); + this.globalCloudRegionState = this.stateProvider.getGlobal(GLOBAL_CLOUD_REGION_KEY); const account$ = this.activeAccountId$.pipe( // Use == here to not trigger on undefined -> null transition @@ -125,8 +148,8 @@ export class DefaultEnvironmentService implements EnvironmentService { this.environment$ = account$.pipe( switchMap((userId) => { const t = userId - ? this.stateProvider.getUser(userId, ENVIRONMENT_KEY).state$ - : this.stateProvider.getGlobal(ENVIRONMENT_KEY).state$; + ? this.stateProvider.getUser(userId, USER_ENVIRONMENT_KEY).state$ + : this.stateProvider.getGlobal(GLOBAL_ENVIRONMENT_KEY).state$; return t; }), map((state) => { @@ -136,8 +159,8 @@ export class DefaultEnvironmentService implements EnvironmentService { this.cloudWebVaultUrl$ = account$.pipe( switchMap((userId) => { const t = userId - ? this.stateProvider.getUser(userId, CLOUD_REGION_KEY).state$ - : this.stateProvider.getGlobal(CLOUD_REGION_KEY).state$; + ? this.stateProvider.getUser(userId, USER_CLOUD_REGION_KEY).state$ + : this.stateProvider.getGlobal(GLOBAL_CLOUD_REGION_KEY).state$; return t; }), map((region) => { @@ -242,7 +265,7 @@ export class DefaultEnvironmentService implements EnvironmentService { if (userId == null) { await this.globalCloudRegionState.update(() => region); } else { - await this.stateProvider.getUser(userId, CLOUD_REGION_KEY).update(() => region); + await this.stateProvider.getUser(userId, USER_CLOUD_REGION_KEY).update(() => region); } } @@ -261,13 +284,13 @@ export class DefaultEnvironmentService implements EnvironmentService { return activeUserId == null ? await firstValueFrom(this.globalState.state$) : await firstValueFrom( - this.stateProvider.getUser(userId ?? activeUserId, ENVIRONMENT_KEY).state$, + this.stateProvider.getUser(userId ?? activeUserId, USER_ENVIRONMENT_KEY).state$, ); } async seedUserEnvironment(userId: UserId) { const global = await firstValueFrom(this.globalState.state$); - await this.stateProvider.getUser(userId, ENVIRONMENT_KEY).update(() => global); + await this.stateProvider.getUser(userId, USER_ENVIRONMENT_KEY).update(() => global); } } diff --git a/libs/common/src/platform/services/key-state/org-keys.state.ts b/libs/common/src/platform/services/key-state/org-keys.state.ts index b39cc9a82a..f67e64b653 100644 --- a/libs/common/src/platform/services/key-state/org-keys.state.ts +++ b/libs/common/src/platform/services/key-state/org-keys.state.ts @@ -4,13 +4,14 @@ import { OrganizationId } from "../../../types/guid"; import { OrgKey } from "../../../types/key"; import { CryptoService } from "../../abstractions/crypto.service"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; -import { KeyDefinition, CRYPTO_DISK, DeriveDefinition } from "../../state"; +import { CRYPTO_DISK, DeriveDefinition, UserKeyDefinition } from "../../state"; -export const USER_ENCRYPTED_ORGANIZATION_KEYS = KeyDefinition.record< +export const USER_ENCRYPTED_ORGANIZATION_KEYS = UserKeyDefinition.record< EncryptedOrganizationKeyData, OrganizationId >(CRYPTO_DISK, "organizationKeys", { deserializer: (obj) => obj, + clearOn: ["logout"], }); export const USER_ORGANIZATION_KEYS = DeriveDefinition.from< diff --git a/libs/common/src/platform/services/key-state/provider-keys.state.ts b/libs/common/src/platform/services/key-state/provider-keys.state.ts index c89df34c80..776fdc77d8 100644 --- a/libs/common/src/platform/services/key-state/provider-keys.state.ts +++ b/libs/common/src/platform/services/key-state/provider-keys.state.ts @@ -3,14 +3,15 @@ import { ProviderKey } from "../../../types/key"; import { EncryptService } from "../../abstractions/encrypt.service"; import { EncString, EncryptedString } from "../../models/domain/enc-string"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; -import { KeyDefinition, CRYPTO_DISK, DeriveDefinition } from "../../state"; +import { CRYPTO_DISK, DeriveDefinition, UserKeyDefinition } from "../../state"; import { CryptoService } from "../crypto.service"; -export const USER_ENCRYPTED_PROVIDER_KEYS = KeyDefinition.record( +export const USER_ENCRYPTED_PROVIDER_KEYS = UserKeyDefinition.record( CRYPTO_DISK, "providerKeys", { deserializer: (obj) => obj, + clearOn: ["logout"], }, ); diff --git a/libs/common/src/platform/services/key-state/user-key.state.ts b/libs/common/src/platform/services/key-state/user-key.state.ts index d0f54c9add..abb26b92da 100644 --- a/libs/common/src/platform/services/key-state/user-key.state.ts +++ b/libs/common/src/platform/services/key-state/user-key.state.ts @@ -3,18 +3,25 @@ import { CryptoFunctionService } from "../../abstractions/crypto-function.servic import { EncryptService } from "../../abstractions/encrypt.service"; import { EncString, EncryptedString } from "../../models/domain/enc-string"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; -import { KeyDefinition, CRYPTO_DISK, DeriveDefinition, CRYPTO_MEMORY } from "../../state"; +import { + KeyDefinition, + CRYPTO_DISK, + DeriveDefinition, + CRYPTO_MEMORY, + UserKeyDefinition, +} from "../../state"; import { CryptoService } from "../crypto.service"; export const USER_EVER_HAD_USER_KEY = new KeyDefinition(CRYPTO_DISK, "everHadUserKey", { deserializer: (obj) => obj, }); -export const USER_ENCRYPTED_PRIVATE_KEY = new KeyDefinition( +export const USER_ENCRYPTED_PRIVATE_KEY = new UserKeyDefinition( CRYPTO_DISK, "privateKey", { deserializer: (obj) => obj, + clearOn: ["logout"], }, ); @@ -58,6 +65,7 @@ export const USER_PUBLIC_KEY = DeriveDefinition.from< return (await cryptoFunctionService.rsaExtractPublicKey(privateKey)) as UserPublicKey; }, }); -export const USER_KEY = new KeyDefinition(CRYPTO_MEMORY, "userKey", { +export const USER_KEY = new UserKeyDefinition(CRYPTO_MEMORY, "userKey", { deserializer: (obj) => SymmetricCryptoKey.fromJSON(obj) as UserKey, + clearOn: ["logout", "lock"], }); diff --git a/libs/common/src/platform/state/derive-definition.ts b/libs/common/src/platform/state/derive-definition.ts index 6c514f8869..8f62d3a342 100644 --- a/libs/common/src/platform/state/derive-definition.ts +++ b/libs/common/src/platform/state/derive-definition.ts @@ -5,6 +5,7 @@ import { DerivedStateDependencies, StorageKey } from "../../types/state"; import { KeyDefinition } from "./key-definition"; import { StateDefinition } from "./state-definition"; +import { UserKeyDefinition } from "./user-key-definition"; declare const depShapeMarker: unique symbol; /** @@ -129,26 +130,28 @@ export class DeriveDefinition( definition: | KeyDefinition + | UserKeyDefinition | [DeriveDefinition, string], options: DeriveDefinitionOptions, ) { - if (isKeyDefinition(definition)) { - return new DeriveDefinition(definition.stateDefinition, definition.key, options); - } else { + if (isFromDeriveDefinition(definition)) { return new DeriveDefinition(definition[0].stateDefinition, definition[1], options); + } else { + return new DeriveDefinition(definition.stateDefinition, definition.key, options); } } static fromWithUserId( definition: | KeyDefinition + | UserKeyDefinition | [DeriveDefinition, string], options: DeriveDefinitionOptions<[UserId, TKeyDef], TTo, TDeps>, ) { - if (isKeyDefinition(definition)) { - return new DeriveDefinition(definition.stateDefinition, definition.key, options); - } else { + if (isFromDeriveDefinition(definition)) { return new DeriveDefinition(definition[0].stateDefinition, definition[1], options); + } else { + return new DeriveDefinition(definition.stateDefinition, definition.key, options); } } @@ -181,10 +184,11 @@ export class DeriveDefinition + | UserKeyDefinition | [DeriveDefinition, string], -): definition is KeyDefinition { - return Object.prototype.hasOwnProperty.call(definition, "key"); +): definition is [DeriveDefinition, string] { + return Array.isArray(definition); } diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts index e48f2fe0a3..f20bc910c0 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts @@ -156,7 +156,6 @@ describe("VaultTimeoutService", () => { expect(vaultTimeoutSettingsService.availableVaultTimeoutActions$).toHaveBeenCalledWith(userId); expect(stateService.setEverBeenUnlocked).toHaveBeenCalledWith(true, { userId: userId }); expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, { userId: userId }); - expect(cryptoService.clearUserKey).toHaveBeenCalledWith(false, userId); expect(cryptoService.clearMasterKey).toHaveBeenCalledWith(userId); expect(cipherService.clearCache).toHaveBeenCalledWith(userId); expect(lockedCallback).toHaveBeenCalledWith(userId); diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.ts index c3270ac2b8..22d658c552 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -96,10 +96,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId }); - await this.cryptoService.clearUserKey(false, userId); await this.cryptoService.clearMasterKey(userId); - await this.cryptoService.clearOrgKeys(true, userId); - await this.cryptoService.clearKeyPair(true, userId); await this.cipherService.clearCache(userId);