Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ade20dd
separate extension/default services
rr-bw Oct 16, 2025
02a04de
update abstraction
rr-bw Oct 17, 2025
ad43e63
create DesktopAuthRequestAnsweringService
rr-bw Oct 17, 2025
39bce23
rename variable
rr-bw Oct 19, 2025
8e8a08e
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 19, 2025
1f96f17
extract listener setup
rr-bw Oct 20, 2025
13d7bcf
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 20, 2025
68a9976
add tests
rr-bw Oct 20, 2025
c544a40
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 20, 2025
34b5f35
update imports
rr-bw Oct 20, 2025
faa715c
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 28, 2025
7ba4672
update authRequestId param requirement and add clarifying comment to โ€ฆ
rr-bw Oct 29, 2025
90be473
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 29, 2025
325a3dd
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 30, 2025
5108949
remove received...() method from Noop, and check its existence beforeโ€ฆ
rr-bw Oct 31, 2025
5e8f24d
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Oct 31, 2025
d503f17
add tests for subscriptions
rr-bw Oct 31, 2025
ed3dea2
update desktop service tests
rr-bw Nov 2, 2025
4140c49
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 2, 2025
7cda1dc
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 4, 2025
1c81266
merge main
rr-bw Nov 5, 2025
6a7b519
on extension, include the authRequestId in the send message
rr-bw Nov 6, 2025
96d6046
update Desktop VaultV2Component message sending
rr-bw Nov 7, 2025
ad8b433
update tests for DefaultServerNotifications
rr-bw Nov 7, 2025
d50f06a
update test in ExtensionAuthRequestAnsweringService
rr-bw Nov 7, 2025
6f1179c
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 7, 2025
84df82e
restructure service architecture
rr-bw Nov 8, 2025
62d9cad
update desktop service tests
rr-bw Nov 8, 2025
ed72ea4
update tests for DefaultAuthRequestAnsweringService
rr-bw Nov 8, 2025
5308685
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 10, 2025
8f79c92
update abstraction
rr-bw Nov 10, 2025
5b23ff6
address PR feedback: 'missing null safety check'
rr-bw Nov 10, 2025
458227c
update activeUserMeetsConditionsToShowApprovalDialog() and param nameโ€ฆ
rr-bw Nov 11, 2025
3683c21
update service test descriptions for clarity
rr-bw Nov 11, 2025
b9d9e30
address PR feedback 'Race condition with concurrent auth requests'
rr-bw Nov 11, 2025
859494f
merge main
rr-bw Nov 11, 2025
11b7304
fix typo
rr-bw Nov 12, 2025
4150ad0
add try/catch when calling receivedPendingAuthRequest()
rr-bw Nov 12, 2025
6c49612
address PR feedback: 'Consider error handling in do/while loop' and pโ€ฆ
rr-bw Nov 12, 2025
8e81218
update variable name
rr-bw Nov 12, 2025
23af699
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 12, 2025
5837211
remove validationService and update logService messages
rr-bw Nov 12, 2025
1536f93
add missing authRequestUserId error handling and update tests
rr-bw Nov 12, 2025
3757c06
add defensive checks for forceSetPasswordReason
rr-bw Nov 13, 2025
aecdf3c
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 13, 2025
37db0d5
Merge branch 'main' into auth/pm-26209/bugfix-desktop-error-on-auth-rโ€ฆ
rr-bw Nov 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -17,32 +19,34 @@ import {
} from "@bitwarden/common/platform/system-notifications/system-notifications.service";
import { UserId } from "@bitwarden/user-core";

import { AuthRequestAnsweringService } from "./auth-request-answering.service";
import { PendingAuthRequestsStateService } from "./pending-auth-requests.state";
import { ExtensionAuthRequestAnsweringService } from "./extension-auth-request-answering.service";

