Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: update access-api ucanto proxy to not need a signer #390

Merged
merged 2 commits into from
Jan 30, 2023
Merged

feat: update access-api ucanto proxy to not need a signer #390

merged 2 commits into from
Jan 30, 2023

Conversation

gobengo
Copy link
Contributor

@gobengo gobengo commented Jan 23, 2023 •
edited
Loading

… and not sign proxyInvocation. using features coming in ucanto 4.2.0

Motivation:

  • Unify UCAN API endpoints #325
  • simplify access-api ucanto proxy using features added to ucanto in feat: support execution of delegations ucanto#199
    • previously, the technique used to proxy the invocation was to issue a new invocation (i.e. proxyInvocation) in the proxy server, and then send that to the upstream. This had at least two limitations:
      1. required the proxy server to be configured with a options.signer to sign the proxyInvocation
      2. for functional use in access-api and proxying upload-api, this proxy options.signer also had to be configured pretty much identically to the ucanto verifier with same did on the upstream, including requiring both to have the same private key
    • now
      • you don't need an options.signer at all! so you definitely don't need one creating signatures with the same private key as the upstream

Steps

  • release ucanto 4.2.0 chore: release main ucanto#200
  • update this source branch package.json + pnpm locks to upgrade ucanto to 4.2.0
  • ensure tsc + tests pass here

@gobengo gobengo mentioned this pull request Jan 23, 2023
Merged
@gobengo gobengo changed the title update access-api ucanto proxy to not need a signer feat: update access-api ucanto proxy to not need a signer Jan 23, 2023
@gobengo gobengo mentioned this pull request Jan 23, 2023
Closed
@gobengo gobengo requested a review from Gozala January 24, 2023 00:51
@gobengo gobengo mentioned this pull request Jan 28, 2023
Merged
@gobengo
update access-api ucanto proxy to not need a signer and not sign prox…
35acf3d 
…yInvocation. using features coming in ucanto 4.2.0
@gobengo gobengo force-pushed the 1674516319-ucanto-upgrade-forproxy branch from 787d282 to 35acf3d Compare January 28, 2023 00:30
@gobengo gobengo temporarily deployed to dev January 28, 2023 00:31 — with GitHub Actions Inactive
@gobengo gobengo marked this pull request as ready for review January 28, 2023 00:32
@gobengo
Copy link
Contributor Author

gobengo commented Jan 28, 2023

note: this is a superset of #405 , which only does the upgrade to ucanto 4.2.3 that this depends on.

Gozala
Gozala approved these changes Jan 28, 2023
View reviewed changes
packages/access-api/src/ucanto/proxy.js
[proxyInvocation],
/** @type {Client.ConnectionView<any>} */ (connection)
[await invocation.delegate()],
connection
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect you to be able to just pass invocation without having to call .delegate()

hugomrdias
hugomrdias approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM can we remove the connections code in a follow up PR ? we should just need one single connection to upload api for these proxied invocations right ?

