Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Vault42 issues its own tokens and is an OAuth2 *client* of other providers. It i
| ![Go](https://img.shields.io/badge/Go-1.26.6-00ADD8?style=flat&logo=go&logoColor=white) | ![Vue](https://img.shields.io/badge/Vue-3.5.41-4FC08D?style=flat&logo=vuedotjs&logoColor=white) | ![.NET](https://img.shields.io/badge/.NET-10.0-512BD4?style=flat&logo=dotnet&logoColor=white) | ![License](https://img.shields.io/badge/License-MIT-155724?style=flat&labelColor=000) |
| ![Go Tests](https://img.shields.io/badge/Tests-4910-155724?style=flat&labelColor=000) | ![Vue Tests](https://img.shields.io/badge/Tests-1305-155724?style=flat&labelColor=000) | ![C# Tests](https://img.shields.io/badge/Tests-264-155724?style=flat&labelColor=000) | ![Total](https://img.shields.io/badge/Total-6479_tests-155724?style=flat&labelColor=000) |
| ![Go Coverage](https://img.shields.io/badge/Coverage-100.00%25_reachable-155724?style=flat&labelColor=000) | ![Vue Coverage](https://img.shields.io/badge/Coverage-99.76%25-155724?style=flat&labelColor=000) | ![C# Coverage](https://img.shields.io/badge/Coverage-100.00%25-155724?style=flat&labelColor=000) | ![Locales](https://img.shields.io/badge/Locales-38-555?style=flat&labelColor=000) |
| ![Go Lines](https://img.shields.io/badge/Lines-48262-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) |
| ![Go Lines](https://img.shields.io/badge/Lines-48278-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) |
| ![Go Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![Vue Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![C# Deps](https://img.shields.io/badge/Deps-6-555?style=flat&labelColor=000) | ![Requirements](https://img.shields.io/badge/Requirements-456-555?style=flat&labelColor=000) |
| ![Go Transitive Deps](https://img.shields.io/badge/Transitive-15-555?style=flat&labelColor=000) | ![Vue Transitive Deps](https://img.shields.io/badge/Transitive-95-555?style=flat&labelColor=000) | ![C# Transitive Deps](https://img.shields.io/badge/Transitive-26-555?style=flat&labelColor=000) | ![Total Deps](https://img.shields.io/badge/Deps-148_total-555?style=flat&labelColor=000) |
<!-- /badges -->
Expand Down
6 changes: 3 additions & 3 deletions docs/badges.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"reachableCoverageNum": 100.00,
"packages": 43,
"goFiles": 192,
"goLines": 48262,
"testFiles": 930,
"goLines": 48278,
"testFiles": 931,
"directDeps": 3,
"transitiveDeps": 15,
"totalTests": 6479,
Expand All @@ -20,7 +20,7 @@
"tests": 4910,
"coverage": "100.00% of reachable",
"coverageNum": 100.00,
"lines": 48262,
"lines": 48278,
"deps": 3,
"transitiveDeps": 15
},
Expand Down
4 changes: 2 additions & 2 deletions docs/compliance-register.json
Original file line number Diff line number Diff line change
Expand Up @@ -1316,13 +1316,13 @@
],
"status": "Met",
"evidence": [
"internal/middleware/cors.go#w.Header().Set(\"Access-Control-Allow-Headers\", \"Content-Type, Authorization, DPoP\")",
"internal/middleware/cors.go#w.Header().Set(\"Access-Control-Allow-Headers\",",
"internal/middleware/cors.go#if r.Method == http.MethodOptions {"
],
"tests": [
"TestASVS_V3_5_2_SensitiveFunctionalityAlwaysTriggersPreflight"
],
"notes": "Met: the preflight response names a closed header set, Content-Type, Authorization and DPoP (internal/middleware/cors.go:62), with no wildcard, and OPTIONS is answered 204 by the middleware (:68). Every authenticated route requires Authorization, which is not a CORS-safelisted request header, so a cross-origin call to sensitive functionality always preflights; a simple request that skips preflight arrives without it and is answered 401. TestASVS_V3_5_2_SensitiveFunctionalityAlwaysTriggersPreflight asserts both halves. Previously Not Applicable."
"notes": "Met: the preflight response names a closed header set -- Content-Type, Authorization, DPoP, X-Requested-With, X-Blob-Checksum and X-Blob-Label (internal/middleware/cors.go:69) -- with no wildcard, and OPTIONS is answered 204 by the middleware (:84). The list grew to cover the headers the SDK actually sends; that does not weaken this control, because every one of them is itself non-safelisted, so a request carrying one still preflights. Every authenticated route requires Authorization, which is not a CORS-safelisted request header, so a cross-origin call to sensitive functionality always preflights; a simple request that skips preflight arrives without it and is answered 401. TestASVS_V3_5_2_SensitiveFunctionalityAlwaysTriggersPreflight asserts both halves. Previously Not Applicable."
},
{
"standard": "OWASP ASVS",
Expand Down
18 changes: 17 additions & 1 deletion internal/middleware/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,23 @@ func CORS(allowedOrigin string, additionalOrigins []string, allowAll bool) func(
}
w.Header().Set("Vary", "Origin, Access-Control-Request-Method, Access-Control-Request-Headers")
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization, DPoP")
// X-Requested-With is on every SDK request; X-Blob-Checksum and
// X-Blob-Label are on the blob writes. None of the three is
// CORS-safelisted, so a cross-origin preflight that omits them fails
// the request before it is sent -- which made the documented
// cross-origin deployment mode unusable rather than merely awkward.
// X-Vault-App is deliberately absent: it is proxy-set, so no browser
// ever asks for it.
w.Header().Set("Access-Control-Allow-Headers",
"Content-Type, Authorization, DPoP, X-Requested-With, X-Blob-Checksum, X-Blob-Label")
// Without this the browser hands the SDK a response whose custom
// headers read as null. The blob helpers return the checksum and
// label to the caller, so same-origin they carry a value and
// cross-origin they silently do not. Retry-After is exposed because a
// client cannot honor a 429 it cannot read; the server already sets
// it on the rate-limited paths.
w.Header().Set("Access-Control-Expose-Headers",
"X-Blob-Checksum, X-Blob-Label, Retry-After")
if origin != "" && origin != "*" {
w.Header().Set("Access-Control-Allow-Credentials", "true")
}
Expand Down
10 changes: 7 additions & 3 deletions internal/middleware/cors_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,13 @@ func TestCORS_Preflight_AllHeaders(t *testing.T) {
})

t.Run("allow-headers includes Authorization", func(t *testing.T) {
// Containment, not equality. Both subtests are named "includes X" and
// both compared the whole string, so every legitimate addition to the
// list broke them while proving nothing the name claimed. The list has
// since had to grow for the SDK's own headers.
got := rec.Header().Get("Access-Control-Allow-Headers")
if got != "Content-Type, Authorization, DPoP" {
t.Errorf("allow-headers = %q", got)
if !strings.Contains(got, "Authorization") {
t.Errorf("allow-headers = %q, should include Authorization", got)
}
})

Expand Down Expand Up @@ -285,7 +289,7 @@ func TestCORS_AllowAll_DPoPHeaderIncluded(t *testing.T) {

t.Run("allow-headers includes DPoP", func(t *testing.T) {
got := rec.Header().Get("Access-Control-Allow-Headers")
if got != "Content-Type, Authorization, DPoP" {
if !strings.Contains(got, "DPoP") {
t.Errorf("allow-headers = %q, should include DPoP", got)
}
})
Expand Down
99 changes: 99 additions & 0 deletions internal/middleware/cors_sdk_headers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package middleware

import (
"net/http"
"net/http/httptest"
"strings"
"testing"
)

// The SDK's own requests have to survive a cross-origin preflight, and its own
// responses have to be readable.
//
// Neither was true. Access-Control-Allow-Headers listed Content-Type,
// Authorization and DPoP; the Vue SDK sends X-Requested-With on every request
// and X-Blob-Checksum and X-Blob-Label on blob writes. None of those three is
// CORS-safelisted, so the browser refuses the request at preflight and it never
// reaches vault42 at all. And with no Access-Control-Expose-Headers at all, the
// blob helpers -- which return the checksum and the label to their caller --
// read null cross-origin while working perfectly same-origin.
//
// That is the failure shape worth naming: the documented cross-origin mode did
// not degrade, it did not error usefully, it simply did not work, and every test
// in the suite is same-origin so nothing saw it.
//
// These lists are duplicated deliberately. The middleware states what is
// allowed; this test states what the SDK needs. If either moves without the
// other, one of them is wrong and this fails rather than a deployment doing it.

// sdkRequestHeaders are the non-safelisted headers the SDK sets on a request.
// X-Vault-App is not here: it is proxy-set, so no browser asks for it.
var sdkRequestHeaders = []string{"X-Requested-With", "X-Blob-Checksum", "X-Blob-Label"}

// sdkResponseHeaders are the headers a browser client has to be able to read.
var sdkResponseHeaders = []string{"X-Blob-Checksum", "X-Blob-Label", "Retry-After"}

func corsPreflight(t *testing.T, origin string) http.Header {
t.Helper()
h := CORS(origin, nil, false)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/user/blobs", nil)
req.Header.Set("Origin", origin)
req.Header.Set("Access-Control-Request-Method", "POST")
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)
return rec.Result().Header
}

func TestCORSAllowsEveryHeaderTheSDKSends(t *testing.T) {
got := corsPreflight(t, "https://app.example.com")
allowed := got.Get("Access-Control-Allow-Headers")
if allowed == "" {
t.Fatal("no Access-Control-Allow-Headers on a preflight response")
}

lower := strings.ToLower(allowed)
for _, want := range sdkRequestHeaders {
if !strings.Contains(lower, strings.ToLower(want)) {
t.Errorf("Access-Control-Allow-Headers omits %s, which the SDK sets on its "+
"requests. The browser refuses the request at preflight, so it never "+
"reaches vault42 and no server-side test can see it.\ngot: %s", want, allowed)
}
}
}

func TestCORSExposesEveryHeaderTheSDKReads(t *testing.T) {
got := corsPreflight(t, "https://app.example.com")
exposed := got.Get("Access-Control-Expose-Headers")
if exposed == "" {
t.Fatal("no Access-Control-Expose-Headers. Only the CORS-safelisted response " +
"headers reach a cross-origin caller, and none of the ones vault42 sets " +
"is on that list, so they all read null.")
}

lower := strings.ToLower(exposed)
for _, want := range sdkResponseHeaders {
if !strings.Contains(lower, strings.ToLower(want)) {
t.Errorf("Access-Control-Expose-Headers omits %s, so a cross-origin caller "+
"reads it as null while the same call works same-origin\ngot: %s", want, exposed)
}
}
}

// Widening the allow-list must not have widened the origin check with it: the
// headers are advertised, the origin is still the one configured.
func TestCORSStillRefusesAnUnconfiguredOrigin(t *testing.T) {
h := CORS("https://app.example.com", nil, false)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/user/blobs", nil)
req.Header.Set("Origin", "https://evil.test")
req.Header.Set("Access-Control-Request-Method", "POST")
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)

if got := rec.Result().Header.Get("Access-Control-Allow-Origin"); got == "https://evil.test" {
t.Fatalf("an unconfigured origin was allowed: %q", got)
}
}
Loading