feat: access-api uses DID env variable when building its ucanto server id by gobengo · Pull Request #275 · storacha/w3up

gobengo · 2022-12-08T22:58:01Z

Motivation:

access service & client should use did:web:web3.storage #237

Notes

This is intended to be safe to merge/deploy as is. The new functionality can be opted-into via env vars
The ucanto server will only be constructed with an opts.id that is a signer withDID(...) iff process.env.DID is set to a DID
- If process.env.DID is unset, the ucanto server id signer will have a did() corresponding to config.PRIVATE_KEY (it will be a did:key of the corresponding public key)
Before this PR, many tests of the access-api worker were configured implicitly via dotenv with whatever the /.env.local file happened to contain. I added a worker test that asserts functionality that is dependent on the details of the env vars, so I made an affordance for tests to be able to configure the access-api worker with environment variables defined in the test case itself.

…er id

…o configureSigner

Gozala

Looks good to me! Thanks

Gozala · 2022-12-08T23:17:37Z

packages/access-api/src/config.js

+export function configureSigner(config) {
+  const signer = Signer.parse(config.PRIVATE_KEY)
+  if (config.DID) {
+    if (!isDID(config.DID)) {


I think it may be better to use existing functionality for this, for example this will parse and throw if it's invalid:

DID.match({ method: 'web' }).from(config.DID)

It will also take care of refusing did:key which would be invalid in this case.

…f new custom code

Gozala · 2022-12-09T00:41:41Z

packages/access-api/src/config.js

-  if (parts[0] !== 'did') return false
-  return true
+  try {
+    return Boolean(DID.match({}).from(object))


What’s the point of catching here to then throw ? You can simply just do .from instead of whole isDID thing.

Alternatively there’s also .is method which you could instead which will return boolean

👍 5184a55

hugomrdias · 2022-12-09T16:16:08Z

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

  })

-  const keypair = Signer.parse(config.PRIVATE_KEY)
+  const signer = configureSigner(config)


can you do this inside the loadConfig ?

👍 b9533c9

hugomrdias · 2022-12-09T16:18:31Z

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

  return {
    log,
-    signer: keypair,
+    signer,


Suggested change

signer,

signer: Signer.parse(config.PRIVATE_KEY).withDID(config.ENV === 'production' ? 'did:web:ucan.web3.storage': `did:web:ucan-${config.ENV}.web3.storage`),

can't we just do this ? any error will be caught at dev time or CI.

I want the advertised DID to come from env variable. A '.web3.storage' domain won't be the best choice 100% of the times, e.g. on localhost or testing various scenarios. env variable gives me flexibility to get something working. once something works/tests end to end, can narrow down the configurability.

hugomrdias · 2022-12-09T16:22:59Z

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

+      environment: {
+        ...process.env,
+        PRIVATE_KEY:
+          'MgCYWjE6vp0cn3amPan2xPO+f6EZ3I+KwuN1w2vx57vpJ9O0Bn4ci4jn8itwc121ujm7lDHkCW24LuKfZwIdmsifVysY=',


why do you need a new key here ?

I don't. Good catch. 🙏 d204331

…signer

…can.test

hugomrdias · 2022-12-09T19:09:53Z

packages/access-api/src/config.js

    COMMITHASH: ACCOUNT_COMMITHASH,

+    DID,
+    signer,


can we get the did:key and did:web from the signer instance ?

console.log('signer did', signer.did().toString())

signer did did:web:exampe.com

console.log('signer archive', signer.toArchive())

signer archive {
id: 'did:web:exampe.com',
keys: {
'did:key:z6MkqBzPG7oNu7At8fktasQuS7QR7Tj7CujaijPMAgzdmAxD': Ed25519Signer(68) [Uint8Array] [
128, 38, 22, 140, 78, 175, 167, 71, 39, 221, 169,
143, 106, 125, 177, 60, 239, 159, 232, 70, 119, 35,
226, 176, 184, 221, 112, 218, 252, 121, 238, 250, 73,
244, 237, 1, 159, 135, 34, 226, 57, 252, 138, 220,
28, 215, 109, 110, 142, 110, 229, 12, 121, 2, 91,
110, 11, 184, 167, 217, 192, 135, 102, 178, 39, 213,
202, 198
]
}
}

hugomrdias · 2022-12-09T19:14:28Z

packages/access-api/src/config.js

    ENV: parseRuntimeEnv(vars.ENV),

-    PRIVATE_KEY: vars.PRIVATE_KEY,
+    PRIVATE_KEY,


we probably dont need this one anymore right ?

hugomrdias

LGTM

🤖 I have created a release *beep* *boop* --- ## [4.0.0](access-api-v3.0.0...access-api-v4.0.0) (2022-12-13) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * access-client store decoupling ([#228](#228)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) * bump to 0.9 ([#116](#116)) ### Features * [#153](#153) ([#177](#177)) ([d6d448c](d6d448c)) * access-api uses DID env variable when building its ucanto server id ([#275](#275)) ([311da78](311da78)) * **access-client:** cli and recover ([#207](#207)) ([adb3a8d](adb3a8d)) * account recover with email ([#149](#149)) ([6c659ba](6c659ba)) * bump to 0.9 ([#116](#116)) ([3e0b63f](3e0b63f)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) ([6496773](6496773)) * follow up on the capabilities extract ([#239](#239)) ([ef5e779](ef5e779)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) ([878f8c9](878f8c9)), closes [#117](#117) [#121](#121) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([c182bbe](c182bbe)) * sync encode/decode delegations ([#276](#276)) ([ab981fb](ab981fb)) * 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)) ### Bug Fixes * 0.9 ([#78](#78)) ([1b1ed01](1b1ed01)) * add validation copy ([#257](#257)) ([7f50af4](7f50af4)), closes [#139](#139) * fix Access API cannot get space/info [#243](#243) ([#255](#255)) ([1bacd54](1bacd54)) * fix d1 migrations ([#264](#264)) ([fb8c09d](fb8c09d)) * make d1 spaces.metadata nullable and change to kysely ([#284](#284)) ([c8a9ce5](c8a9ce5)), closes [#280](#280) * make multiformats 9 go away ([#133](#133)) ([2668880](2668880)) * miniflare dev script ([#137](#137)) ([1c5a4e2](1c5a4e2)) * testing staging deploy ([3d500fa](3d500fa)) * try to deploy api staging ([18b7b29](18b7b29)) * try to deploy staging ([c616818](c616818)) ### Code Refactoring * access-client store decoupling ([#228](#228)) ([a785278](a785278)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…r id (#275) Motivation: * #237 Notes * This is intended to be safe to merge/deploy as is. The new functionality can be opted-into via env vars * The ucanto server will only be constructed with an opts.id that is a signer `withDID(...)` iff `process.env.DID` is set to a DID * If `process.env.DID` is unset, the ucanto server id signer will have a `did()` corresponding to `config.PRIVATE_KEY` (it will be a `did:key` of the corresponding public key) * Before this PR, many tests of the access-api worker were configured implicitly via dotenv with whatever the `/.env.local` file happened to contain. I added a worker test that asserts functionality that is dependent on the details of the env vars, so I made an affordance for [tests to be able to configure the access-api worker with environment variables defined in the test case itself](https://github.com/web3-storage/w3protocol/pull/275/files#diff-e20ffc550d006747790e7b67da280446c1cd1d2d4f72ccb5df04f62cbb6423daR149).

🤖 I have created a release *beep* *boop* --- ## [4.0.0](access-api-v3.0.0...access-api-v4.0.0) (2022-12-13) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * access-client store decoupling ([#228](#228)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) * bump to 0.9 ([#116](#116)) ### Features * [#153](#153) ([#177](#177)) ([7fbf2a1](7fbf2a1)) * access-api uses DID env variable when building its ucanto server id ([#275](#275)) ([e8326ec](e8326ec)) * **access-client:** cli and recover ([#207](#207)) ([720dafb](720dafb)) * account recover with email ([#149](#149)) ([91ad47d](91ad47d)) * bump to 0.9 ([#116](#116)) ([29cf63c](29cf63c)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) ([f6b6d06](f6b6d06)) * follow up on the capabilities extract ([#239](#239)) ([717fcaa](717fcaa)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) ([c3c58b9](c3c58b9)), closes [#117](#117) [#121](#121) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([197439e](197439e)) * sync encode/decode delegations ([#276](#276)) ([9d48372](9d48372)) * 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)) ### Bug Fixes * 0.9 ([#78](#78)) ([561c68b](561c68b)) * add validation copy ([#257](#257)) ([0bd49ce](0bd49ce)), closes [#139](#139) * fix Access API cannot get space/info [#243](#243) ([#255](#255)) ([1a74031](1a74031)) * fix d1 migrations ([#264](#264)) ([5b8a6e7](5b8a6e7)) * make d1 spaces.metadata nullable and change to kysely ([#284](#284)) ([7f09479](7f09479)), closes [#280](#280) * make multiformats 9 go away ([#133](#133)) ([cdb4109](cdb4109)) * miniflare dev script ([#137](#137)) ([f2cffb2](f2cffb2)) * testing staging deploy ([975cffc](975cffc)) * try to deploy api staging ([5130464](5130464)) * try to deploy staging ([4e56657](4e56657)) ### Code Refactoring * access-client store decoupling ([#228](#228)) ([64c7496](64c7496)) --- 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.0.0](storacha/w3up@access-api-v3.0.0...access-api-v4.0.0) (2022-12-13) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](storacha/w3up#246)) * access-client store decoupling ([#228](storacha/w3up#228)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](storacha/w3up#238)) * follow up on the capabilities extract ([#239](storacha/w3up#239)) * doc capabilities & make requierd nb non-optionals ([#159](storacha/w3up#159)) * Remove 0.8 caps and add account delegation to the service ([#123](storacha/w3up#123)) * bump to 0.9 ([#116](storacha/w3up#116)) ### Features * [#153](storacha/w3up#153) ([#177](storacha/w3up#177)) ([d6d448c](storacha/w3up@d6d448c)) * access-api uses DID env variable when building its ucanto server id ([#275](storacha/w3up#275)) ([311da78](storacha/w3up@311da78)) * **access-client:** cli and recover ([#207](storacha/w3up#207)) ([adb3a8d](storacha/w3up@adb3a8d)) * account recover with email ([#149](storacha/w3up#149)) ([6c659ba](storacha/w3up@6c659ba)) * bump to 0.9 ([#116](storacha/w3up#116)) ([3e0b63f](storacha/w3up@3e0b63f)) * doc capabilities & make requierd nb non-optionals ([#159](storacha/w3up#159)) ([6496773](storacha/w3up@6496773)) * follow up on the capabilities extract ([#239](storacha/w3up#239)) ([ef5e779](storacha/w3up@ef5e779)) * Remove 0.8 caps and add account delegation to the service ([#123](storacha/w3up#123)) ([878f8c9](storacha/w3up@878f8c9)), closes [#117](storacha/w3up#117) [#121](storacha/w3up#121) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](storacha/w3up#245)) ([c182bbe](storacha/w3up@c182bbe)) * sync encode/decode delegations ([#276](storacha/w3up#276)) ([ab981fb](storacha/w3up@ab981fb)) * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](storacha/w3up#246)) ([5e663d1](storacha/w3up@5e663d1)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](storacha/w3up#238)) ([2f3bab8](storacha/w3up@2f3bab8)) ### Bug Fixes * 0.9 ([#78](storacha/w3up#78)) ([1b1ed01](storacha/w3up@1b1ed01)) * add validation copy ([#257](storacha/w3up#257)) ([7f50af4](storacha/w3up@7f50af4)), closes [#139](storacha/w3up#139) * fix Access API cannot get space/info [#243](storacha/w3up#243) ([#255](storacha/w3up#255)) ([1bacd54](storacha/w3up@1bacd54)) * fix d1 migrations ([#264](storacha/w3up#264)) ([fb8c09d](storacha/w3up@fb8c09d)) * make d1 spaces.metadata nullable and change to kysely ([#284](storacha/w3up#284)) ([c8a9ce5](storacha/w3up@c8a9ce5)), closes [#280](storacha/w3up#280) * make multiformats 9 go away ([#133](storacha/w3up#133)) ([2668880](storacha/w3up@2668880)) * miniflare dev script ([#137](storacha/w3up#137)) ([1c5a4e2](storacha/w3up@1c5a4e2)) * testing staging deploy ([3d500fa](storacha/w3up@3d500fa)) * try to deploy api staging ([18b7b29](storacha/w3up@18b7b29)) * try to deploy staging ([c616818](storacha/w3up@c616818)) ### Code Refactoring * access-client store decoupling ([#228](storacha/w3up#228)) ([a785278](storacha/w3up@a785278)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

access-api uses DID env variable when building its ucanto server sign…

6504306

…er id

gobengo temporarily deployed to dev December 8, 2022 22:58 — with GitHub Actions Inactive

access-api add test ensuring error when non-did config.DID provided t…

a4785dd

…o configureSigner

gobengo changed the title ~~access-api uses DID env variable when building its ucanto server id~~ feat: access-api uses DID env variable when building its ucanto server id Dec 8, 2022

gobengo temporarily deployed to dev December 8, 2022 23:06 — with GitHub Actions Inactive

gobengo requested review from Gozala and hugomrdias December 8, 2022 23:14

Gozala approved these changes Dec 8, 2022

View reviewed changes

gobengo added 2 commits December 8, 2022 15:49

access-api update isDID type guard to use @ucanto/validator instead o…

91edd06

…f new custom code

pnpm-lock

795582a

gobengo temporarily deployed to dev December 8, 2022 23:51 — with GitHub Actions Inactive

Gozala reviewed Dec 9, 2022

View reviewed changes

hugomrdias suggested changes Dec 9, 2022

View reviewed changes

access-api configureSigner happens inside loadConfig and sets config.…

b9533c9

…signer

gobengo temporarily deployed to dev December 9, 2022 17:40 — with GitHub Actions Inactive

remove unnecessary PRIVATE_KEY environment var from access-api/test/u…

d204331

…can.test

gobengo temporarily deployed to dev December 9, 2022 17:45 — with GitHub Actions Inactive

access-api/src/config remove isDID function

5184a55

gobengo temporarily deployed to dev December 9, 2022 17:48 — with GitHub Actions Inactive

gobengo requested a review from hugomrdias December 9, 2022 17:49

gobengo mentioned this pull request Dec 9, 2022

access service & client should use did:web:web3.storage #237

Closed

5 tasks

hugomrdias suggested changes Dec 9, 2022

View reviewed changes

gobengo added 2 commits December 9, 2022 15:17

improve configureSigner test

a629263

config loadConfig no longer has DID or PRIVATE_KEY on it

852951f

gobengo requested a review from hugomrdias December 9, 2022 23:19

gobengo temporarily deployed to dev December 9, 2022 23:19 — with GitHub Actions Inactive

hugomrdias approved these changes Dec 12, 2022

View reviewed changes

gobengo merged commit 311da78 into main Dec 12, 2022

gobengo deleted the issues/237-access-api-didweb branch December 12, 2022 18:21

gobengo self-assigned this Dec 12, 2022

hugomrdias mentioned this pull request Dec 12, 2022

chore(main): release access-api 4.0.0 #113

Merged

gobengo mentioned this pull request Dec 12, 2022

Use same did across access / upload API services #183

Closed

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

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

Closed

	signer,
	signer: Signer.parse(config.PRIVATE_KEY).withDID(config.ENV === 'production' ? 'did:web:ucan.web3.storage': `did:web:ucan-${config.ENV}.web3.storage`),

Conversation

gobengo commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gozala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gobengo Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugomrdias left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gobengo commented Dec 8, 2022 •

edited

Loading

gobengo Dec 9, 2022 •

edited

Loading