diff --git a/app/scripts/background.js b/app/scripts/background.js index d668f6804..7828c6d80 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -10,7 +10,6 @@ require('./lib/setupFetchDebugging')() // polyfills import 'abortcontroller-polyfill/dist/polyfill-patch-fetch' -const urlUtil = require('url') const endOfStream = require('end-of-stream') const pump = require('pump') const debounce = require('debounce-stream') @@ -352,7 +351,10 @@ function setupController (initState, initLangCode) { const portStream = new PortStream(remotePort) // communication with popup controller.isClientOpen = true - controller.setupTrustedCommunication(portStream, 'MetaMask') + // construct fake URL for identifying internal messages + const metamaskUrl = new URL(window.location) + metamaskUrl.hostname = 'metamask' + controller.setupTrustedCommunication(portStream, metamaskUrl) if (processName === ENVIRONMENT_TYPE_POPUP) { popupIsOpen = true @@ -388,9 +390,10 @@ function setupController (initState, initLangCode) { // communication with page or other extension function connectExternal (remotePort) { - const originDomain = urlUtil.parse(remotePort.sender.url).hostname + const senderUrl = new URL(remotePort.sender.url) + const extensionId = remotePort.sender.id const portStream = new PortStream(remotePort) - controller.setupUntrustedCommunication(portStream, originDomain) + controller.setupUntrustedCommunication(portStream, senderUrl, extensionId) } // diff --git a/app/scripts/controllers/provider-approval.js b/app/scripts/controllers/provider-approval.js index 6b5007051..3ece07e13 100644 --- a/app/scripts/controllers/provider-approval.js +++ b/app/scripts/controllers/provider-approval.js @@ -31,19 +31,25 @@ class ProviderApprovalController extends SafeEventEmitter { * * @param {object} opts - opts for the middleware contains the origin for the middleware */ - createMiddleware ({ origin, getSiteMetadata }) { + createMiddleware ({ senderUrl, extensionId, getSiteMetadata }) { return createAsyncMiddleware(async (req, res, next) => { // only handle requestAccounts if (req.method !== 'eth_requestAccounts') return next() // if already approved or privacy mode disabled, return early const isUnlocked = this.keyringController.memStore.getState().isUnlocked + const origin = senderUrl.hostname if (this.shouldExposeAccounts(origin) && isUnlocked) { res.result = [this.preferencesController.getSelectedAddress()] return } // register the provider request - const metadata = await getSiteMetadata(origin) - this._handleProviderRequest(origin, metadata.name, metadata.icon) + const metadata = { hostname: senderUrl.hostname, origin } + if (extensionId) { + metadata.extensionId = extensionId + } else { + Object.assign(metadata, await getSiteMetadata(origin)) + } + this._handleProviderRequest(metadata) // wait for resolution of request const approved = await new Promise(resolve => this.once(`resolvedRequest:${origin}`, ({ approved }) => resolve(approved))) if (approved) { @@ -54,19 +60,26 @@ class ProviderApprovalController extends SafeEventEmitter { }) } + /** + * @typedef {Object} SiteMetadata + * @param {string} hostname - The hostname of the site + * @param {string} origin - The origin of the site + * @param {string} [siteTitle] - The title of the site + * @param {string} [siteImage] - The icon for the site + * @param {string} [extensionId] - The extension ID of the extension + */ /** * Called when a tab requests access to a full Ethereum provider API * - * @param {string} origin - Origin of the window requesting full provider access - * @param {string} siteTitle - The title of the document requesting full provider access - * @param {string} siteImage - The icon of the window requesting full provider access + * @param {SiteMetadata} siteMetadata - The metadata for the site requesting full provider access */ - _handleProviderRequest (origin, siteTitle, siteImage) { + _handleProviderRequest (siteMetadata) { const { providerRequests } = this.memStore.getState() + const origin = siteMetadata.origin this.memStore.updateState({ providerRequests: [ ...providerRequests, - { origin, siteTitle, siteImage }, + siteMetadata, ], }) const isUnlocked = this.keyringController.memStore.getState().isUnlocked @@ -98,6 +111,7 @@ class ProviderApprovalController extends SafeEventEmitter { [origin]: { siteTitle: providerRequest ? providerRequest.siteTitle : null, siteImage: providerRequest ? providerRequest.siteImage : null, + hostname: providerRequest ? providerRequest.hostname : null, }, }, }) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 22ae6f0f5..3d8ad2fec 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -334,7 +334,7 @@ module.exports = class MetamaskController extends EventEmitter { // Expose no accounts if this origin has not been approved, preventing // account-requring RPC methods from completing successfully const exposeAccounts = this.providerApprovalController.shouldExposeAccounts(origin) - if (origin !== 'MetaMask' && !exposeAccounts) { return [] } + if (origin !== 'metamask' && !exposeAccounts) { return [] } const isUnlocked = this.keyringController.memStore.getState().isUnlocked const selectedAddress = this.preferencesController.getSelectedAddress() // only show address if account is unlocked @@ -1318,23 +1318,25 @@ module.exports = class MetamaskController extends EventEmitter { * Used to create a multiplexed stream for connecting to an untrusted context * like a Dapp or other extension. * @param {*} connectionStream - The Duplex stream to connect to. - * @param {string} originDomain - The domain requesting the stream, which - * may trigger a blacklist reload. + * @param {URL} senderUrl - The URL of the resource requesting the stream, + * which may trigger a blacklist reload. + * @param {string} extensionId - The extension id of the sender, if the sender + * is an extension */ - setupUntrustedCommunication (connectionStream, originDomain) { + setupUntrustedCommunication (connectionStream, senderUrl, extensionId) { // Check if new connection is blacklisted - if (this.phishingController.test(originDomain)) { - log.debug('MetaMask - sending phishing warning for', originDomain) - this.sendPhishingWarning(connectionStream, originDomain) + if (this.phishingController.test(senderUrl.hostname)) { + log.debug('MetaMask - sending phishing warning for', senderUrl.hostname) + this.sendPhishingWarning(connectionStream, senderUrl.hostname) return } // setup multiplexing const mux = setupMultiplex(connectionStream) // connect features - const publicApi = this.setupPublicApi(mux.createStream('publicApi'), originDomain) - this.setupProviderConnection(mux.createStream('provider'), originDomain, publicApi) - this.setupPublicConfig(mux.createStream('publicConfig'), originDomain) + const publicApi = this.setupPublicApi(mux.createStream('publicApi')) + this.setupProviderConnection(mux.createStream('provider'), senderUrl, extensionId, publicApi) + this.setupPublicConfig(mux.createStream('publicConfig'), senderUrl) } /** @@ -1344,15 +1346,15 @@ module.exports = class MetamaskController extends EventEmitter { * functions, like the ability to approve transactions or sign messages. * * @param {*} connectionStream - The duplex stream to connect to. - * @param {string} originDomain - The domain requesting the connection, + * @param {URL} senderUrl - The URL requesting the connection, * used in logging and error reporting. */ - setupTrustedCommunication (connectionStream, originDomain) { + setupTrustedCommunication (connectionStream, senderUrl) { // setup multiplexing const mux = setupMultiplex(connectionStream) // connect features this.setupControllerConnection(mux.createStream('controller')) - this.setupProviderConnection(mux.createStream('provider'), originDomain) + this.setupProviderConnection(mux.createStream('provider'), senderUrl) } /** @@ -1405,11 +1407,14 @@ module.exports = class MetamaskController extends EventEmitter { /** * A method for serving our ethereum provider over a given stream. * @param {*} outStream - The stream to provide over. - * @param {string} origin - The URI of the requesting resource. + * @param {URL} senderUrl - The URI of the requesting resource. + * @param {string} extensionId - The id of the extension, if the requesting + * resource is an extension. + * @param {object} publicApi - The public API */ - setupProviderConnection (outStream, origin, publicApi) { + setupProviderConnection (outStream, senderUrl, extensionId, publicApi) { const getSiteMetadata = publicApi && publicApi.getSiteMetadata - const engine = this.setupProviderEngine(origin, getSiteMetadata) + const engine = this.setupProviderEngine(senderUrl, extensionId, getSiteMetadata) // setup connection const providerStream = createEngineStream({ engine }) @@ -1433,7 +1438,8 @@ module.exports = class MetamaskController extends EventEmitter { /** * A method for creating a provider that is safely restricted for the requesting domain. **/ - setupProviderEngine (origin, getSiteMetadata) { + setupProviderEngine (senderUrl, extensionId, getSiteMetadata) { + const origin = senderUrl.hostname // setup json rpc engine stack const engine = new RpcEngine() const provider = this.provider @@ -1456,7 +1462,8 @@ module.exports = class MetamaskController extends EventEmitter { engine.push(this.preferencesController.requestWatchAsset.bind(this.preferencesController)) // requestAccounts engine.push(this.providerApprovalController.createMiddleware({ - origin, + senderUrl, + extensionId, getSiteMetadata, })) // forward to metamask primary provider @@ -1473,11 +1480,12 @@ module.exports = class MetamaskController extends EventEmitter { * this is a good candidate for deprecation. * * @param {*} outStream - The stream to provide public config over. + * @param {URL} senderUrl - The URL of requesting resource */ - setupPublicConfig (outStream, originDomain) { + setupPublicConfig (outStream, senderUrl) { const configStore = this.createPublicConfigStore({ // check the providerApprovalController's approvedOrigins - checkIsEnabled: () => this.providerApprovalController.shouldExposeAccounts(originDomain), + checkIsEnabled: () => this.providerApprovalController.shouldExposeAccounts(senderUrl.hostname), }) const configStream = asStream(configStore) diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 5e019b533..7eac91a9b 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -797,7 +797,7 @@ describe('MetaMaskController', function () { describe('#setupUntrustedCommunication', function () { let streamTest - const phishingUrl = 'myethereumwalletntw.com' + const phishingUrl = new URL('http://myethereumwalletntw.com') afterEach(function () { streamTest.end() @@ -810,7 +810,7 @@ describe('MetaMaskController', function () { streamTest = createThoughStream((chunk, _, cb) => { if (chunk.name !== 'phishing') return cb() - assert.equal(chunk.data.hostname, phishingUrl) + assert.equal(chunk.data.hostname, phishingUrl.hostname) resolve() cb() }) diff --git a/test/unit/app/controllers/provider-approval-test.js b/test/unit/app/controllers/provider-approval-test.js index c12387a33..eeb9e813b 100644 --- a/test/unit/app/controllers/provider-approval-test.js +++ b/test/unit/app/controllers/provider-approval-test.js @@ -25,14 +25,17 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + + controller._handleProviderRequest(metadata) assert.deepEqual(controller._getMergedState(), { approvedOrigins: {}, - providerRequests: [{ - origin: 'example.com', - siteTitle: 'Example', - siteImage: 'https://example.com/logo.svg', - }], + providerRequests: [metadata], }) }) @@ -41,14 +44,16 @@ describe('ProviderApprovalController', () => { keyringController: mockLockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) assert.deepEqual(controller._getMergedState(), { approvedOrigins: {}, - providerRequests: [{ - origin: 'example.com', - siteTitle: 'Example', - siteImage: 'https://example.com/logo.svg', - }], + providerRequests: [metadata], }) }) @@ -57,19 +62,23 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example1.com', 'Example 1', 'https://example1.com/logo.svg') - controller._handleProviderRequest('example2.com', 'Example 2', 'https://example2.com/logo.svg') + const metadata = [{ + hostname: 'https://example1.com', + origin: 'example1.com', + siteTitle: 'Example 1', + siteImage: 'https://example1.com/logo.svg', + }, { + hostname: 'https://example2.com', + origin: 'example2.com', + siteTitle: 'Example 2', + siteImage: 'https://example2.com/logo.svg', + }] + + controller._handleProviderRequest(metadata[0]) + controller._handleProviderRequest(metadata[1]) assert.deepEqual(controller._getMergedState(), { approvedOrigins: {}, - providerRequests: [{ - origin: 'example1.com', - siteTitle: 'Example 1', - siteImage: 'https://example1.com/logo.svg', - }, { - origin: 'example2.com', - siteTitle: 'Example 2', - siteImage: 'https://example2.com/logo.svg', - }], + providerRequests: metadata, }) }) @@ -78,19 +87,23 @@ describe('ProviderApprovalController', () => { keyringController: mockLockedKeyringController, }) - controller._handleProviderRequest('example1.com', 'Example 1', 'https://example1.com/logo.svg') - controller._handleProviderRequest('example2.com', 'Example 2', 'https://example2.com/logo.svg') + const metadata = [{ + hostname: 'https://example1.com', + origin: 'example1.com', + siteTitle: 'Example 1', + siteImage: 'https://example1.com/logo.svg', + }, { + hostname: 'https://example2.com', + origin: 'example2.com', + siteTitle: 'Example 2', + siteImage: 'https://example2.com/logo.svg', + }] + + controller._handleProviderRequest(metadata[0]) + controller._handleProviderRequest(metadata[1]) assert.deepEqual(controller._getMergedState(), { approvedOrigins: {}, - providerRequests: [{ - origin: 'example1.com', - siteTitle: 'Example 1', - siteImage: 'https://example1.com/logo.svg', - }, { - origin: 'example2.com', - siteTitle: 'Example 2', - siteImage: 'https://example2.com/logo.svg', - }], + providerRequests: metadata, }) }) @@ -101,7 +114,13 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) assert.ok(openPopup.calledOnce) }) @@ -112,7 +131,13 @@ describe('ProviderApprovalController', () => { keyringController: mockLockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) assert.ok(openPopup.calledOnce) }) @@ -131,7 +156,13 @@ describe('ProviderApprovalController', () => { }, }, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) assert.ok(openPopup.notCalled) }) }) @@ -142,12 +173,19 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') assert.deepEqual(controller._getMergedState(), { providerRequests: [], approvedOrigins: { 'example.com': { + hostname: 'https://example.com', siteTitle: 'Example', siteImage: 'https://example.com/logo.svg', }, @@ -160,13 +198,20 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') assert.deepEqual(controller._getMergedState(), { providerRequests: [], approvedOrigins: { 'example.com': { + hostname: 'https://example.com', siteTitle: 'Example', siteImage: 'https://example.com/logo.svg', }, @@ -184,6 +229,7 @@ describe('ProviderApprovalController', () => { providerRequests: [], approvedOrigins: { 'example.com': { + hostname: null, siteTitle: null, siteImage: null, }, @@ -198,7 +244,13 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') controller.rejectProviderRequestByOrigin('example.com') assert.deepEqual(controller._getMergedState(), { @@ -226,7 +278,13 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') controller.clearApprovedOrigins() assert.deepEqual(controller._getMergedState(), { @@ -242,7 +300,13 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') assert.ok(controller.shouldExposeAccounts('example.com')) }) @@ -252,7 +316,13 @@ describe('ProviderApprovalController', () => { keyringController: mockUnlockedKeyringController, }) - controller._handleProviderRequest('example.com', 'Example', 'https://example.com/logo.svg') + const metadata = { + hostname: 'https://example.com', + origin: 'example.com', + siteTitle: 'Example', + siteImage: 'https://example.com/logo.svg', + } + controller._handleProviderRequest(metadata) controller.approveProviderRequestByOrigin('example.com') assert.ok(!controller.shouldExposeAccounts('bad.website')) }) diff --git a/ui/app/components/app/provider-page-container/provider-page-container-content/provider-page-container-content.component.js b/ui/app/components/app/provider-page-container/provider-page-container-content/provider-page-container-content.component.js index 7eda7f2b7..4062b130f 100644 --- a/ui/app/components/app/provider-page-container/provider-page-container-content/provider-page-container-content.component.js +++ b/ui/app/components/app/provider-page-container/provider-page-container-content/provider-page-container-content.component.js @@ -7,15 +7,17 @@ export default class ProviderPageContainerContent extends PureComponent { origin: PropTypes.string.isRequired, selectedIdentity: PropTypes.object.isRequired, siteImage: PropTypes.string, - siteTitle: PropTypes.string.isRequired, + siteTitle: PropTypes.string, + hostname: PropTypes.string, + extensionId: PropTypes.string, } static contextTypes = { t: PropTypes.func, }; - renderConnectVisual = () => { - const { origin, selectedIdentity, siteImage, siteTitle } = this.props + renderConnectVisual = (title, identifier) => { + const { selectedIdentity, siteImage } = this.props return (
@@ -27,11 +29,11 @@ export default class ProviderPageContainerContent extends PureComponent { /> ) : ( - {siteTitle.charAt(0).toUpperCase()} + {title.charAt(0).toUpperCase()} )} -

