From 20accfac880a50716db57d33fefad80c3101da6f Mon Sep 17 00:00:00 2001
From: jiexi <jiexiluan@gmail.com>
Date: Mon, 30 Sep 2024 09:47:58 -0700
Subject: [PATCH] fix: Fix snaps permission connection for `CHAIN_PERMISSIONS`
 feature flag (#27459)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!--
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**

Recent permission flow changes introduced behind the `CHAIN_PERMISSIONS`
feature flag have broken permission connection for Snaps. This PR fixes
it by removing an incorrect forced route direct in the permission
connection component.

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

## **Related issues**

Fixes: https://github.com/MetaMask/metamask-extension/pull/26635

## **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/develop/.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/develop/.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.
---
 .../permission-page-container.component.js    |  6 +-
 .../permissions-connect.component.js          | 91 +++++++++----------
 2 files changed, 48 insertions(+), 49 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
index da7719f6d4dc..f5f69da8f947 100644
--- a/ui/components/app/permission-page-container/permission-page-container.component.js
+++ b/ui/components/app/permission-page-container/permission-page-container.component.js
@@ -147,7 +147,7 @@ export default class PermissionPageContainer extends Component {
     );
 
     const permittedChainsPermission =
-      _request.permissions[PermissionNames.permittedChains];
+      _request.permissions?.[PermissionNames.permittedChains];
     const approvedChainIds = permittedChainsPermission?.caveats.find(
       (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching,
     )?.value;
@@ -155,8 +155,8 @@ export default class PermissionPageContainer extends Component {
     const request = {
       ..._request,
       permissions: { ..._request.permissions },
-      ...(_request.permissions.eth_accounts && { approvedAccounts }),
-      ...(_request.permissions[PermissionNames.permittedChains] && {
+      ...(_request.permissions?.eth_accounts && { approvedAccounts }),
+      ...(_request.permissions?.[PermissionNames.permittedChains] && {
         approvedChainIds,
       }),
     };
diff --git a/ui/pages/permissions-connect/permissions-connect.component.js b/ui/pages/permissions-connect/permissions-connect.component.js
index 403c431330b1..09befa7218cd 100644
--- a/ui/pages/permissions-connect/permissions-connect.component.js
+++ b/ui/pages/permissions-connect/permissions-connect.component.js
@@ -148,9 +148,6 @@ export default class PermissionConnect extends Component {
       history.replace(DEFAULT_ROUTE);
       return;
     }
-    if (process.env.CHAIN_PERMISSIONS) {
-      history.replace(confirmPermissionPath);
-    }
     // if this is an incremental permission request for permitted chains, skip the account selection
     if (
       permissionsRequest?.diff?.permissionDiffMap?.[
@@ -341,33 +338,8 @@ export default class PermissionConnect extends Component {
             <Route
               path={connectPath}
               exact
-              render={() => (
-                <ChooseAccount
-                  accounts={accounts}
-                  nativeCurrency={nativeCurrency}
-                  selectAccounts={(addresses) => this.selectAccounts(addresses)}
-                  selectNewAccountViaModal={(handleAccountClick) => {
-                    showNewAccountModal({
-                      onCreateNewAccount: (address) =>
-                        handleAccountClick(address),
-                      newAccountNumber,
-                    });
-                  }}
-                  addressLastConnectedMap={addressLastConnectedMap}
-                  cancelPermissionsRequest={(requestId) =>
-                    this.cancelPermissionsRequest(requestId)
-                  }
-                  permissionsRequestId={permissionsRequestId}
-                  selectedAccountAddresses={selectedAccountAddresses}
-                  targetSubjectMetadata={targetSubjectMetadata}
-                />
-              )}
-            />
-            <Route
-              path={confirmPermissionPath}
-              exact
               render={() =>
-                process.env.CHAIN_PERMISSIONS && !permissionsRequest?.diff ? (
+                process.env.CHAIN_PERMISSIONS ? (
                   <ConnectPage
                     rejectPermissionsRequest={(requestId) =>
                       this.cancelPermissionsRequest(requestId)
@@ -378,31 +350,58 @@ export default class PermissionConnect extends Component {
                     approveConnection={this.approveConnection}
                   />
                 ) : (
-                  <PermissionPageContainer
-                    request={permissionsRequest || {}}
-                    approvePermissionsRequest={(...args) => {
-                      approvePermissionsRequest(...args);
-                      this.redirect(true);
+                  <ChooseAccount
+                    accounts={accounts}
+                    nativeCurrency={nativeCurrency}
+                    selectAccounts={(addresses) =>
+                      this.selectAccounts(addresses)
+                    }
+                    selectNewAccountViaModal={(handleAccountClick) => {
+                      showNewAccountModal({
+                        onCreateNewAccount: (address) =>
+                          handleAccountClick(address),
+                        newAccountNumber,
+                      });
                     }}
-                    rejectPermissionsRequest={(requestId) =>
+                    addressLastConnectedMap={addressLastConnectedMap}
+                    cancelPermissionsRequest={(requestId) =>
                       this.cancelPermissionsRequest(requestId)
                     }
-                    selectedAccounts={accounts.filter((account) =>
-                      selectedAccountAddresses.has(account.address),
-                    )}
+                    permissionsRequestId={permissionsRequestId}
+                    selectedAccountAddresses={selectedAccountAddresses}
                     targetSubjectMetadata={targetSubjectMetadata}
-                    history={this.props.history}
-                    connectPath={connectPath}
-                    snapsInstallPrivacyWarningShown={
-                      snapsInstallPrivacyWarningShown
-                    }
-                    setSnapsInstallPrivacyWarningShownStatus={
-                      setSnapsInstallPrivacyWarningShownStatus
-                    }
                   />
                 )
               }
             />
+            <Route
+              path={confirmPermissionPath}
+              exact
+              render={() => (
+                <PermissionPageContainer
+                  request={permissionsRequest || {}}
+                  approvePermissionsRequest={(...args) => {
+                    approvePermissionsRequest(...args);
+                    this.redirect(true);
+                  }}
+                  rejectPermissionsRequest={(requestId) =>
+                    this.cancelPermissionsRequest(requestId)
+                  }
+                  selectedAccounts={accounts.filter((account) =>
+                    selectedAccountAddresses.has(account.address),
+                  )}
+                  targetSubjectMetadata={targetSubjectMetadata}
+                  history={this.props.history}
+                  connectPath={connectPath}
+                  snapsInstallPrivacyWarningShown={
+                    snapsInstallPrivacyWarningShown
+                  }
+                  setSnapsInstallPrivacyWarningShownStatus={
+                    setSnapsInstallPrivacyWarningShownStatus
+                  }
+                />
+              )}
+            />
             <Route
               path={snapsConnectPath}
               exact