m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-25 03:20:23 +01:00
Code

[FLASK] Allow Snaps to use eth_accounts as a revokable permission (#19306)

Browse Source 
* Add support for snap authorship component at the top of PermissionConnect

* Add PermissionCellOptions

* Add details popover

* Add action for revoking dynamic permissions

* Improve UI and revoke logic

* Better eth_accounts screen support

* Fix tests

* Fix lint

* More linting fixes

* Fix missing fence

* Add another fence

* Unnest permission page to fix weird CSS issues

* Hide footer on permissions connect when using a snap
This commit is contained in:
Frederik Bolding 2023-07-06 22:54:27 +02:00 committed by GitHub
parent e8bd57fd13
commit f829f0069d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 317 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
app/_locales/en/messages.json generated
View File

@ -366,6 +366,9 @@
"allowThisSiteTo": {
"message": "Allow this site to:"
},
"allowThisSnapTo": {
"message": "Allow this snap to:"
},
"allowWithdrawAndSpend": {
"message": "Allow $1 to withdraw and spend up to the following amount:",
"description": "The url of the site that requested permission to 'withdraw and spend'"
@ -3560,6 +3563,9 @@
"message": "This revokes the permission for a third party to access and transfer all of your NFTs from $1 without further notice.",
"description": "$1 is a link to contract on the block explorer when we're not able to retrieve a erc721 or erc1155 name"
},
"revokePermission": {
"message": "Revoke permission"
},
"revokeSpendingCap": {
"message": "Revoke spending cap for your $1",
"description": "$1 is a token symbol"
@ -3681,6 +3687,9 @@
"selectAccounts": {
"message": "Select the account(s) to use on this site"
},
"selectAccountsForSnap": {
"message": "Select the account(s) to use with this snap"
},
"selectAll": {
"message": "Select all"
},
@ -3770,10 +3779,6 @@
"settingsSearchMatchingNotFound": {
"message": "No matching results found."
},
"shortVersion": {
"message": "v$1",
"description": "$1 is the version number to show"
},
"show": {
"message": "Show"
},

4
app/scripts/metamask-controller.js
View File

@ -2480,6 +2480,10 @@ export default class MetamaskController extends EventEmitter {
this.controllerMessenger,
'SnapController:handleRequest',
),
revokeDynamicSnapPermissions: this.controllerMessenger.call.bind(
this.controllerMessenger,
'SnapController:revokeDynamicPermissions',
),
dismissNotifications: this.dismissNotifications.bind(this),
markNotificationsAsRead: this.markNotificationsAsRead.bind(this),
///: END:ONLY_INCLUDE_IN

2
shared/constants/snaps/permissions.ts
View File

@ -29,3 +29,5 @@ export const ExcludedSnapEndowments = Object.freeze({
'endowment:long-running is deprecated. For more information please see https://github.com/MetaMask/snaps-monorepo/issues/945.',
///: END:ONLY_INCLUDE_IN
});
export const DynamicSnapPermissions = Object.freeze(['eth_accounts']);

101
ui/components/app/permission-cell/permission-cell-options.js Normal file
View File

@ -0,0 +1,101 @@
import React, { useState, useRef } from 'react';
import PropTypes from 'prop-types';
import { useDispatch } from 'react-redux';
import Box from '../../ui/box';
import { useI18nContext } from '../../../hooks/useI18nContext';
import { IconName, ButtonIcon, Text } from '../../component-library';
import { Menu, MenuItem } from '../../ui/menu';
import {
TextColor,
TextVariant,
} from '../../../helpers/constants/design-system';
import Popover from '../../ui/popover/popover.component';
import { DynamicSnapPermissions } from '../../../../shared/constants/snaps/permissions';
import { revokeDynamicSnapPermissions } from '../../../store/actions';
export const PermissionCellOptions = ({
snapId,
permissionName,
description,
}) => {
const t = useI18nContext();
const dispatch = useDispatch();
const ref = useRef(false);
const [showOptions, setShowOptions] = useState(false);
const [showDetails, setShowDetails] = useState(false);
const isRevokable = DynamicSnapPermissions.includes(permissionName);
const handleOpen = () => {
setShowOptions(true);
};
const handleClose = () => {
setShowOptions(false);
};
const handleDetailsOpen = () => {
setShowOptions(false);
setShowDetails(true);
};
const handleDetailsClose = () => {
setShowOptions(false);
setShowDetails(false);
};
const handleRevokePermission = () => {
setShowOptions(false);
dispatch(revokeDynamicSnapPermissions(snapId, [permissionName]));
};
return (
<Box ref={ref}>
<ButtonIcon
iconName={IconName.MoreVertical}
ariaLabel={t('options')}
onClick={handleOpen}
/>
{showOptions && (
<Menu anchorElement={ref.current} onHide={handleClose}>
<MenuItem onClick={handleDetailsOpen}>
<Text
variant={TextVariant.bodySm}
style={{
whiteSpace: 'nowrap',
}}
>
{t('details')}
</Text>
</MenuItem>
{isRevokable && (
<MenuItem onClick={handleRevokePermission}>
<Text
variant={TextVariant.bodySm}
color={TextColor.errorDefault}
style={{
whiteSpace: 'nowrap',
}}
>
{t('revokePermission')}
</Text>
</MenuItem>
)}
</Menu>
)}
{showDetails && (
<Popover title={t('details')} onClose={handleDetailsClose}>
<Box marginLeft={4} marginRight={4} marginBottom={4}>
<Text>{description}</Text>
</Box>
</Popover>
)}
</Box>
);
};
PermissionCellOptions.propTypes = {
snapId: PropTypes.string.isRequired,
permissionName: PropTypes.string.isRequired,
description: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),
};

15
ui/components/app/permission-cell/permission-cell.js
View File

@ -21,14 +21,18 @@ import {
import { formatDate } from '../../../helpers/utils/util';
import { useI18nContext } from '../../../hooks/useI18nContext';
import Tooltip from '../../ui/tooltip';
import { PermissionCellOptions } from './permission-cell-options';
const PermissionCell = ({
snapId,
permissionName,
title,
description,
weight,
avatarIcon,
dateApproved,
revoked,
showOptions,
}) => {
const t = useI18nContext();
@ -107,15 +111,25 @@ const PermissionCell = ({
</Text>
</Box>
<Box>
{showOptions && snapId ? (
<PermissionCellOptions
snapId={snapId}
permissionName={permissionName}
description={description}
/>
) : (
<Tooltip html={<div>{description}</div>} position="bottom">
<Icon color={infoIconColor} name={infoIcon} size={IconSize.Sm} />
</Tooltip>
)}
</Box>
</Box>
);
};
PermissionCell.propTypes = {
snapId: PropTypes.string,
permissionName: PropTypes.string.isRequired,
title: PropTypes.oneOfType([
PropTypes.string.isRequired,
PropTypes.object.isRequired,
@ -125,6 +139,7 @@ PermissionCell.propTypes = {
avatarIcon: PropTypes.any.isRequired,
dateApproved: PropTypes.number,
revoked: PropTypes.bool,
showOptions: PropTypes.bool,
};
export default PermissionCell;

3
ui/components/app/permission-cell/permission-cell.test.js
View File

@ -19,6 +19,7 @@ describe('Permission Cell', () => {
it('renders approved permission cell', () => {
renderWithProvider(
<PermissionCell
permissionName={mockPermissionData.permissionName}
title={mockPermissionData.label}
description={mockPermissionData.description}
weight={mockPermissionData.weight}
@ -36,6 +37,7 @@ describe('Permission Cell', () => {
it('renders revoked permission cell', () => {
renderWithProvider(
<PermissionCell
permissionName={mockPermissionData.permissionName}
title={mockPermissionData.label}
description={mockPermissionData.description}
weight={mockPermissionData.weight}
@ -54,6 +56,7 @@ describe('Permission Cell', () => {
it('renders requested permission cell', () => {
renderWithProvider(
<PermissionCell
permissionName={mockPermissionData.permissionName}
title={mockPermissionData.label}
description={mockPermissionData.description}
weight={mockPermissionData.weight}

38
ui/components/app/permission-page-container/index.scss
View File

@ -10,16 +10,24 @@
justify-content: space-between;
}
@include screen-sm-min {
width: 426px;
flex: 1;
&__footers {
display: flex;
flex-direction: column-reverse;
width: 100%;
flex: 1;
padding-bottom: 20px;
justify-content: space-between;
display: flex;
flex-direction: column;
justify-content: flex-end;
.page-container__footer {
align-items: center;
margin-top: 12px;
@include screen-sm-min {
border-top: none;
}
footer {
width: 100%;
}
}
}
@ -38,6 +46,7 @@
color: var(--color-text-default);
padding-left: 24px;
padding-right: 24px;
margin-top: 2px;
a,
a:hover {
@ -90,19 +99,6 @@
margin-top: 38px;
}
.page-container__footer {
align-items: center;
margin-top: 12px;
@include screen-sm-min {
border-top: none;
}
footer {
width: 100%;
}
}
@include screen-sm-max {
&__title {
position: initial;

28
ui/components/app/permission-page-container/permission-page-container-content/permission-page-container-content.component.js
View File

@ -1,5 +1,8 @@
import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
///: BEGIN:ONLY_INCLUDE_IN(snaps)
import { SubjectType } from '@metamask/permission-controller';
///: END:ONLY_INCLUDE_IN
import PermissionsConnectHeader from '../../permissions-connect-header';
import Tooltip from '../../../ui/tooltip';
import PermissionsConnectPermissionList from '../../permissions-connect-permission-list';
@ -96,12 +99,28 @@ export default class PermissionPageContainerContent extends PureComponent {
return t('connectTo', [selectedIdentities[0]?.addressLabel]);
}
render() {
getHeaderText() {
const { subjectMetadata } = this.props;
const { t } = this.context;
///: BEGIN:ONLY_INCLUDE_IN(snaps)
if (subjectMetadata.subjectType === SubjectType.Snap) {
return t('allowThisSnapTo');
}
///: END:ONLY_INCLUDE_IN
return subjectMetadata.extensionId
? t('allowExternalExtensionTo', [subjectMetadata.extensionId])
: t('allowThisSiteTo');
}
render() {
const { subjectMetadata } = this.props;
const title = this.getTitle();
const headerText = this.getHeaderText();
return (
<div className="permission-approval-container__content">
<div className="permission-approval-container__content-container">
@ -109,12 +128,9 @@ export default class PermissionPageContainerContent extends PureComponent {
iconUrl={subjectMetadata.iconUrl}
iconName={subjectMetadata.name}
headerTitle={title}
headerText={
subjectMetadata.extensionId
? t('allowExternalExtensionTo', [subjectMetadata.extensionId])
: t('allowThisSiteTo')
}
headerText={headerText}
siteOrigin={subjectMetadata.origin}
subjectType={subjectMetadata.subjectType}
/>
<section className="permission-approval-container__permissions-container">
{this.renderRequestedPermissions()}

7
ui/components/app/permission-page-container/permission-page-container.component.js
View File

@ -7,6 +7,7 @@ import {
WALLET_SNAP_PERMISSION_KEY,
} from '@metamask/rpc-methods';
///: END:ONLY_INCLUDE_IN
import { SubjectType } from '@metamask/permission-controller';
import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
import { PageContainerFooter } from '../../ui/page-container';
import PermissionsConnectFooter from '../permissions-connect-footer';
@ -189,7 +190,7 @@ export default class PermissionPageContainer extends Component {
///: END:ONLY_INCLUDE_IN
return (
<div className="page-container permission-approval-container">
<>
{
///: BEGIN:ONLY_INCLUDE_IN(snaps)
<>
@ -210,7 +211,9 @@ export default class PermissionPageContainer extends Component {
allIdentitiesSelected={allIdentitiesSelected}
/>
<div className="permission-approval-container__footers">
{targetSubjectMetadata?.subjectType !== SubjectType.Snap && (
<PermissionsConnectFooter />
)}
<PageContainerFooter
cancelButtonType="default"
onCancel={() => this.onCancel()}
@ -220,7 +223,7 @@ export default class PermissionPageContainer extends Component {
buttonSizeLarge={false}
/>
</div>
</div>
</>
);
}
}

22
ui/components/app/permissions-connect-header/permissions-connect-header.component.js
View File

@ -1,6 +1,9 @@
import PropTypes from 'prop-types';
import React, { Component } from 'react';
import classnames from 'classnames';
///: BEGIN:ONLY_INCLUDE_IN(snaps)
import { SubjectType } from '@metamask/subject-metadata-controller';
///: END:ONLY_INCLUDE_IN
import SiteOrigin from '../../ui/site-origin';
import Box from '../../ui/box';
import {
@ -19,6 +22,7 @@ export default class PermissionsConnectHeader extends Component {
headerText: PropTypes.string,
leftIcon: PropTypes.node,
rightIcon: PropTypes.node,
subjectType: PropTypes.string,
};
static defaultProps = {
@ -29,7 +33,23 @@ export default class PermissionsConnectHeader extends Component {
};
renderHeaderIcon() {
const { iconUrl, iconName, siteOrigin, leftIcon, rightIcon } = this.props;
const {
iconUrl,
iconName,
siteOrigin,
leftIcon,
rightIcon,
///: BEGIN:ONLY_INCLUDE_IN(snaps)
subjectType,
///: END:ONLY_INCLUDE_IN
} = this.props;
///: BEGIN:ONLY_INCLUDE_IN(snaps)
if (subjectType === SubjectType.Snap) {
return null;
}
///: END:ONLY_INCLUDE_IN
return (
<div className="permissions-connect-header__icon">

9
ui/components/app/snaps/snap-authorship-header/snap-authorship-header.js
View File

@ -24,7 +24,11 @@ import { getTargetSubjectMetadata } from '../../../../selectors';
import SnapAvatar from '../snap-avatar';
import SnapVersion from '../snap-version/snap-version';
const SnapAuthorshipHeader = ({ snapId, className }) => {
const SnapAuthorshipHeader = ({
snapId,
className,
boxShadow = 'var(--shadow-size-lg) var(--color-shadow-default)',
}) => {
// We're using optional chaining with snapId, because with the current implementation
// of snap update in the snap controller, we do not have reference to snapId when an
// update request is rejected because the reference comes from the request itself and not subject metadata
@ -51,7 +55,7 @@ const SnapAuthorshipHeader = ({ snapId, className }) => {
display={DISPLAY.FLEX}
padding={4}
style={{
boxShadow: 'var(--shadow-size-lg) var(--color-shadow-default)',
boxShadow,
}}
>
<Box>
@ -91,6 +95,7 @@ SnapAuthorshipHeader.propTypes = {
* The className of the SnapAuthorship
*/
className: PropTypes.string,
boxShadow: PropTypes.string,
};
export default SnapAuthorshipHeader;

7
ui/components/app/snaps/snap-permissions-list/snap-permissions-list.js
View File

@ -6,8 +6,10 @@ import PermissionCell from '../../permission-cell';
import Box from '../../../ui/box';
export default function SnapPermissionsList({
snapId,
permissions,
targetSubjectMetadata,
showOptions,
}) {
const t = useI18nContext();
@ -17,12 +19,15 @@ export default function SnapPermissionsList({
(permission, index) => {
return (
<PermissionCell
snapId={snapId}
permissionName={permission.permissionName}
title={permission.label}
description={permission.description}
weight={permission.weight}
avatarIcon={permission.leftIcon}
dateApproved={permission?.permissionValue?.date}
key={`${permission.permissionName}-${index}`}
showOptions={showOptions}
/>
);
},
@ -32,6 +37,8 @@ export default function SnapPermissionsList({
}
SnapPermissionsList.propTypes = {
snapId: PropTypes.string.isRequired,
permissions: PropTypes.object.isRequired,
targetSubjectMetadata: PropTypes.object.isRequired,
showOptions: PropTypes.bool,
};

4
ui/components/app/snaps/snap-version/snap-version.js
View File

@ -18,10 +18,8 @@ import {
Text,
} from '../../../component-library';
import Preloader from '../../../ui/icon/preloader/preloader-icon.component';
import { useI18nContext } from '../../../../hooks/useI18nContext';
const SnapVersion = ({ version, url }) => {
const t = useI18nContext();
return (
<Button
variant={BUTTON_VARIANT.LINK}
@ -42,7 +40,7 @@ const SnapVersion = ({ version, url }) => {
>
{version ? (
<Text color={Color.textAlternative} variant={TextVariant.bodyMd}>
{t('shortVersion', [version])}
{version}
</Text>
) : (
<Preloader size={18} />

2
ui/components/app/snaps/snap-version/snap-version.test.js
View File

@ -12,7 +12,7 @@ describe('SnapVersion', () => {
const { getByText, container } = renderWithLocalization(
<SnapVersion {...args} />,
);
expect(getByText(`v${args.version}`)).toBeDefined();
expect(getByText(args.version)).toBeDefined();
expect(container.firstChild).toHaveAttribute('href', args.url);
});

3
ui/components/app/snaps/update-snap-permission-list/update-snap-permission-list.js
View File

@ -18,6 +18,7 @@ export default function UpdateSnapPermissionList({
{getWeightedPermissions(t, newPermissions, targetSubjectMetadata).map(
(permission, index) => (
<PermissionCell
permissionName={permission.permissionName}
title={permission.label}
description={permission.description}
weight={permission.weight}
@ -30,6 +31,7 @@ export default function UpdateSnapPermissionList({
{getWeightedPermissions(t, revokedPermissions, targetSubjectMetadata).map(
(permission, index) => (
<PermissionCell
permissionName={permission.permissionName}
title={permission.label}
description={permission.description}
weight={permission.weight}
@ -46,6 +48,7 @@ export default function UpdateSnapPermissionList({
targetSubjectMetadata,
).map((permission, index) => (
<PermissionCell
permissionName={permission.permissionName}
title={permission.label}
description={permission.description}
weight={permission.weight}

25
ui/pages/permissions-connect/choose-account/choose-account.js
View File

@ -1,5 +1,6 @@
import PropTypes from 'prop-types';
import React, { useState } from 'react';
import { SubjectType } from '@metamask/permission-controller';
import { useI18nContext } from '../../../hooks/useI18nContext';
import PermissionsConnectHeader from '../../../components/app/permissions-connect-header';
import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer';
@ -47,6 +48,21 @@ const ChooseAccount = ({
return accounts.length === selectedAccounts.size;
};
const getHeaderText = () => {
if (accounts.length === 0) {
return t('connectAccountOrCreate');
}
///: BEGIN:ONLY_INCLUDE_IN(snaps)
if (targetSubjectMetadata?.subjectType === SubjectType.Snap) {
return t('selectAccountsForSnap');
}
///: END:ONLY_INCLUDE_IN
return t('selectAccounts');
};
const headerText = getHeaderText();
return (
<>
<div className="permissions-connect-choose-account__content">
@ -54,12 +70,9 @@ const ChooseAccount = ({
iconUrl={targetSubjectMetadata?.iconUrl}
iconName={targetSubjectMetadata?.name}
headerTitle={t('connectWithMetaMask')}
headerText={
accounts.length > 0
? t('selectAccounts')
: t('connectAccountOrCreate')
}
headerText={headerText}
siteOrigin={targetSubjectMetadata?.origin}
subjectType={targetSubjectMetadata?.subjectType}
/>
<AccountList
accounts={accounts}
@ -74,7 +87,9 @@ const ChooseAccount = ({
/>
</div>
<div className="permissions-connect-choose-account__footer-container">
{targetSubjectMetadata?.subjectType !== SubjectType.Snap && (
<PermissionsConnectFooter />
)}
<PageContainerFooter
cancelButtonType="default"
onCancel={() => cancelPermissionsRequest(permissionsRequestId)}

40
ui/pages/permissions-connect/permissions-connect.component.js
View File

@ -3,13 +3,22 @@ import React, { Component } from 'react';
import { Switch, Route } from 'react-router-dom';
///: BEGIN:ONLY_INCLUDE_IN(snaps)
import { ethErrors, serializeError } from 'eth-rpc-errors';
import { SubjectType } from '@metamask/subject-metadata-controller';
///: END:ONLY_INCLUDE_IN
import { getEnvironmentType } from '../../../app/scripts/lib/util';
import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
import { MILLISECOND } from '../../../shared/constants/time';
import { DEFAULT_ROUTE } from '../../helpers/constants/routes';
import PermissionPageContainer from '../../components/app/permission-page-container';
import { Icon, IconName, IconSize } from '../../components/component-library';
import {
Box,
Icon,
IconName,
IconSize,
} from '../../components/component-library';
///: BEGIN:ONLY_INCLUDE_IN(snaps)
import SnapAuthorshipHeader from '../../components/app/snaps/snap-authorship-header/snap-authorship-header';
///: END:ONLY_INCLUDE_IN
import ChooseAccount from './choose-account';
import PermissionsRedirect from './redirect';
///: BEGIN:ONLY_INCLUDE_IN(snaps)
@ -268,10 +277,26 @@ export default class PermissionConnect extends Component {
}
renderTopBar() {
const { redirecting } = this.state;
const {
redirecting,
///: BEGIN:ONLY_INCLUDE_IN(snaps)
targetSubjectMetadata,
///: END:ONLY_INCLUDE_IN
} = this.state;
const { page, isRequestingAccounts, totalPages } = this.props;
const { t } = this.context;
return redirecting ? null : (
<Box
style={{
///: BEGIN:ONLY_INCLUDE_IN(snaps)
marginBottom:
targetSubjectMetadata.subjectType === SubjectType.Snap && '14px',
boxShadow:
targetSubjectMetadata.subjectType === SubjectType.Snap &&
'var(--shadow-size-lg) var(--color-shadow-default)',
///: END:ONLY_INCLUDE_IN
}}
>
<div className="permissions-connect__top-bar">
{page === '2' && isRequestingAccounts ? (
<div
@ -292,6 +317,17 @@ export default class PermissionConnect extends Component {
</div>
) : null}
</div>
{
///: BEGIN:ONLY_INCLUDE_IN(snaps)
targetSubjectMetadata.subjectType === SubjectType.Snap && (
<SnapAuthorshipHeader
snapId={targetSubjectMetadata.origin}
boxShadow="none"
/>
)
///: END:ONLY_INCLUDE_IN
}
</Box>
);
}

1
ui/pages/permissions-connect/snaps/snap-install/snap-install.js
View File

@ -179,6 +179,7 @@ export default function SnapInstall({
])}
</Text>
<SnapPermissionsList
snapId={targetSubjectMetadata.origin}
permissions={requestState.permissions || {}}
targetSubjectMetadata={targetSubjectMetadata}
/>

2
ui/pages/settings/snaps/view-snap/view-snap.js
View File

@ -149,8 +149,10 @@ function ViewSnap() {
<Box className="view-snap__permissions" marginTop={12}>
<Text variant={TextVariant.bodyLgMedium}>{t('permissions')}</Text>
<SnapPermissionsList
snapId={decodedSnapId}
permissions={permissions ?? {}}
targetSubjectMetadata={targetSubjectMetadata}
showOptions
/>
</Box>
<Box className="view-snap__connected-sites" marginTop={12}>

2
ui/pages/settings/snaps/view-snap/view-snap.test.js
View File

@ -46,7 +46,7 @@ describe('ViewSnap', () => {
getByText('An example Snap that signs messages using BLS.'),
).toBeDefined();
// Snap version info
expect(getByText('v5.1.2')).toBeDefined();
expect(getByText('5.1.2')).toBeDefined();
// Enable Snap
expect(getByText('Enabled')).toBeDefined();
expect(container.getElementsByClassName('toggle-button')?.length).toBe(1);

13
ui/store/actions.ts
View File

@ -1252,6 +1252,19 @@ export function markNotificationsAsRead(
};
}
export function revokeDynamicSnapPermissions(
snapId: string,
permissionNames: string[],
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
return async (dispatch: MetaMaskReduxDispatch) => {
await submitRequestToBackground('revokeDynamicSnapPermissions', [
snapId,
permissionNames,
]);
await forceUpdateMetamaskState(dispatch);
};
}
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(desktop)