Skip to content

Commit

Permalink
fix(privacy): remove ACL whitelist for window.ipfs
Browse files Browse the repository at this point in the history
This change means no command can be run without explicit approval from
the user.

Rationale can be found in
#619 (comment)
  • Loading branch information
lidel committed Jan 3, 2019
1 parent 690ad80 commit 18c98b7
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 89 deletions.
25 changes: 0 additions & 25 deletions add-on/src/lib/ipfs-proxy/acl-whitelist.json

This file was deleted.

14 changes: 6 additions & 8 deletions add-on/src/lib/ipfs-proxy/enable-command.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { inCommandWhitelist, createCommandWhitelistError } = require('./pre-command')
const { inNoAclPromptWhitelist, createProxyAclError } = require('./pre-acl')
const { createProxyAclError } = require('./pre-acl')

// Artificial API command responsible for backend orchestration
// during window.ipfs.enable()
Expand All @@ -26,13 +26,11 @@ function createEnableCommand (getIpfs, getState, getScope, accessControl, reques
}
// Get the current access flag to decide if it should be added
// to the list of permissions to be prompted about in the next step
if (!inNoAclPromptWhitelist(command)) {
let access = await accessControl.getAccess(scope, command)
if (!access) {
missingAcls.push(command)
} else if (access.allow !== true) {
deniedAcls.push(command)
}
let access = await accessControl.getAccess(scope, command)
if (!access) {
missingAcls.push(command)
} else if (access.allow !== true) {
deniedAcls.push(command)
}
}
// Fail fast if user already denied any of requested permissions
Expand Down
17 changes: 2 additions & 15 deletions add-on/src/lib/ipfs-proxy/pre-acl.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// 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'))

// Creates a "pre" function that is called prior to calling a real function
// on the IPFS instance. It will throw if access is denied, and ask the user if
// no access decision has been made yet.
Expand All @@ -10,9 +6,6 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc
// Check if all access to the IPFS node is disabled
if (!getState().ipfsProxy) throw new Error('User disabled access to API proxy in IPFS Companion')

// No need to verify access if permission is on the whitelist
if (inNoAclPromptWhitelist(permission)) return args

const scope = await getScope()
const access = await getAccessWithPrompt(accessControl, requestAccess, scope, permission)

Expand All @@ -24,10 +17,6 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc
}
}

function inNoAclPromptWhitelist (permission) {
return ACL_WHITELIST.includes(permission)
}

async function getAccessWithPrompt (accessControl, requestAccess, scope, permission) {
let access = await accessControl.getAccess(scope, permission)
if (!access) {
Expand All @@ -37,7 +26,7 @@ async function getAccessWithPrompt (accessControl, requestAccess, scope, permiss
return access
}

// Standardized error thrown when a command is not on the ACL_WHITELIST
// Standardized error thrown when a command access is denied
// TODO: return errors following conventions from https://github.com/ipfs/js-ipfs/pull/1746
function createProxyAclError (scope, permission) {
const err = new Error(`User denied access to selected commands over IPFS proxy: ${permission}`)
Expand Down Expand Up @@ -65,7 +54,5 @@ function createProxyAclError (scope, permission) {

module.exports = {
createPreAcl,
createProxyAclError,
inNoAclPromptWhitelist,
ACL_WHITELIST
createProxyAclError
}
20 changes: 0 additions & 20 deletions test/functional/lib/ipfs-proxy/enable-command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const Sinon = require('sinon')
const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control')
const createEnableCommand = require('../../../../add-on/src/lib/ipfs-proxy/enable-command')
const createRequestAccess = require('../../../../add-on/src/lib/ipfs-proxy/request-access')
const { ACL_WHITELIST } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl')

describe('lib/ipfs-proxy/enable-command', () => {
before(() => {
Expand Down Expand Up @@ -71,25 +70,6 @@ describe('lib/ipfs-proxy/enable-command', () => {
expect(requestAccess.called).to.equal(false)
})

it('should allow access without prompt if permission is on ACL whitelist', async () => {
const getState = () => ({ ipfsProxy: true })
const accessControl = new AccessControl(new Storage())
const getScope = () => 'https://3.foo.tld/path/'
const getIpfs = () => {}
const requestAccess = async () => { throw new Error('Requested access for whitelist permission') }
const enable = createEnableCommand(getIpfs, getState, getScope, accessControl, requestAccess)

let error

try {
await Promise.all(ACL_WHITELIST.map(permission => enable({ commands: [permission] })))
} catch (err) {
error = err
}

expect(error).to.equal(undefined)
})

it('should request access if no grant exists', async () => {
const getState = () => ({ ipfsProxy: true })
const accessControl = new AccessControl(new Storage())
Expand Down
22 changes: 1 addition & 21 deletions test/functional/lib/ipfs-proxy/pre-acl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { URL } = require('url')
const Storage = require('mem-storage-area/Storage')
const Sinon = require('sinon')
const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control')
const { createPreAcl, ACL_WHITELIST } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl')
const { createPreAcl } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl')

describe('lib/ipfs-proxy/pre-acl', () => {
before(() => {
Expand All @@ -31,26 +31,6 @@ describe('lib/ipfs-proxy/pre-acl', () => {
expect(() => { if (error) throw error }).to.throw('User disabled access to API proxy in IPFS Companion')
})

it('should allow access if permission is on whitelist', async () => {
const getState = () => ({ ipfsProxy: true })
const accessControl = new AccessControl(new Storage())
const getScope = () => 'https://ipfs.io/'
const requestAccess = async () => { throw new Error('Requested access for whitelist permission') }

let error

try {
await Promise.all(ACL_WHITELIST.map(permission => {
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)
return preAcl()
}))
} catch (err) {
error = err
}

expect(error).to.equal(undefined)
})

it('should request access if no grant exists', async () => {
const getState = () => ({ ipfsProxy: true })
const accessControl = new AccessControl(new Storage())
Expand Down

0 comments on commit 18c98b7

Please sign in to comment.