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: HTTP to UCAN bridge #325

Merged
merged 45 commits into from
Mar 5, 2024
Merged

feat: HTTP to UCAN bridge #325

merged 45 commits into from
Mar 5, 2024

Conversation

travis
Copy link
Member

@travis travis commented Feb 7, 2024

To support users in languages that do not have existing UCAN invocation implementations, we are going to launch a bridge that allows them to make simple HTTP requests with JSON bodies that we transform into proper UCAN invocations.

This follows the specification here: storacha/specs#112

Values for authorization headers can be generated using the bridge generate-tokens w3cli command proposed here:

storacha/w3cli#175

travis added a commit to storacha/w3cli that referenced this pull request Feb 7, 2024
In service of the UCAN bridge implementation in storacha/w3infra#325 this PR proposes a new `bridge generate-tokens` command that will generate values for the `Authorization` header and `proof` field of the HTTP request users make to the bridge.

This PR is currently intended as a stake-in-the-ground and is expected to change significantly.

TODO
- [ ] clean up and refactor bridge
- [ ] finalize command names
- [ ] decouple from coupon code (?)
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

❤️ love it - thanks for pushing this!

Worth mentioning what you're doing is storacha/RFC#1 (comment)

Can you please define response format?

upload-api/BRIDGE.md Outdated Show resolved Hide resolved
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
@gobengo
Copy link
Contributor

gobengo commented Feb 7, 2024

The proof field of the JSON body is a base64pad encoded

Originally I read this as meaning it was base64 encoded with padding. https://datatracker.ietf.org/doc/html/rfc4648#section-4

It could be good to clarify you mean it is multibase encoded using the multibase encoding for base64+padding. https://www.ietf.org/archive/id/draft-multiformats-multibase-07.html#name-base-64-with-padding-and-mi

@travis
Copy link
Member Author

travis commented Feb 7, 2024

❤️ love it - thanks for pushing this!

<3

Worth mentioning what you're doing is web3-storage/RFC#1 (comment)

Yes! Though there's a pretty big difference in the authorization - I'm not "passing UCAN delegation as JWT bearer token" because of the 8KB limit I mention in the thread with Ben above, instead passing the secret there and embedding the delegation in the JSON body.

Can you please define response format?

Yep definitely - added a TODO!

To support users in languages that do not have existing UCAN invocation implementations, we are going to launch a bridge that allows them to make simple HTTP requests with JSON bodies that we transform into proper UCAN invocations.

So far this PR has:

1) an untested (but type-checking!) implementation of such a bridge
2) a markdown description of the bridge protocol, intended to be the first draft of an eventual specification

Notable design choices:

