From c3eba7f2c83c2156052ade0275208665378ef47a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 4 Mar 2024 14:33:25 -0600 Subject: [PATCH] [PM-6404] Fully Integrate `clearOn` Events (#8134) * Add New KeyDefinitionOption * Add New Services * Add WebStorageServiceProvider Tests * Update Error Message * Add `UserKeyDefinition` * Fix Deserialization Helpers * Fix KeyDefinition * Add `UserKeyDefinition` * Fix Deserialization Helpers * Fix KeyDefinition * Move `ClearEvent` * Cleanup * Fix Imports * Integrate onClear Events * Remove Accidental Addition * Fix Test * Add VaultTimeoutService Tests * Only Register When Current State is Null * Address Feedback --- .../browser/src/background/main.background.ts | 33 ++++++-- .../vault-timeout-service.factory.ts | 8 +- .../active-user-state-provider.factory.ts | 20 +++-- .../global-state-provider.factory.ts | 17 ++-- .../single-user-state-provider.factory.ts | 20 +++-- .../state-event-runner-service.factory.ts | 33 ++++++++ apps/cli/src/bw.ts | 32 +++++-- apps/desktop/src/app/app.component.ts | 18 ++-- apps/desktop/src/main.ts | 18 ++-- apps/web/src/app/app.component.ts | 4 + apps/web/src/app/core/core.module.ts | 35 ++------ .../web-active-user-state.provider.ts | 44 ---------- .../app/platform/web-global-state.provider.ts | 42 ---------- .../web-single-user-state.provider.ts | 43 ---------- .../src/services/jslib-services.module.ts | 25 +++++- libs/common/spec/fake-state.ts | 1 + .../services/environment.service.spec.ts | 16 ++-- ...default-active-user-state.provider.spec.ts | 17 ++-- .../default-active-user-state.provider.ts | 55 ++++-------- .../default-active-user-state.spec.ts | 51 +++++++++++- .../default-active-user-state.ts | 7 ++ .../default-global-state.provider.ts | 41 +++------ .../default-single-user-state.provider.ts | 57 +++++-------- .../default-single-user-state.spec.ts | 53 +++++++++++- .../default-single-user-state.ts | 7 ++ .../specific-state.provider.spec.ts | 24 ++++-- .../vault-timeout.service.spec.ts | 83 ++++++++++++++++++- .../vault-timeout/vault-timeout.service.ts | 12 ++- 28 files changed, 471 insertions(+), 345 deletions(-) create mode 100644 apps/browser/src/platform/background/service-factories/state-event-runner-service.factory.ts delete mode 100644 apps/web/src/app/platform/web-active-user-state.provider.ts delete mode 100644 apps/web/src/app/platform/web-global-state.provider.ts delete mode 100644 apps/web/src/app/platform/web-single-user-state.provider.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index b214ba0d390..ffdd2e30898 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -93,6 +93,7 @@ import { KeyGenerationService } from "@bitwarden/common/platform/services/key-ge import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; +import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { SystemService } from "@bitwarden/common/platform/services/system.service"; import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service"; import { @@ -100,6 +101,7 @@ import { DerivedStateProvider, GlobalStateProvider, SingleUserStateProvider, + StateEventRunnerService, StateProvider, } from "@bitwarden/common/platform/state"; /* eslint-disable import/no-restricted-paths -- We need the implementation to inject, but generally these should not be accessed */ @@ -107,6 +109,7 @@ import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider"; import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; +import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; /* eslint-enable import/no-restricted-paths */ import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service"; import { ApiService } from "@bitwarden/common/services/api.service"; @@ -299,6 +302,7 @@ export default class MainBackground { organizationVaultExportService: OrganizationVaultExportServiceAbstraction; vaultSettingsService: VaultSettingsServiceAbstraction; biometricStateService: BiometricStateService; + stateEventRunnerService: StateEventRunnerService; ssoLoginService: SsoLoginServiceAbstraction; // Passed to the popup for Safari to workaround issues with theming, downloading, etc. @@ -366,10 +370,24 @@ export default class MainBackground { this.keyGenerationService, ) : new BackgroundMemoryStorageService(); - this.globalStateProvider = new DefaultGlobalStateProvider( - this.memoryStorageForStateProviders, + + const storageServiceProvider = new StorageServiceProvider( this.storageService as BrowserLocalStorageService, + this.memoryStorageForStateProviders, ); + + this.globalStateProvider = new DefaultGlobalStateProvider(storageServiceProvider); + + const stateEventRegistrarService = new StateEventRegistrarService( + this.globalStateProvider, + storageServiceProvider, + ); + + this.stateEventRunnerService = new StateEventRunnerService( + this.globalStateProvider, + storageServiceProvider, + ); + this.encryptService = flagEnabled("multithreadDecryption") ? new MultithreadEncryptServiceImplementation( this.cryptoFunctionService, @@ -379,8 +397,8 @@ export default class MainBackground { : new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, true); this.singleUserStateProvider = new DefaultSingleUserStateProvider( - this.memoryStorageForStateProviders, - this.storageService as BrowserLocalStorageService, + storageServiceProvider, + stateEventRegistrarService, ); this.accountService = new AccountServiceImplementation( this.messagingService, @@ -389,8 +407,8 @@ export default class MainBackground { ); this.activeUserStateProvider = new DefaultActiveUserStateProvider( this.accountService, - this.memoryStorageForStateProviders, - this.storageService as BrowserLocalStorageService, + storageServiceProvider, + stateEventRegistrarService, ); this.derivedStateProvider = new BackgroundDerivedStateProvider( this.memoryStorageForStateProviders, @@ -666,6 +684,7 @@ export default class MainBackground { this.stateService, this.authService, this.vaultTimeoutSettingsService, + this.stateEventRunnerService, lockedCallback, logoutCallback, ); @@ -1113,6 +1132,8 @@ export default class MainBackground { this.searchService.clearIndex(); } + await this.stateEventRunnerService.handleEvent("logout", currentUserId as UserId); + if (newActiveUser != null) { // we have a new active user, do not continue tearing down application await this.switchAccount(newActiveUser as UserId); diff --git a/apps/browser/src/background/service-factories/vault-timeout-service.factory.ts b/apps/browser/src/background/service-factories/vault-timeout-service.factory.ts index 7a34b6b4b14..0e4d1420da5 100644 --- a/apps/browser/src/background/service-factories/vault-timeout-service.factory.ts +++ b/apps/browser/src/background/service-factories/vault-timeout-service.factory.ts @@ -21,6 +21,10 @@ import { platformUtilsServiceFactory, PlatformUtilsServiceInitOptions, } from "../../platform/background/service-factories/platform-utils-service.factory"; +import { + stateEventRunnerServiceFactory, + StateEventRunnerServiceInitOptions, +} from "../../platform/background/service-factories/state-event-runner-service.factory"; import { StateServiceInitOptions, stateServiceFactory, @@ -62,7 +66,8 @@ export type VaultTimeoutServiceInitOptions = VaultTimeoutServiceFactoryOptions & SearchServiceInitOptions & StateServiceInitOptions & AuthServiceInitOptions & - VaultTimeoutSettingsServiceInitOptions; + VaultTimeoutSettingsServiceInitOptions & + StateEventRunnerServiceInitOptions; export function vaultTimeoutServiceFactory( cache: { vaultTimeoutService?: AbstractVaultTimeoutService } & CachedServices, @@ -84,6 +89,7 @@ export function vaultTimeoutServiceFactory( await stateServiceFactory(cache, opts), await authServiceFactory(cache, opts), await vaultTimeoutSettingsServiceFactory(cache, opts), + await stateEventRunnerServiceFactory(cache, opts), opts.vaultTimeoutServiceOptions.lockedCallback, opts.vaultTimeoutServiceOptions.loggedOutCallback, ), diff --git a/apps/browser/src/platform/background/service-factories/active-user-state-provider.factory.ts b/apps/browser/src/platform/background/service-factories/active-user-state-provider.factory.ts index e2d8746c7ce..6dafd2952e0 100644 --- a/apps/browser/src/platform/background/service-factories/active-user-state-provider.factory.ts +++ b/apps/browser/src/platform/background/service-factories/active-user-state-provider.factory.ts @@ -9,18 +9,20 @@ import { import { CachedServices, FactoryOptions, factory } from "./factory-options"; import { - DiskStorageServiceInitOptions, - MemoryStorageServiceInitOptions, - observableDiskStorageServiceFactory, - observableMemoryStorageServiceFactory, -} from "./storage-service.factory"; + StateEventRegistrarServiceInitOptions, + stateEventRegistrarServiceFactory, +} from "./state-event-registrar-service.factory"; +import { + StorageServiceProviderInitOptions, + storageServiceProviderFactory, +} from "./storage-service-provider.factory"; type ActiveUserStateProviderFactory = FactoryOptions; export type ActiveUserStateProviderInitOptions = ActiveUserStateProviderFactory & AccountServiceInitOptions & - MemoryStorageServiceInitOptions & - DiskStorageServiceInitOptions; + StorageServiceProviderInitOptions & + StateEventRegistrarServiceInitOptions; export async function activeUserStateProviderFactory( cache: { activeUserStateProvider?: ActiveUserStateProvider } & CachedServices, @@ -33,8 +35,8 @@ export async function activeUserStateProviderFactory( async () => new DefaultActiveUserStateProvider( await accountServiceFactory(cache, opts), - await observableMemoryStorageServiceFactory(cache, opts), - await observableDiskStorageServiceFactory(cache, opts), + await storageServiceProviderFactory(cache, opts), + await stateEventRegistrarServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/platform/background/service-factories/global-state-provider.factory.ts b/apps/browser/src/platform/background/service-factories/global-state-provider.factory.ts index 3615b2b060d..154c212f046 100644 --- a/apps/browser/src/platform/background/service-factories/global-state-provider.factory.ts +++ b/apps/browser/src/platform/background/service-factories/global-state-provider.factory.ts @@ -4,17 +4,14 @@ import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/imp import { CachedServices, FactoryOptions, factory } from "./factory-options"; import { - DiskStorageServiceInitOptions, - MemoryStorageServiceInitOptions, - observableDiskStorageServiceFactory, - observableMemoryStorageServiceFactory, -} from "./storage-service.factory"; + StorageServiceProviderInitOptions, + storageServiceProviderFactory, +} from "./storage-service-provider.factory"; type GlobalStateProviderFactoryOptions = FactoryOptions; export type GlobalStateProviderInitOptions = GlobalStateProviderFactoryOptions & - MemoryStorageServiceInitOptions & - DiskStorageServiceInitOptions; + StorageServiceProviderInitOptions; export async function globalStateProviderFactory( cache: { globalStateProvider?: GlobalStateProvider } & CachedServices, @@ -24,10 +21,6 @@ export async function globalStateProviderFactory( cache, "globalStateProvider", opts, - async () => - new DefaultGlobalStateProvider( - await observableMemoryStorageServiceFactory(cache, opts), - await observableDiskStorageServiceFactory(cache, opts), - ), + async () => new DefaultGlobalStateProvider(await storageServiceProviderFactory(cache, opts)), ); } diff --git a/apps/browser/src/platform/background/service-factories/single-user-state-provider.factory.ts b/apps/browser/src/platform/background/service-factories/single-user-state-provider.factory.ts index 0af6f822148..87eaa8e95a5 100644 --- a/apps/browser/src/platform/background/service-factories/single-user-state-provider.factory.ts +++ b/apps/browser/src/platform/background/service-factories/single-user-state-provider.factory.ts @@ -4,17 +4,19 @@ import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state import { CachedServices, FactoryOptions, factory } from "./factory-options"; import { - DiskStorageServiceInitOptions, - MemoryStorageServiceInitOptions, - observableDiskStorageServiceFactory, - observableMemoryStorageServiceFactory, -} from "./storage-service.factory"; + StateEventRegistrarServiceInitOptions, + stateEventRegistrarServiceFactory, +} from "./state-event-registrar-service.factory"; +import { + StorageServiceProviderInitOptions, + storageServiceProviderFactory, +} from "./storage-service-provider.factory"; type SingleUserStateProviderFactoryOptions = FactoryOptions; export type SingleUserStateProviderInitOptions = SingleUserStateProviderFactoryOptions & - MemoryStorageServiceInitOptions & - DiskStorageServiceInitOptions; + StorageServiceProviderInitOptions & + StateEventRegistrarServiceInitOptions; export async function singleUserStateProviderFactory( cache: { singleUserStateProvider?: SingleUserStateProvider } & CachedServices, @@ -26,8 +28,8 @@ export async function singleUserStateProviderFactory( opts, async () => new DefaultSingleUserStateProvider( - await observableMemoryStorageServiceFactory(cache, opts), - await observableDiskStorageServiceFactory(cache, opts), + await storageServiceProviderFactory(cache, opts), + await stateEventRegistrarServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/platform/background/service-factories/state-event-runner-service.factory.ts b/apps/browser/src/platform/background/service-factories/state-event-runner-service.factory.ts new file mode 100644 index 00000000000..c81384cd4ac --- /dev/null +++ b/apps/browser/src/platform/background/service-factories/state-event-runner-service.factory.ts @@ -0,0 +1,33 @@ +import { StateEventRunnerService } from "@bitwarden/common/platform/state"; + +import { CachedServices, FactoryOptions, factory } from "./factory-options"; +import { + GlobalStateProviderInitOptions, + globalStateProviderFactory, +} from "./global-state-provider.factory"; +import { + StorageServiceProviderInitOptions, + storageServiceProviderFactory, +} from "./storage-service-provider.factory"; + +type StateEventRunnerServiceFactoryOptions = FactoryOptions; + +export type StateEventRunnerServiceInitOptions = StateEventRunnerServiceFactoryOptions & + GlobalStateProviderInitOptions & + StorageServiceProviderInitOptions; + +export function stateEventRunnerServiceFactory( + cache: { stateEventRunnerService?: StateEventRunnerService } & CachedServices, + opts: StateEventRunnerServiceInitOptions, +): Promise { + return factory( + cache, + "stateEventRunnerService", + opts, + async () => + new StateEventRunnerService( + await globalStateProviderFactory(cache, opts), + await storageServiceProviderFactory(cache, opts), + ), + ); +} diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 67c1be16f84..7f76bee69af 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -60,11 +60,13 @@ import { MigrationBuilderService } from "@bitwarden/common/platform/services/mig import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; import { NoopMessagingService } from "@bitwarden/common/platform/services/noop-messaging.service"; import { StateService } from "@bitwarden/common/platform/services/state.service"; +import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { ActiveUserStateProvider, DerivedStateProvider, GlobalStateProvider, SingleUserStateProvider, + StateEventRunnerService, StateProvider, } from "@bitwarden/common/platform/state"; /* eslint-disable import/no-restricted-paths -- We need the implementation to inject, but generally these should not be accessed */ @@ -73,6 +75,7 @@ import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/im import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider"; import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; +import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; /* eslint-enable import/no-restricted-paths */ import { AuditService } from "@bitwarden/common/services/audit.service"; @@ -208,6 +211,7 @@ export class Main { derivedStateProvider: DerivedStateProvider; stateProvider: StateProvider; loginStrategyService: LoginStrategyServiceAbstraction; + stateEventRunnerService: StateEventRunnerService; biometricStateService: BiometricStateService; constructor() { @@ -249,14 +253,26 @@ export class Main { this.memoryStorageService = new MemoryStorageService(); this.memoryStorageForStateProviders = new MemoryStorageServiceForStateProviders(); - this.globalStateProvider = new DefaultGlobalStateProvider( - this.memoryStorageForStateProviders, + const storageServiceProvider = new StorageServiceProvider( this.storageService, + this.memoryStorageForStateProviders, + ); + + this.globalStateProvider = new DefaultGlobalStateProvider(storageServiceProvider); + + const stateEventRegistrarService = new StateEventRegistrarService( + this.globalStateProvider, + storageServiceProvider, + ); + + this.stateEventRunnerService = new StateEventRunnerService( + this.globalStateProvider, + storageServiceProvider, ); this.singleUserStateProvider = new DefaultSingleUserStateProvider( - this.memoryStorageForStateProviders, - this.storageService, + storageServiceProvider, + stateEventRegistrarService, ); this.messagingService = new NoopMessagingService(); @@ -269,8 +285,8 @@ export class Main { this.activeUserStateProvider = new DefaultActiveUserStateProvider( this.accountService, - this.memoryStorageForStateProviders, - this.storageService, + storageServiceProvider, + stateEventRegistrarService, ); this.derivedStateProvider = new DefaultDerivedStateProvider( @@ -530,6 +546,7 @@ export class Main { this.stateService, this.authService, this.vaultTimeoutSettingsService, + this.stateEventRunnerService, lockedCallback, null, ); @@ -639,6 +656,9 @@ export class Main { this.policyService.clear(userId), this.passwordGenerationService.clear(), ]); + + await this.stateEventRunnerService.handleEvent("logout", userId as UserId); + await this.stateService.clean(); process.env.BW_SESSION = null; } diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index a42f80fd6f5..51bde2ef1c4 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -39,6 +39,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { StateEventRunnerService } from "@bitwarden/common/platform/state"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -149,6 +150,7 @@ export class AppComponent implements OnInit, OnDestroy { private configService: ConfigServiceAbstraction, private dialogService: DialogService, private biometricStateService: BiometricStateService, + private stateEventRunnerService: StateEventRunnerService, ) {} ngOnInit() { @@ -219,13 +221,13 @@ export class AppComponent implements OnInit, OnDestroy { const currentUser = await this.stateService.getUserId(); const accounts = await firstValueFrom(this.stateService.accounts$); await this.vaultTimeoutService.lock(currentUser); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Promise.all( - Object.keys(accounts) - .filter((u) => u !== currentUser) - .map((u) => this.vaultTimeoutService.lock(u)), - ); + for (const account of Object.keys(accounts)) { + if (account === currentUser) { + continue; + } + + await this.vaultTimeoutService.lock(account); + } break; } case "locked": @@ -583,6 +585,8 @@ export class AppComponent implements OnInit, OnDestroy { await this.keyConnectorService.clear(); await this.biometricStateService.logout(userBeingLoggedOut as UserId); + await this.stateEventRunnerService.handleEvent("logout", userBeingLoggedOut as UserId); + preLogoutActiveUserId = this.activeUserId; await this.stateService.clean({ userId: userBeingLoggedOut }); } finally { diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 87d85f8174d..dbcb85bacd8 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -13,11 +13,13 @@ import { MigrationBuilderService } from "@bitwarden/common/platform/services/mig import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; import { NoopMessagingService } from "@bitwarden/common/platform/services/noop-messaging.service"; /* eslint-disable import/no-restricted-paths -- We need the implementation to inject, but generally this should not be accessed */ +import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-active-user-state.provider"; import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/default-derived-state.provider"; import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider"; import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; +import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; /* eslint-enable import/no-restricted-paths */ @@ -104,10 +106,11 @@ export class Main { this.storageService = new ElectronStorageService(app.getPath("userData"), storageDefaults); this.memoryStorageService = new MemoryStorageService(); this.memoryStorageForStateProviders = new MemoryStorageServiceForStateProviders(); - const globalStateProvider = new DefaultGlobalStateProvider( - this.memoryStorageForStateProviders, + const storageServiceProvider = new StorageServiceProvider( this.storageService, + this.memoryStorageForStateProviders, ); + const globalStateProvider = new DefaultGlobalStateProvider(storageServiceProvider); const accountService = new AccountServiceImplementation( new NoopMessagingService(), @@ -115,13 +118,18 @@ export class Main { globalStateProvider, ); + const stateEventRegistrarService = new StateEventRegistrarService( + globalStateProvider, + storageServiceProvider, + ); + const stateProvider = new DefaultStateProvider( new DefaultActiveUserStateProvider( accountService, - this.memoryStorageForStateProviders, - this.storageService, + storageServiceProvider, + stateEventRegistrarService, ), - new DefaultSingleUserStateProvider(this.memoryStorageForStateProviders, this.storageService), + new DefaultSingleUserStateProvider(storageServiceProvider, stateEventRegistrarService), globalStateProvider, new DefaultDerivedStateProvider(this.memoryStorageForStateProviders), ); diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 77f6fd6f1ff..73721de7890 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -23,6 +23,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { StateEventRunnerService } from "@bitwarden/common/platform/state"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -89,6 +90,7 @@ export class AppComponent implements OnDestroy, OnInit { private configService: ConfigServiceAbstraction, private dialogService: DialogService, private biometricStateService: BiometricStateService, + private stateEventRunnerService: StateEventRunnerService, private paymentMethodWarningService: PaymentMethodWarningService, private organizationService: OrganizationService, ) {} @@ -284,6 +286,8 @@ export class AppComponent implements OnDestroy, OnInit { this.paymentMethodWarningService.clear(), ]); + await this.stateEventRunnerService.handleEvent("logout", userId as UserId); + this.searchService.clearIndex(); this.authService.logOut(async () => { if (expired) { diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index ee6f53ed018..6aded4875eb 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -14,7 +14,6 @@ import { } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { ModalService as ModalServiceAbstraction } from "@bitwarden/angular/services/modal.service"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/login.service"; import { LoginService } from "@bitwarden/common/auth/services/login.service"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; @@ -28,21 +27,16 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; -import { - ActiveUserStateProvider, - GlobalStateProvider, - SingleUserStateProvider, -} from "@bitwarden/common/platform/state"; -// eslint-disable-next-line import/no-restricted-paths -- Implementation for memory storage +/* eslint-disable import/no-restricted-paths -- Implementation for memory storage */ +import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; +/* eslint-enable import/no-restricted-paths -- Implementation for memory storage */ import { PolicyListService } from "../admin-console/core/policy-list.service"; import { HtmlStorageService } from "../core/html-storage.service"; import { I18nService } from "../core/i18n.service"; -import { WebActiveUserStateProvider } from "../platform/web-active-user-state.provider"; -import { WebGlobalStateProvider } from "../platform/web-global-state.provider"; import { WebMigrationRunner } from "../platform/web-migration-runner"; -import { WebSingleUserStateProvider } from "../platform/web-single-user-state.provider"; +import { WebStorageServiceProvider } from "../platform/web-storage-service.provider"; import { WindowStorageService } from "../platform/window-storage.service"; import { CollectionAdminService } from "../vault/core/collection-admin.service"; @@ -124,24 +118,9 @@ import { WebPlatformUtilsService } from "./web-platform-utils.service"; useFactory: () => new WindowStorageService(window.localStorage), }, { - provide: SingleUserStateProvider, - useClass: WebSingleUserStateProvider, - deps: [OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_STORAGE, OBSERVABLE_DISK_LOCAL_STORAGE], - }, - { - provide: ActiveUserStateProvider, - useClass: WebActiveUserStateProvider, - deps: [ - AccountService, - OBSERVABLE_MEMORY_STORAGE, - OBSERVABLE_DISK_STORAGE, - OBSERVABLE_DISK_LOCAL_STORAGE, - ], - }, - { - provide: GlobalStateProvider, - useClass: WebGlobalStateProvider, - deps: [OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_STORAGE, OBSERVABLE_DISK_LOCAL_STORAGE], + provide: StorageServiceProvider, + useClass: WebStorageServiceProvider, + deps: [OBSERVABLE_DISK_STORAGE, OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_LOCAL_STORAGE], }, { provide: MigrationRunner, diff --git a/apps/web/src/app/platform/web-active-user-state.provider.ts b/apps/web/src/app/platform/web-active-user-state.provider.ts deleted file mode 100644 index e02b134603a..00000000000 --- a/apps/web/src/app/platform/web-active-user-state.provider.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "@bitwarden/common/platform/abstractions/storage.service"; -import { UserKeyDefinition } from "@bitwarden/common/platform/state"; -/* eslint-disable import/no-restricted-paths -- Needed to extend class & in platform owned code */ -import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-active-user-state.provider"; -import { StateDefinition } from "@bitwarden/common/platform/state/state-definition"; -/* eslint-enable import/no-restricted-paths */ - -export class WebActiveUserStateProvider extends DefaultActiveUserStateProvider { - constructor( - accountService: AccountService, - memoryStorage: AbstractMemoryStorageService & ObservableStorageService, - sessionStorage: AbstractStorageService & ObservableStorageService, - private readonly diskLocalStorage: AbstractStorageService & ObservableStorageService, - ) { - super(accountService, memoryStorage, sessionStorage); - } - - protected override getLocationString(keyDefinition: UserKeyDefinition): string { - return ( - keyDefinition.stateDefinition.storageLocationOverrides["web"] ?? - keyDefinition.stateDefinition.defaultStorageLocation - ); - } - - protected override getLocation( - stateDefinition: StateDefinition, - ): AbstractStorageService & ObservableStorageService { - const location = - stateDefinition.storageLocationOverrides["web"] ?? stateDefinition.defaultStorageLocation; - switch (location) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - case "disk-local": - return this.diskLocalStorage; - } - } -} diff --git a/apps/web/src/app/platform/web-global-state.provider.ts b/apps/web/src/app/platform/web-global-state.provider.ts deleted file mode 100644 index 07025864cce..00000000000 --- a/apps/web/src/app/platform/web-global-state.provider.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "@bitwarden/common/platform/abstractions/storage.service"; -import { KeyDefinition } from "@bitwarden/common/platform/state"; -/* eslint-disable import/no-restricted-paths -- Needed to extend class & in platform owned code*/ -import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider"; -import { StateDefinition } from "@bitwarden/common/platform/state/state-definition"; -/* eslint-enable import/no-restricted-paths */ - -export class WebGlobalStateProvider extends DefaultGlobalStateProvider { - constructor( - memoryStorage: AbstractMemoryStorageService & ObservableStorageService, - sessionStorage: AbstractStorageService & ObservableStorageService, - private readonly diskLocalStorage: AbstractStorageService & ObservableStorageService, - ) { - super(memoryStorage, sessionStorage); - } - - protected getLocationString(keyDefinition: KeyDefinition): string { - return ( - keyDefinition.stateDefinition.storageLocationOverrides["web"] ?? - keyDefinition.stateDefinition.defaultStorageLocation - ); - } - - protected override getLocation( - stateDefinition: StateDefinition, - ): AbstractStorageService & ObservableStorageService { - const location = - stateDefinition.storageLocationOverrides["web"] ?? stateDefinition.defaultStorageLocation; - switch (location) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - case "disk-local": - return this.diskLocalStorage; - } - } -} diff --git a/apps/web/src/app/platform/web-single-user-state.provider.ts b/apps/web/src/app/platform/web-single-user-state.provider.ts deleted file mode 100644 index 4e4a807c456..00000000000 --- a/apps/web/src/app/platform/web-single-user-state.provider.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "@bitwarden/common/platform/abstractions/storage.service"; -import { UserKeyDefinition } from "@bitwarden/common/platform/state"; -/* eslint-disable import/no-restricted-paths -- Needed to extend service & and in platform owned file */ -import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; -import { StateDefinition } from "@bitwarden/common/platform/state/state-definition"; -/* eslint-enable import/no-restricted-paths */ - -export class WebSingleUserStateProvider extends DefaultSingleUserStateProvider { - constructor( - memoryStorageService: AbstractMemoryStorageService & ObservableStorageService, - sessionStorageService: AbstractStorageService & ObservableStorageService, - private readonly diskLocalStorageService: AbstractStorageService & ObservableStorageService, - ) { - super(memoryStorageService, sessionStorageService); - } - - protected override getLocationString(keyDefinition: UserKeyDefinition): string { - return ( - keyDefinition.stateDefinition.storageLocationOverrides["web"] ?? - keyDefinition.stateDefinition.defaultStorageLocation - ); - } - - protected override getLocation( - stateDefinition: StateDefinition, - ): AbstractStorageService & ObservableStorageService { - const location = - stateDefinition.storageLocationOverrides["web"] ?? stateDefinition.defaultStorageLocation; - - switch (location) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - case "disk-local": - return this.diskLocalStorageService; - } - } -} diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 3803e959115..daf39717583 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -135,6 +135,7 @@ import { MigrationBuilderService } from "@bitwarden/common/platform/services/mig import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; import { NoopNotificationsService } from "@bitwarden/common/platform/services/noop-notifications.service"; import { StateService } from "@bitwarden/common/platform/services/state.service"; +import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { ValidationService } from "@bitwarden/common/platform/services/validation.service"; import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service"; import { @@ -150,6 +151,8 @@ import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/im import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider"; import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; +import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; +import { StateEventRunnerService } from "@bitwarden/common/platform/state/state-event-runner.service"; /* eslint-enable import/no-restricted-paths */ import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service"; import { ApiService } from "@bitwarden/common/services/api.service"; @@ -551,6 +554,7 @@ import { ModalService } from "./modal.service"; StateServiceAbstraction, AuthServiceAbstraction, VaultTimeoutSettingsServiceAbstraction, + StateEventRunnerService, LOCKED_CALLBACK, LOGOUT_CALLBACK, ], @@ -890,20 +894,35 @@ import { ModalService } from "./modal.service"; LogService, ], }, + { + provide: StorageServiceProvider, + useClass: StorageServiceProvider, + deps: [OBSERVABLE_DISK_STORAGE, OBSERVABLE_MEMORY_STORAGE], + }, + { + provide: StateEventRegistrarService, + useClass: StateEventRegistrarService, + deps: [GlobalStateProvider, StorageServiceProvider], + }, + { + provide: StateEventRunnerService, + useClass: StateEventRunnerService, + deps: [GlobalStateProvider, StorageServiceProvider], + }, { provide: GlobalStateProvider, useClass: DefaultGlobalStateProvider, - deps: [OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_STORAGE], + deps: [StorageServiceProvider], }, { provide: ActiveUserStateProvider, useClass: DefaultActiveUserStateProvider, - deps: [AccountServiceAbstraction, OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_STORAGE], + deps: [AccountServiceAbstraction, StorageServiceProvider, StateEventRegistrarService], }, { provide: SingleUserStateProvider, useClass: DefaultSingleUserStateProvider, - deps: [OBSERVABLE_MEMORY_STORAGE, OBSERVABLE_DISK_STORAGE], + deps: [StorageServiceProvider, StateEventRegistrarService], }, { provide: DerivedStateProvider, diff --git a/libs/common/spec/fake-state.ts b/libs/common/spec/fake-state.ts index 6b03ef6ef80..0f2a09d9c1b 100644 --- a/libs/common/spec/fake-state.ts +++ b/libs/common/spec/fake-state.ts @@ -65,6 +65,7 @@ export class FakeGlobalState implements GlobalState { this.nextMock(newState); return newState; } + /** Tracks update values resolved by `FakeState.update` */ nextMock = jest.fn(); diff --git a/libs/common/src/platform/services/environment.service.spec.ts b/libs/common/src/platform/services/environment.service.spec.ts index a474e469a27..c5548959945 100644 --- a/libs/common/src/platform/services/environment.service.spec.ts +++ b/libs/common/src/platform/services/environment.service.spec.ts @@ -1,3 +1,4 @@ +import { mock } from "jest-mock-extended"; import { firstValueFrom, timeout } from "rxjs"; import { awaitAsync } from "../../../spec"; @@ -14,9 +15,11 @@ import { DefaultDerivedStateProvider } from "../state/implementations/default-de import { DefaultGlobalStateProvider } from "../state/implementations/default-global-state.provider"; import { DefaultSingleUserStateProvider } from "../state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "../state/implementations/default-state.provider"; -/* eslint-disable import/no-restricted-paths */ +import { StateEventRegistrarService } from "../state/state-event-registrar.service"; +/* eslint-enable import/no-restricted-paths */ import { EnvironmentService } from "./environment.service"; +import { StorageServiceProvider } from "./storage-service.provider"; // There are a few main states EnvironmentService could be in when first used // 1. Not initialized, no active user. Hopefully not to likely but possible @@ -26,6 +29,8 @@ import { EnvironmentService } from "./environment.service"; describe("EnvironmentService", () => { let diskStorageService: FakeStorageService; let memoryStorageService: FakeStorageService; + let storageServiceProvider: StorageServiceProvider; + const stateEventRegistrarService = mock(); let accountService: FakeAccountService; let stateProvider: StateProvider; @@ -37,16 +42,17 @@ describe("EnvironmentService", () => { beforeEach(async () => { diskStorageService = new FakeStorageService(); memoryStorageService = new FakeStorageService(); + storageServiceProvider = new StorageServiceProvider(diskStorageService, memoryStorageService); accountService = mockAccountServiceWith(undefined); stateProvider = new DefaultStateProvider( new DefaultActiveUserStateProvider( accountService, - memoryStorageService as any, - diskStorageService, + storageServiceProvider, + stateEventRegistrarService, ), - new DefaultSingleUserStateProvider(memoryStorageService as any, diskStorageService), - new DefaultGlobalStateProvider(memoryStorageService as any, diskStorageService), + new DefaultSingleUserStateProvider(storageServiceProvider, stateEventRegistrarService), + new DefaultGlobalStateProvider(storageServiceProvider), new DefaultDerivedStateProvider(memoryStorageService), ); diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts index d0fe0aa3f45..02300185492 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts @@ -3,17 +3,14 @@ import { mock } from "jest-mock-extended"; import { mockAccountServiceWith, trackEmissions } from "../../../../spec"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "../../abstractions/storage.service"; +import { StorageServiceProvider } from "../../services/storage-service.provider"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { DefaultActiveUserStateProvider } from "./default-active-user-state.provider"; describe("DefaultActiveUserStateProvider", () => { - const memoryStorage = mock(); - const diskStorage = mock(); + const storageServiceProvider = mock(); + const stateEventRegistrarService = mock(); const userId = "userId" as UserId; const accountInfo = { id: userId, @@ -25,7 +22,11 @@ describe("DefaultActiveUserStateProvider", () => { let sut: DefaultActiveUserStateProvider; beforeEach(() => { - sut = new DefaultActiveUserStateProvider(accountService, memoryStorage, diskStorage); + sut = new DefaultActiveUserStateProvider( + accountService, + storageServiceProvider, + stateEventRegistrarService, + ); }); afterEach(() => { diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.provider.ts b/libs/common/src/platform/state/implementations/default-active-user-state.provider.ts index ecb7f0fa955..268b22e5197 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.provider.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.provider.ts @@ -2,13 +2,9 @@ import { Observable, map } from "rxjs"; import { AccountService } from "../../../auth/abstractions/account.service"; import { UserId } from "../../../types/guid"; -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "../../abstractions/storage.service"; +import { StorageServiceProvider } from "../../services/storage-service.provider"; import { KeyDefinition } from "../key-definition"; -import { StateDefinition } from "../state-definition"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { UserKeyDefinition, isUserKeyDefinition } from "../user-key-definition"; import { ActiveUserState } from "../user-state"; import { ActiveUserStateProvider } from "../user-state.provider"; @@ -21,9 +17,9 @@ export class DefaultActiveUserStateProvider implements ActiveUserStateProvider { activeUserId$: Observable; constructor( - protected readonly accountService: AccountService, - protected readonly memoryStorage: AbstractMemoryStorageService & ObservableStorageService, - protected readonly diskStorage: AbstractStorageService & ObservableStorageService, + private readonly accountService: AccountService, + private readonly storageServiceProvider: StorageServiceProvider, + private readonly stateEventRegistrarService: StateEventRegistrarService, ) { this.activeUserId$ = this.accountService.activeAccount$.pipe(map((account) => account?.id)); } @@ -32,7 +28,11 @@ export class DefaultActiveUserStateProvider implements ActiveUserStateProvider { if (!isUserKeyDefinition(keyDefinition)) { keyDefinition = UserKeyDefinition.fromBaseKeyDefinition(keyDefinition); } - const cacheKey = this.buildCacheKey(keyDefinition); + const [location, storageService] = this.storageServiceProvider.get( + keyDefinition.stateDefinition.defaultStorageLocation, + keyDefinition.stateDefinition.storageLocationOverrides, + ); + const cacheKey = this.buildCacheKey(location, keyDefinition); const existingUserState = this.cache[cacheKey]; if (existingUserState != null) { // I have to cast out of the unknown generic but this should be safe if rules @@ -40,36 +40,17 @@ export class DefaultActiveUserStateProvider implements ActiveUserStateProvider { return existingUserState as ActiveUserState; } - const newUserState = this.buildActiveUserState(keyDefinition); + const newUserState = new DefaultActiveUserState( + keyDefinition, + this.accountService, + storageService, + this.stateEventRegistrarService, + ); this.cache[cacheKey] = newUserState; return newUserState; } - private buildCacheKey(keyDefinition: UserKeyDefinition) { - return `${this.getLocationString(keyDefinition)}_${keyDefinition.fullName}`; - } - - protected buildActiveUserState(keyDefinition: UserKeyDefinition): ActiveUserState { - return new DefaultActiveUserState( - keyDefinition, - this.accountService, - this.getLocation(keyDefinition.stateDefinition), - ); - } - - protected getLocationString(keyDefinition: UserKeyDefinition): string { - return keyDefinition.stateDefinition.defaultStorageLocation; - } - - protected getLocation(stateDefinition: StateDefinition) { - // The default implementations don't support the client overrides - // it is up to the client to extend this class and add that support - const location = stateDefinition.defaultStorageLocation; - switch (location) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - } + private buildCacheKey(location: string, keyDefinition: UserKeyDefinition) { + return `${location}_${keyDefinition.fullName}`; } } diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts index c1b7efa3b2d..feb9530987b 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts @@ -12,6 +12,7 @@ import { AccountInfo, AccountService } from "../../../auth/abstractions/account. import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; import { StateDefinition } from "../state-definition"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { UserKeyDefinition } from "../user-key-definition"; import { DefaultActiveUserState } from "./default-active-user-state"; @@ -42,6 +43,7 @@ const testKeyDefinition = new UserKeyDefinition(testStateDefinition, describe("DefaultActiveUserState", () => { const accountService = mock(); let diskStorageService: FakeStorageService; + const stateEventRegistrarService = mock(); let activeAccountSubject: BehaviorSubject<{ id: UserId } & AccountInfo>; let userState: DefaultActiveUserState; @@ -50,7 +52,12 @@ describe("DefaultActiveUserState", () => { accountService.activeAccount$ = activeAccountSubject; diskStorageService = new FakeStorageService(); - userState = new DefaultActiveUserState(testKeyDefinition, accountService, diskStorageService); + userState = new DefaultActiveUserState( + testKeyDefinition, + accountService, + diskStorageService, + stateEventRegistrarService, + ); }); const makeUserId = (id: string) => { @@ -391,6 +398,48 @@ describe("DefaultActiveUserState", () => { "No active user at this time.", ); }); + + it.each([null, undefined])( + "should register user key definition when state transitions from null-ish (%s) to non-null", + async (startingValue: TestState | null) => { + diskStorageService.internalUpdateStore({ + "user_00000000-0000-1000-a000-000000000001_fake_fake": startingValue, + }); + + await userState.update(() => ({ array: ["one"], date: new Date() })); + + expect(stateEventRegistrarService.registerEvents).toHaveBeenCalledWith(testKeyDefinition); + }, + ); + + it("should not register user key definition when state has preexisting value", async () => { + diskStorageService.internalUpdateStore({ + "user_00000000-0000-1000-a000-000000000001_fake_fake": { + date: new Date(2019, 1), + array: [], + }, + }); + + await userState.update(() => ({ array: ["one"], date: new Date() })); + + expect(stateEventRegistrarService.registerEvents).not.toHaveBeenCalled(); + }); + + it.each([null, undefined])( + "should not register user key definition when setting value to null-ish (%s) value", + async (updatedValue: TestState | null) => { + diskStorageService.internalUpdateStore({ + "user_00000000-0000-1000-a000-000000000001_fake_fake": { + date: new Date(2019, 1), + array: [], + }, + }); + + await userState.update(() => updatedValue); + + expect(stateEventRegistrarService.registerEvents).not.toHaveBeenCalled(); + }, + ); }); describe("update races", () => { diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.ts b/libs/common/src/platform/state/implementations/default-active-user-state.ts index 82c705ddbd9..4e7f7a0ff67 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.ts @@ -21,6 +21,7 @@ import { AbstractStorageService, ObservableStorageService, } from "../../abstractions/storage.service"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { StateUpdateOptions, populateOptionsWithDefault } from "../state-update-options"; import { UserKeyDefinition } from "../user-key-definition"; import { ActiveUserState, CombinedState, activeMarker } from "../user-state"; @@ -42,6 +43,7 @@ export class DefaultActiveUserState implements ActiveUserState { protected keyDefinition: UserKeyDefinition, private accountService: AccountService, private chosenStorageLocation: AbstractStorageService & ObservableStorageService, + private stateEventRegistrarService: StateEventRegistrarService, ) { this.activeUserId$ = this.accountService.activeAccount$.pipe( // We only care about the UserId but we do want to know about no user as well. @@ -150,6 +152,11 @@ export class DefaultActiveUserState implements ActiveUserState { const newState = configureState(currentState, combinedDependencies); await this.saveToStorage(key, newState); + if (newState != null && currentState == null) { + // Only register this state as something clearable on the first time it saves something + // worth deleting. This is helpful in making sure there is less of a race to adding events. + await this.stateEventRegistrarService.registerEvents(this.keyDefinition); + } return [userId, newState]; } diff --git a/libs/common/src/platform/state/implementations/default-global-state.provider.ts b/libs/common/src/platform/state/implementations/default-global-state.provider.ts index f1b03e36a77..42adf3b9766 100644 --- a/libs/common/src/platform/state/implementations/default-global-state.provider.ts +++ b/libs/common/src/platform/state/implementations/default-global-state.provider.ts @@ -1,25 +1,21 @@ -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "../../abstractions/storage.service"; +import { StorageServiceProvider } from "../../services/storage-service.provider"; import { GlobalState } from "../global-state"; import { GlobalStateProvider } from "../global-state.provider"; import { KeyDefinition } from "../key-definition"; -import { StateDefinition } from "../state-definition"; import { DefaultGlobalState } from "./default-global-state"; export class DefaultGlobalStateProvider implements GlobalStateProvider { private globalStateCache: Record> = {}; - constructor( - protected readonly memoryStorage: AbstractMemoryStorageService & ObservableStorageService, - protected readonly diskStorage: AbstractStorageService & ObservableStorageService, - ) {} + constructor(private storageServiceProvider: StorageServiceProvider) {} get(keyDefinition: KeyDefinition): GlobalState { - const cacheKey = this.buildCacheKey(keyDefinition); + const [location, storageService] = this.storageServiceProvider.get( + keyDefinition.stateDefinition.defaultStorageLocation, + keyDefinition.stateDefinition.storageLocationOverrides, + ); + const cacheKey = this.buildCacheKey(location, keyDefinition); const existingGlobalState = this.globalStateCache[cacheKey]; if (existingGlobalState != null) { // The cast into the actual generic is safe because of rules around key definitions @@ -27,30 +23,13 @@ export class DefaultGlobalStateProvider implements GlobalStateProvider { return existingGlobalState as DefaultGlobalState; } - const newGlobalState = new DefaultGlobalState( - keyDefinition, - this.getLocation(keyDefinition.stateDefinition), - ); + const newGlobalState = new DefaultGlobalState(keyDefinition, storageService); this.globalStateCache[cacheKey] = newGlobalState; return newGlobalState; } - private buildCacheKey(keyDefinition: KeyDefinition) { - return `${this.getLocationString(keyDefinition)}_${keyDefinition.fullName}`; - } - - protected getLocationString(keyDefinition: KeyDefinition): string { - return keyDefinition.stateDefinition.defaultStorageLocation; - } - - protected getLocation(stateDefinition: StateDefinition) { - const location = stateDefinition.defaultStorageLocation; - switch (location) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - } + private buildCacheKey(location: string, keyDefinition: KeyDefinition) { + return `${location}_${keyDefinition.fullName}`; } } diff --git a/libs/common/src/platform/state/implementations/default-single-user-state.provider.ts b/libs/common/src/platform/state/implementations/default-single-user-state.provider.ts index b24a3a58687..9c01c2a76ed 100644 --- a/libs/common/src/platform/state/implementations/default-single-user-state.provider.ts +++ b/libs/common/src/platform/state/implementations/default-single-user-state.provider.ts @@ -1,11 +1,7 @@ import { UserId } from "../../../types/guid"; -import { - AbstractMemoryStorageService, - AbstractStorageService, - ObservableStorageService, -} from "../../abstractions/storage.service"; +import { StorageServiceProvider } from "../../services/storage-service.provider"; import { KeyDefinition } from "../key-definition"; -import { StateDefinition } from "../state-definition"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { UserKeyDefinition, isUserKeyDefinition } from "../user-key-definition"; import { SingleUserState } from "../user-state"; import { SingleUserStateProvider } from "../user-state.provider"; @@ -16,8 +12,8 @@ export class DefaultSingleUserStateProvider implements SingleUserStateProvider { private cache: Record> = {}; constructor( - protected readonly memoryStorage: AbstractMemoryStorageService & ObservableStorageService, - protected readonly diskStorage: AbstractStorageService & ObservableStorageService, + private readonly storageServiceProvider: StorageServiceProvider, + private readonly stateEventRegistrarService: StateEventRegistrarService, ) {} get( @@ -27,7 +23,11 @@ export class DefaultSingleUserStateProvider implements SingleUserStateProvider { if (!isUserKeyDefinition(keyDefinition)) { keyDefinition = UserKeyDefinition.fromBaseKeyDefinition(keyDefinition); } - const cacheKey = this.buildCacheKey(userId, keyDefinition); + const [location, storageService] = this.storageServiceProvider.get( + keyDefinition.stateDefinition.defaultStorageLocation, + keyDefinition.stateDefinition.storageLocationOverrides, + ); + const cacheKey = this.buildCacheKey(location, userId, keyDefinition); const existingUserState = this.cache[cacheKey]; if (existingUserState != null) { // I have to cast out of the unknown generic but this should be safe if rules @@ -35,38 +35,21 @@ export class DefaultSingleUserStateProvider implements SingleUserStateProvider { return existingUserState as SingleUserState; } - const newUserState = this.buildSingleUserState(userId, keyDefinition); + const newUserState = new DefaultSingleUserState( + userId, + keyDefinition, + storageService, + this.stateEventRegistrarService, + ); this.cache[cacheKey] = newUserState; return newUserState; } - private buildCacheKey(userId: UserId, keyDefinition: UserKeyDefinition) { - return `${this.getLocationString(keyDefinition)}_${keyDefinition.fullName}_${userId}`; - } - - protected buildSingleUserState( + private buildCacheKey( + location: string, userId: UserId, - keyDefinition: UserKeyDefinition, - ): SingleUserState { - return new DefaultSingleUserState( - userId, - keyDefinition, - this.getLocation(keyDefinition.stateDefinition), - ); - } - - protected getLocationString(keyDefinition: UserKeyDefinition): string { - return keyDefinition.stateDefinition.defaultStorageLocation; - } - - protected getLocation(stateDefinition: StateDefinition) { - // The default implementations don't support the client overrides - // it is up to the client to extend this class and add that support - switch (stateDefinition.defaultStorageLocation) { - case "disk": - return this.diskStorage; - case "memory": - return this.memoryStorage; - } + keyDefinition: UserKeyDefinition, + ) { + return `${location}_${keyDefinition.fullName}_${userId}`; } } diff --git a/libs/common/src/platform/state/implementations/default-single-user-state.spec.ts b/libs/common/src/platform/state/implementations/default-single-user-state.spec.ts index 4e02c1a654c..b1216c0bafa 100644 --- a/libs/common/src/platform/state/implementations/default-single-user-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-single-user-state.spec.ts @@ -3,6 +3,7 @@ * @jest-environment ../shared/test.environment.ts */ +import { mock } from "jest-mock-extended"; import { firstValueFrom, of } from "rxjs"; import { Jsonify } from "type-fest"; @@ -11,6 +12,7 @@ import { FakeStorageService } from "../../../../spec/fake-storage.service"; import { UserId } from "../../../types/guid"; import { Utils } from "../../misc/utils"; import { StateDefinition } from "../state-definition"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { UserKeyDefinition } from "../user-key-definition"; import { DefaultSingleUserState } from "./default-single-user-state"; @@ -42,11 +44,17 @@ const userKey = testKeyDefinition.buildKey(userId); describe("DefaultSingleUserState", () => { let diskStorageService: FakeStorageService; let userState: DefaultSingleUserState; + const stateEventRegistrarService = mock(); const newData = { date: new Date() }; beforeEach(() => { diskStorageService = new FakeStorageService(); - userState = new DefaultSingleUserState(userId, testKeyDefinition, diskStorageService); + userState = new DefaultSingleUserState( + userId, + testKeyDefinition, + diskStorageService, + stateEventRegistrarService, + ); }); afterEach(() => { @@ -255,6 +263,49 @@ describe("DefaultSingleUserState", () => { expect(emissions).toHaveLength(2); expect(emissions).toEqual(expect.arrayContaining([initialState, newState])); }); + + it.each([null, undefined])( + "should register user key definition when state transitions from null-ish (%s) to non-null", + async (startingValue: TestState | null) => { + const initialState: Record = {}; + initialState[userKey] = startingValue; + + diskStorageService.internalUpdateStore(initialState); + + await userState.update(() => ({ array: ["one"], date: new Date() })); + + expect(stateEventRegistrarService.registerEvents).toHaveBeenCalledWith(testKeyDefinition); + }, + ); + + it("should not register user key definition when state has preexisting value", async () => { + const initialState: Record = {}; + initialState[userKey] = { + date: new Date(2019, 1), + }; + + diskStorageService.internalUpdateStore(initialState); + + await userState.update(() => ({ array: ["one"], date: new Date() })); + + expect(stateEventRegistrarService.registerEvents).not.toHaveBeenCalled(); + }); + + it.each([null, undefined])( + "should not register user key definition when setting value to null-ish (%s) value", + async (updatedValue: TestState | null) => { + const initialState: Record = {}; + initialState[userKey] = { + date: new Date(2019, 1), + }; + + diskStorageService.internalUpdateStore(initialState); + + await userState.update(() => updatedValue); + + expect(stateEventRegistrarService.registerEvents).not.toHaveBeenCalled(); + }, + ); }); describe("update races", () => { diff --git a/libs/common/src/platform/state/implementations/default-single-user-state.ts b/libs/common/src/platform/state/implementations/default-single-user-state.ts index 9523a94965d..b01e0958b5c 100644 --- a/libs/common/src/platform/state/implementations/default-single-user-state.ts +++ b/libs/common/src/platform/state/implementations/default-single-user-state.ts @@ -18,6 +18,7 @@ import { AbstractStorageService, ObservableStorageService, } from "../../abstractions/storage.service"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { StateUpdateOptions, populateOptionsWithDefault } from "../state-update-options"; import { UserKeyDefinition } from "../user-key-definition"; import { CombinedState, SingleUserState } from "../user-state"; @@ -35,6 +36,7 @@ export class DefaultSingleUserState implements SingleUserState { readonly userId: UserId, private keyDefinition: UserKeyDefinition, private chosenLocation: AbstractStorageService & ObservableStorageService, + private stateEventRegistrarService: StateEventRegistrarService, ) { this.storageKey = this.keyDefinition.buildKey(this.userId); const initialStorageGet$ = defer(() => { @@ -100,6 +102,11 @@ export class DefaultSingleUserState implements SingleUserState { const newState = configureState(currentState, combinedDependencies); await this.chosenLocation.save(this.storageKey, newState); + if (newState != null && currentState == null) { + // Only register this state as something clearable on the first time it saves something + // worth deleting. This is helpful in making sure there is less of a race to adding events. + await this.stateEventRegistrarService.registerEvents(this.keyDefinition); + } return newState; } diff --git a/libs/common/src/platform/state/implementations/specific-state.provider.spec.ts b/libs/common/src/platform/state/implementations/specific-state.provider.spec.ts index d6637516723..f3e5ef7f610 100644 --- a/libs/common/src/platform/state/implementations/specific-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/specific-state.provider.spec.ts @@ -1,8 +1,12 @@ +import { mock } from "jest-mock-extended"; + import { mockAccountServiceWith } from "../../../../spec/fake-account-service"; import { FakeStorageService } from "../../../../spec/fake-storage.service"; import { UserId } from "../../../types/guid"; +import { StorageServiceProvider } from "../../services/storage-service.provider"; import { KeyDefinition } from "../key-definition"; import { StateDefinition } from "../state-definition"; +import { StateEventRegistrarService } from "../state-event-registrar.service"; import { DefaultActiveUserState } from "./default-active-user-state"; import { DefaultActiveUserStateProvider } from "./default-active-user-state.provider"; @@ -12,6 +16,9 @@ import { DefaultSingleUserState } from "./default-single-user-state"; import { DefaultSingleUserStateProvider } from "./default-single-user-state.provider"; describe("Specific State Providers", () => { + const storageServiceProvider = mock(); + const stateEventRegistrarService = mock(); + let singleSut: DefaultSingleUserStateProvider; let activeSut: DefaultActiveUserStateProvider; let globalSut: DefaultGlobalStateProvider; @@ -19,19 +26,20 @@ describe("Specific State Providers", () => { const fakeUser1 = "00000000-0000-1000-a000-000000000001" as UserId; beforeEach(() => { + storageServiceProvider.get.mockImplementation((location) => { + return [location, new FakeStorageService()]; + }); + singleSut = new DefaultSingleUserStateProvider( - new FakeStorageService() as any, - new FakeStorageService(), + storageServiceProvider, + stateEventRegistrarService, ); activeSut = new DefaultActiveUserStateProvider( mockAccountServiceWith(null), - new FakeStorageService() as any, - new FakeStorageService(), - ); - globalSut = new DefaultGlobalStateProvider( - new FakeStorageService() as any, - new FakeStorageService(), + storageServiceProvider, + stateEventRegistrarService, ); + globalSut = new DefaultGlobalStateProvider(storageServiceProvider); }); const fakeDiskStateDefinition = new StateDefinition("fake", "disk"); 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 fe56bde7022..e48f2fe0a3e 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 @@ -11,6 +11,7 @@ import { MessagingService } from "../../platform/abstractions/messaging.service" import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { StateService } from "../../platform/abstractions/state.service"; import { Account } from "../../platform/models/domain/account"; +import { StateEventRunnerService } from "../../platform/state"; import { CipherService } from "../../vault/abstractions/cipher.service"; import { CollectionService } from "../../vault/abstractions/collection.service"; import { FolderService } from "../../vault/abstractions/folder/folder.service.abstraction"; @@ -28,6 +29,7 @@ describe("VaultTimeoutService", () => { let stateService: MockProxy; let authService: MockProxy; let vaultTimeoutSettingsService: MockProxy; + let stateEventRunnerService: MockProxy; let lockedCallback: jest.Mock, [userId: string]>; let loggedOutCallback: jest.Mock, [expired: boolean, userId?: string]>; @@ -48,6 +50,7 @@ describe("VaultTimeoutService", () => { stateService = mock(); authService = mock(); vaultTimeoutSettingsService = mock(); + stateEventRunnerService = mock(); lockedCallback = jest.fn(); loggedOutCallback = jest.fn(); @@ -73,6 +76,7 @@ describe("VaultTimeoutService", () => { stateService, authService, vaultTimeoutSettingsService, + stateEventRunnerService, lockedCallback, loggedOutCallback, ); @@ -103,7 +107,8 @@ describe("VaultTimeoutService", () => { return Promise.resolve(accounts[userId]?.authStatus); }); stateService.getIsAuthenticated.mockImplementation((options) => { - return Promise.resolve(accounts[options.userId]?.isAuthenticated); + // Just like actual state service, if no userId is given fallback to active userId + return Promise.resolve(accounts[options.userId ?? globalSetups?.userId]?.isAuthenticated); }); vaultTimeoutSettingsService.getVaultTimeout.mockImplementation((userId) => { @@ -337,4 +342,80 @@ describe("VaultTimeoutService", () => { expectNoAction("1"); }); }); + + describe("lock", () => { + const setupLock = () => { + setupAccounts( + { + user1: { + authStatus: AuthenticationStatus.Unlocked, + isAuthenticated: true, + }, + user2: { + authStatus: AuthenticationStatus.Unlocked, + isAuthenticated: true, + }, + }, + { + userId: "user1", + }, + ); + }; + + it("should call state event runner with currently active user if no user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock(); + + expect(stateEventRunnerService.handleEvent).toHaveBeenCalledWith("lock", "user1"); + }); + + it("should call messaging service locked message if no user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock(); + + // Currently these pass `undefined` (or what they were given) as the userId back + // but we could change this to give the user that was locked (active) to these methods + // so they don't have to get it their own way, but that is a behavioral change that needs + // to be tested. + expect(messagingService.send).toHaveBeenCalledWith("locked", { userId: undefined }); + }); + + it("should call locked callback if no user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock(); + + // Currently these pass `undefined` (or what they were given) as the userId back + // but we could change this to give the user that was locked (active) to these methods + // so they don't have to get it their own way, but that is a behavioral change that needs + // to be tested. + expect(lockedCallback).toHaveBeenCalledWith(undefined); + }); + + it("should call state event runner with user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock("user2"); + + expect(stateEventRunnerService.handleEvent).toHaveBeenCalledWith("lock", "user2"); + }); + + it("should call messaging service locked message with user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock("user2"); + + expect(messagingService.send).toHaveBeenCalledWith("locked", { userId: "user2" }); + }); + + it("should call locked callback with user passed into lock", async () => { + setupLock(); + + await vaultTimeoutService.lock("user2"); + + expect(lockedCallback).toHaveBeenCalledWith("user2"); + }); + }); }); 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 6163005367f..c3270ac2b80 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -11,6 +11,8 @@ import { CryptoService } from "../../platform/abstractions/crypto.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { StateService } from "../../platform/abstractions/state.service"; +import { StateEventRunnerService } from "../../platform/state"; +import { UserId } from "../../types/guid"; import { CipherService } from "../../vault/abstractions/cipher.service"; import { CollectionService } from "../../vault/abstractions/collection.service"; import { FolderService } from "../../vault/abstractions/folder/folder.service.abstraction"; @@ -29,6 +31,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { private stateService: StateService, private authService: AuthService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private stateEventRunnerService: StateEventRunnerService, private lockedCallback: (userId?: string) => Promise = null, private loggedOutCallback: (expired: boolean, userId?: string) => Promise = null, ) {} @@ -81,7 +84,9 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { await this.logOut(userId); } - if (userId == null || userId === (await this.stateService.getUserId())) { + const currentUserId = await this.stateService.getUserId(); + + if (userId == null || userId === currentUserId) { this.searchService.clearIndex(); await this.folderService.clearCache(); await this.collectionService.clearActiveUserCache(); @@ -98,6 +103,11 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { await this.cipherService.clearCache(userId); + await this.stateEventRunnerService.handleEvent("lock", (userId ?? currentUserId) as UserId); + + // FIXME: We should send the userId of the user that was locked, in the case of this method being passed + // undefined then it should give back the currentUserId. Better yet, this method shouldn't take + // an undefined userId at all. All receivers need to be checked for how they handle getting undefined. this.messagingService.send("locked", { userId: userId }); if (this.lockedCallback != null) {