1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-22 09:57:02 +01:00

Replace the address in SignTypedData_v4 signatures with a 'Verify contract details' link (#16191)

This commit is contained in:
Filip Sekulic 2022-11-21 18:19:49 +01:00 committed by GitHub
parent 704e837405
commit c87a4f5968
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 203 additions and 119 deletions

View File

@ -790,6 +790,9 @@
"contractRequestingAccess": { "contractRequestingAccess": {
"message": "Contract requesting access" "message": "Contract requesting access"
}, },
"contractRequestingSignature": {
"message": "Contract requesting signature"
},
"contractRequestingSpendingCap": { "contractRequestingSpendingCap": {
"message": "Contract requesting spending cap" "message": "Contract requesting spending cap"
}, },

View File

@ -53,20 +53,22 @@ describe('Sign Typed Data V4 Signature Request', function () {
const content = await driver.findElements( const content = await driver.findElements(
'.signature-request-content__info', '.signature-request-content__info',
); );
const verifyContractDetailsButton = await driver.findElement(
'.signature-request-content__verify-contract-details',
);
const origin = content[0]; const origin = content[0];
const address = content[1];
const message = await driver.findElement( const message = await driver.findElement(
'.signature-request-data__node__value', '.signature-request-data__node__value',
); );
assert.equal(await title.getText(), 'Signature request'); assert.equal(await title.getText(), 'Signature request');
assert.equal(await name.getText(), 'Ether Mail'); assert.equal(await name.getText(), 'Ether Mail');
assert.equal(await origin.getText(), 'http://127.0.0.1:8080'); assert.equal(await origin.getText(), 'http://127.0.0.1:8080');
assert.equal(
await address.getText(), verifyContractDetailsButton.click();
`${publicAddress.slice(0, 8)}...${publicAddress.slice( await driver.findElement({ text: 'Contract details', tag: 'h5' });
publicAddress.length - 8, await driver.findElement('[data-testid="recipient"]');
)}`, await driver.clickElement({ text: 'Got it', tag: 'button' });
);
assert.equal(await message.getText(), 'Hello, Bob!'); assert.equal(await message.getText(), 'Hello, Bob!');
// Approve signing typed data // Approve signing typed data
await driver.clickElement( await driver.clickElement(
@ -137,20 +139,22 @@ describe('Sign Typed Data V3 Signature Request', function () {
const content = await driver.findElements( const content = await driver.findElements(
'.signature-request-content__info', '.signature-request-content__info',
); );
const verifyContractDetailsButton = await driver.findElement(
'.signature-request-content__verify-contract-details',
);
const origin = content[0]; const origin = content[0];
const address = content[1];
const messages = await driver.findElements( const messages = await driver.findElements(
'.signature-request-data__node__value', '.signature-request-data__node__value',
); );
assert.equal(await title.getText(), 'Signature request'); assert.equal(await title.getText(), 'Signature request');
assert.equal(await name.getText(), 'Ether Mail'); assert.equal(await name.getText(), 'Ether Mail');
assert.equal(await origin.getText(), 'http://127.0.0.1:8080'); assert.equal(await origin.getText(), 'http://127.0.0.1:8080');
assert.equal(
await address.getText(), verifyContractDetailsButton.click();
`${publicAddress.slice(0, 8)}...${publicAddress.slice( await driver.findElement({ text: 'Contract details', tag: 'h5' });
publicAddress.length - 8, await driver.findElement('[data-testid="recipient"]');
)}`, await driver.clickElement({ text: 'Got it', tag: 'button' });
);
assert.equal(await messages[4].getText(), 'Hello, Bob!'); assert.equal(await messages[4].getText(), 'Hello, Bob!');
// Approve signing typed data // Approve signing typed data

View File

@ -40,6 +40,7 @@ export default function ContractDetailsModal({
tokenId, tokenId,
assetName, assetName,
assetStandard, assetStandard,
isContractRequestingSignature,
}) { }) {
const t = useI18nContext(); const t = useI18nContext();
const [copiedTokenAddress, handleCopyTokenAddress] = useCopyToClipboard(); const [copiedTokenAddress, handleCopyTokenAddress] = useCopyToClipboard();
@ -80,117 +81,123 @@ export default function ContractDetailsModal({
> >
{t('contractDescription')} {t('contractDescription')}
</Typography> </Typography>
<Typography {!isContractRequestingSignature && (
variant={TYPOGRAPHY.H6} <>
display={DISPLAY.FLEX}
marginTop={4}
marginBottom={2}
>
{nft ? t('contractNFT') : t('contractToken')}
</Typography>
<Box
display={DISPLAY.FLEX}
borderRadius={SIZES.SM}
borderStyle={BORDER_STYLE.SOLID}
borderColor={COLORS.BORDER_DEFAULT}
className="contract-details-modal__content__contract"
>
{nft ? (
<Box margin={4}>
<NftCollectionImage
assetName={assetName}
tokenAddress={tokenAddress}
/>
</Box>
) : (
<Identicon
className="contract-details-modal__content__contract__identicon"
address={tokenAddress}
diameter={24}
/>
)}
<Box data-testid="recipient">
<Typography <Typography
fontWeight={FONT_WEIGHT.BOLD} variant={TYPOGRAPHY.H6}
variant={TYPOGRAPHY.H5} display={DISPLAY.FLEX}
marginTop={4} marginTop={4}
marginBottom={2}
> >
{tokenName || ellipsify(tokenAddress)} {nft ? t('contractNFT') : t('contractToken')}
</Typography> </Typography>
{tokenName && ( <Box
<Typography display={DISPLAY.FLEX}
variant={TYPOGRAPHY.H6} borderRadius={SIZES.SM}
display={DISPLAY.FLEX} borderStyle={BORDER_STYLE.SOLID}
color={COLORS.TEXT_ALTERNATIVE} borderColor={COLORS.BORDER_DEFAULT}
marginTop={0} className="contract-details-modal__content__contract"
marginBottom={4} >
> {nft ? (
{ellipsify(tokenAddress)} <Box margin={4}>
</Typography> <NftCollectionImage
)} assetName={assetName}
</Box> tokenAddress={tokenAddress}
<Box
justifyContent={JUSTIFY_CONTENT.FLEX_END}
className="contract-details-modal__content__contract__buttons"
>
<Box marginTop={4} marginRight={5}>
<Tooltip
position="top"
title={
copiedTokenAddress
? t('copiedExclamation')
: t('copyToClipboard')
}
>
<Button
className="contract-details-modal__content__contract__buttons__copy"
type="link"
onClick={() => {
handleCopyTokenAddress(tokenAddress);
}}
>
<IconCopy color="var(--color-icon-muted)" />
</Button>
</Tooltip>
</Box>
<Box marginTop={5} marginRight={5}>
<Tooltip position="top" title={t('openInBlockExplorer')}>
<Button
className="contract-details-modal__content__contract__buttons__block-explorer"
type="link"
onClick={() => {
const blockExplorerTokenLink = getAccountLink(
tokenAddress,
chainId,
{
blockExplorerUrl: rpcPrefs?.blockExplorerUrl ?? null,
},
null,
);
global.platform.openTab({
url: blockExplorerTokenLink,
});
}}
>
<IconBlockExplorer
size={16}
color="var(--color-icon-muted)"
/> />
</Button> </Box>
</Tooltip> ) : (
<Identicon
className="contract-details-modal__content__contract__identicon"
address={tokenAddress}
diameter={24}
/>
)}
<Box data-testid="recipient">
<Typography
fontWeight={FONT_WEIGHT.BOLD}
variant={TYPOGRAPHY.H5}
marginTop={4}
>
{tokenName || ellipsify(tokenAddress)}
</Typography>
{tokenName && (
<Typography
variant={TYPOGRAPHY.H6}
display={DISPLAY.FLEX}
color={COLORS.TEXT_ALTERNATIVE}
marginTop={0}
marginBottom={4}
>
{ellipsify(tokenAddress)}
</Typography>
)}
</Box>
<Box
justifyContent={JUSTIFY_CONTENT.FLEX_END}
className="contract-details-modal__content__contract__buttons"
>
<Box marginTop={4} marginRight={5}>
<Tooltip
position="top"
title={
copiedTokenAddress
? t('copiedExclamation')
: t('copyToClipboard')
}
>
<Button
className="contract-details-modal__content__contract__buttons__copy"
type="link"
onClick={() => {
handleCopyTokenAddress(tokenAddress);
}}
>
<IconCopy color="var(--color-icon-muted)" />
</Button>
</Tooltip>
</Box>
<Box marginTop={5} marginRight={5}>
<Tooltip position="top" title={t('openInBlockExplorer')}>
<Button
className="contract-details-modal__content__contract__buttons__block-explorer"
type="link"
onClick={() => {
const blockExplorerTokenLink = getAccountLink(
tokenAddress,
chainId,
{
blockExplorerUrl:
rpcPrefs?.blockExplorerUrl ?? null,
},
null,
);
global.platform.openTab({
url: blockExplorerTokenLink,
});
}}
>
<IconBlockExplorer
size={16}
color="var(--color-icon-muted)"
/>
</Button>
</Tooltip>
</Box>
</Box>
</Box> </Box>
</Box> </>
</Box> )}
<Typography <Typography
variant={TYPOGRAPHY.H6} variant={TYPOGRAPHY.H6}
display={DISPLAY.FLEX} display={DISPLAY.FLEX}
marginTop={4} marginTop={4}
marginBottom={2} marginBottom={2}
> >
{nft {nft && t('contractRequestingAccess')}
? t('contractRequestingAccess') {isContractRequestingSignature && t('contractRequestingSignature')}
: t('contractRequestingSpendingCap')} {!nft &&
!isContractRequestingSignature &&
t('contractRequestingSpendingCap')}
</Typography> </Typography>
<Box <Box
display={DISPLAY.FLEX} display={DISPLAY.FLEX}
@ -356,4 +363,8 @@ ContractDetailsModal.propTypes = {
* The name of the collection * The name of the collection
*/ */
assetName: PropTypes.string, assetName: PropTypes.string,
/**
* Whether contract requesting signature flow has started
*/
isContractRequestingSignature: PropTypes.bool,
}; };

View File

@ -85,3 +85,6 @@
} }
} }
a.signature-request-content__verify-contract-details {
padding: 0;
}

View File

@ -5,9 +5,13 @@ import LedgerInstructionField from '../ledger-instruction-field';
import { sanitizeMessage } from '../../../helpers/utils/util'; import { sanitizeMessage } from '../../../helpers/utils/util';
import { EVENT } from '../../../../shared/constants/metametrics'; import { EVENT } from '../../../../shared/constants/metametrics';
import SiteOrigin from '../../ui/site-origin'; import SiteOrigin from '../../ui/site-origin';
import Header from './signature-request-header'; import Button from '../../ui/button';
import Footer from './signature-request-footer'; import Typography from '../../ui/typography/typography';
import { COLORS, TYPOGRAPHY } from '../../../helpers/constants/design-system';
import ContractDetailsModal from '../modals/contract-details-modal/contract-details-modal';
import Message from './signature-request-message'; import Message from './signature-request-message';
import Footer from './signature-request-footer';
import Header from './signature-request-header';
export default class SignatureRequest extends PureComponent { export default class SignatureRequest extends PureComponent {
static propTypes = { static propTypes = {
@ -39,6 +43,18 @@ export default class SignatureRequest extends PureComponent {
* Whether the hardware wallet requires a connection disables the sign button if true. * Whether the hardware wallet requires a connection disables the sign button if true.
*/ */
hardwareWalletRequiresConnection: PropTypes.bool.isRequired, hardwareWalletRequiresConnection: PropTypes.bool.isRequired,
/**
* Current network chainId
*/
chainId: PropTypes.string,
/**
* RPC prefs of the current network
*/
rpcPrefs: PropTypes.object,
/**
* Dapp image
*/
siteImage: PropTypes.string,
}; };
static contextTypes = { static contextTypes = {
@ -48,6 +64,7 @@ export default class SignatureRequest extends PureComponent {
state = { state = {
hasScrolledMessage: false, hasScrolledMessage: false,
showContractDetails: false,
}; };
setMessageRootRef(ref) { setMessageRootRef(ref) {
@ -72,6 +89,9 @@ export default class SignatureRequest extends PureComponent {
sign, sign,
isLedgerWallet, isLedgerWallet,
hardwareWalletRequiresConnection, hardwareWalletRequiresConnection,
chainId,
rpcPrefs,
siteImage,
} = this.props; } = this.props;
const { address: fromAddress } = fromAccount; const { address: fromAddress } = fromAccount;
const { message, domain = {}, primaryType, types } = JSON.parse(data); const { message, domain = {}, primaryType, types } = JSON.parse(data);
@ -129,8 +149,19 @@ export default class SignatureRequest extends PureComponent {
className="signature-request-content__info" className="signature-request-content__info"
siteOrigin={origin} siteOrigin={origin}
/> />
<div className="signature-request-content__info"> <div>
{this.formatWallet(fromAddress)} <Button
type="link"
onClick={() => this.setState({ showContractDetails: true })}
className="signature-request-content__verify-contract-details"
>
<Typography
variant={TYPOGRAPHY.H7}
color={COLORS.PRIMARY_DEFAULT}
>
{this.context.t('verifyContractDetails')}
</Typography>
</Button>
</div> </div>
</div> </div>
{isLedgerWallet ? ( {isLedgerWallet ? (
@ -153,6 +184,17 @@ export default class SignatureRequest extends PureComponent {
(messageIsScrollable && !this.state.hasScrolledMessage) (messageIsScrollable && !this.state.hasScrolledMessage)
} }
/> />
{this.state.showContractDetails && (
<ContractDetailsModal
toAddress={domain.verifyingContract}
chainId={chainId}
rpcPrefs={rpcPrefs}
origin={origin}
siteImage={siteImage}
onClose={() => this.setState({ showContractDetails: false })}
isContractRequestingSignature
/>
)}
</div> </div>
); );
} }

View File

@ -2,6 +2,9 @@ import { connect } from 'react-redux';
import { import {
accountsWithSendEtherInfoSelector, accountsWithSendEtherInfoSelector,
doesAddressRequireLedgerHidConnection, doesAddressRequireLedgerHidConnection,
getCurrentChainId,
getRpcPrefsForCurrentProvider,
getSubjectMetadata,
} from '../../../selectors'; } from '../../../selectors';
import { isAddressLedger } from '../../../ducks/metamask/metamask'; import { isAddressLedger } from '../../../ducks/metamask/metamask';
import { getAccountByAddress } from '../../../helpers/utils/util'; import { getAccountByAddress } from '../../../helpers/utils/util';
@ -16,18 +19,33 @@ function mapStateToProps(state, ownProps) {
const hardwareWalletRequiresConnection = const hardwareWalletRequiresConnection =
doesAddressRequireLedgerHidConnection(state, from); doesAddressRequireLedgerHidConnection(state, from);
const isLedgerWallet = isAddressLedger(state, from); const isLedgerWallet = isAddressLedger(state, from);
const chainId = getCurrentChainId(state);
const rpcPrefs = getRpcPrefsForCurrentProvider(state);
const subjectMetadata = getSubjectMetadata(state);
const { iconUrl: siteImage = '' } =
subjectMetadata[txData.msgParams.origin] || {};
return { return {
isLedgerWallet, isLedgerWallet,
hardwareWalletRequiresConnection, hardwareWalletRequiresConnection,
chainId,
rpcPrefs,
siteImage,
// not forwarded to component // not forwarded to component
allAccounts: accountsWithSendEtherInfoSelector(state), allAccounts: accountsWithSendEtherInfoSelector(state),
}; };
} }
function mergeProps(stateProps, dispatchProps, ownProps) { function mergeProps(stateProps, dispatchProps, ownProps) {
const { allAccounts, isLedgerWallet, hardwareWalletRequiresConnection } = const {
stateProps; allAccounts,
isLedgerWallet,
hardwareWalletRequiresConnection,
chainId,
rpcPrefs,
siteImage,
} = stateProps;
const { const {
signPersonalMessage, signPersonalMessage,
signTypedMessage, signTypedMessage,
@ -68,6 +86,9 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
sign, sign,
isLedgerWallet, isLedgerWallet,
hardwareWalletRequiresConnection, hardwareWalletRequiresConnection,
chainId,
rpcPrefs,
siteImage,
}; };
} }