Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improved support for remote API #1613

Merged
merged 7 commits into from
Sep 10, 2020
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 4, 2020

Closes #1586
Closes #1565
Closes #836
Closes #1194
Closes #1389
Closes #1031

This PR adds support for connecting to remote API that is protected by HTTP Basic Auth (#1586), enables entering custom configuration (a hidden feature for advanced users) and adds end-to-end tests.

Terse summary

  • There is UI for changing the API address on the Settings screen
  • Custom API address is stored in and read from window.localStorage[ipfsApi]
  • ipfsApi is a string or a null
  • If ipfsApi is null, ipfs-provider will try default address at /ip4/127.0.0.1/tcp/5001 (and same-origin for a good measure)
  • ipfsApi can be a multiaddr, URL or JSON with config for ipfs-http-client used by webui
  • User can enter URL with inlined basic auth user and password, we convert them to advanced config before passing to http client.
  • E2E tests for using remote API with basic auth (URL or JSON, by setting localStorage[ipfsApi] or via Settings screen)
  • Updated some dependencies related to E2E and made things bit more robust

Visual changes

Settings page

It is now possible to enter URL with inlined basic auth:

Screenshot_2020-09-04 Settings IPFS

JSON with equivalent config for ipfs-http-client is also accepted:

Screenshot_2020-09-04 Settings IPFS(1)

Note: this is sort of an easter egg, we should not mention it in UI. This is mostly future-proofing. If someone has a very custom setup, webui will be able to support it without the need for a new release or a fancy UI for that one person.

Status page

Default state is the same(multiaddr), so most of the users will see no difference:

Screenshot_2020-09-04 Status IPFS(1)

If a custom URL was entered on Settings, we also show URL on Status:

Screenshot_2020-09-04 Status IPFS

If JSON was entered, then we show it as-is. Here, a custom config with Basic Auth header:

Screenshot_2020-09-04 Status IPFS(3)

I believe this is so niche, power user feature, that there is no point in making UI for this. Edit button is enough.
Instead of showing JSON, we could just print [JSON config] placeholder or something – @jessicaschilling thoughts?

This change adds support for connecting to remote API that is protected
by HTTP Basic Auth and enables entering custom configuration as a hidden
feature for advanced users.

Terse summary:
- custom API address is stored in and read from localStorage[ipfsApi]
- ipfsApi is a string or null
- if ipfsApi is null, ipfs-provider will try default /ip4/127.0.0.1/tcp/5001 and same-origin
- ipfsApi tring can be multiaddr, URL or a JSON with constructor
  config for ipfs-http-client
- user can enter URL with inlined basic auth  user and password, we
  convert them to advanced config before passing to http client.
- E2E tests for using remote API with basic auth
  (URL or JSON, by setting localStorage[ipfsApi] or via Settings screen)
@jessicaschilling
Copy link
Contributor

jessicaschilling commented Sep 4, 2020

@lidel LGTM -- thank you for the super-detailed screenshots 😊
Agree that just showing a message rather than the full JSON config is a good idea - plus reduces the risk of folks inadvertently sharing stuff in screenshots they don't want others to see.
How about Custom JSON configuration? (note no square brackets)

@lidel
Copy link
Member Author

lidel commented Sep 7, 2020

@jessicaschilling done:

2020-09-07--16-14-03

One can test this even without basic auth setup, this JSON will work: {"url":"http://127.0.0.1:5001/api/v0"}

@lidel

This comment has been minimized.

This makes tests DRY and adds regression tests for API set to multiaddr
and regular URL in addition to existing basic auth JSON/URL.
@lidel
Copy link
Member Author

lidel commented Sep 9, 2020

Passing config object to ipfs-http-client turned out tot be brittle and risky (eg. right now the object gets mutated and user sees different JSON than the one she entered).

I've addressed that in ipfs-shipyard/ipfs-provider#24 but need to update this PR to use documented parameters instead of url.
Probably safer to do it after some sleep, so will get back to this tomorrow 💤

This update ensures that if user passes a custom config for
ipfs-http-client, the object is not mutated by the http client.
@lidel
Copy link
Member Author

lidel commented Sep 9, 2020

Switched to ipfs-provider 1.1.0, which does not mutate the custom config, so the "undocumented url attribute" is no longer visible to the user. Only documented parameters for ipfs-http-client are used:

Screenshot_2020-09-09 Settings IPFS

@rafaelramalho19 ready for your final review.

test/e2e/remote-api.test.js Show resolved Hide resolved
src/bundles/ipfs-provider.js Outdated Show resolved Hide resolved
src/bundles/ipfs-provider.js Outdated Show resolved Hide resolved
src/bundles/ipfs-provider.js Outdated Show resolved Hide resolved
This removes double JSON serialization, as noted in
#1613 (comment)
and cleans up a bunch of tests to only use documented config keys
for ipfs-http-client.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants