feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works #460

gobengo · 2023-02-28T01:35:22Z

Motivation:

I can access/claim as did:key:{} #455
This implements the second delegation described in Succesfull access/authorize should delegate capabilities to the agent so it can claim them #457
make this test pass once we deploy this to staging Authorize then claim gobengo/w3protocol-test#6

Test Case:

https://github.com/gobengo/w3protocol-test/pull/6/files#diff-698e92d7467a4a470689d4cb120272c6cac28864d4960ac12a3720ab7c35a15cR364

…esult-in-space-being-registered-and-accessdelegate-for-space-allowed

…teAccessDelegateHandler

…laim works

Gozala

It's cool that more tests are added, but I'm afraid it's not going to do what you're aiming for.

Gozala · 2023-02-28T05:43:55Z

packages/access-api/src/routes/validate-email.js

+    // invocations invoked by the did:key
+    const delegateToKey = await ucanto.delegate({
+      issuer: env.signer,
+      audience: {


Audience of ./update capabilities is meant to be non did:key principal, because those are only times ucanto would look at ./update capabilities. Delegation to the agent key is not going to get you anything other than been able to claim that delegation from that agent afterwards.

ok. I updated to not use ./update. If what I updated to is even remotely tolerable on staging, it would be real nice to merge it because any delegation in there with the correct audience will make the tests pass.

gobengo · 2023-02-28T07:12:29Z

@Gozala after your last review e.g. this comment, I updated this to have clicking the confirmation email generate a ucan/attest as described in #457 .

wdyt?

Gozala

I think it's trying to do both old auth and new auth together but mixes two in a weird way.

Gozala · 2023-02-28T17:29:47Z

packages/access-api/src/routes/validate-email.js

+
+    const agentPubkey = accessSessionResult.capability.nb.key
+
+    const update = await ucanto.delegate({


It would really help to add comment above explaining what this delegation intends to do. In the version that I think this implements this delegation is attestation from us that account has authorized agent key to be able to sign ucans where issuer is account did.

Gozala · 2023-02-28T17:30:57Z

packages/access-api/src/routes/validate-email.js

+        },
+      ],
+    })
+    const attestKeyAuthorizesAccount = await ucanto.delegate({


Comment here describing what it's meant to do would also help, because it does not do what spec or issue suggests and it's not clear to me what the intentions are.

Gozala · 2023-02-28T17:34:43Z

packages/access-api/src/routes/validate-email.js

+          with: env.signer.did(),
+          can: 'ucan/attest',
+          nb: {
+            proof: update.cid,


This should be the CID for the (unsigned) delegation from an account to the agent. This delegation should be replacement for the update above. More concretely if update was attesting to the authorization of the key, this attestation should be for the authorization of the specific delegation.

That is the core change in the spec, if previously we were attesting to key authorization which implied sudo privileges, in a new version we attest to the specific delegation authorization. Which means it could be sudo privileges or a very specific capabilities.

Reading further down I think you could just remove this delegation.

this is what's needed to make this work https://github.com/web3-storage/w3protocol/pull/460/files#diff-0cebcdde20e55de2911e90a385f368240bd9109fc54b074b89759628b695e88bR202

Gozala · 2023-02-28T17:38:09Z

packages/access-api/src/routes/validate-email.js

+    })
+
+    // create delegations that should be claimable
+    const delegationAccountToKey = await ucanto.delegate({


This is somewhat correct, except it will have a signature from accountOneOffKey key as opposed to having an "absent" signature. Which may be ok temporarily, but not in MVP because those could fail to validate.

Gozala · 2023-02-28T17:39:49Z

packages/access-api/src/routes/validate-email.js

+    // generate a delegation to the key that we can save in
+    // models.delegations to be found by subsequent access/claim
+    // invocations invoked by the did:key
+    const delegateToKey = await ucanto.delegate({


Probably should be renamed and update comments as it does a different thing now.

renamed/recommented in 685af65

Gozala · 2023-02-28T17:41:39Z

packages/access-api/src/routes/validate-email.js

+          return agentPubkey
+        },
+      },
+      proofs: [delegationAccountToKey],


I'm not sure what is this for ? This could either contain delegation from did:web:web3.storage to a service key, or if they are the same no proof is required.

Gozala · 2023-02-28T17:43:15Z

packages/access-api/src/routes/validate-email.js

+    })
+    await env.models.delegations.putMany(
+      delegateToKey,
+      attestKeyAuthorizesAccount


I think you need to put delegationAccountToKey and delegateToKey instead.

gobengo · 2023-02-28T18:19:35Z

packages/access-api/test/access-authorize.test.js

+    assert.ok(expectAccountToKey, 'expect proofs to contain delegation')
+    assert.deepEqual(expectAccountToKey.issuer.did(), accountDID)
+    assert.deepEqual(expectAccountToKey.audience.did(), issuer.did())
+    assert.deepEqual(expectAccountToKey.capabilities, [


remove this whole delegation if it's not needed for this test

done in 685af65

…tion

gobengo · 2023-02-28T19:22:14Z

I think it's trying to do both old auth and new auth together but mixes two in a weird way.

I removed the new

packages/access-api/src/routes/validate-email.js

Gozala · 2023-02-28T20:34:44Z

packages/access-api/src/routes/validate-email.js

+      capabilities: [
+        {
+          with: env.signer.did(),
+          can: 'access-api/delegation',


Suggested change

can: 'access-api/delegation',

can: 'access/delegation'

Nit: It just occurred to me that you can use access/delegate here as spec-ed except with audience being an agent as opposed to service.

I'm down to change to that post-merge but otherwise I'd prefer to make this a meaningless can

Co-authored-by: Irakli Gozalishvili <[email protected]>

gobengo · 2023-03-01T18:38:14Z

when testing this on staging, I ran into an error due to #464

🤖 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).

…on back to the issuer did:key so that access/claim works (#460) Motivation: * #455 * This implements the second delegation described in #457 * make this test pass once we deploy this to staging gobengo/w3protocol-test#6 Test Case: * https://github.com/gobengo/w3protocol-test/pull/6/files#diff-698e92d7467a4a470689d4cb120272c6cac28864d4960ac12a3720ab7c35a15cR364 --------- Co-authored-by: Irakli Gozalishvili <[email protected]>

🤖 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).

add test for access/authorize then access/delegate

6db052a

gobengo linked an issue Feb 28, 2023 that may be closed by this pull request

successful voucher/claim+redeem flow should result in space being registered and access/delegate for space allowed #458

Closed

gobengo temporarily deployed to dev February 28, 2023 01:36 — with GitHub Actions Inactive

Merge branch 'main' into 458-successful-accessauthorize-flow-should-r…

24f9266

…esult-in-space-being-registered-and-accessdelegate-for-space-allowed

gobengo temporarily deployed to dev February 28, 2023 01:38 — with GitHub Actions Inactive

gobengo added 4 commits February 27, 2023 18:17

add testCanAccessDelegateWithRegisteredSpace against miniflare + crea…

94b1577

…teAccessDelegateHandler

add voucher/claim test that invokes the delegation in email

b92fad2

access/authorize email confirmation delegates back to key so access/c…

1735d15

…laim works

remove unused test parts

c951907

gobengo changed the title ~~space registration that allows access/delegate~~ feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works Feb 28, 2023

fix lint

a8cab05

gobengo marked this pull request as ready for review February 28, 2023 03:45

gobengo temporarily deployed to dev February 28, 2023 03:46 — with GitHub Actions Inactive

gobengo requested review from Gozala and alanshaw February 28, 2023 03:47

gobengo mentioned this pull request Feb 28, 2023

Succesfull access/authorize should delegate capabilities to the agent so it can claim them #457

Closed

Gozala requested changes Feb 28, 2023

View reviewed changes

access/authorize confirmation stores ucan/attest not ./update

1be596f

gobengo temporarily deployed to dev February 28, 2023 07:10 — with GitHub Actions Inactive

gobengo requested a review from Gozala February 28, 2023 07:11

access/authorize confirmation makes ./update available to agent did

013ac0d

gobengo temporarily deployed to dev February 28, 2023 08:54 — with GitHub Actions Inactive

fix ucan/attest nb.proof

722abde

gobengo temporarily deployed to dev February 28, 2023 09:06 — with GitHub Actions Inactive

assert account has no delegations after access/authorize confirmation

83141c4

gobengo temporarily deployed to dev February 28, 2023 09:35 — with GitHub Actions Inactive

Gozala reviewed Feb 28, 2023

View reviewed changes

gobengo commented Feb 28, 2023

View reviewed changes

confirming access/authorize email no longer stores ucan/attest delega…

685af65

…tion

gobengo temporarily deployed to dev February 28, 2023 19:14 — with GitHub Actions Inactive

gobengo requested a review from Gozala February 28, 2023 19:20

gobengo mentioned this pull request Feb 28, 2023

prove you can access/claim iss=did:mailto against staging gobengo/w3protocol-test#7

Closed

4 tasks

Gozala approved these changes Feb 28, 2023

View reviewed changes

Update packages/access-api/src/routes/validate-email.js

48f4744

Co-authored-by: Irakli Gozalishvili <[email protected]>

gobengo temporarily deployed to dev February 28, 2023 21:22 — with GitHub Actions Inactive

gobengo merged commit a466a7d into main Feb 28, 2023

gobengo deleted the 458-successful-accessauthorize-flow-should-result-in-space-being-registered-and-accessdelegate-for-space-allowed branch February 28, 2023 21:24

it-dag-house mentioned this pull request Feb 28, 2023

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

Closed

gobengo mentioned this pull request Mar 1, 2023

run migration 0005 on access-api staging #464

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works #460

feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works #460

gobengo commented Feb 28, 2023 •

edited

Loading

Gozala left a comment

Gozala Feb 28, 2023

gobengo Feb 28, 2023

gobengo commented Feb 28, 2023

Gozala left a comment

Gozala Feb 28, 2023

Gozala Feb 28, 2023

Gozala Feb 28, 2023

Gozala Feb 28, 2023

gobengo Feb 28, 2023

Gozala Feb 28, 2023

Gozala Feb 28, 2023

gobengo Feb 28, 2023

Gozala Feb 28, 2023

Gozala Feb 28, 2023

gobengo Feb 28, 2023

gobengo Feb 28, 2023

gobengo commented Feb 28, 2023 •

edited

Loading

Gozala Feb 28, 2023

gobengo Feb 28, 2023

gobengo commented Mar 1, 2023


		const agentPubkey = accessSessionResult.capability.nb.key

		const update = await ucanto.delegate({

feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works #460

feat: access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works #460

Conversation

gobengo commented Feb 28, 2023 • edited Loading

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

gobengo commented Feb 28, 2023

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

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

Choose a reason for hiding this comment

gobengo commented Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gobengo commented Mar 1, 2023

gobengo commented Feb 28, 2023 •

edited

Loading

gobengo commented Feb 28, 2023 •

edited

Loading