Skip to content

Commit 3ab5b18

Browse files
feat(broker): per-user credential injection + per-(user,server) connection keying (spec 074 T7) (#691)
* feat(broker): per-user credential injection + per-(user,server) connection keying (spec 074, MCP-1040) Wire the resolved per-user upstream credential (T6 CredentialResolver) into the outbound request, replacing any inbound/configured auth and keying brokered connections per (user, server). - transport: edition-neutral BrokeredAuth + EffectiveHeaders primitive that injects the resolved credential into the configured header, dropping any same-named inbound/configured header (case-insensitive). The gateway/IdP token is never forwarded (FR-016/FR-017). Applied in both the HTTP and SSE client builders. - upstream/core: Client carries an optional per-user BrokeredAuth; the headers-auth strategy (HTTP + SSE) injects it and runs even when the upstream has no static headers. - serveredition/broker: HeaderInjector bridges CredentialResolver to the transport layer, rejects stdio brokering as defense-in-depth (FR-002), and ConnectionKey binds a brokered connection to one (user, server) so one user's credential is never reused for another (FR-018). - serveredition/multiuser: Router.BrokeredConnectionKey exposes the per-(user,server) pooling key for brokered upstream connections. - docs: header-injection + per-(user,server) connection keying sections. TDD: outbound carries the user-specific token; the inbound token is absent on the outbound request across both HTTP and SSE paths; two users -> two distinct tokens and connection keys; stdio brokering rejected. Related #688 FR-016, FR-017, FR-018. SC-002, SC-003. * fix(upstream): brokered connections fail closed — no no-auth/OAuth fallback (spec 074, MCP-1040) Maintainer fix-before-merge on PR #691. When a per-user brokered credential is set on a connection (brokeredAuth != nil), the ONLY permitted auth strategy is the brokered headers. Previously the headers -> no-auth -> OAuth fallback chain meant a failure of the per-user headers auth (e.g. 403 on the user's token) would fall through to no-auth (connect unauthenticated) or shared OAuth (borrow another identity) — defeating per-user isolation at the on-the-wire point. Extract httpAuthStrategies()/sseAuthStrategies(): a brokered connection returns ONLY the headers strategy, so a failure refuses the connection (no fallback). Non-brokered connections keep the historical headers -> no-auth -> OAuth chain unchanged. Test: TestClient_BrokeredConnection_FailsClosed_OnlyHeadersStrategy asserts the brokered strategy list is [headers] (no no-auth, no OAuth) for both HTTP and SSE, while non-brokered keeps the full chain. Affected packages green under -tags server -race; golangci-lint v2 clean. Co-Authored-By: Paperclip <noreply@paperclip.ing> --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent 6c66638 commit 3ab5b18

12 files changed

Lines changed: 879 additions & 38 deletions

File tree

docs/features/auth-broker.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,21 @@ On each proxied request the broker resolves the per-user credential to inject, i
9090

9191
Concurrent requests for the same `(user, upstream)` are coalesced (single-flight) so a burst does not trigger duplicate upstream token flows. A policy-decision hook is evaluated per call immediately before the credential is returned; no policy engine ships yet, so it permits every injection by default.
9292

93+
## Header injection
94+
95+
The resolved per-user credential is injected into the configured outbound header (`header`, default `Authorization`) using the value template (`header_format`, default `Bearer {token}`), then the request is forwarded to the upstream.
96+
97+
Injection is a **replacement**, not a merge:
98+
99+
- Any header on the upstream config whose name matches `header` (case-insensitively) is **removed** before the resolved credential is set, so a brokered upstream presents exactly one value for that header.
100+
- The inbound gateway/IdP token is **never forwarded** to the upstream. Brokering exists precisely so the upstream sees a credential minted *for it*, scoped to the calling user — not the token the user presented to the gateway.
101+
102+
Injection applies only to **HTTP-family** upstreams (`http`, `sse`, `streamable-http`). Brokering on a `stdio` upstream is rejected — at config validation, and again as a runtime guard at the injection boundary — with a clear "unsupported in this phase" message.
103+
104+
## Per-(user, server) connection keying
105+
106+
A **shared** upstream that is brokered per-user must carry **each user's own** credential. Brokered upstream connections are therefore keyed by `(user, server)`, never by server alone: one user's connection (and the credential injected on it) is never reused for another user. The server-component of the key reuses the same `name + URL` scheme as the credential store, so a connection and its cached credential stay in lockstep.
107+
93108
## See also
94109

95110
- [OAuth Authentication](./oauth-authentication.md) — upstream OAuth for the personal edition.
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
//go:build server
2+
3+
package broker
4+
5+
import (
6+
"context"
7+
"errors"
8+
9+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth"
11+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/transport"
12+
)
13+
14+
// ErrBrokerStdioUnsupported is returned when brokering is requested for a
15+
// non-HTTP-family (stdio) upstream. Credential injection only works over
16+
// HTTP/SSE/streamable-HTTP transports in this phase (spec 074 FR-002). This is a
17+
// runtime defense-in-depth check; config validation already rejects such blocks
18+
// at load time.
19+
var ErrBrokerStdioUnsupported = errors.New("auth broker: credential injection is only supported on HTTP-family upstreams (http, sse, streamable-http); stdio brokering is unsupported in this phase")
20+
21+
// Fallback header/format used when a brokered server's config has not had its
22+
// defaults applied. These mirror config.AuthBrokerConfig.ApplyDefaults (FR-016);
23+
// config validation normally applies them at load time.
24+
const (
25+
fallbackBrokerHeader = "Authorization"
26+
fallbackBrokerHeaderFormat = "Bearer {token}"
27+
)
28+
29+
// resolver is the subset of *CredentialResolver the injector depends on. It is
30+
// an interface so tests can substitute a fake without a real store/exchanger.
31+
type resolver interface {
32+
Resolve(ctx context.Context, userID string, server *config.ServerConfig) (*UpstreamCredential, error)
33+
}
34+
35+
// HeaderInjector turns a per-user resolved upstream credential into the
36+
// transport-layer BrokeredAuth injected on a proxied request. It is the bridge
37+
// between the credential broker (server edition) and the edition-neutral
38+
// transport layer: the transport never imports the broker, it only receives the
39+
// resolved credential as plain data.
40+
//
41+
// The injector enforces the spec-074 brokering invariants at the injection
42+
// boundary:
43+
// - per-user only: an empty userID is rejected (FR-014);
44+
// - HTTP-family only: stdio brokering is rejected (FR-002);
45+
// - replacement, not forwarding: the produced BrokeredAuth replaces any
46+
// configured/inbound auth header (FR-016/FR-017, enforced in transport).
47+
type HeaderInjector struct {
48+
resolver resolver
49+
}
50+
51+
// NewHeaderInjector builds an injector over a credential resolver. *CredentialResolver
52+
// satisfies the resolver interface.
53+
func NewHeaderInjector(r resolver) *HeaderInjector {
54+
return &HeaderInjector{resolver: r}
55+
}
56+
57+
// InjectFor resolves the per-user credential for (userID, server) and returns
58+
// the transport.BrokeredAuth to inject. It returns:
59+
// - ErrUnauthenticated if userID is empty (FR-014);
60+
// - ErrBrokerNotConfigured if the server has no auth_broker block;
61+
// - ErrBrokerStdioUnsupported if the server is not HTTP-family (FR-002);
62+
// - any resolver error (e.g. *NotConnectedError carrying a connect URL).
63+
func (h *HeaderInjector) InjectFor(ctx context.Context, userID string, server *config.ServerConfig) (*transport.BrokeredAuth, error) {
64+
if userID == "" {
65+
return nil, ErrUnauthenticated
66+
}
67+
if server == nil || server.AuthBroker == nil {
68+
return nil, ErrBrokerNotConfigured
69+
}
70+
// Defense-in-depth: reject brokering on stdio/non-HTTP upstreams (FR-002).
71+
if transport.DetermineTransportType(server) == transport.TransportStdio {
72+
return nil, ErrBrokerStdioUnsupported
73+
}
74+
75+
cred, err := h.resolver.Resolve(ctx, userID, server)
76+
if err != nil {
77+
return nil, err
78+
}
79+
if cred == nil || cred.AccessToken == "" {
80+
return nil, ErrNoCredential
81+
}
82+
83+
header := server.AuthBroker.Header
84+
if header == "" {
85+
header = fallbackBrokerHeader
86+
}
87+
format := server.AuthBroker.HeaderFormat
88+
if format == "" {
89+
format = fallbackBrokerHeaderFormat
90+
}
91+
return &transport.BrokeredAuth{
92+
Header: header,
93+
Format: format,
94+
Token: cred.AccessToken,
95+
}, nil
96+
}
97+
98+
// ConnectionKey derives the pooling key for a brokered upstream connection. It
99+
// binds the connection to a single (user, server) pair so a shared upstream
100+
// brokered per-user never reuses one user's credential/connection for another
101+
// (FR-018). The server component reuses the existing oauth.GenerateServerKey
102+
// scheme (name + URL) so it matches the credential store's keying.
103+
func ConnectionKey(userID string, server *config.ServerConfig) string {
104+
if server == nil {
105+
return userID + "\x00"
106+
}
107+
return userID + "\x00" + oauth.GenerateServerKey(server.Name, server.URL)
108+
}
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
//go:build server
2+
3+
package broker
4+
5+
import (
6+
"context"
7+
"errors"
8+
"testing"
9+
10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
11+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/transport"
12+
)
13+
14+
// fakeResolver returns a per-user token so the injector can be exercised without
15+
// a real store/exchanger. It records the per-user resolution so tests can assert
16+
// one user's credential is never produced for another (FR-018).
17+
type fakeResolver struct {
18+
tokens map[string]string // userID -> access token
19+
err error
20+
}
21+
22+
func (f *fakeResolver) Resolve(_ context.Context, userID string, _ *config.ServerConfig) (*UpstreamCredential, error) {
23+
if f.err != nil {
24+
return nil, f.err
25+
}
26+
tok, ok := f.tokens[userID]
27+
if !ok {
28+
return nil, ErrNoCredential
29+
}
30+
return &UpstreamCredential{AccessToken: tok, TokenType: "Bearer"}, nil
31+
}
32+
33+
func httpBrokerServer() *config.ServerConfig {
34+
s := &config.ServerConfig{
35+
Name: "ghe",
36+
URL: "https://ghe.example/mcp",
37+
Protocol: "streamable-http",
38+
AuthBroker: &config.AuthBrokerConfig{
39+
Mode: config.AuthBrokerModeTokenExchange,
40+
TokenEndpoint: "https://idp.example/token",
41+
},
42+
}
43+
s.AuthBroker.ApplyDefaults()
44+
return s
45+
}
46+
47+
// FR-016: the resolved per-user credential is rendered into the configured
48+
// header/format (default Authorization: Bearer {token}).
49+
func TestInjector_InjectFor_ProducesBrokeredAuth(t *testing.T) {
50+
inj := NewHeaderInjector(&fakeResolver{tokens: map[string]string{"alice": "alice-tok"}})
51+
52+
ba, err := inj.InjectFor(context.Background(), "alice", httpBrokerServer())
53+
if err != nil {
54+
t.Fatalf("InjectFor: %v", err)
55+
}
56+
if ba.Header != "Authorization" {
57+
t.Fatalf("header = %q, want Authorization", ba.Header)
58+
}
59+
if ba.HeaderValue() != "Bearer alice-tok" {
60+
t.Fatalf("header value = %q, want %q", ba.HeaderValue(), "Bearer alice-tok")
61+
}
62+
}
63+
64+
// FR-018 + SC-002/003: two users brokered against the SAME shared upstream get
65+
// two distinct outbound tokens; one user's credential is never reused for the
66+
// other.
67+
func TestInjector_TwoUsers_TwoTokens(t *testing.T) {
68+
inj := NewHeaderInjector(&fakeResolver{tokens: map[string]string{
69+
"alice": "alice-tok",
70+
"bob": "bob-tok",
71+
}})
72+
server := httpBrokerServer()
73+
74+
aliceBA, err := inj.InjectFor(context.Background(), "alice", server)
75+
if err != nil {
76+
t.Fatalf("alice: %v", err)
77+
}
78+
bobBA, err := inj.InjectFor(context.Background(), "bob", server)
79+
if err != nil {
80+
t.Fatalf("bob: %v", err)
81+
}
82+
83+
if aliceBA.HeaderValue() == bobBA.HeaderValue() {
84+
t.Fatalf("two users produced the same outbound token %q (FR-018 violation)", aliceBA.HeaderValue())
85+
}
86+
87+
// And the effective outbound headers must carry each user's own token.
88+
aliceHdr := transport.EffectiveHeaders(nil, aliceBA)
89+
bobHdr := transport.EffectiveHeaders(nil, bobBA)
90+
if aliceHdr["Authorization"] != "Bearer alice-tok" || bobHdr["Authorization"] != "Bearer bob-tok" {
91+
t.Fatalf("cross-user token leak: alice=%q bob=%q", aliceHdr["Authorization"], bobHdr["Authorization"])
92+
}
93+
94+
// Per-(user,server) connection keys must differ so connections are never
95+
// pooled across users (FR-018).
96+
if ConnectionKey("alice", server) == ConnectionKey("bob", server) {
97+
t.Fatalf("connection key collided across users (FR-018 violation)")
98+
}
99+
}
100+
101+
// FR-018: ConnectionKey is stable for the same (user, server) and distinct per
102+
// user and per server so a brokered connection is never reused across either.
103+
func TestConnectionKey_StableAndDistinct(t *testing.T) {
104+
s1 := httpBrokerServer()
105+
s2 := httpBrokerServer()
106+
s2.Name = "other"
107+
s2.URL = "https://other.example/mcp"
108+
109+
k1 := ConnectionKey("alice", s1)
110+
k1again := ConnectionKey("alice", s1)
111+
if k1 != k1again {
112+
t.Fatal("ConnectionKey must be stable for the same (user, server)")
113+
}
114+
if ConnectionKey("alice", s1) == ConnectionKey("alice", s2) {
115+
t.Fatal("ConnectionKey must differ per server")
116+
}
117+
if ConnectionKey("alice", s1) == ConnectionKey("bob", s1) {
118+
t.Fatal("ConnectionKey must differ per user")
119+
}
120+
}
121+
122+
// FR-002: brokering on a stdio upstream is rejected with a clear, actionable
123+
// message — never silently injected.
124+
func TestInjector_RejectsStdio(t *testing.T) {
125+
inj := NewHeaderInjector(&fakeResolver{tokens: map[string]string{"alice": "x"}})
126+
stdio := &config.ServerConfig{
127+
Name: "local",
128+
Protocol: "stdio",
129+
Command: "my-mcp",
130+
AuthBroker: &config.AuthBrokerConfig{
131+
Mode: config.AuthBrokerModeTokenExchange,
132+
TokenEndpoint: "https://idp.example/token",
133+
},
134+
}
135+
stdio.AuthBroker.ApplyDefaults()
136+
137+
_, err := inj.InjectFor(context.Background(), "alice", stdio)
138+
if !errors.Is(err, ErrBrokerStdioUnsupported) {
139+
t.Fatalf("stdio brokering must be rejected with ErrBrokerStdioUnsupported, got %v", err)
140+
}
141+
}
142+
143+
// A server with no auth_broker block is not brokered: the injector returns
144+
// ErrBrokerNotConfigured and the caller proceeds with today's behaviour.
145+
func TestInjector_NotConfigured(t *testing.T) {
146+
inj := NewHeaderInjector(&fakeResolver{})
147+
plain := &config.ServerConfig{Name: "plain", URL: "https://x/mcp", Protocol: "streamable-http"}
148+
149+
_, err := inj.InjectFor(context.Background(), "alice", plain)
150+
if !errors.Is(err, ErrBrokerNotConfigured) {
151+
t.Fatalf("non-brokered server must return ErrBrokerNotConfigured, got %v", err)
152+
}
153+
}
154+
155+
// An empty userID is rejected before any resolution — brokering is strictly
156+
// per-user (FR-014).
157+
func TestInjector_RejectsAnonymous(t *testing.T) {
158+
inj := NewHeaderInjector(&fakeResolver{tokens: map[string]string{"alice": "x"}})
159+
_, err := inj.InjectFor(context.Background(), "", httpBrokerServer())
160+
if !errors.Is(err, ErrUnauthenticated) {
161+
t.Fatalf("anonymous caller must be rejected with ErrUnauthenticated, got %v", err)
162+
}
163+
}

