feat: Add more tests for sessions and edge cases (#4731)

Signed-off-by: maksim.nabokikh <max.nabokih@gmail.com>

server/handlers.go

@@ -449,8 +449,8 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 			if redirectURL != "" {
-				// Session found but consent required — no UI allowed.
-				s.redirectWithError(w, r, authReq, errInteractionRequired, "Consent required")
+				// Session found but user interaction is needed (consent or MFA) — no UI allowed.
+				s.redirectWithError(w, r, authReq, errInteractionRequired, "User interaction required")
 				return
 			}
 			return

server/handlers_test.go

@@ -908,6 +908,61 @@ func TestScopesCoveredByConsent(t *testing.T) {
 	}
 }
 
+// TestConsentSurvivesSessionDeletion verifies that UserIdentity.Consents
+// persists independently from AuthSession lifecycle (logout should not
+// clear consent decisions).
+func TestConsentSurvivesSessionDeletion(t *testing.T) {
+	ctx := t.Context()
+
+	httpServer, s := newTestServerWithSessions(t, nil)
+	defer httpServer.Close()
+
+	userID := "test-user"
+	connectorID := "mock"
+	clientID := "test-client"
+
+	// Create UserIdentity with existing consents.
+	require.NoError(t, s.storage.CreateUserIdentity(ctx, storage.UserIdentity{
+		UserID:      userID,
+		ConnectorID: connectorID,
+		Claims:      storage.Claims{UserID: userID, Username: "testuser"},
+		Consents:    map[string][]string{clientID: {"openid", "email", "profile"}},
+		CreatedAt:   time.Now(),
+		LastLogin:   time.Now(),
+	}))
+
+	// Create and then delete the session (simulating logout).
+	require.NoError(t, s.storage.CreateAuthSession(ctx, storage.AuthSession{
+		UserID: userID, ConnectorID: connectorID, Nonce: "nonce",
+		CreatedAt: time.Now(), LastActivity: time.Now(),
+	}))
+	require.NoError(t, s.storage.DeleteAuthSession(ctx, userID, connectorID))
+
+	// Session is gone.
+	_, err := s.storage.GetAuthSession(ctx, userID, connectorID)
+	require.ErrorIs(t, err, storage.ErrNotFound)
+
+	// Consent survives.
+	ui, err := s.storage.GetUserIdentity(ctx, userID, connectorID)
+	require.NoError(t, err)
+	require.Equal(t, []string{"openid", "email", "profile"}, ui.Consents[clientID],
+		"consent should survive session deletion")
+}
+
+// TestConsentIsolatedBetweenClients verifies that consent given for
+// client-A does not satisfy scope check for client-B.
+func TestConsentIsolatedBetweenClients(t *testing.T) {
+	approvedForA := map[string][]string{"client-a": {"openid", "email"}}
+
+	// client-b should not have consent.
+	require.False(t, scopesCoveredByConsent(approvedForA["client-b"], []string{"openid", "email"}),
+		"consent for client-a should not cover client-b")
+
+	// client-a should have consent.
+	require.True(t, scopesCoveredByConsent(approvedForA["client-a"], []string{"openid", "email"}),
+		"consent for client-a should cover client-a's requested scopes")
+}
+
 func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
 	ctx := t.Context()
 

server/home.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"crypto/subtle"
 	"errors"
 	"fmt"
 	"net/http"
