refactor: Support permitted chains on permission confirmation page (#30443)

## **Description**

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

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30443?quickstart=1)

## **Related issues**

See this comment for some additional context:
https://github.com/MetaMask/metamask-extension/pull/27782/files#r1936463248

## **Manual testing steps**

N/A, the impacted functionality is unreachable.

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

Author: Gudahtt

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(
-      _request.permission,
+    const requestedCaip25CaveatValue = getRequestedCaip25CaveatValue(
+      _request.permissions,
     );
-    const approvedChainIds = getPermittedEthChainIds(requestedSessionsScopes);
 
     const request = {
       ..._request,
       permissions: {
         ..._request.permissions,
-        ...getCaip25PermissionsResponse(approvedAccounts, approvedChainIds),
+        ...getCaip25PermissionsResponse(
+          requestedCaip25CaveatValue,
+          approvedAccounts,
+          requestedChainIds,
+        ),
       },
     };
 

ui/pages/permissions-connect/connect-page/connect-page.tsx

@@ -63,8 +63,8 @@ import {
 import { MetaMetricsContext } from '../../../contexts/metametrics';
 import {
   getCaip25PermissionsResponse,
-  getRequestedSessionScopes,
   PermissionsRequest,
+  getRequestedCaip25CaveatValue,
 } from './utils';
 
 export type ConnectPageRequest = {
@@ -98,11 +98,11 @@ export const ConnectPage: React.FC<ConnectPageProps> = ({
   const t = useI18nContext();
   const trackEvent = useContext(MetaMetricsContext);
 
-  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(
@@ -180,6 +180,7 @@ export const ConnectPage: React.FC<ConnectPageProps> = ({
       permissions: {
         ...request.permissions,
         ...getCaip25PermissionsResponse(
+          requestedCaip25CaveatValue,
           selectedAccountAddresses as Hex[],
           selectedChainIds,
         ),

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,
+              },
+            },
+          ],
+        },
+      });
+    });
+  });
 });

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 {

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

@@ -20,13 +20,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(
@@ -42,8 +42,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 {