diff --git a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts index 1507f4424fc..a7b2c858278 100644 --- a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts +++ b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts @@ -166,28 +166,30 @@ describe("ExtensionAuthRequestAnsweringService", () => { }); describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { - it("should return true if popup is open and user is active, the intended recipient of the auth request, unlocked, and not required to set/change their master password", async () => { - // Arrange - platformUtilsService.isPopupOpen.mockResolvedValue(true); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + it("should return true if popup is open", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - // Act - const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); - // Assert - expect(result).toBe(true); - }); + // Assert + expect(result).toBe(true); + }); - it("should return false if popup is closed", async () => { - // Arrange - platformUtilsService.isPopupOpen.mockResolvedValue(false); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + it("should return false if popup is closed", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - // Act - const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); - // Assert - expect(result).toBe(false); + // Assert + expect(result).toBe(false); + }); }); }); 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 3cd15a1c4fa..a7d852c2b38 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 @@ -64,25 +64,9 @@ describe("DefaultAuthRequestAnsweringService", () => { }); describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { - it("should return true if user is active, the intended recipient of the auth request, Unlocked, and not required to set/change their master password", async () => { + it("should return false if there is no active user", async () => { // Arrange - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - - // Act - const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); - - // Assert - expect(result).toBe(true); - }); - - it("should return false if user is not the active user", async () => { - // Arrange - accountService.activeAccount$ = of({ - id: otherUserId, - email: "other@example.com", - emailVerified: true, - name: "Other User", - }); + accountService.activeAccount$ = of(null); // Act const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); @@ -91,7 +75,7 @@ describe("DefaultAuthRequestAnsweringService", () => { expect(result).toBe(false); }); - it("should return false if active user is not the intended recipient of the auth request", async () => { + it("should return false if the active user is not the intended recipient of the auth request", async () => { // Arrange authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); @@ -102,7 +86,7 @@ describe("DefaultAuthRequestAnsweringService", () => { expect(result).toBe(false); }); - it("should return false if user is not Unlocked", async () => { + it("should return false if the active user is not unlocked", async () => { // Arrange authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); @@ -113,18 +97,7 @@ describe("DefaultAuthRequestAnsweringService", () => { expect(result).toBe(false); }); - it("should return false if user is not Unlocked", async () => { - // Arrange - authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); - - // Act - const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); - - // Assert - expect(result).toBe(false); - }); - - it("should return false if active user is required to set/change their master password", async () => { + it("should return false if the active user is required to set/change their master password", async () => { // Arrange masterPasswordService.forceSetPasswordReason$.mockReturnValue( of(ForceSetPasswordReason.WeakMasterPassword), @@ -136,6 +109,17 @@ describe("DefaultAuthRequestAnsweringService", () => { // Assert expect(result).toBe(false); }); + + it("should return true if the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(true); + }); }); describe("setupUnlockListenersForProcessingAuthRequests()", () => { 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 accb1a7d6c7..2515525dfb1 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 @@ -39,8 +39,7 @@ export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringS ) {} async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { - // If the active user is not the user that the auth request is for, return false - // early (no reason to perform the following async calls) + // If the active user is not the intended recipient of the auth request, return false const activeUserId: UserId | null = await firstValueFrom( this.accountService.activeAccount$.pipe(getOptionalUserId), ); @@ -48,11 +47,14 @@ export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringS return false; } + // If the active user is not unlocked, return false const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); if (authStatus !== AuthenticationStatus.Unlocked) { return false; } + // If the active user is required to set/change their master password, return false + // Note that by this point we know that the authRequestUserId is the active UserId (see check above) const forceSetPasswordReason = await firstValueFrom( this.masterPasswordService.forceSetPasswordReason$(authRequestUserId), );