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 756b177
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 94 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
}
15 changes: 10 additions & 5 deletions docs/window.ipfs.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,21 @@ It looks like this:

## Do I need to confirm every API call?

Not all function calls require a decision from the user. You will be able to call [whitelisted](../add-on/src/lib/ipfs-proxy/acl-whitelist.json) IPFS functions and users will _not_ be prompted to allow/deny access.
Command access need to be confirmed only once [per scope](#how-are-permissions-scoped).

Functions that are not whitelisted need to be confirmed only once [per scope](#how-are-permissions-scoped).
If you provide a list of commands when requesting API instance via `window.ipfs.enable({commands})`
then a single permission dialog will be displayed to the user.

Note that users can modify their permission decisions after the fact so you should not expect to always be allowed to call a function if it was successfully called previously.
For everything else, only the first call requires a decision from the user. You will be able to call
previously whitelisted IPFS commands and users will _not_ be prompted to
allow/deny access the second time.

Note that users can modify their permission decisions after the fact so you should not expect to always be allowed to call a command if it was successfully called previously.


## Can I disable this for now?

Users can permanently deny access to all IPFS functions by disabling `window.ipfs` experiment on _Preferences_ screen.
Users can permanently deny access to all IPFS commands by disabling `window.ipfs` experiment on _Preferences_ screen.

## How are permissions scoped?

Expand Down Expand Up @@ -166,7 +171,7 @@ e.g.

## Are mutable file system (MFS) files sandboxed to a directory?

Yes. To avoid conflicts, each app gets it's own MFS directory where it can store files. When using MFS functions (see [docs](https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/FILES.md#mutable-file-system)) this directory will be automatically added to paths you pass. Your app's MFS directory is based on the **origin and path** where your application is running.
Yes. To avoid conflicts, each app gets it's own MFS directory where it can store files. When using MFS commands (see [docs](https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/FILES.md#mutable-file-system)) this directory will be automatically added to paths you pass. Your app's MFS directory is based on the **origin and path** where your application is running.

e.g.

Expand Down
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 756b177

Please sign in to comment.