From 5d5b215facf0066510ea555ea63c3d3579d5db31 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Fri, 1 Nov 2019 17:37:38 -0700 Subject: [PATCH] update plugin permissions flow add wallet_installPlugins method update inpage provider --- ...dleware.js => internalMethodMiddleware.js} | 34 ++++++++++- .../permissions/loggerMiddleware.js | 23 ++++---- .../controllers/permissions/permissions.js | 58 ++++++++++++------- app/scripts/controllers/plugins.js | 34 +++++++++++ app/scripts/inpage.js | 6 -- package.json | 4 +- yarn.lock | 38 +++++------- 7 files changed, 130 insertions(+), 67 deletions(-) rename app/scripts/controllers/permissions/{requestMiddleware.js => internalMethodMiddleware.js} (64%) diff --git a/app/scripts/controllers/permissions/requestMiddleware.js b/app/scripts/controllers/permissions/internalMethodMiddleware.js similarity index 64% rename from app/scripts/controllers/permissions/requestMiddleware.js rename to app/scripts/controllers/permissions/internalMethodMiddleware.js index ee0653f95..4acd9189c 100644 --- a/app/scripts/controllers/permissions/requestMiddleware.js +++ b/app/scripts/controllers/permissions/internalMethodMiddleware.js @@ -6,7 +6,7 @@ const { errors: rpcErrors } = require('eth-json-rpc-errors') * Create middleware for preprocessing permissions requests. */ module.exports = function createRequestMiddleware ({ - store, storeKey, + store, storeKey, handleInstallPlugins, }) { return createAsyncMiddleware(async (req, res, next) => { @@ -16,8 +16,36 @@ module.exports = function createRequestMiddleware ({ } const prefix = 'wallet_' + const pluginPrefix = prefix + 'plugin_' + if (req.method.startsWith(prefix)) { + switch (req.method.split(prefix)[1]) { + + case 'installPlugins': + + if ( + !Array.isArray(req.params) || typeof req.params[0] !== 'object' + ) { + res.error = rpcErrors.invalidParams(null, req) + return + } + + const requestedPlugins = Object.keys(req.params[0]).filter( + p => p.startsWith(pluginPrefix) + ) + + if (requestedPlugins.length === 0) { + res.error = rpcErrors.invalidParams('Must request at least one plugin.', req) + } + + try { + res.result = await handleInstallPlugins(req.origin, requestedPlugins) + } catch (err) { + res.error = err + } + return + case 'sendDomainMetadata': if ( req.siteMetadata && @@ -27,10 +55,12 @@ module.exports = function createRequestMiddleware ({ } res.result = true return + default: break } - // plugins are handled here + + // plugin metadata is handled here // TODO:plugin handle this better, rename siteMetadata to domainMetadata everywhere } else if ( req.origin !== 'MetaMask' && diff --git a/app/scripts/controllers/permissions/loggerMiddleware.js b/app/scripts/controllers/permissions/loggerMiddleware.js index 73bd367a0..fd5d3e080 100644 --- a/app/scripts/controllers/permissions/loggerMiddleware.js +++ b/app/scripts/controllers/permissions/loggerMiddleware.js @@ -81,23 +81,24 @@ module.exports = function createLoggerMiddleware ({ let accounts const entries = result - .map(perm => { + ? result.map(perm => { if (perm.parentCapability === 'eth_accounts') { accounts = getAccountsFromPermission(perm) } return perm.parentCapability }) - .reduce((acc, m) => { - if (requestedMethods.includes(m)) { - acc[m] = { - lastApproved: time, + .reduce((acc, m) => { + if (requestedMethods.includes(m)) { + acc[m] = { + lastApproved: time, + } + if (m === 'eth_accounts') { + acc[m].accounts = accounts + } } - if (m === 'eth_accounts') { - acc[m].accounts = accounts - } - } - return acc - }, {}) + return acc + }, {}) + : {} if (Object.keys(entries).length > 0) { commitHistory(origin, entries, accounts) diff --git a/app/scripts/controllers/permissions/permissions.js b/app/scripts/controllers/permissions/permissions.js index 11482b683..4b76dca53 100644 --- a/app/scripts/controllers/permissions/permissions.js +++ b/app/scripts/controllers/permissions/permissions.js @@ -9,7 +9,7 @@ const { getExternalRestrictedMethods, pluginRestrictedMethodDescriptions, } = require('./restrictedMethods') -const createRequestMiddleware = require('./requestMiddleware') +const createInternalMethodMiddleware = require('./internalMethodMiddleware') const createLoggerMiddleware = require('./loggerMiddleware') // Methods that do not require any permissions to use: @@ -52,9 +52,10 @@ class PermissionsController { const { origin, isPlugin } = options const engine = new JsonRpcEngine() engine.push(this.createPluginMethodRestrictionMiddleware(isPlugin)) - engine.push(createRequestMiddleware({ + engine.push(createInternalMethodMiddleware({ store: this.store, storeKey: METADATA_STORE_KEY, + handleInstallPlugins: this.handleInstallPlugins.bind(this), })) engine.push(createLoggerMiddleware({ walletPrefix: WALLET_METHOD_PREFIX, @@ -91,6 +92,35 @@ class PermissionsController { }) } + /** + * @param {string} origin - The external domain id. + * @param {Array} requestedPlugins - The names of the requested plugin permissions. + */ + async handleInstallPlugins (origin, requestedPlugins) { + + const existingPerms = this.permissions.getPermissionsForDomain(origin).reduce( + (acc, p) => { + acc[p.parentCapability] = true + return acc + }, {} + ) + + requestedPlugins.forEach(p => { + if (!existingPerms[p]) { + throw rpcErrors.eth.unauthorized(`Not authorized to install plugin '${p}'.`) + } + }) + + const installedPlugins = await this.pluginsController.processRequestedPlugins(requestedPlugins) + + if (installedPlugins.length === 0) { + // TODO:plugins reserve error in Ethereum error space? + throw rpcErrors.eth.custom(4301, 'Failed to install all plugins.', requestedPlugins) + } + + return installedPlugins + } + /** * Returns the accounts that should be exposed for the given origin domain, * if any. This method exists for when a trusted context needs to know @@ -176,31 +206,17 @@ class PermissionsController { const approval = this.pendingApprovals[id] this._closePopup && this._closePopup() - // Load any requested plugins first: - const pluginNames = this.pluginsFromPerms(approved.permissions) try { - await Promise.all(pluginNames.map((plugin) => { - return this.pluginsController.add(plugin) - })) + + // TODO:plugins: perform plugin preflight check? + // e.g., is the plugin valid? can its manifest be fetched? is the manifest valid? + // not strictly necessary, but probably good UX. + // const pluginNames = this.pluginsFromPerms(approved.permissions) const resolve = approval.resolve resolve(approved.permissions) delete this.pendingApprovals[id] - // Once we've approved the initial app permissions, - // we are free to prompt for the plugin permissions: - Promise.all(pluginNames.map(async (pluginName) => { - const plugin = await this.pluginsController.authorize(pluginName) - const { sourceCode, approvedPermissions } = plugin - const ethereumProvider = this.pluginsController.setupProvider(pluginName, async () => { return {name: pluginName } }, true) - await this.pluginsController.run(pluginName, approvedPermissions, sourceCode, ethereumProvider) - })) - .catch((err) => { - // We swallow this error, we don't want the plugin permissions prompt to block the resolution - // Of the main dapp's permissions prompt. - console.error(`Error when adding plugin:`, err) - }) - } catch (reason) { const { reject } = approval reject(reason) diff --git a/app/scripts/controllers/plugins.js b/app/scripts/controllers/plugins.js index 57d850140..805a34769 100644 --- a/app/scripts/controllers/plugins.js +++ b/app/scripts/controllers/plugins.js @@ -137,6 +137,40 @@ class PluginsController extends EventEmitter { }) } + /** + * Adds, authorizes, and runs plugins with a plugin provider. + */ + async processRequestedPlugins (requestedPlugins) { + + const pluginNames = requestedPlugins.map( + perm => perm.replace(/^wallet_plugin_/, '') + ) + + const added = await Promise.all(pluginNames.map(async (pluginName) => { + try { + await this.add(pluginName) + const plugin = await this.authorize(pluginName) + const { sourceCode, approvedPermissions } = plugin + const ethereumProvider = this.setupProvider( + pluginName, async () => { return {name: pluginName } }, true + ) + await this.run( + pluginName, approvedPermissions, sourceCode, ethereumProvider + ) + return pluginName + } catch (err) { + // TODO: figure out what to do with this error + console.error(`Error when adding plugin:`, err) + } + })) + .catch((err) => { + // TODO: will this ever happen? + console.error(`Unexpected error when adding plugins:`, err) + }) + + return added ? added.filter(plugin => !!plugin) : [] + } + /** * Returns a promise representing the complete installation of the requested plugin. * If the plugin is already being installed, the previously pending promise will be returned. diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index a1688b785..6d7214291 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -68,9 +68,3 @@ const proxiedInpageProvider = new Proxy(inpageProvider, { }) window.ethereum = proxiedInpageProvider - -inpageProvider.publicConfigStore.subscribe(function (state) { - if (state.onboardingcomplete) { - window.postMessage('onboardingcomplete', '*') - } -}) diff --git a/package.json b/package.json index f2816b4d8..d01194884 100644 --- a/package.json +++ b/package.json @@ -117,14 +117,14 @@ "gaba": "^1.6.0", "human-standard-token-abi": "^2.0.0", "jazzicon": "^1.2.0", - "json-rpc-engine": "^5.1.4", + "json-rpc-engine": "^5.1.5", "json-rpc-middleware-stream": "^2.1.1", "jsonschema": "^1.2.4", "lodash.debounce": "^4.0.8", "lodash.shuffle": "^4.2.0", "loglevel": "^1.4.1", "luxon": "^1.8.2", - "metamask-inpage-provider": "MetaMask/metamask-inpage-provider#plugins", + "metamask-inpage-provider": "MetaMask/metamask-inpage-provider#snaps", "metamask-logo": "^2.1.4", "mkdirp": "^0.5.1", "multihashes": "^0.4.12", diff --git a/yarn.lock b/yarn.lock index f9b807525..3e9523bd8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6028,6 +6028,14 @@ capnode@^4.0.1: crypto-random-string "^1.0.0" stream "0.0.2" +capnode@^4.0.2: + version "4.0.2" + resolved "https://registry.yarnpkg.com/capnode/-/capnode-4.0.2.tgz#405f88de12220d9ffb0eed5a133c28761158385e" + integrity sha512-U+7VWe6cH1qLnZNvScGTRMU1n3pIDX8QSPmZ5gjEJniuSDIMgBpfiammo3XSZLumwDqYf8zlOnxc7tzEbRa4bA== + dependencies: + crypto-random-string "^1.0.0" + stream "0.0.2" + capture-stack-trace@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/capture-stack-trace/-/capture-stack-trace-1.0.1.tgz#a6c0bbe1f38f3aa0b92238ecb6ff42c344d4135d" @@ -15597,27 +15605,7 @@ json-rpc-engine@^3.4.0, json-rpc-engine@^3.6.0: promise-to-callback "^1.0.0" safe-event-emitter "^1.0.1" -json-rpc-engine@^5.1.3: - version "5.1.3" - resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.1.3.tgz#d7410b649e107ed3437db33797f44c51d507002c" - integrity sha512-/rQm6uts6JtjOVEaeSDCJgHDTlbfKDdoR1Uh3f+6za2SwhJyz+jL9iED2aapU9Yx7decLlI7wjVUIwxRg/R7WQ== - dependencies: - async "^2.0.1" - eth-json-rpc-errors "^1.0.1" - promise-to-callback "^1.0.0" - safe-event-emitter "^1.0.1" - -json-rpc-engine@^5.1.4: - version "5.1.4" - resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.1.4.tgz#c18d1959eb175049fa7301d4866931ae2f879e47" - integrity sha512-nBFWYJ1mvlZL7gqq0M9230SxedL9CbSYO1WgrFi/C1Zo+ZrHUZWLRbr7fUdlLt9TC0G+sf/aEUeuJjR2lHsMvA== - dependencies: - async "^2.0.1" - eth-json-rpc-errors "^1.1.0" - promise-to-callback "^1.0.0" - safe-event-emitter "^1.0.1" - -json-rpc-engine@^5.1.5: +json-rpc-engine@^5.1.3, json-rpc-engine@^5.1.5: version "5.1.5" resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.1.5.tgz#a5f9915356ea916d5305716354080723c63dede7" integrity sha512-HTT9HixG4j8vHYrmJIckgbISW9Q8tCkySv7x7Q8zjMpcw10wSe/dZSQ0w08VkDm3y195K4074UlvD3hxaznvlw== @@ -17982,11 +17970,11 @@ mersenne-twister@^1.0.1: resolved "https://registry.yarnpkg.com/mersenne-twister/-/mersenne-twister-1.1.0.tgz#f916618ee43d7179efcf641bec4531eb9670978a" integrity sha1-+RZhjuQ9cXnvz2Qb7EUx65Zwl4o= -metamask-inpage-provider@MetaMask/metamask-inpage-provider#plugins: - version "4.0.0" - resolved "https://codeload.github.com/MetaMask/metamask-inpage-provider/tar.gz/2884eef783b1728ac34c52e3dda601a98885ee87" +metamask-inpage-provider@MetaMask/metamask-inpage-provider#snaps: + version "5.0.0" + resolved "https://codeload.github.com/MetaMask/metamask-inpage-provider/tar.gz/38abec68d77364fd561a74ab7c60b54982d406d8" dependencies: - capnode "^4.0.1" + capnode "^4.0.2" eth-json-rpc-errors "^2.0.0" fast-deep-equal "^2.0.1" json-rpc-engine "^5.1.5"