Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619

lidel · 2018-11-14T15:35:12Z

Context and main discussion: #589

This PR adds await window.ipfs.enable(opts) as the new way of obtaining IPFS API Proxy object. When provided with a list of commands it will request access rights in bulk and return a promise with IPFS API instance or throw an error (details below).

The goal is to move away from synchronous way of accessing API instance and provide UX incentive for developers to use window.ipfs.enable instead of old API.

This change is backward compatible, old API remains to be exposed on window.ipfs for the time being.

ACL Management

When called without any arguments, command will just return API instance equal to the old window.ipfs or throw an error if IPFS Proxy is disabled in Preferences.

When called with options object (eg. { commands: ['id','peers'] }) ACLs for specified commands will be validated:

if any of the commands is denied or blocked, function will throw
if any of the commands require user approval, user will be presented
with a single prompt dialog that lists all requested permissions and URL
that requests them
- if user approves, ACLs are saved and future calls will not trigger
  prompt
- if user denies, ACLs are saved and an error is thrown for current
  and all future executions (unless user removed scope from blacklist)

To keep change set small I'd like to merge this as-is and work on "Future TODOs" (below) in separate PRs.

Tasks

scaffolding for window.ipfs.enable
decide if display prompt for .enable() (no parms) and keep no-dialog for things like ipfs.cat or display no initial dialog but prompt every api instead
- A: no change for now, logic stays the same
Implement window.ipfs.enable({commands: ['id','version'] })
- update docs
- tests
UX+API for requesting permissions in bulk
- UI remains the same to keep this PR small
add support for future opt-in experiments
- separate namespace for optional experimentation without any stability guarantees
- PoC await window.ipfs.enable({ experiments: { ipfsx: true } }) will return ipfsx version of the API
add deprecation warning to API calls executed on window.ipfs

Open Questions

Should we rename it to window.ipfsProxy.enable? (to avoid breaking websites that manually detect window.ipfs and execute window.ipfs.<command>) or is it enough to deprecate old use for 3 months and then remove support for old ways?
- We will keep the name and go with deprecation period (2019-04-01), as noted in Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619 (comment)
now that we have window.ipfs.enable should we require permission from user for all commands? (remove acl-whitelist for things like dag.get ?)
- We will remove whitelist, and nothing will be allowed without explicit permission from the user (rationale: Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619 (comment))

Future TODOs (separate PRs)

This simply changes the flow and API for getting IPFS proxy instance, does not implement UX nor decrease the size of injected content script.

This change adds async `enable` function that takes optional list of commands to grant access to via user prompt. The goal is to move away from synchronous way of accessing API instance and provide UX incentive to use `window.ipfs.enable` instead. When called without any arguments, command will just return API instance equal to the old `window.ipfs` or throw an error if IPFS Proxy is disabled in Preferences. When called with options object `{ commands: ['id','peers'] }` access rights for specified commands will be validated: - if any of the commands is denied or blocked, function will throw - if any of the commands require user approval, user will be presented with a single prompt dialog that lists all requested permissions and URL that requests them - if user approves, ACLs are saved and future calls will not trigger prompt - if user denies, ACLs are saved and an error is thrown for current and all future executions (unless user removed scope from blacklist) TODO (to be addressed in future commits) - add deprecation warning to API calls executed on `window.ipfs` - improve UX of permission dialog - add ability to return `ipfsx` version fo the API - disable `window.ipfs` injection via manifest in Chromium - stop exposing methods on `window.ipfs` - minimize the size of content script responsible for `window.ipfs` - lazy-init IPFS Proxy client on first call to `window.ipfs.enable()`

- also switch to terser-webpack-plugin

acl and api looked too similar and made code hard to follow, ipfs api core uses 'command' term to identify each endpoint so we follow that practice here

To avoid breaking changes we should deprecate old error codes first. This commit: - restores `permission` attribute and adds a deprecation warning when it is accessed. - adds error `code` to simplify error handling and alight convention with js-ipfs - updates docs

