Skip to content

Commit

Permalink
Fixed CORS behaviour: result is now only dependent on the config, not…
Browse files Browse the repository at this point in the history 
… the actual request; fixed Vary headers
  • Loading branch information
Johannes Koch committed Mar 30, 2021
1 parent ca24f46 commit 8890bbb
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 93 deletions.
16 changes: 8 additions & 8 deletions handler/middleware/cors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,25 @@ func (c *CORS) setCorsRespHeaders(headers http.Header, req *http.Request) {
if !c.isCorsRequest(req) {
return
}

requestOrigin := req.Header.Get("Origin")
if !c.options.AllowsOrigin(requestOrigin) {
return
}

// see https://fetch.spec.whatwg.org/#http-responses
if c.options.AllowsOrigin("*") && !c.isCredentialed(req.Header) {
if !c.options.AllowsOrigin("*") {
headers.Set("Access-Control-Allow-Origin", requestOrigin)
headers.Add("Vary", "Origin")
} else if !c.options.AllowCredentials {
headers.Set("Access-Control-Allow-Origin", "*")
} else {
} else if requestOrigin != "" {
headers.Set("Access-Control-Allow-Origin", requestOrigin)
}

if c.options.AllowCredentials == true {
headers.Set("Access-Control-Allow-Credentials", "true")
headers.Add("Vary", "Origin")
}

if c.isCorsPreflightRequest(req) {
Expand All @@ -131,15 +137,9 @@ func (c *CORS) setCorsRespHeaders(headers http.Header, req *http.Request) {
if c.options.MaxAge != "" {
headers.Set("Access-Control-Max-Age", c.options.MaxAge)
}
} else if c.options.NeedsVary() {
headers.Add("Vary", "Origin")
}
}

func (c *CORS) isCorsRequest(req *http.Request) bool {
return req.Header.Get("Origin") != ""
}

func (c *CORS) isCredentialed(headers http.Header) bool {
return headers.Get("Cookie") != "" || headers.Get("Authorization") != "" || headers.Get("Proxy-Authorization") != ""
}
136 changes: 51 additions & 85 deletions handler/middleware/cors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,50 +199,6 @@ func TestCORSOptions_isCorsPreflightRequest(t *testing.T) {
}
}

func TestCORS_IsCredentialed(t *testing.T) {
type testCase struct {
name string
requestHeaders map[string]string
exp bool
}

tests := []testCase{
{
"Cookie",
map[string]string{"Cookie": "a=b"},
true,
},
{
"Authorization",
map[string]string{"Authorization": "Basic qeinbqtpoib"},
true,
},
{
"Proxy-Authorization",
map[string]string{"Proxy-Authorization": "Basic qeinbqtpoib"},
true,
},
{
"Not credentialed",
map[string]string{},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "http://1.2.3.4/", nil)
for name, value := range tt.requestHeaders {
req.Header.Set(name, value)
}

credentialed := NewCORSHandler(nil, nil).(*CORS).isCredentialed(req.Header)
if credentialed != tt.exp {
t.Errorf("expected: %t, got: %t", tt.exp, credentialed)
}
})
}
}

func TestCORS_ServeHTTP(t *testing.T) {
upstreamHandler := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
rw.Header().Set("Content-Type", "text/plain")
Expand Down Expand Up @@ -308,7 +264,7 @@ func TestCORS_ServeHTTP(t *testing.T) {
},
},
{
"specific origin, cookie credentials",
"specific origin, credentials",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -321,20 +277,7 @@ func TestCORS_ServeHTTP(t *testing.T) {
},
},
{
"specific origin, auth credentials",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
"Authorization": "Basic oertnbin",
},
map[string]string{
"Access-Control-Allow-Origin": "https://www.example.com",
"Access-Control-Allow-Credentials": "true",
"Vary": "Origin",
},
},
{
"any origin, cookie credentials",
"any origin, credentials",
&CORSOptions{AllowedOrigins: []string{"*"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -343,32 +286,18 @@ func TestCORS_ServeHTTP(t *testing.T) {
map[string]string{
"Access-Control-Allow-Origin": "https://www.example.com",
"Access-Control-Allow-Credentials": "true",
"Vary": "",
},
},
{
"any origin, auth credentials",
&CORSOptions{AllowedOrigins: []string{"*"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
"Authorization": "Basic oertnbin",
},
map[string]string{
"Access-Control-Allow-Origin": "https://www.example.com",
"Access-Control-Allow-Credentials": "true",
"Vary": "",
"Vary": "Origin",
},
},
{
"any origin, proxy auth credentials",
&CORSOptions{AllowedOrigins: []string{"*"}, AllowCredentials: true},
"origin mismatch",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}},
map[string]string{
"Origin": "https://www.example.com",
"Proxy-Authorization": "Basic oertnbin",
"Origin": "https://www.example.org",
},
map[string]string{
"Access-Control-Allow-Origin": "https://www.example.com",
"Access-Control-Allow-Credentials": "true",
"Access-Control-Allow-Origin": "",
"Access-Control-Allow-Credentials": "",
"Vary": "",
},
},
Expand Down Expand Up @@ -424,7 +353,7 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
expectedResponseHeaders map[string]string
}{
{
"with ACRM",
"specific origin, with ACRM",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -436,10 +365,11 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "",
"Vary": "Origin",
},
},
{
"with ACRH",
"specific origin, with ACRH",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -451,10 +381,11 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "X-Foo, X-Bar",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "",
"Vary": "Origin",
},
},
{
"with ACRM, ACRH",
"specific origin, with ACRM, ACRH",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -467,10 +398,11 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "X-Foo, X-Bar",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "",
"Vary": "Origin",
},
},
{
"with ACRM, credentials",
"specific origin, with ACRM, credentials",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -482,10 +414,11 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "true",
"Access-Control-Max-Age": "",
"Vary": "Origin",
},
},
{
"with ACRM, max-age",
"specific origin, with ACRM, max-age",
&CORSOptions{AllowedOrigins: []string{"https://www.example.com"}, MaxAge: "3600"},
map[string]string{
"Origin": "https://www.example.com",
Expand All @@ -497,6 +430,39 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "3600",
"Vary": "Origin",
},
},
{
"any origin, with ACRM",
&CORSOptions{AllowedOrigins: []string{"*"}},
map[string]string{
"Origin": "https://www.example.com",
"Access-Control-Request-Method": "POST",
},
map[string]string{
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "POST",
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "",
"Vary": "",
},
},
{
"any origin, with ACRM, credentials",
&CORSOptions{AllowedOrigins: []string{"*"}, AllowCredentials: true},
map[string]string{
"Origin": "https://www.example.com",
"Access-Control-Request-Method": "POST",
},
map[string]string{
"Access-Control-Allow-Origin": "https://www.example.com",
"Access-Control-Allow-Methods": "POST",
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "true",
"Access-Control-Max-Age": "",
"Vary": "Origin",
},
},
{
Expand All @@ -512,6 +478,7 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {
"Access-Control-Allow-Headers": "",
"Access-Control-Allow-Credentials": "",
"Access-Control-Max-Age": "",
"Vary": "",
},
},
}
Expand All @@ -534,7 +501,6 @@ func TestProxy_ServeHTTP_CORS_PFC(t *testing.T) {

res := rec.Result()

tt.expectedResponseHeaders["Vary"] = ""
tt.expectedResponseHeaders["Content-Type"] = ""

for name, expValue := range tt.expectedResponseHeaders {
Expand Down

0 comments on commit 8890bbb

Please sign in to comment.