-
Notifications
You must be signed in to change notification settings - Fork 23
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: @web3-storage/capabilities depends on latest ucanto #541
feat: @web3-storage/capabilities depends on latest ucanto #541
Conversation
@@ -173,7 +173,7 @@ async function authorize(req, env) { | |||
}) | |||
if (confirmResult.error) { |
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 now errors. The message is
error in validate-email Error: error confirming
at authorize3 (/Users/bengo/protocol.ai/w3protocol/packages/access-api/src/routes/validate-email.js:175:13)
at Router.fetch (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@web3-storage/worker-utils/src/router.js:141:15)
... 2 lines matching cause stack trace ...
at Context.<anonymous> (file:///Users/bengo/protocol.ai/w3protocol/packages/access-api/test/access-authorize.test.js:124:17) {
[cause]: InvalidAudience: Expected literal "did:web:test.web3.storage" instead got "did:mailto:dag.house:email"
at confirm (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@ucanto/server/src/handler.js:54:14)
at authorize3 (/Users/bengo/protocol.ai/w3protocol/packages/access-api/src/routes/validate-email.js:170:33)
... 3 lines matching cause stack trace ...
at Context.<anonymous> (file:///Users/bengo/protocol.ai/w3protocol/packages/access-api/test/access-authorize.test.js:124:17) {
cause: LiteralError: Expected literal "did:web:test.web3.storage" instead got "did:mailto:dag.house:email"
at Literal.readWith (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@ucanto/validator/src/schema/schema.js:1018:9)
at Literal.read (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@ucanto/validator/src/schema/schema.js:40:17)
at confirm (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@ucanto/server/src/handler.js:52:35)
at authorize3 (/Users/bengo/protocol.ai/w3protocol/packages/access-api/src/routes/validate-email.js:170:33)
at Router.fetch (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@web3-storage/worker-utils/src/router.js:141:15)
at Object.fetch (/Users/bengo/protocol.ai/w3protocol/packages/access-api/src/index.js:29:19)
at ServiceWorkerGlobalScope.[kDispatchFetch] (/Users/bengo/protocol.ai/w3protocol/node_modules/.pnpm/@[email protected]/node_modules/@miniflare/core/src/standards/event.ts:385:13)
at Context.<anonymous> (file:///Users/bengo/protocol.ai/w3protocol/packages/access-api/test/access-authorize.test.js:124:17) {
expect: 'did:web:test.web3.storage',
actual: 'did:mailto:dag.house:email'
}
}
}
I think this makes sense.... the invocation has audience
of the did:mailto:...
but here I pass id: env.signer.verifier
. I expected the pre-signed invocation would be addressed to us, not the did:mailto
.
I could:
- change this call to
confirm
to passid: theDidMailto
- change the
access/confirm
invocation created byaccess/authorize
handler to useaudience: service
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.
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.
@Gozala any advice on which of the above paths makes sense? or if I 'm missing something?
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.
change this call to confirm to pass id: theDidMailto
Actually I can't do that. id
must be a Verifier, and idk if there is a good Verifier when audience is did:mailto
.
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 found a way of doing with provideAdvanced 6045824#diff-dc3f2f4fda92c2be86974bff5c145103773f6c7cb68d879d52d464392702243dR160
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 not sure I have full context why you chose to switch to provideAdvanced
here. Given how it's used here I would guess that regular provide
is what you want.
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.
after the upgrade, provide
validation errored (iirc InvalidAudience
) on the access/confirm invocation because its aud
was did:mailto
.
@@ -69,85 +69,4 @@ describe('proxy store/list invocations to upload-api', function () { | |||
assert.ok(!result?.error, 'should not be an error') | |||
}) | |||
} | |||
|
|||
it('errors when a bad delegation is given as proof', async () => { |
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.
When I wrote this test, I think it only passed because of the ucanto audience check.
This test no longer makes sense because the final way we did the upload-api proxying doesn't do any signature verification here in access-api, it just proxies to w3infra upload-api and does the validation there.
But this test stubbed that out in a way that didn't even do that, so there was no reason for it to pass.
@@ -75,26 +75,6 @@ describe('ucan', function () { | |||
]) | |||
}) | |||
|
|||
test('should fail with bad service audience', async function () { |
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 tests a part of the api that isn't important.
I also think after https://github.com/web3-storage/ucanto/pull/257/files that ucanto isn't doing this checking for us.
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.
It moved that check into the provider(handler)
decorator.
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.
Looks fine. I'm not sure switch to provideAdvanced
makes sense here, but it also doesn't really harm anything as under the hood provider
does pretty much the same thing as the Schema.literal(service.id)
here.
@@ -173,7 +173,7 @@ async function authorize(req, env) { | |||
}) | |||
if (confirmResult.error) { |
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 not sure I have full context why you chose to switch to provideAdvanced
here. Given how it's used here I would guess that regular provide
is what you want.
…countDID, and access-confirm handler verifies that
@@ -25,7 +24,7 @@ export function accessAuthorizeProvider(ctx) { | |||
const confirmation = await Access.confirm | |||
.invoke({ | |||
issuer: ctx.signer, | |||
audience: DID.parse(capability.nb.iss), | |||
audience: ctx.signer, |
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.
Maybe add a comment so future us aren’t confused why
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.
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.
Motivation: * do the upgrade so that we can merge #539 after (which requires the upgrade)
With this PR we're able to use two different devices on behalf of a single account identified by an email address. An agent (ie, a device like w3console or w3cli) can now: 1) use `access/authorize` to trigger an email verification flow that will give them delegations to act on behalf of an account 2) create a space locally 3) add a storage provider to that space with `provider/add` 4) delegate capabilities to the account they are authorized as that permit the account to delegate all capabilities on those spaces to other agents - in other words, create spaces and assign all "permissions" on those spaces to their account 5) upload data to the space A second agent (ie, another device) can then: 1) use `access/authorize` to trigger an email verification flow that will give them delegations to act on behalf of the same account 2) get a list of spaces they can store data in, which includes the space created on the first device 3) upload data to the space This PR also contains various refactoring of the `Agent` class to minimize its responsibilities and move in the direction of letting user agents take responsibility for state storage. refs #395 * [x] setup tests for access-client agent + access-api * [x] simple test agent createSpace * [x] @gobengo test agent authorize happy path #535 * [x] @gobengo upgrade to ucanto 6.2 #541 * [x] @travis ensure what's proposed here can work in w3up-client, w3ui, w3console * [x] upgrade this branch to `@ucanto/[email protected]` after storacha/ucanto#261 * [x] minimize new public api surface area on access-client Agent * [x] (e.g. `sessionProof`) https://github.com/web3-storage/w3protocol/pull/545/files * [x] `sessionPrincipal` #546 * [x] review comments * [x] `authorize` should access/claim `with=did:mailto:...` https://github.com/web3-storage/w3protocol/pull/556/files# --------- Co-authored-by: Travis Vachon <[email protected]> Co-authored-by: Benjamin Goering <[email protected]> Co-authored-by: Irakli Gozalishvili <[email protected]>
With this PR we're able to use two different devices on behalf of a single account identified by an email address. An agent (ie, a device like w3console or w3cli) can now: 1) use `access/authorize` to trigger an email verification flow that will give them delegations to act on behalf of an account 2) create a space locally 3) add a storage provider to that space with `provider/add` 4) delegate capabilities to the account they are authorized as that permit the account to delegate all capabilities on those spaces to other agents - in other words, create spaces and assign all "permissions" on those spaces to their account 5) upload data to the space A second agent (ie, another device) can then: 1) use `access/authorize` to trigger an email verification flow that will give them delegations to act on behalf of the same account 2) get a list of spaces they can store data in, which includes the space created on the first device 3) upload data to the space This PR also contains various refactoring of the `Agent` class to minimize its responsibilities and move in the direction of letting user agents take responsibility for state storage. refs #395 * [x] setup tests for access-client agent + access-api * [x] simple test agent createSpace * [x] @gobengo test agent authorize happy path #535 * [x] @gobengo upgrade to ucanto 6.2 #541 * [x] @travis ensure what's proposed here can work in w3up-client, w3ui, w3console * [x] upgrade this branch to `@ucanto/[email protected]` after storacha/ucanto#261 * [x] minimize new public api surface area on access-client Agent * [x] (e.g. `sessionProof`) https://github.com/web3-storage/w3protocol/pull/545/files * [x] `sessionPrincipal` #546 * [x] review comments * [x] `authorize` should access/claim `with=did:mailto:...` https://github.com/web3-storage/w3protocol/pull/556/files# --------- Co-authored-by: Travis Vachon <[email protected]> Co-authored-by: Benjamin Goering <[email protected]> Co-authored-by: Irakli Gozalishvili <[email protected]>
Motivation: * do the upgrade so that we can merge #539 after (which requires the upgrade)
🤖 I have created a release *beep* *boop* --- ## [5.0.0](storacha/w3ui@vue-uploader-v4.2.0...vue-uploader-v5.0.0) (2023-07-25) ### ⚠ BREAKING CHANGES * upgrade access client, ucanto and other packages ([storacha#530](storacha/w3ui#530)) ### Features * upgrade access client, ucanto and other packages ([storacha#530](storacha/w3ui#530)) ([8e7321b](storacha/w3ui@8e7321b)) --- 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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](storacha/w3ui@vue-uploader-v4.2.0...vue-uploader-v5.0.0) (2023-07-25) ### ⚠ BREAKING CHANGES * upgrade access client, ucanto and other packages ([storacha#530](storacha/w3ui#530)) ### Features * upgrade access client, ucanto and other packages ([storacha#530](storacha/w3ui#530)) ([4449018](storacha/w3ui@4449018)) --- 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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
Motivation:
canDelegateCapability
and implement space registration #539 after (which requires the upgrade)