R77: bearer allowedUsers gate, session inUse CAS

middleware.go

@@ -831,6 +831,18 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
 		}
 	}
 
+	// Bearer (M2M) path: enforce allowedUsers / allowedUserDomains the same
+	// way every cookie-path entry point does. The bearer path reaches here
+	// via buildPrincipalFromBearerToken with no earlier isAllowedUser check;
+	// without this, a valid bearer whose subject is absent from the
+	// allowlist would still be forwarded, making the restriction a no-op.
+	if p.Source == sourceBearer && !t.isAllowedUser(p.Identifier) {
+		t.logger.Infof("Bearer user %s is not in allowed users or domains", p.Identifier)
+		errorMsg := fmt.Sprintf("Access denied: User not allowed. To log out, visit: %s", t.logoutURLPath)
+		t.sendErrorResponse(rw, req, errorMsg, http.StatusForbidden)
+		return
+	}
+
 	if len(t.allowedRolesAndGroups) > 0 {
 		allowed := false
 		for _, roleOrGroup := range append(groups, roles...) {

review_r77_test.go

@@ -0,0 +1,86 @@
+package traefikoidc
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"sync/atomic"
+	"testing"
+)
+
+// TestServeHTTP_Bearer_AllowedUsersGate guards the bearer (M2M) path against
+// bypassing the allowedUsers / allowedUserDomains gate. Every cookie-path
+// entry point enforces isAllowedUser; the bearer path reaches
+// forwardAuthorized with no earlier check, so a valid bearer whose subject
+// is absent from the allowlist must be rejected with 403 rather than be
+// forwarded (which would make the restriction a no-op on the bearer path).
+func TestServeHTTP_Bearer_AllowedUsersGate(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name string
+		sub  string
+		want int
+	}{
+		{name: "subject allowed", sub: "service-account-1", want: http.StatusOK},
+		{name: "subject not allowed", sub: "other-sa", want: http.StatusForbidden},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusOK)
+			})
+			oidc := makeBearerOIDC(t, next)
+			oidc.allowedUsers = map[string]struct{}{"service-account-1": {}}
+			claims := defaultBearerClaims()
+			claims["sub"] = tc.sub
+			token := makeBearerJWT(t, defaultBearerHeader(), claims)
+			seedVerified(t, oidc, token, claims)
+
+			req := httptest.NewRequest("GET", "/api/work", nil)
+			req.Header.Set("Authorization", "Bearer "+token)
+			rw := httptest.NewRecorder()
+			oidc.ServeHTTP(rw, req)
+			if rw.Code != tc.want {
+				t.Fatalf("status=%d, want %d", rw.Code, tc.want)
+			}
+		})
+	}
+}
+
+// TestSessionPool_ReturnToPool_ExactlyOnce is a regression for the check-
+// then-put TOCTOU in returnToPoolSafely. The old code read sd.inUse,
+// decided to return it, then wrote inUse=false and Put the session back —
+// so two concurrent returns (request path + a background goroutine) could
+// both decide to return the SAME SessionData, putting it into the pool
+// twice and double-decrementing activeSessions. The fix uses a single
+// atomic CompareAndSwap so exactly one caller claims and returns the
+// object. The observable contract: N concurrent returns of one in-use
+// session decrement activeSessions by exactly 1.
+func TestSessionPool_ReturnToPool_ExactlyOnce(t *testing.T) {
+	next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+	oidc := makeBearerOIDC(t, next)
+	sm := oidc.sessionManager
+
+	sd, err := sm.GetSession(httptest.NewRequest("GET", "/", nil))
+	if err != nil {
+		t.Fatalf("GetSession: %v", err)
+	}
+	before := atomic.LoadInt64(&sm.activeSessions)
+
+	var wg sync.WaitGroup
+	for i := 0; i < 16; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			sd.returnToPoolSafely()
+		}()
+	}
+	wg.Wait()
+
+	got := before - atomic.LoadInt64(&sm.activeSessions)
+	if got != 1 {
+		t.Fatalf("returnToPoolSafely returned session %d times, want exactly 1 (double-return would corrupt the pool)", got)
+	}
+}

session.go

@@ -509,7 +509,7 @@ func NewSessionManager(encryptionKey string, forceHTTPS bool, cookieDomain strin
 			refreshMutex:       sync.Mutex{},
 			sessionMutex:       sync.RWMutex{},
 			dirty:              false,
-			inUse:              false,
+			inUse:              atomic.Bool{},
 		}
 		sd.Reset()
 		return sd
