From 8c8539d1f53c568605ef2770b21dfba95d04e9c6 Mon Sep 17 00:00:00 2001
From: Jyoti Puri <jyotipuri@gmail.com>
Date: Wed, 7 Sep 2022 01:17:48 +0530
Subject: [PATCH] Making addPermittedAccount and removePermittedAccount methods
 idempotent (#15709)

---
 .../controllers/permissions/background-api.js |  8 ++----
 .../permissions/background-api.test.js        | 26 ++++++-------------
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/app/scripts/controllers/permissions/background-api.js b/app/scripts/controllers/permissions/background-api.js
index 0ed2354d1..1d1b19c67 100644
--- a/app/scripts/controllers/permissions/background-api.js
+++ b/app/scripts/controllers/permissions/background-api.js
@@ -14,9 +14,7 @@ export function getPermissionBackgroundApiMethods(permissionController) {
       );
 
       if (existing.value.includes(account)) {
-        throw new Error(
-          `eth_accounts permission for origin "${origin}" already permits account "${account}".`,
-        );
+        return;
       }
 
       permissionController.updateCaveat(
@@ -35,9 +33,7 @@ export function getPermissionBackgroundApiMethods(permissionController) {
       );
 
       if (!existing.value.includes(account)) {
-        throw new Error(
-          `eth_accounts permission for origin "${origin}" already does not permit account "${account}".`,
-        );
+        return;
       }
 
       const remainingAccounts = existing.value.filter(
diff --git a/app/scripts/controllers/permissions/background-api.test.js b/app/scripts/controllers/permissions/background-api.test.js
index 8857835b4..a7f433eb1 100644
--- a/app/scripts/controllers/permissions/background-api.test.js
+++ b/app/scripts/controllers/permissions/background-api.test.js
@@ -34,7 +34,7 @@ describe('permission background API methods', () => {
       );
     });
 
-    it('throws if the specified account is already permitted', () => {
+    it('does not add a permitted account', () => {
       const permissionController = {
         getCaveat: jest.fn().mockImplementationOnce(() => {
           return { type: CaveatTypes.restrictReturnedAccounts, value: ['0x1'] };
@@ -42,14 +42,9 @@ describe('permission background API methods', () => {
         updateCaveat: jest.fn(),
       };
 
-      expect(() =>
-        getPermissionBackgroundApiMethods(
-          permissionController,
-        ).addPermittedAccount('foo.com', '0x1'),
-      ).toThrow(
-        `eth_accounts permission for origin "foo.com" already permits account "0x1".`,
-      );
-
+      getPermissionBackgroundApiMethods(
+        permissionController,
+      ).addPermittedAccount('foo.com', '0x1');
       expect(permissionController.getCaveat).toHaveBeenCalledTimes(1);
       expect(permissionController.getCaveat).toHaveBeenCalledWith(
         'foo.com',
@@ -128,7 +123,7 @@ describe('permission background API methods', () => {
       expect(permissionController.updateCaveat).not.toHaveBeenCalled();
     });
 
-    it('throws if the specified account is not permitted', () => {
+    it('does not call permissionController.updateCaveat if the specified account is not permitted', () => {
       const permissionController = {
         getCaveat: jest.fn().mockImplementationOnce(() => {
           return { type: CaveatTypes.restrictReturnedAccounts, value: ['0x1'] };
@@ -137,14 +132,9 @@ describe('permission background API methods', () => {
         updateCaveat: jest.fn(),
       };
 
-      expect(() =>
-        getPermissionBackgroundApiMethods(
-          permissionController,
-        ).removePermittedAccount('foo.com', '0x2'),
-      ).toThrow(
-        `eth_accounts permission for origin "foo.com" already does not permit account "0x2".`,
-      );
-
+      getPermissionBackgroundApiMethods(
+        permissionController,
+      ).removePermittedAccount('foo.com', '0x2');
       expect(permissionController.getCaveat).toHaveBeenCalledTimes(1);
       expect(permissionController.getCaveat).toHaveBeenCalledWith(
         'foo.com',