Feat PM-19877 System Notification Processing (#15611)

* feat(notification-processing): [PM-19877] System Notification Implementation - Minor changes to popup logic and removed content in login component.

* docs(notification-processing): [PM-19877] System Notification Implementation - Added more docs.

* docs(notification-processing): [PM-19877] System Notification Implementation - Added markdown document.

* fix(notification-processing): [PM-19877] System Notification Implementation - Updated condition for if notification is supported.

* fix(notification-processing): [PM-19877] System Notification Implementation - Updated services module with correct platform utils service.

libs/common/src/platform/abstractions/platform-utils.service.ts

@@ -22,7 +22,7 @@ export abstract class PlatformUtilsService {
   abstract isVivaldi(): boolean;
   abstract isSafari(): boolean;
   abstract isMacAppStore(): boolean;
-  abstract isViewOpen(): Promise<boolean>;
+  abstract isPopupOpen(): Promise<boolean>;
   abstract launchUri(uri: string, options?: any): void;
   abstract getApplicationVersion(): Promise<string>;
   abstract getApplicationVersionNumber(): Promise<string>;

libs/common/src/platform/actions/README.md

@@ -0,0 +1,40 @@
+# Platform Actions API
+
+## ActionsService.openPopup()
+
+This document outlines the current behavior of `ActionsService.openPopup()` across different browsers, specifically in two contexts:
+
+- **Window Context**: When the call is triggered from an active browser window (e.g., from a tab's script).
+- **Background Service Worker Context**: When the call is made from a background context, such as a service worker.
+
+The `openPopup()` method has limitations in some environments due to browser-specific restrictions or bugs. Below is a compatibility chart detailing the observed behavior.
+
+---
+
+## Compatibility Table
+
+| Browser | Window Context      | Background Service Worker Context |
+| ------- | ------------------- | --------------------------------- |
+| Safari  | ✅ Works            | ❌ Fails                          |
+| Firefox | ❌ Fails            | ❌ Fails                          |
+| Chrome  | ✅ Works            | ✅ Works                          |
+| Edge    | 🟡 Untested         | 🟡 Untested                       |
+| Vivaldi | ⚠️ Ambiguous (Bug?) | ⚠️ Ambiguous (Bug?)               |
+| Opera   | ✅ Works            | ❌ Fails silently                 |
+
+---
+
+## Notes
+
+- **Safari**: Only works when `openPopup()` is triggered from a window context. Attempts from background service workers fail.
+- **Firefox**: Does not appear to support `openPopup()` in either context.
+- **Chrome**: Fully functional in both contexts, but only on Mac. Windows it does not work in.
+- **Edge**: Behavior has not been tested.
+- **Vivaldi**: `openPopup()` results in an error that _might_ be related to running in a background context, but the cause is currently unclear.
+- **Opera**: Works from window context. Background calls fail silently with no error message.
+
+---
+
+## Summary
+
+When implementing `ActionsService.openPopup()`, prefer triggering it from a window context whenever possible to maximize cross-browser compatibility. Full background service worker support is only reliable in **Chrome**.

libs/common/src/platform/actions/actions-service.ts

@@ -0,0 +1,6 @@
+export abstract class ActionsService {
+  /**
+   * Opens the popup if it is supported.
+   */
+  abstract openPopup(): Promise<void>;
+}

libs/common/src/platform/actions/index.ts

@@ -0,0 +1 @@
+export { ActionsService } from "./actions-service";

libs/common/src/platform/actions/unsupported-actions.service.ts

@@ -0,0 +1,7 @@
+import { ActionsService } from "./actions-service";
+
+export class UnsupportedActionsService implements ActionsService {
+  openPopup(): Promise<void> {
+    throw new Error("Open Popup unsupported.");
+  }
+}

libs/common/src/platform/notifications/index.ts

@@ -1 +0,0 @@
-export { NotificationsService } from "./notifications.service";

libs/common/src/platform/server-notifications/index.ts

@@ -0,0 +1 @@
+export { ServerNotificationsService } from "./server-notifications.service";

libs/common/src/platform/server-notifications/internal/default-notifications.service.spec.ts

@@ -21,9 +21,9 @@ import { SupportStatus } from "../../misc/support-status";
 import { SyncService } from "../../sync";
 
 import {
-  DefaultNotificationsService,
+  DefaultServerNotificationsService,
   DISABLED_NOTIFICATIONS_URL,
-} from "./default-notifications.service";
+} from "./default-server-notifications.service";
 import { SignalRConnectionService, SignalRNotification } from "./signalr-connection.service";
 import { WebPushConnectionService, WebPushConnector } from "./webpush-connection.service";
 import { WorkerWebPushConnectionService } from "./worker-webpush-connection.service";
@@ -52,7 +52,7 @@ describe("NotificationsService", () => {
     notificationsUrl: string,
   ) => Subject<SignalRNotification>;
 
-  let sut: DefaultNotificationsService;
+  let sut: DefaultServerNotificationsService;
 
   beforeEach(() => {
     syncService = mock<SyncService>();
@@ -93,7 +93,7 @@ describe("NotificationsService", () => {
       () => new Subject<SignalRNotification>(),
     );
 
-    sut = new DefaultNotificationsService(
+    sut = new DefaultServerNotificationsService(
       mock<LogService>(),
       syncService,
       appIdService,
@@ -134,7 +134,7 @@ describe("NotificationsService", () => {
     expect(actualNotification.type).toBe(expectedType);
   };
 
-  it("emits notifications through WebPush when supported", async () => {
+  it("emits server notifications through WebPush when supported", async () => {
     const notificationsPromise = firstValueFrom(sut.notifications$.pipe(bufferCount(2)));
 
     emitActiveUser(mockUser1);
@@ -227,7 +227,7 @@ describe("NotificationsService", () => {
   });
 
   it.each([
-    // Temporarily rolling back notifications being connected while locked
+    // Temporarily rolling back server notifications being connected while locked
     // { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Unlocked },
     // { initialStatus: AuthenticationStatus.Unlocked, updatedStatus: AuthenticationStatus.Locked },
     // { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Locked },
@@ -256,7 +256,7 @@ describe("NotificationsService", () => {
   );
 
   it.each([
-    // Temporarily disabling notifications connecting while in a locked state
+    // Temporarily disabling server notifications connecting while in a locked state
     // AuthenticationStatus.Locked,
     AuthenticationStatus.Unlocked,
   ])(
@@ -282,7 +282,7 @@ describe("NotificationsService", () => {
     },
   );
 
-  it("does not connect to any notification stream when notifications are disabled through special url", () => {
+  it("does not connect to any notification stream when server notifications are disabled through special url", () => {
     const subscription = sut.notifications$.subscribe();
     emitActiveUser(mockUser1);
     emitNotificationUrl(DISABLED_NOTIFICATIONS_URL);

libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts

@@ -32,14 +32,14 @@ import { EnvironmentService } from "../../abstractions/environment.service";
 import { LogService } from "../../abstractions/log.service";
 import { MessagingService } from "../../abstractions/messaging.service";
 import { supportSwitch } from "../../misc/support-status";
-import { NotificationsService as NotificationsServiceAbstraction } from "../notifications.service";
+import { ServerNotificationsService } from "../server-notifications.service";
 
 import { ReceiveMessage, SignalRConnectionService } from "./signalr-connection.service";
 import { WebPushConnectionService } from "./webpush-connection.service";
 
 export const DISABLED_NOTIFICATIONS_URL = "http://-";
 
-export class DefaultNotificationsService implements NotificationsServiceAbstraction {
+export class DefaultServerNotificationsService implements ServerNotificationsService {
   notifications$: Observable<readonly [NotificationResponse, UserId]>;
 
   private activitySubject = new BehaviorSubject<"active" | "inactive">("active");
@@ -61,7 +61,7 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
       distinctUntilChanged(),
       switchMap((activeAccountId) => {
         if (activeAccountId == null) {
-          // We don't emit notifications for inactive accounts currently
+          // We don't emit server-notifications for inactive accounts currently
           return EMPTY;
         }
 
@@ -74,8 +74,8 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
   }
 
   /**
-   * Retrieves a stream of push notifications for the given user.
-   * @param userId The user id of the user to get the push notifications for.
+   * Retrieves a stream of push server notifications for the given user.
+   * @param userId The user id of the user to get the push server notifications for.
    */
   private userNotifications$(userId: UserId) {
     return this.environmentService.environment$.pipe(
@@ -109,7 +109,7 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
       }),
       supportSwitch({
         supported: (service) => {
-          this.logService.info("Using WebPush for notifications");
+          this.logService.info("Using WebPush for server notifications");
           return service.notifications$.pipe(
             catchError((err: unknown) => {
               this.logService.warning("Issue with web push, falling back to SignalR", err);
@@ -118,7 +118,7 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
           );
         },
         notSupported: () => {
-          this.logService.info("Using SignalR for notifications");
+          this.logService.info("Using SignalR for server notifications");
           return this.connectSignalR$(userId, notificationsUrl);
         },
       }),
@@ -188,7 +188,6 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
       case NotificationType.SyncCiphers:
       case NotificationType.SyncSettings:
         await this.syncService.fullSync(false);
-
         break;
       case NotificationType.SyncOrganizations:
         // An organization update may not have bumped the user's account revision date, so force a sync
@@ -214,11 +213,11 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
         await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification);
         break;
       case NotificationType.AuthRequest:
-        {
-          this.messagingService.send("openLoginApproval", {
-            notificationId: notification.payload.id,
-          });
-        }
+        // create notification
+
+        this.messagingService.send("openLoginApproval", {
+          notificationId: notification.payload.id,
+        });
         break;
       case NotificationType.SyncOrganizationStatusChanged:
         await this.syncService.fullSync(true);
@@ -237,7 +236,8 @@ export class DefaultNotificationsService implements NotificationsServiceAbstract
         mergeMap(async ([notification, userId]) => this.processNotification(notification, userId)),
       )
       .subscribe({
-        error: (e: unknown) => this.logService.warning("Error in notifications$ observable", e),
+        error: (e: unknown) =>
+          this.logService.warning("Error in server notifications$ observable", e),
       });
   }
 

libs/common/src/platform/server-notifications/internal/index.ts

@@ -1,7 +1,7 @@
 export * from "./worker-webpush-connection.service";
 export * from "./signalr-connection.service";
-export * from "./default-notifications.service";
-export * from "./noop-notifications.service";
+export * from "./default-server-notifications.service";
+export * from "./noop-server-notifications.service";
 export * from "./unsupported-webpush-connection.service";
 export * from "./webpush-connection.service";
 export * from "./websocket-webpush-connection.service";

libs/common/src/platform/server-notifications/internal/noop-server-notifications.service.ts

@@ -4,16 +4,16 @@ import { NotificationResponse } from "@bitwarden/common/models/response/notifica
 import { UserId } from "@bitwarden/common/types/guid";
 
 import { LogService } from "../../abstractions/log.service";
-import { NotificationsService } from "../notifications.service";
+import { ServerNotificationsService } from "../server-notifications.service";
 
-export class NoopNotificationsService implements NotificationsService {
+export class NoopServerNotificationsService implements ServerNotificationsService {
   notifications$: Observable<readonly [NotificationResponse, UserId]> = new Subject();
 
   constructor(private logService: LogService) {}
 
   startListening(): Subscription {
     this.logService.info(
-      "Initializing no-op notification service, no push notifications will be received",
+      "Initializing no-op notification service, no push server notifications will be received",
     );
     return Subscription.EMPTY;
   }

libs/common/src/platform/server-notifications/internal/web-push-notifications-api.service.ts

@@ -10,7 +10,7 @@ export class WebPushNotificationsApiService {
   ) {}
 
   /**
-   * Posts a device-user association to the server and ensures it's installed for push notifications
+   * Posts a device-user association to the server and ensures it's installed for push server notifications
    */
   async putSubscription(pushSubscription: PushSubscriptionJSON): Promise<void> {
     const request = WebPushRequest.from(pushSubscription);

libs/common/src/platform/server-notifications/internal/worker-webpush-connection.service.ts

@@ -40,7 +40,7 @@ interface PushEvent {
 }
 
 /**
- * An implementation for connecting to web push based notifications running in a Worker.
+ * An implementation for connecting to web push based server notifications running in a Worker.
  */
 export class WorkerWebPushConnectionService implements WebPushConnectionService {
   private pushEvent = new Subject<PushEvent>();
@@ -75,7 +75,7 @@ export class WorkerWebPushConnectionService implements WebPushConnectionService
   }
 
   supportStatus$(userId: UserId): Observable<SupportStatus<WebPushConnector>> {
-    // Check the server config to see if it supports sending WebPush notifications
+    // Check the server config to see if it supports sending WebPush server notifications
     // FIXME: get config of server for the specified userId, once ConfigService supports it
     return this.configService.serverConfig$.pipe(
       map((config) =>

libs/common/src/platform/server-notifications/server-notifications.service.ts

@@ -4,20 +4,20 @@ import { NotificationResponse } from "@bitwarden/common/models/response/notifica
 import { UserId } from "@bitwarden/common/types/guid";
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars -- Needed to link to API
-import type { DefaultNotificationsService } from "./internal";
+import type { DefaultServerNotificationsService } from "./internal";
 
 /**
  * A service offering abilities to interact with push notifications from the server.
  */
-export abstract class NotificationsService {
+export abstract class ServerNotificationsService {
   /**
    * @deprecated This method should not be consumed, an observable to listen to server
    * notifications will be available one day but it is not ready to be consumed generally.
-   * Please add code reacting to notifications in {@link DefaultNotificationsService.processNotification}
+   * Please add code reacting to server notifications in {@link DefaultServerNotificationsService.processNotification}
    */
   abstract notifications$: Observable<readonly [NotificationResponse, UserId]>;
   /**
-   * Starts automatic listening and processing of notifications, should only be called once per application,
+   * Starts automatic listening and processing of server notifications, should only be called once per application,
    * or you will risk notifications being processed multiple times.
    */
   abstract startListening(): Subscription;

libs/common/src/platform/sync/core-sync.service.ts

@@ -230,7 +230,7 @@ export abstract class CoreSyncService implements SyncService {
         }),
       ),
     );
-    // Process only notifications for currently active user when user is not logged out
+    // Process only server notifications for currently active user when user is not logged out
     // TODO: once send service allows data manipulation of non-active users, this should process any received notification
     if (activeUserId === notification.userId && status !== AuthenticationStatus.LoggedOut) {
       try {

libs/common/src/platform/system-notifications/index.ts

@@ -0,0 +1 @@
+export { SystemNotificationsService } from "./system-notifications.service";

libs/common/src/platform/system-notifications/system-notifications.service.ts

@@ -0,0 +1,58 @@
+import { Observable } from "rxjs";
+
+// 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({
+  FirstOptionalButton: 0, // this is the first optional button we can set
+  SecondOptionalButton: 1, // this is the second optional button we can set
+  NotificationButton: 2, // this is when you click the notification as a whole
+});
+
+export type ButtonLocationKeys = (typeof ButtonLocation)[keyof typeof ButtonLocation];
+
+export type SystemNotificationsButton = {
+  title: string;
+};
+
+export type SystemNotificationCreateInfo = {
+  id?: string;
+  title: string;
+  body: string;
+  buttons: SystemNotificationsButton[];
+};
+
+export type SystemNotificationClearInfo = {
+  id: string;
+};
+
+export type SystemNotificationEvent = {
+  id: string;
+  buttonIdentifier: number;
+};
+
+/**
+ * A service responsible for displaying operating system level server notifications.
+ */
+export abstract class SystemNotificationsService {
+  abstract notificationClicked$: Observable<SystemNotificationEvent>;
+
+  /**
+   * Creates a notification.
+   * @param createInfo
+   * @returns If a notification is successfully created it will respond back with an
+   *          id that refers to a notification.
+   */
+  abstract create(createInfo: SystemNotificationCreateInfo): Promise<string>;
+
+  /**
+   * Clears a notification.
+   * @param clearInfo Any info needed required to clear a notification.
+   */
+  abstract clear(clearInfo: SystemNotificationClearInfo): Promise<void>;
+
+  /**
+   * Used to know if a given platform supports server notifications.
+   */
+  abstract isSupported(): boolean;
+}

libs/common/src/platform/system-notifications/unsupported-system-notifications.service.ts

@@ -0,0 +1,23 @@
+import { throwError } from "rxjs";
+
+import {
+  SystemNotificationClearInfo,
+  SystemNotificationCreateInfo,
+  SystemNotificationsService,
+} from "./system-notifications.service";
+
+export class UnsupportedSystemNotificationsService implements SystemNotificationsService {
+  notificationClicked$ = throwError(() => new Error("Notification clicked is not supported."));
+
+  async create(createInfo: SystemNotificationCreateInfo): Promise<string> {
+    throw new Error("Create OS Notification unsupported.");
+  }
+
+  clear(clearInfo: SystemNotificationClearInfo): Promise<undefined> {
+    throw new Error("Clear OS Notification unsupported.");
+  }
+
+  isSupported(): boolean {
+    return false;
+  }
+}