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

Reject popup confirmations on close (#12643)

* Background clears confirmations on popup close

* [WIP] Remove clearing confirmations through UI

* Confirmations are now rejected instead of cleared

* Erased commented out code

* Fix linter errors

* Changes after code review

* Moved metrics events from onWindowUnload to background

* PR review fixes

* Added abillity to add reason to rejection of messages

* Fix prettier

* Added type metrics event to signature cancel

* Fix test

* The uncofirmed transactions are now cleared even if Metamask is locked
This commit is contained in:
Olaf Tomalka 2021-11-15 17:13:51 +01:00 committed by GitHub
parent 7f569f2255
commit a323a5fe59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 192 additions and 193 deletions

View File

@ -17,13 +17,19 @@ import {
ENVIRONMENT_TYPE_FULLSCREEN,
} from '../../shared/constants/app';
import { SECOND } from '../../shared/constants/time';
import {
REJECT_NOTFICIATION_CLOSE,
REJECT_NOTFICIATION_CLOSE_SIG,
} from '../../shared/constants/metametrics';
import migrations from './migrations';
import Migrator from './lib/migrator';
import ExtensionPlatform from './platforms/extension';
import LocalStore from './lib/local-store';
import ReadOnlyNetworkStore from './lib/network-store';
import createStreamSink from './lib/createStreamSink';
import NotificationManager from './lib/notification-manager';
import NotificationManager, {
NOTIFICATION_MANAGER_EVENTS,
} from './lib/notification-manager';
import MetamaskController, {
METAMASK_CONTROLLER_EVENTS,
} from './metamask-controller';
@ -475,6 +481,69 @@ function setupController(initState, initLangCode) {
extension.browserAction.setBadgeBackgroundColor({ color: '#037DD6' });
}
notificationManager.on(
NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED,
rejectUnapprovedNotifications,
);
function rejectUnapprovedNotifications() {
Object.keys(
controller.txController.txStateManager.getUnapprovedTxList(),
).forEach((txId) =>
controller.txController.txStateManager.setTxStatusRejected(txId),
);
controller.messageManager.messages
.filter((msg) => msg.status === 'unapproved')
.forEach((tx) =>
controller.messageManager.rejectMsg(
tx.id,
REJECT_NOTFICIATION_CLOSE_SIG,
),
);
controller.personalMessageManager.messages
.filter((msg) => msg.status === 'unapproved')
.forEach((tx) =>
controller.personalMessageManager.rejectMsg(
tx.id,
REJECT_NOTFICIATION_CLOSE_SIG,
),
);
controller.typedMessageManager.messages
.filter((msg) => msg.status === 'unapproved')
.forEach((tx) =>
controller.typedMessageManager.rejectMsg(
tx.id,
REJECT_NOTFICIATION_CLOSE_SIG,
),
);
controller.decryptMessageManager.messages
.filter((msg) => msg.status === 'unapproved')
.forEach((tx) =>
controller.decryptMessageManager.rejectMsg(
tx.id,
REJECT_NOTFICIATION_CLOSE,
),
);
controller.encryptionPublicKeyManager.messages
.filter((msg) => msg.status === 'unapproved')
.forEach((tx) =>
controller.encryptionPublicKeyManager.rejectMsg(
tx.id,
REJECT_NOTFICIATION_CLOSE,
),
);
// We're specifcally avoid using approvalController directly for better
// Error support during rejection
Object.keys(
controller.permissionsController.approvals.state.pendingApprovals,
).forEach((approvalId) =>
controller.permissionsController.rejectPermissionsRequest(approvalId),
);
updateBadge();
}
return Promise.resolve();
}

View File