{siteTitle}

-

{origin}

+

{title}

+

{identifier}

@@ -47,15 +49,23 @@ export default class ProviderPageContainerContent extends PureComponent { } render () { - const { siteTitle } = this.props + const { siteTitle, hostname, extensionId } = this.props const { t } = this.context + const title = extensionId ? + 'External Extension' : + siteTitle || hostname + + const identifier = extensionId ? + `Extension ID: '${extensionId}'` : + hostname + return (

{t('connectRequest')}

- {this.renderConnectVisual()} -

{t('providerRequest', [siteTitle])}

+ {this.renderConnectVisual(title, identifier)} +

{t('providerRequest', [title])}

{t('providerRequestInfo')}
diff --git a/ui/app/components/app/provider-page-container/provider-page-container.component.js b/ui/app/components/app/provider-page-container/provider-page-container.component.js index 1c655d404..b89135806 100644 --- a/ui/app/components/app/provider-page-container/provider-page-container.component.js +++ b/ui/app/components/app/provider-page-container/provider-page-container.component.js @@ -9,7 +9,9 @@ export default class ProviderPageContainer extends PureComponent { rejectProviderRequestByOrigin: PropTypes.func.isRequired, origin: PropTypes.string.isRequired, siteImage: PropTypes.string, - siteTitle: PropTypes.string.isRequired, + siteTitle: PropTypes.string, + hostname: PropTypes.string, + extensionId: PropTypes.string, }; static contextTypes = { @@ -52,7 +54,7 @@ export default class ProviderPageContainer extends PureComponent { } render () { - const {origin, siteImage, siteTitle} = this.props + const {origin, siteImage, siteTitle, hostname, extensionId} = this.props return (

@@ -61,6 +63,8 @@ export default class ProviderPageContainer extends PureComponent { origin={origin} siteImage={siteImage} siteTitle={siteTitle} + hostname={hostname} + extensionId={extensionId} /> this.onCancel()} diff --git a/ui/app/helpers/utils/metametrics.util.js b/ui/app/helpers/utils/metametrics.util.js index 50270c6a8..560d8bd9e 100644 --- a/ui/app/helpers/utils/metametrics.util.js +++ b/ui/app/helpers/utils/metametrics.util.js @@ -67,7 +67,7 @@ const customDimensionsNameIdMap = { } function composeUrlRefParamAddition (previousPath, confirmTransactionOrigin) { - const externalOrigin = confirmTransactionOrigin && confirmTransactionOrigin !== 'MetaMask' + const externalOrigin = confirmTransactionOrigin && confirmTransactionOrigin !== 'metamask' return `&urlref=${externalOrigin ? 'EXTERNAL' : encodeURIComponent(previousPath.replace(/chrome-extension:\/\/\w+/, METAMETRICS_TRACKING_URL))}` } diff --git a/ui/app/pages/provider-approval/provider-approval.component.js b/ui/app/pages/provider-approval/provider-approval.component.js index 70d3d0007..da177defc 100644 --- a/ui/app/pages/provider-approval/provider-approval.component.js +++ b/ui/app/pages/provider-approval/provider-approval.component.js @@ -23,6 +23,8 @@ export default class ProviderApproval extends Component { tabID={providerRequest.tabID} siteImage={providerRequest.siteImage} siteTitle={providerRequest.siteTitle} + hostname={providerRequest.hostname} + extensionId={providerRequest.extensionId} /> ) }