diff --git a/apps/browser/jest.config.js b/apps/browser/jest.config.js index cde02cd995..73f5ada287 100644 --- a/apps/browser/jest.config.js +++ b/apps/browser/jest.config.js @@ -9,7 +9,10 @@ module.exports = { ...sharedConfig, preset: "jest-preset-angular", setupFilesAfterEnv: ["/test.setup.ts"], - moduleNameMapper: pathsToModuleNameMapper(compilerOptions?.paths || {}, { - prefix: "/", - }), + moduleNameMapper: pathsToModuleNameMapper( + { "@bitwarden/common/spec": ["../../libs/common/spec"], ...(compilerOptions?.paths ?? {}) }, + { + prefix: "/", + }, + ), }; diff --git a/apps/browser/src/platform/services/browser-crypto.service.ts b/apps/browser/src/platform/services/browser-crypto.service.ts index 36ee4c6717..50cf5a7d75 100644 --- a/apps/browser/src/platform/services/browser-crypto.service.ts +++ b/apps/browser/src/platform/services/browser-crypto.service.ts @@ -1,7 +1,8 @@ +import { firstValueFrom } from "rxjs"; + import { KeySuffixOptions } from "@bitwarden/common/platform/enums"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CryptoService } from "@bitwarden/common/platform/services/crypto.service"; +import { USER_KEY } from "@bitwarden/common/platform/services/key-state/user-key.state"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; @@ -29,9 +30,9 @@ export class BrowserCryptoService extends CryptoService { return null; } - const userKey = await this.stateService.getUserKey({ userId: userId }); + const userKey = await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId)); if (userKey) { - return new SymmetricCryptoKey(Utils.fromB64ToArray(userKey.keyB64)) as UserKey; + return userKey; } } diff --git a/apps/browser/src/platform/services/browser-state.service.spec.ts b/apps/browser/src/platform/services/browser-state.service.spec.ts index 0cb7e4fd44..b5d1b9c38a 100644 --- a/apps/browser/src/platform/services/browser-state.service.spec.ts +++ b/apps/browser/src/platform/services/browser-state.service.spec.ts @@ -1,6 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { @@ -11,8 +10,10 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { State } from "@bitwarden/common/platform/models/domain/state"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; +import { mockAccountServiceWith } from "@bitwarden/common/spec"; import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; +import { UserId } from "@bitwarden/common/types/guid"; import { Account } from "../../models/account"; import { BrowserComponentState } from "../../models/browserComponentState"; @@ -30,12 +31,12 @@ describe("Browser State Service", () => { let logService: MockProxy; let stateFactory: MockProxy>; let useAccountCache: boolean; - let accountService: MockProxy; let environmentService: MockProxy; let migrationRunner: MockProxy; let state: State; - const userId = "userId"; + const userId = "userId" as UserId; + const accountService = mockAccountServiceWith(userId); let sut: BrowserStateService; @@ -44,7 +45,6 @@ describe("Browser State Service", () => { diskStorageService = mock(); logService = mock(); stateFactory = mock(); - accountService = mock(); environmentService = mock(); migrationRunner = mock(); // turn off account cache for tests @@ -57,6 +57,10 @@ describe("Browser State Service", () => { state.activeUserId = userId; }); + afterEach(() => { + jest.resetAllMocks(); + }); + describe("state methods", () => { let memoryStorageService: MockProxy; diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 175f082163..c76e34e44d 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -73,6 +73,7 @@ import { ConfigService } from "@bitwarden/common/platform/services/config/config import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; +import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service"; import { DerivedStateProvider, StateProvider } from "@bitwarden/common/platform/state"; import { SearchService } from "@bitwarden/common/services/search.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; @@ -109,6 +110,7 @@ import { BrowserApi } from "../../platform/browser/browser-api"; import BrowserPopupUtils from "../../platform/popup/browser-popup-utils"; import { BrowserStateService as StateServiceAbstraction } from "../../platform/services/abstractions/browser-state.service"; import { BrowserConfigService } from "../../platform/services/browser-config.service"; +import { BrowserCryptoService } from "../../platform/services/browser-crypto.service"; import { BrowserEnvironmentService } from "../../platform/services/browser-environment.service"; import { BrowserFileDownloadService } from "../../platform/services/browser-file-download.service"; import { BrowserI18nService } from "../../platform/services/browser-i18n.service"; @@ -210,7 +212,7 @@ function getBgService(service: keyof MainBackground) { { provide: CipherService, useFactory: getBgService("cipherService"), deps: [] }, { provide: CryptoFunctionService, - useFactory: getBgService("cryptoFunctionService"), + useFactory: () => new WebCryptoFunctionService(window), deps: [], }, { @@ -258,12 +260,36 @@ function getBgService(service: keyof MainBackground) { }, { provide: CryptoService, - useFactory: (encryptService: EncryptService) => { - const cryptoService = getBgService("cryptoService")(); + useFactory: ( + cryptoFunctionService: CryptoFunctionService, + encryptService: EncryptService, + platformUtilsService: PlatformUtilsService, + logService: LogServiceAbstraction, + stateService: StateServiceAbstraction, + accountService: AccountServiceAbstraction, + stateProvider: StateProvider, + ) => { + const cryptoService = new BrowserCryptoService( + cryptoFunctionService, + encryptService, + platformUtilsService, + logService, + stateService, + accountService, + stateProvider, + ); new ContainerService(cryptoService, encryptService).attachToGlobal(self); return cryptoService; }, - deps: [EncryptService], + deps: [ + CryptoFunctionService, + EncryptService, + PlatformUtilsService, + LogServiceAbstraction, + StateServiceAbstraction, + AccountServiceAbstraction, + StateProvider, + ], }, { provide: AuthRequestCryptoServiceAbstraction, diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index ed6ba6e031..1364127f65 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -65,6 +65,10 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountStatus(userId, status); } + async setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise { + await this.mock.setMaxAccountStatus(userId, maxStatus); + } + async switchAccount(userId: UserId): Promise { await this.mock.switchAccount(userId); } diff --git a/libs/common/spec/fake-state-provider.ts b/libs/common/spec/fake-state-provider.ts index dabfbb34e0..c87561c964 100644 --- a/libs/common/spec/fake-state-provider.ts +++ b/libs/common/spec/fake-state-provider.ts @@ -140,7 +140,9 @@ export class FakeActiveUserStateProvider implements ActiveUserStateProvider { } export class FakeStateProvider implements StateProvider { + mock = mock(); getUserState$(keyDefinition: KeyDefinition, userId?: UserId): Observable { + this.mock.getUserState$(keyDefinition, userId); if (userId) { return this.getUser(userId, keyDefinition).state$; } @@ -152,6 +154,7 @@ export class FakeStateProvider implements StateProvider { value: T, userId?: UserId, ): Promise<[UserId, T]> { + await this.mock.setUserState(keyDefinition, value, userId); if (userId) { return [userId, await this.getUser(userId, keyDefinition).update(() => value)]; } else { diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index ca9e82335f..4e2a462755 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -47,6 +47,17 @@ export abstract class AccountService { * @param status */ abstract setAccountStatus(userId: UserId, status: AuthenticationStatus): Promise; + /** + * Updates the `accounts$` observable with the new account status if the current status is higher than the `maxStatus`. + * + * This method only downgrades status to the maximum value sent in, it will not increase authentication status. + * + * @example An account is transitioning from unlocked to logged out. If callbacks that set the status to locked occur + * after it is updated to logged out, the account will be in the incorrect state. + * @param userId The user id of the account to be updated. + * @param maxStatus The new status of the account. + */ + abstract setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise; /** * Updates the `activeAccount$` observable with the new active account. * @param userId diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index 4df1a39d6d..e4195365f4 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -207,4 +207,26 @@ describe("accountService", () => { expect(sut.switchAccount("unknown" as UserId)).rejects.toThrowError("Account does not exist"); }); }); + + describe("setMaxAccountStatus", () => { + it("should update the account", async () => { + accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); + await sut.setMaxAccountStatus(userId, AuthenticationStatus.Locked); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: userInfo(AuthenticationStatus.Locked), + }); + }); + + it("should not update if the new max status is higher than the current", async () => { + accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.LoggedOut) }); + await sut.setMaxAccountStatus(userId, AuthenticationStatus.Locked); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: userInfo(AuthenticationStatus.LoggedOut), + }); + }); + }); }); diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 34c8279a46..8ef235d815 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -84,6 +84,24 @@ export class AccountServiceImplementation implements InternalAccountService { } } + async setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise { + await this.accountsState.update( + (accounts) => { + accounts[userId].status = maxStatus; + return accounts; + }, + { + shouldUpdate: (accounts) => { + if (accounts?.[userId] == null) { + throw new Error("Account does not exist"); + } + + return accounts[userId].status > maxStatus; + }, + }, + ); + } + async switchAccount(userId: UserId): Promise { await this.activeAccountIdState.update( (_, accounts) => { 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 d03e84c704..a5abac5e82 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 @@ -1,3 +1,5 @@ +import { firstValueFrom } from "rxjs"; + import { AppIdService } from "../../platform/abstractions/app-id.service"; import { CryptoFunctionService } from "../../platform/abstractions/crypto-function.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; @@ -108,7 +110,7 @@ export class DeviceTrustCryptoService implements DeviceTrustCryptoServiceAbstrac } // At this point of rotating their keys, they should still have their old user key in state - const oldUserKey = await this.stateService.getUserKey(); + const oldUserKey = await firstValueFrom(this.cryptoService.activeUserKey$); const deviceIdentifier = await this.appIdService.getAppId(); const secretVerificationRequest = new SecretVerificationRequest(); 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 1093b36184..0fbb954fb3 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 @@ -1,4 +1,5 @@ import { matches, mock } from "jest-mock-extended"; +import { of } from "rxjs"; import { DeviceType } from "../../enums"; import { AppIdService } from "../../platform/abstractions/app-id.service"; @@ -19,6 +20,7 @@ import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trus import { ProtectedDeviceResponse } from "../models/response/protected-device.response"; import { DeviceTrustCryptoService } from "./device-trust-crypto.service.implementation"; + describe("deviceTrustCryptoService", () => { let deviceTrustCryptoService: DeviceTrustCryptoService; @@ -495,6 +497,7 @@ describe("deviceTrustCryptoService", () => { const fakeNewUserKeyData = new Uint8Array(64); fakeNewUserKeyData.fill(FakeNewUserKeyMarker, 0, 1); fakeNewUserKey = new SymmetricCryptoKey(fakeNewUserKeyData) as UserKey; + cryptoService.activeUserKey$ = of(fakeNewUserKey); }); it("does an early exit when the current device is not a trusted device", async () => { @@ -521,9 +524,7 @@ describe("deviceTrustCryptoService", () => { fakeOldUserKeyData.fill(FakeOldUserKeyMarker, 0, 1); // Mock the retrieval of a user key that differs from the new one passed into the method - stateService.getUserKey.mockResolvedValue( - new SymmetricCryptoKey(fakeOldUserKeyData) as UserKey, - ); + cryptoService.activeUserKey$ = of(new SymmetricCryptoKey(fakeOldUserKeyData) as UserKey); appIdService.getAppId.mockResolvedValue("test_device_identifier"); diff --git a/libs/common/src/platform/abstractions/crypto.service.ts b/libs/common/src/platform/abstractions/crypto.service.ts index 6e7a2ba771..21ddccb9d6 100644 --- a/libs/common/src/platform/abstractions/crypto.service.ts +++ b/libs/common/src/platform/abstractions/crypto.service.ts @@ -12,10 +12,13 @@ import { EncString } from "../models/domain/enc-string"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; export abstract class CryptoService { + activeUserKey$: Observable; /** * Sets the provided user key and stores * any other necessary versions (such as auto, biometrics, * or pin) + * + * @throws when key is null. Use {@link clearUserKey} instead * @param key The user key to set * @param userId The desired user */ diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index d905c6ce97..8de781674f 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -16,7 +16,7 @@ import { UsernameGeneratorOptions } from "../../tools/generator/username"; import { SendData } from "../../tools/send/models/data/send.data"; import { SendView } from "../../tools/send/models/view/send.view"; import { UserId } from "../../types/guid"; -import { DeviceKey, MasterKey, UserKey } from "../../types/key"; +import { DeviceKey, MasterKey } from "../../types/key"; import { UriMatchType } from "../../vault/enums"; import { CipherData } from "../../vault/models/data/cipher.data"; import { LocalData } from "../../vault/models/data/local.data"; @@ -50,6 +50,9 @@ export type InitOptions = { export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; activeAccount$: Observable; + /** + * @deprecated use accountService.activeAccount$ instead + */ activeAccountUnlocked$: Observable; addAccount: (account: T) => Promise; @@ -82,14 +85,6 @@ export abstract class StateService { setClearClipboard: (value: number, options?: StorageOptions) => Promise; getConvertAccountToKeyConnector: (options?: StorageOptions) => Promise; setConvertAccountToKeyConnector: (value: boolean, options?: StorageOptions) => Promise; - /** - * gets the user key - */ - getUserKey: (options?: StorageOptions) => Promise; - /** - * Sets the user key - */ - setUserKey: (value: UserKey, options?: StorageOptions) => Promise; /** * Gets the user's master key */ @@ -150,10 +145,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use getUserKeyMasterKey instead */ getEncryptedCryptoSymmetricKey: (options?: StorageOptions) => Promise; - /** - * @deprecated For migration purposes only, use setUserKeyMasterKey instead - */ - setEncryptedCryptoSymmetricKey: (value: string, options?: StorageOptions) => Promise; /** * @deprecated For legacy purposes only, use getMasterKey instead */ diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 53e5fd4465..40fc2e623b 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -19,7 +19,7 @@ import { UsernameGeneratorOptions } from "../../../tools/generator/username/user import { SendData } from "../../../tools/send/models/data/send.data"; import { SendView } from "../../../tools/send/models/view/send.view"; import { DeepJsonify } from "../../../types/deep-jsonify"; -import { MasterKey, UserKey } from "../../../types/key"; +import { MasterKey } from "../../../types/key"; import { UriMatchType } from "../../../vault/enums"; import { CipherData } from "../../../vault/models/data/cipher.data"; import { CipherView } from "../../../vault/models/view/cipher.view"; @@ -113,7 +113,6 @@ export class AccountData { } export class AccountKeys { - userKey?: UserKey; masterKey?: MasterKey; masterKeyEncryptedUserKey?: string; deviceKey?: ReturnType; @@ -146,7 +145,6 @@ export class AccountKeys { return null; } return Object.assign(new AccountKeys(), obj, { - userKey: SymmetricCryptoKey.fromJSON(obj?.userKey), masterKey: SymmetricCryptoKey.fromJSON(obj?.masterKey), deviceKey: obj?.deviceKey, cryptoMasterKey: SymmetricCryptoKey.fromJSON(obj?.cryptoMasterKey), diff --git a/libs/common/src/platform/services/crypto.service.spec.ts b/libs/common/src/platform/services/crypto.service.spec.ts index 924a8ba269..fd4e5b92c7 100644 --- a/libs/common/src/platform/services/crypto.service.spec.ts +++ b/libs/common/src/platform/services/crypto.service.spec.ts @@ -4,6 +4,7 @@ import { firstValueFrom } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { FakeActiveUserState, FakeSingleUserState } from "../../../spec/fake-state"; import { FakeStateProvider } from "../../../spec/fake-state-provider"; +import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { CsprngArray } from "../../types/csprng"; import { UserId } from "../../types/guid"; import { UserKey, MasterKey, PinKey } from "../../types/key"; @@ -17,7 +18,7 @@ import { EncString } from "../models/domain/enc-string"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; import { CryptoService } from "../services/crypto.service"; -import { USER_EVER_HAD_USER_KEY } from "./key-state/user-key.state"; +import { USER_EVER_HAD_USER_KEY, USER_KEY } from "./key-state/user-key.state"; describe("cryptoService", () => { let cryptoService: CryptoService; @@ -57,42 +58,50 @@ describe("cryptoService", () => { describe("getUserKey", () => { let mockUserKey: UserKey; - let stateSvcGetUserKey: jest.SpyInstance; beforeEach(() => { const mockRandomBytes = new Uint8Array(64) as CsprngArray; mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; + }); - stateSvcGetUserKey = jest.spyOn(stateService, "getUserKey"); + it("retrieves the key state of the requested user", async () => { + await cryptoService.getUserKey(mockUserId); + + expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith(USER_KEY, mockUserId); }); it("returns the User Key if available", async () => { - stateSvcGetUserKey.mockResolvedValue(mockUserKey); + stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(mockUserKey); const userKey = await cryptoService.getUserKey(mockUserId); - expect(stateSvcGetUserKey).toHaveBeenCalledWith({ userId: mockUserId }); expect(userKey).toEqual(mockUserKey); }); - it("sets the Auto key if the User Key if not set", async () => { + it("sets from the Auto key if the User Key if not set", async () => { const autoKeyB64 = "IT5cA1i5Hncd953pb00E58D2FqJX+fWTj4AvoI67qkGHSQPgulAqKv+LaKRAo9Bg0xzP9Nw00wk4TqjMmGSM+g=="; stateService.getUserKeyAutoUnlock.mockResolvedValue(autoKeyB64); + const setKeySpy = jest.spyOn(cryptoService, "setUserKey"); const userKey = await cryptoService.getUserKey(mockUserId); - expect(stateService.setUserKey).toHaveBeenCalledWith(expect.any(SymmetricCryptoKey), { - userId: mockUserId, - }); + expect(setKeySpy).toHaveBeenCalledWith(expect.any(SymmetricCryptoKey), mockUserId); + expect(setKeySpy).toHaveBeenCalledTimes(1); + expect(userKey.keyB64).toEqual(autoKeyB64); }); + + it("returns nullish if there is no auto key and the user key is not set", async () => { + const userKey = await cryptoService.getUserKey(mockUserId); + + expect(userKey).toBeFalsy(); + }); }); describe("getUserKeyWithLegacySupport", () => { let mockUserKey: UserKey; let mockMasterKey: MasterKey; - let stateSvcGetUserKey: jest.SpyInstance; let stateSvcGetMasterKey: jest.SpyInstance; beforeEach(() => { @@ -100,23 +109,22 @@ describe("cryptoService", () => { mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; mockMasterKey = new SymmetricCryptoKey(new Uint8Array(64) as CsprngArray) as MasterKey; - stateSvcGetUserKey = jest.spyOn(stateService, "getUserKey"); stateSvcGetMasterKey = jest.spyOn(stateService, "getMasterKey"); }); it("returns the User Key if available", async () => { - stateSvcGetUserKey.mockResolvedValue(mockUserKey); + stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(mockUserKey); + const getKeySpy = jest.spyOn(cryptoService, "getUserKey"); const userKey = await cryptoService.getUserKeyWithLegacySupport(mockUserId); - expect(stateSvcGetUserKey).toHaveBeenCalledWith({ userId: mockUserId }); + expect(getKeySpy).toHaveBeenCalledWith(mockUserId); expect(stateSvcGetMasterKey).not.toHaveBeenCalled(); expect(userKey).toEqual(mockUserKey); }); it("returns the user's master key when User Key is not available", async () => { - stateSvcGetUserKey.mockResolvedValue(null); stateSvcGetMasterKey.mockResolvedValue(mockMasterKey); const userKey = await cryptoService.getUserKeyWithLegacySupport(mockUserId); @@ -201,6 +209,19 @@ describe("cryptoService", () => { }); }); + it("throws if key is null", async () => { + await expect(cryptoService.setUserKey(null, mockUserId)).rejects.toThrow("No key provided."); + }); + + it("should update the user's lock state", async () => { + await cryptoService.setUserKey(mockUserKey, mockUserId); + + expect(accountService.mock.setAccountStatus).toHaveBeenCalledWith( + mockUserId, + AuthenticationStatus.Unlocked, + ); + }); + describe("Pin Key refresh", () => { let cryptoSvcMakePinKey: jest.SpyInstance; const protectedPin = @@ -259,4 +280,36 @@ 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, + ); + }); + + 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); + + if (clear) { + expect(clearSpy).toHaveBeenCalledWith(mockUserId); + expect(clearSpy).toHaveBeenCalledTimes(1); + } else { + expect(clearSpy).not.toHaveBeenCalled(); + } + }, + ); + }); }); diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index 2991b6e57a..83faf4f7d3 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -6,6 +6,7 @@ import { ProfileOrganizationResponse } from "../../admin-console/models/response import { ProfileProviderOrganizationResponse } from "../../admin-console/models/response/profile-provider-organization.response"; import { ProfileProviderResponse } from "../../admin-console/models/response/profile-provider.response"; import { AccountService } from "../../auth/abstractions/account.service"; +import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { Utils } from "../../platform/misc/utils"; import { OrganizationId, ProviderId, UserId } from "../../types/guid"; @@ -52,9 +53,11 @@ import { USER_EVER_HAD_USER_KEY, USER_PRIVATE_KEY, USER_PUBLIC_KEY, + USER_KEY, } from "./key-state/user-key.state"; export class CryptoService implements CryptoServiceAbstraction { + private readonly activeUserKeyState: ActiveUserState; private readonly activeUserEverHadUserKey: ActiveUserState; private readonly activeUserEncryptedOrgKeysState: ActiveUserState< Record @@ -68,6 +71,8 @@ export class CryptoService implements CryptoServiceAbstraction { private readonly activeUserPrivateKeyState: DerivedState; private readonly activeUserPublicKeyState: DerivedState; + readonly activeUserKey$: Observable; + readonly activeUserOrgKeys$: Observable>; readonly activeUserProviderKeys$: Observable>; readonly activeUserPrivateKey$: Observable; @@ -84,6 +89,8 @@ export class CryptoService implements CryptoServiceAbstraction { protected stateProvider: StateProvider, ) { // User Key + this.activeUserKeyState = stateProvider.getActive(USER_KEY); + this.activeUserKey$ = this.activeUserKeyState.state$; this.activeUserEverHadUserKey = stateProvider.getActive(USER_EVER_HAD_USER_KEY); this.everHadUserKey$ = this.activeUserEverHadUserKey.state$.pipe(map((x) => x ?? false)); @@ -131,13 +138,15 @@ export class CryptoService implements CryptoServiceAbstraction { } async setUserKey(key: UserKey, userId?: UserId): Promise { - // TODO: make this non-nullable in signature - userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id; - if (key != null) { - // Key should never be null anyway - await this.stateProvider.getUser(userId, USER_EVER_HAD_USER_KEY).update(() => true); + if (key == null) { + throw new Error("No key provided. Use ClearUserKey to clear the key"); } - await this.stateService.setUserKey(key, { userId: userId }); + // Set userId to ensure we have one for the account status update + [userId, key] = await this.stateProvider.setUserState(USER_KEY, key, userId); + await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, true, userId); + + await this.accountService.setAccountStatus(userId, AuthenticationStatus.Unlocked); + await this.storeAdditionalKeys(key, userId); } @@ -147,7 +156,7 @@ export class CryptoService implements CryptoServiceAbstraction { } async getUserKey(userId?: UserId): Promise { - let userKey = await this.stateService.getUserKey({ userId: userId }); + let userKey = await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId)); if (userKey) { return userKey; } @@ -197,7 +206,7 @@ export class CryptoService implements CryptoServiceAbstraction { } async hasUserKeyInMemory(userId?: UserId): Promise { - return (await this.stateService.getUserKey({ userId: userId })) != null; + return (await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId))) != null; } async hasUserKeyStored(keySuffix: KeySuffixOptions, userId?: UserId): Promise { @@ -215,7 +224,9 @@ export class CryptoService implements CryptoServiceAbstraction { } async clearUserKey(clearStoredKeys = true, userId?: UserId): Promise { - await this.stateService.setUserKey(null, { userId: userId }); + // 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); } 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 a6d1619e94..d0f54c9add 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 @@ -1,8 +1,9 @@ -import { UserPrivateKey, UserPublicKey } from "../../../types/key"; +import { UserPrivateKey, UserPublicKey, UserKey } from "../../../types/key"; import { CryptoFunctionService } from "../../abstractions/crypto-function.service"; import { EncryptService } from "../../abstractions/encrypt.service"; import { EncString, EncryptedString } from "../../models/domain/enc-string"; -import { KeyDefinition, CRYPTO_DISK, DeriveDefinition } from "../../state"; +import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; +import { KeyDefinition, CRYPTO_DISK, DeriveDefinition, CRYPTO_MEMORY } from "../../state"; import { CryptoService } from "../crypto.service"; export const USER_EVER_HAD_USER_KEY = new KeyDefinition(CRYPTO_DISK, "everHadUserKey", { @@ -57,3 +58,6 @@ export const USER_PUBLIC_KEY = DeriveDefinition.from< return (await cryptoFunctionService.rsaExtractPublicKey(privateKey)) as UserPublicKey; }, }); +export const USER_KEY = new KeyDefinition(CRYPTO_MEMORY, "userKey", { + deserializer: (obj) => SymmetricCryptoKey.fromJSON(obj) as UserKey, +}); diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 6a9abf2be0..96005ef394 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject, concatMap } from "rxjs"; +import { BehaviorSubject, Observable, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { OrganizationData } from "../../admin-console/models/data/organization.data"; @@ -20,7 +20,7 @@ import { UsernameGeneratorOptions } from "../../tools/generator/username"; import { SendData } from "../../tools/send/models/data/send.data"; import { SendView } from "../../tools/send/models/view/send.view"; import { UserId } from "../../types/guid"; -import { DeviceKey, MasterKey, UserKey } from "../../types/key"; +import { DeviceKey, MasterKey } from "../../types/key"; import { UriMatchType } from "../../vault/enums"; import { CipherData } from "../../vault/models/data/cipher.data"; import { LocalData } from "../../vault/models/data/local.data"; @@ -87,8 +87,7 @@ export class StateService< protected activeAccountSubject = new BehaviorSubject(null); activeAccount$ = this.activeAccountSubject.asObservable(); - protected activeAccountUnlockedSubject = new BehaviorSubject(false); - activeAccountUnlocked$ = this.activeAccountUnlockedSubject.asObservable(); + activeAccountUnlocked$: Observable; private hasBeenInited = false; protected isRecoveredSession = false; @@ -109,22 +108,11 @@ export class StateService< private migrationRunner: MigrationRunner, protected useAccountCache: boolean = true, ) { - // If the account gets changed, verify the new account is unlocked - this.activeAccountSubject - .pipe( - concatMap(async (userId) => { - if (userId == null && this.activeAccountUnlockedSubject.getValue() == false) { - return; - } else if (userId == null) { - this.activeAccountUnlockedSubject.next(false); - } - // FIXME: This should be refactored into AuthService or a similar service, - // as checking for the existence of the crypto key is a low level - // implementation detail. - this.activeAccountUnlockedSubject.next((await this.getUserKey()) != null); - }), - ) - .subscribe(); + this.activeAccountUnlocked$ = this.accountService.activeAccount$.pipe( + map((a) => { + return a?.status === AuthenticationStatus.Unlocked; + }), + ); } async init(initOptions: InitOptions = {}): Promise { @@ -522,68 +510,6 @@ export class StateService< return account?.keys?.cryptoMasterKey; } - /** - * @deprecated Do not save the Master Key. Use the User Symmetric Key instead - */ - async setCryptoMasterKey(value: SymmetricCryptoKey, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - account.keys.cryptoMasterKey = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - - const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; - await this.accountService.setAccountStatus(options.userId as UserId, nextStatus); - - if (options.userId == this.activeAccountSubject.getValue()) { - const nextValue = value != null; - - // Avoid emitting if we are already unlocked - if (this.activeAccountUnlockedSubject.getValue() != nextValue) { - this.activeAccountUnlockedSubject.next(nextValue); - } - } - } - - /** - * user key used to encrypt/decrypt data - */ - async getUserKey(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - return account?.keys?.userKey as UserKey; - } - - /** - * user key used to encrypt/decrypt data - */ - async setUserKey(value: UserKey, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - account.keys.userKey = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - - const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; - await this.accountService.setAccountStatus(options.userId as UserId, nextStatus); - - if (options?.userId == this.activeAccountSubject.getValue()) { - const nextValue = value != null; - - // Avoid emitting if we are already unlocked - if (this.activeAccountUnlockedSubject.getValue() != nextValue) { - this.activeAccountUnlockedSubject.next(nextValue); - } - } - } - /** * User's master key derived from MP, saved only if we decrypted with MP */ @@ -885,33 +811,6 @@ export class StateService< ); } - /** - * @deprecated Use UserKey instead - */ - async getDecryptedCryptoSymmetricKey(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - return account?.keys?.cryptoSymmetricKey?.decrypted; - } - - /** - * @deprecated Use UserKey instead - */ - async setDecryptedCryptoSymmetricKey( - value: SymmetricCryptoKey, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - account.keys.cryptoSymmetricKey.decrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - } - @withPrototypeForArrayMembers(GeneratedPasswordHistory) async getDecryptedPasswordGenerationHistory( options?: StorageOptions, @@ -1565,20 +1464,6 @@ export class StateService< )?.keys.cryptoSymmetricKey.encrypted; } - /** - * @deprecated Use UserKey instead - */ - async setEncryptedCryptoSymmetricKey(value: string, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.keys.cryptoSymmetricKey.encrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - @withPrototypeForArrayMembers(GeneratedPasswordHistory) async getEncryptedPasswordGenerationHistory( options?: StorageOptions, diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index 876399924c..ddfa19bce8 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -22,6 +22,7 @@ export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); export const BILLING_BANNERS_DISK = new StateDefinition("billingBanners", "disk"); export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); +export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); export const SSO_DISK = new StateDefinition("ssoLogin", "disk");