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.

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

core/corehttp: wrap gateway with headers, deprecate gateway /api/v0 #10311

Merged
merged 4 commits into from
Jan 24, 2024

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jan 19, 2024

@hacdias
Copy link
Member Author

hacdias commented Jan 19, 2024

Note: the way we built our muxer is making this a bit more complicated than I expected. If we wrap the hostname option with the headers middelware, than all requests will have the headers, including the API and so forth. I have to take a look to see what is the best/most feasible way of implementing this.

@hacdias
Copy link
Member Author

hacdias commented Jan 22, 2024

Conversation with @lidel:

  • set the CORS headers in the whole gateway port
  • update the tests in order to match the new headers
  • Add to config.md (clean recipes and mentions of), release notes, information that the /api/v0 on the gateway port is deprecated and should not be used.
  • Add header to responses of api v0 saying it's deprecated: https://datatracker.ietf.org/doc/draft-ietf-httpapi-deprecation-header/

@hacdias hacdias changed the title core/corehttp: wrap gateway endpoint with headers core/corehttp: wrap gateway with headers, deprecate gateway /api/v0 Jan 22, 2024
@hacdias hacdias self-assigned this Jan 22, 2024
@hacdias hacdias mentioned this pull request Jan 22, 2024
8 tasks
@hacdias hacdias marked this pull request as ready for review January 22, 2024 15:23
@hacdias hacdias requested review from lidel and a team as code owners January 22, 2024 15:23
@hacdias hacdias force-pushed the headers-middleware branch 3 times, most recently from ffeee17 to 7923549 Compare January 23, 2024 08:15
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for cleaning this up ❤️

This simplifies handling of /api/v0 on the gateway port, which we want to remove anyway (and only expose on RPC API port), so no concerns.

In case someone reads this and is confused what is happening in this PR: this is just a cleanup that makes boxo/gateway more flexible. We track missing specs in ipfs/specs#423. For Kubo user there is no functional difference, in both cases end user hitting /api/v0 gets 403 response for non-localhost CORS request:

Before

$ curl -i "http://127.0.0.1:8080/api/v0/resolve?arg=/ipns/ipfs.tech" -H "Origin: https://example.com"
HTTP/1.1 403 Forbidden
Content-Type: text/plain; charset=utf-8
Vary: Origin
X-Content-Type-Options: nosniff
Date: Tue, 23 Jan 2024 18:47:00 GMT
Content-Length: 16

403 - Forbidden

After

curl -i "http://127.0.0.1:8080/api/v0/resolve?arg=/ipns/ipfs.tech" -H "Origin: https://example.com"                                                                                                                                                                                      ~
HTTP/1.1 403 Forbidden
Access-Control-Allow-Headers: Content-Type
Access-Control-Allow-Headers: Range
Access-Control-Allow-Headers: User-Agent
Access-Control-Allow-Headers: X-Requested-With
Access-Control-Allow-Methods: GET
Access-Control-Allow-Methods: HEAD
Access-Control-Allow-Methods: OPTIONS
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: Content-Length
Access-Control-Expose-Headers: Content-Range
Access-Control-Expose-Headers: X-Chunked-Output
Access-Control-Expose-Headers: X-Ipfs-Path
Access-Control-Expose-Headers: X-Ipfs-Roots
Access-Control-Expose-Headers: X-Stream-Output
Content-Type: text/plain; charset=utf-8
Link: <https://github.com/ipfs/kubo/issues/10312>; rel="deprecation"; type="text/html"
Vary: Origin
X-Content-Type-Options: nosniff
Date: Tue, 23 Jan 2024 18:44:52 GMT
Content-Length: 16

403 - Forbidden

test/sharness/t0112-gateway-cors.sh Show resolved Hide resolved
docs/changelogs/v0.27.md Outdated Show resolved Hide resolved
@hacdias hacdias enabled auto-merge (squash) January 24, 2024 09:27
@hacdias hacdias merged commit e166af9 into master Jan 24, 2024
16 checks passed
@hacdias hacdias deleted the headers-middleware branch January 24, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants