From 2b9d83ab39fece5bc6f9b3f9a5c79ee037edae8a Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 12 Aug 2025 12:34:56 -0400 Subject: [PATCH] feat(browser-approval): [PM-23620] Auth Request Answering Service - Correct text for notification. --- .../browser/src/background/main.background.ts | 36 +++++++++++++------ .../actions/browser-actions.service.ts | 4 +-- .../browser-system-notification.service.ts | 11 +++++- .../src/popup/services/services.module.ts | 7 +--- .../auth-request-answering.service.ts | 15 +++++--- .../system-notifications.service.ts | 6 +++- 6 files changed, 53 insertions(+), 26 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 7355051ddbc..5a90e747ffa 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -308,6 +308,7 @@ import CommandsBackground from "./commands.background"; import IdleBackground from "./idle.background"; import { NativeMessagingBackground } from "./nativeMessaging.background"; import RuntimeBackground from "./runtime.background"; +import { SystemNotificationPrefixes } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; export default class MainBackground { messagingService: MessageSender; @@ -1131,7 +1132,9 @@ export default class MainBackground { this.actionsService = new BrowserActionsService(this.logService, this.platformUtilsService); if ("notifications" in chrome) { - this.systemNotificationService = new BrowserSystemNotificationService(); + this.systemNotificationService = new BrowserSystemNotificationService( + this.platformUtilsService, + ); } else { this.systemNotificationService = new UnsupportedSystemNotificationsService(); } @@ -1146,16 +1149,6 @@ export default class MainBackground { this.systemNotificationService, ); - this.systemNotificationService.notificationClicked$ - .pipe( - filter((n) => n.id.startsWith("authRequest_")), - map((n) => ({ event: n, authRequestId: n.id.split("_")[1] })), - switchMap(({ event }) => - this.authRequestAnsweringService.handleAuthRequestNotificationClicked(event), - ), - ) - .subscribe(); - this.serverNotificationsService = new DefaultServerNotificationsService( this.logService, this.syncService, @@ -1413,6 +1406,10 @@ export default class MainBackground { this.accountService, this.authService, ); + + // Putting this here so that all other services are initialized prior to trying to hook up + // subscriptions to the notification chrome events. + this.initNotificationSubscriptions(); } async bootstrap() { @@ -1804,6 +1801,23 @@ export default class MainBackground { } } + /** + * This function is for creating any subscriptions for the background service worker. We do this + * here because it's important to run this during the evaluation period of the browser extension + * service worker. + */ + initNotificationSubscriptions() { + this.systemNotificationService.notificationClicked$ + .pipe( + filter((n) => n.id.startsWith(SystemNotificationPrefixes.AuthRequest + "_")), + map((n) => ({ event: n, authRequestId: n.id.split("_")[1] })), + switchMap(({ event }) => + this.authRequestAnsweringService.handleAuthRequestNotificationClicked(event), + ), + ) + .subscribe(); + } + /** * Temporary solution to handle initialization of the overlay background behind a feature flag. * Will be reverted to instantiation within the constructor once the feature flag is removed. diff --git a/apps/browser/src/platform/actions/browser-actions.service.ts b/apps/browser/src/platform/actions/browser-actions.service.ts index 112a76cbe3f..9d28240622c 100644 --- a/apps/browser/src/platform/actions/browser-actions.service.ts +++ b/apps/browser/src/platform/actions/browser-actions.service.ts @@ -26,7 +26,7 @@ export class BrowserActionsService implements ActionsService { return; } else { this.logService.warning( - `No openPopup function found on browser actions. On browser: ${deviceType} and manifest version: ${BrowserApi.manifestVersion}`, + `No openPopup function found on browser actions. On browser: ${DeviceType[deviceType]} and manifest version: ${BrowserApi.manifestVersion}`, ); } break; @@ -36,7 +36,7 @@ export class BrowserActionsService implements ActionsService { return; default: this.logService.warning( - `Tried to open the popup from an unsupported device type: ${deviceType}`, + `Tried to open the popup from an unsupported device type: ${DeviceType[deviceType]}`, ); } } catch (e) { diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index 9e4f2475511..e0b2716a19b 100644 --- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts +++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts @@ -1,5 +1,7 @@ import { map, merge, Observable } from "rxjs"; +import { DeviceType } from "@bitwarden/common/enums"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ButtonLocation, SystemNotificationClearInfo, @@ -13,7 +15,7 @@ import { fromChromeEvent } from "../browser/from-chrome-event"; export class BrowserSystemNotificationService implements SystemNotificationsService { notificationClicked$: Observable; - constructor() { + constructor(private readonly platformUtilsService: PlatformUtilsService) { this.notificationClicked$ = merge( fromChromeEvent(chrome.notifications.onButtonClicked).pipe( map(([notificationId, buttonIndex]) => ({ @@ -32,6 +34,8 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ async create(createInfo: SystemNotificationCreateInfo): Promise { return new Promise((resolve) => { + const deviceType: DeviceType = this.platformUtilsService.getDevice(); + const options: chrome.notifications.NotificationOptions = { iconUrl: chrome.runtime.getURL("images/icon128.png"), message: createInfo.body, @@ -40,6 +44,11 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ buttons: createInfo.buttons.map((value) => ({ title: value.title })), }; + // Firefox notification api does not support buttons. + if (deviceType === DeviceType.FirefoxExtension) { + delete options.buttons; + } + if (createInfo.id != null) { chrome.notifications.create(createInfo.id, options, (notificationId) => resolve(notificationId), diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index ba671462b28..b498c8433f3 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -596,7 +596,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: SystemNotificationsService, useClass: BrowserSystemNotificationService, - deps: [], + deps: [PlatformUtilsService], }), safeProvider({ provide: Fido2UserVerificationService, @@ -627,11 +627,6 @@ const safeProviders: SafeProvider[] = [ useClass: SsoUrlService, deps: [], }), - safeProvider({ - provide: SystemNotificationsService, - useClass: BrowserSystemNotificationService, - deps: [], - }), safeProvider({ provide: LoginComponentService, useClass: ExtensionLoginComponentService, diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts index 29f57acbc8c..460cc16122d 100644 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts @@ -12,6 +12,7 @@ import { ActionsService } from "@bitwarden/common/platform/actions"; import { ButtonLocation, SystemNotificationEvent, + SystemNotificationPrefixes, SystemNotificationsService, } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UserId } from "@bitwarden/user-core"; @@ -31,9 +32,9 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { if (event.buttonIdentifier === ButtonLocation.NotificationButton) { - // await this.systemNotificationsService.clear({ - // id: `authRequest_${event.id}`, - // }); + await this.systemNotificationsService.clear({ + id: `${event.id}`, + }); await this.actionService.openPopup(); } } @@ -54,10 +55,14 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA ) { // TODO: Handled in 14934 } else { + // Get the user's email to include in the system notification + const accounts = await firstValueFrom(this.accountService.accounts$); + const emailForUser = accounts?.[userId]?.email; + await this.systemNotificationsService.create({ - id: `authRequest_${authRequestId}`, + id: `${SystemNotificationPrefixes.AuthRequest}_${authRequestId}`, title: this.i18nService.t("accountAccessRequested"), - body: "Pending Auth Request to Approve (i18n)", + body: this.i18nService.t("confirmAccessAttempt", emailForUser), buttons: [], }); } diff --git a/libs/common/src/platform/system-notifications/system-notifications.service.ts b/libs/common/src/platform/system-notifications/system-notifications.service.ts index 54369231967..1cafc970a06 100644 --- a/libs/common/src/platform/system-notifications/system-notifications.service.ts +++ b/libs/common/src/platform/system-notifications/system-notifications.service.ts @@ -1,6 +1,10 @@ import { Observable } from "rxjs"; -// This is currently tailored for chrome extension's api, if safari works +export const SystemNotificationPrefixes = Object.freeze({ + AuthRequest: "authRequest", +}); + +// This is currently tailored for Chrome extension's api, if Safari works // differently where clicking a notification button produces a different // identifier we need to reconcile that here. export const ButtonLocation = Object.freeze({