Security: enforce API key auth precedence

docs/CHANGELOG.md

@@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 This repository does not use an `Unreleased` changelog section. Add a concrete
 patch or minor version entry for every user-facing change.
 
+## [0.17.4] - 2026-06-07
+
+### Security
+- API-key bearer credentials now take precedence over session cookies, preventing a browser session from widening a restricted API key's effective permissions.
+- CSRF protection now still applies to state-changing requests that carry a session cookie, even if they also include a `cpam_` bearer-style Authorization header.
+
 ## [0.17.3] - 2026-06-03
 
 ### Added

internal/api/csrf.go

@@ -37,8 +37,11 @@ func CSRFMiddleware() Middleware {
 				return
 			}
 
-			// For state-changing methods: skip CSRF check if using API key auth
-			if authHeader := r.Header.Get("Authorization"); authHeader != "" && strings.HasPrefix(authHeader, "Bearer cpam_") {
+			// For state-changing methods: skip CSRF only for API-key-style requests
+			// that are not also carrying a browser session cookie. Actual API key
+			// validity is enforced later by auth middleware; a forged bearer header
+			// must not disable CSRF for a cookie-authenticated browser request.
+			if authHeader := r.Header.Get("Authorization"); authHeader != "" && strings.HasPrefix(authHeader, "Bearer cpam_") && !hasSessionCookie(r) {
 				next.ServeHTTP(w, r)
 				return
 			}
@@ -71,3 +74,8 @@ func generateCSRFToken() string {
 	_, _ = rand.Read(b)
 	return hex.EncodeToString(b)
 }
+
+func hasSessionCookie(r *http.Request) bool {
+	cookie, err := r.Cookie("session")
+	return err == nil && strings.TrimSpace(cookie.Value) != ""
+}

internal/api/csrf_test.go

@@ -123,6 +123,22 @@ func TestCSRFMiddleware_POSTWithAPIKeyBypassesCSRF(t *testing.T) {
 	}
 }
 
+func TestCSRFMiddleware_POSTWithAPIKeyAndSessionRequiresCSRF(t *testing.T) {
+	handler := CSRFMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	rr := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/pools", nil)
+	req.Header.Set("Authorization", "Bearer cpam_forgedkey123abc")
+	req.AddCookie(&http.Cookie{Name: "session", Value: "browser-session"})
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusForbidden {
+		t.Fatalf("expected 403 when a bearer header is mixed with a session cookie and no CSRF token, got %d", rr.Code)
+	}
+}
+
 func TestCSRFMiddleware_POSTToLoginBypassesCSRF(t *testing.T) {
 	handler := CSRFMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)

internal/api/middleware.go