gobengo added a commit that referenced this pull request Jan 30, 2023
@gobengo
feat: update @ucanto/* to ~4.2.3 (#405)
50c0c80 
Motivation:
* upgrade to latest ucanto
* unblock #390
@gobengo gobengo mentioned this pull request Jan 30, 2023
Open
3 tasks
@gobengo
Copy link
Contributor Author

gobengo commented Jan 30, 2023

can we remove the connections code in a follow up PR ? we should just need one single connection to upload api for these proxied invocations right ?

@hugomrdias Yes. I made this to echo back what I think you're asking for. #406
Let's align over there

@gobengo gobengo merged commit 71cbeb7 into main Jan 30, 2023
@gobengo gobengo deleted the 1674516319-ucanto-upgrade-forproxy branch January 30, 2023 22:33
This was referenced Jan 30, 2023
Merged
Merged
Merged
Merged
@gobengo gobengo mentioned this pull request Jan 30, 2023
Merged
gobengo pushed a commit that referenced this pull request Jan 31, 2023
@hugomrdias
chore(main): release access-api 4.9.0 (#400)
4e90f6b 
🤖 I have created a release *beep* *boop*
---


##
[4.9.0](access-api-v4.8.0...access-api-v4.9.0)
(2023-01-30)


### Features

* access-api handling store/info for space not in db returns failure
with name
([#391](#391))
([9610fcf](9610fcf))
* update @ucanto/* to ~4.2.3
([#405](#405))
([50c0c80](50c0c80))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([71cbeb7](71cbeb7))


### Bug Fixes

* make tests use did:web everywhere
([#397](#397))
([c7d5c34](c7d5c34))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
olizilla pushed a commit that referenced this pull request Jan 31, 2023
@hugomrdias
chore(main): release upload-client 5.4.0 (#396)
2de08e9 
🤖 I have created a release *beep* *boop*
---


##
[5.4.0](upload-client-v5.3.0...upload-client-v5.4.0)
(2023-01-30)


### Features

* update @ucanto/* to ~4.2.3
([#405](#405))
([50c0c80](50c0c80))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([71cbeb7](71cbeb7))


### Bug Fixes

* use nullish coalescing for audience
([#319](#319))
([a1d5ecf](a1d5ecf))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
hugomrdias added a commit that referenced this pull request Jan 31, 2023
@hugomrdias
chore(main): release capabilities 2.2.0 (#299)
a58a749 
🤖 I have created a release *beep* *boop*
---


##
[2.2.0](capabilities-v2.1.0...capabilities-v2.2.0)
(2023-01-30)


### Features

* access-api forwards store/ and upload/ invocations to upload-api
([#334](#334))
([b773376](b773376))
* **capabilities:** implement access/authorize and ./update caps
([#387](#387))
([4242ce0](4242ce0)),
closes [#385](#385)
* embedded key resolution
([#312](#312))
([4da91d5](4da91d5))
* update @ucanto/* to ~4.2.3
([#405](#405))
([50c0c80](50c0c80))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([71cbeb7](71cbeb7))


### Bug Fixes

* fix client cli service did resolve
([#292](#292))
([6be9608](6be9608))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
hugomrdias added a commit that referenced this pull request Jan 31, 2023
@hugomrdias
chore(main): release access 9.3.0 (#346)
fc53a16 
🤖 I have created a release *beep* *boop*
---


##
[9.3.0](access-v9.2.0...access-v9.3.0)
(2023-01-30)


### Features

* access-api forwards store/ and upload/ invocations to upload-api
([#334](#334))
([b773376](b773376))
* access-api handling store/info for space not in db returns failure
with name
([#391](#391))
([9610fcf](9610fcf))
* update @ucanto/* to ~4.2.3
([#405](#405))
([50c0c80](50c0c80))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([71cbeb7](71cbeb7))


### Bug Fixes

* remove unecessary awaits
([#352](#352))
([64da6e5](64da6e5))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@it-dag-house it-dag-house mentioned this pull request Mar 23, 2023
Closed
gobengo added a commit that referenced this pull request Apr 11, 2023
@gobengo
feat: update @ucanto/* to ~4.2.3 (#405)
ec39443 
Motivation:
* upgrade to latest ucanto
* unblock #390
gobengo added a commit that referenced this pull request Apr 11, 2023
@gobengo
feat: update access-api ucanto proxy to not need a signer (#390)
163fb74 
… and not sign proxyInvocation. using features coming in ucanto 4.2.0

Motivation:
* #325
* simplify access-api ucanto proxy using features added to ucanto in
storacha/ucanto#199
* previously, the technique used to proxy the invocation was to issue a
new invocation (i.e. `proxyInvocation`) in the proxy server, and then
send that to the upstream. This had at least two limitations:
1. required the proxy server to be configured with a `options.signer` to
sign the `proxyInvocation`
2. for functional use in access-api and proxying upload-api, this proxy
`options.signer` also had to be configured pretty much identically to
the ucanto verifier with same did on the upstream, including requiring
both to have the same private key
  * now
* you don't need an `options.signer` at all! so you definitely don't
need one creating signatures with the same private key as the upstream

Steps
* [x] release ucanto 4.2.0
storacha/ucanto#200
* [x] update this source branch package.json + pnpm locks to upgrade
ucanto to 4.2.0
* [x] ensure `tsc` + tests pass here
gobengo pushed a commit that referenced this pull request Apr 11, 2023
@hugomrdias
chore(main): release access-api 4.9.0 (#400)
dd00c29 
🤖 I have created a release *beep* *boop*
---


##
[4.9.0](access-api-v4.8.0...access-api-v4.9.0)
(2023-01-30)


### Features

* access-api handling store/info for space not in db returns failure
with name
([#391](#391))
([665dac9](665dac9))
* update @ucanto/* to ~4.2.3
([#405](#405))
([ec39443](ec39443))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([163fb74](163fb74))


### Bug Fixes

* make tests use did:web everywhere
([#397](#397))
([00be288](00be288))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
gobengo pushed a commit that referenced this pull request Apr 11, 2023
@hugomrdias
chore(main): release upload-client 5.4.0 (#396)
65fc6e5 
🤖 I have created a release *beep* *boop*
---


##
[5.4.0](upload-client-v5.3.0...upload-client-v5.4.0)
(2023-01-30)


### Features

* update @ucanto/* to ~4.2.3
([#405](#405))
([ec39443](ec39443))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([163fb74](163fb74))


### Bug Fixes

* use nullish coalescing for audience
([#319](#319))
([7e90085](7e90085))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
gobengo pushed a commit that referenced this pull request Apr 11, 2023
@hugomrdias
chore(main): release capabilities 2.2.0 (#299)
37b7ca1 
🤖 I have created a release *beep* *boop*
---


##
[2.2.0](capabilities-v2.1.0...capabilities-v2.2.0)
(2023-01-30)


### Features

* access-api forwards store/ and upload/ invocations to upload-api
([#334](#334))
([6be7217](6be7217))
* **capabilities:** implement access/authorize and ./update caps
([#387](#387))
([ebe1032](ebe1032)),
closes [#385](#385)
* embedded key resolution
([#312](#312))
([45f367d](45f367d))
* update @ucanto/* to ~4.2.3
([#405](#405))
([ec39443](ec39443))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([163fb74](163fb74))


### Bug Fixes

* fix client cli service did resolve
([#292](#292))
([45e7ad4](45e7ad4))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
gobengo pushed a commit that referenced this pull request Apr 11, 2023
@hugomrdias
chore(main): release access 9.3.0 (#346)
83b3070 
🤖 I have created a release *beep* *boop*
---


##
[9.3.0](access-v9.2.0...access-v9.3.0)
(2023-01-30)


### Features

* access-api forwards store/ and upload/ invocations to upload-api
([#334](#334))
([6be7217](6be7217))
* access-api handling store/info for space not in db returns failure
with name
([#391](#391))
([665dac9](665dac9))
* update @ucanto/* to ~4.2.3
([#405](#405))
([ec39443](ec39443))
* update access-api ucanto proxy to not need a signer
([#390](#390))
([163fb74](163fb74))


### Bug Fixes

* remove unecessary awaits
([#352](#352))
([2e8c1a1](2e8c1a1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugomrdias hugomrdias hugomrdias approved these changes

@Gozala Gozala Gozala approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gobengo @Gozala @hugomrdias