-
Notifications
You must be signed in to change notification settings - Fork 221
feat(vmcp): inject user identity as HTTP headers into backend requests #5291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package middleware | ||
|
|
||
| import ( | ||
| "net/http" | ||
|
|
||
| "github.com/stacklok/toolhive/pkg/auth" | ||
| ) | ||
|
|
||
| const ( | ||
| // HeaderUserSub is the HTTP header name for forwarding the authenticated user's subject claim. | ||
| // Backend MCP servers can read this header to identify the user without calling /introspect. | ||
| HeaderUserSub = "X-User-Sub" | ||
| // HeaderUserEmail is the HTTP header name for forwarding the authenticated user's email claim. | ||
| HeaderUserEmail = "X-User-Email" | ||
| // HeaderUserName is the HTTP header name for forwarding the authenticated user's name claim. | ||
| HeaderUserName = "X-User-Name" | ||
| ) | ||
|
|
||
| // NewClaimInjectionMiddleware returns a middleware that extracts user identity from the | ||
| // request context (populated by auth middleware) and injects it as HTTP headers into the | ||
| // forwarded request. This allows backend MCP servers to receive user identity without | ||
| // needing to implement their own OAuth token validation or /introspect calls. | ||
| // | ||
| // Headers injected (when identity is present): | ||
| // - X-User-Sub: the 'sub' claim (Google/OIDC user ID, always present) | ||
| // - X-User-Email: the 'email' claim (if available in token) | ||
| // - X-User-Name: the 'name' claim (if available in token) | ||
| // | ||
| // This middleware is safe to add unconditionally: if no identity is present in context | ||
| // (e.g., anonymous request), no headers are injected. | ||
| func NewClaimInjectionMiddleware() func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| identity, ok := auth.IdentityFromContext(r.Context()) | ||
| if ok && identity != nil { | ||
| // Clone request to avoid modifying the original | ||
| r = r.Clone(r.Context()) | ||
| if identity.Subject != "" { | ||
| r.Header.Set(HeaderUserSub, identity.Subject) | ||
| } | ||
| if identity.Email != "" { | ||
| r.Header.Set(HeaderUserEmail, identity.Email) | ||
| } | ||
| if identity.Name != "" { | ||
| r.Header.Set(HeaderUserName, identity.Name) | ||
| } | ||
|
Comment on lines
+41
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things if this survives in any form: it injects email/name with no opt-in (the strategy defaults to sub-only for PII reasons), and it doesn't |
||
| } | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package strategies | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "slices" | ||
|
|
||
| "github.com/stacklok/toolhive/pkg/auth" | ||
| authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" | ||
| healthcontext "github.com/stacklok/toolhive/pkg/vmcp/health/context" | ||
| ) | ||
|
|
||
| // ClaimInjectionStrategy injects authenticated user identity claims as HTTP headers | ||
| // into outgoing backend requests. | ||
| // | ||
| // This enables backend MCP servers to identify the caller (e.g. read X-User-Sub) | ||
| // without needing their own OAuth token validation or /introspect calls. The gateway | ||
| // is the sole trust boundary; backends trust the headers because they are unreachable | ||
| // by anything other than the gateway (enforced via Cloud Run IAM or equivalent). | ||
| // | ||
| // Which claims to forward is opt-in via ClaimInjectionConfig.Claims to minimise PII | ||
| // exposure. The default (empty Claims list) injects only X-User-Sub. | ||
| type ClaimInjectionStrategy struct{} | ||
|
|
||
| // NewClaimInjectionStrategy creates a new ClaimInjectionStrategy instance. | ||
| func NewClaimInjectionStrategy() *ClaimInjectionStrategy { | ||
| return &ClaimInjectionStrategy{} | ||
| } | ||
|
|
||
| // Name returns the strategy identifier. | ||
| func (*ClaimInjectionStrategy) Name() string { | ||
| return authtypes.StrategyTypeClaimInjection | ||
| } | ||
|
|
||
| // Authenticate reads the per-request identity from the context and injects the | ||
| // configured claims as X-User-* headers on the outgoing request. | ||
| // | ||
| // The method is a no-op (returns nil without modifying the request) when: | ||
| // - the request is a health check (no real user identity available) | ||
| // - no identity is present in the request context | ||
| // - the identity subject is empty or "anonymous" (unauthenticated mode) | ||
| func (*ClaimInjectionStrategy) Authenticate( | ||
| ctx context.Context, req *http.Request, strategy *authtypes.BackendAuthStrategy, | ||
| ) error { | ||
| // Health-check probes carry no user identity; skip silently. | ||
| if healthcontext.IsHealthCheck(ctx) { | ||
| return nil | ||
| } | ||
|
|
||
| identity, ok := auth.IdentityFromContext(ctx) | ||
| if !ok || identity == nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Skip anonymous sessions (vmcp unauthenticated mode). | ||
| // In anonymous mode the subject is "anonymous" and email is "anonymous@localhost". | ||
| if identity.Subject == "" || identity.Subject == "anonymous" { | ||
| return nil | ||
| } | ||
|
|
||
| // Determine which claims to inject. Default to ["sub"] when not configured. | ||
| claims := []string{"sub"} | ||
| if strategy != nil && strategy.ClaimInjection != nil && len(strategy.ClaimInjection.Claims) > 0 { | ||
| claims = strategy.ClaimInjection.Claims | ||
| } | ||
|
|
||
| if slices.Contains(claims, "sub") && identity.Subject != "" { | ||
| req.Header.Set("X-User-Sub", identity.Subject) | ||
| } | ||
| if slices.Contains(claims, "email") && identity.Email != "" { | ||
| req.Header.Set("X-User-Email", identity.Email) | ||
| } | ||
| if slices.Contains(claims, "name") && identity.Name != "" { | ||
| req.Header.Set("X-User-Name", identity.Name) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // Validate checks strategy configuration. ClaimInjectionConfig is optional | ||
| // (defaults to sub-only injection), so an absent config is valid. | ||
| func (*ClaimInjectionStrategy) Validate(strategy *authtypes.BackendAuthStrategy) error { | ||
| return nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package strategies_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/stacklok/toolhive/pkg/auth" | ||
| "github.com/stacklok/toolhive/pkg/vmcp/auth/strategies" | ||
| authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" | ||
| healthcontext "github.com/stacklok/toolhive/pkg/vmcp/health/context" | ||
| ) | ||
|
|
||
| func newReqWithIdentity(t *testing.T, identity *auth.Identity) *http.Request { | ||
| t.Helper() | ||
| req := httptest.NewRequest(http.MethodPost, "/mcp", http.NoBody) | ||
| if identity != nil { | ||
| req = req.WithContext(auth.WithIdentity(req.Context(), identity)) | ||
| } | ||
| return req | ||
| } | ||
|
|
||
| func strategy(claims ...string) *authtypes.BackendAuthStrategy { | ||
| if len(claims) == 0 { | ||
| return &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeClaimInjection} | ||
| } | ||
| return &authtypes.BackendAuthStrategy{ | ||
| Type: authtypes.StrategyTypeClaimInjection, | ||
| ClaimInjection: &authtypes.ClaimInjectionConfig{Claims: claims}, | ||
| } | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_Name(t *testing.T) { | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| assert.Equal(t, "claim_injection", s.Name()) | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_DefaultSubOnly(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{ | ||
| Subject: "108352771234567890", | ||
| Email: "user@example.com", | ||
| Name: "Test User", | ||
| }} | ||
| req := newReqWithIdentity(t, identity) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy()) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, "108352771234567890", req.Header.Get("X-User-Sub"), "sub injected by default") | ||
| assert.Empty(t, req.Header.Get("X-User-Email"), "email NOT injected by default (opt-in)") | ||
| assert.Empty(t, req.Header.Get("X-User-Name"), "name NOT injected by default (opt-in)") | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_ExplicitClaims(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{ | ||
| Subject: "108352771234567890", | ||
| Email: "user@example.com", | ||
| Name: "Test User", | ||
| }} | ||
| req := newReqWithIdentity(t, identity) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy("sub", "email", "name")) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, "108352771234567890", req.Header.Get("X-User-Sub")) | ||
| assert.Equal(t, "user@example.com", req.Header.Get("X-User-Email")) | ||
| assert.Equal(t, "Test User", req.Header.Get("X-User-Name")) | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_SkipsAnonymous(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| anonymous := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{ | ||
| Subject: "anonymous", | ||
| Email: "anonymous@localhost", | ||
| }} | ||
| req := newReqWithIdentity(t, anonymous) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy("sub", "email")) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Empty(t, req.Header.Get("X-User-Sub"), "anonymous identity must not inject headers") | ||
| assert.Empty(t, req.Header.Get("X-User-Email")) | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_SkipsNoIdentity(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| req := httptest.NewRequest(http.MethodPost, "/mcp", http.NoBody) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy("sub", "email")) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Empty(t, req.Header.Get("X-User-Sub")) | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_SkipsHealthCheck(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{Subject: "sub-123"}} | ||
| req := newReqWithIdentity(t, identity) | ||
| req = req.WithContext(healthcontext.WithHealthCheckMarker(req.Context())) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy("sub")) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Empty(t, req.Header.Get("X-User-Sub"), "health checks must not inject headers") | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_EmptyFieldsNotInjected(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{Subject: "sub-only"}} | ||
| req := newReqWithIdentity(t, identity) | ||
|
|
||
| err := s.Authenticate(req.Context(), req, strategy("sub", "email", "name")) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, "sub-only", req.Header.Get("X-User-Sub")) | ||
| assert.Empty(t, req.Header.Get("X-User-Email"), "empty email must not produce header") | ||
| assert.Empty(t, req.Header.Get("X-User-Name"), "empty name must not produce header") | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_Validate(t *testing.T) { | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| assert.NoError(t, s.Validate(nil), "nil config is valid (defaults to sub-only)") | ||
| assert.NoError(t, s.Validate(&authtypes.BackendAuthStrategy{})) | ||
| } | ||
|
|
||
| func TestClaimInjectionStrategy_NilContext(t *testing.T) { | ||
| t.Parallel() | ||
| s := strategies.NewClaimInjectionStrategy() | ||
| req := httptest.NewRequest(http.MethodPost, "/mcp", http.NoBody) | ||
| // No identity in context — should return nil, not panic. | ||
| err := s.Authenticate(context.Background(), req, strategy("sub")) | ||
| assert.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wires claim injection in unconditionally for every HTTP-transport workload, not just vmcp backends. The comment "When no identity is in context (anonymous request), this middleware is a no-op" isn't accurate either: anonymous mode and the no-OIDC local path both put a non-nil identity in context, so the headers do get sent (
X-User-Sub: anonymous, or the OS username). I'd remove this middleware and rely on theclaim_injectionstrategy, which is opt-in and already handles the anonymous/health-check cases.