@ -38,13 +38,14 @@ export default class DecryptMessageManager extends EventEmitter {
* @property {Array} messages Holds all messages that have been created by this DecryptMessageManager
*
*/
constructor() {
constructor(opts) {
super();
this.memStore = new ObservableStore({
unapprovedDecryptMsgs: {},
unapprovedDecryptMsgCount: 0,
});
this.messages = [];
this.metricsEvent = opts.metricsEvent;
}
/**
@ -237,7 +238,16 @@ export default class DecryptMessageManager extends EventEmitter {
* @param {number} msgId The id of the DecryptMessage to reject.
*
*/
rejectMsg(msgId) {
rejectMsg(msgId, reason = undefined) {
if (reason) {
this.metricsEvent({
event: reason,
category: 'Messages',
properties: {
action: 'Decrypt Message Request',
},
});
}
this._setMsgStatus(msgId, 'rejected');
}

View File

@ -34,13 +34,14 @@ export default class EncryptionPublicKeyManager extends EventEmitter {
* @property {Array} messages Holds all messages that have been created by this EncryptionPublicKeyManager
*
*/
constructor() {
constructor(opts) {
super();
this.memStore = new ObservableStore({
unapprovedEncryptionPublicKeyMsgs: {},
unapprovedEncryptionPublicKeyMsgCount: 0,
});
this.messages = [];
this.metricsEvent = opts.metricsEvent;
}
/**
@ -226,7 +227,16 @@ export default class EncryptionPublicKeyManager extends EventEmitter {
* @param {number} msgId The id of the EncryptionPublicKey to reject.
*
*/
rejectMsg(msgId) {
rejectMsg(msgId, reason = undefined) {
if (reason) {
this.metricsEvent({
event: reason,
category: 'Messages',
properties: {
action: 'Encryption public key Request',
},
});
}
this._setMsgStatus(msgId, 'rejected');
}

View File

@ -35,13 +35,14 @@ export default class MessageManager extends EventEmitter {
* @property {Array} messages Holds all messages that have been created by this MessageManager
*
*/
constructor() {
constructor({ metricsEvent }) {
super();
this.memStore = new ObservableStore({
unapprovedMsgs: {},
unapprovedMsgCount: 0,
});
this.messages = [];
this.metricsEvent = metricsEvent;
}
/**
@ -217,7 +218,18 @@ export default class MessageManager extends EventEmitter {
* @param {number} msgId - The id of the Message to reject.
*
*/
rejectMsg(msgId) {
rejectMsg(msgId, reason = undefined) {
if (reason) {
const msg = this.getMsg(msgId);
this.metricsEvent({
event: reason,
category: 'Transactions',
properties: {
action: 'Sign Request',
type: msg.type,
},
});
}
this._setMsgStatus(msgId, 'rejected');
}

View File

@ -1,4 +1,5 @@
import { strict as assert } from 'assert';
import sinon from 'sinon';
import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
import MessageManager from './message-manager';
@ -6,7 +7,9 @@ describe('Message Manager', function () {
let messageManager;
beforeEach(function () {
messageManager = new MessageManager();
messageManager = new MessageManager({
metricsEvent: sinon.fake(),
});
});
describe('#getMsgList', function () {

View File

@ -1,9 +1,14 @@
import EventEmitter from 'safe-event-emitter';
import ExtensionPlatform from '../platforms/extension';
const NOTIFICATION_HEIGHT = 620;
const NOTIFICATION_WIDTH = 360;
export default class NotificationManager {
export const NOTIFICATION_MANAGER_EVENTS = {
POPUP_CLOSED: 'onPopupClosed',
};
export default class NotificationManager extends EventEmitter {
/**
* A collection of methods for controlling the showing and hiding of the notification popup.
*
@ -12,7 +17,9 @@ export default class NotificationManager {
*/
constructor() {
super();
this.platform = new ExtensionPlatform();
this.platform.addOnRemovedListener(this._onWindowClosed.bind(this));
}
/**
@ -62,6 +69,13 @@ export default class NotificationManager {
}
}
_onWindowClosed(windowId) {
if (windowId === this._popupId) {
this._popupId = undefined;
this.emit(NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED);
}
}
/**
* Checks all open MetaMask windows, and returns the first one it finds that is a notification window (i.e. has the
* type 'popup')

View File

@ -40,13 +40,14 @@ export default class PersonalMessageManager extends EventEmitter {
* @property {Array} messages Holds all messages that have been created by this PersonalMessageManager
*
*/
constructor() {
constructor({ metricsEvent }) {
super();
this.memStore = new ObservableStore({
unapprovedPersonalMsgs: {},
unapprovedPersonalMsgCount: 0,
});
this.messages = [];
this.metricsEvent = metricsEvent;
}
/**
@ -238,7 +239,18 @@ export default class PersonalMessageManager extends EventEmitter {
* @param {number} msgId - The id of the PersonalMessage to reject.
*
*/
rejectMsg(msgId) {
rejectMsg(msgId, reason = undefined) {
if (reason) {
const msg = this.getMsg(msgId);
this.metricsEvent({
event: reason,
category: 'Transactions',
properties: {
action: 'Sign Request',
type: msg.type,
},
});
}
this._setMsgStatus(msgId, 'rejected');
}

View File

@ -1,4 +1,5 @@
import { strict as assert } from 'assert';
import sinon from 'sinon';
import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
import PersonalMessageManager from './personal-message-manager';
@ -6,7 +7,7 @@ describe('Personal Message Manager', function () {
let messageManager;
beforeEach(function () {
messageManager = new PersonalMessageManager();
messageManager = new PersonalMessageManager({ metricsEvent: sinon.fake() });
});
describe('#getMsgList', function () {

View File

@ -32,7 +32,7 @@ export default class TypedMessageManager extends EventEmitter {
/**
* Controller in charge of managing - storing, adding, removing, updating - TypedMessage.
*/
constructor({ getCurrentChainId }) {
constructor({ getCurrentChainId, metricEvents }) {
super();
this._getCurrentChainId = getCurrentChainId;
this.memStore = new ObservableStore({
@ -40,6 +40,7 @@ export default class TypedMessageManager extends EventEmitter {
unapprovedTypedMessagesCount: 0,
});
this.messages = [];
this.metricEvents = metricEvents;
}
/**
@ -301,7 +302,19 @@ export default class TypedMessageManager extends EventEmitter {
* @param {number} msgId - The id of the TypedMessage to reject.
*
*/
rejectMsg(msgId) {
rejectMsg(msgId, reason = undefined) {
if (reason) {
const msg = this.getMsg(msgId);
this.metricsEvent({
event: reason,
category: 'Transactions',
properties: {
action: 'Sign Request',
version: msg.msgParams.version,
type: msg.type,
},
});
}
this._setMsgStatus(msgId, 'rejected');
}

View File

@ -17,6 +17,7 @@ describe('Typed Message Manager', function () {
beforeEach(async function () {
typedMessageManager = new TypedMessageManager({
getCurrentChainId: sinon.fake.returns('0x1'),
metricsEvent: sinon.fake(),
});
msgParamsV1 = {

View File

@ -526,14 +526,33 @@ export default class MetamaskController extends EventEmitter {
}
});
this.networkController.lookupNetwork();
this.messageManager = new MessageManager();
this.personalMessageManager = new PersonalMessageManager();
this.decryptMessageManager = new DecryptMessageManager();
this.encryptionPublicKeyManager = new EncryptionPublicKeyManager();
this.messageManager = new MessageManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
});
this.personalMessageManager = new PersonalMessageManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
});
this.decryptMessageManager = new DecryptMessageManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
});
this.encryptionPublicKeyManager = new EncryptionPublicKeyManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
});
this.typedMessageManager = new TypedMessageManager({
getCurrentChainId: this.networkController.getCurrentChainId.bind(
this.networkController,
),
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
});
this.swapsController = new SwapsController({

View File

@ -162,6 +162,10 @@ export default class ExtensionPlatform {
}
}
addOnRemovedListener(listener) {
extension.windows.onRemoved.addListener(listener);
}
getAllWindows() {
return new Promise((resolve, reject) => {
extension.windows.getAll((windows) => {

View File

@ -140,3 +140,7 @@ export const METAMETRICS_BACKGROUND_PAGE_OBJECT = {
* @property {() => void} identify - Identify an anonymous user. We do not
* currently use this method.
*/
export const REJECT_NOTFICIATION_CLOSE = 'Cancel Via Notification Close';
export const REJECT_NOTFICIATION_CLOSE_SIG =
'Cancel Sig Request Via Notification Close';

View File

@ -5,11 +5,7 @@ import classnames from 'classnames';
import { ObjectInspector } from 'react-inspector';
import LedgerInstructionField from '../ledger-instruction-field';
import {
ENVIRONMENT_TYPE_NOTIFICATION,
MESSAGE_TYPE,
} from '../../../../shared/constants/app';
import { getEnvironmentType } from '../../../../app/scripts/lib/util';
import { MESSAGE_TYPE } from '../../../../shared/constants/app';
import Identicon from '../../ui/identicon';
import AccountListItem from '../account-list-item';
import { conversionUtil } from '../../../../shared/modules/conversion.utils';
@ -45,36 +41,6 @@ export default class SignatureRequestOriginal extends Component {
fromAccount: this.props.fromAccount,
};
componentDidMount = () => {
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
window.addEventListener('beforeunload', this._beforeUnload);
}
};
componentWillUnmount = () => {
this._removeBeforeUnload();
};
_beforeUnload = (event) => {
const { clearConfirmTransaction, cancel } = this.props;
const { metricsEvent } = this.context;
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Sign Request',
name: 'Cancel Sig Request Via Notification Close',
},
});
clearConfirmTransaction();
cancel(event);
};
_removeBeforeUnload = () => {
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
window.removeEventListener('beforeunload', this._beforeUnload);
}
};
renderHeader = () => {
return (
<div className="request-signature__header">
@ -300,7 +266,6 @@ export default class SignatureRequestOriginal extends Component {
large
className="request-signature__footer__cancel-button"
onClick={async (event) => {
this._removeBeforeUnload();
await cancel(event);
metricsEvent({
eventOpts: {
@ -325,7 +290,6 @@ export default class SignatureRequestOriginal extends Component {
className="request-signature__footer__sign-button"
disabled={hardwareWalletRequiresConnection}
onClick={async (event) => {
this._removeBeforeUnload();
await sign(event);
metricsEvent({
eventOpts: {

View File

@ -1,12 +1,10 @@
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';
import { getEnvironmentType } from '../../../../app/scripts/lib/util';
import Identicon from '../../ui/identicon';
import LedgerInstructionField from '../ledger-instruction-field';
import Header from './signature-request-header';
import Footer from './signature-request-footer';
import Message from './signature-request-message';
import { ENVIRONMENT_TYPE_NOTIFICATION } from './signature-request.constants';
export default class SignatureRequest extends PureComponent {
static propTypes = {
@ -17,7 +15,6 @@ export default class SignatureRequest extends PureComponent {
name: PropTypes.string,
}).isRequired,
isLedgerWallet: PropTypes.bool,
clearConfirmTransaction: PropTypes.func.isRequired,
cancel: PropTypes.func.isRequired,
sign: PropTypes.func.isRequired,
hardwareWalletRequiresConnection: PropTypes.func.isRequired,
@ -28,33 +25,6 @@ export default class SignatureRequest extends PureComponent {
metricsEvent: PropTypes.func,
};
componentDidMount() {
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
window.addEventListener('beforeunload', this._beforeUnload);
}
}
_beforeUnload = (event) => {
const {
clearConfirmTransaction,
cancel,
txData: { type },
} = this.props;
const { metricsEvent } = this.context;
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Sign Request',
name: 'Cancel Sig Request Via Notification Close',
},
customVariables: {
type,
},
});
clearConfirmTransaction();
cancel(event);
};
formatWallet(wallet) {
return `${wallet.slice(0, 8)}...${wallet.slice(
wallet.length - 8,
@ -79,7 +49,6 @@ export default class SignatureRequest extends PureComponent {
const { metricsEvent } = this.context;
const onSign = (event) => {
window.removeEventListener('beforeunload', this._beforeUnload);
sign(event);
metricsEvent({
eventOpts: {
@ -95,7 +64,6 @@ export default class SignatureRequest extends PureComponent {
};
const onCancel = (event) => {
window.removeEventListener('beforeunload', this._beforeUnload);
cancel(event);
metricsEvent({
eventOpts: {

View File

@ -1,5 +1,4 @@
import { connect } from 'react-redux';
import { clearConfirmTransaction } from '../../../ducks/confirm-transaction/confirm-transaction.duck';
import {
accountsWithSendEtherInfoSelector,
doesAddressRequireLedgerHidConnection,
@ -28,12 +27,6 @@ function mapStateToProps(state, ownProps) {
};
}
function mapDispatchToProps(dispatch) {
return {
clearConfirmTransaction: () => dispatch(clearConfirmTransaction()),
};
}
function mergeProps(stateProps, dispatchProps, ownProps) {
const {
allAccounts,
@ -83,8 +76,4 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
};
}
export default connect(
mapStateToProps,
mapDispatchToProps,
mergeProps,
)(SignatureRequest);
export default connect(mapStateToProps, null, mergeProps)(SignatureRequest);

View File

@ -9,9 +9,7 @@ import Identicon from '../../components/ui/identicon';
import Tooltip from '../../components/ui/tooltip';
import Copy from '../../components/ui/icon/copy-icon.component';
import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
import { SECOND } from '../../../shared/constants/time';
import { getEnvironmentType } from '../../../app/scripts/lib/util';
import { conversionUtil } from '../../../shared/modules/conversion.utils';
export default class ConfirmDecryptMessage extends Component {
@ -44,44 +42,6 @@ export default class ConfirmDecryptMessage extends Component {
hasCopied: false,
};
componentDidMount = () => {
if (
getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION
) {
window.addEventListener('beforeunload', this._beforeUnload);
}
};
componentWillUnmount = () => {
this._removeBeforeUnload();
};
_beforeUnload = async (event) => {
const {
clearConfirmTransaction,
cancelDecryptMessage,
txData,
} = this.props;
const { metricsEvent } = this.context;
await cancelDecryptMessage(txData, event);
metricsEvent({
eventOpts: {
category: 'Messages',
action: 'Decrypt Message Request',
name: 'Cancel Via Notification Close',
},
});
clearConfirmTransaction();
};
_removeBeforeUnload = () => {
if (
getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION
) {
window.removeEventListener('beforeunload', this._beforeUnload);
}
};
copyMessage = () => {
copyToClipboard(this.state.rawMessage);
this.context.metricsEvent({

View File

@ -5,8 +5,6 @@ import AccountListItem from '../../components/app/account-list-item';
import Button from '../../components/ui/button';
import Identicon from '../../components/ui/identicon';
import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
import { getEnvironmentType } from '../../../app/scripts/lib/util';
import { conversionUtil } from '../../../shared/modules/conversion.utils';
export default class ConfirmEncryptionPublicKey extends Component {
@ -33,44 +31,6 @@ export default class ConfirmEncryptionPublicKey extends Component {
nativeCurrency: PropTypes.string.isRequired,
};
componentDidMount = () => {
if (
getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION
) {
window.addEventListener('beforeunload', this._beforeUnload);
}
};
componentWillUnmount = () => {
this._removeBeforeUnload();
};
_beforeUnload = async (event) => {
const {
clearConfirmTransaction,
cancelEncryptionPublicKey,
txData,
} = this.props;
const { metricsEvent } = this.context;
await cancelEncryptionPublicKey(txData, event);
metricsEvent({
eventOpts: {
category: 'Messages',
action: 'Encryption public key Request',
name: 'Cancel Via Notification Close',
},
});
clearConfirmTransaction();
};
_removeBeforeUnload = () => {
if (
getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION
) {
window.removeEventListener('beforeunload', this._beforeUnload);
}
};
renderHeader = () => {
return (
<div className="request-encryption-public-key__header">

View File

@ -1,7 +1,5 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../shared/constants/app';
import { getEnvironmentType } from '../../../app/scripts/lib/util';
import ConfirmPageContainer from '../../components/app/confirm-page-container';
import { isBalanceSufficient } from '../send/send.utils';
import {
@ -826,11 +824,6 @@ export default class ConfirmTransactionBase extends Component {
};
}
_beforeUnload = () => {
const { txData: { id } = {}, cancelTransaction } = this.props;
cancelTransaction({ id });
};
_beforeUnloadForGasPolling = () => {
this._isMounted = false;
if (this.state.pollingToken) {
@ -840,9 +833,6 @@ export default class ConfirmTransactionBase extends Component {
};
_removeBeforeUnload = () => {
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
window.removeEventListener('beforeunload', this._beforeUnload);
}
window.removeEventListener('beforeunload', this._beforeUnloadForGasPolling);
};
@ -866,10 +856,6 @@ export default class ConfirmTransactionBase extends Component {
},
});
if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
window.addEventListener('beforeunload', this._beforeUnload);
}
getNextNonce();
if (toAddress) {
tryReverseResolveAddress(toAddress);