This adds an opt-in ipfsx experiment. In short, if `experiments:{ipfsx:true}` is passed `window.ipfs.enable` will return IPFS API instance wrapped in ipfsx prototype from https://github.com/ipfs-shipyard/ipfsx ``` let ipfs = await window.ipfs.enable({ commands: ['add','files.addPullStream'], experiments: { ipfsx: true } }) ```

lidel · 2018-12-16T23:43:12Z

add-on/src/contentScripts/ipfs-proxy/page.js

+      await require('postmsg-rpc').call('proxy.enable', opts)
+      // Additional client-side features
+      if (opts && opts.experiments) {
+        if (opts.experiments.ipfsx) {


The idea is to enable opt-in experiments such as ipfsx which is used as an example:

let ipfsx = await window.ipfs.enable({ commands: ['add','files.addPullStream'], experiments: { ipfsx: true } })

alanshaw · 2018-12-21T14:55:53Z

Should we rename it to window.ipfsProxy.enable? (to avoid breaking websites that manually detect window.ipfs and execute window.ipfs.<command>) or is it enough to deprecate old use for 3 months and then remove support for old ways?

I go for "deprecate old use for 3 months and then remove support for old ways" and use something like https://www.npmjs.com/package/deprecate to help communicate it to developers. Perhaps we could search github for projects using it and open an issue for them? window.ipfs is a desireable namespace to have - it's catchy, and honestly, we can't swtich namespace everytime we want to make a breaking change 😆

now that we have window.ipfs.enable should we require permission from user for all commands? (remove acl-whitelist for things like dag.get ?)

What would be the reason for this change?

lidel · 2019-01-02T15:54:03Z

Good points for keeping window.ipfs – you are right, it is still marked as "experiment", so we are off the hook for breaking it, as long we deprecate it over time.
(I will add deprecation notice to commands exposed directly on window.ipfs)

now that we have window.ipfs.enable should we require permission from user for all commands? (remove acl-whitelist for things like dag.get ?)

What would be the reason for this change?

My thinking was around enabling "privacy by default", where dapps have no access to IPFS node unless user granted access.

Rationale:

We added acl-whitelist as a privacy compromise to avoid DoSing user with multiple dialogs. Adding window.ipfs.enable removes the need for such compromise.
- To clarify what I mean by that: due to the way IPFS works there are no "safe" commands. Even a simple read-only dag.get can be abused to uniquely identifying user (by requesting unique CID and then finding user's multiaddr via findprov <cid>).

alanshaw · 2019-01-03T09:26:09Z

* To clarify what I mean by that: due to the way IPFS works there are no "safe" commands. Even a simple read-only  `dag.get` can be abused to uniquely identifying user (by requesting unique CID and then finding user's multiaddr via `findprov <cid>`).

Presumably you'd need permission to add something first to become a provider for that content?

...I appreciate that isn't the point and that there will be other ways to exploit things like this that we haven't considered. I also think this will encourage people to swtich to using .enable() so having considered the rationale I'm 👍 for this.

This change means no command can be run without explicit approval from the user. Rationale can be found in #619 (comment)

lidel · 2019-01-03T13:48:08Z

Thanks, I've added deprecation warning to window.ipfs.<cmd> (I think its better to be noisy about this, so its printed on every call):

Does the text look ok?

Presumably you'd need permission to add something first to become a provider for that content?

Yeah, someone else could pre-generate unique CIDs and just request them and see who is the second provider.

Anyway, I removed ACL whitelist and updated docs in 756b177.
Everything requires an explicit approval from the user now.

I'd like to ship it to Beta this week to start the clock ticking and continue in separate PR – any final feedback before we merge this?

lidel · 2019-01-07T10:43:25Z

Continued in #589

ghost assigned lidel Nov 14, 2018

ghost added the status/in-progress In progress label Nov 14, 2018

feat: scaffolding for window.ipfs.enable

4e6f65d

This simply changes the flow and API for getting IPFS proxy instance, does not implement UX nor decrease the size of injected content script.

lidel force-pushed the feat/window.ipfs-2.0 branch from f755c5e to 4e6f65d Compare December 10, 2018 11:32

lidel changed the title ~~[WIP] "window.ipfs 2.0"~~ [WIP] Add window.ipfs.enable Dec 12, 2018

lidel force-pushed the feat/window.ipfs-2.0 branch 2 times, most recently from 8c700c0 to 8b08439 Compare December 13, 2018 00:23

fix(build): regenerate yarn.lock

fb27d28

- also switch to terser-webpack-plugin

lidel force-pushed the feat/window.ipfs-2.0 branch from 8b08439 to fb27d28 Compare December 13, 2018 00:48

lidel changed the title ~~[WIP] Add window.ipfs.enable~~ Add window.ipfs.enable Dec 14, 2018

lidel changed the title ~~Add window.ipfs.enable~~ window.ipfs.enable(opts) (Bulk Permission Prompt) Dec 14, 2018

lidel changed the title ~~window.ipfs.enable(opts) (Bulk Permission Prompt)~~ Add window.ipfs.enable(opts) (Bulk Permission Prompt) Dec 14, 2018

lidel mentioned this pull request Dec 14, 2018

Meta: window.ipfs v2 #589

Closed

25 tasks

lidel added 3 commits December 14, 2018 21:56

chore: pre-api-whitelist → pre-command

a8db570

acl and api looked too similar and made code hard to follow, ipfs api core uses 'command' term to identify each endpoint so we follow that practice here

lidel force-pushed the feat/window.ipfs-2.0 branch from b464b9f to b633eb4 Compare December 16, 2018 23:33

lidel commented Dec 16, 2018

View reviewed changes

lidel requested review from alanshaw and olizilla December 16, 2018 23:56

lidel mentioned this pull request Dec 19, 2018

The Future of "accessing API of remote IPFS node" ipfs/in-web-browsers#137

Open

16 tasks

lidel requested a review from hugomrdias December 19, 2018 23:26

feat: add deprecation warning to window.ipfs.<cmd>

690ad80

lidel added a commit that referenced this pull request Jan 3, 2019

fix(privacy): remove ACL whitelist for window.ipfs

18c98b7

This change means no command can be run without explicit approval from the user. Rationale can be found in #619 (comment)

fix(privacy): remove ACL whitelist for window.ipfs

756b177

This change means no command can be run without explicit approval from the user. Rationale can be found in #619 (comment)

lidel force-pushed the feat/window.ipfs-2.0 branch from 18c98b7 to 756b177 Compare January 3, 2019 13:33

hugomrdias approved these changes Jan 3, 2019

View reviewed changes

docs(window.ipfs): updates before beta with v2

f3e9c6c

lidel merged commit 978bb33 into master Jan 7, 2019

ghost removed the status/in-progress In progress label Jan 7, 2019

lidel deleted the feat/window.ipfs-2.0 branch January 7, 2019 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619

Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619

lidel commented Nov 14, 2018 •

edited

Loading

lidel Dec 16, 2018 •

edited

Loading

alanshaw commented Dec 21, 2018

lidel commented Jan 2, 2019 •

edited

Loading

alanshaw commented Jan 3, 2019

lidel commented Jan 3, 2019 •

edited

Loading

lidel commented Jan 7, 2019

Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619

Add window.ipfs.enable(opts) (Bulk Permission Prompt) #619

Conversation

lidel commented Nov 14, 2018 • edited Loading

ACL Management

Tasks

Open Questions

Future TODOs (separate PRs)

lidel Dec 16, 2018 • edited Loading

Choose a reason for hiding this comment

alanshaw commented Dec 21, 2018

lidel commented Jan 2, 2019 • edited Loading

alanshaw commented Jan 3, 2019

lidel commented Jan 3, 2019 • edited Loading

lidel commented Jan 7, 2019

lidel commented Nov 14, 2018 •

edited

Loading

lidel Dec 16, 2018 •

edited

Loading

lidel commented Jan 2, 2019 •

edited

Loading

lidel commented Jan 3, 2019 •

edited

Loading