internal/serveredition/multiuser/router.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/smart-mcp-proxy/mcpproxy-go/internal/auth"
1414
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
15+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/serveredition/broker"
1516
"github.com/smart-mcp-proxy/mcpproxy-go/internal/serveredition/workspace"
1617
)
1718

@@ -149,6 +150,29 @@ func (r *Router) GetServerForUser(ctx context.Context, serverName string) (*Serv
149150
return nil, fmt.Errorf("server %q not found or not accessible", serverName)
150151
}
151152

153+
// BrokeredConnectionKey returns the per-(user, server) pooling key for a
154+
// brokered upstream connection (spec 074 FR-018). It resolves the calling user
155+
// and the named server (shared or personal), then keys the connection by both
156+
// so a shared upstream brokered per-user never reuses one user's
157+
// credential/connection for another. The connection pool MUST use this key when
158+
// caching brokered upstream clients.
159+
//
160+
// It errors if there is no auth context or the server is not accessible to the
161+
// user; it does not require the server to actually declare an auth_broker block,
162+
// so callers can key uniformly and decide per server whether to broker.
163+
func (r *Router) BrokeredConnectionKey(ctx context.Context, serverName string) (string, error) {
164+
ac := auth.AuthContextFromContext(ctx)
165+
if ac == nil {
166+
return "", fmt.Errorf("no authentication context")
167+
}
168+
169+
info, err := r.GetServerForUser(ctx, serverName)
170+
if err != nil {
171+
return "", err
172+
}
173+
return broker.ConnectionKey(ac.GetUserID(), info.Config), nil
174+
}
175+
152176
// IsServerAccessible returns true if the user from the context can access
153177
// the named server. This is a convenience method that does not return
154178
// detailed error information.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//go:build server
2+
3+
package multiuser
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// Spec 074 T7 / FR-018: a shared upstream brokered per-user must key its
13+
// connection by (user, server) so one user's credential/connection is never
14+
// reused for another. The router exposes the per-(user,server) connection key
15+
// for the connection pool to use.
16+
17+
func TestRouter_BrokeredConnectionKey_DistinctPerUser(t *testing.T) {
18+
router, _ := setupRouter(t, []string{"shared-ghe"}, nil)
19+
20+
aliceKey, err := router.BrokeredConnectionKey(userCtx("alice"), "shared-ghe")
21+
require.NoError(t, err)
22+
bobKey, err := router.BrokeredConnectionKey(userCtx("bob"), "shared-ghe")
23+
require.NoError(t, err)
24+
25+
assert.NotEmpty(t, aliceKey)
26+
assert.NotEqual(t, aliceKey, bobKey,
27+
"the same shared brokered upstream must key a distinct connection per user (FR-018)")
28+
29+
// Stable for the same (user, server).
30+
aliceKey2, err := router.BrokeredConnectionKey(userCtx("alice"), "shared-ghe")
31+
require.NoError(t, err)
32+
assert.Equal(t, aliceKey, aliceKey2, "connection key must be stable for the same (user, server)")
33+
}
34+
35+
func TestRouter_BrokeredConnectionKey_RequiresAuth(t *testing.T) {
36+
router, _ := setupRouter(t, []string{"shared-ghe"}, nil)
37+
_, err := router.BrokeredConnectionKey(noAuthCtx(), "shared-ghe")
38+
assert.Error(t, err, "no auth context must be rejected")
39+
}
40+
41+
func TestRouter_BrokeredConnectionKey_UnknownServer(t *testing.T) {
42+
router, _ := setupRouter(t, []string{"shared-ghe"}, nil)
43+
_, err := router.BrokeredConnectionKey(userCtx("alice"), "nope")
44+
assert.Error(t, err, "unknown/inaccessible server must error")
45+
}

0 commit comments

Comments
 (0)