feat: add support for access/authorize and update #392

hugomrdias · 2023-01-25T10:32:43Z

closes #386

sequenceDiagram
	autonumber
	actor Alice
  participant Web3
  Alice->>+Web3: access/authorize
  par Web3 responds to Alice
      Web3->>Alice: Ok
  and Web3 sends Alice email
      Web3--)-Alice: ./update
  end
	Alice --) Web3: send DID (ws)
	Web3 -->> Web3: Polls KV for DID
	Alice --) Alice: Click email
	Alice ->>+ Web3: ./update
	Web3 -->> Web3: saves ./update
	Note right of Web3: KV put [AliceDID, ./update, 5mins]
	Web3 ->>-Alice: HTML with ./update
	Note right of Web3: 5. finds Alice DID
	Web3 --) Alice: sends ./update (ws from 4.)

alanshaw

🚀🚀🚀

alanshaw · 2023-01-25T14:54:54Z

packages/access-api/migrations/0004_add_accounts_table.sql

+        bytes BLOB NOT NULL,
+        audience TEXT NOT NULL,
+        issuer TEXT NOT NULL,
+        expiration TEXT,


its a date iso string so we can use sqlite Date functions on it, kysely plugin handles transformations for this. JS Date > ISO String on insert and back on select.

It appears that sqlite doesn't support full ISO-8601, only a subset of its string syntax, so we might get more sqlite compatability by storing as numbers https://www.sqlite.org/lang_datefunc.html

if you check section "2. Time Values" in that link you can see sqlite support date.toISOString

alanshaw · 2023-01-25T14:57:57Z

packages/access-api/migrations/0004_add_accounts_table.sql

