feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ #449

travis · 2023-02-24T08:44:10Z

Introduce a Cloudflare Durable Objects-based space verification workflow.

Rather than stashing a UCAN in KV and polling inside the Websocket handler until eventual consistency makes it available to the registration process in the CLI or w3console, we use a Durable Object to create a direct connection between the Websocket and the HTTP POST that provides the delegation.

It feels most natural to create a Durable Object per space and forward the Websocket request on to the Durable Object and let it handle it entirely. This necessitates adding the space DID information to the HTTP path, which means adding a new endpoint and will require clients to be updated in the wild before we get rid of the KV flow entirely. We should remove KV-based verification once this has rolled out to all of our users.

TODO

write tests
make sure error handling makes sense

packages/access-api/src/routes/validate-ws-did.js

packages/access-api/src/models/validations.js

packages/access-api/src/durable-objects/space-verifier.js

- logic for finding and sending delegation to SpaceVerifier to a module function - make SpaceVerifier feel a bit more Functional and avoid storing state unless we need to - cleanup state after sending delegation - reject with an HTTP 409 (Conflict) if a second user tries to connect to the SpaceVerifier websocket - error message and HTTP status tweaks

…' into feat/durable-object-verification

it would be good to test this a bit more substantially, but this flow is not long for this world so this is probably good enough for now

travis · 2023-03-03T08:13:34Z

OK this is good to go! caught up with @gobengo about how this intersects with the work he and @Gozala are going and his recommendation was to get this shipped. I do think this whole flow will be deprecated soon so I haven't bothered with extensive testing or refactoring tests, but did add one test similar to the access/authorize test that makes sure the happy path (ie, user opens websocket, clicks email, delegation is sent through websocket) works.

My understanding is that with the intention for the access/authorize based work that's in progress is to make that transport agnostic (ie, purely based on ucanto RPC), which will make it slightly trickier to do fancy transport stuff like we're doing with this websocket. After chatting with @gobengo I'm going to do a little research and see what it might take to implement ucanto channel that uses a websocket - that would be the ideal followup to this work and would let us get this same great user experience with that work.

tldr, ready for re-review! this should all be backwards compatible with existing clients so I think we can ship it at will.

gobengo · 2023-03-10T03:23:16Z

packages/access-api/src/durable-objects/space-verifier.js