@@ -588,61 +588,18 @@ func DualAuthMiddleware(
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
 
-			// Strategy 1: Check session cookie
-			if cookie, err := r.Cookie("session"); err == nil && cookie.Value != "" {
-				session, err := sessionStore.Get(ctx, cookie.Value)
-				if err == nil && session != nil && session.IsValid() {
-					// Check if the user's account is still active (cached).
-					isActive, cacheHit := activeCache.check(session.UserID, activeStatusCacheTTL)
-					if !cacheHit {
-						// Cache miss or expired — fetch from DB.
-						user, _ := userStore.GetByID(ctx, session.UserID)
-						if user == nil {
-							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
-							return
-						}
-						isActive = user.IsActive
-						activeCache.set(session.UserID, isActive)
-						if !isActive {
-							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
-							return
-						}
-						ctx = auth.ContextWithSession(ctx, session)
-						ctx = auth.ContextWithRole(ctx, session.Role)
-						ctx = auth.ContextWithUser(ctx, user)
-					} else {
-						if !isActive {
-							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
-							return
-						}
-						ctx = auth.ContextWithSession(ctx, session)
-						ctx = auth.ContextWithRole(ctx, session.Role)
-						if user, _ := userStore.GetByID(ctx, session.UserID); user != nil {
-							ctx = auth.ContextWithUser(ctx, user)
-						}
-					}
-					r = r.WithContext(ctx)
-					next.ServeHTTP(w, r)
-					return
-				}
-			}
-
-			// Strategy 2: Check Authorization header
+			// Strategy 1: Check Authorization header. When a request supplies an API
+			// key and a session cookie, the API key is the active credential so its
+			// scopes cannot be widened by the browser session.
 			authHeader := r.Header.Get("Authorization")
 			if authHeader != "" && strings.HasPrefix(authHeader, "Bearer ") {
-				token := strings.TrimPrefix(authHeader, "Bearer ")
-				token = strings.TrimSpace(token)
+				token := strings.TrimSpace(strings.TrimPrefix(authHeader, "Bearer "))
 
 				if strings.HasPrefix(token, "cpam_") {
-					// Strategy 2: API key authentication
 					prefix, err := auth.ParseAPIKeyPrefix(token)
 					if err != nil {
-						if required {
-							logAuthFailure(logger, r, "invalid API key format")
-							writeJSON(w, http.StatusUnauthorized, apiError{Error: "unauthorized", Detail: "invalid API key format"})
-							return
-						}
-						next.ServeHTTP(w, r)
+						logAuthFailure(logger, r, "invalid API key format")
+						writeJSON(w, http.StatusUnauthorized, apiError{Error: "unauthorized", Detail: "invalid API key format"})
 						return
 					}
 
@@ -653,12 +610,8 @@ func DualAuthMiddleware(
 						return
 					}
 					if storedKey == nil {
-						if required {
-							logAuthFailure(logger, r, "API key not found")
-							writeJSON(w, http.StatusUnauthorized, apiError{Error: "unauthorized", Detail: "invalid API key"})
-							return
-						}
-						next.ServeHTTP(w, r)
+						logAuthFailure(logger, r, "API key not found")
+						writeJSON(w, http.StatusUnauthorized, apiError{Error: "unauthorized", Detail: "invalid API key"})
 						return
 					}
 
@@ -699,6 +652,45 @@ func DualAuthMiddleware(
 				}
 			}
 
+			// Strategy 2: Check session cookie
+			if cookie, err := r.Cookie("session"); err == nil && cookie.Value != "" {
+				session, err := sessionStore.Get(ctx, cookie.Value)
+				if err == nil && session != nil && session.IsValid() {
+					// Check if the user's account is still active (cached).
+					isActive, cacheHit := activeCache.check(session.UserID, activeStatusCacheTTL)
+					if !cacheHit {
+						// Cache miss or expired — fetch from DB.
+						user, _ := userStore.GetByID(ctx, session.UserID)
+						if user == nil {
+							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
+							return
+						}
+						isActive = user.IsActive
+						activeCache.set(session.UserID, isActive)
+						if !isActive {
+							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
+							return
+						}
+						ctx = auth.ContextWithSession(ctx, session)
+						ctx = auth.ContextWithRole(ctx, session.Role)
+						ctx = auth.ContextWithUser(ctx, user)
+					} else {
+						if !isActive {
+							writeJSON(w, http.StatusUnauthorized, apiError{Error: "account disabled"})
+							return
+						}
+						ctx = auth.ContextWithSession(ctx, session)
+						ctx = auth.ContextWithRole(ctx, session.Role)
+						if user, _ := userStore.GetByID(ctx, session.UserID); user != nil {
+							ctx = auth.ContextWithUser(ctx, user)
+						}
+					}
+					r = r.WithContext(ctx)
+					next.ServeHTTP(w, r)
+					return
+				}
+			}
+
 			if required {
 				logAuthFailure(logger, r, "missing authentication")
 				writeJSON(w, http.StatusUnauthorized, apiError{Error: "unauthorized", Detail: "missing authentication"})

internal/api/middleware_test.go

@@ -1837,6 +1837,61 @@ func TestDualAuth_ActiveUserAllowed(t *testing.T) {
 	}
 }
 
+func TestDualAuth_APIKeyTakesPrecedenceOverSessionCookie(t *testing.T) {
+	keyStore := auth.NewMemoryKeyStore()
+	sessionStore := auth.NewMemorySessionStore()
+	userStore := auth.NewMemoryUserStore()
+	ctx := context.Background()
+
+	user := &auth.User{
+		ID:       "u-mixed",
+		Username: "adminuser",
+		Role:     auth.RoleAdmin,
+		IsActive: true,
+	}
+	if err := userStore.Create(ctx, user); err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+	session := &auth.Session{
+		ID:        "sess-mixed",
+		UserID:    user.ID,
+		Role:      auth.RoleAdmin,
+		CreatedAt: time.Now(),
+		ExpiresAt: time.Now().Add(1 * time.Hour),
+	}
+	if err := sessionStore.Create(ctx, session); err != nil {
+		t.Fatalf("failed to create session: %v", err)
+	}
+
+	plaintext, apiKey, err := auth.GenerateAPIKey(auth.GenerateAPIKeyOptions{
+		Name:   "read-only key",
+		Scopes: []string{"pools:read"},
+	})
+	if err != nil {
+		t.Fatalf("failed to generate API key: %v", err)
+	}
+	if err := keyStore.Create(ctx, apiKey); err != nil {
+		t.Fatalf("failed to store API key: %v", err)
+	}
+
+	mw := DualAuthMiddleware(keyStore, sessionStore, userStore, true, newTestLogger())
+	wrapped := mw(RequirePermissionMiddleware(auth.ResourcePools, auth.ActionCreate, newTestLogger())(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			t.Fatal("handler should not be called when the API key lacks create scope")
+		}),
+	))
+
+	rr := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/pools", nil)
+	req.Header.Set("Authorization", "Bearer "+plaintext)
+	req.AddCookie(&http.Cookie{Name: "session", Value: session.ID})
+	wrapped.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusForbidden {
+		t.Fatalf("expected 403 from read-only API key despite admin session cookie, got %d: %s", rr.Code, rr.Body.String())
+	}
+}
+
 func TestLoginRateLimitMiddleware_AllowsBelowLimit(t *testing.T) {
 	loginRL := LoginRateLimitMiddleware(LoginRateLimitConfig{
 		AttemptsPerMinute: 5,