describe("AuthRequestAnsweringService", () => {
describe("ExtensionAuthRequestAnsweringService", () => {
let accountService: MockProxy<AccountService>;
let actionService: MockProxy<ActionsService>;
let authService: MockProxy<AuthService>;
let i18nService: MockProxy<I18nService>;
let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$
let messagingService: MockProxy<MessagingService>;
let pendingAuthRequestsState: MockProxy<PendingAuthRequestsStateService>;
let actionService: MockProxy<ActionsService>;
let i18nService: MockProxy<I18nService>;
let platformUtilsService: MockProxy<PlatformUtilsService>;
let systemNotificationsService: MockProxy<SystemNotificationsService>;

let sut: AuthRequestAnsweringService;

const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId;
const authRequestId = "auth-request-id-123";

beforeEach(() => {
accountService = mock<AccountService>();
actionService = mock<ActionsService>();
authService = mock<AuthService>();
i18nService = mock<I18nService>();
masterPasswordService = { forceSetPasswordReason$: jest.fn() };
masterPasswordService = {
forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)),
};
messagingService = mock<MessagingService>();
pendingAuthRequestsState = mock<PendingAuthRequestsStateService>();
actionService = mock<ActionsService>();
i18nService = mock<I18nService>();
platformUtilsService = mock<PlatformUtilsService>();
systemNotificationsService = mock<SystemNotificationsService>();

Expand All @@ -57,63 +61,77 @@ describe("AuthRequestAnsweringService", () => {
accountService.accounts$ = of({
[userId]: { email: "[email protected]", emailVerified: true, name: "User" },
});
(masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue(
of(ForceSetPasswordReason.None),
);
platformUtilsService.isPopupOpen.mockResolvedValue(false);
i18nService.t.mockImplementation(
(key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`,
);
systemNotificationsService.create.mockResolvedValue("notif-id");

sut = new AuthRequestAnsweringService(
sut = new ExtensionAuthRequestAnsweringService(
accountService,
actionService,
authService,
i18nService,
masterPasswordService,
messagingService,
pendingAuthRequestsState,
actionService,
i18nService,
platformUtilsService,
systemNotificationsService,
);
});

describe("handleAuthRequestNotificationClicked", () => {
it("clears notification and opens popup when notification body is clicked", async () => {
const event: SystemNotificationEvent = {
id: "123",
buttonIdentifier: ButtonLocation.NotificationButton,
};
describe("receivedPendingAuthRequest()", () => {
it("should throw if authRequestId not given", async () => {
// Act
const promise = sut.receivedPendingAuthRequest(userId, undefined);

await sut.handleAuthRequestNotificationClicked(event);
// Assert
await expect(promise).rejects.toThrow("authRequestId not found.");
});

expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" });
expect(actionService.openPopup).toHaveBeenCalledTimes(1);
it("should add a pending marker for the user to state", async () => {
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);

// Assert
expect(pendingAuthRequestsState.add).toHaveBeenCalledTimes(1);
expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId);
});

it("does nothing when an optional button is clicked", async () => {
const event: SystemNotificationEvent = {
id: "123",
buttonIdentifier: ButtonLocation.FirstOptionalButton,
};
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);

await sut.handleAuthRequestNotificationClicked(event);
// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);

expect(systemNotificationsService.clear).not.toHaveBeenCalled();
expect(actionService.openPopup).not.toHaveBeenCalled();
// Assert
expect(messagingService.send).toHaveBeenCalledTimes(1);
expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval");
});
});

describe("receivedPendingAuthRequest", () => {
const authRequestId = "req-abc";
it("should not send an 'openLoginApproval' message if the popup is closed", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);

// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);

// Assert
expect(messagingService.send).not.toHaveBeenCalled();
});

it("creates a system notification when popup is not open", async () => {
it("should create a system notification if the popup is closed", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);

// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);

// Assert
expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested");
expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "[email protected]");
expect(systemNotificationsService.create).toHaveBeenCalledWith({
Expand All @@ -124,16 +142,74 @@ describe("AuthRequestAnsweringService", () => {
});
});

it("does not create a notification when popup is open, user is active, unlocked, and no force set password", async () => {
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);
(masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue(
of(ForceSetPasswordReason.None),
);

// Act
await sut.receivedPendingAuthRequest(userId, authRequestId);

// Assert
expect(systemNotificationsService.create).not.toHaveBeenCalled();
});
});

describe("userMeetsConditionsToShowApprovalDialog()", () => {
it("should return true if popup is open and 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
const result = await sut.userMeetsConditionsToShowApprovalDialog(userId);

// Assert
expect(result).toBe(true);
});

it("should return false if popup is closed", async () => {
// Arrange
platformUtilsService.isPopupOpen.mockResolvedValue(false);
authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked);

// Act
const result = await sut.userMeetsConditionsToShowApprovalDialog(userId);

// Assert
expect(result).toBe(false);
});
});

describe("handleAuthRequestNotificationClicked()", () => {
it("should clear notification and open popup when notification body is clicked", async () => {
// Arrange
const event: SystemNotificationEvent = {
id: "123",
buttonIdentifier: ButtonLocation.NotificationButton,
};

// Act
await sut.handleAuthRequestNotificationClicked(event);

// Assert
expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" });
expect(actionService.openPopup).toHaveBeenCalledTimes(1);
});

it("should do nothing when an optional notification button is clicked", async () => {
// Arrange
const event: SystemNotificationEvent = {
id: "123",
buttonIdentifier: ButtonLocation.FirstOptionalButton,
};

// Act
await sut.handleAuthRequestNotificationClicked(event);

// Assert
expect(systemNotificationsService.clear).not.toHaveBeenCalled();
expect(actionService.openPopup).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { firstValueFrom } from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags";
import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service";
import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { ActionsService } from "@bitwarden/common/platform/actions";
import {
ButtonLocation,
SystemNotificationEvent,
SystemNotificationsService,
} from "@bitwarden/common/platform/system-notifications/system-notifications.service";
import { UserId } from "@bitwarden/user-core";

export class ExtensionAuthRequestAnsweringService
extends DefaultAuthRequestAnsweringService
implements AuthRequestAnsweringService
{
constructor(
protected readonly accountService: AccountService,
protected readonly authService: AuthService,
protected readonly masterPasswordService: MasterPasswordServiceAbstraction,
protected readonly messagingService: MessagingService,
protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService,
private readonly actionService: ActionsService,
private readonly i18nService: I18nService,
private readonly platformUtilsService: PlatformUtilsService,
private readonly systemNotificationsService: SystemNotificationsService,
) {
super(
accountService,
authService,
masterPasswordService,
messagingService,
pendingAuthRequestsState,
);
}

override async receivedPendingAuthRequest(userId: UserId, authRequestId?: string): Promise<void> {
if (!authRequestId) {
throw new Error("authRequestId not found.");
}

// Always persist the pending marker for this user to global state.
await this.pendingAuthRequestsState.add(userId);

const userIsAvailableToViewDialog = await this.userMeetsConditionsToShowApprovalDialog(userId);

if (userIsAvailableToViewDialog) {
// Send message to open dialog immediately for this request
this.messagingService.send("openLoginApproval");
} else {
// Create a system notification
const accounts = await firstValueFrom(this.accountService.accounts$);
const emailForUser = accounts[userId].email;
await this.systemNotificationsService.create({
id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, // the underscore is an important delimiter.
title: this.i18nService.t("accountAccessRequested"),
body: this.i18nService.t("confirmAccessAttempt", emailForUser),
buttons: [],
});
}
}

async userMeetsConditionsToShowApprovalDialog(userId: UserId): Promise<boolean> {
const meetsBasicConditions = await super.userMeetsConditionsToShowApprovalDialog(userId);

// To show an approval dialog immediately on Extension, the popup must be open.
const isPopupOpen = await this.platformUtilsService.isPopupOpen();
const meetsExtensionConditions = meetsBasicConditions && isPopupOpen;

return meetsExtensionConditions;
}

/**
* When a system notification is clicked, this function is used to process that event.
*
* @param event The event passed in. Check initNotificationSubscriptions in main.background.ts.
*/
async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise<void> {
if (event.buttonIdentifier === ButtonLocation.NotificationButton) {
await this.systemNotificationsService.clear({
id: `${event.id}`,
});
await this.actionService.openPopup();
}
}
}
12 changes: 6 additions & 6 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { DefaultPolicyService } from "@bitwarden/common/admin-console/services/p
import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service";
import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service";
import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service";
import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service";
import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction";
Expand All @@ -51,7 +51,6 @@ import { UserVerificationService as UserVerificationServiceAbstraction } from "@
import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/auth-request-answering.service";
import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
import { AuthService } from "@bitwarden/common/auth/services/auth.service";
import { AvatarService } from "@bitwarden/common/auth/services/avatar.service";
Expand Down Expand Up @@ -268,6 +267,7 @@ import {
VaultExportServiceAbstraction,
} from "@bitwarden/vault-export-core";

import { ExtensionAuthRequestAnsweringService } from "../auth/services/auth-request-answering/extension-auth-request-answering.service";
import { AuthStatusBadgeUpdaterService } from "../auth/services/auth-status-badge-updater.service";
import { OverlayNotificationsBackground as OverlayNotificationsBackgroundInterface } from "../autofill/background/abstractions/overlay-notifications.background";
import { OverlayBackground as OverlayBackgroundInterface } from "../autofill/background/abstractions/overlay.background";
Expand Down Expand Up @@ -379,7 +379,7 @@ export default class MainBackground {
serverNotificationsService: ServerNotificationsService;
systemNotificationService: SystemNotificationsService;
actionsService: ActionsService;
authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction;
authRequestAnsweringService: AuthRequestAnsweringService;
stateService: StateServiceAbstraction;
userNotificationSettingsService: UserNotificationSettingsServiceAbstraction;
autofillSettingsService: AutofillSettingsServiceAbstraction;
Expand Down Expand Up @@ -1186,14 +1186,14 @@ export default class MainBackground {

this.pendingAuthRequestStateService = new PendingAuthRequestsStateService(this.stateProvider);

this.authRequestAnsweringService = new AuthRequestAnsweringService(
this.authRequestAnsweringService = new ExtensionAuthRequestAnsweringService(
this.accountService,
this.actionsService,
this.authService,
this.i18nService,
this.masterPasswordService,
this.messagingService,
this.pendingAuthRequestStateService,
this.actionsService,
this.i18nService,
this.platformUtilsService,
this.systemNotificationService,
);
Expand Down
Loading
Loading