From 237cffba5ef6a8c2f97ae7464296392cb2407e1d Mon Sep 17 00:00:00 2001 From: jonkrone Date: Sat, 14 Apr 2018 13:39:33 -0500 Subject: [PATCH 1/8] feat: create and use IpfsApiAccessError --- add-on/src/lib/ipfs-proxy/api-access-error.js | 23 +++++++++++++++++++ add-on/src/lib/ipfs-proxy/pre-acl.js | 6 ++++- add-on/src/lib/ipfs-proxy/request-access.js | 7 +++++- 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 add-on/src/lib/ipfs-proxy/api-access-error.js diff --git a/add-on/src/lib/ipfs-proxy/api-access-error.js b/add-on/src/lib/ipfs-proxy/api-access-error.js new file mode 100644 index 000000000..69c2acef7 --- /dev/null +++ b/add-on/src/lib/ipfs-proxy/api-access-error.js @@ -0,0 +1,23 @@ +'use strict' +/* eslint-env browser */ + + +/** + * An Error that indicates API access was somehow denied by the user + * @type {IpfsApiAccessError} + * @example + * try { + * await ipfs.files.add(Buffer.from('seed')) + * } catch (err) { + * if (err instanceof window.ipfs.types.IpfsApiAccessError) { + * // API access was denied by the user + * } + * } + */ +export default class IpfsApiAccessError extends Error { + constructor (message, permission, scope) { + super(message) + this.permission = permission + this.scope = scope + } +} diff --git a/add-on/src/lib/ipfs-proxy/pre-acl.js b/add-on/src/lib/ipfs-proxy/pre-acl.js index 990a0e4a8..212dc39c7 100644 --- a/add-on/src/lib/ipfs-proxy/pre-acl.js +++ b/add-on/src/lib/ipfs-proxy/pre-acl.js @@ -1,3 +1,5 @@ +const IpfsApiAccessError = require('./api-access-error') + // This are the functions that DO NOT require an allow/deny decision by the user. // All other IPFS functions require authorization. const ACL_WHITELIST = Object.freeze(require('./acl-whitelist.json')) @@ -21,7 +23,9 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc access = await accessControl.setAccess(scope, wildcard ? '*' : permission, allow) } - if (!access.allow) throw new Error(`User denied access to ${permission}`) + if (!access.allow) { + throw new IpfsApiAccessError(`User denied access to ${permission}`, permission, scope) + } return args } diff --git a/add-on/src/lib/ipfs-proxy/request-access.js b/add-on/src/lib/ipfs-proxy/request-access.js index 5f3a99667..497861965 100644 --- a/add-on/src/lib/ipfs-proxy/request-access.js +++ b/add-on/src/lib/ipfs-proxy/request-access.js @@ -1,6 +1,7 @@ 'use strict' const { piggyback } = require('piggybacker') +const IpfsApiAccessError = require('./api-access-error') const DIALOG_WIDTH = 540 const DIALOG_HEIGHT = 220 @@ -98,7 +99,11 @@ function createRequestAccess (browser, screen) { const userTabRemoved = new Promise((resolve, reject) => { onTabRemoved = (id) => { if (id !== tabId) return - reject(new Error(`Failed to obtain access response for ${permission} at ${scope}`)) + reject(new IpfsApiAccessError( + `Failed to obtain access response for ${permission} at ${scope}`, + permission, + scope + )) } browser.tabs.onRemoved.addListener(onTabRemoved) }) From 5f9cd07a2c5b6ad303c68759f60f40b8430653ff Mon Sep 17 00:00:00 2001 From: jonkrone Date: Sat, 14 Apr 2018 13:53:09 -0500 Subject: [PATCH 2/8] test: Test that a well-formed IpfsApiAccessError is thrown when user denies permission --- add-on/src/lib/ipfs-proxy/api-access-error.js | 5 +++-- .../functional/lib/ipfs-proxy/pre-acl.test.js | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/add-on/src/lib/ipfs-proxy/api-access-error.js b/add-on/src/lib/ipfs-proxy/api-access-error.js index 69c2acef7..63e8c0019 100644 --- a/add-on/src/lib/ipfs-proxy/api-access-error.js +++ b/add-on/src/lib/ipfs-proxy/api-access-error.js @@ -1,7 +1,6 @@ 'use strict' /* eslint-env browser */ - /** * An Error that indicates API access was somehow denied by the user * @type {IpfsApiAccessError} @@ -14,10 +13,12 @@ * } * } */ -export default class IpfsApiAccessError extends Error { +class IpfsApiAccessError extends Error { constructor (message, permission, scope) { super(message) this.permission = permission this.scope = scope } } + +module.exports = IpfsApiAccessError diff --git a/test/functional/lib/ipfs-proxy/pre-acl.test.js b/test/functional/lib/ipfs-proxy/pre-acl.test.js index 1a9ee00d8..a32a47b23 100644 --- a/test/functional/lib/ipfs-proxy/pre-acl.test.js +++ b/test/functional/lib/ipfs-proxy/pre-acl.test.js @@ -7,6 +7,7 @@ const Sinon = require('sinon') const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control') const createPreAcl = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl') const ACL_WHITELIST = require('../../../../add-on/src/lib/ipfs-proxy/acl-whitelist.json') +const IpfsApiAccessError = require('../../../../add-on/src/lib/ipfs-proxy/api-access-error') describe('lib/ipfs-proxy/pre-acl', () => { before(() => { @@ -117,6 +118,27 @@ describe('lib/ipfs-proxy/pre-acl', () => { expect(() => { if (error) throw error }).to.throw(`User denied access to ${permission}`) }) + it('should have a well-formed IpfsApiAccessError if denied', async () => { + const getState = () => ({ ipfsProxy: true }) + const accessControl = new AccessControl(new Storage()) + const getScope = () => 'https://ipfs.io/' + const permission = 'files.add' + const requestAccess = Sinon.spy(async () => ({ allow: false })) + const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess) + + let error + + try { + await preAcl() + } catch (err) { + error = err + } + + expect(error).to.be.an.instanceOf(IpfsApiAccessError) + expect(error.permission).to.eql(permission) + expect(error.scope).to.eql(getScope()) + }) + it('should not re-request if allowed', async () => { const getState = () => ({ ipfsProxy: true }) const accessControl = new AccessControl(new Storage()) From 5b0e442cf0b89b776588146bc87272495c5ab20f Mon Sep 17 00:00:00 2001 From: jonkrone Date: Sat, 14 Apr 2018 14:03:16 -0500 Subject: [PATCH 3/8] doc: Update example checking for access denied errors. --- docs/window.ipfs.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/window.ipfs.md b/docs/window.ipfs.md index 767b5697b..cc6844cd8 100644 --- a/docs/window.ipfs.md +++ b/docs/window.ipfs.md @@ -38,19 +38,19 @@ if (window.ipfs) { To add and get content, you could do something like this: ```js -if (window.ipfs) { +if (window.ipfs) { try { - await ipfs.add(Buffer.from('=^.^=')) - } catch (err) { - if (err.message === 'User denied access to ipfs.files.add') { - // Fallback - } else { - throw err - } - } - - const data = await ipfs.cat('QmS5RzFV9v4fucVtjATPxoxABgEmNhhduykzUbQeGyyS3N') - console.log(data.toString()) // =^.^= + const [{ hash }] = await ipfs.add(Buffer.from('=^.^=')) + const data = await ipfs.cat(hash) + console.log(data.toString()) // =^.^= + } catch (err) { + if (err instanceof ipfs.types.IpfsApiAccessError) { + // Fallback + console.log(':(') + } else { + throw err + } + } } else { // Fallback } From 4181d53048948c935114cd8fc6808411b7dbcebb Mon Sep 17 00:00:00 2001 From: jonkrone Date: Sat, 14 Apr 2018 17:49:22 -0500 Subject: [PATCH 4/8] fix: make tests work on windows Tests were not running because some path parser didn't understand apostrophes so I've just converted those to escaped quotes. Some pre-mfs-scope tests failed because path.join presumes we want paths for the current running OS. I changed it to always use posix paths but I'm open to arguments that changing the implementation is a better approach. All of our target environments use posix paths, I believe. --- add-on/src/lib/ipfs-proxy/pre-mfs-scope.js | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js index f8af1e341..a23894f61 100644 --- a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js +++ b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js @@ -1,4 +1,4 @@ -const Path = require('path') +const Path = require('path').posix const DEFAULT_ROOT_PATH = '/dapps' // Creates a "pre" function that is called prior to calling a real function diff --git a/package.json b/package.json index 115eea6ab..471c38a30 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "watch:content-scripts:ipfs-proxy:page": "watchify -p prundupify -t [ browserify-package-json --global ] add-on/src/contentScripts/ipfs-proxy/page.js -o add-on/dist/contentScripts/ipfs-proxy/page.js -v", "watch:content-scripts:ipfs-proxy:content": "nodemon --exec \"browserify -p prundupify -t brfs -t [ browserify-package-json --global ] -s IpfsProxyContent add-on/src/contentScripts/ipfs-proxy/content.js -o add-on/dist/contentScripts/ipfs-proxy/content.js -v\" --watch add-on/src/contentScripts/ipfs-proxy/content.js --watch add-on/dist/contentScripts/ipfs-proxy/page.js", "test": "run-s test:*", - "test:functional": " nyc --reporter=lcov --reporter=text mocha --timeout 15000 --require ignore-styles --reporter mocha-jenkins-reporter 'test/functional/**/*.test.js'", + "test:functional": " nyc --reporter=lcov --reporter=text mocha --timeout 15000 --require ignore-styles --reporter mocha-jenkins-reporter \"test/functional/**/*.test.js\"", "lint": "run-s lint:*", "lint:standard": "standard -v \"add-on/src/**/*.js\" \"test/**/*.js\"", "lint:web-ext": "web-ext lint", From cf4241de923ae2ee536fe3ff3cf1e25303b6123e Mon Sep 17 00:00:00 2001 From: jonkrone Date: Thu, 19 Apr 2018 11:10:56 -0500 Subject: [PATCH 5/8] feat: refactor api-access error --- add-on/src/lib/ipfs-proxy/api-access-error.js | 24 ------------------- add-on/src/lib/ipfs-proxy/pre-acl.js | 6 ++--- add-on/src/lib/ipfs-proxy/request-access.js | 9 +++---- docs/window.ipfs.md | 2 +- .../functional/lib/ipfs-proxy/pre-acl.test.js | 11 +++++---- 5 files changed, 13 insertions(+), 39 deletions(-) delete mode 100644 add-on/src/lib/ipfs-proxy/api-access-error.js diff --git a/add-on/src/lib/ipfs-proxy/api-access-error.js b/add-on/src/lib/ipfs-proxy/api-access-error.js deleted file mode 100644 index 63e8c0019..000000000 --- a/add-on/src/lib/ipfs-proxy/api-access-error.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict' -/* eslint-env browser */ - -/** - * An Error that indicates API access was somehow denied by the user - * @type {IpfsApiAccessError} - * @example - * try { - * await ipfs.files.add(Buffer.from('seed')) - * } catch (err) { - * if (err instanceof window.ipfs.types.IpfsApiAccessError) { - * // API access was denied by the user - * } - * } - */ -class IpfsApiAccessError extends Error { - constructor (message, permission, scope) { - super(message) - this.permission = permission - this.scope = scope - } -} - -module.exports = IpfsApiAccessError diff --git a/add-on/src/lib/ipfs-proxy/pre-acl.js b/add-on/src/lib/ipfs-proxy/pre-acl.js index 212dc39c7..3061d1285 100644 --- a/add-on/src/lib/ipfs-proxy/pre-acl.js +++ b/add-on/src/lib/ipfs-proxy/pre-acl.js @@ -1,5 +1,3 @@ -const IpfsApiAccessError = require('./api-access-error') - // This are the functions that DO NOT require an allow/deny decision by the user. // All other IPFS functions require authorization. const ACL_WHITELIST = Object.freeze(require('./acl-whitelist.json')) @@ -24,7 +22,9 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc } if (!access.allow) { - throw new IpfsApiAccessError(`User denied access to ${permission}`, permission, scope) + const err = new Error(`User denied access to ${permission}`) + err.output = { payload: { isIpfsProxyAclError: true, permission, scope } } + throw err } return args diff --git a/add-on/src/lib/ipfs-proxy/request-access.js b/add-on/src/lib/ipfs-proxy/request-access.js index 497861965..d9f2b0e74 100644 --- a/add-on/src/lib/ipfs-proxy/request-access.js +++ b/add-on/src/lib/ipfs-proxy/request-access.js @@ -1,7 +1,6 @@ 'use strict' const { piggyback } = require('piggybacker') -const IpfsApiAccessError = require('./api-access-error') const DIALOG_WIDTH = 540 const DIALOG_HEIGHT = 220 @@ -99,11 +98,9 @@ function createRequestAccess (browser, screen) { const userTabRemoved = new Promise((resolve, reject) => { onTabRemoved = (id) => { if (id !== tabId) return - reject(new IpfsApiAccessError( - `Failed to obtain access response for ${permission} at ${scope}`, - permission, - scope - )) + const err = new Error(`Failed to obtain access response for ${permission} at ${scope}`) + err.output = { payload: { isIpfsProxyAclError: true, scope, permission } } + reject(err) } browser.tabs.onRemoved.addListener(onTabRemoved) }) diff --git a/docs/window.ipfs.md b/docs/window.ipfs.md index cc6844cd8..4e6d135de 100644 --- a/docs/window.ipfs.md +++ b/docs/window.ipfs.md @@ -44,7 +44,7 @@ if (window.ipfs) { const data = await ipfs.cat(hash) console.log(data.toString()) // =^.^= } catch (err) { - if (err instanceof ipfs.types.IpfsApiAccessError) { + if (err.isIpfsProxyAclError) { // Fallback console.log(':(') } else { diff --git a/test/functional/lib/ipfs-proxy/pre-acl.test.js b/test/functional/lib/ipfs-proxy/pre-acl.test.js index a32a47b23..faba83f0c 100644 --- a/test/functional/lib/ipfs-proxy/pre-acl.test.js +++ b/test/functional/lib/ipfs-proxy/pre-acl.test.js @@ -7,7 +7,6 @@ const Sinon = require('sinon') const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control') const createPreAcl = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl') const ACL_WHITELIST = require('../../../../add-on/src/lib/ipfs-proxy/acl-whitelist.json') -const IpfsApiAccessError = require('../../../../add-on/src/lib/ipfs-proxy/api-access-error') describe('lib/ipfs-proxy/pre-acl', () => { before(() => { @@ -118,7 +117,7 @@ describe('lib/ipfs-proxy/pre-acl', () => { expect(() => { if (error) throw error }).to.throw(`User denied access to ${permission}`) }) - it('should have a well-formed IpfsApiAccessError if denied', async () => { + it('should have a well-formed Error if denied', async () => { const getState = () => ({ ipfsProxy: true }) const accessControl = new AccessControl(new Storage()) const getScope = () => 'https://ipfs.io/' @@ -134,9 +133,11 @@ describe('lib/ipfs-proxy/pre-acl', () => { error = err } - expect(error).to.be.an.instanceOf(IpfsApiAccessError) - expect(error.permission).to.eql(permission) - expect(error.scope).to.eql(getScope()) + expect(error.output.payload).to.deep.eql({ + isIpfsProxyAclError: true, + permission, + scope: getScope() + }) }) it('should not re-request if allowed', async () => { From ce7e71aeeb91c84ec0f94a64b710f9c7a6e6241d Mon Sep 17 00:00:00 2001 From: jonkrone Date: Thu, 19 Apr 2018 11:11:35 -0500 Subject: [PATCH 6/8] feat: use path-browserify instead of builtin path. --- add-on/src/lib/ipfs-proxy/pre-mfs-scope.js | 3 ++- package.json | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js index a23894f61..7450042f3 100644 --- a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js +++ b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js @@ -1,4 +1,5 @@ -const Path = require('path').posix +// Use path-browserify for consistent behavior between browser and tests on windows +const Path = require('path-browserify') const DEFAULT_ROOT_PATH = '/dapps' // Creates a "pre" function that is called prior to calling a real function diff --git a/package.json b/package.json index 471c38a30..36bd49c76 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "lru_map": "0.3.3", "mime-types": "2.1.18", "p-queue": "2.4.0", + "path-browserify": "0.0.0", "piggybacker": "2.0.0", "prundupify": "1.0.0", "tachyons": "4.9.1", From 73eb7cc5e00a321b25fbf764830eacb9596a2850 Mon Sep 17 00:00:00 2001 From: jonkrone Date: Thu, 19 Apr 2018 12:14:45 -0500 Subject: [PATCH 7/8] fix: update yarn lock file. --- add-on/src/lib/ipfs-proxy/pre-mfs-scope.js | 2 +- yarn.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js index 7450042f3..e04315c20 100644 --- a/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js +++ b/add-on/src/lib/ipfs-proxy/pre-mfs-scope.js @@ -1,4 +1,4 @@ -// Use path-browserify for consistent behavior between browser and tests on windows +// Use path-browserify for consistent behavior between browser and tests on Windows const Path = require('path-browserify') const DEFAULT_ROOT_PATH = '/dapps' diff --git a/yarn.lock b/yarn.lock index 0c3f7278f..43ec641ca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7176,7 +7176,7 @@ pascalcase@^0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/pascalcase/-/pascalcase-0.1.1.tgz#b363e55e8006ca6fe21784d2db22bd15d7917f14" -path-browserify@~0.0.0: +path-browserify@0.0.0, path-browserify@~0.0.0: version "0.0.0" resolved "https://registry.yarnpkg.com/path-browserify/-/path-browserify-0.0.0.tgz#a0b870729aae214005b7d5032ec2cbbb0fb4451a" From a84062212d8cd43c399c21531a77accb08ca2cb4 Mon Sep 17 00:00:00 2001 From: jonkrone Date: Wed, 25 Apr 2018 13:00:19 -0500 Subject: [PATCH 8/8] feat: update console.log in window.ipfs error example --- docs/window.ipfs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/window.ipfs.md b/docs/window.ipfs.md index 4e6d135de..cc4efd0be 100644 --- a/docs/window.ipfs.md +++ b/docs/window.ipfs.md @@ -46,7 +46,7 @@ if (window.ipfs) { } catch (err) { if (err.isIpfsProxyAclError) { // Fallback - console.log(':(') + console.log('Unable to get ACL decision from user :(', err) } else { throw err }