✨ Add listeners and single binary — four modes, one codebase (Phase 3)#312
Conversation
Extract reusable auth building blocks from go-processor into a standalone Go module at authbridge/authlib/. Six packages with 34 unit tests: - validation/ — JWKS-backed JWT verifier (lestrrat-go/jwx) - exchange/ — RFC 8693 token exchange + client credentials - cache/ — SHA-256 keyed token cache with TTL eviction - bypass/ — Path pattern matcher for public endpoints - spiffe/ — File-based JWT-SVID source - routing/ — Host-to-audience router with glob matching No Envoy/gRPC dependencies — pure Go library. Phase 2 will add the auth composition layer and config; Phase 3 wires up listeners. Ref: rossoctl#279 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
- exchange: Use http.NewRequestWithContext instead of PostForm so callers can cancel/timeout requests via context - exchange: Limit response body to 1MB via io.LimitReader - exchange: Add tests for connection failure, malformed JSON, and context cancellation - cache: Remove TOCTOU race in Get() — skip expired entry cleanup, rely on Set() eviction instead - routing: Add Matched field to ResolvedRoute so callers can distinguish explicit route match from default action fallback - bypass: Document path.Match limitation (* does not cross /) Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Transitive dependency golang.org/x/crypto v0.32.0 has a known DoS vulnerability flagged by the dependency review CI check. Bump to v0.48.0 to match the existing authproxy module. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Add the application layer that wires Phase 1 authlib building blocks
into two functions all listeners will call:
auth/:
- HandleInbound: bypass → extract bearer → validate JWT → allow/deny
- HandleOutbound: resolve route → cache check → exchange → cache result
- Per-request audience override for waypoint mode
- Actor token source for RFC 8693 act claim chaining
- Per-route token endpoint override
- Fail-closed: nil verifier denies (not allows)
- Case-insensitive bearer token extraction (RFC 7235)
config/:
- YAML config with ${ENV_VAR} expansion (preserves undefined refs)
- Three mode presets: envoy-sidecar, waypoint, proxy-sidecar
- Startup validation rejects invalid mode+listener combos
- Resolve() wires all authlib packages from config
- RouteConfig backwards compat (passthrough: true → action: passthrough)
Also updates exchange.Client to support per-request TokenEndpoint override
and ExchangeRequest.TokenEndpoint field.
56 tests across 8 packages, all passing.
Ref: rossoctl#279
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Close 4 feature parity gaps between old go-processor and new authbridge: - Derive TOKEN_URL and ISSUER from keycloak_url + keycloak_realm (operator sets these, not explicit URLs) - Derive JWKS URL from TOKEN_URL using Keycloak /token -> /certs convention - Support client_id_file and client_secret_file with startup polling (handles race with client-registration and spiffe-helper) - Add CachedActorTokenSource for actor token caching with 30s pre-expiry refresh (matches old actorTokenCache behavior) 7 new tests for derivation logic and file-based credentials. 63 total tests passing. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Two feature parity findings from comprehensive review: 1. ClientCredentials grant now passes audience to Keycloak (matching old go-processor behavior). Without this, Keycloak may issue tokens with wrong audience that target services reject. 2. Waypoint mode now auto-derives audience from destination hostname via AudienceDeriver callback (serviceNameFromHost). This eliminates the need for explicit routes in waypoint mode, matching the old waypoint behavior where audience = first DNS label of host. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
3b4dd1d to
b0d9dbb
Compare
pdettori
left a comment
There was a problem hiding this comment.
Review Summary
Phase 3 of the AuthBridge modular rewrite. Adds four protocol listeners (ext_proc, ext_authz, forward proxy, reverse proxy) and a unified cmd/authbridge binary that consolidates three separate codebases. Well-structured code with good separation of concerns, thorough test coverage (73 tests), and solid security practices (non-root container, pinned base images, response body limits, fail-closed defaults).
No blocking issues found — suggestions only.
Areas reviewed: Go, Dockerfile, Shell, YAML (go.work)
Commits: 9 commits, all signed-off ✅
CI status: all 14 checks passing ✅
|
|
||
| // ServiceNameFromHost extracts the service name from a Kubernetes host. | ||
| // "auth-target-service.authbridge.svc.cluster.local:8081" → "auth-target-service" | ||
| func ServiceNameFromHost(host string) string { |
There was a problem hiding this comment.
suggestion: ServiceNameFromHost is duplicated — identical logic exists as unexported serviceNameFromHost in config/resolve.go (line 165). Both extract the first DNS label from a Kubernetes host. Consider extracting to a shared location (e.g., a hostutil package or the routing package) to avoid drift between the two copies.
There was a problem hiding this comment.
Fixed. Moved ServiceNameFromHost to routing/hostutil.go as a shared exported function. Both config/resolve.go and extauthz/server.go now import from routing. Duplicate removed.
| var httpServers []*http.Server | ||
|
|
||
| // Start listeners based on mode | ||
| switch cfg.Mode { |
There was a problem hiding this comment.
suggestion: This mode switch has no default case. If a new mode is added to config.Validate() but not to main(), the binary would start with zero listeners and hang on <-sigCh — silently doing nothing. Adding default: log.Fatalf("unhandled mode %q", cfg.Mode) would make this fail-fast.
There was a problem hiding this comment.
Fixed. Added default: log.Fatalf("unhandled mode %q", cfg.Mode) to fail fast.
| shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 15*time.Second) | ||
| defer shutdownCancel() | ||
|
|
||
| for _, srv := range grpcServers { |
There was a problem hiding this comment.
suggestion: GracefulStop() blocks indefinitely without respecting the 15s shutdownCtx timeout. If a gRPC server has long-lived streams, the process may never exit, defeating the graceful shutdown contract with Kubernetes. Consider spawning a goroutine that calls srv.Stop() (hard stop) when shutdownCtx expires, similar to the HTTP servers' Shutdown(shutdownCtx) pattern.
for _, srv := range grpcServers {
go func(s *grpc.Server) {
<-shutdownCtx.Done()
s.Stop()
}(srv)
srv.GracefulStop()
}There was a problem hiding this comment.
Fixed. Added goroutine that calls srv.Stop() when shutdownCtx expires, matching the pattern you suggested. GracefulStop runs normally, but if it takes longer than 15s (long-lived ext_proc streams), the hard Stop fires.
|
|
||
| // SetTTL updates the cache expiry based on the token's expires_in value. | ||
| // Called by the auth layer after a successful client-credentials grant. | ||
| func (c *CachedActorTokenSource) SetTTL(expiresIn time.Duration) { |
There was a problem hiding this comment.
suggestion: SetTTL() is defined but never called anywhere in this PR. The comment on line 58 says "The actual TTL is set via SetTTL after the client-credentials grant" but no caller does this. The cache will use the hardcoded 5-minute default forever. If this is wired in a future PR, a brief note (e.g., // TODO(phase4): wire SetTTL from client-credentials response) would clarify intent. If it's already dead code, consider removing it until needed.
| RUN cd cmd/authbridge && CGO_ENABLED=0 GOOS=linux go build -o /authbridge . | ||
|
|
||
| # Stage 2: Get Envoy binary | ||
| FROM docker.io/envoyproxy/envoy:v1.37.1 AS envoy-source |
There was a problem hiding this comment.
nit: The UBI base image is correctly pinned by SHA digest (line 27), but the Envoy source image is only pinned by version tag. For supply-chain consistency, consider pinning to SHA as well:
FROM docker.io/envoyproxy/envoy:v1.37.1@sha256:<digest> AS envoy-sourceThere was a problem hiding this comment.
Fixed. Envoy image now pinned by SHA: docker.io/envoyproxy/envoy:v1.37.1@sha256:70e7c82d8c82...
| } | ||
| } | ||
| w.WriteHeader(resp.StatusCode) | ||
| io.Copy(w, resp.Body) |
There was a problem hiding this comment.
nit: io.Copy error is silently discarded. While there's not much to do if the copy fails mid-response, a debug-level log would help troubleshoot connection issues (e.g., client disconnect during large response bodies).
There was a problem hiding this comment.
Fixed. io.Copy error now logged at debug level: slog.Debug("response copy error", "host", r.Host, "error", err)
| @@ -0,0 +1,260 @@ | |||
| package auth | |||
There was a problem hiding this comment.
praise: Excellent architecture throughout this PR. The clean separation between a pure library (authlib — no gRPC/Envoy deps) and thin protocol translators (listeners at 50-175 lines each) is exactly right. Fail-closed default (nil verifier → deny), 1MB response body limit, case-insensitive bearer extraction (RFC 7235), and 73 tests with good edge case coverage. Well done. 👏
…ation Address review from @pdettori on PR rossoctl#311: 1. actor_cache.go: Always reset expiresAt after token refresh. The IsZero() guard only triggered on the first call; subsequent refreshes left expiresAt stale, degrading the cache to no-caching. 2. auth_test.go, client_test.go: Replace t.Fatal(err) with http.Error() + return in httptest handlers. t.Fatal calls runtime.Goexit on the wrong goroutine, causing undefined behavior. 3. resolve.go: Use suffix-based HasSuffix + TrimSuffix for JWKS URL derivation instead of strings.Replace. Prevents accidentally modifying hostnames containing "token". Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Address review from @pdettori: - Extract ServiceNameFromHost to routing/hostutil.go (shared by config/resolve.go and extauthz listener, avoids duplication) - Add TODO comment on SetTTL (unused, clarifies intent) Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…voy SHA pin Address review from @pdettori on PR rossoctl#312: 1. Use shared routing.ServiceNameFromHost instead of local duplicate in extauthz listener (avoids drift between two copies) 2. Add default case to mode switch in main.go (fail-fast on unhandled mode instead of silent zero-listener hang) 3. GracefulStop with timeout fallback: spawn goroutine that calls Stop() when shutdownCtx expires, preventing indefinite block on long-lived ext_proc streams 4. Pin Envoy image by SHA digest for supply-chain consistency 5. Log io.Copy error in forward proxy for debuggability Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
b0d9dbb to
f23487d
Compare
JWT validation errors can reveal issuer URL, audience, or algorithm details. Return 'token validation failed' to the client and log the full error server-side at INFO level. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…voy SHA pin Address review from @pdettori on PR rossoctl#312: 1. Use shared routing.ServiceNameFromHost instead of local duplicate in extauthz listener (avoids drift between two copies) 2. Add default case to mode switch in main.go (fail-fast on unhandled mode instead of silent zero-listener hang) 3. GracefulStop with timeout fallback: spawn goroutine that calls Stop() when shutdownCtx expires, preventing indefinite block on long-lived ext_proc streams 4. Pin Envoy image by SHA digest for supply-chain consistency 5. Log io.Copy error in forward proxy for debuggability Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Address review from @pdettori on PR rossoctl#313: - Add ReadHeaderTimeout: 10s to HTTP servers (slowloris defense) - Add pipefail to entrypoint.sh set flags Findings 1 (JWT error leakage) fixed in PR rossoctl#311. Findings 2 (GracefulStop) and 4 (Envoy SHA pin) already fixed in PR rossoctl#312 review round. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
f23487d to
613d280
Compare
…voy SHA pin Address review from @pdettori on PR rossoctl#312: 1. Use shared routing.ServiceNameFromHost instead of local duplicate in extauthz listener (avoids drift between two copies) 2. Add default case to mode switch in main.go (fail-fast on unhandled mode instead of silent zero-listener hang) 3. GracefulStop with timeout fallback: spawn goroutine that calls Stop() when shutdownCtx expires, preventing indefinite block on long-lived ext_proc streams 4. Pin Envoy image by SHA digest for supply-chain consistency 5. Log io.Copy error in forward proxy for debuggability Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Address review from @pdettori on PR rossoctl#313: - Add ReadHeaderTimeout: 10s to HTTP servers (slowloris defense) - Add pipefail to entrypoint.sh set flags Findings 1 (JWT error leakage) fixed in PR rossoctl#311. Findings 2 (GracefulStop) and 4 (Envoy SHA pin) already fixed in PR rossoctl#312 review round. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…JWKS tests Address review from @pdettori on PR rossoctl#310: 1. MUST-FIX: Empty audience now returns error in JWKSVerifier.Verify() instead of silently skipping the check. Enforces the confused deputy protection invariant stated in the interface doc. 2. Remove http.Header param from ClientAuth.Apply() interface — callers always passed nil, risking NPE for future header-based implementations. Simplified to form-only. Add header support later if needed. 3. Add 6 JWKSVerifier unit tests with in-memory httptest JWKS server: valid token, expired, wrong issuer, wrong audience, empty audience, invalid signature. Covers the core security path. 4. Validate SubjectToken != empty in Exchange() for clear early error. 5. Fix TOCTOU in LoadRoutes — call ReadFile directly, check IsNotExist. 6. Add TODO for cache eviction strategy (thundering herd on full clear). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Add four protocol listeners and a unified cmd/authbridge binary that replaces three separate codebases (go-processor, waypoint, klaviger). Listeners (thin protocol translators, 50-170 lines each): - listener/extproc/ — Envoy ext_proc gRPC streaming (envoy-sidecar mode) - listener/extauthz/ — Envoy ext_authz gRPC unary (waypoint mode) - listener/forwardproxy/ — HTTP forward proxy (waypoint + proxy-sidecar) - listener/reverseproxy/ — HTTP reverse proxy (proxy-sidecar mode) cmd/authbridge/main.go: - --mode (envoy-sidecar|waypoint|proxy-sidecar) + --config flags - Mode selects which listeners start - Graceful shutdown with 15s timeout on SIGTERM - gRPC health checks on all gRPC servers Infrastructure: - go.work at authbridge/ level for multi-module development - Separate Go module (cmd/authbridge) keeps gRPC/Envoy deps out of authlib - Dockerfile: multi-stage build producing distroless image 17 listener tests + 56 authlib tests = 73 total, all passing. Ref: rossoctl#279 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The go.work file was missing the authproxy module, causing go fmt/vet to fail in CI with "directory prefix . does not contain modules listed in go.work". Also fixes import ordering in go-processor/main_test.go (go fmt). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The Dockerfile now produces a single image containing both Envoy and the authbridge binary, a true drop-in replacement for envoy-with-processor. The entrypoint uses wait -n process supervision (same pattern as PR rossoctl#308). Key changes: - Multi-stage build: Go builder, Envoy binary, ubi9-minimal runtime - entrypoint.sh starts authbridge then Envoy, monitors both with wait -n - Runs as UID 1337 (required by proxy-init iptables rules) - Same base image (ubi9-minimal) and ports as Dockerfile.envoy Verified E2E on Kind cluster: inbound JWT validation, outbound RFC 8693 token exchange, bypass paths, and process supervision all working. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…voy SHA pin Address review from @pdettori on PR rossoctl#312: 1. Use shared routing.ServiceNameFromHost instead of local duplicate in extauthz listener (avoids drift between two copies) 2. Add default case to mode switch in main.go (fail-fast on unhandled mode instead of silent zero-listener hang) 3. GracefulStop with timeout fallback: spawn goroutine that calls Stop() when shutdownCtx expires, preventing indefinite block on long-lived ext_proc streams 4. Pin Envoy image by SHA digest for supply-chain consistency 5. Log io.Copy error in forward proxy for debuggability Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Address review from @pdettori on PR rossoctl#313: - Add ReadHeaderTimeout: 10s to HTTP servers (slowloris defense) - Add pipefail to entrypoint.sh set flags Findings 1 (JWT error leakage) fixed in PR rossoctl#311. Findings 2 (GracefulStop) and 4 (Envoy SHA pin) already fixed in PR rossoctl#312 review round. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
613d280 to
2bd14c8
Compare
…voy SHA pin Address review from @pdettori on PR rossoctl#312: 1. Use shared routing.ServiceNameFromHost instead of local duplicate in extauthz listener (avoids drift between two copies) 2. Add default case to mode switch in main.go (fail-fast on unhandled mode instead of silent zero-listener hang) 3. GracefulStop with timeout fallback: spawn goroutine that calls Stop() when shutdownCtx expires, preventing indefinite block on long-lived ext_proc streams 4. Pin Envoy image by SHA digest for supply-chain consistency 5. Log io.Copy error in forward proxy for debuggability Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Address review from @pdettori on PR rossoctl#313: - Add ReadHeaderTimeout: 10s to HTTP servers (slowloris defense) - Add pipefail to entrypoint.sh set flags Findings 1 (JWT error leakage) fixed in PR rossoctl#311. Findings 2 (GracefulStop) and 4 (Envoy SHA pin) already fixed in PR rossoctl#312 review round. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Summary
Phase 3 of the modular token exchange library (#279). Adds four protocol listeners and a unified
cmd/authbridgebinary that replaces three separate codebases (go-processor, waypoint, klaviger).Depends on: #311 (Phase 2), #310 (Phase 1)
Four listeners
Each is a thin protocol translator — extracts
(authHeader, host, path)from the protocol, callsauth.HandleInbound/HandleOutbound, maps the result back.listener/extproc/listener/extauthz/listener/forwardproxy/listener/reverseproxy/cmd/authbridge/main.go--mode(envoy-sidecar, waypoint, proxy-sidecar) +--configflagsInfrastructure
go.workatauthbridge/level for multi-module local developmentreplacedirective for local authlib resolutionTest coverage
Related issue(s)
Ref #279
Test plan
go test ./...in both modules — 73 tests passinggo vet ./...— cleango build ./...— binary buildscd authbridge && docker build -f cmd/authbridge/Dockerfile .)Assisted-By: Claude (Anthropic AI) noreply@anthropic.com