From a4d5717283c2fed9bbfbd3cb537437732090dc6c Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Fri, 3 May 2024 14:24:30 -0400 Subject: [PATCH] Use account service for account enumeration. (#9023) --- .../services/default-browser-state.service.ts | 10 ------- apps/desktop/src/app/app.component.ts | 8 +++-- .../src/app/services/services.module.ts | 2 +- .../encrypted-message-handler.service.ts | 25 ++++++++++------ .../src/services/native-messaging.service.ts | 30 +++++++------------ .../platform/abstractions/state.service.ts | 4 --- .../src/platform/services/state.service.ts | 22 +------------- 7 files changed, 33 insertions(+), 68 deletions(-) diff --git a/apps/browser/src/platform/services/default-browser-state.service.ts b/apps/browser/src/platform/services/default-browser-state.service.ts index b0b9e3748f..d7bc45bcc3 100644 --- a/apps/browser/src/platform/services/default-browser-state.service.ts +++ b/apps/browser/src/platform/services/default-browser-state.service.ts @@ -1,5 +1,3 @@ -import { BehaviorSubject } from "rxjs"; - import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -15,21 +13,13 @@ import { MigrationRunner } from "@bitwarden/common/platform/services/migration-r import { StateService as BaseStateService } from "@bitwarden/common/platform/services/state.service"; import { Account } from "../../models/account"; -import { browserSession, sessionSync } from "../decorators/session-sync-observable"; import { BrowserStateService } from "./abstractions/browser-state.service"; -@browserSession export class DefaultBrowserStateService extends BaseStateService implements BrowserStateService { - @sessionSync({ - initializer: Account.fromJSON as any, // TODO: Remove this any when all any types are removed from Account - initializeAs: "record", - }) - protected accountsSubject: BehaviorSubject<{ [userId: string]: Account }>; - protected accountDeserializer = Account.fromJSON; constructor( diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 4e540efdc6..bf4bec789a 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -218,8 +218,10 @@ export class AppComponent implements OnInit, OnDestroy { await this.vaultTimeoutService.lock(message.userId); break; case "lockAllVaults": { - const currentUser = await this.stateService.getUserId(); - const accounts = await firstValueFrom(this.stateService.accounts$); + const currentUser = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a.id)), + ); + const accounts = await firstValueFrom(this.accountService.accounts$); await this.vaultTimeoutService.lock(currentUser); for (const account of Object.keys(accounts)) { if (account === currentUser) { @@ -690,7 +692,7 @@ export class AppComponent implements OnInit, OnDestroy { } private async checkForSystemTimeout(timeout: number): Promise { - const accounts = await firstValueFrom(this.stateService.accounts$); + const accounts = await firstValueFrom(this.accountService.accounts$); for (const userId in accounts) { if (userId == null) { continue; diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index a485b925ba..2acf6dde5a 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -221,7 +221,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: EncryptedMessageHandlerService, deps: [ - StateServiceAbstraction, + AccountServiceAbstraction, AuthServiceAbstraction, CipherServiceAbstraction, PolicyServiceAbstraction, diff --git a/apps/desktop/src/services/encrypted-message-handler.service.ts b/apps/desktop/src/services/encrypted-message-handler.service.ts index e38339d5ad..4512e175ce 100644 --- a/apps/desktop/src/services/encrypted-message-handler.service.ts +++ b/apps/desktop/src/services/encrypted-message-handler.service.ts @@ -1,12 +1,13 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -28,7 +29,7 @@ import { UserStatusErrorResponse } from "../models/native-messaging/encrypted-me export class EncryptedMessageHandlerService { constructor( - private stateService: StateService, + private accountService: AccountService, private authService: AuthService, private cipherService: CipherService, private policyService: PolicyService, @@ -62,7 +63,9 @@ export class EncryptedMessageHandlerService { } private async checkUserStatus(userId: string): Promise { - const activeUserId = await this.stateService.getUserId(); + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); if (userId !== activeUserId) { return "not-active-user"; @@ -77,17 +80,19 @@ export class EncryptedMessageHandlerService { } private async statusCommandHandler(): Promise { - const accounts = await firstValueFrom(this.stateService.accounts$); - const activeUserId = await this.stateService.getUserId(); + const accounts = await firstValueFrom(this.accountService.accounts$); + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); if (!accounts || !Object.keys(accounts)) { return []; } return Promise.all( - Object.keys(accounts).map(async (userId) => { + Object.keys(accounts).map(async (userId: UserId) => { const authStatus = await this.authService.getAuthStatus(userId); - const email = await this.stateService.getEmail({ userId }); + const email = accounts[userId].email; return { id: userId, @@ -107,7 +112,9 @@ export class EncryptedMessageHandlerService { } const ciphersResponse: CipherResponse[] = []; - const activeUserId = await this.stateService.getUserId(); + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); const authStatus = await this.authService.getAuthStatus(activeUserId); if (authStatus !== AuthenticationStatus.Unlocked) { diff --git a/apps/desktop/src/services/native-messaging.service.ts b/apps/desktop/src/services/native-messaging.service.ts index 01d9476977..48bdc60047 100644 --- a/apps/desktop/src/services/native-messaging.service.ts +++ b/apps/desktop/src/services/native-messaging.service.ts @@ -1,6 +1,7 @@ import { Injectable, NgZone } from "@angular/core"; import { firstValueFrom } from "rxjs"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; @@ -41,6 +42,7 @@ export class NativeMessagingService { private biometricStateService: BiometricStateService, private nativeMessageHandler: NativeMessageHandlerService, private dialogService: DialogService, + private accountService: AccountService, private ngZone: NgZone, ) {} @@ -51,9 +53,7 @@ export class NativeMessagingService { private async messageHandler(msg: LegacyMessageWrapper | Message) { const outerMessage = msg as Message; if (outerMessage.version) { - // 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 - this.nativeMessageHandler.handleMessage(outerMessage); + await this.nativeMessageHandler.handleMessage(outerMessage); return; } @@ -64,7 +64,7 @@ export class NativeMessagingService { const remotePublicKey = Utils.fromB64ToArray(rawMessage.publicKey); // Validate the UserId to ensure we are logged into the same account. - const accounts = await firstValueFrom(this.stateService.accounts$); + const accounts = await firstValueFrom(this.accountService.accounts$); const userIds = Object.keys(accounts); if (!userIds.includes(rawMessage.userId)) { ipc.platform.nativeMessaging.sendMessage({ @@ -81,7 +81,7 @@ export class NativeMessagingService { }); const fingerprint = await this.cryptoService.getFingerprint( - await this.stateService.getUserId(), + rawMessage.userId, remotePublicKey, ); @@ -98,9 +98,7 @@ export class NativeMessagingService { } } - // 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 - this.secureCommunication(remotePublicKey, appId); + await this.secureCommunication(remotePublicKey, appId); return; } @@ -144,9 +142,7 @@ export class NativeMessagingService { ? firstValueFrom(this.biometricStateService.biometricUnlockEnabled$) : this.biometricStateService.getBiometricUnlockEnabled(message.userId as UserId); if (!(await biometricUnlockPromise)) { - // 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 - this.send({ command: "biometricUnlock", response: "not enabled" }, appId); + await this.send({ command: "biometricUnlock", response: "not enabled" }, appId); return this.ngZone.run(() => this.dialogService.openSimpleDialog({ @@ -172,9 +168,7 @@ export class NativeMessagingService { // we send the master key still for backwards compatibility // with older browser extensions // TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3472) - // 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 - this.send( + await this.send( { command: "biometricUnlock", response: "unlocked", @@ -184,14 +178,10 @@ export class NativeMessagingService { appId, ); } else { - // 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 - this.send({ command: "biometricUnlock", response: "canceled" }, appId); + await this.send({ command: "biometricUnlock", response: "canceled" }, appId); } } catch (e) { - // 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 - this.send({ command: "biometricUnlock", response: "canceled" }, appId); + await this.send({ command: "biometricUnlock", response: "canceled" }, appId); } break; diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index e8edd5d132..f694f32eeb 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -1,5 +1,3 @@ -import { Observable } from "rxjs"; - import { BiometricKey } from "../../auth/types/biometric-key"; import { GeneratorOptions } from "../../tools/generator/generator-options"; import { GeneratedPasswordHistory, PasswordGeneratorOptions } from "../../tools/generator/password"; @@ -24,8 +22,6 @@ export type InitOptions = { }; export abstract class StateService { - accounts$: Observable<{ [userId: string]: T }>; - addAccount: (account: T) => Promise; clearDecryptedData: (userId: UserId) => Promise; clean: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 3324c4d2ab..156a871a2d 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject, firstValueFrom, map } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; @@ -52,9 +52,6 @@ export class StateService< TAccount extends Account = Account, > implements StateServiceAbstraction { - protected accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({}); - accounts$ = this.accountsSubject.asObservable(); - private hasBeenInited = false; protected isRecoveredSession = false; @@ -115,8 +112,6 @@ export class StateService< state = await this.syncAccountFromDisk(authenticatedAccounts[i]); } - await this.pushAccounts(); - return state; }); } @@ -153,7 +148,6 @@ export class StateService< await this.removeAccountFromDisk(options?.userId); await this.removeAccountFromMemory(options?.userId); - await this.pushAccounts(); } /** @@ -856,7 +850,6 @@ export class StateService< }); }); } - await this.pushAccounts(); } protected async scaffoldNewAccountStorage(account: TAccount): Promise { @@ -934,17 +927,6 @@ export class StateService< ); } - protected async pushAccounts(): Promise { - await this.state().then((state) => { - if (state.accounts == null || Object.keys(state.accounts).length < 1) { - this.accountsSubject.next({}); - return; - } - - this.accountsSubject.next(state.accounts); - }); - } - protected reconcileOptions( requestedOptions: StorageOptions, defaultOptions: StorageOptions, @@ -1096,8 +1078,6 @@ export class StateService< return state; }); - - await this.pushAccounts(); } protected createAccount(init: Partial = null): TAccount {