-
Notifications
You must be signed in to change notification settings - Fork 224
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,32 @@ func (i *identityRoundTripper) RoundTrip(req *http.Request) (*http.Response, err | |
| return i.base.RoundTrip(req) | ||
| } | ||
|
|
||
| // claimInjectionRoundTripper injects authenticated user identity claims as HTTP headers | ||
| // so backend MCP servers can identify the user without OAuth token introspection. | ||
| // | ||
| // Headers injected when identity is present: | ||
| // - X-User-Sub: the authenticated user's subject claim (Google/OIDC sub) | ||
| // - X-User-Email: the user's email address (if present in token) | ||
| // - X-User-Name: the user's display name (if present in token) | ||
| type claimInjectionRoundTripper struct { | ||
| base http.RoundTripper | ||
| identity *auth.Identity | ||
| } | ||
|
|
||
| func (c *claimInjectionRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| cloned := req.Clone(req.Context()) | ||
| if c.identity.Subject != "" { | ||
| cloned.Header.Set("X-User-Sub", c.identity.Subject) | ||
| } | ||
| if c.identity.Email != "" { | ||
| cloned.Header.Set("X-User-Email", c.identity.Email) | ||
| } | ||
| if c.identity.Name != "" { | ||
| cloned.Header.Set("X-User-Name", c.identity.Name) | ||
| } | ||
| return c.base.RoundTrip(cloned) | ||
| } | ||
|
|
||
| // Compile-time assertion: mcpSession must implement Session. | ||
| var _ Session = (*mcpSession)(nil) | ||
|
|
||
|
|
@@ -259,7 +285,7 @@ func createMCPClient( | |
|
|
||
| slog.Debug("Applied authentication strategy", "strategy", strategy.Name(), "backendID", target.WorkloadID) | ||
|
|
||
| // Build shared transport chain: auth → identity propagation. | ||
| // Build shared transport chain: auth → identity propagation → claim injection. | ||
|
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. Heads up — this comment is the inverse of the actual execution order. Either flip the order in the comment, or add a sentence clarifying that wrap order is the reverse of execution order. |
||
| // The per-transport sections below may add a size-limiting wrapper on top. | ||
| base := http.RoundTripper(http.DefaultTransport) | ||
| base = &authRoundTripper{ | ||
|
|
@@ -269,6 +295,11 @@ func createMCPClient( | |
| target: target, | ||
| } | ||
| base = &identityRoundTripper{base: base, identity: identity} | ||
| // Inject user identity as HTTP headers so backend MCP servers can read | ||
| // X-User-Sub / X-User-Email without needing their own /introspect calls. | ||
| if identity != nil { | ||
|
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. The commit message says "When no identity is present in context (e.g. anonymous mode), no headers are injected" — but anonymous identity isn't nil. Worth fixing one way or the other — either gate explicitly here (e.g. add an |
||
| base = &claimInjectionRoundTripper{base: base, identity: identity} | ||
|
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. vmcp already has a per-backend outgoing-auth strategy registry — see the constants in Think about a setup with github-tools + atlassian-tools via Could this be a new strategy variant instead? Either fold a |
||
| } | ||
|
|
||
| var c *mcpclient.Client | ||
| switch target.TransportType { | ||
|
|
||
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.
Email (and Name a few lines down) are PII. As written, this sends them to every backend unconditionally whenever an identity is present. A backend that only needs
subfor authorization still gets the user's email — which then very likely ends up in that backend's request logs.If the feature stays in this form, defaulting to
subonly and requiring explicit opt-in for email/name would be much better data-minimization. If it moves to a per-backend strategy (see the other comment), the claim set should be configurable there anyway.