✨ Add auth/ and config/ — composition layer and mode presets (Phase 2)

[huang195]

Summary

Phase 2 of the modular token exchange library (#279). Adds the application layer that composes Phase 1 authlib building blocks into two functions all listeners will call.

Depends on: #310 (Phase 1)

auth/ — Composition layer

Function	Flow
HandleInbound	bypass check → extract bearer → validate JWT → allow/deny
HandleOutbound	resolve route → cache check → token exchange → cache result → allow/deny/replace

Key features:

• Per-request audience override for waypoint mode (derived from destination host)
• Actor token source for RFC 8693 Section 4.1 act claim chaining
• Per-route token endpoint override (different IdP per target service)
• Fail-closed: nil verifier denies requests (not silently allows)
• Case-insensitive bearer extraction (RFC 7235 compliance)
• Three no-token policies: client-credentials (envoy-sidecar), allow (waypoint), deny (proxy-sidecar)

config/ — Mode presets and validation

• YAML config with ${ENV_VAR} expansion (preserves undefined refs as-is)
• Three mode presets with locked vs user-overridable settings
• Startup validation rejects invalid combos (e.g., waypoint + ext-proc listener)
• Resolve() wires all authlib packages from a single config file
• Backwards-compatible route config (passthrough: true to action: passthrough)

Phase 1 changes

• exchange.ExchangeRequest.TokenEndpoint — per-request token URL override

Related issue(s)

Ref #279

Test plan

• go test ./... — 56 tests passing across 8 packages
• go vet ./... — no issues
• CI passes

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[pdettori]

Review Summary

Well-structured Phase 2 adding the auth composition layer (auth/, config/) on top of Phase 1 building blocks. The auth flow logic (HandleInbound, HandleOutbound) is clean and follows fail-closed principles for inbound validation. One correctness bug in CachedActorTokenSource where the cache degrades after the first TTL expiry. Test helpers use t.Fatal from handler goroutines which is technically undefined behavior in Go.

Areas reviewed: Go (auth, config, exchange, routing, bypass, cache, spiffe, validation), Tests
Commits: 6 commits, all signed-off ✅
CI status: All 15 checks passing ✅

Highlights

• Clean six-package decomposition with single-responsibility packages and well-defined interfaces
• Strong security defaults: fail-closed inbound (nil verifier denies), path normalization in bypass, SHA-256 cache keys, 1MB response body limit, 30s pre-expiry cache buffer
• 63 tests with good edge case coverage (case-insensitive bearer, path traversal, context cancellation, cache eviction)
• Good backwards compatibility (passthrough: true → action: passthrough)

[pdettori]

Bug: if c.expiresAt.IsZero() only matches on the very first call. After the initial 5-minute default TTL expires, the refresh path stores a new token (line 50) but does NOT update expiresAt — it's no longer zero, so this guard is skipped.

Result: every subsequent FetchToken call sees the stale expiresAt, treats the token as expired, and calls source(ctx) on every request. The cache degrades to no-caching after the first TTL.

Fix: always reset expiresAt after refresh:

c.token = token
c.expiresAt = time.Now().Add(5 * time.Minute) // default, SetTTL overrides

This type isn't wired up in Phase 2 but will be in Phase 3 — better to fix now before it ships.

[huang195]

Good catch. Fixed in 8cf07a1 — expiresAt is now always reset after refresh:

c.token = token
c.expiresAt = time.Now().Add(5 * time.Minute)

The IsZero() guard was wrong — after the first default TTL expired, subsequent refreshes never updated expiresAt, degrading the cache to no-caching.

[pdettori]

Suggestion: t.Fatal() called from an httptest.Server handler goroutine has undefined behavior in Go — it calls runtime.Goexit() on the wrong goroutine, which can cause test panics or hangs.

This pattern appears here and in several handlers in exchange/client_test.go.

Fix: use t.Error(err); http.Error(w, err.Error(), 500); return instead of t.Fatal(err) in all httptest handlers.

[huang195]

Fixed in 8cf07a1. Replaced all t.Fatal(err) in httptest handlers with http.Error(w, err.Error(), http.StatusBadRequest); return. Applied to both auth/auth_test.go and exchange/client_test.go.

[pdettori]

Suggestion: strings.Replace(..., "/token", "/certs", 1) replaces the first occurrence. A URL like http://token-service.example.com/.../token would incorrectly modify the hostname portion.

In practice this is safe because deriveKeycloakURLs builds clean URLs, but a suffix-based approach would be more robust:

if strings.HasSuffix(cfg.Outbound.TokenURL, "/token") {
    cfg.Inbound.JWKSURL = strings.TrimSuffix(cfg.Outbound.TokenURL, "/token") + "/certs"
}

[huang195]

Fixed in 8cf07a1. Switched to suffix-based approach:

if strings.HasSuffix(cfg.Outbound.TokenURL, "/token") {
    cfg.Inbound.JWKSURL = strings.TrimSuffix(cfg.Outbound.TokenURL, "/token") + "/certs"
}

This prevents accidentally modifying hostnames like http://token-service.example.com/....

[pdettori]

Approved

All 3 review findings addressed in 8cf07a1:

1. actor_cache.go — expiresAt now always reset after refresh (was stale after first TTL) ✅
2. auth_test.go / client_test.go — t.Fatal in httptest handlers replaced with http.Error + return ✅
3. resolve.go — JWKS URL derivation uses HasSuffix/TrimSuffix instead of strings.Replace ✅

PR #310 fixes also landed via b2ca966: audience enforcement, header param removal, 6 JWKSVerifier tests, TOCTOU fix, cache eviction TODO, SubjectToken validation.

Additional proactive improvements: ServiceNameFromHost extracted to shared package, generic JWT errors returned to clients (detail logged server-side).

10 commits, all signed-off. 15 CI checks passing. Clean Phase 2.