OPTIONS does not actually handle CORS pre-flight requests

[laurenceisla]

Problem

While adding Origin validation for CORS requests, we found out that our OPTIONS implementation does not actually handle pre-flight requests, at least not completely #2986 (comment).

This is how it works right now:

Successful request

$ curl -X OPTIONS "http://localhost:3000/projects" \
        -H "Access-Control-Request-Method: POST" \
        -H "Access-Control-Request-Headers: Content-Type" \
        -H "Origin: http://localhost" -i

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Date: Fri, 27 Oct 2023 19:05:19 GMT
Server: postgrest/11.2.0 (8c9213d)
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, PATCH, PUT, OPTIONS, HEAD, POST
Access-Control-Allow-Headers: Authorization, Content-Type, Accept, Accept-Language, Content-Language
Access-Control-Max-Age: 86400

The actual response is given by the wai-cors library. We can notice because our implementation does not return pre-flight headers, only Access-Control-Allow-Origin. In summary, our OPTIONS does nothing here, the library does the response.

Failing request

$ curl -X OPTIONS "http://localhost:3000/projects" \
        -H "Access-Control-Request-Method: TRACE" \
        -H "Access-Control-Request-Headers: Content-Type" \
        -H "Origin: http://localhost" -i

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Date: Fri, 27 Oct 2023 19:16:15 GMT
Server: postgrest/11.2.0 (8c9213d)
Access-Control-Allow-Origin: *
Allow: OPTIONS,GET,HEAD,POST,PUT,PATCH,DELETE

Only when it fails, our OPTIONS implementation goes through and returns this info. This happens because we set the library to not respond with an error and let the request continue:

postgrest/src/PostgREST/Cors.hs

Line 37 in 5c9c7f4

, Wai.corsIgnoreFailures = True

This still fails the pre-flight (in Firefox and Chrome at least) because Access-Control-Allow-Methods is not set (different from Allow which has no meaning in CORS pre-flight).

Solution

We have two options here:

• Let the library return the error which is a 400 with a predefined message indicating the problem.
• Implement our own, which would mean to verify if the OPTIONS CORS request is pre-flight (with the required headers) and respond with a custom error indicating the problem.

Any of these would liberate the simple OPTIONS CORS request (not preflight) to work as we want. For instance, it could bring back the schema definition mentioned in #790.

[wolfgangwalther]

Is there any downside to letting the library handle this? Is there any upside to rolling our own?

[laurenceisla]

The pros are that the library returns the errors nicely, with a message like "Origin header is missing" or
"Unsupported origin..." using:

corsFailure msg = WAI.responseLBS HTTP.status400 [("Content-Type", "text/html; charset-utf-8")] (LB8.fromStrict msg)

The con is that it does not follow our "code, message, details, hint" structure for errors with application/json. An exception could be made since this is relevant mostly (only?) for browsers... or, if possible, we could intercept the library response and adapt it to our structure.

[wolfgangwalther]

I think it would be fine to let the library handle this entirely, since we don't really expect this error to be seen by any api user ever.

[laurenceisla]

Found a problem with the library: it expects that any OPTIONS request that has an Origin header should be considered a pre-flight request and it complains when it doesn't have an Access-Control-Request-Method header (IMO, it shouldn't be that way: that's a valid CORS request not a pre-flight).

Solution is to add an Origin validation just for that case. I'll continue in PR #3052.