Skip to content
Open
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
10 changes: 10 additions & 0 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,16 @@
"type": "string"
}
},
"forward_request_headers_to_remote": {
"description": "A list of inbound request headers to forward to the remote authorizer.",
"title": "Allowed Inbound HTTP Headers to Forward to the Remote",
"type": "array",
"items": {
"type": "string"
},
"uniqueItems": true,
"default": []
},
"forward_response_headers_to_upstream": {
"description": "A list of non simple headers the remote is allowed to return to mutate requests.",
"title": "Allowed Remote HTTP Headers for his Responses",
Expand Down
11 changes: 11 additions & 0 deletions pipeline/authz/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
type AuthorizerRemoteConfiguration struct {
Remote string `json:"remote"`
Headers map[string]string `json:"headers"`
ForwardRequestHeadersToRemote []string `json:"forward_request_headers_to_remote"`
ForwardResponseHeadersToUpstream []string `json:"forward_response_headers_to_upstream"`
Retry *AuthorizerRemoteRetryConfiguration `json:"retry"`
}
Expand Down Expand Up @@ -86,6 +87,16 @@ func (a *AuthorizerRemote) Authorize(r *http.Request, session *authn.Authenticat
if err != nil {
return errors.WithStack(err)
}
for _, name := range c.ForwardRequestHeadersToRemote {
values := r.Header.Values(name)
if len(values) == 0 {
continue
}
req.Header.Del(name)
for _, v := range values {
req.Header.Add(name, v)
}
}
Comment on lines +90 to +99

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid duplicate Authorization / Content-Type when forwarding configured headers.

If forward_request_headers_to_remote contains Authorization or Content-Type, this loop adds them, and Lines 100-104 append them again. That can send duplicated auth/content headers to the remote authorizer.

Proposed fix
-	for _, name := range c.ForwardRequestHeadersToRemote {
-		values := r.Header.Values(name)
-		if len(values) == 0 {
-			continue
-		}
-		req.Header.Del(name)
-		for _, v := range values {
-			req.Header.Add(name, v)
-		}
-	}
-	req.Header.Add("Content-Type", r.Header.Get("Content-Type"))
+	req.Header.Set("Content-Type", r.Header.Get("Content-Type"))
 	authz := r.Header.Get("Authorization")
 	if authz != "" {
-		req.Header.Add("Authorization", authz)
+		req.Header.Set("Authorization", authz)
+	}
+	for _, name := range c.ForwardRequestHeadersToRemote {
+		values := r.Header.Values(name)
+		if len(values) == 0 {
+			continue
+		}
+		req.Header.Del(name)
+		for _, v := range values {
+			req.Header.Add(name, v)
+		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for _, name := range c.ForwardRequestHeadersToRemote {
values := r.Header.Values(name)
if len(values) == 0 {
continue
}
req.Header.Del(name)
for _, v := range values {
req.Header.Add(name, v)
}
}
req.Header.Set("Content-Type", r.Header.Get("Content-Type"))
authz := r.Header.Get("Authorization")
if authz != "" {
req.Header.Set("Authorization", authz)
}
for _, name := range c.ForwardRequestHeadersToRemote {
values := r.Header.Values(name)
if len(values) == 0 {
continue
}
req.Header.Del(name)
for _, v := range values {
req.Header.Add(name, v)
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pipeline/authz/remote.go` around lines 90 - 99, The loop that copies headers
from r to req using c.ForwardRequestHeadersToRemote can cause duplicate
Authorization or Content-Type because those headers are also appended later;
update the loop in remote.go (the block iterating over
c.ForwardRequestHeadersToRemote and using r.Header.Values/name,
req.Header.Del/Add) to skip forwarding when name equals "Authorization" or
"Content-Type" (case-insensitive), or otherwise de-duplicate by checking
req.Header.Get(name) before adding, so the later explicit append of
Authorization/Content-Type (lines after this loop) will not produce duplicates.

req.Header.Add("Content-Type", r.Header.Get("Content-Type"))
authz := r.Header.Get("Authorization")
if authz != "" {
Expand Down
22 changes: 22 additions & 0 deletions pipeline/authz/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestAuthorizerRemoteAuthorize(t *testing.T) {
setup func(t *testing.T) *httptest.Server
session *authn.AuthenticationSession
sessionHeaderMatch *http.Header
requestHeaders http.Header
body string
config json.RawMessage
wantErr bool
Expand Down Expand Up @@ -161,6 +162,24 @@ func TestAuthorizerRemoteAuthorize(t *testing.T) {
sessionHeaderMatch: &http.Header{"X-Foo": []string{""}},
config: json.RawMessage(`{"forward_response_headers_to_upstream":["X-Foo"]}`),
},
{
name: "ok with forwarded request headers",
setup: func(t *testing.T) *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "sig-value", r.Header.Get("X-Signature"))
assert.Equal(t, "ts-value", r.Header.Get("X-Timestamp"))
assert.Empty(t, r.Header.Get("X-Not-Forwarded"))
w.WriteHeader(http.StatusOK)
}))
},
session: &authn.AuthenticationSession{},
requestHeaders: http.Header{
"X-Signature": {"sig-value"},
"X-Timestamp": {"ts-value"},
"X-Not-Forwarded": {"nope"},
},
config: json.RawMessage(`{"forward_request_headers_to_remote":["X-Signature","X-Timestamp"]}`),
},
{
name: "authentication session",
setup: func(t *testing.T) *httptest.Server {
Expand Down Expand Up @@ -201,6 +220,9 @@ func TestAuthorizerRemoteAuthorize(t *testing.T) {
"User-Agent": {"Fancy Browser 5.1"},
},
}
for name, values := range tt.requestHeaders {
r.Header[name] = values
}
if tt.body != "" {
r.Body = io.NopCloser(strings.NewReader(tt.body))
}
Expand Down
10 changes: 10 additions & 0 deletions spec/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,16 @@
"type": "string"
}
},
"forward_request_headers_to_remote": {
"description": "A list of inbound request headers to forward to the remote authorizer.",
"title": "Allowed Inbound HTTP Headers to Forward to the Remote",
"type": "array",
"items": {
"type": "string"
},
"uniqueItems": true,
"default": []
},
"forward_response_headers_to_upstream": {
"description": "A list of non simple headers the remote is allowed to return to mutate requests.",
"title": "Allowed Remote HTTP Headers for his Responses",
Expand Down
Loading