feat!: Ensure user has onboarded before allowing usage of SnapController (#3731)

Adds a new required constructor argument `ensureOnboardingComplete`.
This should be a promise that resolves once the user has been onboarded.
We can use this to ensure that a user has fully onboarded and decided
whether to opt-out of Snaps before using Snaps at all. The function is
used as part of `assertCanUsePlatform`.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Require onboarding completion via new `ensureOnboardingComplete` hook
and asynchronously gate Snap operations (install, start, handle
requests, registry updates) until onboarding finishes.
> 
> - **SnapController API**:
> - Add required constructor hook `ensureOnboardingComplete():
Promise<void>` and store internally.
> - Make platform check async: `#assertCanUsePlatform` now awaits
onboarding completion, then validates feature flags.
> - Await platform check in `updateRegistry`, `startSnap`,
`installSnaps`, `#updateSnap`, and `handleRequest`.
> - **Tests**:
> - Add test ensuring requests wait for onboarding completion before
execution.
> - Update helpers to pass mocked `ensureOnboardingComplete` by default.
> - Adjust call-count expectations and minor timing in affected tests;
remove a couple of unused delegated actions in test utils.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
dd19141. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: FrederikBolding

packages/snaps-controllers/src/snaps/SnapController.test.tsx

@@ -795,7 +795,7 @@ describe('SnapController', () => {
       [MOCK_SNAP_ID]: expectedSnapObject,
     });
 
-    expect(messenger.call).toHaveBeenCalledTimes(10);
+    expect(messenger.call).toHaveBeenCalledTimes(9);
 
     expect(messenger.call).toHaveBeenNthCalledWith(
       1,
@@ -1869,10 +1869,12 @@ describe('SnapController', () => {
       },
     });
 
-    const results = await Promise.allSettled([
-      snapController.removeSnap(snap.id),
-      promise,
-    ]);
+    const removeSnap = async () => {
+      await sleep(1);
+      return snapController.removeSnap(snap.id);
+    };
+
+    const results = await Promise.allSettled([removeSnap(), promise]);
 
     expect(results[0].status).toBe('fulfilled');
     expect(results[1].status).toBe('rejected');
@@ -2681,6 +2683,55 @@ describe('SnapController', () => {
       },
     );
 
+    it('ensures onboarding has completed before processing requests', async () => {
+      const rootMessenger = getControllerMessenger();
+      const messenger = getSnapControllerMessenger(rootMessenger);
+
+      const callActionSpy = jest.spyOn(messenger, 'call');
+
+      const { promise, resolve } = createDeferredPromise();
+      const ensureOnboardingComplete = jest.fn().mockReturnValue(promise);
+      const snapController = getSnapController(
+        getSnapControllerOptions({
+          messenger,
+          state: {
+            snaps: getPersistedSnapsState(),
+          },
+          ensureOnboardingComplete,
+        }),
+      );
+
+      const snap = snapController.getExpect(MOCK_SNAP_ID);
+
+      const requestPromise = snapController.handleRequest({
+        snapId: snap.id,
+        origin: METAMASK_ORIGIN,
+        handler: HandlerType.OnRpcRequest,
+        request: {
+          jsonrpc: '2.0',
+          method: 'test',
+          params: {},
+        },
+      });
+
+      await sleep(100);
+
+      expect(callActionSpy).not.toHaveBeenCalledWith(
+        'ExecutionService:executeSnap',
+        expect.objectContaining({ snapId: MOCK_SNAP_ID }),
+      );
+
+      resolve();
+      expect(await requestPromise).toBeUndefined();
+
+      expect(callActionSpy).toHaveBeenCalledWith(
+        'ExecutionService:executeSnap',
+        expect.objectContaining({ snapId: MOCK_SNAP_ID }),
+      );
+
+      snapController.destroy();
+    });
+
     it('throws if the snap does not have permission to handle JSON-RPC requests from dapps', async () => {
       const rootMessenger = getControllerMessenger();
       const messenger = getSnapControllerMessenger(rootMessenger);
@@ -5448,7 +5499,7 @@ describe('SnapController', () => {
 
       expect(result).toStrictEqual({ [MOCK_LOCAL_SNAP_ID]: truncatedSnap });
 
-      expect(messenger.call).toHaveBeenCalledTimes(12);
+      expect(messenger.call).toHaveBeenCalledTimes(11);
 
       expect(messenger.call).toHaveBeenNthCalledWith(
         1,
@@ -5598,7 +5649,7 @@ describe('SnapController', () => {
         [MOCK_LOCAL_SNAP_ID]: truncatedSnap,
       });
 
-      expect(messenger.call).toHaveBeenCalledTimes(23);
+      expect(messenger.call).toHaveBeenCalledTimes(22);
 
       expect(messenger.call).toHaveBeenNthCalledWith(
         1,
@@ -6845,7 +6896,7 @@ describe('SnapController', () => {
       expect(result).toStrictEqual({
         [MOCK_SNAP_ID]: truncatedSnap,
       });
-      expect(messenger.call).toHaveBeenCalledTimes(10);
+      expect(messenger.call).toHaveBeenCalledTimes(9);
 
       expect(messenger.call).toHaveBeenNthCalledWith(
         1,
@@ -7538,7 +7589,7 @@ describe('SnapController', () => {
         [MOCK_SNAP_ID]: { version: newVersionRange },
       });
 
-      expect(messenger.call).toHaveBeenCalledTimes(21);
+      expect(messenger.call).toHaveBeenCalledTimes(20);
 
       expect(messenger.call).toHaveBeenNthCalledWith(
         3,
@@ -8414,7 +8465,7 @@ describe('SnapController', () => {
           date: expect.any(Number),
         },
       ]);
-      expect(callActionSpy).toHaveBeenCalledTimes(21);
+      expect(callActionSpy).toHaveBeenCalledTimes(20);
 
       expect(callActionSpy).toHaveBeenNthCalledWith(
         12,
@@ -8579,7 +8630,7 @@ describe('SnapController', () => {
         [MOCK_SNAP_ID]: { version: '1.1.0' },
       });
 
-      expect(callActionSpy).toHaveBeenCalledTimes(21);
+      expect(callActionSpy).toHaveBeenCalledTimes(20);
       expect(callActionSpy).toHaveBeenNthCalledWith(
         12,
         'ApprovalController:addRequest',
@@ -8695,7 +8746,7 @@ describe('SnapController', () => {
         [MOCK_SNAP_ID]: { version: '1.1.0' },
       });
 
-      expect(callActionSpy).toHaveBeenCalledTimes(22);
+      expect(callActionSpy).toHaveBeenCalledTimes(21);
       expect(callActionSpy).toHaveBeenNthCalledWith(
         12,
         'ApprovalController:addRequest',
@@ -8836,7 +8887,7 @@ describe('SnapController', () => {
 
       const isRunning = controller.isRunning(MOCK_SNAP_ID);
 
-      expect(callActionSpy).toHaveBeenCalledTimes(12);
+      expect(callActionSpy).toHaveBeenCalledTimes(11);
 
       expect(callActionSpy).toHaveBeenNthCalledWith(
         1,
@@ -9191,7 +9242,7 @@ describe('SnapController', () => {
         [MOCK_SNAP_ID]: { version: '1.1.0' },
       });
 
-      expect(callActionSpy).toHaveBeenCalledTimes(23);
+      expect(callActionSpy).toHaveBeenCalledTimes(22);
 
       expect(callActionSpy).toHaveBeenNthCalledWith(
         12,

packages/snaps-controllers/src/snaps/SnapController.ts

@@ -815,6 +815,13 @@ type SnapControllerArgs = {
    * MetaMetrics event tracking hook.
    */
   trackEvent: TrackEventHook;
+
+  /**
+   * A hook that returns a promise that resolves when the onboarding has completed.
+   *
+   * @returns A promise that resolves when onboarding is complete.
+   */
+  ensureOnboardingComplete: () => Promise<void>;
 };
 
 type AddSnapArgs = {
@@ -932,6 +939,8 @@ export class SnapController extends BaseController<
 
   readonly #trackSnapExport: ReturnType<typeof throttleTracking>;
 
+  readonly #ensureOnboardingComplete: () => Promise<void>;
+
   constructor({
     closeAllConnections,
     messenger,
@@ -951,6 +960,7 @@ export class SnapController extends BaseController<
     getFeatureFlags = () => ({}),
     clientCryptography,
     trackEvent,
+    ensureOnboardingComplete,
   }: SnapControllerArgs) {
     super({
       messenger,
@@ -1034,6 +1044,7 @@ export class SnapController extends BaseController<
     this.#rollbackSnapshots = new Map();
     this.#snapsRuntimeData = new Map();
     this.#trackEvent = trackEvent;
+    this.#ensureOnboardingComplete = ensureOnboardingComplete;
 
     this.#pollForLastRequestStatus();
 
@@ -1467,7 +1478,7 @@ export class SnapController extends BaseController<
    * Also updates any preinstalled Snaps to the latest allowlisted version.
    */
   async updateRegistry(): Promise<void> {
-    this.#assertCanUsePlatform();
+    await this.#assertCanUsePlatform();
     await this.messenger.call('SnapsRegistry:update');
 
     const blockedSnaps = await this.messenger.call(
@@ -1645,9 +1656,12 @@ export class SnapController extends BaseController<
   }
 
   /**
-   * Asserts whether the Snaps platform is allowed to run.
+   * Waits for onboarding and then asserts whether the Snaps platform is allowed to run.
    */
-  #assertCanUsePlatform() {
+  async #assertCanUsePlatform() {
+    // Ensure the user has onboarded before allowing access to Snaps.
+    await this.#ensureOnboardingComplete();
+
     const flags = this.#getFeatureFlags();
     assert(
       flags.disableSnaps !== true,
@@ -1730,7 +1744,7 @@ export class SnapController extends BaseController<
    * @param snapId - The id of the Snap to start.
    */
   async startSnap(snapId: SnapId): Promise<void> {
-    this.#assertCanUsePlatform();
+    await this.#assertCanUsePlatform();
     const snap = this.state.snaps[snapId];
 
     if (!snap.enabled) {
@@ -2630,7 +2644,7 @@ export class SnapController extends BaseController<
     origin: string,
     requestedSnaps: RequestSnapsParams,
   ): Promise<RequestSnapsResult> {
-    this.#assertCanUsePlatform();
+    await this.#assertCanUsePlatform();
 
     const result: RequestSnapsResult = {};
 
@@ -2916,7 +2930,7 @@ export class SnapController extends BaseController<
     if (!automaticUpdate) {
       this.#assertCanInstallSnaps();
     }
-    this.#assertCanUsePlatform();
+    await this.#assertCanUsePlatform();
 
     const snap = this.getExpect(snapId);
 
@@ -3551,7 +3565,7 @@ export class SnapController extends BaseController<
     handler: handlerType,
     request: rawRequest,
   }: SnapRpcHookArgs & { snapId: SnapId }): Promise<unknown> {
-    this.#assertCanUsePlatform();
+    await this.#assertCanUsePlatform();
 
     const snap = this.get(snapId);
 

packages/snaps-controllers/src/test-utils/controller.tsx

@@ -479,7 +479,6 @@ export const getSnapControllerMessenger = (
       'ExecutionService:executeSnap',
       'ExecutionService:terminateSnap',
       'ExecutionService:handleRpcRequest',
-      'NetworkController:getNetworkClientById',
       'PermissionController:getEndowments',
       'PermissionController:hasPermission',
       'PermissionController:hasPermissions',
@@ -490,7 +489,6 @@ export const getSnapControllerMessenger = (
       'PermissionController:revokePermissionForAllSubjects',
       'PermissionController:updateCaveat',
       'PermissionController:getSubjectNames',
-      'SelectedNetworkController:getNetworkClientIdForDomain',
       'SubjectMetadataController:getSubjectMetadata',
       'SubjectMetadataController:addSubjectMetadata',
       'SnapsRegistry:get',
@@ -574,6 +572,7 @@ export const getSnapControllerOptions = (
     clientCryptography: {},
     encryptor: getSnapControllerEncryptor(),
     trackEvent: jest.fn(),
+    ensureOnboardingComplete: jest.fn().mockResolvedValue(undefined),
     ...opts,
   } as SnapControllerConstructorParams;
 
@@ -608,6 +607,7 @@ export const getSnapControllerWithEESOptions = ({
     encryptor: getSnapControllerEncryptor(),
     fetchFunction: jest.fn(),
     trackEvent: jest.fn(),
+    ensureOnboardingComplete: jest.fn().mockResolvedValue(undefined),
     ...options,
   } as SnapControllerConstructorParams & {
     rootMessenger: ReturnType<typeof getControllerMessenger>;