From 68e1c4319c0dd5f041cbba2e3c34c21cc0b19b5e Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sat, 23 May 2026 12:22:51 +0100 Subject: [PATCH] feat(middleware): per-request context object (requestState) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- middleware.go | 108 ++++++++++++++++++++++++++++++++++++++++++++++-- requeststate.go | 71 +++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 requeststate.go diff --git a/middleware.go b/middleware.go index 7e0ef2e..ec37a3a 100644 --- a/middleware.go +++ b/middleware.go @@ -311,6 +311,19 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { host := utils.DetermineHost(req) redirectURL := buildFullURL(scheme, host, t.redirURLPath) + // Capture per-request state: one RLock on sd.sessionMutex covers all the + // getter values the handler chain needs (instead of 5-7 separate + // session.GetX() calls each acquiring their own RLock under Yaegi). + // metadataSnap is also stored once so downstream handlers don't repeat + // the atomic.Value.Load. + rs := (&requestState{ + scheme: scheme, + host: host, + redirectURL: redirectURL, + next: t.next, + metadata: t.metadataSnap(), + }).captureSession(session) + // Check if the current request is the OIDC callback t.logger.Debugf("Checking callback URL match: request_path=%q, configured_callback=%q", req.URL.Path, t.redirURLPath) if req.URL.Path == t.redirURLPath { @@ -328,7 +341,7 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { return } - userIdentifier := session.GetUserIdentifier() + userIdentifier := rs.userIdentifier // User authorization check if authenticated && userIdentifier != "" { if !t.isAllowedUser(userIdentifier) { @@ -345,11 +358,11 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // methods (validateAzureTokens/validateStandardTokens) before reaching this point. // Redundant validation here was causing issues with Azure AD tokens that have // JWT format but unverifiable signatures. See issue #89. - t.processAuthorizedRequest(rw, req, session, redirectURL) + t.processAuthorizedRequestRS(rw, req, rs) return } - refreshTokenPresent := session.GetRefreshToken() != "" + refreshTokenPresent := rs.refreshToken != "" // Decide whether to answer with 401 instead of a redirect. AJAX requests // cannot follow a 302 into an IdP, and sub-resource loads (script/image/ @@ -456,6 +469,95 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // - req: The HTTP request to process. // - session: The user's session data containing tokens and claims. // - redirectURL: The callback URL for re-authentication if needed. +// processAuthorizedRequestRS is the requestState-aware variant of +// processAuthorizedRequest. It reads SessionData fields from the captured +// snapshot in rs instead of calling session.GetX() (each of which acquires +// sd.sessionMutex.RLock — under Yaegi every RLock pays ~1-5ms of interpreter +// dispatch). Only session-mutating operations (Save, ResetRedirectCount, +// Clear, IsDirty) still go through the session pointer because those write +// state and have no snapshot. +func (t *TraefikOidc) processAuthorizedRequestRS(rw http.ResponseWriter, req *http.Request, rs *requestState) { + session := rs.session + redirectURL := rs.redirectURL + userIdentifier := rs.userIdentifier + if userIdentifier == "" { + t.logger.Info("No user identifier found in session during final processing, initiating re-auth") + session.ResetRedirectCount() + t.defaultInitiateAuthentication(rw, req, session, redirectURL) + return + } + + // Check if session has been invalidated via backchannel or front-channel logout + idToken := rs.idToken + if t.enableBackchannelLogout || t.enableFrontchannelLogout { + if idToken != "" { + sid, sub, createdAt := t.extractSessionInfo(idToken) + if t.isSessionInvalidated(sid, sub, createdAt) { + t.logger.Infof("Session for user %s has been invalidated via IdP-initiated logout", userIdentifier) + if err := session.Clear(req, rw); err != nil { + t.logger.Errorf("Error clearing invalidated session: %v", err) + } + session.ResetRedirectCount() + t.defaultInitiateAuthentication(rw, req, session, redirectURL) + return + } + } + } + + // Resolve ID-token claims at most once per request. SessionData caches + // the parsed claims keyed on the raw ID token. + var ( + idClaims map[string]interface{} + idClaimsErr error + ) + if idToken != "" { + idClaims, idClaimsErr = session.GetIDTokenClaims(t.extractClaimsFunc) + } + + var ( + groupClaims map[string]interface{} + groupClaimsErr error + ) + if idToken != "" { + groupClaims, groupClaimsErr = idClaims, idClaimsErr + } else if rs.accessToken != "" { + groupClaims, groupClaimsErr = t.extractClaimsFunc(rs.accessToken) + } else if len(t.allowedRolesAndGroups) > 0 { + t.logger.Error("No token available but roles/groups checks are required") + session.ResetRedirectCount() + t.defaultInitiateAuthentication(rw, req, session, redirectURL) + return + } + + if groupClaimsErr != nil && len(t.allowedRolesAndGroups) > 0 { + t.logger.Errorf("Failed to extract claims for roles/groups check: %v", groupClaimsErr) + session.ResetRedirectCount() + t.defaultInitiateAuthentication(rw, req, session, redirectURL) + return + } + + // Persist any dirty session state BEFORE forwardAuthorized writes the + // response. + if session.IsDirty() { + if err := session.Save(req, rw); err != nil { + t.logger.Errorf("Failed to save session after processing headers: %v", err) + } + } else { + t.logger.Debug("Session not dirty, skipping save in processAuthorizedRequest") + } + + p := &principal{ + Source: sourceSession, + Identifier: userIdentifier, + AccessToken: rs.accessToken, + IDToken: idToken, + RefreshToken: rs.refreshToken, + Claims: groupClaims, + } + + t.forwardAuthorized(rw, req, p) +} + func (t *TraefikOidc) processAuthorizedRequest(rw http.ResponseWriter, req *http.Request, session *SessionData, redirectURL string) { userIdentifier := session.GetUserIdentifier() if userIdentifier == "" { diff --git a/requeststate.go b/requeststate.go new file mode 100644 index 0000000..8aff012 --- /dev/null +++ b/requeststate.go @@ -0,0 +1,71 @@ +// Package traefikoidc provides OIDC authentication middleware for Traefik. +// requestState bundles read-mostly fields for a single ServeHTTP call. +package traefikoidc + +import "net/http" + +// requestState is a per-request context object allocated at the top of +// ServeHTTP and threaded through to downstream handlers. It caches values +// that would otherwise require a Yaegi-dispatched lock acquisition each time +// they're read: +// +// - The metadata snapshot (atomic.Value.Load once, not per-handler). +// - SessionData getter results (one RLock on sd.sessionMutex covers all +// fields, instead of 5-7 separate RLock/RUnlock pairs scattered through +// the handler chain). +// +// The struct is alloc'd at request entry, populated under at most one RLock +// of sd.sessionMutex, and discarded at request exit. It is NOT shared across +// requests and never written from another goroutine, so no synchronization +// on its fields is required. +// +// Cross-request global caches (tokenCache, JWKCache, sessionEntries, +// sessionInvalidationCache) remain — they're orthogonal. requestState's job +// is to eliminate redundant per-handler reads of values that don't change +// within a single request. +type requestState struct { + // Globals snapshotted once. + metadata *MetadataSnapshot + + // SessionData fields snapshotted under one RLock. The pointer to the + // SessionData is retained so handlers that genuinely need to mutate + // (Save, Clear, etc.) still have access. + session *SessionData + + authenticated bool + accessToken string + idToken string + refreshToken string + userIdentifier string + createdAtUnixSec int64 + + // Output: scheme/host/redirect path determined at top of ServeHTTP. + scheme string + host string + redirectURL string + + // Carry the next handler so forwardAuthorized doesn't need to close over t. + next http.Handler +} + +// captureSession populates requestState's SessionData-derived fields under a +// single RLock of sd.sessionMutex. Returns the populated rs for chaining. +// +// Replaces a sequence of SessionData.GetX() calls each of which acquires +// sd.sessionMutex.RLock(). Under Yaegi each RLock costs ~1-5ms of +// interpreter dispatch; batching saves the rest. +func (rs *requestState) captureSession(sd *SessionData) *requestState { + if sd == nil { + return rs + } + rs.session = sd + sd.sessionMutex.RLock() + rs.authenticated = sd.getAuthenticatedUnsafe() + rs.accessToken = sd.getAccessTokenUnsafe() + rs.idToken = sd.getIDTokenUnsafe() + rs.refreshToken = sd.getRefreshTokenUnsafe() + rs.userIdentifier = sd.getUserIdentifierUnsafe() + rs.createdAtUnixSec = sd.getCreatedAtUnsafe() + sd.sessionMutex.RUnlock() + return rs +}