Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useEffect, useRef, useState } from 'react';
import { useHistory } from 'react-router-dom';
import { useSelector } from 'react-redux';
import { isSnapId } from '@metamask/snaps-utils';
import { SubjectType } from '@metamask/permission-controller';
import { Content, Header, Page } from '../page';
import {
Box,
Expand All @@ -27,8 +28,14 @@ import {
REVIEW_PERMISSIONS,
GATOR_PERMISSIONS,
} from '../../../../helpers/constants/routes';
import { getConnectedSitesListWithNetworkInfo } from '../../../../selectors';
import {
getConnectedSitesListWithNetworkInfo,
getTargetSubjectMetadata,
} from '../../../../selectors';
import { getGatorPermissionsMap } from '../../../../selectors/gator-permissions/gator-permissions';
import { isGatorPermissionsRevocationFeatureEnabled } from '../../../../../shared/modules/environment';
import { getURLHostName } from '../../../../helpers/utils/util';
import { getNetworkNameByChainId } from '../../../../../shared/modules/feature-flags';
import { ConnectionListItem } from './connection-list-item';

export const PermissionsPage = () => {
Expand All @@ -39,10 +46,85 @@ export const PermissionsPage = () => {
const sitesConnectionsList = useSelector(
getConnectedSitesListWithNetworkInfo,
);
const gatorPermissionsMap = useSelector(getGatorPermissionsMap);

// Get all unique site origins from gator permissions with their first chainId and all unique addresses
const getUniqueSiteOriginsFromGatorPermissions = (permissionsMap) => {
const siteOriginsMap = new Map();

Object.values(permissionsMap).forEach((permissionTypeMap) => {
Object.values(permissionTypeMap).forEach((permissions) => {
permissions.forEach((permission) => {
if (permission.siteOrigin) {
if (!siteOriginsMap.has(permission.siteOrigin)) {
// Store the first permission data for this site origin
siteOriginsMap.set(permission.siteOrigin, {
addresses: new Set(),
chainId: permission.permissionResponse.chainId,
});
}

// Add address to the set if it exists
const permissionData = siteOriginsMap.get(permission.siteOrigin);
if (permission.permissionResponse.address) {
permissionData.addresses.add(
permission.permissionResponse.address.toLowerCase(),
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Undefined Object Access and Type Errors

The getUniqueSiteOriginsFromGatorPermissions function accesses permission.permissionResponse.chainId and permission.permissionResponse.address without ensuring permissionResponse is defined, which can cause TypeErrors. It also calls .toLowerCase() on the address without confirming it's a string, potentially leading to further runtime errors.

Fix in Cursor Fix in Web

}
}
});
});
});

// Convert Set to Array for each site origin
siteOriginsMap.forEach((permissionData) => {
permissionData.addresses = Array.from(permissionData.addresses);
});

return siteOriginsMap;
};

// Get merged connections list using useSelector to access getTargetSubjectMetadata
const mergedConnectionsList = useSelector((state) => {
if (!isGatorPermissionsRevocationFeatureEnabled()) {
return sitesConnectionsList;
}

const gatorSiteOriginsMap =
getUniqueSiteOriginsFromGatorPermissions(gatorPermissionsMap);
const mergedConnections = { ...sitesConnectionsList };

// Add sites that only have gator permissions but no site connections
gatorSiteOriginsMap.forEach((permissionData, siteOrigin) => {
if (!mergedConnections[siteOrigin]) {
const { addresses, chainId } = permissionData;
const networkName = getNetworkNameByChainId(chainId);

// Get subject metadata for name and iconUrl
const subjectMetadata = getTargetSubjectMetadata(state, siteOrigin);
const siteName = subjectMetadata?.name || getURLHostName(siteOrigin);
const siteIconUrl = subjectMetadata?.iconUrl || null;

// Create a minimal connection object for sites that only have gator permissions
mergedConnections[siteOrigin] = {
addresses: addresses || [],
origin: siteOrigin,
name: siteName,
iconUrl: siteIconUrl,
subjectType: SubjectType.Website,
networkIconUrl: '',
networkName: networkName || '',
extensionId: null,
};
}
});

return mergedConnections;
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Selector Callback Captures External Dependencies

The mergedConnectionsList selector's callback captures sitesConnectionsList and gatorPermissionsMap from its closure. This prevents useSelector from tracking these external dependencies, resulting in stale data when they change. Additionally, the getUniqueSiteOriginsFromGatorPermissions helper function is defined within the component, causing it to be recreated on each render and impacting selector memoization.

Fix in Cursor Fix in Web


useEffect(() => {
setTotalConnections(Object.keys(sitesConnectionsList).length);
}, [sitesConnectionsList]);
setTotalConnections(Object.keys(mergedConnectionsList).length);
}, [mergedConnectionsList]);

const handleConnectionClick = (connection) => {
const hostName = connection.origin;
Expand Down Expand Up @@ -100,7 +182,7 @@ export const PermissionsPage = () => {
<Content padding={0}>
<Box ref={headerRef}></Box>
{totalConnections > 0 ? (
renderConnectionsList(sitesConnectionsList)
renderConnectionsList(mergedConnectionsList)
) : (
<Box
data-testid="no-connections"
Expand Down
Loading