From cc5f33a2389405170cd4104e158676785281606f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 11 May 2026 16:58:11 -0400 Subject: [PATCH 1/4] feat(pipeline): Add Scheme field to Context Plugins that compose target URLs from a request (token-broker's serverURL construction is the current case) have to either hardcode "http://" or carry their own scheme config. The transport already knows the scheme - it's on the :scheme pseudo-header in ext_proc / ext_authz and on r.URL.Scheme in the Go HTTP listeners - but the framework never surfaced it to plugins. Add a Scheme string field to pipeline.Context alongside Host/Path. Free-form string (not enum) so ws/wss/gRPC-Web and future transports pass through without a framework PR. Empty when the listener can't determine - plugins that need a concrete scheme pick a default themselves rather than assume, so missing listener plumbing doesn't hide behind a framework-supplied default. This commit adds the field and documents it. Listeners populate it in the next commit; a plugin consumer (token-broker) follows after. Resolves #397 (part 1 of 3). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/pipeline/context.go | 20 ++++++++++++++++---- authbridge/docs/framework-architecture.md | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/authbridge/authlib/pipeline/context.go b/authbridge/authlib/pipeline/context.go index c130cdbbd..ffede67cf 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -82,10 +82,22 @@ func (d *Direction) UnmarshalJSON(data []byte) error { type Context struct { Direction Direction Method string - Host string - Path string - Headers http.Header - Body []byte // nil unless at least one plugin declares BodyAccess: true + // Scheme is the URL scheme of the request, typically "http" or + // "https" — future transports ("ws" / "wss" / gRPC-specific + // schemes) pass through unchanged as free-form strings. Populated + // by the listener at pctx construction from the transport-native + // field: the :scheme pseudo-header in ext_proc and ext_authz, + // r.URL.Scheme in the forward and reverse proxies. + // + // Empty when the listener can't determine scheme (legacy test + // fixtures, an unrecognized transport, etc.). Plugins that need a + // concrete scheme should pick a default explicitly — treating "" + // as "assume http" would silently mask missing listener plumbing. + Scheme string + Host string + Path string + Headers http.Header + Body []byte // nil unless at least one plugin declares BodyAccess: true // StartedAt is the wall-clock time this context was constructed by the // listener at the start of a request. Used on the response path to diff --git a/authbridge/docs/framework-architecture.md b/authbridge/docs/framework-architecture.md index 31cabe360..f1cfc113f 100644 --- a/authbridge/docs/framework-architecture.md +++ b/authbridge/docs/framework-architecture.md @@ -107,6 +107,7 @@ The entire surface a plugin sees: type Context struct { Direction Direction // Inbound | Outbound Method string // HTTP method + Scheme string // "http" | "https" | ... ; empty when the listener can't determine it Host string // :authority / Host Path string // :path Headers http.Header From a58bafc43342f3321d14837edcd62dee61316a41 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 11 May 2026 17:05:58 -0400 Subject: [PATCH 2/4] feat(listeners): Populate pctx.Scheme in all 4 listeners Each listener reads scheme from its transport-native source and sets the new Context.Scheme field at pctx construction: ext_proc :scheme pseudo-header on the headers block (inbound + outbound, headers-only + body paths: 4 call sites) ext_authz httpReq.GetScheme() on the CheckRequest's HTTP attribute (shared by inbound + outbound pctx) forwardproxy r.URL.Scheme on the incoming proxy request; Go populates it from the absolute-URL request line that HTTP forward proxies receive reverseproxy r.TLS presence-based: TLS set -> https, absent -> http. r.URL.Scheme is empty on Go server-side requests so it's not usable; r.TLS is the reliable signal. X-Forwarded-Proto support is deferred pending a trusted-proxy policy. Populated unconditionally on both inbound and outbound pctx where applicable (ext_proc, ext_authz). Empty-string passes through when the listener can't determine - plugins default explicitly per the field's godoc. Tests: ext_proc 5 cases (inbound+outbound x http+https, plus missing-pseudo-header-is-empty) ext_authz 3 cases (http, https, empty passes through); asserts both inbound and outbound pctx see the same scheme forwardproxy 1 case wiring a live httptest backend through the proxy, assertion on pctx.Scheme == "http" reverseproxy 2 cases using httptest.NewServer (plaintext) and NewTLSServer (tls) to confirm r.TLS-based derivation All new tests use a small schemeCapturePlugin (duplicated per package rather than promoted to authlib/plugintesting: the plugin shape is only useful in listener tests and not worth exporting). Resolves #397 (part 2 of 3). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../authbridge/listener/extauthz/server.go | 3 + .../listener/extauthz/server_test.go | 79 ++++++++++++++++++- .../cmd/authbridge/listener/extproc/server.go | 4 + .../listener/extproc/server_test.go | 79 ++++++++++++++++++- .../listener/forwardproxy/server.go | 1 + .../listener/forwardproxy/server_test.go | 61 +++++++++++++- .../listener/reverseproxy/server.go | 17 ++++ .../listener/reverseproxy/server_test.go | 77 ++++++++++++++++++ 8 files changed, 313 insertions(+), 8 deletions(-) diff --git a/authbridge/cmd/authbridge/listener/extauthz/server.go b/authbridge/cmd/authbridge/listener/extauthz/server.go index 59ac6bdb2..af43b405b 100644 --- a/authbridge/cmd/authbridge/listener/extauthz/server.go +++ b/authbridge/cmd/authbridge/listener/extauthz/server.go @@ -44,10 +44,12 @@ func (s *Server) Check(ctx context.Context, req *authv3.CheckRequest) (*authv3.C host = headers["host"] } path := httpReq.GetPath() + scheme := httpReq.GetScheme() // Inbound validation via pipeline inPctx := &pipeline.Context{ Direction: pipeline.Inbound, + Scheme: scheme, Host: host, Path: path, Headers: mapToHTTPHeader(headers), @@ -60,6 +62,7 @@ func (s *Server) Check(ctx context.Context, req *authv3.CheckRequest) (*authv3.C // Outbound exchange via pipeline outPctx := &pipeline.Context{ Direction: pipeline.Outbound, + Scheme: scheme, Host: host, Path: path, Headers: mapToHTTPHeader(headers), diff --git a/authbridge/cmd/authbridge/listener/extauthz/server_test.go b/authbridge/cmd/authbridge/listener/extauthz/server_test.go index d332b83ba..1499bcc68 100644 --- a/authbridge/cmd/authbridge/listener/extauthz/server_test.go +++ b/authbridge/cmd/authbridge/listener/extauthz/server_test.go @@ -12,12 +12,12 @@ import ( "google.golang.org/grpc/codes" authpkg "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) type mockVerifier struct { @@ -151,3 +151,76 @@ func TestCheck_MissingHTTPAttributes(t *testing.T) { t.Fatal("expected DeniedResponse for missing attributes") } } + +// schemeCapturePlugin records pctx.Scheme. Duplicated in each listener +// test package because these are the smallest-unit Plugin types that +// don't belong in authlib. +type schemeCapturePlugin struct { + got string +} + +func (p *schemeCapturePlugin) Name() string { return "scheme-capture" } +func (p *schemeCapturePlugin) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *schemeCapturePlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.got = pctx.Scheme + return pipeline.Action{Type: pipeline.Continue} +} +func (p *schemeCapturePlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// TestCheck_PopulatesSchemeFromAttribute verifies that both the +// inbound and outbound pctx constructed by Check() read Scheme from +// the CheckRequest's HTTP attribute, so plugins composing target +// URLs or branching on transport see the right value in waypoint +// mode. +func TestCheck_PopulatesSchemeFromAttribute(t *testing.T) { + tests := []struct { + name string + scheme string + want string + }{ + {name: "http", scheme: "http", want: "http"}, + {name: "https", scheme: "https", want: "https"}, + {name: "empty_passes_through", scheme: "", want: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + inCap := &schemeCapturePlugin{} + outCap := &schemeCapturePlugin{} + in, err := plugintesting.BuildPipeline([]pipeline.Plugin{inCap}) + if err != nil { + t.Fatalf("BuildPipeline inbound: %v", err) + } + out, err := plugintesting.BuildPipeline([]pipeline.Plugin{outCap}) + if err != nil { + t.Fatalf("BuildPipeline outbound: %v", err) + } + srv := &Server{ + InboundPipeline: pipeline.NewHolder(in), + OutboundPipeline: pipeline.NewHolder(out), + } + + req := &authv3.CheckRequest{ + Attributes: &authv3.AttributeContext{ + Request: &authv3.AttributeContext_Request{ + Http: &authv3.AttributeContext_HttpRequest{ + Scheme: tc.scheme, + Headers: map[string]string{":authority": "agent.example"}, + Path: "/x", + }, + }, + }, + } + _, _ = srv.Check(context.Background(), req) + if inCap.got != tc.want { + t.Errorf("inbound pctx.Scheme = %q, want %q", inCap.got, tc.want) + } + if outCap.got != tc.want { + t.Errorf("outbound pctx.Scheme = %q, want %q", outCap.got, tc.want) + } + }) + } +} diff --git a/authbridge/cmd/authbridge/listener/extproc/server.go b/authbridge/cmd/authbridge/listener/extproc/server.go index bf6afc115..c1a400e4d 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server.go +++ b/authbridge/cmd/authbridge/listener/extproc/server.go @@ -127,6 +127,7 @@ func (s *Server) handleInbound(stream extprocv3.ExternalProcessor_ProcessServer, ctx := stream.Context() pctx := &pipeline.Context{ Direction: pipeline.Inbound, + Scheme: getHeader(headers, ":scheme"), Path: getHeader(headers, ":path"), Headers: headerMapToHTTP(headers), Body: body, @@ -147,6 +148,7 @@ func (s *Server) handleInboundBody(stream extprocv3.ExternalProcessor_ProcessSer ctx := stream.Context() pctx := &pipeline.Context{ Direction: pipeline.Inbound, + Scheme: getHeader(headers, ":scheme"), Path: getHeader(headers, ":path"), Headers: headerMapToHTTP(headers), Body: body, @@ -559,6 +561,7 @@ func (s *Server) handleOutbound(stream extprocv3.ExternalProcessor_ProcessServer ctx := stream.Context() pctx := &pipeline.Context{ Direction: pipeline.Outbound, + Scheme: getHeader(headers, ":scheme"), Host: getHeader(headers, ":authority"), Path: getHeader(headers, ":path"), Headers: headerMapToHTTP(headers), @@ -595,6 +598,7 @@ func (s *Server) handleOutboundBody(stream extprocv3.ExternalProcessor_ProcessSe ctx := stream.Context() pctx := &pipeline.Context{ Direction: pipeline.Outbound, + Scheme: getHeader(headers, ":scheme"), Host: getHeader(headers, ":authority"), Path: getHeader(headers, ":path"), Headers: headerMapToHTTP(headers), diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index e70ec6f36..1bbb16d67 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -16,13 +16,13 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/session" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) // mockStream implements ExternalProcessor_ProcessServer for testing. @@ -1331,7 +1331,7 @@ func TestRecordOutboundReject_EmitsDeniedPhase(t *testing.T) { // Seed an active session so ActiveSession() returns it instead of // empty — mirrors the real request flow (inbound recorded first). store.Append("sess-active", pipeline.SessionEvent{ - At: time.Now().Add(-50 * time.Millisecond), + At: time.Now().Add(-50 * time.Millisecond), Direction: pipeline.Inbound, Phase: pipeline.SessionRequest, A2A: &pipeline.A2AExtension{Method: "message/send", SessionID: "sess-active"}, }) @@ -1486,3 +1486,74 @@ func keysOf(m map[string]json.RawMessage) []string { } return out } + +// schemeCapturePlugin captures the pctx.Scheme value it sees on +// OnRequest. Used by the per-listener scheme-wiring tests to verify +// each listener populates the field from its transport-native source. +type schemeCapturePlugin struct { + got string +} + +func (p *schemeCapturePlugin) Name() string { return "scheme-capture" } +func (p *schemeCapturePlugin) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *schemeCapturePlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.got = pctx.Scheme + return pipeline.Action{Type: pipeline.Continue} +} +func (p *schemeCapturePlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// TestExtProc_PopulatesSchemeFromPseudoHeader verifies that each pctx +// construction path in the ext_proc listener (inbound + outbound, +// headers-only + body) reads the :scheme pseudo-header and surfaces +// it on pctx.Scheme. Plugins composing target URLs or branching on +// transport depend on this being populated unconditionally. +func TestExtProc_PopulatesSchemeFromPseudoHeader(t *testing.T) { + tests := []struct { + name string + direction string + scheme string + want string + }{ + {name: "inbound_http", direction: "inbound", scheme: "http", want: "http"}, + {name: "inbound_https", direction: "inbound", scheme: "https", want: "https"}, + {name: "outbound_http", direction: "outbound", scheme: "http", want: "http"}, + {name: "outbound_https", direction: "outbound", scheme: "https", want: "https"}, + {name: "missing_pseudo_header_is_empty", direction: "inbound", scheme: "", want: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + capturer := &schemeCapturePlugin{} + p, err := plugintesting.BuildPipeline([]pipeline.Plugin{capturer}) + if err != nil { + t.Fatalf("BuildPipeline: %v", err) + } + srv := &Server{ + InboundPipeline: pipeline.NewHolder(p), + OutboundPipeline: pipeline.NewHolder(p), + } + + hdrs := []string{ + "x-authbridge-direction", tc.direction, + ":path", "/x", + ":authority", "agent.example", + } + if tc.scheme != "" { + hdrs = append(hdrs, ":scheme", tc.scheme) + } + stream := &mockStream{ + ctx: context.Background(), + requests: []*extprocv3.ProcessingRequest{ + inboundRequest(makeHeaders(hdrs...)), + }, + } + _ = srv.Process(stream) + if capturer.got != tc.want { + t.Errorf("pctx.Scheme = %q, want %q", capturer.got, tc.want) + } + }) + } +} diff --git a/authbridge/cmd/authbridge/listener/forwardproxy/server.go b/authbridge/cmd/authbridge/listener/forwardproxy/server.go index f3c96d43f..c41945743 100644 --- a/authbridge/cmd/authbridge/listener/forwardproxy/server.go +++ b/authbridge/cmd/authbridge/listener/forwardproxy/server.go @@ -56,6 +56,7 @@ func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) { pctx := &pipeline.Context{ Direction: pipeline.Outbound, + Scheme: r.URL.Scheme, Host: r.Host, Path: r.URL.Path, Headers: r.Header.Clone(), diff --git a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go index 77e7d43df..4fba313bd 100644 --- a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go @@ -403,4 +403,63 @@ func TestRecordOutboundReject_SkipsWithoutInvocations(t *testing.T) { if v := store.View(session.DefaultSessionID); v != nil { t.Errorf("expected no event, got %+v", v) } -} \ No newline at end of file +} + +// schemeCapturePlugin captures pctx.Scheme for the scheme-wiring +// test below. +type schemeCapturePlugin struct { + got string +} + +func (p *schemeCapturePlugin) Name() string { return "scheme-capture" } +func (p *schemeCapturePlugin) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *schemeCapturePlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.got = pctx.Scheme + return pipeline.Action{Type: pipeline.Continue} +} +func (p *schemeCapturePlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// TestForwardProxy_PopulatesSchemeFromRequestURL verifies the +// forward-proxy listener surfaces r.URL.Scheme on pctx. For HTTP +// forward proxies the agent's request line carries the full URL +// including scheme, so Go's net/http populates r.URL.Scheme +// reliably. +func TestForwardProxy_PopulatesSchemeFromRequestURL(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer backend.Close() + + capturer := &schemeCapturePlugin{} + p, err := pipeline.New([]pipeline.Plugin{capturer}) + if err != nil { + t.Fatalf("pipeline.New: %v", err) + } + srv := &Server{OutboundPipeline: pipeline.NewHolder(p), Client: http.DefaultClient} + proxy := httptest.NewServer(srv.Handler()) + defer proxy.Close() + + // Use the backend's http:// URL so the proxy actually dials it. + // pctx.Scheme is observed BEFORE the outbound call, so the value + // we assert on is whatever r.URL.Scheme was when the pipeline + // ran, independent of whether the backend responds OK. + req, _ := http.NewRequest("GET", backend.URL+"/x", nil) + proxyClient := &http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyURL(mustParseURL(proxy.URL)), + }, + } + resp, err := proxyClient.Do(req) + if err != nil { + t.Fatalf("proxy request failed: %v", err) + } + resp.Body.Close() + + if capturer.got != "http" { + t.Errorf("pctx.Scheme = %q, want http", capturer.got) + } +} diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server.go b/authbridge/cmd/authbridge/listener/reverseproxy/server.go index 66d2409e8..6491aa97b 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server.go @@ -75,6 +75,7 @@ func (s *Server) Handler() http.Handler { func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) { pctx := &pipeline.Context{ Direction: pipeline.Inbound, + Scheme: requestScheme(r), Host: r.Host, Path: r.URL.Path, Headers: r.Header.Clone(), @@ -232,6 +233,22 @@ func (s *Server) recordInboundReject(pctx *pipeline.Context, action pipeline.Act // writeRejection renders a pipeline Reject to the http.ResponseWriter, // preserving the plugin's status, headers, and body. +// requestScheme derives the URL scheme for an incoming server-side +// request. On server requests Go does not populate r.URL.Scheme (it's +// only set for client-side / proxy requests where the full URL is on +// the request line), so we read it from r.TLS instead: TLS present => +// https, absent => http. +// +// Does not consult X-Forwarded-Proto. Honoring that header is only +// safe when the upstream proxy is trusted; wiring a trust policy is +// deferred until we have a concrete multi-hop deployment story. +func requestScheme(r *http.Request) string { + if r.TLS != nil { + return "https" + } + return "http" +} + func writeRejection(w http.ResponseWriter, action pipeline.Action) { status, headers, body := action.Violation.Render() for k, vs := range headers { diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go index f16a791ae..bd13d9546 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go @@ -385,3 +385,80 @@ func TestRecordInboundReject_SkipsWithoutInvocations(t *testing.T) { t.Errorf("expected no event, got %+v", v) } } + +// schemeCapturePlugin captures pctx.Scheme for the scheme-wiring +// test below. +type schemeCapturePlugin struct { + got string +} + +func (p *schemeCapturePlugin) Name() string { return "scheme-capture" } +func (p *schemeCapturePlugin) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *schemeCapturePlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.got = pctx.Scheme + return pipeline.Action{Type: pipeline.Continue} +} +func (p *schemeCapturePlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// TestReverseProxy_SchemeFromTLS verifies that pctx.Scheme is derived +// from r.TLS (presence = "https", absence = "http") rather than +// r.URL.Scheme, which Go leaves empty for server-side requests. +// httptest.NewServer is plaintext (TLS nil); NewTLSServer sets TLS so +// "https" falls out. +func TestReverseProxy_SchemeFromTLS(t *testing.T) { + // Backend that the reverse proxy forwards to. Scheme on backend is + // irrelevant here — we're asserting on pctx.Scheme at the listener + // entry, which reflects the caller→proxy leg, not the proxy→backend + // leg. + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer backend.Close() + + tests := []struct { + name string + useTLS bool + want string + }{ + {name: "plaintext_inbound", useTLS: false, want: "http"}, + {name: "tls_inbound", useTLS: true, want: "https"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + capturer := &schemeCapturePlugin{} + p, err := pipeline.New([]pipeline.Plugin{capturer}) + if err != nil { + t.Fatalf("pipeline.New: %v", err) + } + srv, err := NewServer(pipeline.NewHolder(p), nil, backend.URL) + if err != nil { + t.Fatalf("NewServer: %v", err) + } + + var proxy *httptest.Server + var client *http.Client + if tc.useTLS { + proxy = httptest.NewTLSServer(srv.Handler()) + client = proxy.Client() // trusts the test cert + } else { + proxy = httptest.NewServer(srv.Handler()) + client = http.DefaultClient + } + defer proxy.Close() + + resp, err := client.Get(proxy.URL + "/api") + if err != nil { + t.Fatalf("client.Get: %v", err) + } + resp.Body.Close() + + if capturer.got != tc.want { + t.Errorf("pctx.Scheme = %q, want %q", capturer.got, tc.want) + } + }) + } +} From 2f4e963d81d4d59f52282cf61dd6c0e5aded139f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 11 May 2026 17:09:51 -0400 Subject: [PATCH 3/4] feat(tokenbroker): Use pctx.Scheme for serverURL composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the D2 follow-up from PR #391's review. token-broker's serverURL construction was hardcoded to "http://" + host, which is fine for in-cluster cleartext but silently breaks any cross-cluster or internet-facing broker target. With pctx.Scheme (issue #397, part 2) surfaced by every listener, the plugin now reads the scheme from the framework instead of inventing it: serverURL := pctx.Scheme + "://" + host Defaults to "http" when pctx.Scheme is empty — matches the previous hardcoded behavior, so existing callers upgrade without surprise. The X-Server-Url header sent to the broker now reflects the actual scheme the agent used to call the target. Test: 3 cases (https, http-explicit, empty-defaults-http) asserting the X-Server-Url the broker received matches pctx.Scheme. Resolves #397 (part 3 of 3). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../authlib/plugins/tokenbroker/plugin.go | 12 ++++- .../tokenbroker/plugin_request_test.go | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index d6c3454ee..e7f74a587 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -224,8 +224,16 @@ func (p *TokenBroker) OnRequest(ctx context.Context, pctx *pipeline.Context) pip "broker route requires authorization token") } - // Derive server URL from host - serverURL := "http://" + host + // Derive server URL from scheme + host. pctx.Scheme is populated + // by the listener from :scheme (ext_proc / ext_authz) or + // r.URL.Scheme / r.TLS (forward / reverse proxy). Defaults to + // "http" when the listener couldn't determine one — matches the + // previous hardcoded behavior. + scheme := pctx.Scheme + if scheme == "" { + scheme = "http" + } + serverURL := scheme + "://" + host // Use the plugin's configured broker URL brokerURL := p.cfg.BrokerURL diff --git a/authbridge/authlib/plugins/tokenbroker/plugin_request_test.go b/authbridge/authlib/plugins/tokenbroker/plugin_request_test.go index ae9030f4f..683402f20 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin_request_test.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin_request_test.go @@ -475,3 +475,52 @@ func TestTokenBroker_OnResponse(t *testing.T) { t.Errorf("OnResponse() action.Type = %v, want %v", action.Type, pipeline.Continue) } } + +// TestTokenBroker_ServerURL_SchemeAware verifies the serverURL sent +// to the broker (via X-Server-Url) uses pctx.Scheme when populated. +// Defaults to "http" when pctx.Scheme is empty — matches the +// previous hardcoded behavior so existing callers upgrade without +// surprise. See issue #397. +func TestTokenBroker_ServerURL_SchemeAware(t *testing.T) { + tests := []struct { + name string + scheme string + want string + }{ + {name: "scheme_https", scheme: "https", want: "https://api.example.com"}, + {name: "scheme_http_explicit", scheme: "http", want: "http://api.example.com"}, + {name: "scheme_empty_defaults_http", scheme: "", want: "http://api.example.com"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var gotServerURL string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotServerURL = r.Header.Get("X-Server-Url") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"token": "x"}) + })) + defer srv.Close() + + p := NewTokenBroker() + cfg := `{"broker_url": "` + srv.URL + `", "default_policy": "broker"}` + if err := p.Configure(json.RawMessage(cfg)); err != nil { + t.Fatalf("Configure: %v", err) + } + + pctx := &pipeline.Context{ + Scheme: tc.scheme, + Host: "api.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer user-token"}, + }, + } + if action := p.OnRequest(context.Background(), pctx); action.Type != pipeline.Continue { + t.Fatalf("OnRequest action = %v, want Continue", action.Type) + } + if gotServerURL != tc.want { + t.Errorf("X-Server-Url = %q, want %q", gotServerURL, tc.want) + } + }) + } +} From 2f09932d1fc010c908254fd14ececc4a29f41031 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 11 May 2026 19:45:16 -0400 Subject: [PATCH 4/4] fixup: PR #399 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five small fixes from review: - reverseproxy/server.go requestScheme: document the intentional divergence from Context.Scheme's "empty when undetermined" convention. reverseproxy always returns "http" or "https" and is confidently wrong behind a TLS-terminating upstream (r.TLS is nil on the inner hop even though the caller's scheme was https). Named in the helper's godoc so a reader comparing listeners sees the contract difference acknowledged. - extproc scheme test: use outboundRequest(...) for outbound cases instead of the shape-identical inboundRequest(...). Reads cleaner and guards against the helpers drifting later. - forwardproxy + reverseproxy scheme tests: switch from pipeline.New to plugintesting.BuildPipeline (a thin wrapper over pipeline.New) so the four listener scheme tests share one construction style. Existing non-scheme tests in those files stay on pipeline.New — aligning only the scheme tests keeps blast radius narrow. - extproc's schemeCapturePlugin godoc now explicitly calls out the byte-for-byte duplication across the four listener test packages and why it's intentional (not worth exporting through authlib/ plugintesting). A reader spotting the duplication on grep finds the explanation in the extproc copy. No behavior change. All listener tests still pass. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../authbridge/listener/extproc/server_test.go | 16 +++++++++++++++- .../listener/forwardproxy/server_test.go | 7 +++++-- .../authbridge/listener/reverseproxy/server.go | 9 +++++++++ .../listener/reverseproxy/server_test.go | 7 +++++-- 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index 1bbb16d67..a977f2fc8 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -1490,6 +1490,12 @@ func keysOf(m map[string]json.RawMessage) []string { // schemeCapturePlugin captures the pctx.Scheme value it sees on // OnRequest. Used by the per-listener scheme-wiring tests to verify // each listener populates the field from its transport-native source. +// +// Duplicated byte-for-byte across the four listener test packages +// (extauthz, extproc, forwardproxy, reverseproxy) rather than +// promoted to authlib/plugintesting — the plugin shape is only +// useful in listener tests and not worth exporting. Each package +// has a copy with this same godoc. type schemeCapturePlugin struct { got string } @@ -1544,10 +1550,18 @@ func TestExtProc_PopulatesSchemeFromPseudoHeader(t *testing.T) { if tc.scheme != "" { hdrs = append(hdrs, ":scheme", tc.scheme) } + // Use the direction-matching helper even though + // inboundRequest and outboundRequest share the same + // shape today — the test reads more clearly and guards + // against future drift if the helpers diverge. + reqFn := inboundRequest + if tc.direction == "outbound" { + reqFn = outboundRequest + } stream := &mockStream{ ctx: context.Background(), requests: []*extprocv3.ProcessingRequest{ - inboundRequest(makeHeaders(hdrs...)), + reqFn(makeHeaders(hdrs...)), }, } _ = srv.Process(stream) diff --git a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go index 4fba313bd..876414044 100644 --- a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go @@ -435,9 +435,12 @@ func TestForwardProxy_PopulatesSchemeFromRequestURL(t *testing.T) { defer backend.Close() capturer := &schemeCapturePlugin{} - p, err := pipeline.New([]pipeline.Plugin{capturer}) + // BuildPipeline is a thin wrapper over pipeline.New; using it + // across all four listener scheme tests keeps the construction + // one-liner identical and the tests grep-parallel. + p, err := plugintesting.BuildPipeline([]pipeline.Plugin{capturer}) if err != nil { - t.Fatalf("pipeline.New: %v", err) + t.Fatalf("BuildPipeline: %v", err) } srv := &Server{OutboundPipeline: pipeline.NewHolder(p), Client: http.DefaultClient} proxy := httptest.NewServer(srv.Handler()) diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server.go b/authbridge/cmd/authbridge/listener/reverseproxy/server.go index 6491aa97b..89beeee6e 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server.go @@ -239,6 +239,15 @@ func (s *Server) recordInboundReject(pctx *pipeline.Context, action pipeline.Act // the request line), so we read it from r.TLS instead: TLS present => // https, absent => http. // +// Contract note: this listener intentionally diverges from the +// Context.Scheme godoc's "empty when undetermined" convention — it +// always returns "http" or "https" based on r.TLS. The fallback is +// confidently wrong when reverseproxy sits behind a TLS-terminating +// upstream (LB, ingress): r.TLS is nil on the inner hop even though +// the caller's actual scheme was https. Consumers that need the +// caller's scheme in that topology should plumb X-Forwarded-Proto +// once a trusted-upstream policy exists (not in this PR). +// // Does not consult X-Forwarded-Proto. Honoring that header is only // safe when the upstream proxy is trusted; wiring a trust policy is // deferred until we have a concrete multi-hop deployment story. diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go index bd13d9546..a45d81dde 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go @@ -430,9 +430,12 @@ func TestReverseProxy_SchemeFromTLS(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { capturer := &schemeCapturePlugin{} - p, err := pipeline.New([]pipeline.Plugin{capturer}) + // BuildPipeline matches the construction used in + // extproc/extauthz scheme tests — keeps the four + // listener tests grep-parallel. + p, err := plugintesting.BuildPipeline([]pipeline.Plugin{capturer}) if err != nil { - t.Fatalf("pipeline.New: %v", err) + t.Fatalf("BuildPipeline: %v", err) } srv, err := NewServer(pipeline.NewHolder(p), nil, backend.URL) if err != nil {