From cdcd1809f0aaa2390c4a3dcd0f49776b3415c16b Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Oct 2023 15:41:01 -0400 Subject: [PATCH] Expand account service (#6622) * Define account service observable responsibilities * Establish account service observables and update methods * Update Account Service observables from state service This is a temporary stop-gap to avoid needing to reroute all account activity and status changes through the account service. That can be done as part of the breakup of state service. * Add matchers for Observable emissions * Fix null active account * Test account service * Transition account status to account info * Remove unused matchers * Remove duplicate class * Replay active account for late subscriptions * Add factories for background services * Fix state service for web * Allow for optional messaging This is a temporary hack until the flow of account status can be reversed from state -> account to account -> state. The foreground account service will still logout, it's just the background one cannot send messages * Fix add account logic * Do not throw on recoverable errors It's possible that duplicate entries exist in `activeAccounts` exist in the wild. If we throw on adding a duplicate account this will cause applications to be unusable until duplicates are removed it is not necessary to throw since this is recoverable. with some potential loss in current account status * Add documentation to abstraction * Update libs/common/spec/utils.ts Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> * Fix justin's comment :fist-shake: --------- Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> --- .../account-service.factory.ts | 38 ++++ .../browser/src/background/main.background.ts | 7 +- .../state-service.factory.ts | 8 +- .../services/browser-state.service.spec.ts | 4 + .../services/browser-state.service.ts | 3 + .../src/popup/services/services.module.ts | 15 +- apps/cli/src/bw.ts | 8 +- .../src/app/services/services.module.ts | 2 + apps/desktop/src/main.ts | 2 + apps/web/src/app/core/state/state.service.ts | 3 + .../src/services/jslib-services.module.ts | 1 + libs/common/spec/utils.ts | 38 ++++ .../src/auth/abstractions/account.service.ts | 48 ++++- .../src/auth/services/account.service.spec.ts | 181 ++++++++++++++++++ .../src/auth/services/account.service.ts | 81 +++++++- .../src/platform/services/state.service.ts | 30 ++- libs/common/src/types/guid.d.ts | 5 + 17 files changed, 464 insertions(+), 10 deletions(-) create mode 100644 apps/browser/src/auth/background/service-factories/account-service.factory.ts create mode 100644 libs/common/src/auth/services/account.service.spec.ts create mode 100644 libs/common/src/types/guid.d.ts diff --git a/apps/browser/src/auth/background/service-factories/account-service.factory.ts b/apps/browser/src/auth/background/service-factories/account-service.factory.ts new file mode 100644 index 00000000000..759ff8efdd5 --- /dev/null +++ b/apps/browser/src/auth/background/service-factories/account-service.factory.ts @@ -0,0 +1,38 @@ +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; + +import { + FactoryOptions, + CachedServices, + factory, +} from "../../../platform/background/service-factories/factory-options"; +import { + LogServiceInitOptions, + logServiceFactory, +} from "../../../platform/background/service-factories/log-service.factory"; +import { + MessagingServiceInitOptions, + messagingServiceFactory, +} from "../../../platform/background/service-factories/messaging-service.factory"; + +type AccountServiceFactoryOptions = FactoryOptions; + +export type AccountServiceInitOptions = AccountServiceFactoryOptions & + MessagingServiceInitOptions & + LogServiceInitOptions; + +export function accountServiceFactory( + cache: { accountService?: AccountService } & CachedServices, + opts: AccountServiceInitOptions +): Promise { + return factory( + cache, + "accountService", + opts, + async () => + new AccountServiceImplementation( + await messagingServiceFactory(cache, opts), + await logServiceFactory(cache, opts) + ) + ); +} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9d4cba04e7c..5c47c1aaf9e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -14,6 +14,7 @@ import { InternalPolicyService as InternalPolicyServiceAbstraction } from "@bitw import { ProviderService as ProviderServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; +import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthRequestCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-crypto.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; @@ -24,6 +25,7 @@ import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/ import { TwoFactorService as TwoFactorServiceAbstraction } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { UserVerificationApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification-api.service.abstraction"; import { UserVerificationService as UserVerificationServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthRequestCryptoServiceImplementation } from "@bitwarden/common/auth/services/auth-request-crypto.service.implementation"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { DeviceTrustCryptoService } from "@bitwarden/common/auth/services/device-trust-crypto.service.implementation"; @@ -225,6 +227,7 @@ export default class MainBackground { authRequestCryptoService: AuthRequestCryptoServiceAbstraction; popupUtilsService: PopupUtilsService; browserPopoutWindowService: BrowserPopoutWindowService; + accountService: AccountServiceAbstraction; // Passed to the popup for Safari to workaround issues with theming, downloading, etc. backgroundWindow = window; @@ -279,12 +282,14 @@ export default class MainBackground { new KeyGenerationService(this.cryptoFunctionService) ) : new MemoryStorageService(); + this.accountService = new AccountServiceImplementation(this.messagingService, this.logService); this.stateService = new BrowserStateService( this.storageService, this.secureStorageService, this.memoryStorageService, this.logService, - new StateFactory(GlobalState, Account) + new StateFactory(GlobalState, Account), + this.accountService ); this.platformUtilsService = new BrowserPlatformUtilsService( this.messagingService, diff --git a/apps/browser/src/platform/background/service-factories/state-service.factory.ts b/apps/browser/src/platform/background/service-factories/state-service.factory.ts index 7d3aaf9b6f3..31a0316c09a 100644 --- a/apps/browser/src/platform/background/service-factories/state-service.factory.ts +++ b/apps/browser/src/platform/background/service-factories/state-service.factory.ts @@ -1,6 +1,10 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; +import { + accountServiceFactory, + AccountServiceInitOptions, +} from "../../../auth/background/service-factories/account-service.factory"; import { Account } from "../../../models/account"; import { BrowserStateService } from "../../services/browser-state.service"; @@ -26,7 +30,8 @@ export type StateServiceInitOptions = StateServiceFactoryOptions & DiskStorageServiceInitOptions & SecureStorageServiceInitOptions & MemoryStorageServiceInitOptions & - LogServiceInitOptions; + LogServiceInitOptions & + AccountServiceInitOptions; export async function stateServiceFactory( cache: { stateService?: BrowserStateService } & CachedServices, @@ -43,6 +48,7 @@ export async function stateServiceFactory( await memoryStorageServiceFactory(cache, opts), await logServiceFactory(cache, opts), opts.stateServiceOptions.stateFactory, + await accountServiceFactory(cache, opts), opts.stateServiceOptions.useAccountCache ) ); diff --git a/apps/browser/src/platform/services/browser-state.service.spec.ts b/apps/browser/src/platform/services/browser-state.service.spec.ts index 0712416172c..c63aae74036 100644 --- a/apps/browser/src/platform/services/browser-state.service.spec.ts +++ b/apps/browser/src/platform/services/browser-state.service.spec.ts @@ -1,5 +1,6 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractMemoryStorageService, @@ -27,6 +28,7 @@ describe("Browser State Service", () => { let logService: MockProxy; let stateFactory: MockProxy>; let useAccountCache: boolean; + let accountService: MockProxy; let state: State; const userId = "userId"; @@ -38,6 +40,7 @@ describe("Browser State Service", () => { diskStorageService = mock(); logService = mock(); stateFactory = mock(); + accountService = mock(); // turn off account cache for tests useAccountCache = false; @@ -62,6 +65,7 @@ describe("Browser State Service", () => { memoryStorageService, logService, stateFactory, + accountService, useAccountCache ); }); diff --git a/apps/browser/src/platform/services/browser-state.service.ts b/apps/browser/src/platform/services/browser-state.service.ts index ec6851beb8f..ae5abb8a897 100644 --- a/apps/browser/src/platform/services/browser-state.service.ts +++ b/apps/browser/src/platform/services/browser-state.service.ts @@ -1,5 +1,6 @@ import { BehaviorSubject } from "rxjs"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractStorageService, @@ -42,6 +43,7 @@ export class BrowserStateService memoryStorageService: AbstractMemoryStorageService, logService: LogService, stateFactory: StateFactory, + accountService: AccountService, useAccountCache = true ) { super( @@ -50,6 +52,7 @@ export class BrowserStateService memoryStorageService, logService, stateFactory, + accountService, useAccountCache ); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 2622b8ef13b..06235809458 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -23,6 +23,7 @@ import { } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; +import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthRequestCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-crypto.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; @@ -453,17 +454,25 @@ function getBgService(service: keyof MainBackground) { storageService: AbstractStorageService, secureStorageService: AbstractStorageService, memoryStorageService: AbstractMemoryStorageService, - logService: LogServiceAbstraction + logService: LogServiceAbstraction, + accountService: AccountServiceAbstraction ) => { return new BrowserStateService( storageService, secureStorageService, memoryStorageService, logService, - new StateFactory(GlobalState, Account) + new StateFactory(GlobalState, Account), + accountService ); }, - deps: [AbstractStorageService, SECURE_STORAGE, MEMORY_STORAGE, LogServiceAbstraction], + deps: [ + AbstractStorageService, + SECURE_STORAGE, + MEMORY_STORAGE, + LogServiceAbstraction, + AccountServiceAbstraction, + ], }, { provide: UsernameGenerationServiceAbstraction, diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index ffaec215e26..b63dda690f7 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -12,9 +12,11 @@ import { OrganizationService } from "@bitwarden/common/admin-console/services/or import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthRequestCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-crypto.service.abstraction"; import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction"; +import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthRequestCryptoServiceImplementation } from "@bitwarden/common/auth/services/auth-request-crypto.service.implementation"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { DeviceTrustCryptoService } from "@bitwarden/common/auth/services/device-trust-crypto.service.implementation"; @@ -152,6 +154,7 @@ export class Main { authRequestCryptoService: AuthRequestCryptoServiceAbstraction; configApiService: ConfigApiServiceAbstraction; configService: CliConfigService; + accountService: AccountService; constructor() { let p = null; @@ -191,12 +194,15 @@ export class Main { this.memoryStorageService = new MemoryStorageService(); + this.accountService = new AccountServiceImplementation(null, this.logService); + this.stateService = new StateService( this.storageService, this.secureStorageService, this.memoryStorageService, this.logService, - new StateFactory(GlobalState, Account) + new StateFactory(GlobalState, Account), + this.accountService ); this.cryptoService = new CryptoService( diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index f4841073c9c..c586d8677c3 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -11,6 +11,7 @@ import { import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { AbstractThemingService } from "@bitwarden/angular/services/theming/theming.service.abstraction"; import { PolicyService as PolicyServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/login.service"; import { LoginService } from "@bitwarden/common/auth/services/login.service"; @@ -120,6 +121,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); MEMORY_STORAGE, LogService, STATE_FACTORY, + AccountServiceAbstraction, STATE_SERVICE_USE_CACHE, ], }, diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 1eb229281c8..1c4f415e1c8 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -2,6 +2,7 @@ import * as path from "path"; import { app } from "electron"; +import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; @@ -93,6 +94,7 @@ export class Main { this.memoryStorageService, this.logService, new StateFactory(GlobalState, Account), + new AccountServiceImplementation(null, this.logService), // will not broadcast logouts. This is a hack until we can remove messaging dependency false // Do not use disk caching because this will get out of sync with the renderer service ); diff --git a/apps/web/src/app/core/state/state.service.ts b/apps/web/src/app/core/state/state.service.ts index c95077bfbcc..4848ad4fb78 100644 --- a/apps/web/src/app/core/state/state.service.ts +++ b/apps/web/src/app/core/state/state.service.ts @@ -6,6 +6,7 @@ import { STATE_FACTORY, STATE_SERVICE_USE_CACHE, } from "@bitwarden/angular/services/injection-tokens"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractMemoryStorageService, @@ -30,6 +31,7 @@ export class StateService extends BaseStateService { @Inject(MEMORY_STORAGE) memoryStorageService: AbstractMemoryStorageService, logService: LogService, @Inject(STATE_FACTORY) stateFactory: StateFactory, + accountService: AccountService, @Inject(STATE_SERVICE_USE_CACHE) useAccountCache = true ) { super( @@ -38,6 +40,7 @@ export class StateService extends BaseStateService { memoryStorageService, logService, stateFactory, + accountService, useAccountCache ); } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index e52e9c394e4..060593d4a53 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -489,6 +489,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction"; MEMORY_STORAGE, LogService, STATE_FACTORY, + AccountServiceAbstraction, STATE_SERVICE_USE_CACHE, ], }, diff --git a/libs/common/spec/utils.ts b/libs/common/spec/utils.ts index 3cab011c6b0..8d6f8920318 100644 --- a/libs/common/spec/utils.ts +++ b/libs/common/spec/utils.ts @@ -1,4 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { Observable } from "rxjs"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; @@ -40,3 +41,40 @@ export function makeStaticByteArray(length: number, start = 0) { * Use to mock a return value of a static fromJSON method. */ export const mockFromJson = (stub: any) => (stub + "_fromJSON") as any; + +/** + * Tracks the emissions of the given observable. + * + * Call this function before you expect any emissions and then use code that will cause the observable to emit values, + * then assert after all expected emissions have occurred. + * @param observable + * @returns An array that will be populated with all emissions of the observable. + */ +export function trackEmissions(observable: Observable): T[] { + const emissions: T[] = []; + observable.subscribe((value) => { + switch (value) { + case undefined: + case null: + emissions.push(value); + return; + default: + // process by type + break; + } + + switch (typeof value) { + case "string": + case "number": + case "boolean": + emissions.push(value); + break; + case "object": + emissions.push({ ...value }); + break; + default: + emissions.push(JSON.parse(JSON.stringify(value))); + } + }); + return emissions; +} diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index 26c260eb6d5..30fe32e2597 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -1,4 +1,50 @@ -export abstract class AccountService {} +import { Observable } from "rxjs"; + +import { UserId } from "../../types/guid"; +import { AuthenticationStatus } from "../enums/authentication-status"; + +export type AccountInfo = { + status: AuthenticationStatus; + email: string; + name: string | undefined; +}; + +export abstract class AccountService { + accounts$: Observable>; + activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; + accountLock$: Observable; + accountLogout$: Observable; + /** + * Updates the `accounts$` observable with the new account data. + * @param userId + * @param accountData + */ + abstract addAccount(userId: UserId, accountData: AccountInfo): void; + /** + * updates the `accounts$` observable with the new preferred name for the account. + * @param userId + * @param name + */ + abstract setAccountName(userId: UserId, name: string): void; + /** + * updates the `accounts$` observable with the new email for the account. + * @param userId + * @param email + */ + abstract setAccountEmail(userId: UserId, email: string): void; + /** + * Updates the `accounts$` observable with the new account status. + * Also emits the `accountLock$` or `accountLogout$` observable if the status is `Locked` or `LoggedOut` respectively. + * @param userId + * @param status + */ + abstract setAccountStatus(userId: UserId, status: AuthenticationStatus): void; + /** + * Updates the `activeAccount$` observable with the new active account. + * @param userId + */ + abstract switchAccount(userId: UserId): void; +} export abstract class InternalAccountService extends AccountService { abstract delete(): void; diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts new file mode 100644 index 00000000000..3b28f39cf11 --- /dev/null +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -0,0 +1,181 @@ +import { MockProxy, mock } from "jest-mock-extended"; +import { firstValueFrom } from "rxjs"; + +import { trackEmissions } from "../../../spec/utils"; +import { LogService } from "../../platform/abstractions/log.service"; +import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { UserId } from "../../types/guid"; +import { AccountInfo } from "../abstractions/account.service"; +import { AuthenticationStatus } from "../enums/authentication-status"; + +import { AccountServiceImplementation } from "./account.service"; + +describe("accountService", () => { + let messagingService: MockProxy; + let logService: MockProxy; + let sut: AccountServiceImplementation; + const userId = "userId" as UserId; + function userInfo(status: AuthenticationStatus): AccountInfo { + return { status, email: "email", name: "name" }; + } + + beforeEach(() => { + messagingService = mock(); + logService = mock(); + + sut = new AccountServiceImplementation(messagingService, logService); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("activeAccount$", () => { + it("should emit undefined if no account is active", () => { + const emissions = trackEmissions(sut.activeAccount$); + + expect(emissions).toEqual([undefined]); + }); + + it("should emit the active account and status", async () => { + const emissions = trackEmissions(sut.activeAccount$); + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + sut.switchAccount(userId); + + expect(emissions).toEqual([ + undefined, // initial value + { id: userId, ...userInfo(AuthenticationStatus.Unlocked) }, + ]); + }); + + it("should remember the last emitted value", async () => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + sut.switchAccount(userId); + + expect(await firstValueFrom(sut.activeAccount$)).toEqual({ + id: userId, + ...userInfo(AuthenticationStatus.Unlocked), + }); + }); + }); + + describe("addAccount", () => { + it("should emit the new account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + + expect(emissions).toEqual([ + {}, // initial value + { [userId]: userInfo(AuthenticationStatus.Unlocked) }, + ]); + }); + }); + + describe("setAccountName", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should emit the updated account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountName(userId, "new name"); + + expect(emissions).toEqual([ + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "name" } }, + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "new name" } }, + ]); + }); + }); + + describe("setAccountEmail", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should emit the updated account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountEmail(userId, "new email"); + + expect(emissions).toEqual([ + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "email" } }, + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "new email" } }, + ]); + }); + }); + + describe("setAccountStatus", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should not emit if the status is the same", async () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); + sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); + + expect(emissions).toEqual([{ userId: userInfo(AuthenticationStatus.Unlocked) }]); + }); + + it("should maintain an accounts cache", async () => { + expect(await firstValueFrom(sut.accounts$)).toEqual({ + [userId]: userInfo(AuthenticationStatus.Unlocked), + }); + }); + + it("should emit if the status is different", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountStatus(userId, AuthenticationStatus.Locked); + + expect(emissions).toEqual([ + { userId: userInfo(AuthenticationStatus.Unlocked) }, // initial value from beforeEach + { userId: userInfo(AuthenticationStatus.Locked) }, + ]); + }); + + it("should emit logout if the status is logged out", () => { + const emissions = trackEmissions(sut.accountLogout$); + sut.setAccountStatus(userId, AuthenticationStatus.LoggedOut); + + expect(emissions).toEqual([userId]); + }); + + it("should emit lock if the status is locked", () => { + const emissions = trackEmissions(sut.accountLock$); + sut.setAccountStatus(userId, AuthenticationStatus.Locked); + + expect(emissions).toEqual([userId]); + }); + }); + + describe("switchAccount", () => { + let emissions: { id: string; status: AuthenticationStatus }[]; + + beforeEach(() => { + emissions = []; + sut.activeAccount$.subscribe((value) => emissions.push(value)); + }); + + it("should emit undefined if no account is provided", () => { + sut.switchAccount(undefined); + + expect(emissions).toEqual([undefined]); + }); + + it("should emit the active account and status", () => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + sut.switchAccount(userId); + sut.setAccountStatus(userId, AuthenticationStatus.Locked); + sut.switchAccount(undefined); + sut.switchAccount(undefined); + expect(emissions).toEqual([ + undefined, // initial value + { id: userId, ...userInfo(AuthenticationStatus.Unlocked) }, + { id: userId, ...userInfo(AuthenticationStatus.Locked) }, + ]); + }); + + it("should throw if switched to an unknown account", () => { + expect(() => sut.switchAccount(userId)).toThrowError("Account does not exist"); + }); + }); +}); diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 02c12050952..33388218db3 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -1,16 +1,93 @@ -import { InternalAccountService } from "../../auth/abstractions/account.service"; +import { + BehaviorSubject, + Subject, + combineLatestWith, + map, + distinctUntilChanged, + shareReplay, +} from "rxjs"; + +import { AccountInfo, InternalAccountService } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { UserId } from "../../types/guid"; +import { AuthenticationStatus } from "../enums/authentication-status"; export class AccountServiceImplementation implements InternalAccountService { + private accounts = new BehaviorSubject>({}); + private activeAccountId = new BehaviorSubject(undefined); + private lock = new Subject(); + private logout = new Subject(); + + accounts$ = this.accounts.asObservable(); + activeAccount$ = this.activeAccountId.pipe( + combineLatestWith(this.accounts$), + map(([id, accounts]) => (id ? { id, ...accounts[id] } : undefined)), + distinctUntilChanged(), + shareReplay({ bufferSize: 1, refCount: false }) + ); + accountLock$ = this.lock.asObservable(); + accountLogout$ = this.logout.asObservable(); constructor(private messagingService: MessagingService, private logService: LogService) {} + addAccount(userId: UserId, accountData: AccountInfo): void { + this.accounts.value[userId] = accountData; + this.accounts.next(this.accounts.value); + } + + setAccountName(userId: UserId, name: string): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], name }); + } + + setAccountEmail(userId: UserId, email: string): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], email }); + } + + setAccountStatus(userId: UserId, status: AuthenticationStatus): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], status }); + + if (status === AuthenticationStatus.LoggedOut) { + this.logout.next(userId); + } else if (status === AuthenticationStatus.Locked) { + this.lock.next(userId); + } + } + + switchAccount(userId: UserId) { + if (userId == null) { + // indicates no account is active + this.activeAccountId.next(undefined); + return; + } + + if (this.accounts.value[userId] == null) { + throw new Error("Account does not exist"); + } + this.activeAccountId.next(userId); + } + + // TODO: update to use our own account status settings. Requires inverting direction of state service accounts flow async delete(): Promise { try { - this.messagingService.send("logout"); + this.messagingService?.send("logout"); } catch (e) { this.logService.error(e); throw e; } } + + private setAccountInfo(userId: UserId, accountInfo: AccountInfo) { + if (this.accounts.value[userId] == null) { + throw new Error("Account does not exist"); + } + + // Avoid unnecessary updates + // TODO: Faster comparison, maybe include a hash on the objects? + if (JSON.stringify(this.accounts.value[userId]) === JSON.stringify(accountInfo)) { + return; + } + + this.accounts.value[userId] = accountInfo; + this.accounts.next(this.accounts.value); + } } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index d0983448d62..c8d45b6d4e3 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -6,6 +6,8 @@ import { OrganizationData } from "../../admin-console/models/data/organization.d import { PolicyData } from "../../admin-console/models/data/policy.data"; import { ProviderData } from "../../admin-console/models/data/provider.data"; import { Policy } from "../../admin-console/models/domain/policy"; +import { AccountService } from "../../auth/abstractions/account.service"; +import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { EnvironmentUrls } from "../../auth/models/domain/environment-urls"; import { ForceResetPasswordReason } from "../../auth/models/domain/force-reset-password-reason"; @@ -27,6 +29,7 @@ import { GeneratedPasswordHistory, PasswordGeneratorOptions } from "../../tools/ import { UsernameGeneratorOptions } from "../../tools/generator/username"; import { SendData } from "../../tools/send/models/data/send.data"; import { SendView } from "../../tools/send/models/view/send.view"; +import { UserId } from "../../types/guid"; import { CipherData } from "../../vault/models/data/cipher.data"; import { CollectionData } from "../../vault/models/data/collection.data"; import { FolderData } from "../../vault/models/data/folder.data"; @@ -110,6 +113,7 @@ export class StateService< protected memoryStorageService: AbstractMemoryStorageService, protected logService: LogService, protected stateFactory: StateFactory, + protected accountService: AccountService, protected useAccountCache: boolean = true ) { // If the account gets changed, verify the new account is unlocked @@ -168,6 +172,8 @@ export class StateService< } await this.pushAccounts(); this.activeAccountSubject.next(state.activeUserId); + // TODO: Temporary update to avoid routing all account status changes through account service for now. + this.accountService.switchAccount(state.activeUserId as UserId); return state; }); @@ -184,6 +190,12 @@ export class StateService< state.accounts[userId] = this.createAccount(); const diskAccount = await this.getAccountFromDisk({ userId: userId }); state.accounts[userId].profile = diskAccount.profile; + // TODO: Temporary update to avoid routing all account status changes through account service for now. + this.accountService.addAccount(userId as UserId, { + status: AuthenticationStatus.Locked, + name: diskAccount.profile.name, + email: diskAccount.profile.email, + }); return state; }); } @@ -198,6 +210,12 @@ export class StateService< }); await this.scaffoldNewAccountStorage(account); await this.setLastActive(new Date().getTime(), { userId: account.profile.userId }); + // TODO: Temporary update to avoid routing all account status changes through account service for now. + this.accountService.addAccount(account.profile.userId as UserId, { + status: AuthenticationStatus.Locked, + name: account.profile.name, + email: account.profile.email, + }); await this.setActiveUser(account.profile.userId); this.activeAccountSubject.next(account.profile.userId); } @@ -208,6 +226,9 @@ export class StateService< state.activeUserId = userId; await this.storageService.save(keys.activeUserId, userId); this.activeAccountSubject.next(state.activeUserId); + // TODO: temporary update to avoid routing all account status changes through account service for now. + this.accountService.switchAccount(userId as UserId); + return state; }); @@ -548,6 +569,9 @@ export class StateService< this.reconcileOptions(options, await this.defaultInMemoryOptions()) ); + const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; + this.accountService.setAccountStatus(options.userId as UserId, nextStatus); + if (options.userId == this.activeAccountSubject.getValue()) { const nextValue = value != null; @@ -581,6 +605,9 @@ export class StateService< this.reconcileOptions(options, await this.defaultInMemoryOptions()) ); + const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; + this.accountService.setAccountStatus(options.userId as UserId, nextStatus); + if (options?.userId == this.activeAccountSubject.getValue()) { const nextValue = value != null; @@ -3062,7 +3089,6 @@ export class StateService< this.reconcileOptions({ userId: account.profile.userId }, await this.defaultOnDiskOptions()) ); } - // protected async pushAccounts(): Promise { await this.pruneInMemoryAccounts(); @@ -3180,6 +3206,8 @@ export class StateService< return state; }); + // TODO: Invert this logic, we should remove accounts based on logged out emit + this.accountService.setAccountStatus(userId as UserId, AuthenticationStatus.LoggedOut); } protected async pruneInMemoryAccounts() { diff --git a/libs/common/src/types/guid.d.ts b/libs/common/src/types/guid.d.ts new file mode 100644 index 00000000000..f77655a95f3 --- /dev/null +++ b/libs/common/src/types/guid.d.ts @@ -0,0 +1,5 @@ +import { Opaque } from "type-fest"; + +type Guid = Opaque; + +type UserId = Opaque;