feat: forward request headers to remote authorizer#1271
Open
richiejenkins wants to merge 1 commit into
Open
Conversation
Adds a forward_request_headers_to_remote config field on the remote authorizer. When set, the listed headers from the inbound request are copied onto the outbound request to the remote authorization endpoint. Useful for forwarding signature, timestamp, or nonce headers to an external verifier service. Mirrors the existing forward_response_headers_to_upstream field but in the request direction. Default behavior is unchanged when the field is omitted. Signed-off-by: Richie Jenkins <53573503+richiejenkins@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a configurable Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pipeline/authz/remote.go`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68a40fd2-22c6-482b-bffb-4b7d4ad72166
📒 Files selected for processing (4)
.schema/config.schema.jsonpipeline/authz/remote.gopipeline/authz/remote_test.gospec/config.schema.json
Comment on lines
+90
to
+99
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
forward_request_headers_to_remoteconfig field on theremoteauthorizer. When set, listed headers are copied from the inbound request to the outbound request to the remote endpoint.Useful for forwarding signature, timestamp, or nonce headers to an external verifier service. Mirrors the existing
forward_response_headers_to_upstreamfield, in the request direction.Related issue(s)
#1270
Checklist
introduces a new feature.
Summary by CodeRabbit
New Features
Tests