refactor: Support permitted chains on permission confirmation page

The permissions confirmation page currently ignores the `requestedChainIds` prop when the connection is confirmed. This hasn't resulted in a bug because this page is only used for snap permissions. Permission requests for `eth_account` or `endowment:permitted-chains` are handled by the "ChooseAccount" or "ConnectPage" components (the former if a snap is also requested alongside, the latter otherwise). This PR fixes the problem regardless, as it's confusing for the component to have this prop but to ignore it when processing the confirmation. This was extracted from #27782
MetaMask · Feb 19, 2025 · b02db3c · b02db3c
1 parent 0832bdd
commit b02db3c
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 44 deletions.
diff --git a/ui/components/app/permission-page-container/permission-page-container.component.js b/ui/components/app/permission-page-container/permission-page-container.component.js
@@ -4,10 +4,7 @@ import {
   SnapCaveatType,
   WALLET_SNAP_PERMISSION_KEY,
 } from '@metamask/snaps-rpc-methods';
-import {
-  Caip25EndowmentPermissionName,
-  getPermittedEthChainIds,
-} from '@metamask/multichain';
+import { Caip25EndowmentPermissionName } from '@metamask/multichain';
 import { SubjectType } from '@metamask/permission-controller';
 import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
 import { PageContainerFooter } from '../../ui/page-container';
@@ -24,7 +21,7 @@ import {
 } from '../../../helpers/constants/design-system';
 import { Box } from '../../component-library';
 import {
-  getRequestedSessionScopes,
+  getRequestedCaip25CaveatValue,
   getCaip25PermissionsResponse,
 } from '../../../pages/permissions-connect/connect-page/utils';
 import { containsEthPermissionsAndNonEvmAccount } from '../../../helpers/utils/permissions';
@@ -145,22 +142,26 @@ export default class PermissionPageContainer extends Component {
       approvePermissionsRequest,
       rejectPermissionsRequest,
       selectedAccounts,
+      requestedChainIds,
     } = this.props;
 
     const approvedAccounts = selectedAccounts.map(
       (selectedAccount) => selectedAccount.address,
     );
 
