Use side effects for wallet_snap permission

packages/examples/examples/insights/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps-monorepo.git"
   },
   "source": {
-    "shasum": "Ndblid1yVfWgEbdlNKR6snzgiTCNBLF1Zxd7EDpDORk=",
+    "shasum": "+rfL2d5iP9dQMMKAsoGzQx/MpsLoH1px8GXFwg+xHvs=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/examples/examples/signer/packages/core/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps-monorepo.git"
   },
   "source": {
-    "shasum": "wSZHQDJImPu8oR0+tQBzszPWlr/XIDcI+v7Kf/Pq/vA=",
+    "shasum": "HBXPKk8+lODtF82apQ4pbnenuhGwtbgo/ItWgSxKvgk=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/rpc-methods/jest.config.js

@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
   ],
   coverageThreshold: {
     global: {
-      branches: 84.95,
+      branches: 85.45,
       functions: 94.73,
-      lines: 94.37,
-      statements: 94.4,
+      lines: 95.01,
+      statements: 95.03,
     },
   },
 });

packages/rpc-methods/package.json

@@ -30,7 +30,7 @@
   "dependencies": {
     "@metamask/browser-passworder": "^4.0.2",
     "@metamask/key-tree": "^7.0.0",
-    "@metamask/permission-controller": "^3.0.0",
+    "@metamask/permission-controller": "^3.1.0",
     "@metamask/snaps-ui": "^0.31.0",
     "@metamask/snaps-utils": "^0.31.0",
     "@metamask/types": "^1.1.0",

packages/rpc-methods/src/permitted/requestSnaps.test.ts

@@ -190,12 +190,13 @@ describe('implementation', () => {
         invoker: 'https://metamask.github.io',
         parentCapability: WALLET_SNAP_PERMISSION_KEY,
       },
+      {
+        data: {
+          [WALLET_SNAP_PERMISSION_KEY]: { [MOCK_SNAP_ID]: getTruncatedSnap() },
+        },
+      },
     ]);
 
-    hooks.installSnaps.mockImplementation(() => ({
-      [MOCK_SNAP_ID]: getTruncatedSnap(),
-    }));
-
     const engine = new JsonRpcEngine();
     engine.push((req, res, next, end) => {
       const result = implementation(
@@ -226,10 +227,6 @@ describe('implementation', () => {
       },
     });
 
-    expect(hooks.installSnaps).toHaveBeenCalledWith({
-      [MOCK_SNAP_ID]: {},
-    });
-
     expect(response.result).toStrictEqual({
       [MOCK_SNAP_ID]: getTruncatedSnap(),
     });
@@ -294,4 +291,50 @@ describe('implementation', () => {
       [MOCK_SNAP_ID]: getTruncatedSnap(),
     });
   });
+
+  it('throws with the appropriate error if the side-effect fails', async () => {
+    const { implementation } = requestSnapsHandler;
+
+    const hooks = getMockHooks();
+
+    hooks.requestPermissions.mockImplementation(async () => {
+      throw new Error('error');
+    });
+
+    const engine = new JsonRpcEngine();
+    engine.push((req, res, next, end) => {
+      const result = implementation(
+        req as JsonRpcRequest<RequestedPermissions>,
+        res as PendingJsonRpcResponse<InstallSnapsResult>,
+        next,
+        end,
+        hooks,
+      );
+
+      result?.catch(end);
+    });
+
+    const response = (await engine.handle({
+      jsonrpc: '2.0',
+      id: 1,
+      method: 'wallet_requestSnaps',
+      params: {
+        [MOCK_SNAP_ID]: {},
+      },
+    })) as JsonRpcSuccess<InstallSnapsResult>;
+
+    expect(hooks.requestPermissions).toHaveBeenCalledWith({
+      [WALLET_SNAP_PERMISSION_KEY]: {
+        caveats: [
+          { type: SnapCaveatType.SnapIds, value: { [MOCK_SNAP_ID]: {} } },
+        ],
+      },
+    });
+
+    expect(response).toStrictEqual({
+      error: { code: -32603, data: { originalError: {} }, message: 'error' },
+      id: 1,
+      jsonrpc: '2.0',
+    });
+  });
 });

packages/rpc-methods/src/permitted/requestSnaps.ts

