mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-12 04:04:26 +00:00
main
217
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  lukaszraczyloandGitHub
|
f0af6282dc |
feat(templates): add toJson helper for object claims (#165)
Header templates can now emit an object or array claim as one JSON
header. Example: {{get .Claims "tenant" | toJson}}. The backend parses
the claim, so the middleware does not need to know its schema.
The helper adds no new access path. The validator treats toJson like
default and checks its arguments. A claim must be on the whitelist, and
a dump of the whole claims map is rejected. An absent or empty claim
skips the header. The JSON encoder escapes control characters.
A rendered header value over 8192 bytes is still dropped. The drop is
now logged at error level, with the header name and without the value.
Other sanitizer drops stay at debug level.
Tests cover value types, edge cases, whitelist bypass attempts, token
leaks and header injection. A new yaegi check runs the helper under the
interpreter. README, configuration docs and the plugin manifest show the
new helper.
|
||
|
lukaszraczylo
|
7c5a86f4e1 | chore: remove dependabot config | ||
|
lukaszraczyloandGitHub
|
94a8664403 |
perf(session): cut yaegi allocations on authenticated requests (#164)
Under yaegi, the authenticated cookie path allocated about 3.8 MB per request. A yaegi harness measures bytes per request with a browser-like cookie jar. With it, these changes cut bytes by 83%, allocations by 69% and time by about 93%. - Replace range loops over tokens with index loops. Under yaegi, ranging over a string allocates O(n^2) bytes. - Pool gzip readers for combined session cookies. - Memoize the validated ID token within one request. - Cache content-only token validation across requests. The key is the token type and SHA-256, and the cache holds at most 4096 entries. The entry keeps the parsed exp and iat claims. The expiry and freshness checks still run on every request. New tests pin the validator results, the memo and the cache behavior. |
||
|
lukaszraczylo
|
74decb21c2 |
docs: note where streamingRefresh sends the WebSocket cookie
On a WebSocket upgrade, the refreshed session cookie arrives on the 101 response. Traefik keeps middleware headers on that response, and RFC 6455 section 4.1 has clients store cookies from it. Claude-Session: https://claude.ai/code/session_01G1d1n1BrHHHw8dBUkbiBPC |
||
|
lukaszraczylo
|
7ae75d2523 |
docs: add #162 changelog entry and rollback cookie note
Add the #162 fix to the changelog. Document that a rollback to v1.0.25 or earlier invalidates sessions, because those releases cannot read the encrypted cookies from v1.0.26. Tell operators which cookies to delete. Claude-Session: https://claude.ai/code/session_01G1d1n1BrHHHw8dBUkbiBPC |
||
|
lukaszraczylo
|
937776516d |
feat(middleware): refresh expired sessions on SSE/WebSocket bypass
The SSE and WebSocket bypass does not check the token expiry. A session stays usable for streaming until the session cookie expires, also after the IdP disables the user. The bypass now forwards the configured headers (#162), so this gap also gives the proxy secret to such a session. Add the streamingRefresh option. The default is false. When it is true and the access token has expired, the bypass refreshes the session. It uses the ID token expiry if the access token is opaque. If the IdP rejects the refresh with invalid_grant, the request gets 401. If the IdP cannot be reached, the request is forwarded as before. The bypass waits for the full coordinated refresh. A shorter wait can drop a rotated refresh token, and the next refresh then logs the user out. applyBypassUserHeaders now takes the ResponseWriter, so a refresh can write the new session cookie. Add a yaegi check for the invalid_grant path through the real token endpoint and RefreshCoordinator. Claude-Session: https://claude.ai/code/session_01G1d1n1BrHHHw8dBUkbiBPC |
||
|
lukaszraczylo
|
c9844d0ff2 |
fix(middleware): forward configured headers on SSE/WebSocket bypass (#162)
The SSE and WebSocket bypass forwarded only X-Forwarded-User. It did not apply the configured headers. Backends that authenticate the proxy with a shared-secret header returned 401 on WebSocket upgrades. Version 0.8.x sent WebSocket upgrades through the normal flow, so this is a regression. The bypass now renders the header templates through applyHeaderTemplates, which forwardAuthorized also uses. Templates use the ID-token claims from the session cookie. If the session has no ID token, they use the access-token claims. The bypass now also rejects a session that the IdP logged out through back-channel or front-channel logout. Without this check, a logged-out cookie can open streams with the proxy secret attached. Add a yaegi check for the WebSocket case. Move tokenWithGroups to the shared JWT test helpers, as FIX-41 requires. Claude-Session: https://claude.ai/code/session_01G1d1n1BrHHHw8dBUkbiBPC |
||
|
|
45e99551f9 |
Security & correctness hardening: auth flow, cache, recovery, DCR, docs (#155)
* Audit review (rounds 1-31): fix confirmed issues, remove dead code, harden tests
Security/correctness fixes:
- fail-open token validation preserved with lenient-audience; access token that
fails verification and has no ID token to corroborate now fails closed
- token_manager blacklist-expiry: fix impossible tautology (x>24h && x<24h)
- constant-time OAuth state compare; clear state/nonce/code_verifier on
callback token-exchange failure to prevent replay of half-failed flows
- StopAllTasks gated on last-instance close
- background-task panic recovery; remove task-registry counter leak
- circuit breaker half-open: release probe slot; CB + per-session outcome
recorded once per refresh operation, not once per waiter
- Redis connection pool: handle nil receive when pool closed concurrently
- FileStore: atomic temp-write + rename
- errors_recovery health-check data race -> atomic
Test-isolation / hardening:
- reset helpers for Once-pinned global cache manager and transport pool
- coverage: utils 70->93% (redirect URL security contract); fuzz targets for
bearer parsers; concurrent ServeHTTP race tests for the shared request path
Dead-code removal (verified orphaned, build+suite green): internal/providers
(30 files, 0 importers), internal/compat, internal/features, input_validation.
Verified: govulncheck, go mod tidy, yaegi-validate, cross-compile (darwin+
linux), golangci-lint (new-from-rev clean), 3x full race+shuffle green.
* Lint: fix real int-truncation bug; document intentional gosec findings
- TransportPool configKey: write conn-count ints verbatim (strconv) instead of
byte() truncation — a config value >=256 previously truncated and made two
distinct configs collide on one key, silently sharing a transport with the
wrong settings (real fix, gosec G115).
- Document design-intentional gosec findings with //nolint:gosec + rationale:
G117 (clients_secret intentionally marshaled into stored creds JSON),
G704 (tokenURL is the trusted discovery endpoint), G124 (delete/test cookie,
Secure set conditionally), G118 (long-lived background refresh) and pool
utils short-ID rune, testutil mock server redirects.
Result: full-tree golangci-lint now reports 0 issues (was ~12). Build, vet,
and the full test suite remain green.
* Fix singleton ResourceManager data race (unlocked read vs test reset)
GetResourceManager returned the process-global via an unprotected read
(singleton_resources.go:50), while resetResourceManagerForTesting and
ResetResourceManager write it under resourceManagerMutex. A live background
task (graceful-degradation health check started by one test) reading the
singleton while a shuffled test resets it was a data race (found via
-race -shuffle: Write at singleton_resources_test.go:941/942 vs Read at
autocleanup.go:554 / error_recovery.go:910).
Fix: hold resourceManagerMutex around the read, matching the sibling
GetGlobalTaskRegistry (autocleanup.go:414). Added a deterministic
concurrent regression test (GetResourceManager vs reset under -race).
Verified: 4x full -race -count=1 -shuffle=on ./... all exit 0, 0 races,
0 fails; source-order suite green; lint clean.
* Fix 6 issues surfaced by adversarial agent team (round 35)
Concurrency/lifecycle (P1, highest-value):
- singleton_resources.go: RegisterBackgroundTask now REPLACES a stopped
task instead of reusing it. BackgroundTask.Start is sync.Once-guarded,
so a task stopped on instance close and re-registered (Traefik
close+recreate for the same provider) was permanently dead: IsTaskRunning
returned false (started && !stopped) so callers retried a start that
was a no-op. Metadata refresh, token cleanup, and health checks now
resume across a recreate cycle.
- error_recovery.go: the shared graceful-degradation health task was
bound to the FIRST instance's performHealthChecks (later instances'
checks never ran) and any instance's Close stopped it for all. Added a
global live-instance registry; the shared task runs every instance's
checks and is stopped only when the last instance closes (mirrors the
singleton-token-cleanup gate).
Storage/cache:
- hybrid.go recordL2Error: Store(now) then immediately compared
time.Since(t)<1s against its own write => ALWAYS fired, so a single
transient L2 error flipped the whole cache into fallback (full L2
bypass -> upstream stampede). Now enters fallback only on a PRIOR
error within the window (true "multiple errors"). Updated two tests
that had encoded the old single-error-fallback bug.
- redis_pool.go: ConnectionPool.Put checked closed but sent on
p.connections without holding p.mu; Close closed it under p.mu.
Window => 'send on closed channel' panic at shutdown. Put now holds
p.mu (the lock only Close used).
HTTP/transport:
- jwk.go fetchJWKS: added an explicit >1 MiB guard (was silent partial
read, could yield a truncated keyset and spurious missing-kid).
- http_client_pool.go configKey: now includes every transport-behavior
field it actually consumes (compression, HTTP2, keep-alives, timeouts,
buffer sizes, dial) so behaviorally different configs no longer share
one transport built with the first config's settings.
Each fix has a regression test. Verified: full source-order suite green,
golangci-lint 0 issues, 3x full -race -count=1 -shuffle=on ./... all exit 0.
* Fix round-36 issues: DCR validation, auth-method default, shared metadata task, replay false-positive
Four verified fixes from the round-36 agent team (each confirmed against source
before acting):
- settings.go: Config.Validate now skips the client-credential block when
Dynamic Client Registration is enabled and no clientID is pre-provisioned
(RFC 7591 provides credentials at registration). Previously the
unconditional 'clientID is required' made DCR unreachable via New.
- dynamic_client_registration.go: default registered token_endpoint_auth_method
to client_secret_post to match the plugin's runtime default; an enforcing
IdP would otherwise reject the plugin's token exchange.
- utilities.go: gate the shared per-provider singleton-metadata-refresh task
stop on lastInstance (matching singleton-token-cleanup) so one instance's
tear down no longer kills 2h metadata refresh for a surviving sibling.
- token_manager.go: stop self-recording the JTI into the per-instance
tokenBlacklist on every verify. On the cookie path the same valid token is
re-presented per request; an LRU eviction of the raw-token cache (MaxSize
1000 / 5 MiB) then turned a legitimate cache miss into a false 'token
replay detected'. Replay tracking stays in the shared shardedReplayCache.
Regression tests: new review_r36_regression_test.go (4 tests); updated
dynamic_client_registration_test.go and security_edge_cases_test.go which had
encoded the old defaults.
Verified: full source-order suite, full -race -count=1 -shuffle=on ./...
(all packages), and golangci-lint run ./... (0 issues).
* Fix dead CircuitBreaker.ResetTimeout config (round 39)
error_recovery.go: ResetTimeout was copied into the CircuitBreaker but never
consulted by the state machine (used only for a metric), so the documented
"how long to wait before fully closing the circuit" (reset_timeout, default
30s) had no effect: Open->HalfOpen used Timeout (60s), and a single
half-open success closed the circuit immediately, losing the intended
open -> half-open -> (reset cooldown) -> closed hysteresis and letting a
stray success immediately reopen (bounce).
Now the circuit remains half-open after entering it until resetTimeout has
elapsed; only then can a successful request fully close it. Half-open
requests are still allowed as probes. Default behavior is more
conservative (up to Timeout+ResetTimeout before full close).
Tests: updated TestCircuitBreakerHalfOpenTransition to the corrected
semantics; added TestCircuitBreakerResetTimeoutEnforced (fails on the old
dead-field code, where resetTimeout was ignored).
Verified: full source-order suite, full -race -count=1 -shuffle=on ./...
(all packages), golangci-lint run ./... (0 issues).
* Apply configured conn limits in pooled transport (round 41)
http_client_pool.go SharedTransportPool.GetOrCreateTransport built the shared
http.Transport with hardcoded MaxConnsPerHost=5, MaxIdleConnsPerHost=2,
MaxIdleConns=10, IdleConnTimeout=30s while HTTPClientConfig's versions of
those fields drove configKey (cache identity) and were validated and tuned
per preset (Default: 20/10/50/30s; OIDCProvider: 50/25/100/90s). The
config was thus valid, keyed, and validated but silently ignored in the
transport used by main.go (CreatePooledHTTPClient) - dead config that left
OIDC-provider connection tuning without effect. The sibling factory path
(http_client_factory.go) already applied these values.
Now the pooled transport derives MaxConnsPerHost, MaxIdleConnsPerHost,
MaxIdleConns, and IdleConnTimeout from the config, consistent with the
factory path and with configKey.
Test: added TestSharedTransportPoolAppliesConfigLimits, which fails on the
old hardcoded code (fixed values) and passes now.
Verified: full source-order suite, full -race -count=1 -shuffle=on ./...
(all packages), golangci-lint run ./... (0 issues).
* Harden redirect-count accessors against pooled-session data race (round 44)
* Fix bearer Authorization template strip order + nonce constant-time compare (round 45)
* Sanitize claim-derived identifier on auth-bypass headers (round 48)
* R51: guarantee openid scope in buildAuthURL when filter drops it
* R53: enforce 2048-bit minimum RSA key size on verify
* R54: refresh JWKS on unknown-kid to pick up key rotation
* R55: blacklist session tokens on user logout
* R56: honor RateLimit config as sustained req rate
* R57: FIFO eviction; keep fresh replay JTIs in cache
* R58: idempotent RefreshCoordinator Shutdown; no close-close
* R59: honor backend TTL on cache repopulation
* R60: accept single-string groups/roles; fix mem over-evict
* R61: detect truncated chunked token via stored total
* R62: drain queued tasks on goroutine pool shutdown
* R63: Shutdown waits for in-flight refresh goroutines
* R64: cap metadata cache grace by configured limits
* R65: validate ES client-assertion key curve matches alg
* R66: check expiry on lenient-audience token path
* R67: apply allowedRolesAndGroups gate to SSE/WS bypass
* R68: cache expiry TOCTOU, logout open-redirect, session max-age
* R69: fix double pool-return on session-timeout path
* R70: pooled-client cookie jar, refresh Save-failure cleanup
* R71: SSE/WS allowlist, session max-age, sec headers
* R72: validate combined cookie chunk metadata
* R73: logout jti replay, expired ajax, no-value
* R74: recv handler panics, cache TTL race fix
* R75: azure unauth copy-paste, dcr guard, cookie domain
* R76: dcr singleflight, L1 no-resurrect, TLS loopback
* R77: bearer allowedUsers gate, session inUse CAS
* R78: hybrid Clear flush, reauth reacquire, pool CAS
* R79: invalidation ttl tracks maxage, empty tpl hdr
* R80: req secret for introspection, pool conn on panic
* R81: reject empty access token on callback+refresh
* R82: req discovery endpoints, single-use code retry
* R83: DCR require client_secret for confidential auth, validate redirect_uri scheme; reject callback query/fragment
* R86: atomic SetIfAbsent for replay cache; close double-accept and singleton-read race
* R87: validate backchannel logout token exp; live-refresh JWKS for logout kid-miss
* R88: make StopAllTasks cover ResourceManager singleton tasks
TaskRegistry.CreateSingletonTask delegates to ResourceManager.RegisterBackgroundTask
and stores the task in the ResourceManager's registry, not TaskRegistry.tasks.
TaskRegistry.StopAllTasks previously iterated its own (always-empty) registry,
silently leaking every process-global singleton task's ticker goroutine past the
last Close (memory-monitor, replay-cache-cleanup) — tasks kept running real
periodic work with no instance to serve.
Add ResourceManager.StopAllTasks (stops all rm-owned tasks) and have
TaskRegistry.StopAllTasks stop both its own RegisterTask-direct tasks and the
ResourceManager-owned singletons.
Regression TestR88StopAllTasksStopsResourceManagerOwnedTasks (review_r88_test.go):
fails on old code (task still running after StopAllTasks), passes on new.
* R89: normalize excluded-url paths; apply defaults to pooled HTTP clients
Two verified defects from R89 scout round:
1. Excluded-URL dot-segment traversal widening (P1, auth bypass).
shouldBypassAuth passes req.URL.Path raw (middleware.go:54); Go's server
leaves '..' in URL.Path (url.ParseRequestURI does not normalize), so an
exclusion for /public also matched /public/../admin, letting a real
sibling (/admin) reach the backend unauthenticated. pathExcluded now
path.Clean()s the request path ('.' -> '' to preserve root semantics)
before boundary matching. Regression TestR89ExcludedPathDotTraversal
fails on old (all 4 traversal cases wrongly excluded), passes on new.
2. Pooled HTTP client had no zero-value defaults (P2, unbounded stall).
CreateHTTPClient applies defaults; CreatePooledHTTPClient and
GetOrCreateTransport did not, so a partial config yielded Timeout=0 and
ResponseHeaderTimeout=0 (a hanging upstream stalls forever). Extracted
applyHTTPClientDefaults (single source of truth) and applied it in all
three construction paths; pool cache key now reflects normalized values.
Regression TestR89PooledClientAppliesDefaults fails on old, passes on new.
R89 clean/declined: session max-age IS absolute-and-enforced-every-request
(verified, review_r68 already locks it); AllowedUserDomains fail-closed (P2
'*' not a wildcard, P3 TrimSpace — declined as config-scoped).
* R92: validate all time claims on lenient-audience access tokens
On the lenient-audience path (validateStandardTokensRS), jwt.Verify
short-circuits at the aud check before reaching exp/iat/nbf validation,
leaving every time claim unvalidated. accessTokenUnexpired previously
re-patched only 'exp' (the R66 fix), so a token with a not-yet-reached
'nbf' but a valid exp was trusted for authorization before it was valid.
accessTokenUnexpired now re-applies the full main-path time validation:
verifyExpiration, verifyIssuedAt (iat required), and verifyNotBefore when
present — all clock-skew-aware. Regression TestR92LenientPathRejectsFutureNbf
(plus valid control) fails on old code (future-nbf token accepted), passes on
new.
R92 clean/declined: bearer auth (aud enforced, full claims — clean);
refresh rotation (correctly bound to session; no local reuse detection =
design); logout cookie completeness (all chunk names cleared, no revive;
two P3 non-security notes declined).
* fix(auth): harden OIDC flow, token validation, session, and bearer handling
- replay entry covers exp + clock-skew window; callback GET-only (RFC 6749)
- callback rate-limit returns 429 + Retry-After; panic-recovery sets no-store
- cross-chunk integrity for chunked tokens; bounded chunked-token read scan
- HTTPError classification so 5xx/429 token-endpoint responses retry
- redaction of token/code/state in logs; per-source external auth gate
* fix(cache): harden cache backends, resilience, recovery, and transports
- RESP -ERR no longer closes healthy conn / aborts batch; global cap replacement-aware
- global health-check pass serialized (latent data race); MemoryMonitor via ResourceManager
- token-exchange retry, open-circuit rejections counted as failures
- TLS-posture matching transport reuse (no silent InsecureSkipVerify handoff)
* fix(config,dcr): env fallbacks, DCR self-heal, per-source auth gate
- implement documented OIDC_* env fallbacks; hold metadataMu for clientAssertion write
- re-register client when client_secret expires; require secret for confidential
- add opt-in per-source external auth rate limiting (PerSourceLoginRateLimit)
* docs: complete config reference and config examples
- CONFIGURATION.md: add 11 previously-omitted optional parameters
- README: document 6 implemented-but-undocumented options
- fix example config (raw .env block made invalid YAML)
* build: pin toolchain to go1.26.6 (resolves 5 reachable stdlib CVEs)
- net/http, asn1, idna GO-2026 CVEs cleared; govulncheck now 0 findings
* fix(jwk): use time.Since for elapsed-time computation (S1012)
* test: add R-round regression and security suite
- fail-on-old regression tests for every shipped fix (R-rounds)
- fuzz targets (parseJWT, bearer), concurrency-stress, shuffle-flake guards
- coverage-closing contract tests (health-check backend, TLS-posture reuse)
- r154 ID-token fixture built from parts to avoid false-positive secret scan
* test: mark synthetic PKCE verifier with trufflehog:ignore
Quiet the Lob detector's false positive on the fake test-only code_verifier,
matching the repo's existing // trufflehog:ignore convention in token_test.go.
Local trufflehog filesystem scan is now 0 verified / 0 unverified.
* fix(circuit-breaker): stop open-circuit rejections from re-arming the open timer (FIX-01)
ExecuteWithContext called the promoted BaseRecoveryMechanism.RecordFailure()
on every request rejected while the circuit is open. RecordFailure sets
lastFailureTime, the same field allowRequest read as the Open->HalfOpen
timer, so each rejection re-armed the timer. Under steady traffic shorter
than Timeout the breaker never left Open (the default-on token breaker,
token_resilience.go:31-36, is affected in production).
CircuitBreaker now tracks its own openedAt timestamp, set only on a
genuine Closed->Open or HalfOpen->Open transition in recordFailure.
allowRequest and IsAvailable gate on openedAt instead of lastFailureTime.
An open-circuit rejection still increments total_failures directly via
atomic.AddInt64, preserving the R180 metrics contract, but no longer
touches lastFailureTime or the open timer.
Adds TestCircuitBreakerOpenRejectionsDoNotExtendOpenWindow, which opens
the breaker (MaxFailures 1, Timeout 50ms) and sends a probe every 10ms;
it fails at head because no probe is ever admitted within 500ms.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(circuit-breaker): ignore terminal client errors in the breaker state machine (FIX-09)
After FIX-01, an ordinary fn() error still trips recordFailure(), which
reopens a half-open circuit unconditionally and holds it half-open for a
full ResetTimeout (15s/30s in production). A per-user HTTPError such as
invalid_grant from a replayed code is not a downstream-health signal, so
under a 1% user-error rate roughly 78% of half-open cycles reopened for
another ResetTimeout, extending real outages.
recordFailure() is now skipped for a terminal client error (HTTPError
4xx other than 429); the promoted RecordFailure() metrics counter still
runs, since the operation did return an error to its caller. 429 and
non-HTTPError failures (network errors, 5xx) are unaffected and still
trip the state machine.
Adds three regression tests: a half-open probe returning HTTPError 400
must stay half-open and later close on success (fails at head: reopens);
HTTPError 429 and a net.OpError must still reopen half-open (already
passing, pinned as a guard against over-broadening the exemption).
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(retry): stop single-use retry from matching HTTP response bodies (FIX-14)
ExecuteSingleUseWithContext substring-matched singleUseRetryableErrors
against the raw err.Error() text. helpers.go returns a *HTTPError whose
Message carries up to 10 KiB of the token-endpoint response body once a
response is received, so an IdP 500 that happened to mention "connection
refused" in its own body was retried up to MaxAttempts times even though
the request had definitely reached the server. Re-sending a consumed
authorization code or a rotated refresh token surfaces invalid_grant or
triggers refresh-token-family revocation on rotating IdPs.
Adds isNeverSentError: an *HTTPError (a response was received) is never
retried, and the fragment match now applies only to a genuine *net.OpError
(a dial/connect failure), the concrete type net/http.Client.Do returns
before any request reaches the wire.
Adds TestExecuteSingleUseWithContextNeverRetriesAfterHTTPResponse, which
fails at head (fn called 3 times instead of 1) and passes after the fix,
plus a guard that a real dial error is still retried.
Updates the existing TestRetryExecutor_SingleUse_RetriesPreSendError
(review_r82_test.go) to simulate the dial failure as a *net.OpError
instead of a bare string, matching what client.Do actually returns; the
bare-string form no longer satisfies the tightened, type-based match.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(circuit-breaker): address review for FIX-09
isTerminalClientError used errors.As(err, &httpErr) against an
interpreted *HTTPError target. Under yaegi v0.16.1 (Makefile:9) this
panics with "errors: *target must be interface or implement error",
regardless of the error's own concrete type. CircuitBreaker calls
this on every fn() error, on the default-on token-exchange/refresh
path, so every failed exchange or refresh panicked the goroutine
instead of returning its error, and the panic on the refresh path
(refresh_coordinator.go, an unrecovered goroutine) crashed the
process. Switch to a plain type assertion: the production error
reaches the breaker unwrapped, so no Unwrap chain walk is needed.
Extend cmd/yaegicheck/main.go to drive CircuitBreaker.ExecuteWithContext
under the real interpreter: a plain error (the first call shape that
panicked), a half-open 400 (must stay half-open), a closed-state 400
(must stay closed), and a 429 (must still reopen half-open). make
yaegi-validate now fails on the pre-fix code and passes after.
Also add a Closed-state regression test pinning the existing scope
choice (the terminal-client-error exemption applies to Closed as well
as HalfOpen, not only HalfOpen): a 400 leaves the breaker Closed, and a
following genuine 5xx still opens it. This is coverage-only -- the
Closed-state behavior itself already matched this before the change,
confirmed by running the new test against the pre-fix code (PASS).
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(retry): address review for FIX-14
isNeverSentError used errors.As(err, &httpErr) against an interpreted
*HTTPError target. Under yaegi v0.16.1 (Makefile:9) this panics with
"errors: *target must be interface or implement error", regardless of
the error's own concrete type. ExecuteSingleUseWithContext calls this
on every fn() error, on the default-on authorization-code-exchange and
refresh path, so every failed exchange or refresh panicked instead of
returning its error. Switch to a plain type assertion: the production
error reaches here unwrapped, so no Unwrap chain walk is needed. The
errors.As(err, &opErr) match against *net.OpError is untouched -- a
compiled stdlib type, yaegi-safe.
Also narrow the *net.OpError match to Op "dial" or "proxyconnect".
isNeverSentError previously matched singleUseRetryableErrors against
ANY *net.OpError, including "read"/"write", which happen after the
connection is already established and the single-use request may
already have reached the IdP. Only a failed connect proves the request
never went out. Correct the doc comment, which claimed a *net.OpError
is always pre-wire -- it is not.
Extend cmd/yaegicheck/main.go to drive
RetryExecutor.ExecuteSingleUseWithContext under the real interpreter: a
*HTTPError whose Message happens to contain a retryable-looking
fragment (must be called once, never retried) and a real client.Do
dial failure against a closed local port (must be retried to
MaxAttempts). make yaegi-validate now fails on the pre-fix code and
passes after.
Add a regression pinning the Op-scoping fix: a *net.OpError{Op: "read"}
carrying syscall.EHOSTUNREACH is not retried by
ExecuteSingleUseWithContext (fn called once). Confirmed this fails
against the pre-fix code (fn called 3 times) before applying the fix.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(recovery): stop gdInstances leak from TokenResilienceManager (FIX-18)
Register GracefulDegradation in gdInstances synchronously in
NewGracefulDegradation, before the health-check goroutine starts. A
Close() that ran immediately after construction could otherwise race
the async registration and delete nothing, then the later registration
would add an already-closed instance permanently.
Add Close to ErrorRecoveryManager and TokenResilienceManager.
NewTokenResilienceManager builds its own ErrorRecoveryManager (and
therefore its own GracefulDegradation), separate from the plugin
instance own errorRecoveryManager. Production Close() only ever closed
the first one, so the second leaked in gdInstances forever and the
shared health-check task last-instance stop gate never fired.
TraefikOidc.Close now calls Close on both managers.
Regression test TestFix18_ClosingBothManagersEmptiesGDInstances fails
to build at head (Close undefined on both managers) and passes after
the fix.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(tasks): keep pending singleton task instead of orphaning it (FIX-19)
RegisterBackgroundTask replaced any existing task with
started==0 && stopped==0, including one that is merely pending its
first Start() call. CreateSingletonTask (and the two other call sites
sharing its Register-then-Start pattern) does not hold tasksMu across
that sequence, so a second concurrent caller lands in that window and
replaces a task the first caller is about to start. The first caller
then starts an object no longer reachable from the registry, and
StopAllTasks/Shutdown never reach it again.
Add a startRefused flag to BackgroundTask, set on Starts refused by
the circuit breaker or the concurrency limiter (the two early-return
paths that leave started==0 and stopped==0 permanently).
RegisterBackgroundTask now replaces an existing task only when it is
stopped or its one Start attempt was refused, keeping a task that is
simply not started yet.
review_r122_test.go pinned the old "started==0 && stopped==0 always
means replace" behavior with a fixture that force-sets those fields
directly, without ever exercising a real Start() rejection. Update the
fixture to also set startRefused=1, the precise signal that case now
carries, preserving the R122 invariant (a circuit-breaker-rejected
task is still always replaced) alongside the new one.
Regression test TestFix19_RegisterBackgroundTaskKeepsPendingTaskNoOrphan
fails at head (orphan task keeps ticking after StopAllTasks: 13 -> 25)
and passes after the fix.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(lifecycle): re-check last-instance under shared mutex before stop (FIX-35)
Close() decided whether it was the last live instance once, early
in its shutdown sequence (unregisterLiveInstance return value), then
reused that boolean much later to gate three separate singleton
stops (singleton-token-cleanup, singleton-metadata-refresh-*, and
the general StopAllTasks). Session and cache teardown run in
between, taking real time, during which a concurrently-created new
instance (an overlapping Traefik reload) can register and adopt the
same singletons. The stale decision then stops them anyway, leaving
the surviving instance with no memory monitor, token cleanup, or
metadata refresh until the next reload.
Add liveInstanceMu, taken by registerLiveInstance,
unregisterLiveInstance and a new isLastInstanceNow. Close() now calls
isLastInstanceNow() fresh, immediately before each of the three
singleton stops, instead of reusing one early decision.
main.go now calls registerLiveInstance before New adopts the
memory-monitor singleton (previously called after), so a concurrent
Close elsewhere always sees a new instance counted before it can
decide to stop the singletons that instance depends on.
Regression test
TestFix35_ConcurrentNewKeepsSingletonAliveAcrossOldClose fails to
build at head (isLastInstanceNow undefined) and passes after the fix.
A throwaway scratch probe (not committed) confirmed the runtime bug
independently: reproducing the old stale-decision pattern inline
wrongly stops memory-monitor even after a new instance registered and
adopted it.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(refresh): reject CoordinateRefresh after Shutdown, correct stale ctx comments (FIX-36)
CoordinateRefresh called wg.Add(1) unconditionally, with no check
on rc.stopChan, so a request arriving during or after Shutdown still
registered and ran a new refresh operation. go doc sync.WaitGroup.Add
requires that Adds with a positive delta happening when the counter
is zero happen before a Wait; an Add racing the last Done during an
in-progress Wait can panic with WaitGroup misuse.
Add lifecycleMu (RWMutex). CoordinateRefresh now takes RLock around
its stopChan check and its wg.Add(1) call; Shutdown takes Lock()
before closing stopChan. Lock() cannot succeed while any
CoordinateRefresh call holds RLock, so every Add that will ever
happen has completed before Shutdown reaches wg.Wait().
CoordinateRefresh called after Shutdown now returns an error instead
of running.
The comment on the isNew branch claimed in-flight refreshes "are
aborted promptly via rc.ctx when stopChan closes" - rc.ctx does not
exist. An initial version of this fix added it, deriving
executeRefreshAsync timeout context from it and canceling it in
Shutdown, so Shutdown would return before a genuinely in-flight
refresh finished. That directly regressed two existing, deliberately
written tests - TestRefreshCoordinator_ShutdownWaitsForInFlight
(review_r154) and TestRefreshCoordinatorShutdownWaitsForInflight
(review_r63) - which pin that Shutdown must wait for an in-flight
refresh to finish naturally, not abandon it (avoiding a leaked
refresh goroutine). This fix keeps that contract: Shutdown still
waits via wg.Wait(), bounded only by each operation own
RefreshTimeout, and the stale comments are corrected to describe
that behavior accurately instead of a rc.ctx mechanism that does not
exist.
Regression test TestFix36_CoordinateRefreshRejectedAfterShutdown
fails at head (CoordinateRefresh after Shutdown wrongly succeeds
and runs refreshFunc) and passes after the fix. The R63/R154 tests
were re-run and confirmed still green.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* test(refresh): replace timing-margin sleeps with deterministic waits (FIX-22)
Four tests raced the 100ms DeduplicationCleanupDelay cleanup timer
with a 150ms fixed sleep, a 50ms margin. A late timer let the next
call join the previous operation instead of starting a new one,
under-counting circuit-breaker/rate-limit attempts on loaded CI
runners. A fifth test (internal/cache/backends) raced singleflights
own 100ms delayed-cleanup goroutine the same way.
Set DeduplicationCleanupDelay=0 on the four RefreshCoordinator tests
(TestRefreshCoordinatorIntegration CircuitBreaker and RateLimiting
subtests, TestCircuitBreakerProtection,
TestRefreshCoordinator_SuccessOnlyAfterCombinedOperations), matching
the pattern already used elsewhere in refresh_coordinator_test.go.
Delay=0 alone is not fully deterministic: close(operation.done) and
the synchronous cleanup call are two separate statements in the same
deferred closure, so a waiter woken by the channel close can run
concurrently with, and observe the map entry before, that same
goroutines next statement. Confirmed this manually: an
intermediate delay-only version (no polling) failed 1 run out of 20
under -race -shuffle=on with the exact under-counting symptom the
finding describes.
Add waitForRefreshDrain, which polls
RefreshCoordinator.inFlightRefreshes for the tokens removal instead
of sleeping a fixed margin, and use it in all four loops. For
internal/cache/backends/review_r141_test.go, poll
SingleflightCache.calls under its own mutex the same way, per the
todo acceptance criterion.
All four tests plus the r141 test now pass go test -race -count=20
-shuffle=on with no fixed sleep at the previously-flaky sites, and
keep their existing assertions.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* test(refresh): assert bounded Shutdown and goroutine exit (FIX-23)
TestRefreshCoordinatorShutdownStopsCleanup called Shutdown() with no
assertion, so it passed on base regardless of whether Shutdown
actually stopped the cleanup goroutine. Only the sibling
TestRefreshCoordinatorShutdownIdempotent caught anything (a panic on
double-close).
Assert Shutdown returns within a 2s deadline, then independently
observe the cleanup goroutine exit via rc.wg (the same WaitGroup
Shutdown wg.Wait() depends on, bracketing exactly cleanupRoutine).
Mutation check performed and reverted: temporarily removing the
"case <-rc.stopChan: return" branch from cleanupRoutine makes the
test fail with "Shutdown did not return within the deadline" (the
goroutine loops on the ticker forever, wg.Done() never fires).
refresh_coordinator.go is unmodified in this commit (git diff
--stat shows only review_r58_shutdown_test.go); the mutation was
applied and reverted locally to prove the pin is live before this
commit.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(recovery): address review for FIX-18
Fix the close-before-goroutine race the review reopened: delete the
duplicate async registration in startHealthCheckRoutine (only
NewGracefulDegradation registers now), and make the goroutine check
gd.stopChan under gd.mutex before creating or adopting the shared
health-check task, so a closed instance can never (re)start it.
Close() now stops the shared task by name via
GetResourceManager().StopBackgroundTask instead of reading this
instance own gd.healthCheckTask field. With two or more instances,
that field can still be nil on the last-closing instance even though
a sibling already started the shared task, which would have skipped
the stop and leaked the task.
Close every remaining GracefulDegradation NewWithContext can leak: add
a deferred cleanup that closes t.errorRecoveryManager and
t.tokenResilienceManager on any error return after they are
constructed.
Add TestFix18_CloseImmediatelyAfterNewDoesNotReRegister (pins the
exact race; fails deterministically pre-fix, green across
-count=300 -cpu=1,4,16 post-fix), TestFix18_TraefikOidcCloseEmptiesGDInstances
(drives the real TraefikOidc.Close instead of calling the managers
Close directly), and TestFix18_NewWithContextClosesManagersOnEarlyError
(pins the NewWithContext error-path leak). Make the existing
TestFix18_ClosingBothManagersEmptiesGDInstances wait for each
instance async health-check goroutine to settle before closing, via
the new gd.healthRoutineSettled helper, so it exercises the
task-already-assigned path deterministically instead of sometimes
racing the goroutine.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(lifecycle): address review for FIX-35
Close the check-then-stop window the review reopened: isLastInstanceNow
released liveInstanceMu as soon as it returned, before the actual
singleton stop ran (StopBackgroundTask / StopAllTasks, which can block
up to 5s per task). A concurrent New() could register and adopt the
very singleton in that gap, and the stop would kill it anyway.
Add stopIfLastInstance, which holds liveInstanceMu across both the
check and the stop callback, and use it at all three gated stops in
Close() (singleton-token-cleanup, singleton-metadata-refresh-*,
StopAllTasks) in place of the bare isLastInstanceNow() check.
Add TestFix35_StopIfLastInstanceHoldsLockAcrossCheckAndStop, which
proves the lock is genuinely held for the stop full duration (a
concurrent registerLiveInstance cannot complete while a slow stop
callback is still running). Rewrite
TestFix35_ConcurrentNewKeepsSingletonAliveAcrossOldClose to drive the
real (*TraefikOidc).Close via a new closeTestHook (nil in production,
invoked once in Close right after unregisterLiveInstance) instead of
re-implementing Close stop decision inline — the previous version
stayed green even with the production fix fully reverted, since it
never called Close at all.
Both new tests fail to build pre-fix (stopIfLastInstance and
closeTestHook are new symbols); green post-fix across
-count=50 -cpu=1,4,16.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(session): expire chunk cookies before writing replacement chunks (FIX-05)
SetRefreshToken and SetIDToken re-Get and fill the new chunk sessions
before calling the expire helper for the old ones. gorilla Registry.Get
returns the same cached *sessions.Session per cookie name for a request,
so the expire pass re-Gets the objects this call just wrote and wipes
their Values, silently dropping the replacement token.
Reorder both methods to match SetAccessToken: validate chunk count,
sizes and reassembly first (pure computation, no store.Get), then
expire the old chunks, then Get and fill the new ones. A session whose
refresh or ID token was loaded from existing chunk cookies and gets
replaced with another token that also needs chunking no longer loses
the new token.
Adds fix05_session_chunk_replace_test.go, adapted from
.claude/review-2026-09-10/repro/zz_g1_chunkwipe_test.go and
zz_g1_chunkwipe_id_test.go with helpers and test names renamed to
describe the behavior pinned.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(session): gate deferred pool-return on ownership generation (FIX-10)
ServeHTTP defers session.returnToPoolSafely() right after GetSession.
The invalidated-session paths deeper in the same request (backchannel/
frontchannel logout detection) can session.Clear() that exact object
and re-acquire a fresh one before ServeHTTP returns. The deferred call
stays bound to the original pointer regardless, so if a concurrent
request pops the same pooled object before this deferred call fires,
its CompareAndSwap(true, false) still succeeds against the new owner
and Reset()s a session another request is actively using (ABA on the
pool-return CAS).
Add a generation counter to SessionData, bumped once by GetSession and
newSession on every handout. ownerGeneration() reads the current
value; returnToPoolIfOwner(gen) only acts if the generation still
matches what the caller captured at acquire time, so a stale deferred
call from a previous ownership epoch is a deliberate no-op instead of
releasing a live session. ServeHTTP now captures the generation right
after GetSession and defers returnToPoolIfOwner instead of the plain
call.
This is the generation-counter alternative the finding names, chosen
over nil-ing the deferred pointer: the Clear+reacquire happens inside
processAuthorizedRequestRS/processAuthorizedRequest, separate
functions from the ServeHTTP-scope defer, so nil-ing a local variable
there cannot reach the already-bound deferred call.
Adds fix10_session_pool_aba_test.go, adapted from
.claude/review-2026-09-10/repro/zz_g1_aba_test.go with the helper
names and test renamed, and rewritten to drive the ABA sequence
deterministically (the pool handing back the same object after Clear
is unspecified sync.Pool timing, not part of the fixed contract, and
did not reproduce on every run).
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(middleware): require real preflight headers for the OPTIONS auth bypass (FIX-02)
shouldBypassAuth bypassed OIDC auth for any request whose method is
OPTIONS. A genuine CORS preflight always carries both an Origin header
and an Access-Control-Request-Method header; gating on the method
alone let an unauthenticated OPTIONS on any protected path reach the
backend with no session check. Reproduced against http.FileServer:
200 with the protected file body, no cookie, no Origin.
Require both headers before treating an OPTIONS request as a
preflight bypass. A genuine preflight still bypasses unconditionally;
every other OPTIONS request now goes through the normal auth
pipeline.
Adds fix02_options_bypass_test.go covering shouldBypassAuth directly
(bare OPTIONS, Origin-only, and a genuine preflight) plus an
end-to-end ServeHTTP reproduction of the leak and its fix. Updates the
existing TestShouldBypassAuth_PassesOptions in review_r124_test.go
(touched_outside) to send a genuine preflight instead of a bare
OPTIONS, since it asserted the vulnerable bypass-on-method-alone
behavior.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(middleware): drop oversized ID token header instead of truncating (FIX-34)
forwardAuthorized truncated an ID token longer than headerTemplateMaxLen
(8192 bytes) and still set X-Auth-Request-Token to the truncated value.
A truncated JWT is structurally invalid: a downstream that verifies the
signature rejects it (fine), but one that only base64-decodes claims
without verifying reads a corrupted value.
Drop the header instead of truncating, matching the fail-closed
pattern already used for X-Forwarded-User / X-Auth-Request-User in the
same function when identifier sanitization fails.
Adds fix34_idtoken_header_test.go (oversized token drops the header;
in-budget token still forwards unmodified). Updates
review_r155_regression_test.go (touched_outside): its existing test
asserted the header was truncated to exactly headerTemplateMaxLen
bytes, which is the behavior this fix replaces; renamed to
TestR155_XAuthRequestToken_DroppedWhenOverHeaderBudget and rewritten
to assert the header is absent.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(middleware): hand next.ServeHTTP the real writer, not the tracking wrapper (FIX-06)
ServeHTTP wraps rw in *trackingWriter (rw = tw) for panic-commit
tracking and then forwards that SAME wrapped writer to every
t.next.ServeHTTP call: excluded, SSE, WebSocket, options bypass, the
default case, and forwardAuthorized (the authenticated path). Traefik
runs the plugin interpreted under yaegi v0.16.1, whose stdlib interop
layer composes a boundary wrapper only for ResponseWriter+Hijacker
crossing into compiled code, never for Flusher or the Unwrap pattern
http.ResponseController relies on. So Traefik's compiled ReverseProxy
calling http.NewResponseController(rw).Flush() for SSE gets "feature
not supported" in production, even though trackingWriter declares
Flush/Unwrap and native tests (which never cross an interpreter
boundary) pass.
Reproduced with a throwaway yaegi run probe (not committed; yaegi
v0.16.1, GOPATH/bin/yaegi) mirroring trackingWriter exactly:
wrapped: ResponseController.Flush err=feature not supported, recorderFlushed=false
unwrapped: ResponseController.Flush err=<nil>, recorderFlushed=true
matching the finding's own reproduction.
Add downstreamWriter(rw), used at every t.next.ServeHTTP call site: it
unwraps *trackingWriter to the real writer ServeHTTP received, and
marks the trackingWriter's new calledNext field so the deferred
recover (which may run in a different function than the call site, via
forwardAuthorized) knows control passed downstream.
Handing next the real writer means wroteHeader can no longer observe
writes next makes directly (it bypasses tw entirely). The deferred
panic recovery is updated to skip its fallback 500 once calledNext is
true: guessing in that state risks appending "Internal Server Error"
to a response next already committed, which TestR155_ServeHTTP_
PanicAfterCommitKeepsBodyIntact exists to prevent and is the more
severe failure mode. A panic in the middleware's own pre-forward code
is unaffected and still gets a clean 500
(TestR179_PanicRecoverySetsNoStore). Updates review_r74_test.go
(touched_outside): its existing assertion that a downstream (next)
panic forces a 500 is no longer achievable without that same
corruption risk; rewritten to assert the recovered-but-unwritten
default (200) Go's http.Server already gives any handler that returns
without writing.
Adds fix06_downstream_writer_test.go: asserts next receives the
unwrapped writer on the SSE, WebSocket and authenticated
(forwardAuthorized) paths.
Claude-Session: https://claude.ai/code/session_01KwtV5ecajCrftcf8TkfPad
* fix(middleware): make FIX-06 actually load and behave correctly under yaegi (FIX-06)
trackingWriter now declares Header() explicitly instead of relying on
the embedded http.ResponseWriter promotion. yaegi v0.16.1 does not
count a promoted method when it statically checks whether
*trackingWriter satisfies http.ResponseWriter, so the plugin refused
to load under yaegi at all ("missing Header method"), invisible to
every native test. Verified with a temp GOPATH symlinked to this
worktree (the shared GOPATH symlink resolves to the main checkout, not
this branch) via `yaegi run ./cmd/yaegicheck/main.go`: FAIL before
this change, OK after.
ServeHTTP's panic-recovery defer now sends a header-only 500
(Cache-Control: no-store) whenever nothing was written through the
tracking wrapper, even after control has passed to next
(tw.calledNext). WriteHeader is idempotent, so this is a no-op if next
already committed its own response; only the fallback body Write is
still gated on !calledNext, since only a body Write can corrupt an
already-committed response. Previously the whole 500 was skipped once
calledNext was true, so a downstream panic before any write produced
an empty 200 instead of a 500, telling the client a failed request
succeeded. review_r74_test.go is restored to asserting 500 (its
|
||
|
Alex SedovandGitHub
|
d3012d5169 |
feat: add RFC 8707 resource indicator via resource config (#161)
* feat: add RFC 8707 resource indicator via resource config `Config.Resource` is an alternative to `audience` for IdPs that implement RFC 8707. `buildAuthURL` sends it as the `resource` parameter on the authorization request, before `extraAuthParams` so an operator-configured `resource` key there cannot override or duplicate it. `exchangeTokens` repeats it on both the `authorization_code` and `refresh_token` grants (RFC 8707 §2.2), so a refreshed token keeps the audience binding of the original grant. Only one resource is accepted. RFC 8707 permits a list, but no provider that supports the parameter handles a list usefully: one rejects any request naming more than one resource with HTTP 400 `invalid_target`; node-oidc-provider accepts several and issues an opaque token bound to none of them. `Config.Validate()` requires an absolute URI of at most 256 characters with no fragment, wildcard, or control characters. `http`/`https` values go through `isValidSecureURL`, so HTTPS is required outside loopback; other schemes (`urn:`) pass as-is. When `resource` is set and `audience` is not, `audience` defaults to the resource for access-token validation; `strictAudienceValidation` applies the same way whichever of the two is set. That default is validation-only. A new `TraefikOidc.explicitAudience` mirrors `Config.Audience` verbatim, and `buildAuthURL` keys the non-standard `audience` request parameter (the Auth0 custom-API pattern) on it rather than on `audience`, so a resource-derived default is never sent as an unsolicited `audience` parameter alongside `resource` — a combination that misbehaves against providers such as Ory Hydra, which give `audience` a different meaning. `audience_test.go` and `url_helpers_ultra_test.go` set `audience` on the struct directly, bypassing `New()`, and now set `explicitAudience` too. docs/CONFIGURATION.md carries only the reference-table row and a short pointer; the new docs/RFC8707.md holds the request behaviour, the single-resource rationale, the `resource`/`audience` relationship, and a provider-support table from interop testing: node-oidc-provider supports it; Zitadel accepts and silently ignores it (`oidc.AuthRequest` has no `resource` field, confirmed against a live instance); Authentik returns `invalid_target`; Keycloak has no support through 26.6; Ory Hydra/Fosite uses its own `audience` parameter; Azure AD/Entra ID v2 is reported to reject `resource` combined with v2-style `scope`; Auth0 and AWS Cognito claims are unverified. README.md, docs/index.html, and .traefik.yml each point at docs/RFC8707.md instead of repeating it, and the comments on `Config.Resource`, in main.go, and in url_helpers.go defer to it as well. rfc8707_resource_test.go covers the authorization-URL parameter (set, unset, query-bearing value percent-encoded rather than merged, `extraAuthParams` unable to override), both grants sending `resource` in the form body, the `Config.Validate()` rules table, audience defaulting through `New()`, and a regression pair: a resource-derived audience default is not sent as `audience`, while an explicitly configured one still is. * fix: accept Resource in place of Audience for bearer-auth validation `NewWithContext` rejected `EnableBearerAuth=true` whenever `config.Audience` was empty, even though `t.audience` already falls back to `config.Resource` in that case. The check now fails only when both `Audience` and `Resource` are empty, and the error names both fields. `TestStartupValidation_BearerAcceptsResourceInPlaceOfAudience` covers the `Resource`-only configuration. `docs/RFC8707.md` links the `audience` parameter note to `docs/AUTH0_AUDIENCE_GUIDE.md` instead of restating it, and drops the provider table's GitHub row and the interop-testing paragraph, which are covered elsewhere. |
||
|
Martin FreyandGitHub
|
61e60733a5 | fix: workaround for yaegi type checking using reflection (#159) | ||
|
Martin FreyandGitHub
|
7e44725edc | fix: avoid nil dereference in Close (#157) | ||
|
|
15dba0e8d3 |
feat: add cookiePath config to scope session cookies to subpath (#143)
Fixes #122. Co-authored-by: Hermes Agent <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> |
||
|
lukaszraczylo
|
f728f61777 |
fix(config): wire introspectionURL override and repair dead endpoint overrides (#152)
Issue #152 asks for a manual introspection endpoint because some IdPs
omit introspection_endpoint from discovery. Analysis found the two
documented overrides revocationURL and oidcEndSessionURL were dead
code: the
|
||
|
lukaszraczylo
|
78b6f193b3 |
fix(security): allow loopback hosts for loopback providerURL (#154)
Issue #154: a providerURL that points at a localhost IdP caused an infinite redirect loop. validateHost rejected loopback hosts unconditionally, so buildAuthURL returned an empty string. Derive allowLoopbackHosts from a loopback providerURL at construction time. Discovery validation already used this derivation, so the two validation layers now agree. This adds no new config surface. Cloud metadata hosts (169.254.169.254, metadata.google.internal), link-local, and unspecified addresses stay blocked unconditionally. Non-loopback providers keep identical behaviour. Add issue154_regression_test.go: 8 tests pin the new allowance, the default block, and the security envelope. |
||
|
|
edb7174cc8 |
docs: pin documented plugin version to v1.0.29 (#153)
The install snippet in the README still pinned v0.7.10 and the example configs v0.8.0, so anyone following the docs installed a release far behind the current v1.0.29. Co-authored-by: Markus Renschler <[email protected]> |
||
|
lukaszraczylo
|
e51bd9cdbe |
fix(catalog): analyzer-valid testData and explicit New() error return
The plugin catalog analyzer failed v1.0.28 with "invalid handler type: <nil>" (#151). Two defects combined. The analyzer feeds .traefik.yml testData to New() verbatim. The header values used quadruple braces on a false premise: no configuration path collapses them. Template validation rejected them, so New() failed. The testData now carries plain, valid templates. New() also returned failures through a multi-value tail call (return NewWithContext(...)). Yaegi v0.16.1 zeroes both results of that statement on the error path. The analyzer therefore saw (nil, nil) and reported the opaque handler-type message instead of the real validation error. New() now splits the return explicitly. Construction failures surface as real error messages under yaegi and in Traefik logs. The regression tests pin both fixes. README and CONFIGURATION.md now give correct escaping guidance: plain double braces everywhere; file provider users escape with a raw string. Quadruple braces do not work anywhere. Fixes #151. |
||
|
lukaszraczyloandGitHub
|
f320d0a316 |
fix(settings): repair #149 + AST-based header template validation + allowedClaims (#150)
* fix(settings): allow documented range + static header templates (#149)
Upgrading past the release where NewWithContext began calling
config.Validate() (
|
||
|
lukaszraczylo
|
48a19e04f8 |
fix(settings): tolerate whitespace in header template delimiters (#148)
validateTemplateSecure matched safe functions with literal strings.Contains
("{{get "), so a space after {{ ("{{ get") failed the whitelist and
Config.Validate() rejected the config, surfacing as Traefik's opaque
"invalid handler type: <nil>". Go's text/template treats {{ get and {{get
identically, so the validator was stricter than the engine.
Normalize inner-delimiter whitespace before matching. This also closes a
security gap where a leading space let dangerous patterns ("{{ range",
"{{ call") evade detection.
Adds unit + regression coverage.
|
||
|
lukaszraczyloandGitHub
|
546ceb949c |
security: remediate audit findings (ranks 1–16 + 22 Lows) + yaegi load validation (#144)
* fix(security): encrypt session cookies + fail closed on invalid config
Batch 1 of security audit remediation (ranks 1, 2, 6).
- session.go: derive independent HMAC + AES-256 keys via stdlib HKDF-SHA256
and build the gorilla cookie store with both, so session cookies are now
encrypted, not merely signed. The single-key store previously left OIDC
access/refresh/ID tokens recoverable from raw cookie bytes. Cookie format
changes, so existing sessions are invalidated on deploy (one-time re-login).
- main.go: call config.Validate() at construction and error out on failure,
instead of silently substituting a public hardcoded encryption key for
empty/short keys (which allowed session forgery). The yaegi analyzer
passes via .traefik.yml testData.
- settings.go: isValidSecureURL permits plaintext HTTP for loopback hosts
only (RFC 8252); remote providers must still use HTTPS.
- tests: complete configs that did not satisfy Validate(); add regression
tests in security_audit_fixes_test.go.
Configs below documented minimums (rateLimit < 10, key < 32 chars) are now
rejected at startup (fail closed).
* fix(security): validate discovered OIDC endpoints + pin introspection host
Batch 2 of security audit remediation (ranks 3, 4).
- url_helpers.go: add validateDiscoveredEndpoint, an SSRF screen for endpoints
taken from the provider discovery document (jwks_uri, token, authorization,
revocation, end_session, introspection, registration). Blocks link-local
(cloud metadata 169.254.169.254), multicast, unspecified and private
addresses (unless allowPrivateIPAddresses); blocks loopback unless the
configured providerURL is itself loopback (dev/test). Cross-domain JWKS
hosts (e.g. Google) stay allowed. Add sameHost helper.
- main.go: updateMetadataEndpoints screens every discovered endpoint and
blanks any that fail (fail closed downstream). The introspection endpoint
carries the client secret via HTTP Basic, so it is additionally pinned to
the providerURL host to stop a poisoned discovery document exfiltrating the
secret to an attacker-controlled host.
- tests: regression tests for the SSRF guard and the host pin.
* fix(security): close open redirects + anchor excluded-URL matching
Batch 3 of security audit remediation (ranks 5, 14, 15).
- auth_flow.go: run the stored incoming path through normalizeLogoutPath
before using it as the post-login redirect, so //evil.com and /\evil.com
payloads become host-relative (open-redirect, rank 5).
- url_helpers.go: excluded-URL matching is anchored at a natural boundary
(exact, sub-path "/", or file extension "."), so excluding "/public" no
longer also bypasses auth on "/publicsecret"; "/favicon" still matches
"/favicon.ico" (rank 14).
- internal/utils: X-Forwarded-Host is sanitized (first value only; reject
CRLF/whitespace/multi-value) before building redirect URLs (rank 15).
- helpers.go: the logout redirect used when there is no provider end-session
endpoint is host-relative, never an absolute URL derived from the
client-controllable request host (logout open-redirect, rank 15).
- tests: update two logout cases that asserted the old absolute redirect;
add regression tests.
* fix(security): reject unverified Azure tokens; fix transport TLS reuse
Batch 4 of security audit remediation (ranks 7, 11).
- token_validation_rs.go: an Azure nonce-bearing access token that cannot be
cryptographically verified no longer returns "authenticated" when there is
no ID token to corroborate it; it refreshes (if possible) or forces
re-authentication instead of failing open (rank 7).
- http_client_pool.go: the at-limit transport-reuse path now takes the write
lock before mutating refCount (fixes a data race) and only reuses a
transport whose TLS settings (CA pool + InsecureSkipVerify) match the
caller's, never one with a different trust store; if none matches it returns
nil so the caller falls back to a verifying default transport (rank 11).
- tests: add a transport-pool TLS-isolation regression test.
* fix(security): stop logging templated header values (token leak)
Batch 5 of security audit remediation (rank 16).
middleware.go: templated downstream headers commonly carry the access token
(e.g. "Authorization: Bearer {{.AccessToken}}"). The debug log line printed
the full header value, leaking credentials into logs. Log the header name and
byte length instead.
* fix(security): cache-key collision, cache-config divergence, fleet cleanup
Batch 6 of security audit remediation (ranks 9, 10, 12).
- token_manager.go: detectTokenType keys its cache on a SHA-256 hash of the
full token instead of the first 32 chars (which are only the base64url JWT
header). Distinct tokens sharing alg+kid no longer collide and get
mis-classified (rank 10).
- cache_manager.go: the process-global cache manager is initialized once and
shared across plugin instances; it now logs a loud warning when a later
instance requests a different explicit Redis backend that is silently
ignored, surfacing the cross-instance state-isolation hazard (rank 9).
- singleton_resources.go / main.go / utilities.go: track a process-global live
instance count; the shared singleton-token-cleanup task is stopped only when
the LAST instance shuts down, so one instance's Close() (e.g. a config reload)
no longer kills cleanup for surviving instances (rank 12).
- tests: update TestDetectTokenTypeCaching for the new key; add regression tests.
* fix(security): bound introspection cache + cookie lifetime to config
Batch 7 of security audit remediation (ranks 8, 13).
- token_introspection.go: when requireTokenIntrospection is enabled, cap the
positive introspection-result cache at 30s (instead of 5m) so a token
revoked at the provider stops passing within ~30s, matching the operator's
near-real-time revocation expectation (rank 8).
- session.go: bind the cookie store's MaxAge to the configured sessionMaxAge,
so the cookie codec's cryptographic timestamp validity is no longer fixed at
gorilla's 30-day default; a stolen cookie is valid only for the configured
session lifetime (rank 13).
- tests: add a cookie-lifetime regression test.
* fix(security): low-severity hardening (cache, DoS caps, PKCE, throttle)
Batch 8 of security audit remediation — low severity
(ranks 24, 25, 27, 29, 31, 36, 37, 41, 45, 46, 49).
- universal_cache.go: updateLocalCache updates an existing key in place instead
of orphaning its LRU element and double-counting currentSize/currentMemory
(rank 36 — the only production-reachable bug in this batch).
- jwk.go / metadata_cache.go / token_introspection.go: bound response bodies
with io.LimitReader (1 MiB) to prevent memory exhaustion from a hostile or
buggy provider (ranks 24, 25).
- jwk.go: skip JWKs not usable for signature verification (use != sig, or
key_ops without "verify") when building the key set (rank 49).
- auth_flow.go: fail closed at the callback when PKCE is enabled but the code
verifier is missing, instead of silently dropping it (rank 27).
- utilities.go / main.go: match allowedUserDomains case-insensitively (rank 31).
- bearer_auth.go: a single success no longer wipes an active per-IP penalty;
the counter resets only when no penalty is in effect (rank 29).
- main.go: handle (not discard) the NewSessionManager error (rank 37).
- error_recovery.go: take a write lock in isServiceDegraded (it deletes from a
map); compare retryable-error substrings case-insensitively (ranks 45, 46).
- singleton_resources.go: bind the generic-cache cleanup goroutine to the
resource-manager shutdown channel so it cannot outlive its owner (rank 41).
- tests: update the bearer throttle test to the corrected penalty semantics.
* fix(security): header sanitization, issuer pinning, fail-closed paths
Batch 9 of security audit remediation (ranks 18, 19, 20, 21, 22, 30, 33, 34).
- middleware.go / bearer_auth.go: sanitize claim-derived values on the cookie
auth path before injecting them into downstream headers. Drop group/role and
identifier values containing control chars, bidi-override runes, or the
, ; = delimiters (a comma would inject phantom entries into X-User-Groups);
reject control/bidi/over-length in rendered templated header output (but
permit , ; = in free-form values such as a bearer token). The bearer path
already sanitized; the cookie path did not (ranks 33, 34).
- main.go / metadata_cache.go: pin the discovered issuer to the configured
provider host (sameHost) and refuse/never-cache a mismatch, so a poisoned
discovery document cannot redefine the JWT trust anchor (ranks 21, 22).
- token_introspection.go: when a distinct API audience is configured, fail
closed on a missing or mismatched introspection audience; aud parsed as
string-or-array per RFC 7662 (rank 19).
- logout.go: front-channel logout requires a matching issuer; an empty iss is
rejected (blocks unauthenticated forced-logout via a known sid) (rank 30).
- token_validation_rs.go: an opaque access token with no ID token and no
successful introspection fails closed (re-auth) instead of authenticating
(ranks 18, 20).
- tests: realistic same-host provider mocks; regression tests for the header
sanitization distinction and the fail-closed paths.
* chore(security): remove unwired dead code with latent footguns
Batch 10 of security audit remediation — delete confirmed-dead, unwired
subsystems (ranks 26, 35, 50). None had a production caller (grep-verified);
removal eliminates the latent footguns and ~2.1k lines of dead code.
- token_validator.go (deleted): an unused *TokenValidator whose validateJWT set
Valid=true with NO signature verification — a severe footgun if ever wired
(rank 50). The wired RS-aware validators are unaffected.
- security_monitoring.go (deleted): an unused *SecurityMonitor / ExtractClientIP
that trusted spoofable X-Forwarded-For / X-Real-IP. The live bearer throttle
uses clientIPForBearer (RemoteAddr-only), unchanged (rank 35).
- dynamic_client_registration.go: removed the RFC 7592 management methods
(Update/Read/DeleteClientRegistration) that dereferenced an attacker-
influenced RegistrationClientURI with the registration token attached and no
HTTPS/SSRF gate, and had no callers. The wired RFC 7591 RegisterClient and
credential-store helpers are kept (rank 26).
- tests: removed the tests covering the deleted code.
* chore: add Makefile with yaegi load validation
No Makefile existed. The new `yaegi-validate` target interprets the plugin
under the yaegi interpreter the same way Traefik loads it, catching yaegi-only
incompatibilities (unsupported stdlib symbols, reflection edge cases) that the
native `go build` / `go test` toolchain does not. Importing the plugin forces
yaegi to interpret every file plus its vendored deps; CreateConfig + New
exercise the instantiation path.
- cmd/yaegicheck/main.go: the load driver, marked //go:build ignore so it is
excluded from `go build ./...` (avoids VCS-stamping a main binary, which
fails in git-worktree layouts) yet is run explicitly by yaegi.
- Makefile: build / fmt / vet / lint / test / vendor / yaegi-validate / check
targets; `make check` runs vet + tests + yaegi-validate.
Verified: `make yaegi-validate` passes on this branch — the HKDF cookie
encryption, net-based endpoint validation, and claim sanitizers all interpret
and instantiate cleanly under yaegi.
* ci: bump workflow Go toolchain to 1.25; pin yaegi-validate to v0.16.1
Traefik v3.7.1 (the deployed version) is built with `go 1.25.0`, so the PR and
release workflows now use Go 1.25.x to match the toolchain Traefik uses.
Important distinction: the CI Go version is the build TOOLCHAIN. The plugin's
actual interpreter-compatibility ceiling is the yaegi version Traefik bundles
(v0.16.1, which declares go 1.21 and ships a ~Go 1.22 stdlib symbol surface),
NOT the CI Go version. That ceiling is enforced by `make yaegi-validate` plus
the go.mod language directive — e.g. it is why HKDF is hand-rolled with
hmac+sha256 rather than Go 1.24's crypto/hkdf, which yaegi v0.16.1 lacks.
Also pin Makefile YAEGI_VERSION to v0.16.1 (what Traefik v3.7.1 vendors) so
yaegi-validate exercises the real deployed interpreter instead of @latest,
which could pass on a newer yaegi that supports symbols the deployed one does
not.
* docs: align README/CONFIGURATION with branch behavior changes
- excludedURLs: documented as segment/extension-boundary matching (was
"prefix-matched") — "/public" no longer also matches "/publicsecret" (rank 14).
- Front-channel logout now requires a matching `iss`; requests without one are
rejected with 400 (rank 30).
- Add an "Upgrading from an earlier release" note: session cookies are now
AES-256 encrypted with lifetime tracking sessionMaxAge (one-time re-login on
upgrade), and invalid configuration (rateLimit < 10, key < 32 bytes, missing
callbackURL, non-HTTPS remote providerURL) now fails closed at startup.
* fix: remove staticcheck-flagged unused functions; wire staticcheck into make check
CI Static Analysis (standalone staticcheck) failed with U1000 "unused":
- dynamic_client_registration.go: deleteCredentialsFromStore — its only caller
was the RFC 7592 DeleteClientRegistration removed in the dead-code batch.
- token_test.go: createTestJWTSimple — its only callers were the TokenValidator
tests removed in the same batch.
Both confirmed to have zero remaining callers and removed. build / vet /
go test ./... / staticcheck ./... all green.
The pre-commit hook runs golangci-lint, but CI runs standalone staticcheck
(which flags U1000). Add a `staticcheck` Makefile target and include it in
`make check` so this class of finding is caught locally before push.
* fix(test): stabilize flaky TestWorkerPool_TaskPanic
tasksFailed is incremented in the worker's deferred recover(), which runs after the panicking task's own defer wg.Done(). wg.Wait() could therefore return before the failure was recorded, so reading the counter immediately raced and flaked on slow CI runners. Poll until the failure lands (2s budget) instead. Verified 200x plain + 50x under -race/GOMAXPROCS=1.
|
||
|
lukaszraczylo
|
f75b2f20e0 |
fix: resolve cache eviction lock-up and migrate telemetry [patch-release]
universal_cache: stop the write-lock convoy / 100%-CPU spin (observed via pprof: one ServeHTTP goroutine holding c.mu.Lock for hours while 119 requests queued). The per-request populate path (updateLocalCache) PushFronted a duplicate LRU node + overwrote items[key] without removing the prior node; once eviction deleted the key, orphan nodes at Back() were never removable and the eviction loop spun forever under the write lock. Replace the entry in place (mirroring setLocal) and harden evictOldest with a forward-progress guard. Adds universal_cache_orphan_test.go. telemetry: delete the hand-rolled client; call oss-telemetry v0.2.3 (vendored, Yaegi-safe) directly from New(), once per process via sync.Once. version: add version.go + workflow-prepare.sh so the release semver is stamped into source at build time (the value cannot be resolved at runtime under Yaegi). dev/source builds keep the 0.0.0-dev sentinel and emit no telemetry. |
||
|
paiking1andGitHub
|
cf6ed1da55 |
feat: feat: add extraAuthParams (extra authorization request parameters) (#139)
Adds optional extraAuthParams map[string]string config. Extra params are appended to the authorization request but can never override plugin-managed params (client_id, state, nonce, etc.).v1.0.23 |
||
|
lukaszraczylo
|
f821b8829b |
fix: remove write-lock convoy in getLocal + fix mutateState CAS bug
UniversalCache.getLocal(): when a cached token expires, the RLock fast path (line 385-398) previously fell through to c.mu.Lock() (write lock). Under Yaegi, the write-lock holder takes 10-100ms for LRU manipulation, and Go's RWMutex writer-priority blocks ALL new RLock callers. A single expired-token event turned every concurrent request from read-parallel into write-serialized — the convoy that produced the 737-goroutine pileup at 0x400275a608 (pprof captured at /tmp/traefik-spike-1779663149). Fix: return (nil, false) immediately on expiry for Token/JWK/Session cache types. The periodic cleanup goroutine handles eviction. Write lock is never taken on the read path for these cache types. refreshAttemptTracker.mutateState(): the CAS loop used t.state.CompareAndSwap(t.state.Load(), next) — a second Load that can see a different value from a concurrent writer, silently overwriting their update. Fixed to CompareAndSwap(cur, next) using the snapshot we computed the mutation from.v1.0.22 |
||
|
lukaszraczylo
|
5f9c574f95 |
refactor: delete dead non-RS validators; tests use RS variants
After v1.0.20 the non-RS validation chain had no production callers —
middleware.ServeHTTP dispatched exclusively through isUserAuthenticatedRS.
The orphaned functions stayed reachable only from a handful of test
files and risked silent logic drift against their RS counterparts.
Deleted from production code (~440 LOC):
- auth_flow.go: isUserAuthenticated
- token_manager.go: validateAzureTokens
- token_manager.go: validateGoogleTokens
- token_manager.go: validateStandardTokens
- token_manager.go: validateTokenExpiry
- removed now-unused encoding/base64 and encoding/json imports
from token_manager.go (only the deleted validateStandardTokens
needed them; the RS variant in token_validation_rs.go keeps its
own imports).
Added (3 LOC):
- token_validation_rs.go: validateGoogleTokensRS (trivial delegator,
parity with the deleted non-RS variant so isUserAuthenticatedRS
can dispatch cleanly).
Tests ported (10 call sites across 3 files):
- audience_test.go: ts.tOidc.validateStandardTokens
- azure_oidc_test.go: tOidc.validateAzureTokens,
ts.tOidc.validateGoogleTokens,
ts.tOidc.validateAzureTokens,
ts.tOidc.isUserAuthenticated
- issue134_followup_graph_test.go: oidc.validateAzureTokens (4x)
Each ported site now constructs a *requestState from its existing
*SessionData via (&requestState{}).captureSession(session) and calls
the *RS variant. Same data, different read source.
Net diff: -440 LOC production, ~+25 LOC tests, +3 LOC stub.
Production now has a single source of truth for token validation;
no parallel implementations to keep in sync.
All tests pass with -race; golangci-lint clean.
v1.0.21
|
||
|
lukaszraczylo
|
7c6f09fb20 |
feat(middleware): RS-aware token validators (kill ~21 RLocks/request)
Adds token_validation_rs.go with requestState-aware variants of the
token validation path:
isUserAuthenticatedRS(rs) -> dispatches by provider
validateStandardTokensRS(rs) -> standard path (eliminates 17 RLocks)
validateAzureTokensRS(rs) -> Azure path (eliminates 10 RLocks)
validateGoogleTokensRS(rs) -> delegates to standard
validateTokenExpiryRS(rs, tok) -> shared expiry check (eliminates 4 RLocks)
middleware.ServeHTTP now calls isUserAuthenticatedRS(rs) on the hot
path. The pre-v1.0.20 non-RS variants are kept untouched for tests
and any future caller that doesn't have a captured snapshot.
Why
---
The standard validation path read SessionData via session.GetX() 17
times, with GetRefreshToken alone called 11 times (every "return
'needs refresh'" branch re-reads it). Each call acquires
sd.sessionMutex.RLock(). Under Yaegi each RLock costs ~1-5ms of
interpreter dispatch. The captured snapshot already lives on rs, so
the RS variants substitute direct struct field reads.
Per-request cost on the hot authenticated path
----------------------------------------------
ServeHTTP enters:
+ 1 RLock to populate rs (was 0)
Validation path:
Standard: was 17 RLocks, now 0
Azure: was 10 RLocks, now 0
processAuthorizedRequestRS:
was 4-6 GetX calls, now 0 (already in v1.0.19)
Net: ~22-27 fewer Yaegi-dispatched RLock acquisitions per authenticated
request on the hot path.
Caveats
-------
* Refresh / expired / callback paths still use the non-RS validators
because they can mutate session state between validation and use.
* The RS variants are by-design line-for-line equivalents of the
originals. If logic in the originals changes, the RS variants need
matching updates. This is acceptable for now; a future refactor
could collapse them once the non-RS callers are gone.
All tests pass with -race; golangci-lint clean.
v1.0.20
|
||
|
lukaszraczylo
|
68e1c4319c |
feat(middleware): per-request context object (requestState)
Adds requeststate.go and threads a *requestState through the ServeHTTP -> processAuthorizedRequestRS -> forwardAuthorized path. rs is allocated once at the top of ServeHTTP, populates SessionData field snapshots under a SINGLE sd.sessionMutex.RLock, and caches the MetadataSnapshot. Downstream handlers read the cached fields instead of calling session.GetX() / t.metadataSnap() repeatedly. Why --- Under Yaegi each method dispatch (including RWMutex.RLock) costs ~1-5ms of interpreter overhead. SessionData getters each take an RLock on sd.sessionMutex; the previous hot path called 5-7 of them per request (GetAuthenticated, GetAccessToken, GetIDToken, GetRefreshToken, GetUserIdentifier, plus the same set again inside processAuthorizedRequest). With one batched RLock + cached fields, that drops to a single RLock for the whole handler chain. This is scoped — not a wholesale architectural refactor: * requestState is per-request (alloc at ServeHTTP entry, dropped on return). It is NOT a shared cache and never escapes the request. * The original processAuthorizedRequest is kept unchanged for any callers we don't migrate this round (bearer path, callback handlers, expired-token handlers). New code path is the RS-aware processAuthorizedRequestRS, which middleware.ServeHTTP now uses for the happy authenticated-and-not-needing-refresh case. * Cross-request caches (tokenCache, JWKCache, sessionEntries, sessionInvalidationCache) are unchanged. rs is additive, not a replacement. What this does NOT change ------------------------- * The refresh path still calls session.GetX() in middleware.go (handleExpiredToken, refreshToken, defaultInitiateAuthentication) because those flows can mutate session state and a stale rs would be wrong. * validateStandardTokens still has its own session.GetX() calls. Deep plumbing into the token-verification path is a follow-up. * No semantic changes to authentication, refresh, or session lifecycle — only the read path is optimised. All tests pass with -race; golangci-lint clean.v1.0.19 |
||
|
lukaszraczylo
|
17e3f8ef62 |
fix: snapshot patterns for refresh-tracker and metadata URLs
Two related lock-free snapshot refactors addressing the remaining
post-v1.0.16 code-review findings.
1. refreshAttemptTracker: per-field atomic.Load/Store -> atomic.Value
snapshot of *attemptState (refresh_coordinator.go).
Previously each tracker held five independently-atomic fields. The
cooldown-exit reset wrote cooldownEndNano = 0 first, then separately
stored attempts = 1 and windowStartNano = now. A concurrent
isInCooldown call could observe cooldownEndNano = 0 (reset just
completed) with attempts still at MaxRefreshAttempts, immediately
triggering a fresh cooldown — a benign double-trigger race that
nonetheless meant the state machine had observable intermediate
states.
New design: state is a *attemptState (immutable) published via
atomic.Value. All transitions (record/success/failure/window-reset/
cooldown-enter/cooldown-exit) go through mutateState, which runs a
CAS loop: load current snapshot -> construct fresh snapshot ->
CompareAndSwap. Either the entire new state publishes or none of
it does — no intermediate visibility, no cross-field race.
Under Yaegi this collapses 3-5 per-field atomic dispatches into one
atomic.Value.Load on the read path. Write paths pay an extra
allocation for the new snapshot but avoid the cross-field hazard.
2. MetadataSnapshot: hot-path readers use atomic.Value instead of
metadataMu.RLock (middleware.go, types.go, main.go, utilities.go).
middleware.ServeHTTP previously took metadataMu.RLock on every
non-bypass request to read the single field issuerURL. Under Yaegi
each RLock acquisition costs 1-5ms of interpreter dispatch.
updateMetadataEndpoints now also publishes an immutable
*MetadataSnapshot via atomic.Value; the hot-path reader loads it
in one op via t.metadataSnap(). Falls back to the legacy
metadataMu.RLock pattern when the snapshot is unpublished (some
test setups initialize the struct fields directly without going
through updateMetadataEndpoints).
Less-frequent callers (helpers, logout, token_introspection) still
take metadataMu.RLock and are unchanged. The snapshot strictly
subsets the metadataMu-protected fields, so those readers see
identical data.
Note on atomic.Pointer[T]: this would have been the cleaner type but
yaegi v0.16.1's stdlib (used by traefik:v3.7.1) exposes only the
legacy unsafe.Pointer-based atomic primitives — no generic Pointer[T].
atomic.Value provides the same semantics via interface{} + type assert.
All tests pass with -race; golangci-lint clean.
v1.0.18
|
||
|
lukaszraczylo
|
827926bc3a |
fix(refresh-coordinator): trim per-request mutex/map ops
Three related changes addressing post-v1.0.15 code-review findings and the user's observation that we have been "throwing maps around" — under Yaegi every sync.Map / atomic / mutex dispatch costs ~1-5ms of interpreter overhead, so the number of dispatches per request matters as much as whether they are lock-free. 1. Remove cleanupTimers map + cleanupTimerMu sync.Mutex. scheduleDelayedCleanup previously tracked every pending timer in a map guarded by a mutex so a duplicate scheduling could cancel the prior timer. That "shouldn't happen" path was the only consumer of the map, but the mutex fired on every successful refresh completion — another per-request Yaegi-dispatched lock. performCleanup is already idempotent (LoadAndDelete on the sync.Map), so a duplicate firing is at worst a no-op second call. Dropped the map entirely; time.AfterFunc callback now calls performCleanup directly. Net: -1 sync.Mutex, -1 map field, -2 Lock/Unlock pairs per refresh completion. Shutdown simplified — no need to enumerate-and-stop timers since the callbacks no longer need teardown. 2. Reorder applyLeaderGates: cooldown check BEFORE recordRefreshAttempt. Previously incremented the attempt counter and then checked cooldown. Under burst load (many concurrent leaders with different token hashes but the same session) every goroutine could increment past MaxRefreshAttempts before any one of them observed the threshold, so the gate fired too late — same thundering-herd shape that drove v1.0.14 into the ground. Reordering makes the gate authoritative: only attempts that pass the gate are recorded. Semantic change: with MaxRefreshAttempts=N, exactly N attempts now run to completion before the (N+1)th is denied. Previously the Nth was denied as it tried to record (off-by-one stricter). Test assertion updated to N (was N-1). 3. Fix getOrCreateOperation MaxConcurrentRefreshes overshoot. The previous CAS-loop allowed a transient overshoot of up to N-1 leaders when several goroutines all observed `current < max` in the same scheduling slice before any one of them succeeded their CAS — visible to readers as currentInFlightRefreshes > MaxConcurrentRefreshes for a brief window. Replaced with the ticket-and-return pattern: increment optimistically, decrement if we overshot. Strictly bounded: only the goroutine that produces max+1 sees max+1 as committed; the rest decrement back immediately. No CAS retry loop needed. What was NOT done in this commit, and why: * metadataMu.RLock cached via atomic snapshot — code-reviewer flagged this at severity 7 (3 RLocks per request: middleware.go:213, token_manager.go:349, token_manager.go:408). The clean fix is an atomic.Pointer[*MetadataSnapshot], but generic atomic.Pointer[T] is NOT exposed by yaegi v0.16.1's stdlib (only legacy unsafe.Pointer primitives). atomic.Value would work but requires a snapshot-struct refactor across ~15 call sites (helpers/logout/token_introspection/ token_manager/main/middleware). Deferred to a focused future PR. * isInCooldown multi-field reset race — the cooldown-reset CAS wins on cooldownEndNano, then separately stores attempts/consecutiveFailures/ windowStartNano. A concurrent isInCooldown can briefly see the pre-reset attempts value and trigger a fresh cooldown. Semantic glitch (double-cooldown), not a correctness disaster. Fix is a single atomic pointer swap of an immutable snapshot — same atomic.Pointer constraint as above. Deferred. All tests pass with -race; golangci-lint clean.v1.0.17 |
||
|
lukaszraczylo
|
abbfdb02a7 |
fix(jwk): replace JWKCache.mutex with singleflight pattern
JWKCache.GetJWKS previously held a sync.RWMutex.Lock() across the entire HTTP round-trip to the IdP's JWKS endpoint (jwk.go:93). On a cold cache (cold pod, JWK rotation, transient network blip) every concurrent request piled up on this single global write-lock. Under Yaegi each Lock() acquisition costs 10-50ms of interpreter dispatch — same architectural shape as the bugs v1.0.14 and v1.0.15 already fixed, just one that hadn't surfaced as the dominant bottleneck yet. Code-review post-spike #2 flagged this at confidence 9/10 as the next likely death-spiral on pod cold-start. Change replaces the lock with a sync.Map-based singleflight: the first caller for a given JWKS URL performs the fetch; concurrent callers attach to the same *jwksFetch and wait on its done channel for the result. Cold-cache cost is now O(1) HTTP fetch regardless of how many goroutines are waiting, and no Yaegi-dispatched lock is held during the fetch itself. Correctness: - LoadOrStore winner does the work; losers wait on a done channel. - Done channel close is in a defer, so panics in fetchJWKS still unblock waiters. - Map entry is removed in the same defer, so a fresh failed fetch can be retried by the next request without waiting for any stale entry. - ctx.Done() unblocks waiters independently of the leader's progress. - Re-checks the cache after winning LoadOrStore, since another fetch may have populated the cache between the initial miss and the win. Cleanup: also removes a stray yaegi-extract output file (github_com-lukaszraczylo-traefikoidc.go) that was accidentally committed during local yaegi compatibility testing. All tests pass with -race; golangci-lint clean.v1.0.16 |
||
|
lukaszraczylo
|
72e2b682bb |
fix: eliminate per-request global mutexes in Yaegi hot paths
The v1.0.14 fix replaced one contended sync.RWMutex (RefreshCoordinator.
refreshMutex) with sync.Map. Production showed the same death-spiral
signature recurring ~2 hours later — same shape, different mutex:
65 goroutines stuck on a sync.(*RWMutex).Lock at one address, pod
pinned at 1000m CPU, identical Yaegi runCfg/reflect.Value.Call stack
pattern. The mutex was RefreshCoordinator.attemptsMutex.
Generalising: under Yaegi (interpreted Go for traefik plugins), any
per-request global mutex acquisition is a latent serialization point.
reflect.Value.Call dispatch on a held lock turns a microsecond
critical section into a multi-millisecond one, and on a GOMAXPROCS=1
pod the queue is unbounded.
This commit removes every per-request global mutex on the hot path:
1. RefreshCoordinator.attemptsMutex (sync.RWMutex)
sessionRefreshAttempts: map -> sync.Map.
refreshAttemptTracker: all fields atomic (int32, int64 UnixNano,
cooldownEndNano == 0 as the not-in-cooldown sentinel, replacing
the inCooldown bool).
isInCooldown / recordRefreshAttempt / recordRefreshSuccess /
recordRefreshFailure all become lock-free. Cooldown entry uses
CompareAndSwapInt64 so only one goroutine logs the transition.
2. RefreshCircuitBreaker.mutex (sync.RWMutex)
lastFailureTime / lastSuccessTime -> atomic.Int64 UnixNano.
state and failures already atomic.
AllowRequest / RecordSuccess / RecordFailure now pure atomic ops.
3. TraefikOidc.firstRequestMutex (sync.Mutex)
firstRequestReceived bool -> firstRequestStarted int32.
metadataRefreshStarted bool -> metadataRefreshStartedAtomic int32.
ServeHTTP bootstrap path uses CompareAndSwapInt32 — fires once,
zero steady-state cost. Previously the mutex was acquired on
every non-health request forever.
4. TraefikOidc.metadataRetryMutex (sync.Mutex)
lastMetadataRetryTime time.Time -> lastMetadataRetryNano int64.
The 30-second retry throttle is now a CAS on lastMetadataRetryNano.
cleanupStaleEntries iterates via sync.Map.Range; eviction is a
CompareAndDelete by pointer identity so a tracker freshly re-used by
a concurrent caller is not lost.
Empirical evidence (3 specialist-agent analysis of the v1.0.14 spike,
profiles in /tmp/traefik-spike-1779511683/):
* mutex profile: 97% delay in sync.(*Mutex).Unlock via
HTTPHandlerSwitcher -> accesslog -> metrics -> backoff.RetryNotify
* 65 stuck goroutines at one RWMutex address (0x40022eb648),
identical Yaegi CFG pointer, all on rc.attemptsMutex via
recordRefreshAttempt + isInCooldown
* traffic driver: long-lived in-cluster Go-http-client doing
~5.4 req/s POST embeddings via OIDC cookie session → same
sessionID → contention all funnels to one tracker entry
Yaegi support for sync/atomic confirmed at
github.com/traefik/[email protected].1/stdlib/go1_22_sync_atomic.go:
AddInt32/Int64, LoadInt32/Int64, StoreInt32/Int64,
CompareAndSwapInt32/Int64 all exposed via reflect.ValueOf. Yaegi
dispatches each call through reflect.Value.Call to the COMPILED
atomic.* function, which executes a single hardware CAS/LOCK-XADD
instruction. Each atomic op still pays Yaegi dispatch cost but
cannot block — no queueing, no death spiral.
Trade-off acknowledged: v1.0.15 issues ~6-8 atomic/sync.Map ops per
leader-path request vs the 4 mutex ops of v1.0.14. Under low
contention this is a modest CPU bump. Under high contention it's
an unbounded → bounded transformation. Net win.
All tests pass with -race; golangci-lint clean.
v1.0.15
|
||
|
lukaszraczylo
|
ae4ccaa89d |
fix(refresh-coordinator): replace global RWMutex with sync.Map
Under Yaegi, the RefreshCoordinator.refreshMutex was held for tens of
milliseconds per request because every operation inside the critical
section (map access, isInCooldown, recordRefreshAttempt,
isUnderMemoryPressure, atomic ops, struct allocation) is dispatched
through reflect.Value.Call with full arg boxing/unboxing.
Concurrent refreshes on the same coordinator serialized into a queue
that grew without bound. Live capture in production (3 Grafana
dashboards left open) showed:
* 63 goroutines stuck on rc.refreshMutex.Lock() for 1-11 minutes
* pod pinned at 1000m CPU (GOMAXPROCS=1)
* 5.15M allocs/sec, 0.45 RPS effective throughput
* yaegi.call.func9 accounting for 92.66% of cumulative allocs
* mutex profile dominated by sync.(*Mutex).Unlock via the request chain
Change inFlightRefreshes from map[string]*refreshOperation+RWMutex to
sync.Map and rewrite getOrCreateOperation to:
1. Speculatively allocate the candidate operation.
2. Atomically LoadOrStore by tokenHash. Joiners take the existing
operation; leader takes the new one. No global lock acquired.
3. Leader runs rate-limit / cooldown / memory-pressure gates AFTER
the atomic store. Joiners share the leader's outcome via op.done.
4. Reserve the concurrent-refresh slot via CompareAndSwap so the
count cannot overshoot in absence of the old serializing lock.
5. On any gate failure the leader calls failCandidate, which deletes
the entry from sync.Map, records the error on op.result and closes
op.done so any joiner that snuck in returns the same error.
performCleanup becomes a single sync.Map.LoadAndDelete, eliminating
the lock entirely on the cleanup path.
Net effect: critical section is no longer Yaegi-interpreted; it
collapses to atomic instructions on a sharded sync.Map. Refresh
contention disappears even under Yaegi.
All tests pass with -race; golangci-lint clean.
v1.0.14
|
||
|
lukaszraczylo
|
984fd1c08f |
docs: add Telemetry section linking to oss-telemetry opt-out docs
Discloses the single anonymous adoption ping sent on first plugin instantiation. Points users to the upstream README section for the disclosure pattern and to the local telemetry.go for the inline implementation. |
||
|
lukaszraczylo
|
99bdd23986 |
feat: anonymous usage telemetry via inline oss-telemetry
Adds a yaegi-safe inline telemetry helper that fires a single
fire-and-forget ping at plugin load. Helps track adoption and version
spread. No persistent identifiers are collected.
Implementation notes:
- inline (no external dep) so Traefik plugin loader does not need to
resolve a new vendored module
- stdlib-only, no generics, no range-over-int — verified to load under
yaegi 0.16.x (full plugin import + CreateConfig/New symbol lookup OK)
- avoids `switch{case A,B,C:}` blocks where some yaegi releases
mis-evaluate comma-separated case lists
- sync.Once guards against amplified pings on Traefik dynamic config
reloads (which re-instantiate the middleware)
Opt out via any of:
DO_NOT_TRACK=1
OSS_TELEMETRY_DISABLED=1
TRAEFIKOIDC_DISABLE_TELEMETRY=1
v1.0.12
|
||
|
lukaszraczyloandGitHub
|
a548665edb |
feat: opt-in M2M bearer-token authentication (supersedes #93) (#140)
* docs: bearer-token auth design spec * docs: harden bearer-auth spec with security review findings * feat(bearer): opt-in M2M bearer-token authentication Adds an opt-in Authorization: Bearer <jwt> path for machine-to-machine clients. Replaces and supersedes the broken approach in PR #93 (synthetic-session that omitted user_identifier and skipped ID-token rejection / replay-protection-semantics / kid-pinning / etc.). Design Two auth entrypoints feed one shared post-auth pipeline: cookie path ─┐ ├── forwardAuthorized(rw, req, *principal) bearer path ─┘ (roles/groups, header injection, security headers, cookie strip, forward) buildPrincipalFromSession and buildPrincipalFromBearerToken produce the same `principal` value type. forwardAuthorized is session-agnostic and runs the existing post-auth work; processAuthorizedRequest now wraps it with the session-specific concerns (backchannel-logout, dirty/Save). The cookie path's behaviour is byte-identical to before this PR; the existing test suite passes unmodified. Security hardening baked into the bearer path - Audience MANDATORY. Startup fails when EnableBearerAuth=true and Audience is empty. - BearerIdentifierClaim defaults to "sub"; "email" is rejected at startup to avoid the unverified-email spoofing footgun. Cookie path's UserIdentifierClaim is unaffected and still defaults to "email". - ID tokens explicitly rejected via the existing detectTokenType helper (nonce, typ=at+jwt, token_use, scope, aud-vs-clientID heuristics); belt-and-braces nonce/token_use=id rejection on top. - alg pinned to asymmetric allowlist (RS/PS/ES 256/384/512) BEFORE JWKS fetch, blocking alg=none and alg=HS* probes from amplifying into upstream calls. - kid length capped at 256 bytes and charset-restricted before JWKS fetch, blocking pathological-kid JWKS amplification. - Multi-audience tokens require azp == clientID. - iat upper-age bound (MaxTokenAgeSeconds, default 24h) bounds clock- manipulation and forever-token abuse. - Identifier sanitization: length cap, control-char + bidi-override + delimiter (, ; =) rejection. - Per-IP failure throttle: configurable threshold/window/penalty; returns 429 + Retry-After. Limits offline-guessing-style attacks and protects the shared rate-limiter / JWKS endpoint. - JTI replay marking suppressed via new internal verifyOpts {skipReplayMarking} so the same bearer can be reused until exp; the blacklist Get stays active so RevokeToken still terminates a bearer token immediately. The existing exported VerifyToken interface is unchanged so all mocks continue to work. - Cookie wins by default when both bearer and cookie are present (safer against browser/extension/proxy bearer injection). Operator can flip via BearerOverridesCookie. - Authorization header stripped on forward by default; also stripped on excluded URLs so the token can't leak into health/metrics downstream logs. - Optional RFC 7662 introspection via existing requireTokenIntrospection. Introspection-endpoint failure returns 503 (distinguishes infra from token rejection). - 401s use RFC 6750 WWW-Authenticate hints (toggleable). Failure reason is logged at debug; raw tokens are never logged. Implementation - principal.go: pure-data principal type and buildPrincipalFromSession. - bearer_auth.go: alg/kid pin, classifier, identifier sanitization, multi-aud azp gate, iat age check, per-IP failure tracker, handleBearerRequest, buildPrincipalFromBearerToken. - token_manager.go: VerifyToken now wraps a new verifyTokenWithOpts that accepts internal-only verifyOpts. Existing callers, the TokenVerifier interface, and all mocks unchanged. - middleware.go: extracted forwardAuthorized from processAuthorizedRequest; wired bearer detection after init wait + after bypass; excluded-URL Authorization strip when bearer enabled. - settings.go: ten new config fields with defaults applied in CreateConfig. - main.go: startup validation for audience + identifier-claim guard; bearer failure tracker init. Tests - bearer_auth_test.go: table-driven helper tests for every new component (parseBearerJOSEHeader, sanitizeBearerIdentifier, resolveBearerIdentifier, enforceMultiAudienceAzp, enforceIatAge, bearerFailureTracker, detectBearerToken). Integration tests through ServeHTTP covering happy path, ID-token rejection, alg=none rejection, oversized kid, multi-aud with/without azp, iat-too-old, bidi identifier, replay (100x reuse), 429 throttle trip, excluded-URL strip, roles gate, cookie-wins precedence, BearerOverridesCookie, oversized token, malformed JWT, feature-off pass-through. Startup validation for audience- required and email-identifier-rejected. - All existing tests pass unmodified (cookie-path regression). - go vet clean. golangci-lint clean (0 issues). Race detector clean on bearer tests. Documentation - README.md: bearer auth section with security highlights and config snippet; doc link in the index. - .traefik.yml: commented config block exposing every bearer knob. - docs/CONFIGURATION.md: new subsection with full parameter table. - docs/BEARER_AUTH.md: threat model, hardening matrix, failure response table, operational guidance, known follow-ups. - docs/superpowers/specs/2026-05-18-bearer-token-auth-design.md: design spec + security-review hardening history. * fix(cache): redact raw cache keys in debug logs (CodeQL go/clear-text-logging) CodeQL flagged 9 high-severity alerts (go/clear-text-logging) where the in-memory cache and the hybrid L1+L2 backend printed `key=%s` at debug. Cache callers (token cache, blacklist, introspection cache) pass raw access / refresh / id tokens as cache keys, so any debug-enabled deployment would write them to log streams. Pre-existing issue. CodeQL started flagging it on this PR because the new bearer-auth path adds a data-flow source (req.Header.Get("Authorization")) that reaches the existing logging sinks via the same cache. The cookie path had the same risk but wasn't tracked as taint by CodeQL. Fix: hash the key (SHA-256[:8] hex) before printing. Same approach the bearer-auth logger uses for principal identifiers (spec §13). Doesn't change cache semantics — same key still produces the same hash, so debug correlation across log lines is preserved without exposing the raw value. Touches both affected packages: - internal/cache/cache.go (2 sites: Set + LRU eviction) - internal/cache/backends/hybrid.go (12 sites: L1/L2 read/write/fallback) New helper `redactKey` colocated with each package (unexported, package-local) keeps the change blast radius narrow. Tests green; lint clean. * docs(bearer): how to obtain bearer tokens from the OIDC provider Adds a section walking operators through the OAuth 2.0 client_credentials flow (RFC 6749 §4.4) and the JWT bearer assertion alternative (RFC 7523), with a worked Auth0-shape curl example, a per-provider quick reference (Auth0, Okta, Keycloak, Entra v2, Cognito, GitLab, Google), operational notes (token TTL, caching, JWKS rotation, revocation, scope vs audience, secret hygiene), and a three-line validation loop. Most common operator confusion: "I enabled the feature but tokens get 401'd" — almost always missing or wrong audience. The new section makes the audience-matching requirement loud, with per-provider parameter names so people don't have to dig through IdP docs. Locations: - docs/BEARER_AUTH.md — full section under "Quick start" - README.md — short snippet + deep linkv1.0.11 |
||
|
lukaszraczyloandGitHub
|
8c5df82dcf |
fix(azure): treat Microsoft proprietary access tokens as opaque (#134) (#138)
Followup to issue #134 — two reporters returned saying that even with the JWKS caching fix in v1.0.7/v1.0.8, every request emitted: ERROR: TraefikOidcPlugin: UNKNOWN token verification failed: signature verification failed: crypto/rsa: verification error ERROR: TraefikOidcPlugin: DIAGNOSTIC: Signature verification failed for kid=<kid>, alg=RS256: crypto/rsa: verification error Root cause: when an Azure tenant is configured without a custom API resource, Microsoft issues access tokens for Microsoft Graph (or Azure Mgmt). These tokens carry a `nonce` value in the JWT *header*; the bytes that get signed contain SHA256(nonce), while the wire token ships the original nonce. Any standard JWS verifier rejects the signature, which is exactly Microsoft's intent — they document the format as proprietary and tell client apps not to validate it (https://learn.microsoft.com/en-us/entra/identity-platform/access-tokens "you can't validate tokens for Microsoft Graph according to these rules due to their proprietary format"). validateAzureTokens was nonetheless attempting JWT verification on every JWT-shaped access token, then silently falling back to the ID token when verification failed. Auth still worked end-to-end, but every request spammed two error log lines. Two-layer defense: * validateAzureTokens now detects the proprietary-nonce header before calling verifyToken on the access token. When detected, the token is treated as opaque (matching the existing branch for non-JWT tokens) and validation proceeds via the ID token, exactly as Microsoft prescribes. * VerifyJWTSignatureAndClaims downgrades the DIAGNOSTIC error log to debug for tokens carrying the same proprietary marker, in case any path outside validateAzureTokens reaches it. Authorization still hinges on a separately-verifiable ID token — the confused-deputy guard from CWE-441 is preserved (and explicitly tested).v1.0.10 |
||
|
lukaszraczyloandGitHub
|
aa96e9dbee |
Add sponsorship
Just in case you appreciate this project, feel generous and want to sponsor my caffeine addiction. |
||
|
lukaszraczyloandGitHub
|
1e33bb0a4d |
feat(auth): support private_key_jwt and client_secret_basic (#137)
revocation endpoints, joining the existing client_secret_post default. Both are opt-in via the new clientAuthMethod config field. Closes #135. private_key_jwt (RFC 7523 §2.2 / OpenID Connect Core §9) ======================================================== Plugin signs a short-lived JWT with a configured private key and presents it as client_assertion. Use when the IdP enforces short secret TTLs or requires secretless client auth (Microsoft Entra ID / Azure AD, Okta, Auth0, Keycloak). New Config fields: clientAuthMethod (default: client_secret_post) clientAssertionPrivateKey (inline PEM) clientAssertionKeyPath (PEM file path; mutually exclusive) clientAssertionKeyID (JWS kid header — required) clientAssertionAlg (default: RS256; RS/PS/ES 256–512 supported) PEM forms accepted: PKCS#8, PKCS#1, SEC1. Assertion claims: iss=sub=clientID, aud=tokenURL, iat=now, exp=now+60s, random 16-byte hex jti per request. ECDSA signatures are raw r||s per RFC 7515 (not ASN.1). client_secret_basic (RFC 6749 §2.3.1) ===================================== Sends credentials in the Authorization: Basic header instead of the body. Both halves are form-urlencoded individually before base64 — that encoding step is required by the spec and is NOT what stdlib's http.Request.SetBasicAuth does, so the plugin uses its own helper. The form body omits client_id and client_secret on this path. Wire-up ======= Both methods are dispatched at the same two call sites: helpers.go:exchangeTokens — auth_code + refresh_token grants token_manager.go:RevokeTokenWithProvider — RFC 7009 revocation Existing clientSecret deployments are unaffected — empty clientAuthMethod maps to the historical client_secret_post behavior, and clientAssertion remains nil unless the new fields are set. Yaegi compatibility =================== All required crypto/rsa, crypto/ecdsa, crypto/x509, encoding/pem and crypto/sha256/384/512 symbols are exposed by the traefik/yaegi stdlib symbol tables (RSA SignPKCS1v15 + SignPSS, ECDSA Sign, ParsePKCS8/1PrivateKey, ParseECPrivateKey). Tests (16 new) ============== Algorithm-family coverage: TestIssue135_SignerRSAFamily — RS256/384/512 + PS256/384/512 TestIssue135_SignerECDSAFamily — ES256/384/512, raw r||s shape TestIssue135_SignerRejectsAlgKeyMismatch TestIssue135_SignerJTIUniqueness — 50 sigs, all jti distinct TestIssue135_SignerPEMVariants — PKCS#8, PKCS#1, SEC1 Config validation: TestIssue135_ConfigValidation — full Validate() matrix TestIssue135_ConfigKeyPathLoadsFile Wire-up: TestIssue135_AuthCodeExchangeUsesAssertion TestIssue135_RefreshTokenUsesAssertion TestIssue135_BackcompatClientSecretPath TestIssue135_RevocationUsesAssertion TestIssue135_BuildSignerFromInlineConfig TestIssue135_BuildSignerDefaultsToRS256 TestIssue135_ClientSecretBasicAuth — Authorization header, no body creds TestIssue135_ClientSecretBasicURLEncodesReservedChars — :, +, /, @, =, & TestIssue135_ClientSecretBasicRevocation — revocation parity Documentation ============= README.md — required-row note + 5 optional rows + dedicated section docs/CONFIGURATION.md — new Client Authentication section with three method subsections, OpenSSL keygen snippet, RFC links docs/index.html — 5 new config-table rows + Private Key JWT explainer card .traefik.yml + examples/complete-traefik-config.yaml — commented opt-in example Out of scope (deferred) ======================= mTLS / tls_client_auth (RFC 8705) — separate change; requires per-call http.Client with tls.Config.Certificates and conflicts with the current pooled HTTP client architecture.v1.0.8 |
||
|
lukaszraczyloandGitHub
|
bfd702a447 |
fix(jwk): keep parsed JWKS in local cache only (#134) (#136)
Under yaegi (Traefik's plugin runtime) json.Marshal exposes unexported
struct fields with an X-prefixed name. parsedJWKS{ keys map[string]
crypto.PublicKey } therefore round-tripped through Redis as
{"Xkeys":{"<kid>":{"N":<huge>,"E":65537}}} — *rsa.PublicKey.N is a
*big.Int that marshals to a JSON number hundreds of digits long. On
read, json.Unmarshal into interface{} parses numbers as float64, which
cannot represent that range:
Failed to deserialize value for key .../discovery/v2.0/keys:parsed:
json: cannot unmarshal number 2251513...
into Go value of type float64
Auth still worked (the JWKCache rebuilt the keys in memory on every
miss) but the error log spammed every request.
Two structural problems were behind it:
* parsedJWKS holds crypto.PublicKey interface values that aren't
meaningfully JSON-serializable. Even on compiled Go (where the
unexported field marshals to {}), the post-roundtrip type assertion
v.(*parsedJWKS) silently failed and the cache was useless.
* The same pattern applied to *JWKSet — the struct shape survived JSON
but the type assertion still failed, defeating the cache for every
call that went through Redis.
Both keys now use the new UniversalCache.SetLocal/GetLocal pair, which
skips the configured distributed backend entirely. JWK rotation is rare
and a per-replica HTTP fetch on cold cache is cheap, so cross-replica
coherence buys nothing for these entries.
Stale Redis entries written by previous versions are simply ignored —
the new code never reads under those keys, and Redis TTL retires them.
Includes regression coverage for the Azure round-trip, the
poisoned-stale-data scenario, and the SetLocal/GetLocal isolation
contract.
patch-release
v1.0.7
|
||
|
lukaszraczylo
|
68c150eba4 |
fix(cache/redis): honor enableTLS for Redis backend (#133)
The redis.enableTLS / redis.tlsSkipVerify settings were accepted by the config layer but silently dropped before reaching the connection pool, so the plugin always dialed Redis in plaintext. This blocked TLS-only Redis deployments such as AWS ElastiCache with in-transit encryption. - Add EnableTLS, TLSSkipVerify, TLSServerName to backends.Config and PoolConfig and forward them through universal_cache_singleton -> backends.Config -> PoolConfig. - In the connection pool, dial via tls.Dialer.DialContext (TLS 1.2 minimum) with SNI defaulting to the host part of the configured Address when TLSServerName is empty, so ElastiCache cluster endpoints validate out of the box. Plain dial path now also propagates ctx. - Add regression tests covering successful TLS negotiation with skip- verify, rejection of self-signed certs without skip-verify, rejection of plain TCP servers when EnableTLS=true, and unaffected plaintext behavior. - Document maxRefreshTokenAgeSeconds (added inv1.0.5 |
||
|
lukaszraczylo
|
9cbca4c4fb |
fix(refresh): honor userIdentifierClaim in token refresh path (#132)
patch-release
The refresh path in token_manager.go hardcoded the "email" claim when
extracting the user identifier from a refreshed ID token, ignoring the
configured userIdentifierClaim. Keycloak users without an email claim
(using sub or another identifier) were kicked out on refresh even
though their initial login worked.
The callback path (auth_flow.go:226-239) already honored
userIdentifierClaim with "sub" fallback; PR #100 (commit
v1.0.3
|
||
|
lukaszraczylo
|
684a990f59 |
fix: reduce yaegi CPU footprint + require auth on SSE/WebSocket bypass
minor-release Behaviour changes (potentially breaking for operators relying on the prior unauthenticated SSE bypass): * SSE (`Accept: text/event-stream`) and WebSocket upgrade requests now return 401 when no authenticated session is present. Previously the bypass forwarded unconditionally, which let any caller reach the backend by setting the right header. Excluded URLs are unchanged. Operators relying on unauthenticated SSE/WS access must move the path into ExcludedURLs. Performance fixes (target: long-running dashboards like Grafana / ArgoCD where many panels poll concurrently while the page stays open): * Stop honouring isTestMode() for the singleton-token-cleanup interval under yaegi (the Traefik plugin runtime). In production the plugin was running a 20 Hz no-op cleanup ticker because runtime.Compiler == "yaegi" tripped the test-mode branch. * processAuthorizedRequest now resolves ID-token claims at most once per request via SessionData.GetIDTokenClaims (already cached on the session) and reuses them for both groups/roles extraction and header-template rendering. Previously every authenticated request parsed the JWT twice. * Added extractGroupsAndRolesFromClaims to drive groups/roles off pre-parsed claims; extractGroupsAndRoles still works for tests. * Removed the unconditional session.MarkDirty() in the header-templates branch. Templates only mutate request headers, not session state, so the prior MarkDirty was re-encrypting and rewriting all session cookies on every authenticated request that used header templates. Other: * Added isWebSocketUpgrade (RFC 6455 handshake detection — Connection: Upgrade + Upgrade: websocket, tolerant of multi-token Connection headers and case). * Renamed applySSEUserHeaders -> applyBypassUserHeaders; it now returns bool so the dispatcher can reject unauthenticated SSE/WS with 401. * Added tests for SSE and WS bypass covering both the auth-rejection path and the authenticated forward path.v1.0.1 |
||
|
lukaszraczyloandGitHub
|
1b6c8616fd |
fix(refresh): coalesce refresh-token grants + bound goroutines + cache hot path (target v0.8.27) (#131)
* fix(refresh): wire RefreshCoordinator into the live refresh path
The RefreshCoordinator existed but was never instantiated. The actual
refresh path used only session.refreshMutex, which is per-SessionData
instance - and SessionData is pulled from a sync.Pool per request -
so concurrent requests sharing a refresh token had ZERO coordination.
Symptom: when access_token expired (e.g. 5min Zitadel default), every
in-flight request from a polling client (Grafana panels) entered the
refresh path simultaneously and POSTed the same refresh_token to the
IdP. With refresh-token rotation enabled (Zitadel/Authentik default),
only one grant succeeded; the rest got invalid_grant and each cleared
the entire session. Subsequent requests then thrashed in re-auth loops.
This commit:
- adds refreshCoordinator field on TraefikOidc
- instantiates it in NewWithContext with DefaultRefreshCoordinatorConfig
- shuts it down in Close() under shutdownOnce
- routes refreshToken() through the coordinator via coordinatedTokenRefresh,
which collapses concurrent grants to a single upstream call per
refresh_token hash
- exports refreshCoordinatorSessionID for both internal hashing and the
middleware-level wireup so dedup keys stay aligned
Behavioural notes:
- nil-coordinator fallback preserves existing tests that build TraefikOidc
literals without going through the constructor
- followers receive the same TokenResponse/error as the leader, so no
per-instance code paths change
- existing TestGetNewTokenWithRefreshToken_Concurrency still passes
because it hits GetNewTokenWithRefreshToken directly, below the
coordinator boundary
Tests:
- refresh_coordinator_wireup_test.go: 50 concurrent refreshes coalesce
to <=2 upstream calls; distinct tokens still run in parallel; nil
coordinator falls back cleanly
* perf(cache): bound L1 backfill goroutines in HybridBackend
Get() and GetMany() previously spawned a goroutine per L2 hit to write
the value through to L1. Under sustained polling traffic (e.g. a Grafana
dashboard refreshing every 30s with N panels) this minted thousands of
goroutines, each running in Yaegi - directly contributing to the
~1000% CPU spike that pairs with the refresh-token herd.
Replace the per-hit goroutines with a single l1BackfillWorker fed by
l1BackfillBuffer, mirroring the existing asyncWriteBuffer/asyncWriteWorker
pattern for L2 writes. Buffer overflow drops the backfill (counted via
l1BackfillDrops) - a dropped backfill just means the next L2 hit for
that key re-queues it, which is safe.
Tests:
- TestHybridBackend_L1BackfillBounded: 1000 distinct L2 hits keep
goroutine count within +20 of baseline (pre-fix it grew by ~1000)
- TestHybridBackend_L1BackfillFullDrops: drops are accounted for when
the buffer is saturated and the worker is stopped
* feat(refresh): implement isRefreshTokenExpired heuristic
Replace the placeholder `return false` with a real check based on the
issued_at timestamp that SetRefreshToken already stamps into the session.
Gated by a new MaxRefreshTokenAgeSeconds config field (default 21600 =
6h, matching the existing comment). 0 disables the check.
This wires the previously-dead refreshTokenExpired branch in middleware.go,
which short-circuits AJAX requests with a 401 instead of letting them
hammer the IdP for a refresh token that's almost certainly stale - the
classic Grafana-after-long-pause failure mode.
Behaviour:
- maxRefreshTokenAge=0 disables the check (preserves prior behaviour)
- legacy sessions without issued_at still attempt one refresh; the IdP
remains the source of truth on first try
- nil-receiver and nil-session guards keep test code that builds
TraefikOidc literals safe
Tests:
- TestIsRefreshTokenExpired_DisabledWhenAgeZero
- TestIsRefreshTokenExpired_LegacySessionWithoutTimestamp
- TestIsRefreshTokenExpired_WithinWindow
- TestIsRefreshTokenExpired_BeyondWindow
- TestIsRefreshTokenExpired_NilGuards
* perf(token): skip parseJWT on cache hit in VerifyToken
The token cache fast-return existed but ran AFTER parseJWT, so every
validation paid for base64 + JSON unmarshal even on a hit. Under bursty
traffic (e.g. 10+ concurrent panel requests on every Grafana dashboard
refresh, each calling validateStandardTokens which verifies BOTH the
access token and the ID token), this is two redundant parses per
request multiplied by the panel count.
Move the cache lookup ahead of parseJWT. On a hit the function returns
nil immediately. On a miss the original flow runs unchanged.
Also nil-guard t.tokenCache to keep partial-literal test instances safe
(matches the same pattern we already use for tokenBlacklist).
Tests:
- TestVerifyToken_CacheHitSkipsParse: cache pre-populated with claims
for a token whose body would fail parseJWT - returns nil iff the
fast-path bypasses the parse
- TestVerifyToken_CacheMissStillParses: a syntactically valid but
unsigned token still errors past parseJWT on cache miss
* feat(refresh): cross-replica refresh-grant dedup via shared cache
The in-process RefreshCoordinator added in
v0.8.27
|
||
|
lukaszraczylo
|
4d28fa01ab |
perf(jwk,cache): cache parsed public keys + RLock token cache reads
Hot-path JWT verification rebuilt the public key on every call: jwk -> ToRSAPublicKey -> x509.MarshalPKIXPublicKey -> pem.Encode -> verifySignature -> pem.Decode -> x509.ParsePKIXPublicKey -> verify Under yaegi this pinned a CPU when many concurrent dashboard panels poll behind the middleware. The PEM round trip is pure waste. * jwk.go: cache pre-parsed crypto.PublicKey per kid alongside the raw JWKSet (parallel cache entry, same 1h TTL, invalidates together). * jwt.go: split verifySignatureWithKey from verifySignature; existing PEM-input entry point preserved for backchannel-logout callers. * token_manager.go: VerifyJWTSignatureAndClaims now goes straight from jwks cache to verifySignatureWithKey, no PEM round trip and no per-request availableKids slice. * universal_cache.go: token/JWK/session Get() takes RLock when the entry is unexpired, so concurrent token verifications no longer serialize on a single mutex. LRU semantics for general and metadata caches are unchanged (tests cover the strict-LRU contract there). * mocks: MockJWKCache, EnhancedMockJWKCache, mockJWKCacheForLogout, staticJWKCache satisfy the extended interface.v0.8.26 |
||
|
lukaszraczyloandGitHub
|
2d1b04c637 |
review fixes apr 2026 (#130)
* Multiple fixes - refresh coordinator dedup + memory pressure wire - middleware sse consolidation + timer leak + claim cache - universal cache sync backfill + isDebug gate - lazy background task race - memory monitor stw cached + refresh() api * fix(auth): suppress OIDC redirects on non-navigation requests - [x] Add isNonNavigationRequest using Sec-Fetch-Mode and Accept headers - [x] Add comprehensive TestIsNonNavigationRequest - [x] Update ServeHTTP to 401 non-navigation and AJAX requests Fixes #129 * feat(config): add custom CA and insecure skip verify for OIDC TLS - [x] Add CACertPath, CACertPEM, InsecureSkipVerify to Config - [x] Implement loadCACertPool for CA bundle loading - [x] Update HTTPClientConfig with RootCAs and InsecureSkipVerify - [x] Apply CA pool and skip verify to pooled HTTP clients - [x] Enhance configKey to distinguish TLS configs - [x] Add comprehensive ca_cert_test.go Fixes #125 * feat(oidc): add custom CA certificate support for private OIDC providers - [x] Add caCertPath, caCertPEM, insecureSkipVerify config options - [x] Update traefik.yml with new OIDC client config fields - [x] Add configuration schema descriptions for new options - [x] Update README table and add Custom CA Certificates section * Fix the documentation. * test(redis): add oversized argument rejection test - [x] Add TestRedisConn_RejectOversizedArgumentBytes - [x] Import strings package * Dependencies cleanupv0.8.25 |
||
|
lukaszraczyloandGitHub
|
ccbb98b9dd | fix-issue-122 (#128) v0.8.24 | ||
|
Serhii VasylievandGitHub
|
1362cc0dac |
Improve debug logging around callback URL matching (#126)
* Add debug logging around callback URL matching in ServeHTTP The callback URL comparison at the core of OIDC flow had zero logging, making it extremely difficult to diagnose redirect loop issues caused by misconfigured callbackURL (e.g., full URL vs path-only). Every other path comparison in ServeHTTP already logs debug info (logout, backchannel, frontchannel, excluded URLs), but the callback URL check was completely silent. Added debug logs that show: - The values being compared (request path vs configured callback) - Whether the match succeeded or failed - Configured redirURLPath during initialization This would have immediately revealed the root cause of issue #1 where callbackURL was set as a full URL but compared against req.URL.Path which only contains the path component. Closes #3 * improve-callback-url-logging: Add init-time logging for callbackURL configv0.8.23 |
||
|
|
249dcad1b3 |
fix: prevent deadlock in SessionData.Clear method (#114)
Move mutex unlock before calling Save() to prevent potential deadlock when Save() method needs to acquire the same mutex. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <[email protected]>v0.8.22 |
||
|
lukaszraczyloandGitHub
|
de4b4d7258 |
fix(cache): remove sync.Pool for Yaegi compatibility (#121)
- [x] Remove sync.Pool implementation that causes reflection panics - [x] Replace pool-based NewRESPWriter with direct instantiation - [x] Replace pool-based NewRESPReader with direct instantiation - [x] Convert Release() methods to no-ops for API compatibility - [x] Add documentation explaining sync.Pool removal for Yaegi - [x] Remove "sync" import Resolves #120v0.8.21 |
||
|
lukaszraczyloandGitHub
|
9d52f1b018 |
feat(core): refactor linters config and improve code quality (#119)
- [x] Reorganize golangci-lint configuration with documented disable reasons - [x] Simplify errcheck and revive linter rules with targeted exclusions - [x] Pre-compile regex patterns in input_validation.go for performance - [x] Fix type assertions in memory_shard.go and resp.go with safety checks - [x] Replace string comparison with EqualFold for case-insensitive matching - [x] Fix loop variable captures in jwk.go and logout.go - [x] Change high goroutine log level from Info to Debug in autocleanup.go - [x] Replace deprecated "cancelled" spelling with "canceled" throughout - [x] Add nolint annotations for intentional unused parameters - [x] Improve comment formatting for deprecated functions - [x] Fix comment spelling: "marshalling" → "marshaling" - [x] Refactor provider warnings formatting in internal/providers/warnings.go - [x] Simplify metrics summary building in internal/recovery/metrics.go - [x] Pre-allocate slice in error_recovery.go GetDegradedServices - [x] Refactor context cancellation checks in redis.gov0.8.20 |
||
|
lukaszraczyloandGitHub
|
57724918fe |
fix 116 (#118)
* Fix cache serialisation * fix(cache): add integer overflow protection for serialization - [x] Add maxCacheEntrySize constant (64 MiB) to prevent memory overflow - [x] Validate byte slice size before adding marker byte - [x] Validate JSON-serialized data size before marker addition - [x] Add comprehensive overflow protection test cases * docs: add security fix documentation for integer overflow protection * test: fix goroutine tests to use mock OIDC servers The TestContextAwareGoroutineManagement tests were making real HTTP calls to hardcoded URLs like https://example.com, causing failures in CI when those requests timeout or return HTTP errors. Changes: - Added createMockOIDCServer() helper function using httptest - Updated GoroutineCleanupOnContextCancel to use mock server - Updated NoGoroutineLeakOnMultipleInstances to use 3 mock servers - Updated SingletonTasksAcrossInstances to use mock servers array This prevents network calls and makes tests more reliable and faster. Fixes test failures in GitHub Actions CI.v0.8.19 |
||
|
lukaszraczyloandGitHub
|
775de2ada1 |
Fix cache serialisation (#117)
* Fix cache serialisation * fix(cache): add integer overflow protection for serialization - [x] Add maxCacheEntrySize constant (64 MiB) to prevent memory overflow - [x] Validate byte slice size before adding marker byte - [x] Validate JSON-serialized data size before marker addition - [x] Add comprehensive overflow protection test cases |