-    const requestedSessionsScopes = getRequestedSessionScopes(
+    const requestedCaip25CaveatValue = getRequestedCaip25CaveatValue(
       _request.permission,
     );
-    const approvedChainIds = getPermittedEthChainIds(requestedSessionsScopes);
 
     const request = {
       ..._request,
       permissions: {
         ..._request.permissions,
-        ...getCaip25PermissionsResponse(approvedAccounts, approvedChainIds),
+        ...getCaip25PermissionsResponse(
+          requestedCaip25CaveatValue,
+          approvedAccounts,
+          requestedChainIds,
+        ),
       },
     };
 

diff --git a/ui/pages/permissions-connect/connect-page/connect-page.tsx b/ui/pages/permissions-connect/connect-page/connect-page.tsx
@@ -37,9 +37,9 @@ import { TEST_CHAINS } from '../../../../shared/constants/network';
 import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer';
 import { getMultichainNetwork } from '../../../selectors/multichain';
 import {
-  getRequestedSessionScopes,
   getCaip25PermissionsResponse,
   PermissionsRequest,
+  getRequestedCaip25CaveatValue,
 } from './utils';
 
 export type ConnectPageRequest = {
@@ -64,11 +64,11 @@ export const ConnectPage: React.FC<ConnectPageProps> = ({
 }) => {
   const t = useI18nContext();
 
-  const requestedSessionsScopes = getRequestedSessionScopes(
+  const requestedCaip25CaveatValue = getRequestedCaip25CaveatValue(
     request.permissions,
   );
-  const requestedAccounts = getEthAccounts(requestedSessionsScopes);
-  const requestedChainIds = getPermittedEthChainIds(requestedSessionsScopes);
+  const requestedAccounts = getEthAccounts(requestedCaip25CaveatValue);
+  const requestedChainIds = getPermittedEthChainIds(requestedCaip25CaveatValue);
 
   const networkConfigurations = useSelector(getNetworkConfigurationsByChainId);
   const [nonTestNetworks, testNetworks] = useMemo(
@@ -144,6 +144,7 @@ export const ConnectPage: React.FC<ConnectPageProps> = ({
       permissions: {
         ...request.permissions,
         ...getCaip25PermissionsResponse(
+          requestedCaip25CaveatValue,
           selectedAccountAddresses as Hex[],
           selectedChainIds,
         ),

diff --git a/ui/pages/permissions-connect/connect-page/utils.test.ts b/ui/pages/permissions-connect/connect-page/utils.test.ts
@@ -6,10 +6,24 @@ import { Hex } from '@metamask/utils';
 import { CHAIN_IDS } from '../../../../shared/constants/network';
 import { getCaip25PermissionsResponse } from './utils';
 
+const baseCaip25CaveatValue = {
+  requiredScopes: {},
+  optionalScopes: {
+    'wallet:eip155': {
+      accounts: [],
+    },
+  },
+  isMultichainOrigin: false,
+};
+
 describe('getCaip25PermissionsResponse', () => {
   describe('No accountAddresses or chainIds requested', () => {
     it(`should construct a valid ${Caip25EndowmentPermissionName} empty permission`, () => {
-      const result = getCaip25PermissionsResponse([], []);
+      const result = getCaip25PermissionsResponse(
+        baseCaip25CaveatValue,
+        [],
+        [],
+      );
 
       expect(result).toEqual({
         [Caip25EndowmentPermissionName]: {
@@ -35,7 +49,11 @@ describe('getCaip25PermissionsResponse', () => {
   describe('Request approval for chainIds', () => {
     it(`should construct a valid ${Caip25EndowmentPermissionName} permission from the passed chainIds`, () => {
       const hexChainIds: Hex[] = [CHAIN_IDS.ARBITRUM];
-      const result = getCaip25PermissionsResponse([], hexChainIds);
+      const result = getCaip25PermissionsResponse(
+        baseCaip25CaveatValue,
+        [],
+        hexChainIds,
+      );
 
       expect(result).toEqual({
         [Caip25EndowmentPermissionName]: {
@@ -60,11 +78,16 @@ describe('getCaip25PermissionsResponse', () => {
       });
     });
   });
+
   describe('Request approval for accountAddresses', () => {
     it(`should construct a valid ${Caip25EndowmentPermissionName} permission from the passed accountAddresses`, () => {
       const addresses: Hex[] = ['0x4c286da233db3d63d44dc2ec8adc8b6dfb595cb4'];
 
-      const result = getCaip25PermissionsResponse(addresses, []);
+      const result = getCaip25PermissionsResponse(
+        baseCaip25CaveatValue,
+        addresses,
+        [],
+      );
 
       expect(result).toEqual({
         [Caip25EndowmentPermissionName]: {
@@ -88,12 +111,17 @@ describe('getCaip25PermissionsResponse', () => {
       });
     });
   });
+
   describe('Request approval for accountAddresses and chainIds', () => {
     it(`should construct a valid ${Caip25EndowmentPermissionName} permission from the passed accountAddresses and chainIds`, () => {
       const addresses: Hex[] = ['0x4c286da233db3d63d44dc2ec8adc8b6dfb595cb4'];
       const hexChainIds: Hex[] = [CHAIN_IDS.ARBITRUM, CHAIN_IDS.LINEA_MAINNET];
 
-      const result = getCaip25PermissionsResponse(addresses, hexChainIds);
+      const result = getCaip25PermissionsResponse(
+        baseCaip25CaveatValue,
+        addresses,
+        hexChainIds,
+      );
 
       expect(result).toEqual({
         [Caip25EndowmentPermissionName]: {
@@ -127,4 +155,54 @@ describe('getCaip25PermissionsResponse', () => {
       });
     });
   });
+
+  describe('Request approval including non-evm scopes', () => {
+    it('only modifies evm related scopes', () => {
+      const addresses: Hex[] = ['0x4c286da233db3d63d44dc2ec8adc8b6dfb595cb4'];
+      const hexChainIds: Hex[] = ['0x1'];
+
+      const result = getCaip25PermissionsResponse(
+        {
+          ...baseCaip25CaveatValue,
+          requiredScopes: {
+            'bip122:000000000019d6689c085ae165831e93': {
+              accounts: [],
+            },
+          },
+        },
+        addresses,
+        hexChainIds,
+      );
+
+      expect(result).toEqual({
+        [Caip25EndowmentPermissionName]: {
+          caveats: [
+            {
+              type: Caip25CaveatType,
+              value: {
+                requiredScopes: {
+                  'bip122:000000000019d6689c085ae165831e93': {
+                    accounts: [],
+                  },
+                },
+                optionalScopes: {
+                  'wallet:eip155': {
+                    accounts: [
+                      'wallet:eip155:0x4c286da233db3d63d44dc2ec8adc8b6dfb595cb4',
+                    ],
+                  },
+                  'eip155:1': {
+                    accounts: [
+                      'eip155:1:0x4c286da233db3d63d44dc2ec8adc8b6dfb595cb4',
+                    ],
+                  },
+                },
+                isMultichainOrigin: false,
+              },
+            },
+          ],
+        },
+      });
+    });
+  });
 });
diff --git a/ui/pages/permissions-connect/connect-page/utils.ts b/ui/pages/permissions-connect/connect-page/utils.ts
@@ -13,57 +13,49 @@ export type PermissionsRequest = Record<
 >;
 
 /**
- * Takes in an incoming {@link PermissionsRequest} and attempts to return the {@link Caip25CaveatValue} with the Ethereum accounts set.
+ * Takes in an incoming {@link PermissionsRequest} and attempts to return the {@link Caip25CaveatValue}.
  *
  * @param permissions - The {@link PermissionsRequest} with the target name of the {@link Caip25EndowmentPermissionName}.
- * @returns The {@link Caip25CaveatValue} with the Ethereum accounts set.
+ * @returns The {@link Caip25CaveatValue}.
  */
-export function getRequestedSessionScopes(
+export function getRequestedCaip25CaveatValue(
   permissions?: PermissionsRequest,
-): Pick<Caip25CaveatValue, 'requiredScopes' | 'optionalScopes'> {
+): Caip25CaveatValue {
   return (
     permissions?.[Caip25EndowmentPermissionName]?.caveats?.find(
       (caveat) => caveat.type === Caip25CaveatType,
     )?.value ?? {
       optionalScopes: {},
       requiredScopes: {},
+      isMultichainOrigin: false,
     }
   );
 }
 
 /**
- * Parses the CAIP-25 authorized permissions object after UI confirmation.
+ * Modifies the requested CAIP-25 permissions object after UI confirmation.
  *
- * @param addresses - The list of permitted addresses.
- * @param hexChainIds - The list of permitted chains.
- * @returns The granted permissions with the target name of the {@link Caip25EndowmentPermissionName}.
+ * @param caip25CaveatValue - The requested CAIP-25 caveat value to modify.
+ * @param ethAccountAddresses - The list of permitted eth addresses.
+ * @param ethChainIds - The list of permitted eth chainIds.
  */
 export function getCaip25PermissionsResponse(
-  addresses: Hex[],
-  hexChainIds: Hex[],
+  caip25CaveatValue: Caip25CaveatValue,
+  ethAccountAddresses: Hex[],
+  ethChainIds: Hex[],
 ): {
   [Caip25EndowmentPermissionName]: {
     caveats: [{ type: string; value: Caip25CaveatValue }];
   };
 } {
-  const caveatValue: Caip25CaveatValue = {
-    requiredScopes: {},
-    optionalScopes: {
-      'wallet:eip155': {
-        accounts: [],
-      },
-    },
-    isMultichainOrigin: false,
-  };
-
   const caveatValueWithChains = setPermittedEthChainIds(
-    caveatValue,
-    hexChainIds,
+    caip25CaveatValue,
+    ethChainIds,
   );
 
   const caveatValueWithAccounts = setEthAccounts(
     caveatValueWithChains,
-    addresses,
+    ethAccountAddresses,
   );
 
   return {

diff --git a/ui/pages/permissions-connect/permissions-connect.component.js b/ui/pages/permissions-connect/permissions-connect.component.js
@@ -21,13 +21,13 @@ import SnapInstall from './snaps/snap-install';
 import SnapUpdate from './snaps/snap-update';
 import SnapResult from './snaps/snap-result';
 import { ConnectPage } from './connect-page/connect-page';
-import { getRequestedSessionScopes } from './connect-page/utils';
+import { getRequestedCaip25CaveatValue } from './connect-page/utils';
 
 const APPROVE_TIMEOUT = MILLISECOND * 1200;
 
 function getDefaultSelectedAccounts(currentAddress, permissions) {
-  const requestedSessionsScopes = getRequestedSessionScopes(permissions);
-  const requestedAccounts = getEthAccounts(requestedSessionsScopes);
+  const requestedCaip25CaveatValue = getRequestedCaip25CaveatValue(permissions);
+  const requestedAccounts = getEthAccounts(requestedCaip25CaveatValue);
 
   if (requestedAccounts.length > 0) {
     return new Set(
@@ -43,8 +43,8 @@ function getDefaultSelectedAccounts(currentAddress, permissions) {
 }
 
 function getRequestedChainIds(permissions) {
-  const requestedSessionsScopes = getRequestedSessionScopes(permissions);
-  return getPermittedEthChainIds(requestedSessionsScopes);
+  const requestedCaip25CaveatValue = getRequestedCaip25CaveatValue(permissions);
+  return getPermittedEthChainIds(requestedCaip25CaveatValue);
 }
 
 export default class PermissionConnect extends Component {