Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject popup confirmations on close #12643

Merged
merged 13 commits into from
Nov 15, 2021
71 changes: 70 additions & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}

Expand Down
14 changes: 12 additions & 2 deletions app/scripts/lib/decrypt-message-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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');
}

Expand Down
14 changes: 12 additions & 2 deletions app/scripts/lib/encryption-public-key-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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');
}

Expand Down
16 changes: 14 additions & 2 deletions app/scripts/lib/message-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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');
}

Expand Down
5 changes: 4 additions & 1 deletion app/scripts/lib/message-manager.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { strict as assert } from 'assert';
import sinon from 'sinon';
import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
import MessageManager from './message-manager';

describe('Message Manager', function () {
let messageManager;

beforeEach(function () {
messageManager = new MessageManager();
messageManager = new MessageManager({
metricsEvent: sinon.fake(),
});
});

describe('#getMsgList', function () {
Expand Down
16 changes: 15 additions & 1 deletion app/scripts/lib/notification-manager.js
Original file line number Diff line number Diff line change
@@ -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.
*
Expand All @@ -12,7 +17,9 @@ export default class NotificationManager {
*/

constructor() {
super();
this.platform = new ExtensionPlatform();
this.platform.addOnRemovedListener(this._onWindowClosed.bind(this));
}

/**
Expand Down Expand Up @@ -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')
Expand Down
16 changes: 14 additions & 2 deletions app/scripts/lib/personal-message-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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');
}

Expand Down
3 changes: 2 additions & 1 deletion app/scripts/lib/personal-message-manager.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { strict as assert } from 'assert';
import sinon from 'sinon';
import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
import PersonalMessageManager from './personal-message-manager';

describe('Personal Message Manager', function () {
let messageManager;

beforeEach(function () {
messageManager = new PersonalMessageManager();
messageManager = new PersonalMessageManager({ metricsEvent: sinon.fake() });
});

describe('#getMsgList', function () {
Expand Down
17 changes: 15 additions & 2 deletions app/scripts/lib/typed-message-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ 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({
unapprovedTypedMessages: {},
unapprovedTypedMessagesCount: 0,
});
this.messages = [];
this.metricEvents = metricEvents;
}

/**
Expand Down Expand Up @@ -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');
}

Expand Down
1 change: 1 addition & 0 deletions app/scripts/lib/typed-message-manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('Typed Message Manager', function () {
beforeEach(async function () {
typedMessageManager = new TypedMessageManager({
getCurrentChainId: sinon.fake.returns('0x1'),
metricsEvent: sinon.fake(),
});

msgParamsV1 = {
Expand Down
27 changes: 23 additions & 4 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
4 changes: 4 additions & 0 deletions app/scripts/platforms/extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Loading