Jl/caip25 permission migration/fix wallet request permissions delay g…

…rant (#29613)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Restores wallet_requestPermissions atomicity by delaying CAIP-25 grant
until after other grants if they are requested

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.

app/scripts/lib/rpc-method-middleware/handlers/request-accounts.test.ts

@@ -39,7 +39,8 @@ const createMockedHandler = () => {
       '0x3': {},
     },
   };
-  const requestCaip25PermissionForOrigin = jest.fn().mockResolvedValue({});
+  const requestCaip25ApprovalForOrigin = jest.fn().mockResolvedValue({});
+  const grantPermissionsForOrigin = jest.fn().mockReturnValue({});
   const response: PendingJsonRpcResponse<string[]> = {
     jsonrpc: '2.0' as const,
     id: 0,
@@ -53,7 +54,8 @@ const createMockedHandler = () => {
       getUnlockPromise,
       sendMetrics,
       metamaskState,
-      requestCaip25PermissionForOrigin,
+      requestCaip25ApprovalForOrigin,
+      grantPermissionsForOrigin,
     });
 
   return {
@@ -64,7 +66,8 @@ const createMockedHandler = () => {
     getUnlockPromise,
     sendMetrics,
     metamaskState,
-    requestCaip25PermissionForOrigin,
+    requestCaip25ApprovalForOrigin,
+    grantPermissionsForOrigin,
     handler,
   };
 };
@@ -127,25 +130,37 @@ describe('requestEthereumAccountsHandler', () => {
   });
 
   describe('eip155 account permissions do not exist', () => {
-    it('requests the CAIP-25 permission', async () => {
-      const { handler, requestCaip25PermissionForOrigin } =
-        createMockedHandler();
+    it('requests the CAIP-25 approval', async () => {
+      const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler();
 
       await handler({ ...baseRequest, origin: 'http://test.com' });
-      expect(requestCaip25PermissionForOrigin).toHaveBeenCalledWith();
+      expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith();
     });
 
-    it('throws an error if the CAIP-25 permission approval is rejected', async () => {
-      const { handler, requestCaip25PermissionForOrigin, end } =
+    it('throws an error if the CAIP-25 approval is rejected', async () => {
+      const { handler, requestCaip25ApprovalForOrigin, end } =
         createMockedHandler();
-      requestCaip25PermissionForOrigin.mockRejectedValue(
+      requestCaip25ApprovalForOrigin.mockRejectedValue(
         new Error('approval rejected'),
       );
 
       await handler(baseRequest);
       expect(end).toHaveBeenCalledWith(new Error('approval rejected'));
     });
 
+    it('grants the CAIP-25 approval', async () => {
+      const {
+        handler,
+        requestCaip25ApprovalForOrigin,
+        grantPermissionsForOrigin,
+      } = createMockedHandler();
+
+      requestCaip25ApprovalForOrigin.mockResolvedValue({ foo: 'bar' });
+
+      await handler({ ...baseRequest, origin: 'http://test.com' });
+      expect(grantPermissionsForOrigin).toHaveBeenCalledWith({ foo: 'bar' });
+    });
+
     it('returns the newly granted and properly ordered eth accounts', async () => {
       const { handler, getAccounts, response } = createMockedHandler();
       getAccounts

app/scripts/lib/rpc-method-middleware/handlers/request-accounts.ts

@@ -35,7 +35,8 @@ const requestEthereumAccounts = {
     getUnlockPromise: true,
     sendMetrics: true,
     metamaskState: true,
-    requestCaip25PermissionForOrigin: true,
+    requestCaip25ApprovalForOrigin: true,
+    grantPermissionsForOrigin: true,
   },
 };
 export default requestEthereumAccounts;
@@ -59,7 +60,8 @@ const locks = new Set();
  * @param options.getUnlockPromise - A hook that resolves when the wallet is unlocked.
  * @param options.sendMetrics - A hook that helps track metric events.
  * @param options.metamaskState - The MetaMask app state.
- * @param options.requestCaip25PermissionForOrigin - A hook that requests the CAIP-25 permission for the origin.
+ * @param options.requestCaip25ApprovalForOrigin - A hook that requests approval for the CAIP-25 permission for the origin.
+ * @param options.grantPermissionsForOrigin - A hook that grants permission for the approved permissions for the origin.
  * @param options.metamaskState.metaMetricsId - The MetaMetrics ID.
  * @param options.metamaskState.permissionHistory - The permission history keyed by origin.
  * @param options.metamaskState.accounts - The accounts available in the wallet keyed by address.
@@ -75,7 +77,8 @@ async function requestEthereumAccountsHandler(
     getUnlockPromise,
     sendMetrics,
     metamaskState,
-    requestCaip25PermissionForOrigin,
+    requestCaip25ApprovalForOrigin,
+    grantPermissionsForOrigin,
   }: {
     getAccounts: (ignoreLock?: boolean) => string[];
     getUnlockPromise: (shouldShowUnlockRequest: true) => Promise<void>;
@@ -88,14 +91,15 @@ async function requestEthereumAccountsHandler(
       permissionHistory: Record<string, unknown>;
       accounts: Record<string, unknown>;
     };
-    requestCaip25PermissionForOrigin: (
+    requestCaip25ApprovalForOrigin: (
       requestedPermissions?: RequestedPermissions,
-    ) => Promise<
-      ValidPermission<
+    ) => Promise<RequestedPermissions>;
+    grantPermissionsForOrigin: (approvedPermissions: RequestedPermissions) => {
+      [Caip25EndowmentPermissionName]: ValidPermission<
         typeof Caip25EndowmentPermissionName,
         Caveat<typeof Caip25CaveatType, Caip25CaveatValue>
-      >
-    >;
+      >;
+    };
   },
 ) {
   const { origin } = req;
@@ -125,7 +129,8 @@ async function requestEthereumAccountsHandler(
   }
 
   try {
-    await requestCaip25PermissionForOrigin();
+    const caip25Approval = await requestCaip25ApprovalForOrigin();
+    await grantPermissionsForOrigin(caip25Approval);
   } catch (error) {
     return end(error as unknown as Error);
   }