-
Notifications
You must be signed in to change notification settings - Fork 22
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: add static uploads API #148
Conversation
Deploying with Cloudflare Pages
|
does this closes #150 ? |
you need to add
to tsconfig.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is amazing ❤️ Learned so much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can can
Co-authored-by: Oli Evans <oli.evans@gmail.com>
Co-authored-by: Oli Evans <oli.evans@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful work ✨🎷🐩
…3protocol into feat/static-uploading-api
Co-authored-by: Oli Evans <oli.evans@gmail.com>
Co-authored-by: Oli Evans <oli.evans@gmail.com>
Co-authored-by: Oli Evans <oli.evans@gmail.com>
const proofs = await Promise.all([ | ||
StoreCapabilities.add.delegate({ | ||
issuer: account, | ||
audience: serviceSigner, | ||
with: account.did(), | ||
expiration: Infinity, | ||
}), | ||
UploadCapabilities.add.delegate({ | ||
issuer: account, | ||
audience: serviceSigner, | ||
with: account.did(), | ||
expiration: Infinity, | ||
}), | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work? what ties issuer + account together? why is issuer
allowed to invoke an upload with this proof chain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work? what ties issuer + account together?
I'm not sure I understand what you mean, here also what test does here is hopefully not what we do in the actual client. Idea is that space
should delegate all of it's capabilities, that is { can: *, with: space.did() }
to the service DID so that service in the future could delegate it back to the user.
Problem here is that it delegates just specific capabilities not all of them. That would imply that if user looses key we can delegate back only these two capabilities but nothing else.
why is
issuer
allowed to invoke an upload with this proof chain?
This is precisely what the ownership section here tries to address.
If with
DID is the same as iss
DID you don't need any proof because it's self-evident that you own that resource, otherwise how would you would not be able to issue such UCAN because you would not be able to sign without a private key.
P.S.: If you think there is a good / better place way to capture that let me know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean how does space tied to the agent then
- On space creation:
a. space delegates{ can: *, with: space.did() }
toagent.did()
- So agent can act on user behalf
b. space delegates{ can: *, with: space.did() }
toservice.did()
- So service can delegate to users other agent after email verification. Key recovery & rotation is the same flow.
c. space should delegate to user email as per During registration we should delegate from space to an email of the user #165 - On login to existing space
a. User types email address and agent asks service for the delegation to (local)agent.did()
b. Service delegates capabilities from 1. .b to requestingagent.did()
and sends an email with a link to import it. After clicking it agent imports delegation and now has access to the space.
As an aside we would be able to replace delegation form space -> service, with space to user email address so we never actually have user space capabilities, because with ucan-mailto will be able to do email to did delegations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my question was really "why is the issuer allowed to invoke store/add
on this space when the proof chain the issuer provides doesn't include a delegation from space to issuer".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test passes and i assumed it would fail as it is currently written.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that maybe just how it works, i'm just trying to clarify my concern around such a self evident proof looking like maybe it is dangerously powerful. Why is a proof chain that never addresses our signer called issuer
as the audience
for the capbailities usable by issuer
to prove they can do a thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm still not getting it! In this test we do this:
const account = await Signer.generate() const issuer = await Signer.generate() // The "user" that will ask the service to accept the upload const proofs = await Promise.all([ StoreCapabilities.add.delegate({ issuer: account, audience: serviceSigner, with: account.did(), expiration: Infinity, }), UploadCapabilities.add.delegate({ issuer: account, audience: serviceSigner, with: account.did(), expiration: Infinity, }), ]) // ... trim server and conn creation ... const dataCID = await uploadFile({ issuer, proofs }, file, { connection } )to my eyes that looks like we set up a proof that says
I don't know what uploadFile
does there but assuming it issues some invocation using proofs
as proofs it should not work. Issuer there should be an account
or delegations should change to delegate to issuer instead of serviceSigner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again it kind of depends what uploadFile
does it may be using issuer.did()
in with
field in which case proofs are pointless and validator won't look at them because issuer
could issue any capability as long as with
is issuer.did()
without proofs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that maybe just how it works, i'm just trying to clarify my concern around such a self evident proof looking like maybe it is dangerously powerful. Why is a proof chain that never addresses our signer called issuer as the audience for the capbailities usable by issuer to prove they can do a thing?
No if what happens under the hood looks roughly like the following, validator would have to say you're not authorized and if it does not I'm very surprised that such bug has escaped:
{
iss: 'did:key:iss'
aud: 'did:key:service'
att: [
{ can: 'store/add', with: 'did:key:acc', link: 'bag...car' },
],
proofs: [
{ iss: 'did:key:acc'
aud: 'did:key:service'
att: [{ can: "store/add", with: "did:key:acc" }]
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that what @olizilla was describing is actually what's in this test, I'm surprised it's passing. I have created test in ucanto to check for principal aligment storacha/ucanto#134
I'll continue investigating why this isn't failing
🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2022-11-16) ### Features * a base for a new web3.storage upload-client ([#141](#141)) ([9d4b5be](9d4b5be)) * add static uploads API ([#148](#148)) ([d85d051](d85d051)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](upload-client-v4.0.0...upload-client-v5.0.0) (2022-12-07) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * upload/list items have a shards array property ([#229](#229)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * rename callback ([#198](#198)) ### Features * [#153](#153) ([#177](#177)) ([d6d448c](d6d448c)) * a base for a new web3.storage upload-client ([#141](#141)) ([9d4b5be](9d4b5be)) * **access-client:** cli and recover ([#207](#207)) ([adb3a8d](adb3a8d)) * account recover with email ([#149](#149)) ([6c659ba](6c659ba)) * add static uploads API ([#148](#148)) ([d85d051](d85d051)) * allow custom shard size ([#185](#185)) ([0cada8a](0cada8a)) * explicit resource ([#181](#181)) ([785924b](785924b)) * follow up on the capabilities extract ([#239](#239)) ([ef5e779](ef5e779)) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([c182bbe](c182bbe)) * support pagination ([#204](#204)) ([a5296a6](a5296a6)), closes [#201](#201) * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) ([5e663d1](5e663d1)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) ([2f3bab8](2f3bab8)) * upload/list items have a shards array property ([#229](#229)) ([c00da35](c00da35)) ### Bug Fixes * export types ([c8d0a3f](c8d0a3f)) * list response results return type ([#206](#206)) ([4bca6c1](4bca6c1)) * store export ([#197](#197)) ([9b836e6](9b836e6)) * upload client list needs empty nb ([#194](#194)) ([ff7fbb8](ff7fbb8)) * uploadFile requires BlobLike not Blob ([1cc7681](1cc7681)) ### Code Refactoring * rename callback ([#198](#198)) ([5b8ef7d](5b8ef7d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR adds static methods allowing streaming uploads to the service. It is integrated with ucanto 0.9. Essentially the client is "bring your own agent" - it requires an `issuer` (a `Signer`) and `proofs` (delegated capabilities) to be passed to each method. The API is oriented around 3 use cases: 1. Simple and easy to use `uploadFile`/`uploadDirectory` methods that return CIDs. 2. More advanced APIs that split out DAG creation, CAR creation, storing and registering uploads. 3. Streaming APIs that have the same split, but cater for uploads of arbitrary size using streams. Every method has tests and the [README has extensive examples and API reference documentation](https://github.com/web3-storage/w3protocol/blob/777327609cd2c96b2be4e5e8d48e9a45f17b9a7b/packages/upload-client/README.md). closes #150 Co-authored-by: Oli Evans <oli.evans@gmail.com>
🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2022-11-16) ### Features * a base for a new web3.storage upload-client ([#141](#141)) ([72c08d7](72c08d7)) * add static uploads API ([#148](#148)) ([8b735fa](8b735fa)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](upload-client-v4.0.0...upload-client-v5.0.0) (2022-12-07) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * upload/list items have a shards array property ([#229](#229)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * rename callback ([#198](#198)) ### Features * [#153](#153) ([#177](#177)) ([7fbf2a1](7fbf2a1)) * a base for a new web3.storage upload-client ([#141](#141)) ([72c08d7](72c08d7)) * **access-client:** cli and recover ([#207](#207)) ([720dafb](720dafb)) * account recover with email ([#149](#149)) ([91ad47d](91ad47d)) * add static uploads API ([#148](#148)) ([8b735fa](8b735fa)) * allow custom shard size ([#185](#185)) ([2130405](2130405)) * explicit resource ([#181](#181)) ([c127431](c127431)) * follow up on the capabilities extract ([#239](#239)) ([717fcaa](717fcaa)) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([197439e](197439e)) * support pagination ([#204](#204)) ([363e3ac](363e3ac)), closes [#201](#201) * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) ([65d191c](65d191c)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) ([309aff0](309aff0)) * upload/list items have a shards array property ([#229](#229)) ([723b281](723b281)) ### Bug Fixes * export types ([c44a252](c44a252)) * list response results return type ([#206](#206)) ([e49c685](e49c685)) * store export ([#197](#197)) ([d2296c5](d2296c5)) * upload client list needs empty nb ([#194](#194)) ([7894fb9](7894fb9)) * uploadFile requires BlobLike not Blob ([56ec496](56ec496)) ### Code Refactoring * rename callback ([#198](#198)) ([2349763](2349763)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR adds static methods allowing streaming uploads to the service. It is integrated with ucanto 0.9.
Essentially the client is "bring your own agent" - it requires an
issuer
(aSigner
) andproofs
(delegated capabilities) to be passed to each method.The API is oriented around 3 use cases:
uploadFile
/uploadDirectory
methods that return CIDs.Every method has tests and the README has extensive examples and API reference documentation.
closes #150