@@ -654,7 +654,7 @@ func (sm *SessionManager) cleanupSessionPool() {
 
 		if poolSession := sm.sessionPool.Get(); poolSession != nil {
 			sessionData, ok := poolSession.(*SessionData)
-			if ok && sessionData != nil && !sessionData.inUse {
+			if ok && sessionData != nil && !sessionData.inUse.Load() {
 				sessionData.Reset()
 				cleaned++
 			}
@@ -724,7 +724,7 @@ func (sm *SessionManager) PeriodicChunkCleanup() {
 	for i := 0; i < 10; i++ {
 		if poolSession := sm.sessionPool.Get(); poolSession != nil {
 			sessionData, ok := poolSession.(*SessionData)
-			if ok && sessionData != nil && !sessionData.inUse {
+			if ok && sessionData != nil && !sessionData.inUse.Load() {
 				sessionData.Reset()
 				poolCleaned++
 			}
@@ -1065,13 +1065,13 @@ func (sm *SessionManager) GetSession(r *http.Request) (*SessionData, error) {
 	atomic.AddInt64(&sm.poolHits, 1)
 	atomic.AddInt64(&sm.activeSessions, 1)
 
-	sessionData.inUse = true
+	sessionData.inUse.Store(true)
 	sessionData.request = r
 	sessionData.dirty = false
 
 	handleError := func(err error, message string) (*SessionData, error) {
 		if sessionData != nil {
-			sessionData.inUse = false
+			sessionData.inUse.Store(false)
 			sessionData.Reset()
 			sm.sessionPool.Put(sessionData)
 			atomic.AddInt64(&sm.activeSessions, -1)
@@ -1305,7 +1305,7 @@ type SessionData struct {
 
 	dirty bool
 
-	inUse bool
+	inUse atomic.Bool
 
 	// cachedClaimsToken is the ID token string whose claims were last parsed and
 	// cached. A lazy, per-request cache to avoid re-parsing the JWT on every
@@ -1681,8 +1681,14 @@ func (sd *SessionData) Clear(r *http.Request, w http.ResponseWriter) error {
 // It ensures the session is marked as not in use and properly reset before pooling.
 func (sd *SessionData) returnToPoolSafely() {
 	if sd != nil && sd.manager != nil {
-		if sd.inUse {
+		// Exactly-once return: only the goroutine that flips inUse with a
-			sd.inUse = false
+		// compare-and-swap claims the object. Without the CAS, two
+		// concurrent calls (e.g. request path + a goroutine cleanup) can
+		// both observe inUse and both Put the same SessionData back into
+		// the pool, leaving it present twice — which later yields two
+		// concurrent holders of one session (double side-effects) plus an
+		// extra activeSessions decrement.
+		if sd.inUse.CompareAndSwap(true, false) {
 			sd.Reset()
 			sd.manager.sessionPool.Put(sd)
 			atomic.AddInt64(&sd.manager.activeSessions, -1)
@@ -1844,7 +1850,7 @@ func (sd *SessionData) Reset() {
 	}
 
 	sd.dirty = false
-	sd.inUse = false
+	sd.inUse.Store(false)
 	sd.request = nil
 	sd.useCombinedStorage = true // Reset to use combined storage by default
 
@@ -1864,7 +1870,7 @@ func (sd *SessionData) Reset() {
 // It only returns the session if it's not currently in use.
 func (sd *SessionData) ReturnToPool() {
 	if sd != nil && sd.manager != nil {
-		if !sd.inUse {
+		if !sd.inUse.Load() {
 			sd.Reset()
 			sd.manager.sessionPool.Put(sd)
 			atomic.AddInt64(&sd.manager.activeSessions, -1)

session_behaviour_test.go

@@ -610,13 +610,13 @@ func (s *SessionBehaviourSuite) TestSessionData_ReturnToPool() {
 	s.Require().NoError(err)
 
 	// Initially in use
-	s.True(session.inUse)
+	s.True(session.inUse.Load())
 
 	// Return to pool safely
 	session.returnToPoolSafely()
 
 	// Should no longer be in use
-	s.False(session.inUse)
+	s.False(session.inUse.Load())
 }
 
 // TestTokenCompression tests token compression functionality

token_manager.go

@@ -444,7 +444,7 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
 
 	t.logger.Debug("Attempting to refresh token (mutex acquired)")
 
-	if !session.inUse {
+	if !session.inUse.Load() {
 		t.logger.Debug("refreshToken aborted: Session no longer in use")
 		return false
 	}