From 92a65b7b368a8dbf55350657674c90169b04c30b Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 31 Jan 2022 14:33:31 -0500 Subject: [PATCH] [bug] Allow for GlobalState to be extended and modified in clients (#646) Some clients have unique global setting defaults (and unique global settings) For example: the web vault defaults to light theme, but most clients with theme support default to system theme. The current way we handle GlobalState is buried in jslib and not easily extendible in clients. To fix this, we need to treat GlobalState as a generic in the StateService and StateMigration service and allow for its extension in those methods and anywhere GlobalState is inited. --- angular/src/services/jslib-services.module.ts | 18 +++++++-- common/src/factories/accountFactory.ts | 13 ++++++ common/src/factories/globalStateFactory.ts | 13 ++++++ common/src/factories/stateFactory.ts | 25 ++++++++++++ common/src/models/domain/account.ts | 12 ------ common/src/models/domain/globalState.ts | 2 +- common/src/models/domain/state.ts | 11 ++++- common/src/services/state.service.ts | 40 ++++++++++++------- common/src/services/stateMigration.service.ts | 13 +++--- 9 files changed, 109 insertions(+), 38 deletions(-) create mode 100644 common/src/factories/accountFactory.ts create mode 100644 common/src/factories/globalStateFactory.ts create mode 100644 common/src/factories/stateFactory.ts diff --git a/angular/src/services/jslib-services.module.ts b/angular/src/services/jslib-services.module.ts index 0133c80287..a08bce2410 100644 --- a/angular/src/services/jslib-services.module.ts +++ b/angular/src/services/jslib-services.module.ts @@ -76,7 +76,11 @@ import { PasswordRepromptService } from "./passwordReprompt.service"; import { UnauthGuardService } from "./unauth-guard.service"; import { ValidationService } from "./validation.service"; -import { Account, AccountFactory } from "jslib-common/models/domain/account"; +import { Account } from "jslib-common/models/domain/account"; +import { GlobalState } from "jslib-common/models/domain/globalState"; + +import { GlobalStateFactory } from "jslib-common/factories/globalStateFactory"; +import { StateFactory } from "jslib-common/factories/stateFactory"; @NgModule({ declarations: [], @@ -338,7 +342,7 @@ import { Account, AccountFactory } from "jslib-common/models/domain/account"; secureStorageService, logService, stateMigrationService, - new AccountFactory(Account) + new StateFactory(GlobalState, Account) ), deps: [ StorageServiceAbstraction, @@ -349,7 +353,15 @@ import { Account, AccountFactory } from "jslib-common/models/domain/account"; }, { provide: StateMigrationServiceAbstraction, - useClass: StateMigrationService, + useFactory: ( + storageService: StorageServiceAbstraction, + secureStorageService: StorageServiceAbstraction + ) => + new StateMigrationService( + storageService, + secureStorageService, + new GlobalStateFactory(GlobalState) + ), deps: [StorageServiceAbstraction, "SECURE_STORAGE"], }, { diff --git a/common/src/factories/accountFactory.ts b/common/src/factories/accountFactory.ts new file mode 100644 index 0000000000..1fe5aee369 --- /dev/null +++ b/common/src/factories/accountFactory.ts @@ -0,0 +1,13 @@ +import { Account } from "../models/domain/account"; + +export class AccountFactory { + private accountConstructor: new (init: Partial) => T; + + constructor(accountConstructor: new (init: Partial) => T) { + this.accountConstructor = accountConstructor; + } + + create(args: Partial) { + return new this.accountConstructor(args); + } +} diff --git a/common/src/factories/globalStateFactory.ts b/common/src/factories/globalStateFactory.ts new file mode 100644 index 0000000000..a2c25c4661 --- /dev/null +++ b/common/src/factories/globalStateFactory.ts @@ -0,0 +1,13 @@ +import { GlobalState } from "../models/domain/globalState"; + +export class GlobalStateFactory { + private globalStateConstructor: new (init: Partial) => T; + + constructor(globalStateConstructor: new (init: Partial) => T) { + this.globalStateConstructor = globalStateConstructor; + } + + create(args?: Partial) { + return new this.globalStateConstructor(args); + } +} diff --git a/common/src/factories/stateFactory.ts b/common/src/factories/stateFactory.ts new file mode 100644 index 0000000000..5df194af67 --- /dev/null +++ b/common/src/factories/stateFactory.ts @@ -0,0 +1,25 @@ +import { Account } from "../models/domain/account"; +import { GlobalState } from "../models/domain/globalState"; +import { AccountFactory } from "./accountFactory"; +import { GlobalStateFactory } from "./globalStateFactory"; + +export class StateFactory { + private globalStateFactory: GlobalStateFactory; + private accountFactory: AccountFactory; + + constructor( + globalStateConstructor: new (init: Partial) => TGlobal, + accountConstructor: new (init: Partial) => TAccount + ) { + this.globalStateFactory = new GlobalStateFactory(globalStateConstructor); + this.accountFactory = new AccountFactory(accountConstructor); + } + + createGlobal(args: Partial): TGlobal { + return this.globalStateFactory.create(args); + } + + createAccount(args: Partial): TAccount { + return this.accountFactory.create(args); + } +} diff --git a/common/src/models/domain/account.ts b/common/src/models/domain/account.ts index 2ca63686f6..51be98be6e 100644 --- a/common/src/models/domain/account.ts +++ b/common/src/models/domain/account.ts @@ -175,15 +175,3 @@ export class Account { }); } } - -export class AccountFactory { - private accountConstructor: new (init: Partial) => T; - - constructor(accountConstructor: new (init: Partial) => T) { - this.accountConstructor = accountConstructor; - } - - create(args: Partial) { - return new this.accountConstructor(args); - } -} diff --git a/common/src/models/domain/globalState.ts b/common/src/models/domain/globalState.ts index 322f6d645f..326ba11ba3 100644 --- a/common/src/models/domain/globalState.ts +++ b/common/src/models/domain/globalState.ts @@ -13,7 +13,7 @@ export class GlobalState { ssoOrganizationIdentifier?: string; ssoState?: string; rememberedEmail?: string; - theme?: ThemeType = ThemeType.Light; + theme?: ThemeType = ThemeType.System; window?: WindowState = new WindowState(); twoFactorToken?: string; disableFavicon?: boolean; diff --git a/common/src/models/domain/state.ts b/common/src/models/domain/state.ts index 16edca2a9a..c87ff6e8fb 100644 --- a/common/src/models/domain/state.ts +++ b/common/src/models/domain/state.ts @@ -1,9 +1,16 @@ import { Account } from "./account"; import { GlobalState } from "./globalState"; -export class State { +export class State< + TAccount extends Account = Account, + TGlobalState extends GlobalState = GlobalState +> { accounts: { [userId: string]: TAccount } = {}; - globals: GlobalState = new GlobalState(); + globals: TGlobalState; activeUserId: string; authenticatedAccounts: string[] = []; + + constructor(globals: TGlobalState) { + this.globals = globals; + } } diff --git a/common/src/services/state.service.ts b/common/src/services/state.service.ts index e3a3c5350f..8ce399e992 100644 --- a/common/src/services/state.service.ts +++ b/common/src/services/state.service.ts @@ -1,6 +1,6 @@ import { StateService as StateServiceAbstraction } from "../abstractions/state.service"; -import { Account, AccountData, AccountFactory } from "../models/domain/account"; +import { Account, AccountData } from "../models/domain/account"; import { LogService } from "../abstractions/log.service"; import { StorageService } from "../abstractions/storage.service"; @@ -39,6 +39,8 @@ import { StateMigrationService } from "../abstractions/stateMigration.service"; import { EnvironmentUrls } from "../models/domain/environmentUrls"; import { WindowState } from "../models/domain/windowState"; +import { StateFactory } from "../factories/stateFactory"; + const keys = { global: "global", authenticatedAccounts: "authenticatedAccounts", @@ -52,13 +54,17 @@ const partialKeys = { masterKey: "_masterkey", }; -export class StateService - implements StateServiceAbstraction +export class StateService< + TAccount extends Account = Account, + TGlobalState extends GlobalState = GlobalState +> implements StateServiceAbstraction { accounts = new BehaviorSubject<{ [userId: string]: TAccount }>({}); activeAccount = new BehaviorSubject(null); - protected state: State = new State(); + protected state: State = new State( + this.createGlobals() + ); private hasBeenInited: boolean = false; @@ -67,7 +73,7 @@ export class StateService protected secureStorageService: StorageService, protected logService: LogService, protected stateMigrationService: StateMigrationService, - protected accountFactory: AccountFactory + protected stateFactory: StateFactory ) {} async init(): Promise { @@ -2086,8 +2092,8 @@ export class StateService ); } - protected async getGlobals(options: StorageOptions): Promise { - let globals: GlobalState; + protected async getGlobals(options: StorageOptions): Promise { + let globals: TGlobalState; if (this.useMemory(options.storageLocation)) { globals = this.getGlobalsFromMemory(); } @@ -2096,28 +2102,28 @@ export class StateService globals = await this.getGlobalsFromDisk(options); } - return globals ?? new GlobalState(); + return globals ?? this.createGlobals(); } - protected async saveGlobals(globals: GlobalState, options: StorageOptions) { + protected async saveGlobals(globals: TGlobalState, options: StorageOptions) { return this.useMemory(options.storageLocation) ? this.saveGlobalsToMemory(globals) : await this.saveGlobalsToDisk(globals, options); } - protected getGlobalsFromMemory(): GlobalState { + protected getGlobalsFromMemory(): TGlobalState { return this.state.globals; } - protected async getGlobalsFromDisk(options: StorageOptions): Promise { - return await this.storageService.get(keys.global, options); + protected async getGlobalsFromDisk(options: StorageOptions): Promise { + return await this.storageService.get(keys.global, options); } - protected saveGlobalsToMemory(globals: GlobalState): void { + protected saveGlobalsToMemory(globals: TGlobalState): void { this.state.globals = globals; } - protected async saveGlobalsToDisk(globals: GlobalState, options: StorageOptions): Promise { + protected async saveGlobalsToDisk(globals: TGlobalState, options: StorageOptions): Promise { if (options.useSecureStorage) { await this.secureStorageService.save(keys.global, globals, options); } else { @@ -2412,7 +2418,11 @@ export class StateService } protected createAccount(init: Partial = null): TAccount { - return this.accountFactory.create(init); + return this.stateFactory.createAccount(init); + } + + protected createGlobals(init: Partial = null): TGlobalState { + return this.stateFactory.createGlobal(init); } protected async deAuthenticateAccount(userId: string) { diff --git a/common/src/services/stateMigration.service.ts b/common/src/services/stateMigration.service.ts index 95059da3d0..c1bf03a7d4 100644 --- a/common/src/services/stateMigration.service.ts +++ b/common/src/services/stateMigration.service.ts @@ -20,6 +20,8 @@ import { ThemeType } from "../enums/themeType"; import { EnvironmentUrls } from "../models/domain/environmentUrls"; +import { GlobalStateFactory } from "../factories/globalStateFactory"; + // Originally (before January 2022) storage was handled as a flat key/value pair store. // With the move to a typed object for state storage these keys should no longer be in use anywhere outside of this migration. const v1Keys: { [key: string]: string } = { @@ -126,10 +128,11 @@ const partialKeys = { masterKey: "_masterkey", }; -export class StateMigrationService { +export class StateMigrationService { constructor( protected storageService: StorageService, - protected secureStorageService: StorageService + protected secureStorageService: StorageService, + protected globalStateFactory: GlobalStateFactory ) {} async needsMigration(): Promise { @@ -174,7 +177,7 @@ export class StateMigrationService { // 1. Check for an existing storage value from the old storage structure OR // 2. Check for a value already set by processes that run before migration OR // 3. Assign the default value - const globals = (await this.get(keys.global)) ?? new GlobalState(); + const globals = (await this.get(keys.global)) ?? this.globalStateFactory.create(); globals.stateVersion = StateVersion.Two; globals.environmentUrls = (await this.get(v1Keys.environmentUrls)) ?? globals.environmentUrls; @@ -438,8 +441,8 @@ export class StateMigrationService { return this.storageService.save(key, value, this.options); } - protected async getGlobals(): Promise { - return await this.get(keys.global); + protected async getGlobals(): Promise { + return await this.get(keys.global); } protected async getCurrentStateVersion(): Promise {