diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 124cf6a78d8..2c48a8563e7 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -652,7 +652,7 @@ export default class MainBackground { this.kdfConfigService, ); - this.appIdService = new AppIdService(this.globalStateProvider); + this.appIdService = new AppIdService(this.storageService, this.logService); this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); this.organizationService = new OrganizationService(this.stateProvider); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index fb77e41a4b6..be1b1cc9e2b 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -413,7 +413,7 @@ export class ServiceContainer { this.kdfConfigService, ); - this.appIdService = new AppIdService(this.globalStateProvider); + this.appIdService = new AppIdService(this.storageService, this.logService); const customUserAgent = "Bitwarden_CLI/" + diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index 460af8623e6..887c8fb626a 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -32,6 +32,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { ClientType } from "@bitwarden/common/enums"; +import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; @@ -41,6 +42,7 @@ import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwar import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { BiometricsService } from "@bitwarden/common/platform/biometrics/biometric.service"; import { ThemeType } from "@bitwarden/common/platform/enums"; +import { AppIdService as DefaultAppIdService } from "@bitwarden/common/platform/services/app-id.service"; import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; // eslint-disable-next-line import/no-restricted-paths -- Implementation for memory storage import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service"; @@ -207,6 +209,11 @@ const safeProviders: SafeProvider[] = [ InternalUserDecryptionOptionsServiceAbstraction, ], }), + safeProvider({ + provide: AppIdService, + useClass: DefaultAppIdService, + deps: [OBSERVABLE_DISK_LOCAL_STORAGE, LogService], + }), ]; @NgModule({ diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 7b9c33f3d0a..ab52ab8e433 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -370,7 +370,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: AppIdServiceAbstraction, useClass: AppIdService, - deps: [GlobalStateProvider], + deps: [OBSERVABLE_DISK_STORAGE, LogService], }), safeProvider({ provide: AuditServiceAbstraction, diff --git a/libs/common/src/platform/abstractions/app-id.service.ts b/libs/common/src/platform/abstractions/app-id.service.ts index c2c1a23ef5e..7fb02ee9e09 100644 --- a/libs/common/src/platform/abstractions/app-id.service.ts +++ b/libs/common/src/platform/abstractions/app-id.service.ts @@ -1,8 +1,4 @@ -import { Observable } from "rxjs"; - export abstract class AppIdService { - abstract appId$: Observable; - abstract anonymousAppId$: Observable; abstract getAppId(): Promise; abstract getAnonymousAppId(): Promise; } diff --git a/libs/common/src/platform/services/app-id.service.spec.ts b/libs/common/src/platform/services/app-id.service.spec.ts index 62806204db0..23691769bf2 100644 --- a/libs/common/src/platform/services/app-id.service.spec.ts +++ b/libs/common/src/platform/services/app-id.service.spec.ts @@ -1,19 +1,18 @@ -import { FakeGlobalState, FakeGlobalStateProvider, ObservableTracker } from "../../../spec"; +import { mock } from "jest-mock-extended"; + +import { FakeStorageService } from "../../../spec"; +import { LogService } from "../abstractions/log.service"; import { Utils } from "../misc/utils"; import { ANONYMOUS_APP_ID_KEY, APP_ID_KEY, AppIdService } from "./app-id.service"; describe("AppIdService", () => { - let globalStateProvider: FakeGlobalStateProvider; - let appIdState: FakeGlobalState; - let anonymousAppIdState: FakeGlobalState; + let fakeStorageService: FakeStorageService; let sut: AppIdService; beforeEach(() => { - globalStateProvider = new FakeGlobalStateProvider(); - appIdState = globalStateProvider.getFake(APP_ID_KEY); - anonymousAppIdState = globalStateProvider.getFake(ANONYMOUS_APP_ID_KEY); - sut = new AppIdService(globalStateProvider); + fakeStorageService = new FakeStorageService(); + sut = new AppIdService(fakeStorageService, mock()); }); afterEach(() => { @@ -22,7 +21,7 @@ describe("AppIdService", () => { describe("getAppId", () => { it("returns the existing appId when it exists", async () => { - appIdState.stateSubject.next("existingAppId"); + fakeStorageService.internalUpdateStore({ [APP_ID_KEY]: "existingAppId" }); const appId = await sut.getAppId(); @@ -30,7 +29,7 @@ describe("AppIdService", () => { }); it("creates a new appId only once", async () => { - appIdState.stateSubject.next(null); + fakeStorageService.internalUpdateStore({ [APP_ID_KEY]: null }); const appIds: string[] = []; const promises = [async () => appIds.push(await sut.getAppId())]; @@ -41,7 +40,7 @@ describe("AppIdService", () => { }); it.each([null, undefined])("returns a new appId when %s", async (value) => { - appIdState.stateSubject.next(value); + fakeStorageService.internalUpdateStore({ [APP_ID_KEY]: value }); const appId = await sut.getAppId(); @@ -49,27 +48,17 @@ describe("AppIdService", () => { }); it.each([null, undefined])("stores the new guid when %s", async (value) => { - appIdState.stateSubject.next(value); + fakeStorageService.internalUpdateStore({ [APP_ID_KEY]: value }); const appId = await sut.getAppId(); - expect(appIdState.nextMock).toHaveBeenCalledWith(appId); - }); - - it("emits only once when creating a new appId", async () => { - appIdState.stateSubject.next(null); - - const tracker = new ObservableTracker(sut.appId$); - const appId = await sut.getAppId(); - - expect(tracker.emissions).toEqual([appId]); - await expect(tracker.pauseUntilReceived(2, 50)).rejects.toThrow("Timeout exceeded"); + expect(fakeStorageService.mock.save).toHaveBeenCalledWith(APP_ID_KEY, appId, undefined); }); }); describe("getAnonymousAppId", () => { it("returns the existing appId when it exists", async () => { - anonymousAppIdState.stateSubject.next("existingAppId"); + fakeStorageService.internalUpdateStore({ [ANONYMOUS_APP_ID_KEY]: "existingAppId" }); const appId = await sut.getAnonymousAppId(); @@ -77,7 +66,7 @@ describe("AppIdService", () => { }); it("creates a new anonymousAppId only once", async () => { - anonymousAppIdState.stateSubject.next(null); + fakeStorageService.internalUpdateStore({ [ANONYMOUS_APP_ID_KEY]: null }); const appIds: string[] = []; const promises = [async () => appIds.push(await sut.getAnonymousAppId())]; @@ -88,7 +77,7 @@ describe("AppIdService", () => { }); it.each([null, undefined])("returns a new appId when it does not exist", async (value) => { - anonymousAppIdState.stateSubject.next(value); + fakeStorageService.internalUpdateStore({ [ANONYMOUS_APP_ID_KEY]: value }); const appId = await sut.getAnonymousAppId(); @@ -98,22 +87,16 @@ describe("AppIdService", () => { it.each([null, undefined])( "stores the new guid when it an existing one is not found", async (value) => { - anonymousAppIdState.stateSubject.next(value); + fakeStorageService.internalUpdateStore({ [ANONYMOUS_APP_ID_KEY]: value }); const appId = await sut.getAnonymousAppId(); - expect(anonymousAppIdState.nextMock).toHaveBeenCalledWith(appId); + expect(fakeStorageService.mock.save).toHaveBeenCalledWith( + ANONYMOUS_APP_ID_KEY, + appId, + undefined, + ); }, ); - - it("emits only once when creating a new anonymousAppId", async () => { - anonymousAppIdState.stateSubject.next(null); - - const tracker = new ObservableTracker(sut.anonymousAppId$); - const appId = await sut.getAnonymousAppId(); - - expect(tracker.emissions).toEqual([appId]); - await expect(tracker.pauseUntilReceived(2, 50)).rejects.toThrow("Timeout exceeded"); - }); }); }); diff --git a/libs/common/src/platform/services/app-id.service.ts b/libs/common/src/platform/services/app-id.service.ts index 3578d96c409..97d7876743b 100644 --- a/libs/common/src/platform/services/app-id.service.ts +++ b/libs/common/src/platform/services/app-id.service.ts @@ -1,59 +1,46 @@ -import { Observable, concatMap, distinctUntilChanged, firstValueFrom, share } from "rxjs"; - import { AppIdService as AppIdServiceAbstraction } from "../abstractions/app-id.service"; +import { LogService } from "../abstractions/log.service"; +import { AbstractStorageService } from "../abstractions/storage.service"; import { Utils } from "../misc/utils"; -import { APPLICATION_ID_DISK, GlobalStateProvider, KeyDefinition } from "../state"; -export const APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "appId", { - deserializer: (value: string) => value, - cleanupDelayMs: 0, - debug: { - enableRetrievalLogging: true, - enableUpdateLogging: true, - }, -}); -export const ANONYMOUS_APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "anonymousAppId", { - deserializer: (value: string) => value, -}); +// export const APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "appId", { +// deserializer: (value: string) => value, +// cleanupDelayMs: 0, +// debug: { +// enableRetrievalLogging: true, +// enableUpdateLogging: true, +// }, +// }); +// export const ANONYMOUS_APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "anonymousAppId", { +// deserializer: (value: string) => value, +// }); + +export const APP_ID_KEY = "global_applicationId_appId"; +export const ANONYMOUS_APP_ID_KEY = "global_applicationId_appId"; export class AppIdService implements AppIdServiceAbstraction { - appId$: Observable; - anonymousAppId$: Observable; - - constructor(globalStateProvider: GlobalStateProvider) { - const appIdState = globalStateProvider.get(APP_ID_KEY); - const anonymousAppIdState = globalStateProvider.get(ANONYMOUS_APP_ID_KEY); - this.appId$ = appIdState.state$.pipe( - concatMap(async (appId) => { - if (!appId) { - return await appIdState.update(() => Utils.newGuid(), { - shouldUpdate: (v) => v == null, - }); - } - return appId; - }), - distinctUntilChanged(), - share(), - ); - this.anonymousAppId$ = anonymousAppIdState.state$.pipe( - concatMap(async (appId) => { - if (!appId) { - return await anonymousAppIdState.update(() => Utils.newGuid(), { - shouldUpdate: (v) => v == null, - }); - } - return appId; - }), - distinctUntilChanged(), - share(), - ); - } + constructor( + private readonly storageService: AbstractStorageService, + private readonly logService: LogService, + ) {} async getAppId(): Promise { - return await firstValueFrom(this.appId$); + this.logService.info("Retrieving application id"); + return await this.getEnsuredValue(APP_ID_KEY); } async getAnonymousAppId(): Promise { - return await firstValueFrom(this.anonymousAppId$); + return await this.getEnsuredValue(ANONYMOUS_APP_ID_KEY); + } + + private async getEnsuredValue(key: string) { + let value = await this.storageService.get(key); + + if (value == null) { + value = Utils.newGuid(); + await this.storageService.save(key, value); + } + + return value; } }