proposal: store/add handler should have more helpful error when trying to invoke against space that has not had email confirmed

[gobengo]

Reproduction Steps

• this test that:
  • creates a new space keypair
  • creates a new keypair for 'alice'
  • uses proof of delegation iss=space aud=alice can=store/*
  • uses @web3-storage/upload to store a blob using a ucanto http connection to access-api

Expected Behavior

• either of:
  • the blob uploads fine using upload-client, i.e. uploadFile returns a resolved promise
  • calling upload-client uploadFile returns a rejected promise with error that the store/add invocation gets an error result indicating that the invocation was not fulfilled because the store/add nb.space had not yet been 'registered' with the access-api. e.g. in http status codes a 4xx error, e.g. 400 bad request or even 404 [space] Not Found

Actual Behavior

• await uploadFile(...) throws due to an error with the underlying store/add invocation being sent to upload-api. Upload-api returns the following result:

  Upload-api returns the following result:

    {
      name: 'Error',
      error: true,
      stack: 'Error: did:web:staging.web3.storage is not delegated capability store/add on did:key:z6MkfFSzetWCdChXXGmspXynkwPxSqTwkrBZ5r4KNZaBcSSc\n' +
        '    at null. (/upload-api/service/store/add.js:36:16)\n' +
        '    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n' +
        '    at null.invoke2 (/node_modules/@ucanto/server/src/server.js:129:14)\n' +
        '    at null.execute2 (/node_modules/@ucanto/server/src/server.js:90:18)\n' +
        '    at null.handle (/node_modules/@ucanto/server/src/server.js:71:18)\n' +
        '    at null.ucanInvocationRouter (/upload-api/functions/ucan-invocation-router.js:87:20)\n' +
        '    at Runtime.handler (/node_modules/src/awslambda.ts:332:1)',
      message: 'did:web:staging.web3.storage is not delegated capability store/add on did:key:z6MkfFSzetWCdChXXGmspXynkwPxSqTwkrBZ5r4KNZaBcSSc'
    }
      

  • This error initially looks like an error in the ucan signature checks automated by the use of ucanto/server inside access-api (ucanto endpoint).
    • But actually it is explicitly thrown after those checks by the upload-api service add handler. https://github.com/web3-storage/w3infra/blob/main/upload-api/service/store/add.js#L36

error stdio from test case

TAP version 13
# Subtest: /Users/bengo/bengo.is/monorepo/experiments/w3protocol-test/dist/src/w3protocol.test.js
not ok 1 - /Users/bengo/bengo.is/monorepo/experiments/w3protocol-test/dist/src/w3protocol.test.js
  ---
  duration_ms: 2941.654334
  failureType: 'subtestsFailed'
  exitCode: 1
  stdout: |-
    {
      alice: 'did:key:z6Mkja9ZQYF7EUBC1ZK9TjC5uLQqchRoNJgtNR8YvkVdKCTF',
      space: 'did:key:z6MkfFSzetWCdChXXGmspXynkwPxSqTwkrBZ5r4KNZaBcSSc'
    }
    TAP version 13
    # Subtest: can list items in a space
    ok 1 - can list items in a space # SKIP 'only' option not set
      ---
      duration_ms: 0.507
      ...
    # Subtest: w3protocol-test can upload file
    not ok 2 - w3protocol-test can upload file
      ---
      duration_ms: 2725.244125
      failureType: 'testCodeFailure'
      error: 'failed store/add invocation'
      code: 'ERR_TEST_FAILURE'
      stack: |-
        add (file:///Users/bengo/bengo.is/monorepo/common/temp/node_modules/.pnpm/@web3-storage+upload-client@5.3.0/node_modules/@web3-storage/upload-client/src/store.js:59:11)
        process.processTicksAndRejections (node:internal/process/task_queues:95:5)
        async queue.add.signal (file:///Users/bengo/bengo.is/monorepo/common/temp/node_modules/.pnpm/@web3-storage+upload-client@5.3.0/node_modules/@web3-storage/upload-client/src/sharding.js:97:27)
        async run (file:///Users/bengo/bengo.is/monorepo/common/temp/node_modules/.pnpm/p-queue@7.3.0/node_modules/p-queue/dist/index.js:115:36)
      ...
    1..2
    # tests 2
    # pass 0
    # fail 1
    # cancelled 0
    # skipped 1
    # todo 0
    # duration_ms 2779.472125
stderr: |-

(node:48391) ExperimentalWarning: The test runner is an experimental feature. This feature could change at any time

(Use node --trace-warnings ... to show where the warning was created)

(node:48391) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time

error uploading file

error cause {

name: 'Error',

error: true,

stack: 'Error: did:web:staging.web3.storage is not delegated capability store/add on did:key:z6MkfFSzetWCdChXXGmspXynkwPxSqTwkrBZ5r4KNZaBcSSc\n' +

'    at null. (/upload-api/service/store/add.js:36:16)\n' +

'    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n' +

'    at null.invoke2 (/node_modules/@ucanto/server/src/server.js:129:14)\n' +

'    at null.execute2 (/node_modules/@ucanto/server/src/server.js:90:18)\n' +

'    at null.handle (/node_modules/@ucanto/server/src/server.js:71:18)\n' +

'    at null.ucanInvocationRouter (/upload-api/functions/ucan-invocation-router.js:87:20)\n' +

'    at Runtime.handler (/node_modules/src/awslambda.ts:332:1)',

message: 'did:web:staging.web3.storage is not delegated capability store/add on did:key:z6MkfFSzetWCdChXXGmspXynkwPxSqTwkrBZ5r4KNZaBcSSc'

}
error: 'test failed'

code: 'ERR_TEST_FAILURE'

...

1..1
tests 1
pass 0
fail 1
cancelled 0
skipped 0
todo 0
duration_ms 2944.138417

Diagnosis

• the store/add handler is running, and then it's calling ctx.access.verifyInvocation on the store/add invocation
• that verifyInvocation is invoking space/info on access-api
• pretty sure but untested
  • access-api handles space/info by looking for the space id in its db. If the db contains none (e.g. if the space has not been 'registered' by an email I think), it returns a failure result like new Failure('Space not found.')
  • upload-api verifyInvocation sees that failure and so returns falsy
    • this is misleading. The invocation verified totally fine. The only reason it got a failure is because access-api db didn't contain an entry for that space.
  • so the upload-api store/add handler throws the error reported in actual behavior
    • but it could provide a much more intelligble result type than saying ${issuer} is not delegated capability ${Store.add.can} on ${space} when the underlying issue is that the space is not in the db (and that the capability delegations are not the issue)

Proposal

• logically, the store/add service method definition should include in its result type a Failure case that makes sense for: "I was able to make sense of your request to add something to the storage space, and according to the ucans you might have authority, but I am refusing to because the space in question is not one I am allowed to store in. Trying again won't help unless you change something about the space itself (e.g. register it with an email address using voucher protocol or provider+accounts protocol)"
  • This is similar to the HTTP status codes for 404 [space] not found or 507 insufficient storage
• when upload-api handles store/add request for a space that is not in access-api db, it returns the result type for 'unable to store' and not the current vague error result type whose message mentions ${issuer} is not delegated capability ${Store.add.can} on ${space}
• as I think about it, I think the 507 result status is the most useful here

Implementation

• access-api should respond to space/info invocations for space dids not in its db with a well-defined error result type w3up#382
• when upload-api verifyInvocation calls space/info and gets a 404 error result, it should return a SpaceNotFound type instead of just false (which might imply that the ucan proofs can't be validated, when in fact they can)
  • alternatively, come up with an alternate/richer abstraction than verifyInvocation: (...) => boolean with richer return types
• when upload-api store/add handler is invoked, asks access-api about it, and gets a SpaceNotFound error, the store/add handler should return an error indicating why the store/add could not be handled

[gobengo]

released w3protocol#382 , which helps a bit. Also started PR in w3protocol-test whose CI reproduces the error of using @web3-storage/upload-client on a space that has not been registered (afaict pre voucher/claim) (https://github.com/gobengo/w3protocol-test/actions/runs/4049655853/jobs/6966264349#step:6:46)

[gobengo]

This happens now, which is much more helpful:
(the behavior changed in upload-api due to #154 which relied on storacha/w3up#510)

 {
      name: 'Error',
      error: true,
      stack: 'Error: Space has no storage provider\n' +
        '    at Object.allocateSpace (/upload-api/access.js:40:13)\n' +
        '    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n' +
        '    at async Promise.all (index 0)\n' +
        '    at <anonymous> (/node_modules/@web3-storage/upload-api/src/store/add.js:19:58)\n' +
        '    at invoke2 (/node_modules/@ucanto/server/src/server.js:119:14)\n' +
        '    at execute (/node_modules/@ucanto/server/src/server.js:87:18)\n' +
        '    at execute3 (/upload-api/functions/ucan-invocation-router.js:173:20)\n' +
        '    at async Promise.all (index 0)\n' +
        '    at ucanInvocationRouter (/upload-api/functions/ucan-invocation-router.js:130:19)\n' +
        '    at Runtime.handler (/node_modules/src/awslambda.ts:333:1)',
      message: 'Space has no storage provider'
}