From b2088661093b84a884332c81ed6e8820239f5fec Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 30 Jan 2023 14:04:22 -0500 Subject: [PATCH] [PS-1854] Split folder service back/foreground (#4209) * Split folder service back/foreground Also splits for folderApiService, since that depends on folderService. TODO: this split will need to be done for any dependents of a split service. * Prefer popup-specific services VaultFilterService * Prefer popup-specific services i18n * Prefer popup-specific services configService * StateService is required for browserSync * Add Policy Api Service * Remove unused orgService from PolicyApiService * Fixup missed dependency * Attach cryptography services in popup context * Improve session syncer initialization --- .../browser/src/background/main.background.ts | 6 +- .../session-syncer.spec.ts | 4 + .../session-sync-observable/session-syncer.ts | 22 ++-- .../src/popup/services/services.module.ts | 103 ++++++++++++++---- .../src/services/browser-config.service.ts | 12 ++ .../src/services/browser-i18n.service.ts | 17 +++ .../src/abstractions/config/server-config.ts | 10 ++ .../src/services/config/config.service.ts | 2 +- libs/common/src/services/i18n.service.ts | 2 +- 9 files changed, 142 insertions(+), 36 deletions(-) create mode 100644 apps/browser/src/services/browser-config.service.ts create mode 100644 apps/browser/src/services/browser-i18n.service.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fcde4e7c15..c7fda5d334 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -98,6 +98,7 @@ import { BrowserStateService as StateServiceAbstraction } from "../services/abst import AutofillService from "../services/autofill.service"; import { BrowserEnvironmentService } from "../services/browser-environment.service"; import { BrowserFolderService } from "../services/browser-folder.service"; +import { BrowserI18nService } from "../services/browser-i18n.service"; import { BrowserOrganizationService } from "../services/browser-organization.service"; import { BrowserPolicyService } from "../services/browser-policy.service"; import { BrowserSettingsService } from "../services/browser-settings.service"; @@ -107,7 +108,6 @@ import BrowserLocalStorageService from "../services/browserLocalStorage.service" import BrowserMessagingService from "../services/browserMessaging.service"; import BrowserMessagingPrivateModeBackgroundService from "../services/browserMessagingPrivateModeBackground.service"; import BrowserPlatformUtilsService from "../services/browserPlatformUtils.service"; -import I18nService from "../services/i18n.service"; import { KeyGenerationService } from "../services/keyGeneration.service"; import { LocalBackedSessionStorageService } from "../services/localBackedSessionStorage.service"; import { VaultFilterService } from "../services/vaultFilter.service"; @@ -266,7 +266,7 @@ export default class MainBackground { }, window ); - this.i18nService = new I18nService(BrowserApi.getUILanguage(window)); + this.i18nService = new BrowserI18nService(BrowserApi.getUILanguage(window), this.stateService); this.encryptService = flagEnabled("multithreadDecryption") ? new MultithreadEncryptServiceImplementation( this.cryptoFunctionService, @@ -593,7 +593,7 @@ export default class MainBackground { await this.stateService.init(); await (this.vaultTimeoutService as VaultTimeoutService).init(true); - await (this.i18nService as I18nService).init(); + await (this.i18nService as BrowserI18nService).init(); await (this.eventUploadService as EventUploadService).init(true); await this.runtimeBackground.init(); await this.notificationBackground.init(); diff --git a/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts b/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts index 05d0da7527..2de87b551d 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts @@ -187,6 +187,10 @@ describe("session syncer", () => { jest.spyOn(SyncedItemMetadata, "builder").mockReturnValue(builder); storageService.getBypassCache.mockResolvedValue("test"); + // Expect no circular messaging + await awaitAsync(); + expect(sendMessageSpy).toHaveBeenCalledTimes(0); + await sut.updateFromMessage({ command: `${sessionKey}_update`, id: "different_id" }); await awaitAsync(); diff --git a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts index 8c24ffb185..536e8a4871 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject, concatMap, ReplaySubject, Subject, Subscription } from "rxjs"; +import { BehaviorSubject, concatMap, ReplaySubject, skip, Subject, Subscription } from "rxjs"; import { AbstractMemoryStorageService } from "@bitwarden/common/abstractions/storage.service"; import { Utils } from "@bitwarden/common/misc/utils"; @@ -28,7 +28,7 @@ export class SessionSyncer { } } - init() { + async init() { switch (this.subject.constructor) { case ReplaySubject: // ignore all updates currently in the buffer @@ -41,22 +41,24 @@ export class SessionSyncer { break; } - this.observe(); + await this.observe(); // must be synchronous - this.memoryStorageService.has(this.metaData.sessionKey).then((hasInSessionMemory) => { - if (hasInSessionMemory) { - this.update(); - } - }); + const hasInSessionMemory = await this.memoryStorageService.has(this.metaData.sessionKey); + if (hasInSessionMemory) { + await this.update(); + } this.listenForUpdates(); } - private observe() { + private async observe() { + const stream = this.subject.pipe(skip(this.ignoreNUpdates)); + this.ignoreNUpdates = 0; + // This may be a memory leak. // There is no good time to unsubscribe from this observable. Hopefully Manifest V3 clears memory from temporary // contexts. If so, this is handled by destruction of the context. - this.subscription = this.subject + this.subscription = stream .pipe( concatMap(async (next) => { if (this.ignoreNUpdates > 0) { diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 677e8648bc..5d9da20d2a 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -12,8 +12,11 @@ import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/abstractions/auth.service"; import { CipherService } from "@bitwarden/common/abstractions/cipher.service"; import { CollectionService } from "@bitwarden/common/abstractions/collection.service"; +import { ConfigApiServiceAbstraction } from "@bitwarden/common/abstractions/config/config-api.service.abstraction"; +import { ConfigServiceAbstraction } from "@bitwarden/common/abstractions/config/config.service.abstraction"; import { CryptoService } from "@bitwarden/common/abstractions/crypto.service"; import { CryptoFunctionService } from "@bitwarden/common/abstractions/cryptoFunction.service"; +import { EncryptService } from "@bitwarden/common/abstractions/encrypt.service"; import { EnvironmentService } from "@bitwarden/common/abstractions/environment.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; @@ -21,8 +24,11 @@ import { ExportService } from "@bitwarden/common/abstractions/export.service"; import { FileDownloadService } from "@bitwarden/common/abstractions/fileDownload/fileDownload.service"; import { FileUploadService } from "@bitwarden/common/abstractions/fileUpload.service"; import { FolderApiServiceAbstraction } from "@bitwarden/common/abstractions/folder/folder-api.service.abstraction"; -import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction"; -import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; +import { + FolderService, + InternalFolderService, +} from "@bitwarden/common/abstractions/folder/folder.service.abstraction"; +import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/abstractions/i18n.service"; import { KeyConnectorService } from "@bitwarden/common/abstractions/keyConnector.service"; import { LogService as LogServiceAbstraction } from "@bitwarden/common/abstractions/log.service"; import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/abstractions/login.service"; @@ -33,12 +39,18 @@ import { PasswordGenerationService } from "@bitwarden/common/abstractions/passwo import { PasswordRepromptService as PasswordRepromptServiceAbstraction } from "@bitwarden/common/abstractions/passwordReprompt.service"; import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/abstractions/policy/policy-api.service.abstraction"; -import { PolicyService } from "@bitwarden/common/abstractions/policy/policy.service.abstraction"; +import { + InternalPolicyService, + PolicyService, +} from "@bitwarden/common/abstractions/policy/policy.service.abstraction"; import { ProviderService } from "@bitwarden/common/abstractions/provider.service"; import { SearchService as SearchServiceAbstraction } from "@bitwarden/common/abstractions/search.service"; import { SendService } from "@bitwarden/common/abstractions/send.service"; import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; -import { StateService as BaseStateServiceAbstraction } from "@bitwarden/common/abstractions/state.service"; +import { + StateService as BaseStateServiceAbstraction, + StateService, +} from "@bitwarden/common/abstractions/state.service"; import { StateMigrationService } from "@bitwarden/common/abstractions/stateMigration.service"; import { AbstractMemoryStorageService, @@ -56,7 +68,10 @@ import { StateFactory } from "@bitwarden/common/factories/stateFactory"; import { GlobalState } from "@bitwarden/common/models/domain/global-state"; import { AuthService } from "@bitwarden/common/services/auth.service"; import { ConsoleLogService } from "@bitwarden/common/services/consoleLog.service"; +import { ContainerService } from "@bitwarden/common/services/container.service"; +import { FolderApiService } from "@bitwarden/common/services/folder/folder-api.service"; import { LoginService } from "@bitwarden/common/services/login.service"; +import { PolicyApiService } from "@bitwarden/common/services/policy/policy-api.service"; import { SearchService } from "@bitwarden/common/services/search.service"; import MainBackground from "../../background/main.background"; @@ -64,7 +79,10 @@ import { BrowserApi } from "../../browser/browserApi"; import { Account } from "../../models/account"; import { AutofillService } from "../../services/abstractions/autofill.service"; import { BrowserStateService as StateServiceAbstraction } from "../../services/abstractions/browser-state.service"; +import { BrowserConfigService } from "../../services/browser-config.service"; import { BrowserEnvironmentService } from "../../services/browser-environment.service"; +import { BrowserFolderService } from "../../services/browser-folder.service"; +import { BrowserI18nService } from "../../services/browser-i18n.service"; import { BrowserOrganizationService } from "../../services/browser-organization.service"; import { BrowserPolicyService } from "../../services/browser-policy.service"; import { BrowserSettingsService } from "../../services/browser-settings.service"; @@ -108,7 +126,7 @@ function getBgService(service: keyof MainBackground) { DebounceNavigationService, { provide: LOCALE_ID, - useFactory: () => getBgService("i18nService")().translationLocale, + useFactory: () => getBgService("i18nService")().translationLocale, deps: [], }, { @@ -143,7 +161,7 @@ function getBgService(service: keyof MainBackground) { useFactory: ( cipherService: CipherService, logService: ConsoleLogService, - i18nService: I18nService + i18nService: I18nServiceAbstraction ) => { return new PopupSearchService( getBgService("searchService")(), @@ -152,7 +170,7 @@ function getBgService(service: keyof MainBackground) { i18nService ); }, - deps: [CipherService, LogServiceAbstraction, I18nService], + deps: [CipherService, LogServiceAbstraction, I18nServiceAbstraction], }, { provide: AuditService, useFactory: getBgService("auditService"), deps: [] }, { @@ -168,13 +186,26 @@ function getBgService(service: keyof MainBackground) { }, { provide: FolderService, - useFactory: getBgService("folderService"), - deps: [], + useFactory: ( + cryptoService: CryptoService, + i18nService: I18nServiceAbstraction, + cipherService: CipherService, + stateService: StateServiceAbstraction + ) => { + return new BrowserFolderService(cryptoService, i18nService, cipherService, stateService); + }, + deps: [CryptoService, I18nServiceAbstraction, CipherService, StateServiceAbstraction], + }, + { + provide: InternalFolderService, + useExisting: FolderService, }, { provide: FolderApiServiceAbstraction, - useFactory: getBgService("folderApiService"), - deps: [], + useFactory: (folderService: InternalFolderService, apiService: ApiService) => { + return new FolderApiService(folderService, apiService); + }, + deps: [InternalFolderService, ApiService], }, { provide: CollectionService, @@ -197,8 +228,22 @@ function getBgService(service: keyof MainBackground) { }, { provide: TotpService, useFactory: getBgService("totpService"), deps: [] }, { provide: TokenService, useFactory: getBgService("tokenService"), deps: [] }, - { provide: I18nService, useFactory: getBgService("i18nService"), deps: [] }, - { provide: CryptoService, useFactory: getBgService("cryptoService"), deps: [] }, + { + provide: I18nServiceAbstraction, + useFactory: (stateService: BrowserStateService) => { + return new BrowserI18nService(BrowserApi.getUILanguage(window), stateService); + }, + deps: [StateService], + }, + { + provide: CryptoService, + useFactory: (encryptService: EncryptService) => { + const cryptoService = getBgService("cryptoService")(); + new ContainerService(cryptoService, encryptService).attachToGlobal(self); + return cryptoService; + }, + deps: [EncryptService], + }, { provide: EventUploadService, useFactory: getBgService("eventUploadService"), @@ -221,8 +266,14 @@ function getBgService(service: keyof MainBackground) { }, { provide: PolicyApiServiceAbstraction, - useFactory: getBgService("policyApiService"), - deps: [], + useFactory: ( + policyService: InternalPolicyService, + apiService: ApiService, + stateService: StateService + ) => { + return new PolicyApiService(policyService, apiService, stateService); + }, + deps: [InternalPolicyService, ApiService, StateService], }, { provide: PlatformUtilsService, @@ -296,17 +347,22 @@ function getBgService(service: keyof MainBackground) { }, { provide: VaultFilterService, - useFactory: () => { + useFactory: ( + stateService: StateServiceAbstraction, + organizationService: OrganizationService, + folderService: FolderService, + policyService: PolicyService + ) => { return new VaultFilterService( - getBgService("stateService")(), - getBgService("organizationService")(), - getBgService("folderService")(), + stateService, + organizationService, + folderService, getBgService("cipherService")(), getBgService("collectionService")(), - getBgService("policyService")() + policyService ); }, - deps: [], + deps: [StateServiceAbstraction, OrganizationService, FolderService, PolicyService], }, { provide: ProviderService, @@ -388,6 +444,11 @@ function getBgService(service: keyof MainBackground) { }, deps: [StateServiceAbstraction, PlatformUtilsService], }, + { + provide: ConfigServiceAbstraction, + useClass: BrowserConfigService, + deps: [StateServiceAbstraction, ConfigApiServiceAbstraction], + }, ], }) export class ServicesModule {} diff --git a/apps/browser/src/services/browser-config.service.ts b/apps/browser/src/services/browser-config.service.ts new file mode 100644 index 0000000000..14aae684bd --- /dev/null +++ b/apps/browser/src/services/browser-config.service.ts @@ -0,0 +1,12 @@ +import { BehaviorSubject } from "rxjs"; + +import { ServerConfig } from "@bitwarden/common/abstractions/config/server-config"; +import { ConfigService } from "@bitwarden/common/services/config/config.service"; + +import { browserSession, sessionSync } from "../decorators/session-sync-observable"; + +@browserSession +export class BrowserConfigService extends ConfigService { + @sessionSync({ initializer: ServerConfig.fromJSON }) + protected _serverConfig: BehaviorSubject; +} diff --git a/apps/browser/src/services/browser-i18n.service.ts b/apps/browser/src/services/browser-i18n.service.ts new file mode 100644 index 0000000000..1af1e54502 --- /dev/null +++ b/apps/browser/src/services/browser-i18n.service.ts @@ -0,0 +1,17 @@ +import { ReplaySubject } from "rxjs"; + +import { StateService } from "@bitwarden/common/abstractions/state.service"; + +import { browserSession, sessionSync } from "../decorators/session-sync-observable"; + +import I18nService from "./i18n.service"; + +@browserSession +export class BrowserI18nService extends I18nService { + @sessionSync({ ctor: String }) + protected _locale: ReplaySubject; + + constructor(systemLanguage: string, private stateService: StateService) { + super(systemLanguage); + } +} diff --git a/libs/common/src/abstractions/config/server-config.ts b/libs/common/src/abstractions/config/server-config.ts index c0334176d0..15dfa8c978 100644 --- a/libs/common/src/abstractions/config/server-config.ts +++ b/libs/common/src/abstractions/config/server-config.ts @@ -1,3 +1,5 @@ +import { Jsonify } from "type-fest"; + import { ServerConfigData, ThirdPartyServerConfigData, @@ -37,4 +39,12 @@ export class ServerConfig { expiresSoon(): boolean { return this.getAgeInMilliseconds() >= eighteenHoursInMilliseconds; } + + static fromJSON(obj: Jsonify): ServerConfig { + if (obj == null) { + return null; + } + + return new ServerConfig(obj); + } } diff --git a/libs/common/src/services/config/config.service.ts b/libs/common/src/services/config/config.service.ts index 123d08ace5..e82d3ccb31 100644 --- a/libs/common/src/services/config/config.service.ts +++ b/libs/common/src/services/config/config.service.ts @@ -7,7 +7,7 @@ import { StateService } from "../../abstractions/state.service"; import { ServerConfigData } from "../../models/data/server-config.data"; export class ConfigService implements ConfigServiceAbstraction { - private _serverConfig = new BehaviorSubject(null); + protected _serverConfig = new BehaviorSubject(null); serverConfig$ = this._serverConfig.asObservable(); constructor( diff --git a/libs/common/src/services/i18n.service.ts b/libs/common/src/services/i18n.service.ts index ce88f7f706..022fe194dd 100644 --- a/libs/common/src/services/i18n.service.ts +++ b/libs/common/src/services/i18n.service.ts @@ -3,7 +3,7 @@ import { Observable, ReplaySubject } from "rxjs"; import { I18nService as I18nServiceAbstraction } from "../abstractions/i18n.service"; export class I18nService implements I18nServiceAbstraction { - private _locale = new ReplaySubject(1); + protected _locale = new ReplaySubject(1); locale$: Observable = this._locale.asObservable(); // First locale is the default (English) supportedTranslationLocales: string[] = ["en"];