CORS config preventing validation

[jmandel]

It seems like CORS support for validation is intended, from

org.hl7.fhir.validator-wrapper/src/jvmMain/kotlin/Module.kt

Lines 66 to 78 in d830da3

	install(CORS) {
	allowMethod(HttpMethod.Options)
	allowMethod(HttpMethod.Get)
	allowMethod(HttpMethod.Put)
	allowMethod(HttpMethod.Delete)
	allowMethod(HttpMethod.Patch)
	allowHeader(HttpHeaders.Authorization)
	allowHeader(HttpHeaders.AccessControlAllowOrigin)
	allowNonSimpleContentTypes = true
	allowCredentials = true
	allowSameOrigin = true
	allowHost("*", listOf("http", "https"))
	}

But currently Access-Control-Allow-Origin response headers omit POST.

Example reproduction:

$ curl -vvv 'https://validator.fhir.org/validate'   -X 'OPTIONS'   -H 'Accept: */*'   -H 'Accept-Language: en-US,en;q=0.9'   -H 'Access-Control-Request-Headers: content-type'   -H 'Access-Control-Request-Method: POST'   -H 'Cache-Control: no-cache'   -H 'Connection: keep-alive'   -H 'Origin: http:https://localhost:5173'   -H 'Pragma: no-cache'   -H 'Referer: http:https://localhost:5173/'   -H 'Sec-Fetch-Dest: empty'   -H 'Sec-Fetch-Mode: cors'   -H 'Sec-Fetch-Site: cross-site'   -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36'

Result:

< Access-Control-Allow-Origin: http:https://localhost:5173
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Methods: DELETE, OPTIONS, PATCH, PUT
< Access-Control-Allow-Headers: Access-Control-Allow-Origin, Authorization, Content-Type
< Access-Control-Max-Age: 86400

Expected:

< Access-Control-Allow-Methods: DELETE, OPTIONS, PATCH, POST, PUT

Adding allowMethod(HttpMethod.Post) to the CORS config block does not help (and shouldn't according to the docs, because POST is in the default config).

The config looks ok to me.

Looks like we're using a 2 year old version of ktor though.

[dotasek]

@jmandel if the config is correct, maybe I should take a look at the nginx config on the server to see if it's messing this up. I can't think of an immediately easy way to check cross origin with a local build, but I'm sure there's something.

And yes, this is an older version of ktor. The entire project needs a full update, but this is a bit of a hairy process. See these notes:

org.hl7.fhir.validator-wrapper/gradle.properties

Line 12 in d830da3

# IMPORTANT NOTES ON UPDATING the following kotlin related dependency versions.

[jmandel]

No, my curl reproduction shows the problem (no POST in the allowed methods list) even locally, against a server launched via grandle run without any nginx in the mix. YMMV, so worth double-checking this result.

[dotasek]

OK, after a re-read and some exploration, I don't think this is an issue:

https://stackoverflow.com/questions/71409753/do-browsers-block-post-requests-if-post-isn-t-in-the-access-control-allow-method

GET, HEAD, or POST do not appear in Access-Control-Allow-Methods because there is no requirement that they are explicitly listed.

[jmandel]

Thanks, that's a good reference! Still my local web app example fails with cors errors. Let me explore whether there is some other reason.

[jmandel]

OK, so one issue that's making this challenging to debug is that the server appears to restart (or ... something, returning 503s for several seconds) when an invalid request is submitted.

(Additionally, the server doesn't return CORS headers with 5xx messages, so the client can't see error details.)

Is this restart issue known behavior?

[jmandel]

Ooh, even more pertinent: sending a pre-flight OPTIONS request, which is automatic/required behavior for CORS, triggers the server to restart, which causes the subsequent POST to fail.

Reproduction using just OPTIONS; the server is "down" by the time the 2nd request is sent:

curl -vvv 'https://validator.fhir.org/validate' \
  -X 'OPTIONS' \
  -H 'Access-Control-Request-Headers: content-type' \
  -H 'Access-Control-Request-Method: POST' \
  -H 'Origin: http:https://localhost:5173' \
  -H 'Pragma: no-cache' \
  -H 'Referer: http:https://localhost:5173/' \
  -H 'Sec-Fetch-Dest: empty' \
  -H 'Sec-Fetch-Mode: cors' \
  -H 'Sec-Fetch-Site: cross-site'; \
curl -vvv 'https://validator.fhir.org/validate' \
  -X 'OPTIONS' \
  -H 'Access-Control-Request-Headers: content-type' \
  -H 'Access-Control-Request-Method: POST' \
  -H 'Origin: http:https://localhost:5173' \
  -H 'Pragma: no-cache' \
  -H 'Referer: http:https://localhost:5173/' \
  -H 'Sec-Fetch-Dest: empty' \
  -H 'Sec-Fetch-Mode: cors' \
  -H 'Sec-Fetch-Site: cross-site'

You'll see a valid required followed by a 503.

[dotasek]

I will take a look. One thing this could be is that I set up some limits to connections that are present on the server to prevent abuse.

Two things to try:

Does this behaviour replicate with any OPTIONS request, maybe a totally stripped down version of what you sent?

Can you try this, but point to localhost, where it doesn't live behind NGINX (and might leave a very clear log)?

When I get a moment, I will try this against the server myself.

[jmandel]

I can't get the "validate" operation to return successfully when I run locally, so all my testing has been against the deployed server.

And yes, you can strip the params and reproduce just with:

 curl -X OPTIONS 'https://validator.fhir.org/validate'
 curl -X OPTIONS 'https://validator.fhir.org/validate'

[dotasek]

I can now get that test to pass with a change to the nginx config. There was rate-limiting in place, but I hadn't set the burst parameter, so it would only allow 6 validation queries per minute... so OPTIONS requests and GETs back-to-back were happening under the 10 second wait required by rate limiting.

[jmandel]

Okay. Are these limits applied per...IP? If so that's going to cause issues during conferences etc when many independent users are in the same room.

Could we address limitations by scaling the backend, rather than cutting off requests?

[dotasek]

That's actually a good question; this was configured from nginx's documentation on rate limiting: https://blog.nginx.org/blog/rate-limiting-nginx

A read of this, plus the documentation on the 'limit_req' module (https://nginx.org/en/docs/http/ngx_http_limit_req_module.html), indicates it is indeed the IP that gets used a key in default and most documented configurations.

I think, considering the resource intensive nature of the validator, that we do need some form of rate limiting, even if we scale the backend.

This somewhat ancient doc identifies our exact issue and suggests a workaround: https://serverfault.com/questions/704572/nginx-1-6-2-limit-req-zone-is-there-a-key-that-identifies-unique-users

[jmandel]

I don't think we want to require cookies in the context of cors at least.

It would be good only to kick in hard limits only if/when servers are actually struggling to keep up with load. I'd suggest We might ease up the restrictions and keep an eye on things. Then the logs may be revealing, especially around connectathons.

Curious @grahamegrieve what approach you take to rate limiting in the context of your terminology server.

[dotasek]

I wonder how much access nginx has to the underlying machine... the number of connections doesn't concern me as much as available system resources.