Skip to content

Commit

Permalink
Add Snap website field to Snap settings page and make UI changes (#21095
Browse files Browse the repository at this point in the history
)

## **Description**
This PR adds a website URL to the Snap settings page and makes coupe of
changes to the UI elements as requested in related task.

PR summary:
- Add new selector `getSnapRegistryData` for getting Snap registry data.
- Add Snap website URL to Snap Authorship component.
- Change origin to be plain text instead of link.
- Change color of text to default for connected websites.
- Check website against Phishing list using `PhishingController` before
displaying it and display it only if it's not blacklisted.

## **Manual testing steps**
1. Install one or more Snaps.
2. Go to Settings page.
3. Click on one of the Snaps listed.
4. Check the UI changes requested in related task.

## **Screenshots/Recordings**

### **Before**
![Screenshot 2023-09-28 at 16 11
29](https://github.com/MetaMask/metamask-extension/assets/13301024/8d5c8ead-b539-4eb9-bcc8-c161b7c4109d)
![Screenshot 2023-09-28 at 16 11
48](https://github.com/MetaMask/metamask-extension/assets/13301024/fa249118-f536-46ca-a9a9-6e81d13ed078)

### **After**
![Screenshot 2023-09-28 at 15 21
58](https://github.com/MetaMask/metamask-extension/assets/13301024/4bf8e6e5-b114-4b54-9e4e-3c14ce40db50)
![Screenshot 2023-09-28 at 16 07
43](https://github.com/MetaMask/metamask-extension/assets/13301024/71df9135-d5c1-40a1-a49c-7152765912f8)
![Screenshot 2023-10-02 at 17 18
33](https://github.com/MetaMask/metamask-extension/assets/13301024/1e576df3-8b97-4d46-a978-741669c74daf)

## **Related issues**
Fixes: #20988

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] 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)).
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
  • Loading branch information
david0xd authored Oct 4, 2023
1 parent 54b2f70 commit 2e1ab26
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 8 deletions.
6 changes: 6 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,7 @@ export default class MetamaskController extends EventEmitter {
assetsContractController,
backup,
approvalController,
phishingController,
} = this;

return {
Expand Down Expand Up @@ -2822,6 +2823,11 @@ export default class MetamaskController extends EventEmitter {
dismissNotifications: this.dismissNotifications.bind(this),
markNotificationsAsRead: this.markNotificationsAsRead.bind(this),
updateCaveat: this.updateCaveat.bind(this),
getPhishingResult: async (website) => {
await phishingController.maybeUpdateState();

return phishingController.test(website);
},
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
updateSnapRegistry: this.preferencesController.updateSnapRegistry.bind(
Expand Down
35 changes: 34 additions & 1 deletion test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,40 @@
"requestState": { "test": "value" }
}
},
"pendingApprovalCount": 1
"pendingApprovalCount": 1,
"database": {
"verifiedSnaps": {
"npm:@metamask/test-snap-bip44": {
"id": "npm:@metamask/test-snap-bip44",
"metadata": {
"name": "BIP-44",
"author": {
"name": "Consensys",
"website": "https://consensys.io/"
},
"website": "https://snaps.consensys.io/",
"summary": "An example Snap that signs messages using BLS.",
"description": "An example Snap that signs messages using BLS.",
"audits": [
{
"auditor": "Consensys Diligence",
"report": "https://consensys.io/diligence/audits/"
}
],
"category": "interoperability",
"support": {
"contact": "https://github.com/MetaMask"
},
"sourceCode": "https://github.com/MetaMask/test-snaps"
},
"versions": {
"5.1.2": {
"checksum": "L1k+dT9Q+y3KfIqzaH09MpDZVPS9ZowEh9w01ZMTWMU="
}
}
}
}
}
},
"send": {
"amountMode": "INPUT",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getSnapPrefix } from '@metamask/snaps-utils';
import classnames from 'classnames';
import PropTypes from 'prop-types';
import React from 'react';
import React, { useEffect, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
AlignItems,
Expand All @@ -15,6 +15,7 @@ import {
FlexDirection,
FontWeight,
JustifyContent,
OverflowWrap,
TextColor,
TextVariant,
} from '../../../../helpers/constants/design-system';
Expand All @@ -25,8 +26,15 @@ import {
} from '../../../../helpers/utils/util';
import { useI18nContext } from '../../../../hooks/useI18nContext';
import { useOriginMetadata } from '../../../../hooks/useOriginMetadata';
import { getTargetSubjectMetadata } from '../../../../selectors';
import { disableSnap, enableSnap } from '../../../../store/actions';
import {
getSnapRegistryData,
getTargetSubjectMetadata,
} from '../../../../selectors';
import {
disableSnap,
enableSnap,
getPhishingResult,
} from '../../../../store/actions';
import { Box, ButtonLink, Text } from '../../../component-library';
import ToggleButton from '../../../ui/toggle-button';
import Tooltip from '../../../ui/tooltip/tooltip';
Expand All @@ -53,6 +61,24 @@ const SnapAuthorshipExpanded = ({ snapId, className, snap }) => {
const subjectMetadata = useSelector((state) =>
getTargetSubjectMetadata(state, snapId),
);
const snapRegistryData = useSelector((state) =>
getSnapRegistryData(state, snapId),
);
const { website = undefined } = snapRegistryData?.metadata ?? {};
const [safeWebsite, setSafeWebsite] = useState(null);

useEffect(() => {
const performPhishingCheck = async () => {
const phishingResult = await getPhishingResult(website);

if (!phishingResult.result) {
setSafeWebsite(website);
}
};
if (website) {
performPhishingCheck();
}
}, [website]);

const friendlyName = snapId && getSnapName(snapId, subjectMetadata);

Expand Down Expand Up @@ -134,6 +160,33 @@ const SnapAuthorshipExpanded = ({ snapId, className, snap }) => {
</Box>
</Box>
<Box padding={4} width={BlockSize.Full}>
{safeWebsite && (
<Box
display={Display.Flex}
flexDirection={FlexDirection.Row}
justifyContent={JustifyContent.spaceBetween}
width={BlockSize.Full}
marginBottom={4}
>
<Text variant={TextVariant.bodyMd} fontWeight={FontWeight.Medium}>
{t('snapDetailWebsite')}
</Text>
<Box
paddingLeft={8}
display={Display.Flex}
flexDirection={FlexDirection.Column}
alignItems={AlignItems.flexEnd}
>
<ButtonLink
href={installOrigin.origin}
target="_blank"
overflowWrap={OverflowWrap.Anywhere}
>
{safeWebsite}
</ButtonLink>
</Box>
</Box>
)}
{installOrigin && installInfo && (
<Box
display={Display.Flex}
Expand All @@ -149,9 +202,7 @@ const SnapAuthorshipExpanded = ({ snapId, className, snap }) => {
flexDirection={FlexDirection.Column}
alignItems={AlignItems.flexEnd}
>
<ButtonLink href={installOrigin.origin} target="_blank">
{installOrigin.host}
</ButtonLink>
<Text>{installOrigin.host}</Text>
<Text color={Color.textMuted}>
{t('installedOn', [
formatDate(installInfo.date, 'dd MMM yyyy'),
Expand Down
1 change: 0 additions & 1 deletion ui/pages/settings/snaps/view-snap/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@

&__subject-name {
font-size: 14px;
color: var(--color-primary-default);
}
}
}
12 changes: 12 additions & 0 deletions ui/pages/settings/snaps/view-snap/view-snap.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { waitFor, screen } from '@testing-library/react';
import { renderWithProvider } from '../../../../../test/lib/render-helpers';
import mockState from '../../../../../test/data/mock-state.json';
import ViewSnap from './view-snap';
Expand All @@ -12,6 +13,11 @@ jest.mock('../../../../store/actions.ts', () => {
removeSnap: jest.fn(),
removePermissionsFor: jest.fn(),
updateCaveat: jest.fn(),
getPhishingResult: jest.fn().mockImplementation(() => {
return {
result: false,
};
}),
};
});

Expand Down Expand Up @@ -45,6 +51,12 @@ describe('ViewSnap', () => {
expect(
getByText('An example Snap that signs messages using BLS.'),
).toBeDefined();
// Snap website
await waitFor(() => {
const websiteElement = screen.queryByText('https://snaps.consensys.io/');
expect(websiteElement).toBeDefined();
expect(getByText('https://snaps.consensys.io/')).toBeDefined();
});
// Snap version info
expect(getByText('5.1.2')).toBeDefined();
// Enable Snap
Expand Down
12 changes: 12 additions & 0 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,18 @@ export function getTargetSubjectMetadata(state, origin) {
return metadata;
}

/**
* Retrieve registry data for requested Snap.
*
* @param state - Redux state object.
* @param snapId - ID of a Snap.
* @returns Object containing metadata stored in Snaps registry for requested Snap.
*/
export function getSnapRegistryData(state, snapId) {
const snapsRegistryData = state.metamask.database.verifiedSnaps;
return snapsRegistryData ? snapsRegistryData[snapId] : null;
}

export function getRpcPrefsForCurrentProvider(state) {
const { rpcPrefs } = getProviderConfig(state);
return rpcPrefs || {};
Expand Down
18 changes: 18 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -780,4 +780,22 @@ describe('Selectors', () => {
isInfuraBlocked = selectors.getInfuraBlocked(modifiedMockState);
expect(isInfuraBlocked).toBe(true);
});

it('#getSnapRegistryData', () => {
const mockSnapId = 'npm:@metamask/test-snap-bip44';
expect(selectors.getSnapRegistryData(mockState, mockSnapId)).toStrictEqual(
expect.objectContaining({
id: mockSnapId,
versions: {
'5.1.2': {
checksum: 'L1k+dT9Q+y3KfIqzaH09MpDZVPS9ZowEh9w01ZMTWMU=',
},
},
metadata: expect.objectContaining({
website: 'https://snaps.consensys.io/',
name: 'BIP-44',
}),
}),
);
});
});
4 changes: 4 additions & 0 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,10 @@ export function enableSnap(
await forceUpdateMetamaskState(dispatch);
};
}

export async function getPhishingResult(website: string) {
return await submitRequestToBackground('getPhishingResult', [website]);
}
///: END:ONLY_INCLUDE_IN

// TODO: Clean this up.
Expand Down

0 comments on commit 2e1ab26

Please sign in to comment.