bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-08 04:33:38 +00:00
Code Issues Releases Wiki Activity

add missing authRequestUserId error handling and update tests

Browse Source
This commit is contained in:
rr-bw
2025-11-12 13:09:44 -08:00
parent 58372116cf
commit 1536f93dd3
4 changed files with 94 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts
View File

@@ -85,12 +85,20 @@ describe("ExtensionAuthRequestAnsweringService", () => {
});
describe("receivedPendingAuthRequest()", () => {
it("should throw if authRequestUserId not given", async () => {
// Act
const promise = sut.receivedPendingAuthRequest(undefined, authRequestId);
// Assert
await expect(promise).rejects.toThrow("authRequestUserId required");
});
it("should throw if authRequestId not given", async () => {
// Act
const promise = sut.receivedPendingAuthRequest(userId, undefined);
// Assert
await expect(promise).rejects.toThrow("authRequestId not found.");
await expect(promise).rejects.toThrow("authRequestId required");
});
it("should add a pending marker for the user to state", async () => {
@@ -102,63 +110,69 @@ describe("ExtensionAuthRequestAnsweringService", () => {
expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId);
});
it("should send an 'openLoginApproval' message if the popup is open and the user is Unlocked, active, 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", () => {
describe("given the popup is open", () => {
it("should send an 'openLoginApproval' message", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
expect(messagingService.send).toHaveBeenCalledTimes(1);
expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", {
notificationId: authRequestId,
// Assert
expect(messagingService.send).toHaveBeenCalledTimes(1);
expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", {
notificationId: authRequestId,
});
});
it("should not create a system notification", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
expect(systemNotificationsService.create).not.toHaveBeenCalled();
});
});
});
it("should not send an 'openLoginApproval' message if the popup is closed", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
describe("given the popup is closed", () => {
it("should not send an 'openLoginApproval' message", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
expect(messagingService.send).not.toHaveBeenCalled();
});
// Assert
expect(messagingService.send).not.toHaveBeenCalled();
});
it("should create a system notification if the popup is closed", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
it("should create a system notification", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested");
expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com");
expect(systemNotificationsService.create).toHaveBeenCalledWith({
id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`,
title: "accountAccessRequested",
body: "confirmAccessAttempt:user@example.com",
buttons: [],
// Assert
expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested");
expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com");
expect(systemNotificationsService.create).toHaveBeenCalledWith({
id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`,
title: "accountAccessRequested",
body: "confirmAccessAttempt:user@example.com",
buttons: [],
});
});
});
});
it("should not create a system notification if the popup is open and the user is Unlocked, active, and not required to set/change their master password", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
expect(systemNotificationsService.create).not.toHaveBeenCalled();
});
});
describe("activeUserMeetsConditionsToShowApprovalDialog()", () => {

5
apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts
View File

@@ -48,8 +48,11 @@ export class ExtensionAuthRequestAnsweringService
authRequestUserId: UserId,
authRequestId: string,
): Promise<void> {
if (!authRequestUserId) {
throw new Error("authRequestUserId required");
}
if (!authRequestId) {
throw new Error("authRequestId not found.");
throw new Error("authRequestId required");
}
// Always persist the pending marker for this user to global state.

41
apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts
View File

@@ -76,6 +76,14 @@ describe("DesktopAuthRequestAnsweringService", () => {
});
describe("receivedPendingAuthRequest()", () => {
it("should throw if authRequestUserId not given", async () => {
// Act
const promise = sut.receivedPendingAuthRequest(undefined, undefined);
// Assert
await expect(promise).rejects.toThrow("authRequestUserId required");
});
it("should add a pending marker for the user to state", async () => {
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
@@ -85,7 +93,7 @@ describe("DesktopAuthRequestAnsweringService", () => {
expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId);
});
describe("given the user is Unlocked, active, and not required to set/change their master password", () => {
describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => {
describe("given the Desktop window is visible", () => {
it("should send an 'openLoginApproval' message", async () => {
// Arrange
@@ -149,7 +157,7 @@ describe("DesktopAuthRequestAnsweringService", () => {
});
});
describe("given the user is Locked", () => {
describe("given the active user is Locked", () => {
it("should NOT send an 'openLoginApproval' message", async () => {
// Arrange
(global as any).ipc.platform.isWindowVisible.mockResolvedValue(true);
@@ -179,21 +187,27 @@ describe("DesktopAuthRequestAnsweringService", () => {
});
});
describe("given the user is NOT the active user", () => {
it("should NOT send an 'openLoginApproval' message", async () => {
// Arrange
describe("given the active user is not the intended recipient of the auth request", () => {
beforeEach(() => {
// Different active user for these tests
const differentUserId = "different-user-id" as UserId;
(global as any).ipc.platform.isWindowVisible.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
accountService.activeAccount$ = of({
id: differentUserId,
email: "different@example.com",
emailVerified: true,
name: "Different User",
});
});
it("should NOT send an 'openLoginApproval' message", async () => {
// Arrange
(global as any).ipc.platform.isWindowVisible.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Pass in userId, not differentUserId (the active user), to mimic an auth
// request coming in for a user who is not the active user
await sut.receivedPendingAuthRequest(userId, authRequestId); // pass in userId, not differentUserId
// Assert
expect(messagingService.send).not.toHaveBeenCalled();
@@ -201,17 +215,12 @@ describe("DesktopAuthRequestAnsweringService", () => {
it("should create a system notification", async () => {
// Arrange
const differentUserId = "different-user-id" as UserId;
(global as any).ipc.platform.isWindowVisible.mockResolvedValue(true);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);
accountService.activeAccount$ = of({
id: differentUserId,
email: "different@example.com",
emailVerified: true,
name: "Different User",
});
// Act
// Pass in userId, not differentUserId (the active user), to mimic an auth
// request coming in for a user who is not the active user
await sut.receivedPendingAuthRequest(userId, authRequestId);
// Assert
@@ -223,7 +232,7 @@ describe("DesktopAuthRequestAnsweringService", () => {
});
});
describe("given the user is required to set/change their master password", () => {
describe("given the active user is required to set/change their master password", () => {
it("should NOT send an 'openLoginApproval' message", async () => {
// Arrange
(global as any).ipc.platform.isWindowVisible.mockResolvedValue(true);

4
apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts
View File

@@ -43,6 +43,10 @@ export class DesktopAuthRequestAnsweringService
authRequestUserId: UserId,
authRequestId: string,
): Promise<void> {
if (!authRequestUserId) {
throw new Error("authRequestUserId required");
}
// Always persist the pending marker for this user to global state.
await this.pendingAuthRequestsState.add(authRequestUserId);