From cf6569bfea7516cf3e86f2854d5c61c17a547983 Mon Sep 17 00:00:00 2001 From: Dave <3836813+enmande@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:23:22 -0500 Subject: [PATCH] feat(user-decryption-options) [PM-26413]: Remove ActiveUserState from UserDecryptionOptionsService (#16894) * feat(user-decryption-options) [PM-26413]: Update UserDecryptionOptionsService and tests to use UserId-only APIs. * feat(user-decryption-options) [PM-26413]: Update InternalUserDecryptionOptionsService call sites to use UserId-only API. * feat(user-decryption-options) [PM-26413] Update userDecryptionOptions$ call sites to use the UserId-only API. * feat(user-decryption-options) [PM-26413]: Update additional call sites. * feat(user-decryption-options) [PM-26413]: Update dependencies and an additional call site. * feat(user-verification-service) [PM-26413]: Replace where allowed by unrestricted imports invocation of UserVerificationService.hasMasterPassword (deprecated) with UserDecryptionOptions.hasMasterPasswordById$. Additional work to complete as tech debt tracked in PM-27009. * feat(user-decryption-options) [PM-26413]: Update for non-null strict adherence. * feat(user-decryption-options) [PM-26413]: Update type safety and defensive returns. * chore(user-decryption-options) [PM-26413]: Comment cleanup. * feat(user-decryption-options) [PM-26413]: Update tests. * feat(user-decryption-options) [PM-26413]: Standardize null-checking on active account id for new API consumption. * feat(vault-timeout-settings-service) [PM-26413]: Add test cases to illustrate null active account from AccountService. * fix(fido2-user-verification-service-spec) [PM-26413]: Update test harness to use FakeAccountService. * fix(downstream-components) [PM-26413]: Prefer use of the getUserId operator in all authenticated contexts for user id provided to UserDecryptionOptionsService. --------- Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com> --- .../browser/src/background/main.background.ts | 7 +- .../src/popup/services/services.module.ts | 8 +- .../fido2-user-verification.service.spec.ts | 31 ++++--- .../fido2-user-verification.service.ts | 16 +++- .../service-container/service-container.ts | 5 +- ...sktop-set-initial-password.service.spec.ts | 4 +- .../web-set-initial-password.service.spec.ts | 4 +- .../settings/account/account.component.ts | 10 +- .../password-settings.component.ts | 7 +- .../security/security-keys.component.ts | 22 ++++- .../settings/security/security.component.ts | 14 ++- .../organization-options.component.ts | 5 +- ...initial-password.service.implementation.ts | 7 +- ...fault-set-initial-password.service.spec.ts | 10 +- .../src/services/jslib-services.module.ts | 3 +- .../login-decryption-options.component.ts | 2 +- libs/auth/src/angular/sso/sso.component.ts | 2 +- .../two-factor-auth.component.spec.ts | 4 +- .../two-factor-auth.component.ts | 2 +- ...-decryption-options.service.abstraction.ts | 35 ++++--- .../login-strategies/login.strategy.spec.ts | 3 +- .../common/login-strategies/login.strategy.ts | 3 +- .../sso-login.strategy.spec.ts | 4 +- .../login-strategies/sso-login.strategy.ts | 2 +- .../user-decryption-options.service.spec.ts | 93 ++++++++++--------- .../user-decryption-options.service.ts | 42 ++++----- .../user-verification.service.abstraction.ts | 3 + .../user-verification.service.spec.ts | 23 +---- .../user-verification.service.ts | 19 ++-- .../device-trust.service.implementation.ts | 17 +++- .../services/device-trust.service.spec.ts | 3 +- .../vault-timeout-settings.service.spec.ts | 25 ++++- .../vault-timeout-settings.service.ts | 17 ++-- 33 files changed, 280 insertions(+), 172 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fecc47af981..78b5e323798 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -728,7 +728,9 @@ export default class MainBackground { this.appIdService = new AppIdService(this.storageService, this.logService); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); + this.userDecryptionOptionsService = new UserDecryptionOptionsService( + this.singleUserStateProvider, + ); this.organizationService = new DefaultOrganizationService(this.stateProvider); this.policyService = new DefaultPolicyService(this.stateProvider, this.organizationService); @@ -859,8 +861,6 @@ export default class MainBackground { this.stateProvider, ); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); - this.devicesApiService = new DevicesApiServiceImplementation(this.apiService); this.deviceTrustService = new DeviceTrustService( this.keyGenerationService, @@ -876,6 +876,7 @@ export default class MainBackground { this.userDecryptionOptionsService, this.logService, this.configService, + this.accountService, ); this.devicesService = new DevicesServiceImplementation( diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index eebf0a08a22..c462319dc2e 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -36,6 +36,7 @@ import { LoginEmailService, SsoUrlService, LogoutService, + UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -607,7 +608,12 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: Fido2UserVerificationService, useClass: Fido2UserVerificationService, - deps: [PasswordRepromptService, UserVerificationService, DialogService], + deps: [ + PasswordRepromptService, + UserDecryptionOptionsServiceAbstraction, + DialogService, + AccountServiceAbstraction, + ], }), safeProvider({ provide: AnimationControlService, diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts index 97a22bb2cf3..e55e3091244 100644 --- a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts +++ b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts @@ -1,11 +1,15 @@ import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { DialogService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; import { PasswordRepromptService } from "@bitwarden/vault"; // FIXME (PM-22628): Popup imports are forbidden in background @@ -31,21 +35,24 @@ describe("Fido2UserVerificationService", () => { let fido2UserVerificationService: Fido2UserVerificationService; let passwordRepromptService: MockProxy; - let userVerificationService: MockProxy; + let userDecryptionOptionsService: MockProxy; let dialogService: MockProxy; + let accountService: FakeAccountService; let cipher: CipherView; beforeEach(() => { passwordRepromptService = mock(); - userVerificationService = mock(); + userDecryptionOptionsService = mock(); dialogService = mock(); + accountService = mockAccountServiceWith(newGuid() as UserId); cipher = createCipherView(); fido2UserVerificationService = new Fido2UserVerificationService( passwordRepromptService, - userVerificationService, + userDecryptionOptionsService, dialogService, + accountService, ); (UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({ @@ -67,7 +74,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(true); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -82,7 +89,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( true, @@ -98,7 +105,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( true, @@ -114,7 +121,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -176,7 +183,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(true); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -191,7 +198,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( false, @@ -207,7 +214,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( false, @@ -223,7 +230,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.ts b/apps/browser/src/vault/services/fido2-user-verification.service.ts index 9bf9be70fc8..db3951d44d9 100644 --- a/apps/browser/src/vault/services/fido2-user-verification.service.ts +++ b/apps/browser/src/vault/services/fido2-user-verification.service.ts @@ -3,7 +3,8 @@ import { firstValueFrom } from "rxjs"; import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { DialogService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -15,8 +16,9 @@ import { SetPinComponent } from "../../auth/popup/components/set-pin.component"; export class Fido2UserVerificationService { constructor( private passwordRepromptService: PasswordRepromptService, - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private dialogService: DialogService, + private accountService: AccountService, ) {} /** @@ -78,7 +80,15 @@ export class Fido2UserVerificationService { } private async handleMasterPasswordReprompt(): Promise { - const hasMasterPassword = await this.userVerificationService.hasMasterPassword(); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + + if (!activeAccount?.id) { + return false; + } + + const hasMasterPassword = await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(activeAccount.id), + ); // TDE users have no master password, so we need to use the UserVerification prompt return hasMasterPassword diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 365205a1329..122dd6ea052 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -512,7 +512,9 @@ export class ServiceContainer { ")"; this.biometricStateService = new DefaultBiometricStateService(this.stateProvider); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); + this.userDecryptionOptionsService = new UserDecryptionOptionsService( + this.singleUserStateProvider, + ); this.ssoUrlService = new SsoUrlService(); this.organizationService = new DefaultOrganizationService(this.stateProvider); @@ -702,6 +704,7 @@ export class ServiceContainer { this.userDecryptionOptionsService, this.logService, this.configService, + this.accountService, ); this.loginStrategyService = new LoginStrategyService( diff --git a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts index 53a1c7dbd4c..717af25a1dc 100644 --- a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts +++ b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts @@ -119,7 +119,9 @@ describe("DesktopSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, diff --git a/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts b/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts index 70f7686a2cd..647c9ae83d9 100644 --- a/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts +++ b/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts @@ -123,7 +123,9 @@ describe("WebSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, diff --git a/apps/web/src/app/auth/settings/account/account.component.ts b/apps/web/src/app/auth/settings/account/account.component.ts index 8bae8cd2c1f..3e618b89dbe 100644 --- a/apps/web/src/app/auth/settings/account/account.component.ts +++ b/apps/web/src/app/auth/settings/account/account.component.ts @@ -1,11 +1,10 @@ import { Component, OnInit, OnDestroy } from "@angular/core"; -import { firstValueFrom, from, lastValueFrom, map, Observable, Subject, takeUntil } from "rxjs"; +import { firstValueFrom, lastValueFrom, map, Observable, Subject, takeUntil } from "rxjs"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { DialogService } from "@bitwarden/components"; import { HeaderModule } from "../../../layouts/header/header.module"; @@ -42,8 +41,7 @@ export class AccountComponent implements OnInit, OnDestroy { constructor( private accountService: AccountService, private dialogService: DialogService, - private userVerificationService: UserVerificationService, - private configService: ConfigService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private organizationService: OrganizationService, ) {} @@ -56,7 +54,7 @@ export class AccountComponent implements OnInit, OnDestroy { map((organizations) => organizations.some((o) => o.userIsManagedByOrganization === true)), ); - const hasMasterPassword$ = from(this.userVerificationService.hasMasterPassword()); + const hasMasterPassword$ = this.userDecryptionOptionsService.hasMasterPasswordById$(userId); this.showChangeEmail$ = hasMasterPassword$; diff --git a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts index 0e37c856935..ee283d26415 100644 --- a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts +++ b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts @@ -5,6 +5,8 @@ import { firstValueFrom } from "rxjs"; import { ChangePasswordComponent } from "@bitwarden/angular/auth/password-management/change-password"; import { InputPasswordFlow } from "@bitwarden/auth/angular"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { CalloutModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; @@ -24,12 +26,15 @@ export class PasswordSettingsComponent implements OnInit { constructor( private router: Router, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private accountService: AccountService, ) {} async ngOnInit() { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const userHasMasterPassword = await firstValueFrom( - this.userDecryptionOptionsService.hasMasterPassword$, + this.userDecryptionOptionsService.hasMasterPasswordById$(userId), ); + if (!userHasMasterPassword) { await this.router.navigate(["/settings/security/two-factor"]); return; diff --git a/apps/web/src/app/auth/settings/security/security-keys.component.ts b/apps/web/src/app/auth/settings/security/security-keys.component.ts index 27a555ff343..b62828a2783 100644 --- a/apps/web/src/app/auth/settings/security/security-keys.component.ts +++ b/apps/web/src/app/auth/settings/security/security-keys.component.ts @@ -1,11 +1,10 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { firstValueFrom, map } from "rxjs"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { DialogService } from "@bitwarden/components"; import { ChangeKdfModule } from "../../../key-management/change-kdf/change-kdf.module"; @@ -23,20 +22,28 @@ export class SecurityKeysComponent implements OnInit { showChangeKdf = true; constructor( - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private accountService: AccountService, private apiService: ApiService, private dialogService: DialogService, ) {} async ngOnInit() { - this.showChangeKdf = await this.userVerificationService.hasMasterPassword(); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.showChangeKdf = await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(userId), + ); } async viewUserApiKey() { const entityId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); + + if (!entityId) { + throw new Error("Active account not found"); + } + await ApiKeyComponent.open(this.dialogService, { data: { keyType: "user", @@ -55,6 +62,11 @@ export class SecurityKeysComponent implements OnInit { const entityId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); + + if (!entityId) { + throw new Error("Active account not found"); + } + await ApiKeyComponent.open(this.dialogService, { data: { keyType: "user", diff --git a/apps/web/src/app/auth/settings/security/security.component.ts b/apps/web/src/app/auth/settings/security/security.component.ts index 629de32efc4..85bc29fac63 100644 --- a/apps/web/src/app/auth/settings/security/security.component.ts +++ b/apps/web/src/app/auth/settings/security/security.component.ts @@ -1,7 +1,9 @@ import { Component, OnInit } from "@angular/core"; -import { Observable } from "rxjs"; +import { firstValueFrom, Observable } from "rxjs"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -20,7 +22,8 @@ export class SecurityComponent implements OnInit { consolidatedSessionTimeoutComponent$: Observable; constructor( - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private accountService: AccountService, private configService: ConfigService, ) { this.consolidatedSessionTimeoutComponent$ = this.configService.getFeatureFlag$( @@ -29,6 +32,9 @@ export class SecurityComponent implements OnInit { } async ngOnInit() { - this.showChangePassword = await this.userVerificationService.hasMasterPassword(); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.showChangePassword = userId + ? await firstValueFrom(this.userDecryptionOptionsService.hasMasterPasswordById$(userId)) + : false; } } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts index 3b707f2d78c..37b881406e3 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts @@ -95,7 +95,10 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy { combineLatest([ this.organization$, resetPasswordPolicies$, - this.userDecryptionOptionsService.userDecryptionOptions$, + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.userDecryptionOptionsService.userDecryptionOptionsById$(userId)), + ), managingOrg$, ]) .pipe(takeUntil(this.destroy$)) diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index dcd4fd93cba..df5220b5255 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -198,10 +198,13 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi userId: UserId, ) { const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), ); userDecryptionOpts.hasMasterPassword = true; - await this.userDecryptionOptionsService.setUserDecryptionOptions(userDecryptionOpts); + await this.userDecryptionOptionsService.setUserDecryptionOptionsById( + userId, + userDecryptionOpts, + ); await this.kdfConfigService.setKdfConfig(userId, kdfConfig); await this.masterPasswordService.setMasterKey(masterKey, userId); await this.keyService.setUserKey(masterKeyEncryptedUserKey[0], userId); diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts index 7a1dfc91e67..8b95090e776 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts @@ -149,7 +149,9 @@ describe("DefaultSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, @@ -362,7 +364,8 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); @@ -560,7 +563,8 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index ff5caff540c..bcb601a993c 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -684,7 +684,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: InternalUserDecryptionOptionsServiceAbstraction, useClass: UserDecryptionOptionsService, - deps: [StateProvider], + deps: [SingleUserStateProvider], }), safeProvider({ provide: UserDecryptionOptionsServiceAbstraction, @@ -1292,6 +1292,7 @@ const safeProviders: SafeProvider[] = [ UserDecryptionOptionsServiceAbstraction, LogService, ConfigService, + AccountServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts index 26293285008..fb07069998b 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts @@ -135,7 +135,7 @@ export class LoginDecryptionOptionsComponent implements OnInit { try { const userDecryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(this.activeAccountId), ); if ( diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index 0b6bb1159f4..bf618ba39f4 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -460,7 +460,7 @@ export class SsoComponent implements OnInit { // must come after 2fa check since user decryption options aren't available if 2fa is required const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId), ); const tdeEnabled = userDecryptionOpts.trustedDeviceOption diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts index af9fb03e01e..8c12060168b 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts @@ -176,7 +176,9 @@ describe("TwoFactorAuthComponent", () => { selectedUserDecryptionOptions = new BehaviorSubject( mockUserDecryptionOpts.withMasterPassword, ); - mockUserDecryptionOptionsService.userDecryptionOptions$ = selectedUserDecryptionOptions; + mockUserDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + selectedUserDecryptionOptions, + ); TestBed.configureTestingModule({ declarations: [TestTwoFactorComponent], diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index 8e10539823d..ca19d3652bb 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -473,7 +473,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { } const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId), ); const tdeEnabled = await this.isTrustedDeviceEncEnabled(userDecryptionOpts.trustedDeviceOption); diff --git a/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts b/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts index e46fb09cff6..443e43b3151 100644 --- a/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts @@ -1,34 +1,45 @@ import { Observable } from "rxjs"; +import { UserId } from "@bitwarden/common/types/guid"; + import { UserDecryptionOptions } from "../models"; +/** + * Public service for reading user decryption options. + * For use in components and services that need to evaluate user decryption settings. + */ export abstract class UserDecryptionOptionsServiceAbstraction { /** - * Returns what decryption options are available for the current user. - * @remark This is sent from the server on authentication. + * Returns the user decryption options for the given user id. + * Will only emit when options are set (does not emit null/undefined + * for an unpopulated state), and should not be called in an unauthenticated context. + * @param userId The user id to check. */ - abstract userDecryptionOptions$: Observable; + abstract userDecryptionOptionsById$(userId: UserId): Observable; /** * Uses user decryption options to determine if current user has a master password. * @remark This is sent from the server, and does not indicate if the master password * was used to login and/or if a master key is saved locally. */ - abstract hasMasterPassword$: Observable; - - /** - * Returns the user decryption options for the given user id. - * @param userId The user id to check. - */ - abstract userDecryptionOptionsById$(userId: string): Observable; + abstract hasMasterPasswordById$(userId: UserId): Observable; } +/** + * Internal service for managing user decryption options. + * For use only in authentication flows that need to update decryption options + * (e.g., login strategies). Extends consumer methods from {@link UserDecryptionOptionsServiceAbstraction}. + * @remarks Most consumers should use UserDecryptionOptionsServiceAbstraction instead. + */ export abstract class InternalUserDecryptionOptionsServiceAbstraction extends UserDecryptionOptionsServiceAbstraction { /** - * Sets the current decryption options for the user, contains the current configuration + * Sets the current decryption options for the user. Contains the current configuration * of the users account related to how they can decrypt their vault. * @remark Intended to be used when user decryption options are received from server, does * not update the server. Consider syncing instead of updating locally. * @param userDecryptionOptions Current user decryption options received from server. */ - abstract setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise; + abstract setUserDecryptionOptionsById( + userId: UserId, + userDecryptionOptions: UserDecryptionOptions, + ): Promise; } diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index e9eed27d5a1..38d62cfdd83 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -257,7 +257,8 @@ describe("LoginStrategy", () => { expect(environmentService.seedUserEnvironment).toHaveBeenCalled(); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, UserDecryptionOptions.fromResponse(idTokenResponse), ); expect(masterPasswordService.mock.setMasterPasswordUnlockData).toHaveBeenCalledWith( diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 35f13246593..b8e4ee9e822 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -195,7 +195,8 @@ export abstract class LoginStrategy { // We must set user decryption options before retrieving vault timeout settings // as the user decryption options help determine the available timeout actions. - await this.userDecryptionOptionsService.setUserDecryptionOptions( + await this.userDecryptionOptionsService.setUserDecryptionOptionsById( + userId, UserDecryptionOptions.fromResponse(tokenResponse), ); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 03de5f36c2d..acbb680ae6d 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -134,7 +134,9 @@ describe("SsoLoginStrategy", () => { ); const userDecryptionOptions = new UserDecryptionOptions(); - userDecryptionOptionsService.userDecryptionOptions$ = of(userDecryptionOptions); + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + of(userDecryptionOptions), + ); ssoLoginStrategy = new SsoLoginStrategy( {} as SsoLoginStrategyData, diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index ec7914b087e..d806f6d733e 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -393,7 +393,7 @@ export class SsoLoginStrategy extends LoginStrategy { // Check for TDE-related conditions const userDecryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), ); if (!userDecryptionOptions) { diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts index c2fafc1a2f6..efb00b9aa63 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts @@ -1,12 +1,8 @@ import { firstValueFrom } from "rxjs"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { - FakeAccountService, - FakeStateProvider, - mockAccountServiceWith, -} from "@bitwarden/common/spec"; +import { FakeSingleUserStateProvider } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; +import { newGuid } from "@bitwarden/guid"; import { UserDecryptionOptions } from "../../models/domain/user-decryption-options"; @@ -17,15 +13,10 @@ import { describe("UserDecryptionOptionsService", () => { let sut: UserDecryptionOptionsService; - - const fakeUserId = Utils.newGuid() as UserId; - let fakeAccountService: FakeAccountService; - let fakeStateProvider: FakeStateProvider; + let fakeStateProvider: FakeSingleUserStateProvider; beforeEach(() => { - fakeAccountService = mockAccountServiceWith(fakeUserId); - fakeStateProvider = new FakeStateProvider(fakeAccountService); - + fakeStateProvider = new FakeSingleUserStateProvider(); sut = new UserDecryptionOptionsService(fakeStateProvider); }); @@ -42,55 +33,71 @@ describe("UserDecryptionOptionsService", () => { }, }; - describe("userDecryptionOptions$", () => { - it("should return the active user's decryption options", async () => { - await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions); + describe("userDecryptionOptionsById$", () => { + it("should return user decryption options for a specific user", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom(sut.userDecryptionOptions$); + fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions); + + const result = await firstValueFrom(sut.userDecryptionOptionsById$(userId)); expect(result).toEqual(userDecryptionOptions); }); }); - describe("hasMasterPassword$", () => { - it("should return the hasMasterPassword property of the active user's decryption options", async () => { - await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions); + describe("hasMasterPasswordById$", () => { + it("should return true when user has a master password", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom(sut.hasMasterPassword$); + fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions); + + const result = await firstValueFrom(sut.hasMasterPasswordById$(userId)); expect(result).toBe(true); }); - }); - describe("userDecryptionOptionsById$", () => { - it("should return the user decryption options for the given user", async () => { - const givenUser = Utils.newGuid() as UserId; - await fakeAccountService.addAccount(givenUser, { - name: "Test User 1", - email: "test1@email.com", - emailVerified: false, - }); - await fakeStateProvider.setUserState( - USER_DECRYPTION_OPTIONS, - userDecryptionOptions, - givenUser, - ); + it("should return false when user does not have a master password", async () => { + const userId = newGuid() as UserId; + const optionsWithoutMasterPassword = { + ...userDecryptionOptions, + hasMasterPassword: false, + }; - const result = await firstValueFrom(sut.userDecryptionOptionsById$(givenUser)); + fakeStateProvider + .getFake(userId, USER_DECRYPTION_OPTIONS) + .nextState(optionsWithoutMasterPassword); - expect(result).toEqual(userDecryptionOptions); + const result = await firstValueFrom(sut.hasMasterPasswordById$(userId)); + + expect(result).toBe(false); }); }); - describe("setUserDecryptionOptions", () => { - it("should set the active user's decryption options", async () => { - await sut.setUserDecryptionOptions(userDecryptionOptions); + describe("setUserDecryptionOptionsById", () => { + it("should set user decryption options for a specific user", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom( - fakeStateProvider.getActive(USER_DECRYPTION_OPTIONS).state$, - ); + await sut.setUserDecryptionOptionsById(userId, userDecryptionOptions); + + const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS); + const result = await firstValueFrom(fakeState.state$); expect(result).toEqual(userDecryptionOptions); }); + + it("should overwrite existing user decryption options", async () => { + const userId = newGuid() as UserId; + const initialOptions = { ...userDecryptionOptions, hasMasterPassword: false }; + const updatedOptions = { ...userDecryptionOptions, hasMasterPassword: true }; + + const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS); + fakeState.nextState(initialOptions); + + await sut.setUserDecryptionOptionsById(userId, updatedOptions); + + const result = await firstValueFrom(fakeState.state$); + + expect(result).toEqual(updatedOptions); + }); }); }); diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts index 7c44a6f1682..a0075d1987b 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts @@ -1,16 +1,11 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Observable, map } from "rxjs"; +import { Observable, filter, map } from "rxjs"; import { - ActiveUserState, - StateProvider, + SingleUserStateProvider, USER_DECRYPTION_OPTIONS_DISK, UserKeyDefinition, } from "@bitwarden/common/platform/state"; -// FIXME: remove `src` and fix import -// eslint-disable-next-line no-restricted-imports -import { UserId } from "@bitwarden/common/src/types/guid"; +import { UserId } from "@bitwarden/common/types/guid"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction"; import { UserDecryptionOptions } from "../../models"; @@ -27,25 +22,26 @@ export const USER_DECRYPTION_OPTIONS = new UserKeyDefinition; + constructor(private singleUserStateProvider: SingleUserStateProvider) {} - userDecryptionOptions$: Observable; - hasMasterPassword$: Observable; + userDecryptionOptionsById$(userId: UserId): Observable { + return this.singleUserStateProvider + .get(userId, USER_DECRYPTION_OPTIONS) + .state$.pipe(filter((options): options is UserDecryptionOptions => options != null)); + } - constructor(private stateProvider: StateProvider) { - this.userDecryptionOptionsState = this.stateProvider.getActive(USER_DECRYPTION_OPTIONS); - - this.userDecryptionOptions$ = this.userDecryptionOptionsState.state$; - this.hasMasterPassword$ = this.userDecryptionOptions$.pipe( - map((options) => options?.hasMasterPassword ?? false), + hasMasterPasswordById$(userId: UserId): Observable { + return this.userDecryptionOptionsById$(userId).pipe( + map((options) => options.hasMasterPassword ?? false), ); } - userDecryptionOptionsById$(userId: UserId): Observable { - return this.stateProvider.getUser(userId, USER_DECRYPTION_OPTIONS).state$; - } - - async setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise { - await this.userDecryptionOptionsState.update((_) => userDecryptionOptions); + async setUserDecryptionOptionsById( + userId: UserId, + userDecryptionOptions: UserDecryptionOptions, + ): Promise { + await this.singleUserStateProvider + .get(userId, USER_DECRYPTION_OPTIONS) + .update((_) => userDecryptionOptions); } } diff --git a/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts b/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts index d9749d9467c..b9bc9108e52 100644 --- a/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts @@ -48,6 +48,9 @@ export abstract class UserVerificationService { * @param userId The user id to check. If not provided, the current user is used * @returns True if the user has a master password * @deprecated Use UserDecryptionOptionsService.hasMasterPassword$ instead + * @remark To facilitate deprecation, many call sites were removed as part of PM-26413. + * Those remaining are blocked by currently-disallowed imports of auth/common. + * PM-27009 has been filed to track completion of this deprecation. */ abstract hasMasterPassword(userId?: string): Promise; /** diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts index e7fbc002af8..e570c0f4a43 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts @@ -3,10 +3,7 @@ import { of } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { - UserDecryptionOptions, - UserDecryptionOptionsServiceAbstraction, -} from "@bitwarden/auth/common"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { @@ -146,11 +143,7 @@ describe("UserVerificationService", () => { describe("server verification type", () => { it("correctly returns master password availability", async () => { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: true, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); const result = await sut.getAvailableVerificationOptions("server"); @@ -168,11 +161,7 @@ describe("UserVerificationService", () => { }); it("correctly returns OTP availability", async () => { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: false, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await sut.getAvailableVerificationOptions("server"); @@ -526,11 +515,7 @@ describe("UserVerificationService", () => { // Helpers function setMasterPasswordAvailability(hasMasterPassword: boolean) { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: hasMasterPassword, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(hasMasterPassword)); masterPasswordService.masterKeyHash$.mockReturnValue( of(hasMasterPassword ? "masterKeyHash" : null), ); diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.ts b/libs/common/src/auth/services/user-verification/user-verification.service.ts index 7a537b883e3..7d93120148b 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.ts @@ -258,16 +258,19 @@ export class UserVerificationService implements UserVerificationServiceAbstracti } async hasMasterPassword(userId?: string): Promise { - if (userId) { - const decryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), - ); + const resolvedUserId = userId ?? (await firstValueFrom(this.accountService.activeAccount$))?.id; - if (decryptionOptions?.hasMasterPassword != undefined) { - return decryptionOptions.hasMasterPassword; - } + if (!resolvedUserId) { + return false; } - return await firstValueFrom(this.userDecryptionOptionsService.hasMasterPassword$); + + // Ideally, this method would accept a UserId over string. To avoid scope creep in PM-26413, we are + // doing the cast here. Future work should be done to make this type-safe, and should be considered + // as part of PM-27009. + + return await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(resolvedUserId as UserId), + ); } async hasMasterPasswordAndMasterKeyHash(userId?: string): Promise { diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index aa14c7f0c4f..59bd7bc11f2 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map, Observable, Subject } from "rxjs"; +import { firstValueFrom, map, Observable, Subject, switchMap } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -9,6 +9,7 @@ import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common" // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; +import { AccountService } from "../../../auth/abstractions/account.service"; import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response"; import { DevicesApiServiceAbstraction } from "../../../auth/abstractions/devices-api.service.abstraction"; import { SecretVerificationRequest } from "../../../auth/models/request/secret-verification.request"; @@ -87,10 +88,18 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private logService: LogService, private configService: ConfigService, + private accountService: AccountService, ) { - this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( - map((options) => { - return options?.trustedDeviceOption != null; + this.supportsDeviceTrust$ = this.accountService.activeAccount$.pipe( + switchMap((account) => { + if (account == null) { + return [false]; + } + return this.userDecryptionOptionsService.userDecryptionOptionsById$(account.id).pipe( + map((options) => { + return options?.trustedDeviceOption != null; + }), + ); }), ); } diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts index e735295f42b..024a21766ee 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts @@ -914,7 +914,7 @@ describe("deviceTrustService", () => { platformUtilsService.supportsSecureStorage.mockReturnValue(supportsSecureStorage); decryptionOptions.next({} as any); - userDecryptionOptionsService.userDecryptionOptions$ = decryptionOptions; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(decryptionOptions); return new DeviceTrustService( keyGenerationService, @@ -930,6 +930,7 @@ describe("deviceTrustService", () => { userDecryptionOptionsService, logService, configService, + accountService, ); } }); diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts index f3fec6d849a..ba58fa80922 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts @@ -53,9 +53,11 @@ describe("VaultTimeoutSettingsService", () => { policyService = mock(); userDecryptionOptionsSubject = new BehaviorSubject(null); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; - userDecryptionOptionsService.hasMasterPassword$ = userDecryptionOptionsSubject.pipe( - map((options) => options?.hasMasterPassword ?? false), + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue( + userDecryptionOptionsSubject.pipe(map((options) => options?.hasMasterPassword ?? false)), ); userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( userDecryptionOptionsSubject, @@ -127,6 +129,23 @@ describe("VaultTimeoutSettingsService", () => { expect(result).not.toContain(VaultTimeoutAction.Lock); }); + + it("should return only LogOut when userId is not provided and there is no active account", async () => { + // Set up accountService to return null for activeAccount + accountService.activeAccount$ = of(null); + pinStateService.isPinSet.mockResolvedValue(false); + biometricStateService.biometricUnlockEnabled$ = of(false); + + // Call availableVaultTimeoutActions$ which internally calls userHasMasterPassword without a userId + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + // Since there's no active account, userHasMasterPassword returns false, + // meaning no master password is available, so Lock should not be available + expect(result).toEqual([VaultTimeoutAction.LogOut]); + expect(result).not.toContain(VaultTimeoutAction.Lock); + }); }); describe("canLock", () => { diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts index 4ca23bb24bf..00e53596de4 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts @@ -290,14 +290,19 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA } private async userHasMasterPassword(userId: string): Promise { + let resolvedUserId: UserId; if (userId) { - const decryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), - ); - - return !!decryptionOptions?.hasMasterPassword; + resolvedUserId = userId as UserId; } else { - return await firstValueFrom(this.userDecryptionOptionsService.hasMasterPassword$); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + if (!activeAccount) { + return false; // No account, can't have master password + } + resolvedUserId = activeAccount.id; } + + return await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(resolvedUserId), + ); } }