@@ -24,7 +25,7 @@ func (s *Server) handleHome(w http.ResponseWriter, r *http.Request) {
 	if cookie, err := r.Cookie(s.sessionConfig.CookieName); err == nil && cookie.Value != "" {
 		if userID, connectorID, nonce, err := parseSessionCookie(cookie.Value, s.sessionConfig.CookieEncryptionKey); err == nil {
 			session, err := s.storage.GetAuthSession(ctx, userID, connectorID)
-			if err == nil && session.Nonce == nonce {
+			if err == nil && subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) == 1 {
 				data.LoggedIn = true
 				data.IPAddress = session.IPAddress
 				data.UserAgent = session.UserAgent

server/logout.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"crypto/subtle"
 	"errors"
 	"net/http"
 	"net/url"
@@ -94,7 +95,7 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 		if cookie, err := r.Cookie(s.sessionConfig.CookieName); err == nil && cookie.Value != "" {
 			if uid, cid, nonce, err := parseSessionCookie(cookie.Value, s.sessionConfig.CookieEncryptionKey); err == nil {
 				// Verify the session exists and nonce matches before trusting the cookie.
-				if session, err := s.storage.GetAuthSession(ctx, uid, cid); err == nil && session.Nonce == nonce {
+				if session, err := s.storage.GetAuthSession(ctx, uid, cid); err == nil && subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) == 1 {
 					userID = uid
 					connectorID = cid
 					s.logger.DebugContext(ctx, "logout: identified user from session cookie",
@@ -178,7 +179,7 @@ func (s *Server) handleLogoutCallback(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if session.Nonce != nonce {
+	if subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) != 1 {
 		s.renderError(r, w, http.StatusBadRequest, "Invalid session.")
 		return
 	}

server/logout_test.go

@@ -293,6 +293,62 @@ func TestDiscoveryWithoutSessions(t *testing.T) {
 	require.Empty(t, d.EndSession)
 }
 
+// TestHandleLogoutFromCookie tests logout without id_token_hint,
+// where the user is identified by their session cookie alone.
+func TestHandleLogoutFromCookie(t *testing.T) {
+	httpServer, server := newTestServerWithSessions(t, nil)
+	defer httpServer.Close()
+
+	ctx := t.Context()
+	userID := "test-user"
+	connectorID := "mock"
+	nonce := "testnonce"
+
+	require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{
+		UserID: userID, ConnectorID: connectorID, Nonce: nonce,
+		CreatedAt: time.Now(), LastActivity: time.Now(),
+	}))
+
+	rr := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/logout", nil)
+	req.AddCookie(&http.Cookie{
+		Name:  "dex_session",
+		Value: sessionCookieValue(userID, connectorID, nonce, server.sessionConfig.CookieEncryptionKey),
+	})
+	server.ServeHTTP(rr, req)
+
+	require.Equal(t, http.StatusOK, rr.Code)
+	require.Contains(t, rr.Body.String(), "successfully logged out")
+
+	// Session should be deleted.
+	_, err := server.storage.GetAuthSession(ctx, userID, connectorID)
+	require.ErrorIs(t, err, storage.ErrNotFound)
+
+	// Cookie should be cleared.
+	for _, c := range rr.Result().Cookies() {
+		if c.Name == "dex_session" {
+			require.Equal(t, -1, c.MaxAge)
+		}
+	}
+}
+
+// TestLogoutCallbackWithExpiredSession tests that /logout/callback
+// returns an error when the session has expired or been deleted.
+func TestLogoutCallbackWithExpiredSession(t *testing.T) {
+	httpServer, server := newTestServerWithSessions(t, nil)
+	defer httpServer.Close()
+
+	// No session created — cookie points to nonexistent session.
+	rr := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/logout/callback", nil)
+	req.AddCookie(&http.Cookie{
+		Name:  "dex_session",
+		Value: sessionCookieValue("user-1", "mock", "nonce", server.sessionConfig.CookieEncryptionKey),
+	})
+	server.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusBadRequest, rr.Code)
+}
+
 func TestRevokeRefreshTokensReturnsConnectorData(t *testing.T) {
 	httpServer, server := newTestServerWithSessions(t, nil)
 	defer httpServer.Close()

server/refreshhandlers_test.go

@@ -336,13 +336,11 @@ func TestRefreshTokenAuthTime(t *testing.T) {
 					"access token should not have auth_time when sessions are disabled")
 			}
 
-			// TODO: newIDToken in handleRefreshToken is currently called with time.Time{},
-			// so the ID token does not include auth_time. Once fixed, uncomment:
-			// if tc.wantAuthTime {
-			// 	idClaims := decodeJWTClaims(t, resp.IDToken)
-			// 	assert.Equal(t, float64(loginTime.Unix()), idClaims["auth_time"],
-			// 		"id token auth_time should match UserIdentity.LastLogin")
-			// }
+			if tc.wantAuthTime {
+				idClaims := decodeJWTClaims(t, resp.IDToken)
+				assert.Equal(t, float64(loginTime.Unix()), idClaims["auth_time"],
+					"id token auth_time should match UserIdentity.LastLogin")
+			}
 		})
 	}
 }

server/session.go

@@ -5,6 +5,7 @@ import (
 	"crypto/aes"
 	"crypto/cipher"
 	"crypto/rand"
+	"crypto/subtle"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -178,7 +179,9 @@ func (s *Server) getValidSession(ctx context.Context, w http.ResponseWriter, r *
 	}
 
 	// Verify nonce to prevent cookie forgery.
-	if session.Nonce != nonce {
+	// Use constant-time comparison to prevent timing attacks that could
+	// allow an attacker to recover the nonce byte-by-byte.
+	if subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) != 1 {
 		s.logger.DebugContext(ctx, "auth session nonce mismatch")
 		s.clearSessionCookie(w)
 		return nil