1) I chose to include JUST the base64pad-encoded "secret" in the Authorization header to avoid running afoul of maximum header size restrictions that exist in [some HTTP environments](https://stackoverflow.com/questions/686217/maximum-on-http-header-values).
2) The `proof` field of the JSON body is a base64pad encoded "delegation archive" (created with `ucanto`'s `Delegation.archive` function)

TODO
- [ ] factor core bridge logic out to a separate library
- [ ] factor HTTP input wrangling out to a separate function
- [ ] rename `UPLOAD_API_DID` and `ACCESS_SERVICE_URL` environment variables to `W3UP_SERVICE_DID` and `W3UP_SERVICE_URL`
- [ ] add tests
- [ ] expand and formalize bridge specification, move it to the specs repo (?)
Copy link

seed-deploy bot commented Feb 9, 2024

View stack outputs

update spec to use https:// github.com/ucan-wg/ucan-http-bearer-token?tab=readme-ov-file#ucan-as-bearer-token-specification-v030 and move the secret to a new `X-Auth-Secret` header

update implementation per this spec - currently typechecks cleanly but throws an error when I actually try to invoke:

```
TypeError: proof.export is not a function or its return value is not iterable
    at delegate (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/delegation.js:466:33)
    at IssuedInvocation.buildIPLDView (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/invocation.js:101:12)
    at writeInvocations (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/message.js:114:35)
    at MessageBuilder.buildIPLDView (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/message.js:79:52)
    at Object.build (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/message.js:30:49)
    at execute (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/client/src/connection.js:48:31)
    at Connection.execute (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/client/src/connection.js:35:12)
    at IssuedInvocation.execute (/Users/travis/dev/pl/w3infra/node_modules/@ucanto/core/src/invocation.js:115:39)
    at handlerFn (/Users/travis/dev/pl/w3infra/upload-api/functions/bridge.js:124:38)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
```

I think this is because the `@ipld/dag-ucan` `Ucan.View` is not a full `Delegation`
Gozala
Gozala previously requested changes Feb 12, 2024
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
upload-api/BRIDGE.md Outdated Show resolved Hide resolved
upload-api/functions/bridge.js Outdated Show resolved Hide resolved
upload-api/functions/bridge.js Outdated Show resolved Hide resolved
upload-api/functions/bridge.js Outdated Show resolved Hide resolved
upload-api/functions/bridge.js Outdated Show resolved Hide resolved
upload-api/functions/bridge.js Outdated Show resolved Hide resolved
worked with @gobengo and @Gozala to refine the design on this bridge - this is roughly the latest thinking, missing just a few things from PR reviews
1) add content negotiation and support CBOR
2) better parsing abstractions
3) validate tasks before casting

one slightly weird thing I'm doing is converting the string I get from the AWS lambda to a ReadableStream and then reading it back again - seems silly in this implementation, but is necessary because I plan to pull out the abstraction and support other deployment environments like Cloudflare workers or Node's `fetch` API.
@travis travis changed the title wip: first draft of UCAN bridge feat: HTTP to UCAN bridge Feb 22, 2024
also error on unsupported content type
The bridge was configured to point at staging even when it was running in a PR environment, because we don't customize the `ACCESS_SERVICE_URL` in those environments.

It would be nice to figure out how to set the proper environment variables in different environments, but I suspect the right way to do that might change with the SST upgrade, so for now let's just add a function that encodes the rules we know, since these aren't likely to change soon and would require some code changes anyway.
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM

message: `${contentType} is not supported`
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Try/catch around this incase decode throws?

Comment on lines 147 to 148
p: receipt.root.data?.ocm,
s: receipt.root.data?.sig
Copy link
Member

Choose a reason for hiding this comment

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

I hope typescript understands that you just checked for existence?

Suggested change
p: receipt.root.data?.ocm,
s: receipt.root.data?.sig
p: receipt.root.data.ocm,
s: receipt.root.data.sig

if (!authorizationHeader) {
return {
statusCode: 401,
body: 'request has no Authorization header'
Copy link
Member

Choose a reason for hiding this comment

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

I like to say "missing" rather than "request has no" 😬

return bridgeReceipts.ok ? {
statusCode: 200,
headers: {
"Content-Type": "application/vnd.ipld.dag-json"
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if these had a content type export like CAR...how do you feel about sending a couple of PRs?

Copy link
Member Author

Choose a reason for hiding this comment

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

oo I like that - will dig into that today!

travis added a commit to storacha/w3cli that referenced this pull request Mar 5, 2024
In service of the UCAN bridge implementation in
storacha/w3infra#325 this PR introduces a
new `bridge generate-tokens` command that will generate values for the
`X-Auth-Secret` and `Authorization` headers of the HTTP request users
make to the bridge.
@travis travis dismissed Gozala’s stale review March 5, 2024 18:50

addressed all feedback

@seed-deploy seed-deploy bot temporarily deployed to pr325 March 5, 2024 19:00 Inactive
@travis travis merged commit f5a092a into main Mar 5, 2024
3 checks passed
@travis travis deleted the feat/ucan-bridge branch March 5, 2024 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants