Allow custom headers to be returned as part of pre-flight requests #423
Conversation
miike
force-pushed
the
acah
branch
2 times, most recently
from
8d28487 to
2d7a3c8
Compare
| val allowedHeaders = (List("Content-Type", "SP-Anonymous") ++ config.cors.allowedCorsHeaders).map(CIString(_)) | ||
| val corsHeaders = NonEmptyList.fromListUnsafe(allowedHeaders) |
There was a problem hiding this comment.
Think we can remove the "unsafe" method. Which is harmless in this case, but stands out as worrisome when you see it in collector code.
val corsHeaders = NonEmptyList("Content-Type", "SP-Anonymous" :: config.cors.allowedCorsHeaders).map(CIString(_))| @@ -52,6 +52,7 @@ | |||
|
|
|||
| cors { | |||
| accessControlMaxAge = 60 minutes | |||
| allowedCorsHeaders = ["X-Example"] | |||
There was a problem hiding this comment.
This file provides defaults for the app. So by adding "X-Example" here, it means all pipelines everywhere will respond with "X-Example" in the list of allowed headers.
Guessing that's probably not what you want?
| @@ -139,11 +140,13 @@ class Service[F[_]: Sync]( | |||
| } | |||
|
|
|||
| override def preflightResponse(req: Request[F]): F[Response[F]] = Sync[F].pure { | |||
| val allowedHeaders = (List("Content-Type", "SP-Anonymous") ++ config.cors.allowedCorsHeaders).map(CIString(_)) | |||
There was a problem hiding this comment.
What about just making Content-Type and SP-Anonymous a part of the allowedCorsHeaders setting? That would avoid rebuilding this list on every request. And actually allowedCorsHeaders is a misnomer if it does not include all allowed headers.
There was a problem hiding this comment.
Do you mean as part of the config? My only concern would be if people remove this from a default config.
Certain services (e.g., Datadog) have a requirement for adding non CORS-safelisted headers to network requests in order to record tracing information for later reporting.
Currently requests which included non-safelisted headers error out when performing the pre-flight OPTIONS request. This adds the ability to specify a custom list (in addition to the default headers) that are returned as part of the
Access-Control-Allow-Headersresponse.Context: https://discourse.snowplow.io/t/access-to-xmlhttprequest-has-been-blocked-by-cors-policy-request-header-field-traceparent-is-not-allowed-by-access-control-allow-headers-in-preflight-response/9922/3