+    IF NOT EXISTS accounts (
+        did TEXT NOT NULL PRIMARY KEY,
+        inserted_at TEXT NOT NULL DEFAULT (strftime ('%Y-%m-%dT%H:%M:%fZ', 'now')),
+        updated_at TEXT NOT NULL DEFAULT (strftime ('%Y-%m-%dT%H:%M:%fZ', 'now')),


Is updated_at relevant?

not really just a standard columns lol maybe in the future

do you prefer that i remove both updated_at ?

alanshaw · 2023-01-25T14:59:25Z

packages/access-api/migrations/0004_add_accounts_table.sql

+        inserted_at TEXT NOT NULL DEFAULT (strftime ('%Y-%m-%dT%H:%M:%fZ', 'now')),
+        updated_at TEXT NOT NULL DEFAULT (strftime ('%Y-%m-%dT%H:%M:%fZ', 'now')),
+        UNIQUE (cid),
+        FOREIGN KEY (audience) REFERENCES accounts (did) ON UPDATE CASCADE ON DELETE CASCADE


What does ON UPDATE CASCADE mean here? Why would we update the audience? Does it mean it updates the did in accounts?

If we change something in accounts, delegation stays in sync. Its just a good default so we dont store dangling delegations because these are not issued by us.
Updates any change to PK/FK pair (should never happen) and deletes delegations if we delete account.

alanshaw · 2023-01-25T15:05:39Z

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

+  constructor(d1) {
+    /** @type {GenericPlugin<DelegationRecord>} */
+    const objectPlugin = new GenericPlugin({
+      expiration: (v) => new Date(v),


Does this work ok with this nullable field?

alanshaw · 2023-01-25T15:09:18Z

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

+      .where('delegations.cid', '=', cid)
+      .where((qb) =>
+        qb
+          .where('delegations.expiration', '>=', new Date())


So this is a TEXT field in the DB - what happens to new Date() here?

https://github.com/web3-storage/w3protocol/blob/9dfdb6a648a1ef0929385e452c5589039a5227ed/packages/access-api/src/utils/d1.js#L35

alanshaw · 2023-01-25T15:27:11Z

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

+  if (req.query && req.query.ucan && req.query.mode === 'account') {
+    return account(req, env)


Maybe should be called session as it's a session authorization, as per https://github.com/web3-storage/specs/blob/feat/w3-authorization/w3-session.md#session

Suggested change

if (req.query && req.query.ucan && req.query.mode === 'account') {

return account(req, env)

if (req.query && req.query.ucan && req.query.mode === 'session') {

return session(req, env)

alanshaw · 2023-01-25T16:30:09Z

packages/access-client/src/types.ts

+    authorize: ServiceMethod<
+      AccessAuthorize,
+      EncodedDelegation<[AccessSession]> | undefined,
+      Failure
+    >


This doesn't return anything.

Suggested change

authorize: ServiceMethod<

AccessAuthorize,

EncodedDelegation<[AccessSession]> | undefined,

Failure

>

authorize: ServiceMethod<AccessAuthorize, undefined, Failure>

it does for tests
https://github.com/web3-storage/w3protocol/blob/9dfdb6a648a1ef0929385e452c5589039a5227ed/packages/access-api/src/service/access-authorize.js#L36

but its not a encoded delegation 🤫 i will fix it

vasco-santos

LGTM
Left some non blocking feedback inline

vasco-santos · 2023-01-30T12:58:52Z

packages/access-api/migrations/0004_add_accounts_table.sql

+        bytes BLOB NOT NULL,
+        audience TEXT NOT NULL,
+        issuer TEXT NOT NULL,
+        expiration TEXT,


nit: maybe rename to expire_at to be consistent with other date rows?

vasco-santos · 2023-01-30T13:01:01Z

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

+// eslint-disable-next-line no-unused-vars
+import * as Ucanto from '@ucanto/interface'


should we import types needed like below DelegationRecord instead of importing directly in source and ignoring eslint error?

perhaps we could disable no-unused-vars since it is not ts-aware and instead only use @typescript-eslint/no-unused-vars. https://stackoverflow.com/a/61555310

@vasco-santos the interface package is made with an empty .js file to enable imports like this, because generic types in jsdoc typedefs are a pain.

@gobengo that could work and it looks cleaner

fwiw what I proposed before doesn't work, but this does https://stackoverflow.com/a/66261985

https://github.com/web3-storage/w3protocol/pull/409/files#r1092603863

vasco-santos · 2023-01-30T13:03:53Z

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

-      new Kysely({ dialect: new D1Dialect({ database: d1 }) })
+      new Kysely({
+        dialect: new D1Dialect({ database: d1 }),
+        plugins: [objectPlugin],


nice we can use plugins here directly 👌🏼

vasco-santos · 2023-01-30T13:09:01Z

packages/access-api/src/utils/html.js

+// @ts-ignore
 // eslint-disable-next-line no-unused-vars
 import * as Ucanto from '@ucanto/interface'


not needed anymore right?

vasco-santos · 2023-01-30T13:20:53Z

packages/access-api/src/service/access-authorize.js

+          issuer: ctx.signer,
+          audience: DID.parse(capability.nb.as),
+          with: ctx.signer.did(),
+          lifetimeInSeconds: 86_400 * 7, // 7 days


should this be spec'ed? I could not find this anywhere

~~also might be good to use an absolute datetime here instead of a duration (so that the expiry datetime is unambiguous)~~

I realized that comment only made sense because I thought lifetimeInSeconds was in the nb of an invocation, but just realized it's a ucanto argument

storacha/specs#43

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

gobengo · 2023-01-30T23:26:57Z

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

@@ -25,7 +31,11 @@ export async function validateEmail(req, env) {
      return new HtmlResponse(
        (
          <ValidateEmail
-            delegation={delegation}
+            email={delegation.capabilities[0].nb.identity.replace(


It could be good to define a utility function for extracting the email from the ability, because this logic using String#replace won't work for did:mailto using the syntax we changed to once realizing @ was not a valid DID character.

and then it could be used on line 84 below as well

after reading more, I also don't really understand where nb.identity is coming from or what type it would be (I don't see nb.identity in the access capabilities). If I'm reading right, it might be type any when the replace method is called on it here. We might avoid runtime bugs by clarifying what the return type of env.models.validations.put is expecting (e.g. specific Capability types within the delegation, not generic allowing any Capabilities tuple) and/or doing more explicit parsing of req.query.ucan instead of using the type assertion on line 26

it would be good to add a test for what should happen if someone tries to inject malicious req.query.ucan and IMO it's best to ensure we don't create db records unless the req.query.ucan parses into a well-known ability can

this is from the old recover cap it will go away soon, regarding ucan validation i agree we should validate the caps for the specific mode and the signature.

#411

gobengo · 2023-01-30T23:36:12Z

packages/capabilities/src/access.js

+  }
+ * ```
+ */
+export const session = capability({


when I saw this used elsewhere, I assumed it was a new access/session can value and was confused at first. Now I understand.

It seems to me that since this is a special 'can' value within ucanto atm, we should make it so you can import it from ucanto. e.g. we could export this. agree @Gozala ?

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

Motivation: * #408 Unblocks: * people importing a canonical CapabilityParser for `access/claim` * adding advanced tests e.g. by [reverting this and finishing the TODOs](fd0b56f) * these involve `can=./update` abilities, so may make most sense to add after #392 which defines tools for them

🤖 I have created a release *beep* *boop* --- ## [2.3.0](capabilities-v2.2.0...capabilities-v2.3.0) (2023-02-10) ### Features * add `pre` caveat to `store/list` and `upload/list` ([#423](#423)) ([a0f6d28](a0f6d28)) * add access/delegate capability parser exported from @web3-storage/capabilities ([#420](#420)) ([e8e2b1a](e8e2b1a)) * add support for access/authorize and update ([#392](#392)) ([9c8ca0b](9c8ca0b)), closes [#386](#386) * define access/claim in @web3-storage/capabilities ([#409](#409)) ([4d72ba3](4d72ba3)) --- 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* --- ## [4.10.0](access-api-v4.9.0...access-api-v4.10.0) (2023-02-16) ### Features * add support for access/authorize and update ([#392](#392)) ([9c8ca0b](9c8ca0b)), closes [#386](#386) * optional override for Postmark email From: field ([#354](#354)) ([f6b2350](f6b2350)) * rm /reproduce-cloudflare-error route ([#426](#426)) ([99cbd2f](99cbd2f)) * rm upload-api-proxy ability to route to separate environment audiences ([#407](#407)) ([5cfe274](5cfe274)) ### Bug Fixes * align postmark/welcome.txt with .html version ([#431](#431)) ([a53d6e6](a53d6e6)) * avoid email delegation via GET request ([#430](#430)) ([d282d6a](d282d6a)) --- 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* --- ## [9.4.0](access-v9.3.0...access-v9.4.0) (2023-02-21) ### Features * add support for access/authorize and update ([#392](#392)) ([9c8ca0b](9c8ca0b)), closes [#386](#386) ### Bug Fixes * look for URL in channel before falling back to default ([#440](#440)) ([0741295](0741295)) --- 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: Travis Vachon <[email protected]>

🤖 I have created a release *beep* *boop* --- ## [9.4.0](access-v9.3.0...access-v9.4.0) (2023-02-23) ### Features * add support for access/authorize and update ([#392](#392)) ([9c8ca0b](9c8ca0b)), closes [#386](#386) ### Bug Fixes * look for URL in channel before falling back to default ([#440](#440)) ([0741295](0741295)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

closes #386

Motivation: * #408 Unblocks: * people importing a canonical CapabilityParser for `access/claim` * adding advanced tests e.g. by [reverting this and finishing the TODOs](fd0b56f) * these involve `can=./update` abilities, so may make most sense to add after #392 which defines tools for them

🤖 I have created a release *beep* *boop* --- ## [2.3.0](capabilities-v2.2.0...capabilities-v2.3.0) (2023-02-10) ### Features * add `pre` caveat to `store/list` and `upload/list` ([#423](#423)) ([9cce414](9cce414)) * add access/delegate capability parser exported from @web3-storage/capabilities ([#420](#420)) ([7834cf2](7834cf2)) * add support for access/authorize and update ([#392](#392)) ([bf41071](bf41071)), closes [#386](#386) * define access/claim in @web3-storage/capabilities ([#409](#409)) ([2fb34dd](2fb34dd)) --- 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* --- ## [4.10.0](access-api-v4.9.0...access-api-v4.10.0) (2023-02-16) ### Features * add support for access/authorize and update ([#392](#392)) ([bf41071](bf41071)), closes [#386](#386) * optional override for Postmark email From: field ([#354](#354)) ([00db0ec](00db0ec)) * rm /reproduce-cloudflare-error route ([#426](#426)) ([158f309](158f309)) * rm upload-api-proxy ability to route to separate environment audiences ([#407](#407)) ([eefb6c6](eefb6c6)) ### Bug Fixes * align postmark/welcome.txt with .html version ([#431](#431)) ([0d72795](0d72795)) * avoid email delegation via GET request ([#430](#430)) ([e0f67e8](e0f67e8)) --- 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* --- ## [9.4.0](access-v9.3.0...access-v9.4.0) (2023-02-21) ### Features * add support for access/authorize and update ([#392](#392)) ([bf41071](bf41071)), closes [#386](#386) ### Bug Fixes * look for URL in channel before falling back to default ([#440](#440)) ([6fa2cba](6fa2cba)) --- 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: Travis Vachon <[email protected]>

🤖 I have created a release *beep* *boop* --- ## [9.4.0](access-v9.3.0...access-v9.4.0) (2023-02-23) ### Features * add support for access/authorize and update ([#392](#392)) ([bf41071](bf41071)), closes [#386](#386) ### Bug Fixes * look for URL in channel before falling back to default ([#440](#440)) ([6fa2cba](6fa2cba)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

hugomrdias requested review from alanshaw, gobengo and Gozala January 25, 2023 10:32

hugomrdias temporarily deployed to dev January 25, 2023 10:34 — with GitHub Actions Inactive

alanshaw requested changes Jan 25, 2023

View reviewed changes

alanshaw reviewed Jan 25, 2023

View reviewed changes

hugomrdias requested a review from alanshaw January 25, 2023 17:02

hugomrdias temporarily deployed to dev January 25, 2023 17:03 — with GitHub Actions Inactive

hugomrdias temporarily deployed to dev January 26, 2023 12:19 — with GitHub Actions Inactive

hugomrdias requested a review from vasco-santos January 27, 2023 10:07

Gozala force-pushed the main branch from 953cbae to 6e83725 Compare January 27, 2023 22:00

vasco-santos approved these changes Jan 30, 2023

View reviewed changes

gobengo reviewed Jan 30, 2023

View reviewed changes

packages/access-api/src/models/accounts.js Show resolved Hide resolved

gobengo reviewed Jan 30, 2023

View reviewed changes

packages/access-api/src/models/accounts.js Show resolved Hide resolved

gobengo mentioned this pull request Jan 31, 2023

feat: define access/claim in @web3-storage/capabilities #409

Merged

hugomrdias requested a review from gobengo January 31, 2023 16:27

gobengo approved these changes Jan 31, 2023

View reviewed changes

hugomrdias added 6 commits February 1, 2023 14:47

feat: add support for access/authorize and update

18e7410

fix: feedback

36e74df

fix: set session expiration to 7 days

7e1bca2

chore: feedback

92f996d

chore: change expiration to expires_at

ec9e691

chore: lock

cc18554

hugomrdias force-pushed the feat/access-authorize-api branch from a51b5fa to cc18554 Compare February 1, 2023 14:49

hugomrdias merged commit 9c8ca0b into main Feb 1, 2023

hugomrdias deleted the feat/access-authorize-api branch February 1, 2023 14:50

hugomrdias temporarily deployed to dev February 1, 2023 14:50 — with GitHub Actions Inactive

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

chore(main): release access-api 4.10.0 #416

Merged

This was referenced Feb 1, 2023

chore(main): release capabilities 2.3.0 #417

Merged

chore(main): release access 9.4.0 #415

Merged

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

chore(main): release access 9.4.0 #444

Merged

gobengo mentioned this pull request Feb 27, 2023

test registration via access/authorize gobengo/w3protocol-test#5

Closed

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

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

Closed

gobengo pushed a commit that referenced this pull request Apr 11, 2023

feat: add support for access/authorize and update (#392)

bf41071

closes #386

		if (req.query && req.query.ucan && req.query.mode === 'account') {
		return account(req, env)

		// eslint-disable-next-line no-unused-vars
		import * as Ucanto from '@ucanto/interface'

feat: add support for access/authorize and update #392

feat: add support for access/authorize and update #392

Conversation

hugomrdias commented Jan 25, 2023 • edited Loading

alanshaw 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugomrdias Jan 25, 2023 • edited Loading

Choose a reason for hiding this comment

vasco-santos 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

gobengo Jan 31, 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

gobengo Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugomrdias commented Jan 25, 2023 •

edited

Loading

hugomrdias Jan 25, 2023 •

edited

Loading

gobengo Jan 31, 2023 •

edited

Loading

gobengo Jan 30, 2023 •

edited

Loading