@@ -57,7 +57,12 @@ export type RequestSnapsHooks = {
    */
   requestPermissions: (
     permissions: RequestedPermissions,
-  ) => Promise<PermissionConstraint[]>;
+  ) => Promise<
+    [
+      Record<string, PermissionConstraint>,
+      { data: Record<string, unknown>; id: string; origin: string },
+    ]
+  >;
 
   /**
    * Gets the current permissions for the requesting origin.
@@ -177,9 +182,6 @@ async function requestSnapsImplementation(
     );
   }
 
-  // Request the permission for the installing DApp to talk to the snap, if needed
-  // TODO: Should this be part of the install flow?
-
   try {
     if (!Object.keys(requestedSnaps).length) {
       throw new Error('Request must have at least one requested snap.');
@@ -192,33 +194,27 @@ async function requestSnapsImplementation(
     } as RequestedPermissions;
     const existingPermissions = await getPermissions();
 
-    let approvedPermissions = [];
-
     if (!existingPermissions) {
-      approvedPermissions = await requestPermissions(requestedPermissions);
-    } else if (!hasRequestedSnaps(existingPermissions, requestedSnaps)) {
+      const [, metadata] = await requestPermissions(requestedPermissions);
+      res.result = metadata.data[
+        WALLET_SNAP_PERMISSION_KEY
+      ] as InstallSnapsResult;
+    } else if (hasRequestedSnaps(existingPermissions, requestedSnaps)) {
+      res.result = await handleInstallSnaps(requestedSnaps, installSnaps);
+    } else {
       const mergedPermissionsRequest = getSnapPermissionsRequest(
         existingPermissions,
         requestedPermissions,
       );
-      approvedPermissions = await requestPermissions(mergedPermissionsRequest);
-    }
 
-    if (
-      (!existingPermissions ||
-        !hasRequestedSnaps(existingPermissions, requestedSnaps)) &&
-      !approvedPermissions?.length
-    ) {
-      throw ethErrors.provider.userRejectedRequest({ data: req });
+      const [, metadata] = await requestPermissions(mergedPermissionsRequest);
+      res.result = metadata.data[
+        WALLET_SNAP_PERMISSION_KEY
+      ] as InstallSnapsResult;
     }
-  } catch (error) {
-    return end(error);
-  }
-
-  try {
-    res.result = await handleInstallSnaps(requestedSnaps, installSnaps);
   } catch (error) {
     res.error = error;
   }
+
   return end();
 }

packages/rpc-methods/src/restricted/invokeSnap.test.ts

@@ -1,13 +1,15 @@
 import {
   Caveat,
   OriginString,
+  PermissionsRequest,
   PermissionType,
 } from '@metamask/permission-controller';
 import { SnapCaveatType, SnapId } from '@metamask/snaps-utils';
 import {
   MOCK_SNAP_ID,
   MOCK_ORIGIN,
   getTruncatedSnap,
+  MockControllerMessenger,
 } from '@metamask/snaps-utils/test-utils';
 import { Json } from '@metamask/utils';
 
@@ -17,6 +19,8 @@ import {
   validateCaveat,
   InvokeSnapCaveatSpecifications,
   WALLET_SNAP_PERMISSION_KEY,
+  handleSnapInstall,
+  InstallSnaps,
 } from './invokeSnap';
 
 describe('builder', () => {
@@ -224,3 +228,56 @@ describe('implementation', () => {
     expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled();
   });
 });
+
+describe('handleSnapInstall', () => {
+  it('calls SnapController:install with the right parameters', async () => {
+    const messenger = new MockControllerMessenger<InstallSnaps, never>();
+
+    const sideEffectMessenger = messenger.getRestricted({
+      name: 'PermissionController',
+      allowedActions: ['SnapController:install'],
+    });
+
+    const expectedResult = {
+      [MOCK_SNAP_ID]: getTruncatedSnap(),
+    };
+
+    messenger.registerActionHandler(
+      'SnapController:install',
+      async () => expectedResult,
+    );
+
+    jest.spyOn(sideEffectMessenger, 'call');
+
+    const requestedSnaps = {
+      [MOCK_SNAP_ID]: {},
+    };
+
+    const requestData = {
+      permissions: {
+        [WALLET_SNAP_PERMISSION_KEY]: {
+          caveats: [
+            {
+              type: SnapCaveatType.SnapIds,
+              value: requestedSnaps,
+            },
+          ],
+        },
+      },
+      metadata: { origin: MOCK_ORIGIN, id: 'foo' },
+    } as PermissionsRequest;
+
+    const result = await handleSnapInstall({
+      requestData,
+      messagingSystem: sideEffectMessenger,
+    });
+
+    expect(sideEffectMessenger.call).toHaveBeenCalledWith(
+      'SnapController:install',
+      MOCK_ORIGIN,
+      requestedSnaps,
+    );
+
+    expect(result).toBe(expectedResult);
+  });
+});

packages/rpc-methods/src/restricted/invokeSnap.ts

@@ -7,6 +7,7 @@ import {
   Caveat,
   RestrictedMethodParameters,
   PermissionValidatorConstraint,
+  PermissionSideEffect,
 } from '@metamask/permission-controller';
 import {
   Snap,
@@ -15,6 +16,8 @@ import {
   SnapRpcHookArgs,
   SnapCaveatType,
   assertIsValidSnapId,
+  RequestedSnapPermissions,
+  InstallSnapsResult,
 } from '@metamask/snaps-utils';
 import {
   isJsonRpcRequest,
@@ -30,6 +33,17 @@ import { MethodHooksObject } from '../utils';
 
 export const WALLET_SNAP_PERMISSION_KEY = 'wallet_snap';
 
+// Redeclare installSnaps action type to avoid circular dependencies
+export type InstallSnaps = {
+  type: `SnapController:install`;
+  handler: (
+    origin: string,
+    requestedSnaps: RequestedSnapPermissions,
+  ) => Promise<InstallSnapsResult>;
+};
+
+type AllowedActions = InstallSnaps;
+
 export type InvokeSnapMethodHooks = {
   getSnap: (snapId: SnapId) => Snap | undefined;
   handleSnapRpcRequest: ({
@@ -51,6 +65,9 @@ type InvokeSnapSpecification = ValidPermissionSpecification<{
   methodImplementation: ReturnType<typeof getInvokeSnapImplementation>;
   allowedCaveats: Readonly<NonEmptyArray<string>> | null;
   validator: PermissionValidatorConstraint;
+  sideEffect: {
+    onPermitted: PermissionSideEffect<AllowedActions, never>['onPermitted'];
+  };
 }>;
 
 type InvokeSnapParams = {
@@ -77,6 +94,26 @@ export function validateCaveat(caveat: Caveat<string, any>) {
   }
 }
 
+/**
+ * The side-effect method to handle the snap install.
+ *
+ * @param params - The side-effect params.
+ * @param params.requestData - The request data associated to the requested permission.
+ * @param params.messagingSystem - The messenger to call an action.
+ */
+export const handleSnapInstall: PermissionSideEffect<
+  AllowedActions,
+  never
+>['onPermitted'] = async ({ requestData, messagingSystem }) => {
+  const snaps = requestData.permissions[WALLET_SNAP_PERMISSION_KEY].caveats?.[0]
+    .value as RequestedSnapPermissions;
+
+  return messagingSystem.call(
+    `SnapController:install`,
+    requestData.metadata.origin,
+    snaps,
+  );
+};
 /**
  * The specification builder for the `wallet_snap_*` permission.
  *
@@ -106,6 +143,9 @@ const specificationBuilder: PermissionSpecificationBuilder<
         });
       }
     },
+    sideEffect: {
+      onPermitted: handleSnapInstall,
+    },
   };
 };
 

packages/snaps-controllers/coverage.json

@@ -1,6 +1,6 @@
 {
-  "branches": 89.53,
+  "branches": 89.51,
   "functions": 95.17,
   "lines": 96.64,
-  "statements": 96.55
+  "statements": 96.54
 }

packages/snaps-controllers/package.json

@@ -37,7 +37,7 @@
     "@metamask/approval-controller": "^2.0.0",
     "@metamask/base-controller": "^2.0.0",
     "@metamask/object-multiplex": "^1.1.0",
-    "@metamask/permission-controller": "^3.0.0",
+    "@metamask/permission-controller": "^3.1.0",
     "@metamask/post-message-stream": "^6.1.1",
     "@metamask/rpc-methods": "^0.31.0",
     "@metamask/snaps-execution-environments": "^0.31.0",