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.

Sign up for GitHub

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

Use URL origin instead of hostname for permission domains #8717

Merged
merged 5 commits into from
Jun 1, 2020
Merged

Use URL origin instead of hostname for permission domains #8717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a67433d
use URL.origin for tabs and permissions
rekmarks Jun 1, 2020
c9411c8
fix activeTab URL origin
rekmarks Jun 1, 2020
7ad6d99
fix e2e test failure
rekmarks Jun 1, 2020
74438ea
fix e2e test
rekmarks Jun 1, 2020
2a0265c
remove try/catch
rekmarks Jun 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/scripts/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ function setupController (initState, initLangCode) {
if (remotePort.sender && remotePort.sender.tab && remotePort.sender.url) {
const tabId = remotePort.sender.tab.id
const url = new URL(remotePort.sender.url)
const origin = url.hostname
const { origin } = url

remotePort.onMessage.addListener((msg) => {
if (msg.data && msg.data.method === 'eth_requestAccounts') {
Expand Down
6 changes: 3 additions & 3 deletions app/scripts/metamask-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1487,7 +1487,7 @@ export default class MetamaskController extends EventEmitter {
* @private
* @param {*} connectionStream - The duplex stream to the per-page script,
* for sending the reload attempt to.
* @param {string} hostname - The URL that triggered the suspicion.
* @param {string} hostname - The hostname that triggered the suspicion.
*/
sendPhishingWarning (connectionStream, hostname) {
const mux = setupMultiplex(connectionStream)
Expand Down Expand Up @@ -1538,7 +1538,7 @@ export default class MetamaskController extends EventEmitter {
setupProviderConnection (outStream, sender, isInternal) {
const origin = isInternal
? 'metamask'
: (new URL(sender.url)).hostname
: (new URL(sender.url)).origin
let extensionId
if (sender.id !== extension.runtime.id) {
extensionId = sender.id
Expand Down Expand Up @@ -1577,7 +1577,7 @@ export default class MetamaskController extends EventEmitter {
/**
* A method for creating a provider that is safely restricted for the requesting domain.
* @param {Object} options - Provider engine options
* @param {string} options.origin - The hostname of the sender
* @param {string} options.origin - The origin of the sender
* @param {string} options.location - The full URL of the sender
* @param {extensionId} [options.extensionId] - The extension ID of the sender, if the sender is an external extension
* @param {tabId} [options.tabId] - The tab ID of the sender - if the sender is within a tab
Expand Down
8 changes: 3 additions & 5 deletions app/scripts/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { EventEmitter } from 'events'
import Dnode from 'dnode'
import Eth from 'ethjs'
import EthQuery from 'eth-query'
import urlUtil from 'url'
import launchMetaMaskUi from '../../ui'
import StreamProvider from 'web3-stream-provider'
import { setupMultiplex } from './lib/stream-utils.js'
Expand Down Expand Up @@ -95,10 +94,9 @@ async function queryCurrentActiveTab (windowType) {
extension.tabs.query({ active: true, currentWindow: true }, (tabs) => {
const [activeTab] = tabs
const { title, url } = activeTab
const { hostname: origin, protocol } = url ? urlUtil.parse(url) : {}
resolve({
title, origin, protocol, url,
})
const { origin, protocol } = url ? new URL(url) : {}

resolve({ title, origin, protocol, url })
})
})
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/signature-request.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('MetaMask', function () {
const address = content[1]
assert.equal(await title.getText(), 'Signature Request')
assert.equal(await name.getText(), 'Ether Mail')
assert.equal(await origin.getText(), '127.0.0.1')
assert.equal(await origin.getText(), 'http://127.0.0.1:8080')
assert.equal(await address.getText(), publicAddress.slice(0, 8) + '...' + publicAddress.slice(publicAddress.length - 8))
})

Expand Down
4 changes: 2 additions & 2 deletions test/unit/app/controllers/metamask-controller-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ describe('MetaMaskController', function () {
'mock tx params',
{
...message,
origin: 'mycrypto.com',
origin: 'http://mycrypto.com',
tabId: 456,
},
]
Expand Down Expand Up @@ -865,7 +865,7 @@ describe('MetaMaskController', function () {
'mock tx params',
{
...message,
origin: 'mycrypto.com',
origin: 'http://mycrypto.com',
},
]
)
Expand Down
34 changes: 17 additions & 17 deletions ui/app/selectors/tests/permissions.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('selectors', function () {
'icon': 'https://peepeth.com/favicon-32x32.png',
'name': 'Peepeth',
},
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'icon': 'https://remix.ethereum.org/icon.png',
'name': 'Remix - Ethereum IDE',
},
Expand Down Expand Up @@ -45,7 +45,7 @@ describe('selectors', function () {
},
],
},
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'permissions': [
{
'@context': [
Expand All @@ -62,7 +62,7 @@ describe('selectors', function () {
],
'date': 1585685128948,
'id': '6b9615cc-64e4-4317-afab-3c4f8ee0244a',
'invoker': 'remix.ethereum.org',
'invoker': 'https://remix.ethereum.org',
'parentCapability': 'eth_accounts',
},
],
Expand All @@ -80,7 +80,7 @@ describe('selectors', function () {
extensionId,
name: 'Remix - Ethereum IDE',
icon: 'https://remix.ethereum.org/icon.png',
key: 'remix.ethereum.org',
key: 'https://remix.ethereum.org',
}])
})

Expand All @@ -93,7 +93,7 @@ describe('selectors', function () {
'icon': 'https://peepeth.com/favicon-32x32.png',
'name': 'Peepeth',
},
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'icon': 'https://remix.ethereum.org/icon.png',
'name': 'Remix - Ethereum IDE',
},
Expand Down Expand Up @@ -121,7 +121,7 @@ describe('selectors', function () {
},
],
},
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'permissions': [
{
'@context': [
Expand All @@ -139,7 +139,7 @@ describe('selectors', function () {
],
'date': 1585685128948,
'id': '6b9615cc-64e4-4317-afab-3c4f8ee0244a',
'invoker': 'remix.ethereum.org',
'invoker': 'https://remix.ethereum.org',
'parentCapability': 'eth_accounts',
},
],
Expand All @@ -152,7 +152,7 @@ describe('selectors', function () {
extensionId,
name: 'Remix - Ethereum IDE',
icon: 'https://remix.ethereum.org/icon.png',
key: 'remix.ethereum.org',
key: 'https://remix.ethereum.org',
}])
})
})
Expand All @@ -161,7 +161,7 @@ describe('selectors', function () {
const mockState = {
activeTab: {
'title': 'Eth Sign Tests',
'origin': 'remix.ethereum.org',
'origin': 'https://remix.ethereum.org',
'protocol': 'https:',
'url': 'https://remix.ethereum.org/',
},
Expand All @@ -185,7 +185,7 @@ describe('selectors', function () {
},
cachedBalances: {},
domains: {
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'permissions': [
{
'@context': [
Expand All @@ -206,7 +206,7 @@ describe('selectors', function () {
],
'date': 1586359844177,
'id': '3aa65a8b-3bcb-4944-941b-1baa5fe0ed8b',
'invoker': 'remix.ethereum.org',
'invoker': 'https://remix.ethereum.org',
'parentCapability': 'eth_accounts',
},
],
Expand Down Expand Up @@ -269,7 +269,7 @@ describe('selectors', function () {
],
}],
permissionsHistory: {
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'eth_accounts': {
'accounts': {
'0x7250739de134d33ec7ab1ee592711e15098c9d2d': 1586359844192,
Expand Down Expand Up @@ -323,7 +323,7 @@ describe('selectors', function () {
const mockState = {
activeTab: {
'title': 'Eth Sign Tests',
'origin': 'remix.ethereum.org',
'origin': 'https://remix.ethereum.org',
'protocol': 'https:',
'url': 'https://remix.ethereum.org/',
},
Expand All @@ -343,7 +343,7 @@ describe('selectors', function () {
},
},
domains: {
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'permissions': [
{
'@context': [
Expand All @@ -361,7 +361,7 @@ describe('selectors', function () {
],
'date': 1586359844177,
'id': '3aa65a8b-3bcb-4944-941b-1baa5fe0ed8b',
'invoker': 'remix.ethereum.org',
'invoker': 'https://remix.ethereum.org',
'parentCapability': 'eth_accounts',
},
],
Expand Down Expand Up @@ -412,13 +412,13 @@ describe('selectors', function () {
},
},
domainMetadata: {
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'icon': 'https://remix.ethereum.org/icon.png',
'name': 'Remix - Ethereum IDE',
},
},
permissionsHistory: {
'remix.ethereum.org': {
'https://remix.ethereum.org': {
'eth_accounts': {
'accounts': {
'0x7250739de134d33ec7ab1ee592711e15098c9d2d': 1586359844192,
Expand Down