Skip to content

Commit d481390

Browse files
fix faro.receiver cors not allowing x-scope-orgid, traceparent headers (#4051)
* fix faro.receiver cors not allowing x-scope-orgid, traceparent headers * sort values listed in Access-Control-Request-Headers header in test * fix faro.receiver readme to include allowed headers list * Update docs/sources/reference/components/faro/faro.receiver.md Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com> --------- Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
1 parent 5101137 commit d481390

File tree

4 files changed

+88
-1
lines changed

4 files changed

+88
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ Main (unreleased)
6161

6262
- Fixed a bug in `prometheus.write.queue` which caused retries even when `max_retry_attempts` was set to `0`. (@ptodev)
6363

64+
- Fix issue with `faro.receiver` cors not allowing X-Scope-OrgID and traceparent headers. (@mar4uk)
65+
6466
v1.10.0
6567
-----------------
6668

docs/sources/reference/components/faro/faro.receiver.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ The default value, `[]`, disables CORS support.
105105
To support requests from all origins, set `cors_allowed_origins` to `["*"]`.
106106
The `*` character indicates a wildcard.
107107

108+
You can use the following headers for cross-domain requests: `Content-Type`, `Traceparent`, `X-API-Key`, `X-Faro-Session-Id`, or `X-Scope-OrgID`.
109+
108110
When the `api_key` argument is non-empty, client requests must have an HTTP header called `X-API-Key` matching the value of the `api_key` argument.
109111
Requests that are missing the header or have the wrong value are rejected with an `HTTP 401 Unauthorized` status code.
110112
If the `api_key` argument is empty, no authentication checks are performed, and the `X-API-Key` HTTP header is ignored.

internal/component/faro/receiver/handler.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919

2020
const apiKeyHeader = "x-api-key"
2121

22+
var defaultAllowedHeaders = []string{"content-type", "traceparent", apiKeyHeader, "x-faro-session-id", "x-scope-orgid"}
23+
2224
type handler struct {
2325
log log.Logger
2426
rateLimiter *rate.Limiter
@@ -71,7 +73,7 @@ func (h *handler) Update(args ServerArguments) {
7173
if len(args.CORSAllowedOrigins) > 0 {
7274
h.cors = cors.New(cors.Options{
7375
AllowedOrigins: args.CORSAllowedOrigins,
74-
AllowedHeaders: []string{apiKeyHeader, "content-type", "x-faro-session-id"},
76+
AllowedHeaders: defaultAllowedHeaders,
7577
})
7678
} else {
7779
h.cors = nil // Disable cors.

internal/component/faro/receiver/handler_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"net/http"
77
"net/http/httptest"
8+
"sort"
89
"strings"
910
"testing"
1011

@@ -227,6 +228,86 @@ func TestValidAPIKey(t *testing.T) {
227228
require.Len(t, exporter2.payloads, 1)
228229
}
229230

231+
func TestCORSPreflightWithDisallowedHeader(t *testing.T) {
232+
var (
233+
exporter1 = &testExporter{"exporter1", false, nil}
234+
exporter2 = &testExporter{"exporter2", false, nil}
235+
236+
h = newHandler(
237+
util.TestLogger(t),
238+
prometheus.NewRegistry(),
239+
[]exporter{exporter1, exporter2},
240+
)
241+
)
242+
243+
h.Update(ServerArguments{
244+
CORSAllowedOrigins: []string{
245+
"https://example.com",
246+
},
247+
})
248+
249+
// Test preflight request with disallowed header
250+
req, err := http.NewRequest(http.MethodOptions, "/collect", nil)
251+
require.NoError(t, err)
252+
req.Header.Set("Origin", "https://example.com")
253+
req.Header.Set("Access-Control-Request-Method", "POST")
254+
req.Header.Set("Access-Control-Request-Headers", "x-custom-header")
255+
256+
rr := httptest.NewRecorder()
257+
h.ServeHTTP(rr, req)
258+
259+
// The preflight should succeed (204), but x-custom-header should NOT be in allowed headers
260+
require.Equal(t, http.StatusNoContent, rr.Result().StatusCode)
261+
allowedHeaders := rr.Header().Get("Access-Control-Allow-Headers")
262+
263+
// When requesting a disallowed header, CORS returns empty Access-Control-Allow-Headers
264+
// This effectively tells the browser that no custom headers are allowed
265+
require.Equal(t, "", allowedHeaders, "CORS should return empty allowed headers when disallowed header is requested")
266+
}
267+
268+
func TestCORSPreflightWithAllowedHeader(t *testing.T) {
269+
var (
270+
exporter1 = &testExporter{"exporter1", false, nil}
271+
exporter2 = &testExporter{"exporter2", false, nil}
272+
273+
h = newHandler(
274+
util.TestLogger(t),
275+
prometheus.NewRegistry(),
276+
[]exporter{exporter1, exporter2},
277+
)
278+
)
279+
280+
h.Update(ServerArguments{
281+
CORSAllowedOrigins: []string{
282+
"https://example.com",
283+
},
284+
})
285+
286+
// Test preflight request with allowed headers
287+
req, err := http.NewRequest(http.MethodOptions, "/collect", nil)
288+
require.NoError(t, err)
289+
req.Header.Set("Origin", "https://example.com")
290+
req.Header.Set("Access-Control-Request-Method", "POST")
291+
292+
sortedHeaders := make([]string, 0, len(defaultAllowedHeaders))
293+
sortedHeaders = append(sortedHeaders, defaultAllowedHeaders...)
294+
sort.Strings(sortedHeaders)
295+
// Library github.com/rs/cors expects values listed in Access-Control-Request-Headers header
296+
// are unique and sorted;
297+
// see https://github.com/rs/cors/blob/1084d89a16921942356d1c831fbe523426cf836e/cors.go#L115-L120
298+
req.Header.Set("Access-Control-Request-Headers", strings.Join(sortedHeaders, ","))
299+
300+
rr := httptest.NewRecorder()
301+
h.ServeHTTP(rr, req)
302+
303+
// The preflight should succeed and include the requested headers
304+
require.Equal(t, http.StatusNoContent, rr.Result().StatusCode)
305+
allowedHeaders := rr.Header().Get("Access-Control-Allow-Headers")
306+
for _, allowed := range defaultAllowedHeaders {
307+
require.Contains(t, allowedHeaders, allowed)
308+
}
309+
}
310+
230311
func TestRateLimiter(t *testing.T) {
231312
var (
232313
exporter1 = &testExporter{"exporter1", false, nil}

0 commit comments

Comments
 (0)