+  /**
+   * @param {Request} req
+   */
+  async fetch(req) {


I would expect to see an authorization check somewhere to make we don't give the ucan to someone who shouldn't have it?

If someone else tries to connects first before the client we expect, what would happen? (It's been a long day, but I read it like that would deny service to the desired user)

I wonder if (long term) maybe this should be like AgentVerifier and be keyed off of the agent did, not space did.
(or agent did + cid(authorizationRequest) even).

It seems like keying this off of space and only allowing one server/ucan at a time will prevent more than one of these flows happening at a time for a space - not really a problem for voucher but it makes this less useful with new access/authorize + access/claim flow which is not 1:1 with a single space.
(we dont have to optimize in this PR for the new access stuff, but this will come up in #395 or #433)

I would expect to see an authorization check somewhere to make we don't give the ucan to someone who shouldn't have it?

hm, I would probably handle this further up the stack, but also I'm not sure we do this with the existing implementation and if we do I think we're still doing that for this flow?

tbh I'm not entirely sure whether we need to do additional auth here or not, but I'm not sure what that would look like concretely - do you have some sort of signature verification in mind?

I wonder if (long term) maybe this should be like AgentVerifier and be keyed off of the agent did, not space did.
(or agent did + cid(authorizationRequest) even).

I think this is exactly the direction we should go with the new stuff coming in yep

It seems like keying this off of space and only allowing one server/ucan at a time will prevent more than one of these flows happening at a time for a space - not really a problem for voucher but it makes this less useful with new access/authorize + access/claim flow which is not 1:1 with a single space.

yep - keying off Agent will be the way to go for the path that handles the access/authorize flow

I think invocation CID might be better yet as agent in theory might start multiple authorization flows or maybe we'll want to deliver other messages over websocket.

hm I don't totally follow, but seems reasonable as long as both sides of the equation (ie, the code calling registerSpace and the code handling the request from the email link) can both calculate the invocation CID

gobengo

I don't think this introduces any new issues, but it does definitely improve the perf of the voucher websocket

Gozala

Thanks for improving things. I think it's good as is, yet I would really appreciate some code comments because without been familiar with these code paths I struggled to follow along. Maybe some comments could be added in a followup change ?

Gozala · 2023-03-11T00:29:20Z

packages/access-api/src/durable-objects/space-verifier.js

+  // hostname is totally ignored by the durable object but must be set so set it to example.com
+  const response = await durableObject.fetch('https://example.com/delegation', {
+    method: 'PUT',
+    body: ucan,


Does body need to be a string or can we pass binary data ? If we could pass binary it might be a better option.

ah good question, I suspect binary would work? noted and a good change for the next version of this I think

Gozala · 2023-03-11T00:33:31Z

packages/access-api/src/durable-objects/space-verifier.js

+  /**
+   * @param {Request} req
+   */
+  async fetch(req) {


I think invocation CID might be better yet as agent in theory might start multiple authorization flows or maybe we'll want to deliver other messages over websocket.

Gozala · 2023-03-11T00:42:23Z

packages/access-api/src/service/voucher-claim.js

+    // For testing
+    if (ctx.config.ENV === 'test') {
+      return encoded
+    }


Nit: Is this really necessary now hat we can provide debug email thing for tests ? Because ucan is already in the query param. I have changed it in other placed, must have missed it here.

ah good question and I think no - I had originally been planning to rework the tests to get rid of this, but paused on that because we'll probably end up porting all of this over to the new session stuff, and it will make sense to rework tests and generally clean all of this up at that point.

travis · 2023-03-13T22:21:44Z

Thanks for improving things. I think it's good as is, yet I would really appreciate some code comments because without been familiar with these code paths I struggled to follow along. Maybe some comments could be added in a followup change ?

thank youuuuu - yes, I think this all needs a bunch of cleanup, and more than that needs to be ported over to the new session based stuff - I think most of your comments make sense to implement as part of that next tranche of work, but that may be coming very soon as I'm quite annoyed by having to wait for my account authorization in the new code paths ;-p

…ast ⏩ fast ⏩ fast ⏩ (#449)" This reverts commit 02d7552.

… it ⏩ fast ⏩ fast ⏩ fast ⏩ " (#547) Reverts #449 This is causing problems in staging - I think it's mainly a configuration issue ("Cannot read properties of undefined (reading 'idFromName')" indicates that the SpaceVerifier isn't set up in staging) but don't want to debug this right now while we're sprinting on account stuff.

🤖 I have created a release *beep* *boop* --- ## [11.0.0-rc.0](access-v10.0.0...access-v11.0.0-rc.0) (2023-03-20) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) ### Features * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([b1bbc90](b1bbc90)) * implement new account-based multi-device flow ([#433](#433)) ([1ddc6a0](1ddc6a0)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([02d7552](02d7552)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([ea4e872](ea4e872)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([be4386d](be4386d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([5fb56f7](5fb56f7)) * access-api serves access/claim invocations ([#456](#456)) ([baacf35](baacf35)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([a466a7d](a466a7d)) * allow multiple providers ([#595](#595)) ([96c5a2e](96c5a2e)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([b1bbc90](b1bbc90)) * handle access/delegate invocations without error ([#427](#427)) ([4f0bd1c](4f0bd1c)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([d0df525](d0df525)) * implement new account-based multi-device flow ([#433](#433)) ([1ddc6a0](1ddc6a0)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([5144293](5144293)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([4510c9a](4510c9a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([02d7552](02d7552)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([6a72855](6a72855)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([ea4e872](ea4e872)) * upgrade capabilities to latest ucanto ([#463](#463)) ([2d786ee](2d786ee)) * upgrade to new ucanto ([#498](#498)) ([dcb41a9](dcb41a9)) * write invocations and receipts into ucan log ([#592](#592)) ([754bf52](754bf52)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([f4c640d](f4c640d)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([a205ad1](a205ad1)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([6c8242d](6c8242d)) * allow injecting email ([#466](#466)) ([e19847f](e19847f)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([a6dafcb](a6dafcb)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([c1fea63](c1fea63)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([030e7b7](030e7b7)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([be4386d](be4386d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…st ⏩ fast ⏩ (#449) Introduce a [Cloudflare Durable Objects](https://developers.cloudflare.com/workers/runtime-apis/durable-objects/)-based space verification workflow. Rather than stashing a UCAN in KV and polling inside the Websocket handler until eventual consistency makes it available to the registration process in the CLI or w3console, we use a Durable Object to create a direct connection between the Websocket and the HTTP POST that provides the delegation. It feels most natural to create a Durable Object per space and forward the Websocket request on to the Durable Object and let it handle it entirely. This necessitates adding the space DID information to the HTTP path, which means adding a new endpoint and will require clients to be updated in the wild before we get rid of the KV flow entirely. We should remove KV-based verification once this has rolled out to all of our users. TODO - [x] write tests - [ ] make sure error handling makes sense --------- Co-authored-by: Benjamin Goering <171782+gobengo@users.noreply.github.com>

… it ⏩ fast ⏩ fast ⏩ fast ⏩ " (#547) Reverts #449 This is causing problems in staging - I think it's mainly a configuration issue ("Cannot read properties of undefined (reading 'idFromName')" indicates that the SpaceVerifier isn't set up in staging) but don't want to debug this right now while we're sprinting on account stuff.

🤖 I have created a release *beep* *boop* --- ## [11.0.0-rc.0](access-v10.0.0...access-v11.0.0-rc.0) (2023-03-20) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) ### Features * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([a08b513](a08b513)) * implement new account-based multi-device flow ([#433](#433)) ([6152e55](6152e55)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([3868d97](3868d97)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([362024f](362024f)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([29daa02](29daa02)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([46da0df](46da0df)) * access-api serves access/claim invocations ([#456](#456)) ([2ec16e9](2ec16e9)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([fc62691](fc62691)) * allow multiple providers ([#595](#595)) ([aba57b3](aba57b3)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([a08b513](a08b513)) * handle access/delegate invocations without error ([#427](#427)) ([db01d07](db01d07)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([ab83b19](ab83b19)) * implement new account-based multi-device flow ([#433](#433)) ([6152e55](6152e55)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([743a72f](743a72f)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([3029e4a](3029e4a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([3868d97](3868d97)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([4cd6cd9](4cd6cd9)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([362024f](362024f)) * upgrade capabilities to latest ucanto ([#463](#463)) ([e375ae4](e375ae4)) * upgrade to new ucanto ([#498](#498)) ([790750d](790750d)) * write invocations and receipts into ucan log ([#592](#592)) ([d52a281](d52a281)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([1d3d562](1d3d562)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([d90825a](d90825a)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([89f2acd](89f2acd)) * allow injecting email ([#466](#466)) ([b4b0173](b4b0173)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([660f773](660f773)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([8c6dea8](8c6dea8)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([02c0c28](02c0c28)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([29daa02](29daa02)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

wip: prototype of durable object validation

dae84aa

travis requested review from gobengo, alanshaw, olizilla and Gozala February 24, 2023 08:44

alanshaw requested changes Feb 24, 2023

View reviewed changes

travis and others added 2 commits March 1, 2023 17:00

Merge branch 'main' into feat/durable-object-verification

2f34b29

travis marked this pull request as ready for review March 1, 2023 09:00

travis changed the title ~~wip: prototype of Durable Object-based validation~~ feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ Mar 1, 2023

travis and others added 6 commits March 2, 2023 13:38

chore: make linter and tsc happy

8ad6936

Merge remote-tracking branch 'origin/feat/durable-object-verification…

297442a

…' into feat/durable-object-verification

chore: one more linter fix

569bb88

Merge branch 'main' into feat/durable-object-verification

02c6643

chore: ok really, seriously, linter?

330c8ac

Merge remote-tracking branch 'origin/feat/durable-object-verification…

98dc6d8

…' into feat/durable-object-verification

travis temporarily deployed to dev March 2, 2023 07:23 — with GitHub Actions Inactive

chore: add test case for new durable object websocket flow

44e3599

it would be good to test this a bit more substantially, but this flow is not long for this world so this is probably good enough for now

travis temporarily deployed to dev March 3, 2023 07:49 — with GitHub Actions Inactive

Merge branch 'main' into feat/durable-object-verification

ed348e2

travis temporarily deployed to dev March 3, 2023 07:54 — with GitHub Actions Inactive

travis requested a review from alanshaw March 3, 2023 08:08

heyjay44 mentioned this pull request Mar 3, 2023

Move validation websocket logic to a durable object #293

Closed

heyjay44 linked an issue Mar 3, 2023 that may be closed by this pull request

Move validation websocket logic to a durable object #293

Closed

gobengo reviewed Mar 10, 2023

View reviewed changes

Merge branch 'main' into feat/durable-object-verification

db4638d

gobengo self-requested a review March 10, 2023 23:55

gobengo approved these changes Mar 10, 2023

View reviewed changes

Gozala approved these changes Mar 11, 2023

View reviewed changes

Merge branch 'main' into feat/durable-object-verification

0a6f5c8

alanshaw approved these changes Mar 14, 2023

View reviewed changes

gobengo merged commit 02d7552 into main Mar 14, 2023

gobengo deleted the feat/durable-object-verification branch March 14, 2023 21:27

This was referenced Mar 14, 2023

chore(main): release access 11.0.0-rc.0 #511

Merged

chore(main): release access-api 11.0.0-rc.0 #453

Closed

travis added a commit that referenced this pull request Mar 15, 2023

Revert "feat: move validation flow to a Durable Object to make it ⏩ f…

84a7fb5

…ast ⏩ fast ⏩ fast ⏩ (#449)" This reverts commit 02d7552.

travis mentioned this pull request Mar 15, 2023

chore: Revert "feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ " #547

Merged

This was referenced Mar 21, 2023

chore(main): release access-api 11.0.0-rc.0 #584

Closed

chore(main): release access-api 5.0.0 #588

Merged

it-dag-house mentioned this pull request Mar 23, 2023

chore(main): release access-api 11.0.0-rc.0 #620

Closed

gobengo mentioned this pull request Mar 24, 2023

w3 authorize should detect authz confirmation within 5 second time budget storacha/w3cli#67

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ #449

feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ #449

travis commented Feb 24, 2023 •

edited

Loading

travis commented Mar 3, 2023

gobengo Mar 10, 2023 •

edited

Loading

travis Mar 10, 2023

travis Mar 10, 2023

travis Mar 10, 2023

Gozala Mar 11, 2023

travis Mar 13, 2023

gobengo left a comment

Gozala left a comment

Gozala Mar 11, 2023

travis Mar 13, 2023

Gozala Mar 11, 2023

Gozala Mar 11, 2023

travis Mar 13, 2023

travis commented Mar 13, 2023

feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ #449

feat: move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ #449

Conversation

travis commented Feb 24, 2023 • edited Loading

travis commented Mar 3, 2023

gobengo Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gobengo left a comment

Choose a reason for hiding this comment

Gozala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travis commented Mar 13, 2023

travis commented Feb 24, 2023 •

edited

Loading

gobengo Mar 10, 2023 •

edited

Loading