mcp: fix authorization header removal (#5719)

## Summary

Remove Authorization header for the MCP server upstream.

## Related issues

Fix https://github.com/pomerium/pomerium/issues/5718

## User Explanation

<!-- How would you explain this change to the user? If this
change doesn't create any user-facing changes, you can leave
this blank. If filled out, add the `docs` label -->

## Checklist

- [x] reference any related issues
- [x] updated unit tests
- [x] add appropriate label (`enhancement`, `bug`, `breaking`,
`dependencies`, `ci`)
- [x] ready for review
This commit is contained in:
Denis Mishin 2025-07-10 17:37:07 -07:00 committed by GitHub
parent 28da6dc174
commit 8fa26c63f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 27 additions and 8 deletions

View file

@ -75,7 +75,7 @@ func (a *Authorize) handleResultAllowed(
_ *envoy_service_auth_v3.CheckRequest,
result *evaluator.Result,
) (*envoy_service_auth_v3.CheckResponse, error) {
return a.okResponse(result.Headers), nil
return a.okResponse(result.Headers, result.HeadersToRemove), nil
}
func (a *Authorize) handleResultDenied(
@ -115,12 +115,13 @@ func invalidClientCertReason(reasons criteria.Reasons) bool {
reasons.Has(criteria.ReasonInvalidClientCertificate)
}
func (a *Authorize) okResponse(headers http.Header) *envoy_service_auth_v3.CheckResponse {
func (a *Authorize) okResponse(headersToSet http.Header, headersToRemove []string) *envoy_service_auth_v3.CheckResponse {
return &envoy_service_auth_v3.CheckResponse{
Status: &status.Status{Code: int32(codes.OK), Message: "OK"},
HttpResponse: &envoy_service_auth_v3.CheckResponse_OkResponse{
OkResponse: &envoy_service_auth_v3.OkHttpResponse{
Headers: toEnvoyHeaders(headers),
Headers: toEnvoyHeaders(headersToSet),
HeadersToRemove: headersToRemove,
},
},
}
@ -298,7 +299,7 @@ func (a *Authorize) requireWebAuthnResponse(
// If we're already on a webauthn route, return OK.
// https://github.com/pomerium/pomerium-console/issues/3210
if checkRequestURL.Path == urlutil.WebAuthnURLPath || checkRequestURL.Path == urlutil.DeviceEnrolledPath {
return a.okResponse(result.Headers), nil
return a.okResponse(result.Headers, result.HeadersToRemove), nil
}
if !a.shouldRedirect(in, request) {

View file

@ -314,11 +314,26 @@ func TestAuthorize_okResponse(t *testing.T) {
Status: &status.Status{Code: 0, Message: "OK"},
},
},
{
"ok reply with headers to remove",
&evaluator.Result{
Allow: evaluator.NewRuleResult(true),
HeadersToRemove: []string{"x-header-to-remove"},
},
&envoy_service_auth_v3.CheckResponse{
Status: &status.Status{Code: 0, Message: "OK"},
HttpResponse: &envoy_service_auth_v3.CheckResponse_OkResponse{
OkResponse: &envoy_service_auth_v3.OkHttpResponse{
HeadersToRemove: []string{"x-header-to-remove"},
},
},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := a.okResponse(tc.reply.Headers)
got := a.okResponse(tc.reply.Headers, tc.reply.HeadersToRemove)
assert.Equal(t, tc.want.Status.Code, got.Status.Code)
assert.Equal(t, tc.want.Status.Message, got.Status.Message)
want, _ := protojson.Marshal(tc.want.GetOkResponse())

View file

@ -147,6 +147,7 @@ type Result struct {
Allow RuleResult
Deny RuleResult
Headers http.Header
HeadersToRemove []string
Traces []contextutil.PolicyEvaluationTrace
AdditionalLogFields map[log.AuthorizeLogField]any
}
@ -322,6 +323,7 @@ func (e *Evaluator) Evaluate(ctx context.Context, req *Request) (*Result, error)
Allow: policyOutput.Allow,
Deny: policyOutput.Deny,
Headers: headersOutput.Headers,
HeadersToRemove: headersOutput.HeadersToRemove,
Traces: policyOutput.Traces,
AdditionalLogFields: headersOutput.AdditionalLogFields,
}

View file

@ -17,6 +17,7 @@ import (
// HeadersResponse is the output from the headers.rego script.
type HeadersResponse struct {
Headers http.Header
HeadersToRemove []string
AdditionalLogFields map[log.AuthorizeLogField]any
}

View file

@ -123,7 +123,7 @@ func (e *headersEvaluatorEvaluation) fillMCPHeaders(ctx context.Context) (err er
return nil
}
e.response.Headers.Del("Authorization")
e.response.HeadersToRemove = append(e.response.HeadersToRemove, "Authorization")
return nil
}

View file

@ -521,7 +521,7 @@ func TestHeadersEvaluator(t *testing.T) {
})
require.NoError(t, err)
// Should delete Authorization header when no upstream OAuth2 is configured
assert.Empty(t, output.Headers.Get("Authorization"))
assert.Contains(t, output.HeadersToRemove, "Authorization")
})
t.Run("no mcp config", func(t *testing.T) {

View file

@ -84,7 +84,7 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe
a.logAuthorizeCheck(ctx, req, &evaluator.Result{
Allow: evaluator.NewRuleResult(true, criteria.ReasonMCPHandshake),
}, s, u)
return a.okResponse(make(http.Header)), nil
return a.okResponse(make(http.Header), nil), nil
}
}