diff --git a/apps/desktop/src/vault/app/vault/vault-v2.component.ts b/apps/desktop/src/vault/app/vault/vault-v2.component.ts index 217104b6bdc..fca81c449d9 100644 --- a/apps/desktop/src/vault/app/vault/vault-v2.component.ts +++ b/apps/desktop/src/vault/app/vault/vault-v2.component.ts @@ -23,9 +23,11 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -224,6 +226,7 @@ export class VaultV2Component private cipherArchiveService: CipherArchiveService, private policyService: PolicyService, private archiveCipherUtilitiesService: ArchiveCipherUtilitiesService, + private masterPasswordService: MasterPasswordServiceAbstraction, ) {} async ngOnInit() { @@ -333,13 +336,12 @@ export class VaultV2Component this.searchBarService.setEnabled(true); this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault")); - // If there are pending auth requests for this user, a LoginApprovalDialogComponent will open - this.messagingService.send("openLoginApproval"); - this.activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ).catch((): any => null); + await this.sendOpenLoginApprovalMessage(this.activeUserId); + if (this.activeUserId) { this.cipherService .failedToDecryptCiphers$(this.activeUserId) @@ -1005,4 +1007,27 @@ export class VaultV2Component } return repromptResult; } + + /** + * Sends a message that will retrieve any pending auth requests from the server. If there are + * pending auth requests for this user, a LoginApprovalDialogComponent will open. If there + * are no pending auth requests, nothing happens (see AppComponent: "openLoginApproval"). + */ + private async sendOpenLoginApprovalMessage(activeUserId: UserId) { + // This is a defensive check against a race condition where a user may have successfully logged + // in with no forceSetPasswordReason, but while the vault component is loading, a sync sets + // forceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission (see DefaultSyncService). + // This could potentially happen if an Admin upgrades the user's permissions as the user is logging + // in. In this rare case we do not want to send an "openLoginApproval" message. + // + // This also keeps parity with other usages of the "openLoginApproval" message. That is: don't send + // an "openLoginApproval" message if the user is required to set/change their password. + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason === ForceSetPasswordReason.None) { + // If there are pending auth requests for this user, a LoginApprovalDialogComponent will open + this.messagingService.send("openLoginApproval"); + } + } } diff --git a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts index a7d852c2b38..94ebdb0232f 100644 --- a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts @@ -267,6 +267,29 @@ describe("DefaultAuthRequestAnsweringService", () => { // Assert expect(messagingService.send).toHaveBeenCalledTimes(1); }); + + it("should NOT process pending auth requests when switching to an Unlocked user who is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); }); describe("authentication status transitions", () => { @@ -380,6 +403,24 @@ describe("DefaultAuthRequestAnsweringService", () => { expect(messagingService.send).toHaveBeenCalledTimes(2); expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); }); + + it("should NOT process pending auth requests when active account transitions to Unlocked but is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); }); describe("subscription cleanup", () => { diff --git a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts index 2515525dfb1..ea8067ed351 100644 --- a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts @@ -103,6 +103,16 @@ export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringS * approval dialog. */ private async processPendingAuthRequests(): Promise { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + // Only continue if the active user is not required to set/change their master password + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason !== ForceSetPasswordReason.None) { + return; + } + // Prune any stale pending requests (older than 15 minutes) // This comes from GlobalSettings.cs // public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15); @@ -114,7 +124,6 @@ export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringS (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; if (pendingAuthRequestsInState.length > 0) { - const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some( (e) => e.userId === activeUserId, );