From 0fc63ea0baf9fc71bab876ce250e6b5f85e84731 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Tue, 24 Mar 2026 23:18:21 +0800 Subject: [PATCH] [management] Allow multiple header auths with same header name (#5678) --- .../modules/reverseproxy/service/service.go | 7 +- .../reverseproxy/service/service_test.go | 104 ++++++++++++++++++ proxy/internal/auth/middleware_test.go | 68 ++++++++++++ 3 files changed, 173 insertions(+), 6 deletions(-) diff --git a/management/internals/modules/reverseproxy/service/service.go b/management/internals/modules/reverseproxy/service/service.go index be04777a1..7ca2c3043 100644 --- a/management/internals/modules/reverseproxy/service/service.go +++ b/management/internals/modules/reverseproxy/service/service.go @@ -850,7 +850,7 @@ func IsPortBasedProtocol(mode string) bool { } const ( - maxCustomHeaders = 16 + maxCustomHeaders = 16 maxHeaderKeyLen = 128 maxHeaderValueLen = 4096 ) @@ -947,7 +947,6 @@ func containsCRLF(s string) bool { } func validateHeaderAuths(headers []*HeaderAuthConfig) error { - seen := make(map[string]struct{}) for i, h := range headers { if h == nil || !h.Enabled { continue @@ -968,10 +967,6 @@ func validateHeaderAuths(headers []*HeaderAuthConfig) error { if canonical == "Host" { return fmt.Errorf("header_auths[%d]: Host header cannot be used for auth", i) } - if _, dup := seen[canonical]; dup { - return fmt.Errorf("header_auths[%d]: duplicate header %q (same canonical form already configured)", i, h.Header) - } - seen[canonical] = struct{}{} if len(h.Value) > maxHeaderValueLen { return fmt.Errorf("header_auths[%d]: value exceeds maximum length of %d", i, maxHeaderValueLen) } diff --git a/management/internals/modules/reverseproxy/service/service_test.go b/management/internals/modules/reverseproxy/service/service_test.go index 3fe07b1d0..ff54cb79f 100644 --- a/management/internals/modules/reverseproxy/service/service_test.go +++ b/management/internals/modules/reverseproxy/service/service_test.go @@ -935,3 +935,107 @@ func TestExposeServiceRequest_Validate_HTTPAllowsAuth(t *testing.T) { req := ExposeServiceRequest{Port: 8080, Mode: "http", Pin: "123456"} require.NoError(t, req.Validate()) } + +func TestValidate_HeaderAuths(t *testing.T) { + t.Run("single valid header", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "X-API-Key", Value: "secret"}, + }, + } + require.NoError(t, rp.Validate()) + }) + + t.Run("multiple headers same canonical name allowed", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "Authorization", Value: "Bearer token-1"}, + {Enabled: true, Header: "Authorization", Value: "Bearer token-2"}, + }, + } + require.NoError(t, rp.Validate()) + }) + + t.Run("multiple headers different case same canonical allowed", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "x-api-key", Value: "key-1"}, + {Enabled: true, Header: "X-Api-Key", Value: "key-2"}, + }, + } + require.NoError(t, rp.Validate()) + }) + + t.Run("multiple different headers allowed", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "Authorization", Value: "Bearer tok"}, + {Enabled: true, Header: "X-API-Key", Value: "key"}, + }, + } + require.NoError(t, rp.Validate()) + }) + + t.Run("empty header name rejected", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "", Value: "val"}, + }, + } + err := rp.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "header name is required") + }) + + t.Run("hop-by-hop header rejected", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "Connection", Value: "val"}, + }, + } + err := rp.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "hop-by-hop") + }) + + t.Run("host header rejected", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "Host", Value: "val"}, + }, + } + err := rp.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "Host header cannot be used") + }) + + t.Run("disabled entries skipped", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: false, Header: "", Value: ""}, + {Enabled: true, Header: "X-Key", Value: "val"}, + }, + } + require.NoError(t, rp.Validate()) + }) + + t.Run("value too long rejected", func(t *testing.T) { + rp := validProxy() + rp.Auth = AuthConfig{ + HeaderAuths: []*HeaderAuthConfig{ + {Enabled: true, Header: "X-Key", Value: strings.Repeat("a", maxHeaderValueLen+1)}, + }, + } + err := rp.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds maximum length") + }) +} diff --git a/proxy/internal/auth/middleware_test.go b/proxy/internal/auth/middleware_test.go index a4924d380..6063f070e 100644 --- a/proxy/internal/auth/middleware_test.go +++ b/proxy/internal/auth/middleware_test.go @@ -932,3 +932,71 @@ func TestProtect_HeaderAuth_SubsequentRequestUsesSessionCookie(t *testing.T) { assert.Equal(t, "header-user", capturedData2.GetUserID()) assert.Equal(t, "header", capturedData2.GetAuthMethod()) } + +// TestProtect_HeaderAuth_MultipleValuesSameHeader verifies that the proxy +// correctly handles multiple valid credentials for the same header name. +// In production, the mgmt gRPC authenticateHeader iterates all configured +// header auths and accepts if any hash matches (OR semantics). The proxy +// creates one Header scheme per entry, but a single gRPC call checks all. +func TestProtect_HeaderAuth_MultipleValuesSameHeader(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + // Mock simulates mgmt behavior: accepts either token-a or token-b. + accepted := map[string]bool{"Bearer token-a": true, "Bearer token-b": true} + mock := &mockAuthenticator{fn: func(_ context.Context, req *proto.AuthenticateRequest) (*proto.AuthenticateResponse, error) { + ha := req.GetHeaderAuth() + if ha != nil && accepted[ha.GetHeaderValue()] { + token, err := sessionkey.SignToken(kp.PrivateKey, "header-user", "example.com", auth.MethodHeader, time.Hour) + require.NoError(t, err) + return &proto.AuthenticateResponse{Success: true, SessionToken: token}, nil + } + return &proto.AuthenticateResponse{Success: false}, nil + }} + + // Single Header scheme (as if one entry existed), but the mock checks both values. + hdr := NewHeader(mock, "svc1", "acc1", "Authorization") + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "acc1", "svc1", nil)) + + var backendCalled bool + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalled = true + w.WriteHeader(http.StatusOK) + })) + + t.Run("first value accepted", func(t *testing.T) { + backendCalled = false + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("Authorization", "Bearer token-a") + req = req.WithContext(proxy.WithCapturedData(req.Context(), proxy.NewCapturedData(""))) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, backendCalled, "first token should be accepted") + }) + + t.Run("second value accepted", func(t *testing.T) { + backendCalled = false + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("Authorization", "Bearer token-b") + req = req.WithContext(proxy.WithCapturedData(req.Context(), proxy.NewCapturedData(""))) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, backendCalled, "second token should be accepted") + }) + + t.Run("unknown value rejected", func(t *testing.T) { + backendCalled = false + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("Authorization", "Bearer token-c") + req = req.WithContext(proxy.WithCapturedData(req.Context(), proxy.NewCapturedData(""))) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.False(t, backendCalled, "unknown token should be rejected") + }) +}