fix: ensure push notifications are able to be subscribed to when the …

…wallet is locked (#4653)

## Explanation

Push notifications currently are only able to be subscribed to when the
wallet is unlocked on initialisation. So if a user closes and re-opens a
browser, they will not attach the push subscription, and thus will
receive silent push notifications.

We now add an additional method that allows us to decouple the
authenticated methods (create new registration token and send to
server), which can only be done when the wallet is locked, from the
subscription methods (which subscribe to the push listener).
Now users who had closed their browser and re-opened will ensure that
they receive the correct notification and not a silent notification.

## References


## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/notification-services-controller`

- **ADDED**: New method and allowed action
`NotificationServicesPushController:subscribeToPushNotifications` to the
`NotificationServicesController`.
- **CHANGED**: Decoupled logic of creating new reg token and subscribing
to push notifications in the `NotificationServicesPushController`
`enablePushNotifications()` method.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Prithpal Sooriya <[email protected]>

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts

@@ -50,6 +50,11 @@ export type NotificationServicesPushControllerUpdateTriggerPushNotifications = {
   handler: (UUIDs: string[]) => Promise<void>;
 };
 
+export type NotificationServicesPushControllerSubscribeToNotifications = {
+  type: `NotificationServicesPushController:subscribeToPushNotifications`;
+  handler: () => Promise<void>;
+};
+
 export type NotificationServicesPushControllerOnNewNotification = {
   type: `NotificationServicesPushController:onNewNotifications`;
   payload: [INotification];
@@ -213,7 +218,8 @@ export type AllowedActions =
   // Push Notifications Controller Requests
   | NotificationServicesPushControllerEnablePushNotifications
   | NotificationServicesPushControllerDisablePushNotifications
-  | NotificationServicesPushControllerUpdateTriggerPushNotifications;
+  | NotificationServicesPushControllerUpdateTriggerPushNotifications
+  | NotificationServicesPushControllerSubscribeToNotifications;
 
 // Events
 export type NotificationServicesControllerStateChangeEvent =
@@ -338,6 +344,11 @@ export default class NotificationServicesController extends BaseController<
   };
 
   #pushNotifications = {
+    subscribeToPushNotifications: async () => {
+      await this.messagingSystem.call(
+        'NotificationServicesPushController:subscribeToPushNotifications',
+      );
+    },
     enablePushNotifications: async (UUIDs: string[]) => {
       if (!this.#isPushIntegrated) {
         return;
@@ -399,18 +410,21 @@ export default class NotificationServicesController extends BaseController<
       if (this.#isPushNotificationsSetup) {
         return;
       }
-      if (!this.#isUnlocked) {
-        return;
-      }
 
-      const storage = await this.#getUserStorage();
-      if (!storage) {
-        return;
-      }
+      // If wallet is unlocked, we can create a fresh push subscription
+      // Otherwise we can subscribe to original subscription
+      if (this.#isUnlocked) {
+        const storage = await this.#getUserStorage();
+        if (!storage) {
+          return;
+        }
 
-      const uuids = Utils.getAllUUIDs(storage);
-      await this.#pushNotifications.enablePushNotifications(uuids);
-      this.#isPushNotificationsSetup = true;
+        const uuids = Utils.getAllUUIDs(storage);
+        await this.#pushNotifications.enablePushNotifications(uuids);
+        this.#isPushNotificationsSetup = true;
+      } else {
+        await this.#pushNotifications.subscribeToPushNotifications();
+      }
     },
   };
 

packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts

@@ -15,38 +15,49 @@ const MOCK_FCM_TOKEN = 'mockFcmToken';
 const MOCK_TRIGGERS = ['uuid1', 'uuid2'];
 
 describe('NotificationServicesPushController', () => {
+  const arrangeServicesMocks = () => {
+    const activatePushNotificationsMock = jest
+      .spyOn(services, 'activatePushNotifications')
+      .mockResolvedValue(MOCK_FCM_TOKEN);
+
+    const deactivatePushNotificationsMock = jest
+      .spyOn(services, 'deactivatePushNotifications')
+      .mockResolvedValue(true);
+
+    const unsubscribeMock = jest.fn();
+    const listenToPushNotificationsMock = jest
+      .spyOn(services, 'listenToPushNotifications')
+      .mockResolvedValue(unsubscribeMock);
+
+    const updateTriggerPushNotificationsMock = jest
+      .spyOn(services, 'updateTriggerPushNotifications')
+      .mockResolvedValue({
+        isTriggersLinkedToPushNotifications: true,
+      });
+
+    return {
+      activatePushNotificationsMock,
+      deactivatePushNotificationsMock,
+      listenToPushNotificationsMock,
+      updateTriggerPushNotificationsMock,
+    };
+  };
+
   describe('enablePushNotifications', () => {
     afterEach(() => {
       jest.clearAllMocks();
     });
 
     it('should update the state with the fcmToken', async () => {
+      arrangeServicesMocks();
       const { controller, messenger } = arrangeMockMessenger();
       mockAuthBearerTokenCall(messenger);
-      jest
-        .spyOn(services, 'activatePushNotifications')
-        .mockResolvedValue(MOCK_FCM_TOKEN);
-
-      const unsubscribeMock = jest.fn();
-      jest
-        .spyOn(services, 'listenToPushNotifications')
-        .mockResolvedValue(unsubscribeMock);
 
       await controller.enablePushNotifications(MOCK_TRIGGERS);
       expect(controller.state.fcmToken).toBe(MOCK_FCM_TOKEN);
 
       expect(services.listenToPushNotifications).toHaveBeenCalled();
     });
-
-    it('should fail if a jwt token is not provided', async () => {
-      const { controller, messenger } = arrangeMockMessenger();
-      mockAuthBearerTokenCall(messenger).mockResolvedValue(
-        null as unknown as string,
-      );
-      await expect(controller.enablePushNotifications([])).rejects.toThrow(
-        expect.any(Error),
-      );
-    });
   });
 
   describe('disablePushNotifications', () => {
@@ -55,13 +66,15 @@ describe('NotificationServicesPushController', () => {
     });
 
     it('should update the state removing the fcmToken', async () => {
+      arrangeServicesMocks();
       const { controller, messenger } = arrangeMockMessenger();
       mockAuthBearerTokenCall(messenger);
       await controller.disablePushNotifications(MOCK_TRIGGERS);
       expect(controller.state.fcmToken).toBe('');
     });
 
     it('should fail if a jwt token is not provided', async () => {
+      arrangeServicesMocks();
       const { controller, messenger } = arrangeMockMessenger();
       mockAuthBearerTokenCall(messenger).mockResolvedValue(
         null as unknown as string,
@@ -78,6 +91,7 @@ describe('NotificationServicesPushController', () => {
     });
 
     it('should call updateTriggerPushNotifications with the correct parameters', async () => {
+      arrangeServicesMocks();
       const { controller, messenger } = arrangeMockMessenger();
       mockAuthBearerTokenCall(messenger);
       const spy = jest

packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts

@@ -2,6 +2,7 @@ import type {
   RestrictedControllerMessenger,
   ControllerGetStateAction,
   ControllerStateChangeEvent,
+  StateMetadata,
 } from '@metamask/base-controller';
 import { BaseController } from '@metamask/base-controller';
 import type { AuthenticationController } from '@metamask/profile-sync-controller';
@@ -33,7 +34,6 @@ export type NotificationServicesPushControllerEnablePushNotificationsAction = {
   type: `${typeof controllerName}:enablePushNotifications`;
   handler: NotificationServicesPushController['enablePushNotifications'];
 };
-
 export type NotificationServicesPushControllerDisablePushNotificationsAction = {
   type: `${typeof controllerName}:disablePushNotifications`;
   handler: NotificationServicesPushController['disablePushNotifications'];
@@ -43,12 +43,17 @@ export type NotificationServicesPushControllerUpdateTriggerPushNotificationsActi
     type: `${typeof controllerName}:updateTriggerPushNotifications`;
     handler: NotificationServicesPushController['updateTriggerPushNotifications'];
   };
+export type NotificationServicesPushControllerSubscribeToNotificationsAction = {
+  type: `${typeof controllerName}:subscribeToPushNotifications`;
+  handler: NotificationServicesPushController['subscribeToPushNotifications'];
+};
 
 export type Actions =
   | NotificationServicesPushControllerGetStateAction
   | NotificationServicesPushControllerEnablePushNotificationsAction
   | NotificationServicesPushControllerDisablePushNotificationsAction
-  | NotificationServicesPushControllerUpdateTriggerPushNotificationsAction;
+  | NotificationServicesPushControllerUpdateTriggerPushNotificationsAction
+  | NotificationServicesPushControllerSubscribeToNotificationsAction;
 
 export type AllowedActions =
   AuthenticationController.AuthenticationControllerGetBearerToken;
@@ -88,7 +93,7 @@ export type NotificationServicesPushControllerMessenger =
 export const defaultState: NotificationServicesPushControllerState = {
   fcmToken: '',
 };
-const metadata = {
+const metadata: StateMetadata<NotificationServicesPushControllerState> = {
   fcmToken: {
     persist: true,
     anonymous: true,
@@ -182,6 +187,10 @@ export default class NotificationServicesPushController extends BaseController<
       'NotificationServicesPushController:updateTriggerPushNotifications',
       this.updateTriggerPushNotifications.bind(this),
     );
+    this.messagingSystem.registerActionHandler(
+      'NotificationServicesPushController:subscribeToPushNotifications',
+      this.subscribeToPushNotifications.bind(this),
+    );
   }
 
   async #getAndAssertBearerToken() {
@@ -198,38 +207,14 @@ export default class NotificationServicesPushController extends BaseController<
     return bearerToken;
   }
 
-  /**
-   * Enables push notifications for the application.
-   *
-   * This method sets up the necessary infrastructure for handling push notifications by:
-   * 1. Registering the service worker to listen for messages.
-   * 2. Fetching the Firebase Cloud Messaging (FCM) token from Firebase.
-   * 3. Sending the FCM token to the server responsible for sending notifications, to register the device.
-   *
-   * @param UUIDs - An array of UUIDs to enable push notifications for.
-   */
-  async enablePushNotifications(UUIDs: string[]) {
-    if (!this.#config.isPushEnabled) {
-      return;
+  async subscribeToPushNotifications() {
+    if (this.#pushListenerUnsubscribe) {
+      this.#pushListenerUnsubscribe();
+      this.#pushListenerUnsubscribe = undefined;
     }
 
-    const bearerToken = await this.#getAndAssertBearerToken();
-
     try {
-      // Activate Push Notifications
-      const regToken = await activatePushNotifications({
-        bearerToken,
-        triggers: UUIDs,
-        env: this.#env,
-        createRegToken,
-        platform: this.#config.platform,
-      });
-
-      if (!regToken) {
-        return;
-      }
-
-      this.#pushListenerUnsubscribe ??= await listenToPushNotifications({
+      this.#pushListenerUnsubscribe = await listenToPushNotifications({
         env: this.#env,
         listenToPushReceived: async (n) => {
           this.messagingSystem.publish(
@@ -249,15 +234,55 @@ export default class NotificationServicesPushController extends BaseController<
           this.#config.onPushNotificationClicked(e, n);
         },
       });
+    } catch (e) {
+      // Do nothing, we are silently failing if push notification registration fails
+    }
+  }
 
-      // Update state
-      this.update((state) => {
-        state.fcmToken = regToken;
-      });
-    } catch (error) {
-      log.error('Failed to enable push notifications:', error);
-      throw new Error('Failed to enable push notifications');
+  /**
+   * Enables push notifications for the application.
+   *
+   * This method sets up the necessary infrastructure for handling push notifications by:
+   * 1. Registering the service worker to listen for messages.
+   * 2. Fetching the Firebase Cloud Messaging (FCM) token from Firebase.
+   * 3. Sending the FCM token to the server responsible for sending notifications, to register the device.
+   *
+   * @param UUIDs - An array of UUIDs to enable push notifications for.
+   */
+  async enablePushNotifications(UUIDs: string[]) {
+    if (!this.#config.isPushEnabled) {
+      return;
     }
+
+    // Handle creating new reg token (if available)
+    try {
+      const bearerToken = await this.#getAndAssertBearerToken().catch(
+        () => null,
+      );
+
+      // If there is a bearer token, lets try to refresh/create new reg token
+      if (bearerToken) {
+        // Activate Push Notifications
+        const regToken = await activatePushNotifications({
+          bearerToken,
+          triggers: UUIDs,
+          env: this.#env,
+          createRegToken,
+          platform: this.#config.platform,
+        }).catch(() => null);
+
+        if (regToken) {
+          this.update((state) => {
+            state.fcmToken = regToken;
+          });
+        }
+      }
+    } catch {
+      // Do nothing, we are silently failing
+    }
+
+    // New token created, (re)subscribe to push notifications
+    await this.subscribeToPushNotifications();
   }
 
   /**