Gateway: add support for HTTP OPTIONS request type

[lidel]

This potentially closes #934

OPTIONS is a noop request that is used by the browsers to check if server explicitly accepts cross-site XMLHttpRequest (XHR). The acceptance is indicated by the presence of CORS headers.

Before this fix user could enable CORS headers in the Gateway config (note that this disabled by default):

ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Methods '["PUT", "GET", "POST"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers '["X-Requested-With"]'

..but requests sent from JavaScript failed due to the lack of support for OPTIONS request type.
I described details of the problem with XHR in ipfs/ipfs-companion#45 (comment).

With this fix sending cross-site XHR to the local Gateway works as expected.

Reference:

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Preflighted_requests
• https://en.wikipedia.org/wiki/Cross-origin_resource_sharing#Preflight_example

[ghost]

Thanks! -- could you add a test which makes sure this works for both the gateway and the readonly api at :8080/api?

[lidel]

@lgierth where should I add OPTION tests?
Gateway in core/corehttp/gateway_test.go and API in commands/http/handler_test.go ?

BTW: while OPTIONS itself works with API, CORS headers such as Access-Control-Allow-* are removed from /api/v0/* responses as a security measure (even if user explicitly added them):

• https://github.com/ipfs/go-ipfs/blob/5d9ee59908099df3f7e85679f7384c98d4ac8111/commands/http/handler.go#L71-L82
• Fix CORS Support #1529 (comment)

In short: cross-site XHR currently does not work for API resources.

[rht]

Is this enabled in both ro and writable case? Shouldn't it be enabled for ro only, ref: #934 (comment).

[lidel]

I feel we should always produce OPTIONS response.
Gateway owner can decide what types of requests are accepted by customizing Access-Control-Allow-Method header (GET is ro, POST, PUT, DELETE are rw).
All major browsers respect this header and refuse to do XHR requests if method is not whitelisted.

[rht]

ic

[rht]

@lidel I think the test is to be put in test/sharness/t0110-gateway.sh.

To speed-up this PR, here is a quick test template:

test_gateway_options_request() {
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Methods '["PUT", "GET", "POST"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers '["X-Requested-With"]'

test_expect_success "OPTIONS to gateway succeeds" '
    curl -svX OPTIONS "http://localhost:$port/ipfs/" 2>curl_output
'
test_expect_success "OPTIONS to gateway output looks good" '
    grep "Origin:" curl_output &&
    grep "Access-Control-Request-Method:" curl_output &&
    grep "Access-Control-Request-Headers:" curl_output
'
}

[rht]

I figured that the cors wrapper in https://github.com/ipfs/go-ipfs/blob/a2b0287a5f6fb5c0f35b63688ad5ac81c451afce/commands/http/handler.go#L102 was meant to handle preflight (see https://github.com/rs/cors/blob/45d446e551b0020074e771dee17d2bb2fd2e9b44/cors.go#L245).

Somehow it doesn't get enabled.

[rht]

1. It appears that the github.com/rs/cors library used in the current master is outdated. Either 1. use that lib, or 2. replicate the essentially functionality (only show Origin, Access-Control-Request-Method, Access-Control-Request-Headers, instead of addUserHeaders).
2. I tested the pr and it didn't work on the ro api. Either leave this to a separate PR, or put a few lines in commands/http/handler.go. Edit: ic, because it was intentionally removed Gateway: add support for HTTP OPTIONS request type #2232 (comment)

[rht]

Just updated the test. It should be runnable.

[lidel]

Thank you @rht, I decided to add CORS headers and OPTIONS tests as a separate file in 15d717c

• I don't want to pollute other gateway tests with CORS headers
• (as of now) changing headers requires daemon restart anyway to see the changes
• we may want to add more CORS-related tests in future as our browser addons and IPFS-based app ecosystem matures

I feel that the goal of this PR is to add missing support for OPTIONS and that is all.
Further CORS improvements should be separate PRs.

Let me know if I should add anything else for this to be merged.

[rht]

There could be test for gateway ACL as well.
The test file LGTM.
The travis-ci build stall is orthogonal to this pr.
This is ready to merge, I think.

[jbenet]

LGTM!

[jbenet]

thanks @lidel