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(rpc): Opt-in HTTP RPC API Authorization #10218

Merged
merged 10 commits into from
Nov 17, 2023
Merged

feat(rpc): Opt-in HTTP RPC API Authorization #10218

merged 10 commits into from
Nov 17, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Nov 14, 2023

Closes #10187 ← see design requirements and purpose there.

I tried not to touch go-ipfs-cmds. However, what I did in Kubo could've been done there too.

Closes #1532
Closes #2389
CC ipfs/ipfs-webui#1586 ipfs/go-ipfs-api#172


Feature Summary

This PR provides Kubo users with a basic HTTP Auth primitives for locking the RPC API down, and exposing only a subset of commands per access token defined in API.Authorization map.

Future work, such as UCANs mentioned here, or sandboxing MSF, keys, IPNS names per user hinted here, could be built on top of this at a later time.

@hacdias hacdias requested review from lidel and a team as code owners November 14, 2023 12:32
core/corehttp/commands.go Outdated Show resolved Hide resolved
config/api.go Outdated Show resolved Hide resolved
cmd/ipfs/main.go Outdated Show resolved Hide resolved
@hacdias
Copy link
Member Author

hacdias commented Nov 15, 2023

@lidel I have some concerns / questions here:

  • client/rpc already allows to create a new client with a custom http.Client. This allows users to use AuthorizedRoundTripper to create a client. That is nice. However, I think adding a new option would make it much easier. But the way our API is made means that we need to update the coreiface package to include the new option.
  • What I have done in Kubo could've been done inside go-ipfs-cmds. Not really sure where it is best to do this, but I want to avoid touching packages we do not need to touch.
  • --api-secret asks for the Authorization header value now. Should we do it like that or accept the same structure type:value as in the configuration?

@lidel lidel changed the title feat(rpc): opt-in HTTP secrets to protect RPC API feat(rpc): Support opt-in HTTP RPC API Authorization Nov 15, 2023
@lidel lidel changed the title feat(rpc): Support opt-in HTTP RPC API Authorization feat(rpc): Opt-in HTTP RPC API Authorization Nov 15, 2023
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hacdias quick answers:

  1. client/rpc passing custom client is fine for now, let's keep this PR small, we can improve later (add support for creating RPC client with arbitrary options like custom headers and auth).
  2. limiting blast radius sgtm, goal here is to establish tests, configuration and UX. we can upstream later if needed.
  3. yes, rename --api-secret to --api-auth and make it use the same syntax as config (basic:user:pass or bearer:foo). To make UX nicer, let's assume bearer when : is not present in the string)
  • see comments inline below

config/api.go Outdated Show resolved Hide resolved
core/commands/root.go Outdated Show resolved Hide resolved
core/commands/root.go Outdated Show resolved Hide resolved
core/corehttp/commands.go Outdated Show resolved Hide resolved
test/cli/auth_test.go Outdated Show resolved Hide resolved
test/cli/auth_test.go Outdated Show resolved Hide resolved
config/api.go Outdated Show resolved Hide resolved
@lidel lidel force-pushed the http-auth-secrets branch from 40e4eb8 to 25e8d3d Compare November 15, 2023 14:44
@hacdias hacdias requested a review from lidel November 16, 2023 10:53
@hacdias hacdias self-assigned this Nov 16, 2023
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @hacdias!

I've added some additional docs and negative tests, should be ready for 0.25.0-rc1.
CI is green, merging.

@lidel lidel merged commit 01cc5ea into master Nov 17, 2023
29 checks passed
@lidel lidel deleted the http-auth-secrets branch November 17, 2023 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: Opt-in API.HTTPAuthSecrets for securing Kubo RPC Secure/Authenticated API access API Tokens
3 participants