Making permission and approval controller methods idempotent (#15848)

app/scripts/metamask-controller.actions.test.js

@@ -1,6 +1,11 @@
 import { strict as assert } from 'assert';
 import sinon from 'sinon';
 import proxyquire from 'proxyquire';
+
+import {
+  ApprovalRequestNotFoundError,
+  PermissionsRequestNotFoundError,
+} from '@metamask/controllers';
 import { ORIGIN_METAMASK } from '../../shared/constants/app';
 
 const Ganache = require('../../test/e2e/ganache');
@@ -238,4 +243,137 @@ describe('MetaMaskController', function () {
       assert.deepEqual(transaction1, transaction2);
     });
   });
+
+  describe('#removePermissionsFor', function () {
+    it('should not propagate PermissionsRequestNotFoundError', function () {
+      const error = new PermissionsRequestNotFoundError('123');
+      metamaskController.permissionController = {
+        revokePermissions: () => {
+          throw error;
+        },
+      };
+      // Line below will not throw error, in case it throws this test case will fail.
+      metamaskController.removePermissionsFor({ subject: 'test_subject' });
+    });
+
+    it('should propagate Error other than PermissionsRequestNotFoundError', function () {
+      const error = new Error();
+      metamaskController.permissionController = {
+        revokePermissions: () => {
+          throw error;
+        },
+      };
+      assert.throws(() => {
+        metamaskController.removePermissionsFor({ subject: 'test_subject' });
+      }, error);
+    });
+  });
+
+  describe('#rejectPermissionsRequest', function () {
+    it('should not propagate PermissionsRequestNotFoundError', function () {
+      const error = new PermissionsRequestNotFoundError('123');
+      metamaskController.permissionController = {
+        rejectPermissionsRequest: () => {
+          throw error;
+        },
+      };
+      // Line below will not throw error, in case it throws this test case will fail.
+      metamaskController.rejectPermissionsRequest('DUMMY_ID');
+    });
+
+    it('should propagate Error other than PermissionsRequestNotFoundError', function () {
+      const error = new Error();
+      metamaskController.permissionController = {
+        rejectPermissionsRequest: () => {
+          throw error;
+        },
+      };
+      assert.throws(() => {
+        metamaskController.rejectPermissionsRequest('DUMMY_ID');
+      }, error);
+    });
+  });
+
+  describe('#acceptPermissionsRequest', function () {
+    it('should not propagate PermissionsRequestNotFoundError', function () {
+      const error = new PermissionsRequestNotFoundError('123');
+      metamaskController.permissionController = {
+        acceptPermissionsRequest: () => {
+          throw error;
+        },
+      };
+      // Line below will not throw error, in case it throws this test case will fail.
+      metamaskController.acceptPermissionsRequest('DUMMY_ID');
+    });
+
+    it('should propagate Error other than PermissionsRequestNotFoundError', function () {
+      const error = new Error();
+      metamaskController.permissionController = {
+        acceptPermissionsRequest: () => {
+          throw error;
+        },
+      };
+      assert.throws(() => {
+        metamaskController.acceptPermissionsRequest('DUMMY_ID');
+      }, error);
+    });
+  });
+
+  describe('#resolvePendingApproval', function () {
+    it('should not propagate ApprovalRequestNotFoundError', function () {
+      const error = new ApprovalRequestNotFoundError('123');
+      metamaskController.approvalController = {
+        accept: () => {
+          throw error;
+        },
+      };
+      // Line below will not throw error, in case it throws this test case will fail.
+      metamaskController.resolvePendingApproval('DUMMY_ID', 'DUMMY_VALUE');
+    });
+
+    it('should propagate Error other than ApprovalRequestNotFoundError', function () {
+      const error = new Error();
+      metamaskController.approvalController = {
+        accept: () => {
+          throw error;
+        },
+      };
+      assert.throws(() => {
+        metamaskController.resolvePendingApproval('DUMMY_ID', 'DUMMY_VALUE');
+      }, error);
+    });
+  });
+
+  describe('#rejectPendingApproval', function () {
+    it('should not propagate ApprovalRequestNotFoundError', function () {
+      const error = new ApprovalRequestNotFoundError('123');
+      metamaskController.approvalController = {
+        reject: () => {
+          throw error;
+        },
+      };
+      // Line below will not throw error, in case it throws this test case will fail.
+      metamaskController.rejectPendingApproval('DUMMY_ID', {
+        code: 1,
+        message: 'DUMMY_MESSAGE',
+        data: 'DUMMY_DATA',
+      });
+    });
+
+    it('should propagate Error other than ApprovalRequestNotFoundError', function () {
+      const error = new Error();
+      metamaskController.approvalController = {
+        reject: () => {
+          throw error;
+        },
+      };
+      assert.throws(() => {
+        metamaskController.rejectPendingApproval('DUMMY_ID', {
+          code: 1,
+          message: 'DUMMY_MESSAGE',
+          data: 'DUMMY_DATA',
+        });
+      }, error);
+    });
+  });
 });

app/scripts/metamask-controller.js

@@ -39,6 +39,8 @@ import {
   CollectibleDetectionController,
   PermissionController,
   SubjectMetadataController,
+  PermissionsRequestNotFoundError,
+  ApprovalRequestNotFoundError,
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   RateLimitController,
   NotificationController,
@@ -1498,7 +1500,6 @@ export default class MetamaskController extends EventEmitter {
     const {
       addressBookController,
       alertController,
-      approvalController,
       appStateController,
       collectiblesController,
       collectibleDetectionController,
@@ -1820,16 +1821,9 @@ export default class MetamaskController extends EventEmitter {
       initializeThreeBox: this.initializeThreeBox.bind(this),
 
       // permissions
-      removePermissionsFor:
+      removePermissionsFor: this.removePermissionsFor,
-        permissionController.revokePermissions.bind(permissionController),
+      approvePermissionsRequest: this.acceptPermissionsRequest,
-      approvePermissionsRequest:
+      rejectPermissionsRequest: this.rejectPermissionsRequest,
-        permissionController.acceptPermissionsRequest.bind(
-          permissionController,
-        ),
-      rejectPermissionsRequest:
-        permissionController.rejectPermissionsRequest.bind(
-          permissionController,
-        ),
       ...getPermissionBackgroundApiMethods(permissionController),
 
       ///: BEGIN:ONLY_INCLUDE_IN(flask)
@@ -1947,14 +1941,8 @@ export default class MetamaskController extends EventEmitter {
       ),
 
       // approval controller
-      resolvePendingApproval:
+      resolvePendingApproval: this.resolvePendingApproval,
-        approvalController.accept.bind(approvalController),
+      rejectPendingApproval: this.rejectPendingApproval,
-      rejectPendingApproval: async (id, error) => {
-        approvalController.reject(
-          id,
-          new EthereumRpcError(error.code, error.message, error.data),
-        );
-      },
 
       // Notifications
       updateViewedNotifications: announcementController.updateViewed.bind(
@@ -4361,4 +4349,57 @@ export default class MetamaskController extends EventEmitter {
 
     return this.keyringController.setLocked();
   }
+
+  removePermissionsFor = (subjects) => {
+    try {
+      this.permissionController.revokePermissions(subjects);
+    } catch (exp) {
+      if (!(exp instanceof PermissionsRequestNotFoundError)) {
+        throw exp;
+      }
+    }
+  };
+
+  rejectPermissionsRequest = (requestId) => {
+    try {
+      this.permissionController.rejectPermissionsRequest(requestId);
+    } catch (exp) {
+      if (!(exp instanceof PermissionsRequestNotFoundError)) {
+        throw exp;
+      }
+    }
+  };
+
+  acceptPermissionsRequest = (request) => {
+    try {
+      this.permissionController.acceptPermissionsRequest(request);
+    } catch (exp) {
+      if (!(exp instanceof PermissionsRequestNotFoundError)) {
+        throw exp;
+      }
+    }
+  };
+
+  resolvePendingApproval = (id, value) => {
+    try {
+      this.approvalController.accept(id, value);
+    } catch (exp) {
+      if (!(exp instanceof ApprovalRequestNotFoundError)) {
+        throw exp;
+      }
+    }
+  };
+
+  rejectPendingApproval = (id, error) => {
+    try {
+      this.approvalController.reject(
+        id,
+        new EthereumRpcError(error.code, error.message, error.data),
+      );
+    } catch (exp) {
+      if (!(exp instanceof ApprovalRequestNotFoundError)) {
+        throw exp;
+      }
+    }
+  };
 }