[FLASK] Fix an issue with installing and updating snaps with 0 permissions (#15796)

* Fix an issue with installing snaps with 0 permissions

* Fix some issues

* Fix lint

* Fix code fencing

* UI improvements for no permission use-case

* Fix redirect

* Delay BIP44 test slightly

* Add more delay

* Fix update UI

app/_locales/en/messages.json

@@ -1702,6 +1702,9 @@
   "initialTransactionConfirmed": {
     "message": "Your initial transaction was confirmed by the network. Click OK to go back."
   },
+  "install": {
+    "message": "Install"
+  },
   "insufficientBalance": {
     "message": "Insufficient balance."
   },

patches/@metamask+snap-controllers+0.20.0.patch

@@ -0,0 +1,62 @@
+diff --git a/node_modules/@metamask/snap-controllers/dist/snaps/SnapController.js b/node_modules/@metamask/snap-controllers/dist/snaps/SnapController.js
+index ad84417..158e8e6 100644
+--- a/node_modules/@metamask/snap-controllers/dist/snaps/SnapController.js
++++ b/node_modules/@metamask/snap-controllers/dist/snaps/SnapController.js
+@@ -30,6 +30,7 @@ const RequestQueue_1 = require("./RequestQueue");
+ const utils_3 = require("./utils");
+ const Timer_1 = require("./Timer");
+ exports.controllerName = 'SnapController';
++exports.SNAP_APPROVAL_INSTALL = 'wallet_installSnap';
+ exports.SNAP_APPROVAL_UPDATE = 'wallet_updateSnap';
+ const TRUNCATED_SNAP_PROPERTIES = new Set([
+     'initialPermissions',
+@@ -738,7 +739,7 @@ class SnapController extends controllers_1.BaseControllerV2 {
+                 id: snapId,
+                 versionRange,
+             });
+-            await this.authorize(snapId);
++            await this.authorize(origin, snapId);
+             await this._startSnap({
+                 snapId,
+                 sourceCode,
+@@ -1073,18 +1074,34 @@ class SnapController extends controllers_1.BaseControllerV2 {
+      * @param snapId - The id of the Snap.
+      * @returns The snap's approvedPermissions.
+      */
+-    async authorize(snapId) {
++     async authorize(origin, snapId) {
+         console.info(`Authorizing snap: ${snapId}`);
+         const snapsState = this.state.snaps;
+         const snap = snapsState[snapId];
+         const { initialPermissions } = snap;
+         try {
+-            if ((0, utils_1.isNonEmptyArray)(Object.keys(initialPermissions))) {
+-                const processedPermissions = this.processSnapPermissions(initialPermissions);
+-                const [approvedPermissions] = await this.messagingSystem.call('PermissionController:requestPermissions', { origin: snapId }, processedPermissions);
+-                return Object.values(approvedPermissions).map((perm) => perm.parentCapability);
++            const processedPermissions = this.processSnapPermissions(initialPermissions);
++            const id = (0, nanoid_1.nanoid)();
++            const isApproved = await this.messagingSystem.call('ApprovalController:addRequest', {
++                origin,
++                id,
++                type: exports.SNAP_APPROVAL_INSTALL,
++                requestData: {
++                    // Mirror previous installation metadata
++                    metadata: { id, origin: snapId, dappOrigin: origin },
++                    permissions: processedPermissions,
++                    snapId,
++                },
++            }, true);
++            if (!isApproved) {
++                throw eth_rpc_errors_1.ethErrors.provider.userRejectedRequest();
++            }
++            if ((0, utils_1.isNonEmptyArray)(Object.keys(processedPermissions))) {
++                await this.messagingSystem.call('PermissionController:grantPermissions', {
++                    approvedPermissions: processedPermissions,
++                    subject: { origin: snapId },
++                });
+             }
+-            return [];
+         }
+         finally {
+             const runtime = this.getRuntimeExpect(snapId);

test/e2e/snaps/test-snap-bip-44.spec.js

@@ -79,6 +79,7 @@ describe('Test Snap bip-44', function () {
         await driver.waitUntilXWindowHandles(1, 5000, 10000);
         windowHandles = await driver.getAllWindowHandles();
         await driver.switchToWindowWithTitle('Test Snaps', windowHandles);
+        await driver.delay(1000);
         await driver.clickElement('#sendBip44');
 
         // check the results of the public key test

test/e2e/snaps/test-snap-confirm.spec.js

@@ -64,6 +64,7 @@ describe('Test Snap Confirm', function () {
         await driver.waitUntilXWindowHandles(1, 5000, 10000);
         windowHandles = await driver.getAllWindowHandles();
         await driver.switchToWindowWithTitle('Test Snaps', windowHandles);
+        await driver.delay(1000);
         await driver.clickElement('#sendConfirmButton');
 
         // hit 'approve' on the custom confirm

test/e2e/snaps/test-snap-error.spec.js

@@ -64,6 +64,7 @@ describe('Test Snap Error', function () {
         await driver.waitUntilXWindowHandles(1, 5000, 10000);
         windowHandles = await driver.getAllWindowHandles();
         await driver.switchToWindowWithTitle('Test Snaps', windowHandles);
+        await driver.delay(1000);
         await driver.clickElement('#sendError');
 
         await driver.navigate(PAGES.HOME);

test/e2e/snaps/test-snap-managestate.spec.js

@@ -73,6 +73,7 @@ describe('Test Snap manageState', function () {
         windowHandles = await driver.getAllWindowHandles();
         await driver.switchToWindowWithTitle('Test Snaps', windowHandles);
         await driver.fill('#dataManageState', '23');
+        await driver.delay(1000);
         await driver.clickElement('#sendManageState');
 
         // check the results of the public key test

test/e2e/snaps/test-snap-notification.spec.js

@@ -72,6 +72,7 @@ describe('Test Snap Notification', function () {
         await driver.waitUntilXWindowHandles(1, 5000, 10000);
         windowHandles = await driver.getAllWindowHandles();
         await driver.switchToWindowWithTitle('Test Snaps', windowHandles);
+        await driver.delay(1000);
         await driver.clickElement('#sendInAppNotification');
 
         // try to go to the MM pages

ui/pages/home/home.container.js

@@ -5,7 +5,7 @@ import {
   activeTabHasPermissions,
   getFirstPermissionRequest,
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
-  getFirstSnapUpdateRequest,
+  getFirstSnapInstallOrUpdateRequest,
   ///: END:ONLY_INCLUDE_IN
   getIsMainnet,
   getOriginOfCurrentTab,
@@ -98,7 +98,7 @@ const mapStateToProps = (state) => {
 
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   if (!firstPermissionsRequest) {
-    firstPermissionsRequest = getFirstSnapUpdateRequest(state);
+    firstPermissionsRequest = getFirstSnapInstallOrUpdateRequest(state);
     firstPermissionsRequestId = firstPermissionsRequest?.metadata.id || null;
   }
   ///: END:ONLY_INCLUDE_IN

ui/pages/permissions-connect/flask/snap-install/snap-install.js

@@ -35,10 +35,13 @@ export default function SnapInstall({
   );
 
   const onSubmit = useCallback(
-    () => approveSnapInstall(request),
+    () => approveSnapInstall(request.metadata.id),
     [request, approveSnapInstall],
   );
 
+  const hasPermissions =
+    request?.permissions && Object.keys(request.permissions).length > 0;
+
   const bip44LegacyEntropyPermissions =
     request.permissions &&
     Object.keys(request.permissions).filter((v) =>
@@ -88,18 +91,22 @@ export default function SnapInstall({
           snapVersion={targetSubjectMetadata.version}
           boxProps={{ alignItems: ALIGN_ITEMS.CENTER }}
         />
-        <Typography
-          boxProps={{
-            padding: [4, 4, 0, 4],
-          }}
-          variant={TYPOGRAPHY.H7}
-          as="span"
-        >
-          {t('snapRequestsPermission')}
-        </Typography>
-        <PermissionsConnectPermissionList
-          permissions={request.permissions || {}}
-        />
+        {hasPermissions && (
+          <>
+            <Typography
+              boxProps={{
+                padding: [4, 4, 0, 4],
+              }}
+              variant={TYPOGRAPHY.H7}
+              as="span"
+            >
+              {t('snapRequestsPermission')}
+            </Typography>
+            <PermissionsConnectPermissionList
+              permissions={request.permissions || {}}
+            />
+          </>
+        )}
       </Box>
       <Box
         className="footers"
@@ -116,7 +123,7 @@ export default function SnapInstall({
           onSubmit={
             shouldShowWarning ? () => setIsShowingWarning(true) : onSubmit
           }
-          submitText={t('approveAndInstall')}
+          submitText={t(hasPermissions ? 'approveAndInstall' : 'install')}
         />
       </Box>
       {isShowingWarning && (

ui/pages/permissions-connect/flask/snap-update/snap-update.js

@@ -33,7 +33,7 @@ export default function SnapUpdate({
   );
 
   const onSubmit = useCallback(
-    () => approveSnapUpdate(request),
+    () => approveSnapUpdate(request.metadata.id),
     [request, approveSnapUpdate],
   );
 
@@ -48,6 +48,15 @@ export default function SnapUpdate({
     [request.permissions],
   );
 
+  const approvedPermissions = request.approvedPermissions ?? {};
+  const revokedPermissions = request.unusedPermissions ?? {};
+  const newPermissions = request.newPermissions ?? {};
+  const hasPermissions =
+    Object.keys(approvedPermissions).length +
+      Object.keys(revokedPermissions).length +
+      Object.keys(newPermissions).length >
+    0;
+
   return (
     <Box
       className="page-container snap-update"
@@ -80,20 +89,24 @@ export default function SnapUpdate({
         >
           {t('snapUpdateExplanation', [`${request.metadata.dappOrigin}`])}
         </Typography>
-        <Typography
-          boxProps={{
-            padding: [2, 4, 0, 4],
-          }}
-          variant={TYPOGRAPHY.H7}
-          as="span"
-        >
-          {t('snapRequestsPermission')}
-        </Typography>
-        <UpdateSnapPermissionList
-          approvedPermissions={request.approvedPermissions || {}}
-          revokedPermissions={request.unusedPermissions || {}}
-          newPermissions={request.newPermissions || {}}
-        />
+        {hasPermissions && (
+          <>
+            <Typography
+              boxProps={{
+                padding: [2, 4, 0, 4],
+              }}
+              variant={TYPOGRAPHY.H7}
+              as="span"
+            >
+              {t('snapRequestsPermission')}
+            </Typography>
+            <UpdateSnapPermissionList
+              approvedPermissions={approvedPermissions}
+              revokedPermissions={revokedPermissions}
+              newPermissions={newPermissions}
+            />
+          </>
+        )}
       </Box>
       <Box
         className="footers"

ui/pages/permissions-connect/permissions-connect.component.js

@@ -1,6 +1,9 @@
 import PropTypes from 'prop-types';
 import React, { Component } from 'react';
 import { Switch, Route } from 'react-router-dom';
+///: BEGIN:ONLY_INCLUDE_IN(flask)
+import { ethErrors, serializeError } from 'eth-rpc-errors';
+///: END:ONLY_INCLUDE_IN
 import { getEnvironmentType } from '../../../app/scripts/lib/util';
 import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
 import { MILLISECOND } from '../../../shared/constants/time';
@@ -38,6 +41,8 @@ export default class PermissionConnect extends Component {
     snapInstallPath: PropTypes.string.isRequired,
     snapUpdatePath: PropTypes.string.isRequired,
     isSnap: PropTypes.bool.isRequired,
+    approvePendingApproval: PropTypes.func.isRequired,
+    rejectPendingApproval: PropTypes.func.isRequired,
     ///: END:ONLY_INCLUDE_IN
     totalPages: PropTypes.string.isRequired,
     page: PropTypes.string.isRequired,
@@ -235,6 +240,8 @@ export default class PermissionConnect extends Component {
       ///: BEGIN:ONLY_INCLUDE_IN(flask)
       snapInstallPath,
       snapUpdatePath,
+      approvePendingApproval,
+      rejectPendingApproval,
       ///: END:ONLY_INCLUDE_IN
     } = this.props;
     const {
@@ -305,13 +312,17 @@ export default class PermissionConnect extends Component {
               render={() => (
                 <SnapInstall
                   request={permissionsRequest || {}}
-                  approveSnapInstall={(...args) => {
-                    approvePermissionsRequest(...args);
+                  approveSnapInstall={(requestId) => {
+                    approvePendingApproval(requestId, true);
                     this.redirect(true);
                   }}
-                  rejectSnapInstall={(requestId) =>
-                    this.cancelPermissionsRequest(requestId)
-                  }
+                  rejectSnapInstall={(requestId) => {
+                    rejectPendingApproval(
+                      requestId,
+                      serializeError(ethErrors.provider.userRejectedRequest()),
+                    );
+                    this.redirect(false);
+                  }}
                   targetSubjectMetadata={targetSubjectMetadata}
                 />
               )}
@@ -328,13 +339,17 @@ export default class PermissionConnect extends Component {
               render={() => (
                 <SnapUpdate
                   request={permissionsRequest || {}}
-                  approveSnapUpdate={(...args) => {
-                    approvePermissionsRequest(...args);
+                  approveSnapUpdate={(requestId) => {
+                    approvePendingApproval(requestId, true);
                     this.redirect(true);
                   }}
-                  rejectSnapUpdate={(requestId) =>
-                    this.cancelPermissionsRequest(requestId)
-                  }
+                  rejectSnapUpdate={(requestId) => {
+                    rejectPendingApproval(
+                      requestId,
+                      serializeError(ethErrors.provider.userRejectedRequest()),
+                    );
+                    this.redirect(false);
+                  }}
                   targetSubjectMetadata={targetSubjectMetadata}
                 />
               )}

ui/pages/permissions-connect/permissions-connect.container.js

@@ -6,7 +6,7 @@ import {
   getPermissionsRequests,
   getSelectedAddress,
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
-  getSnapUpdateRequests,
+  getSnapInstallOrUpdateRequests,
   ///: END:ONLY_INCLUDE_IN
   getTargetSubjectMetadata,
 } from '../../selectors';
@@ -19,6 +19,10 @@ import {
   showModal,
   getCurrentWindowTab,
   getRequestAccountTabIds,
+  ///: BEGIN:ONLY_INCLUDE_IN(flask)
+  resolvePendingApproval,
+  rejectPendingApproval,
+  ///: END:ONLY_INCLUDE_IN
 } from '../../store/actions';
 import {
   CONNECT_ROUTE,
@@ -42,7 +46,7 @@ const mapStateToProps = (state, ownProps) => {
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   permissionsRequests = [
     ...permissionsRequests,
-    ...getSnapUpdateRequests(state),
+    ...getSnapInstallOrUpdateRequests(state),
   ];
   ///: END:ONLY_INCLUDE_IN
   const currentAddress = getSelectedAddress(state);
@@ -139,6 +143,12 @@ const mapDispatchToProps = (dispatch) => {
       dispatch(approvePermissionsRequest(request)),
     rejectPermissionsRequest: (requestId) =>
       dispatch(rejectPermissionsRequest(requestId)),
+    ///: BEGIN:ONLY_INCLUDE_IN(flask)
+    approvePendingApproval: (id, value) =>
+      dispatch(resolvePendingApproval(id, value)),
+    rejectPendingApproval: (id, error) =>
+      dispatch(rejectPendingApproval(id, error)),
+    ///: END:ONLY_INCLUDE_IN
     showNewAccountModal: ({ onCreateNewAccount, newAccountNumber }) => {
       return dispatch(
         showModal({

ui/selectors/permissions.js

@@ -281,14 +281,17 @@ export function getLastConnectedInfo(state) {
 }
 
 ///: BEGIN:ONLY_INCLUDE_IN(flask)
-export function getSnapUpdateRequests(state) {
+export function getSnapInstallOrUpdateRequests(state) {
   return Object.values(state.metamask.pendingApprovals)
-    .filter(({ type }) => type === 'wallet_updateSnap')
+    .filter(
+      ({ type }) =>
+        type === 'wallet_installSnap' || type === 'wallet_updateSnap',
+    )
     .map(({ requestData }) => requestData);
 }
 
-export function getFirstSnapUpdateRequest(state) {
-  const requests = getSnapUpdateRequests(state);
+export function getFirstSnapInstallOrUpdateRequest(state) {
+  const requests = getSnapInstallOrUpdateRequests(state);
   return requests && requests[0] ? requests[0] : null;
 }
 ///: END:ONLY_INCLUDE_IN