sec(auth): constant-time token validation via subtle.ConstantTimeCompare (closes #392)

internal/auth/service.go

@@ -319,6 +319,13 @@ func (s *Service) ValidateSession(ctx context.Context, token string) (*Session,
 		return nil, fmt.Errorf("session not found")
 	}
 
+	// Constant-time comparison after fetch to close the SQL-equality timing oracle.
+	// GetSession does `WHERE token = $1` which is not constant-time in PostgreSQL;
+	// align with ValidateUserAPIKey / validateResetToken (issue #392 PR #837).
+	if subtle.ConstantTimeCompare([]byte(session.Token), []byte(hashedToken)) != 1 {
+		return nil, fmt.Errorf("session not found")
+	}
+
 	if session.ExpiresAt.IsZero() {
 		return nil, fmt.Errorf("session has no expiry: data integrity error")
 	}

internal/auth/service_apikeys.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/rand"
 	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -265,6 +266,13 @@ func (s *Service) ValidateUserAPIKey(ctx context.Context, apiKey string) (*UserA
 		return nil, nil, fmt.Errorf("invalid API key")
 	}
 
+	// Constant-time comparison after fetch to close the SQL-equality timing oracle.
+	// The DB lookup already found the row by hash; this redundant check ensures the
+	// hot path does not leak timing information through early return differences.
+	if subtle.ConstantTimeCompare([]byte(key.KeyHash), []byte(keyHash)) != 1 {
+		return nil, nil, fmt.Errorf("invalid API key")
+	}
+
 	err = validateAPIKeyStatus(key)
 	if err != nil {
 		return nil, nil, err

internal/auth/service_apikeys_test.go

@@ -651,6 +651,34 @@ func TestService_ValidateUserAPIKey(t *testing.T) {
 		assert.Nil(t, resultKey)
 		mockStore.AssertExpectations(t)
 	})
+
+	t.Run("reject when stored hash does not match derived hash", func(t *testing.T) {
+		// Simulates a DB row whose key_hash field was tampered or
+		// belongs to a different key -- the constant-time check must reject it.
+		mockStore := new(MockStore)
+		service := &Service{store: mockStore}
+
+		apiKey := "test-api-key-123456"
+		hash := sha256.Sum256([]byte(apiKey))
+		keyHash := base64.RawURLEncoding.EncodeToString(hash[:])
+
+		tamperedRecord := &UserAPIKey{
+			ID:       "key-1",
+			UserID:   "user-123",
+			KeyHash:  keyHash + "tampered",
+			IsActive: true,
+		}
+
+		mockStore.On("GetAPIKeyByHash", ctx, keyHash).Return(tamperedRecord, nil)
+
+		resultKey, resultUser, err := service.ValidateUserAPIKey(ctx, apiKey)
+
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid API key")
+		assert.Nil(t, resultUser)
+		assert.Nil(t, resultKey)
+		mockStore.AssertExpectations(t)
+	})
 }
 
 func TestService_UpdateLastUsed(t *testing.T) {

internal/auth/service_password.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"context"
+	"crypto/subtle"
 	"errors"
 	"fmt"
 	"strings"
@@ -419,6 +420,11 @@ func (s *Service) ResetTokenStatus(ctx context.Context, token string) (ResetToke
 		return ResetTokenStateUsed, ResetTokenFlowReset, nil
 	}
 
+	// Constant-time comparison after fetch to close the SQL-equality timing oracle.
+	if subtle.ConstantTimeCompare([]byte(user.PasswordResetToken), []byte(tokenHash)) != 1 {
+		return ResetTokenStateUsed, ResetTokenFlowReset, nil
+	}
+
 	if user.PasswordResetExpiry == nil || time.Now().After(*user.PasswordResetExpiry) {
 		flow := ResetTokenFlowReset
 		if !user.Active {
@@ -448,6 +454,11 @@ func (s *Service) validateResetToken(ctx context.Context, token string) (*User,
 		return nil, fmt.Errorf("invalid or expired reset token")
 	}
 
+	// Constant-time comparison after fetch to close the SQL-equality timing oracle.
+	if subtle.ConstantTimeCompare([]byte(user.PasswordResetToken), []byte(tokenHash)) != 1 {
+		return nil, fmt.Errorf("invalid or expired reset token")
+	}
+
 	if user.PasswordResetExpiry == nil || time.Now().After(*user.PasswordResetExpiry) {
 		return nil, fmt.Errorf("reset token has expired")
 	}

internal/auth/service_password_test.go

@@ -309,7 +309,7 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 		testUser := &User{
 			ID:                  "user-123",
 			Email:               "test@example.com",
-			PasswordResetToken:  "hashed-token", // Now stores the hash
+			PasswordResetToken:  hashSessionToken("valid-reset-token"),
 			PasswordResetExpiry: &expiry,
 			Active:              true,
 		}
@@ -360,7 +360,7 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 		expiredUser := &User{
 			ID:                  "user-456",
 			Email:               "expired@example.com",
-			PasswordResetToken:  "hashed-expired-token",
+			PasswordResetToken:  hashSessionToken("expired-reset-token"),
 			PasswordResetExpiry: &expiry,
 			Active:              true,
 		}
@@ -389,7 +389,7 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 		testUser := &User{
 			ID:                  "user-123",
 			Email:               "test@example.com",
-			PasswordResetToken:  "hashed-token",
+			PasswordResetToken:  hashSessionToken("valid-reset-token"),
 			PasswordResetExpiry: &expiry,
 			Active:              true,
 		}
@@ -422,7 +422,7 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 		testUser := &User{
 			ID:                  "user-123",
 			Email:               "test@example.com",
-			PasswordResetToken:  "hashed-token",
+			PasswordResetToken:  hashSessionToken("valid-reset-token"),
 			PasswordResetExpiry: &expiry,
 			PasswordHistory:     []string{hash},
 			Active:              true,
@@ -462,7 +462,7 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 			ID:                  "user-123",
 			Email:               "test@example.com",
 			PasswordHash:        currentHash,
-			PasswordResetToken:  "hashed-token",
+			PasswordResetToken:  hashSessionToken("valid-reset-token"),
 			PasswordResetExpiry: &expiry,
 			Active:              true,
 		}
@@ -483,6 +483,37 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 
 		mockStore.AssertExpectations(t)
 	})
+
+	t.Run("constant-time mismatch rejects token even when DB row exists", func(t *testing.T) {
+		// The DB returned a user but the stored reset-token hash does not match
+		// the hash we derived from the supplied token string. The constant-time
+		// guard must reject the request to avoid timing-based disclosure.
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+
+		expiry := time.Now().Add(time.Hour)
+		mismatchUser := &User{
+			ID:                  "user-999",
+			Email:               "mismatch@example.com",
+			PasswordResetToken:  "completely-different-hash",
+			PasswordResetExpiry: &expiry,
+			Active:              true,
+		}
+
+		mockStore.On("GetUserByResetToken", ctx, mock.AnythingOfType("string")).Return(mismatchUser, nil).Once()
+
+		req := PasswordResetConfirm{
+			Token:       "some-token",
+			NewPassword: "SecureT3st@789",
+		}
+
+		err := service.ConfirmPasswordReset(ctx, req)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid or expired reset token")
+
+		mockStore.AssertExpectations(t)
+	})
 }
 
 // TestService_ResetTokenStatus covers the read-only token-status probe
@@ -498,7 +529,7 @@ func TestService_ResetTokenStatus(t *testing.T) {
 
 		expiry := time.Now().Add(time.Hour)
 		mockStore.On("GetUserByResetToken", ctx, mock.AnythingOfType("string")).
-			Return(&User{ID: "u1", Active: true, PasswordResetExpiry: &expiry}, nil).Once()
+			Return(&User{ID: "u1", Active: true, PasswordResetToken: hashSessionToken("valid-token"), PasswordResetExpiry: &expiry}, nil).Once()
 
 		state, flow, err := service.ResetTokenStatus(ctx, "valid-token")
 		require.NoError(t, err)
@@ -515,7 +546,7 @@ func TestService_ResetTokenStatus(t *testing.T) {
 
 		expiry := time.Now().Add(time.Hour)
 		mockStore.On("GetUserByResetToken", ctx, mock.AnythingOfType("string")).
-			Return(&User{ID: "u2", Active: false, PasswordResetExpiry: &expiry}, nil).Once()
+			Return(&User{ID: "u2", Active: false, PasswordResetToken: hashSessionToken("valid-invite-token"), PasswordResetExpiry: &expiry}, nil).Once()
 
 		state, flow, err := service.ResetTokenStatus(ctx, "valid-invite-token")
 		require.NoError(t, err)
@@ -532,7 +563,7 @@ func TestService_ResetTokenStatus(t *testing.T) {
 
 		expiry := time.Now().Add(-time.Hour)
 		mockStore.On("GetUserByResetToken", ctx, mock.AnythingOfType("string")).
-			Return(&User{ID: "u3", Active: true, PasswordResetExpiry: &expiry}, nil).Once()
+			Return(&User{ID: "u3", Active: true, PasswordResetToken: hashSessionToken("expired-token"), PasswordResetExpiry: &expiry}, nil).Once()
 
 		state, flow, err := service.ResetTokenStatus(ctx, "expired-token")
 		require.NoError(t, err)
@@ -589,6 +620,32 @@ func TestService_ResetTokenStatus(t *testing.T) {
 
 		mockStore.AssertExpectations(t)
 	})
+
+	t.Run("constant-time mismatch returns used even if DB row exists", func(t *testing.T) {
+		// The DB returned a row but the stored token hash does not match the
+		// hash derived from the supplied token. The constant-time check must
+		// treat this as a missing/consumed token rather than branching on
+		// expiry state, to avoid leaking information through timing.
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+
+		expiry := time.Now().Add(time.Hour)
+		mockStore.On("GetUserByResetToken", ctx, mock.AnythingOfType("string")).
+			Return(&User{
+				ID:                  "u9",
+				Active:              true,
+				PasswordResetToken:  "different-hash-entirely",
+				PasswordResetExpiry: &expiry,
+			}, nil).Once()
+
+		state, flow, err := service.ResetTokenStatus(ctx, "some-token")
+		require.NoError(t, err)
+		assert.Equal(t, ResetTokenStateUsed, state)
+		assert.Equal(t, ResetTokenFlowReset, flow)
+
+		mockStore.AssertExpectations(t)
+	})
 }
 
 // Test password validation rules.

internal/auth/service_security_test.go

@@ -244,6 +244,7 @@ func TestValidateUserAPIKey_LastUsedSingleflight(t *testing.T) {
 		ID:        keyID,
 		UserID:    user.ID,
 		IsActive:  true,
+		KeyHash:   keyHash,
 		KeyPrefix: rawKey[:8],
 	}
 

internal/auth/service_test.go

@@ -190,6 +190,34 @@ func TestService_ValidateSession(t *testing.T) {
 
 		mockStore.AssertExpectations(t)
 	})
+
+	t.Run("constant-time mismatch rejects session even when DB row exists", func(t *testing.T) {
+		// The DB returned a session row but its stored token hash does not
+		// match the hash we derived from the supplied token string. The
+		// constant-time guard (added alongside the API-key and reset-token
+		// equivalents in PR #837) must reject the request to avoid leaking
+		// timing information through the SQL equality oracle.
+		mockStore := new(MockStore)
+		mockEmail := new(MockEmailSender)
+		service := createTestService(mockStore, mockEmail)
+
+		hashedToken := hashSessionToken("some-token")
+		mismatchSession := &Session{
+			Token:     "completely-different-hash",
+			UserID:    "user-999",
+			Email:     "mismatch@example.com",
+			ExpiresAt: time.Now().Add(time.Hour),
+		}
+
+		mockStore.On("GetSession", ctx, hashedToken).Return(mismatchSession, nil).Once()
+
+		session, err := service.ValidateSession(ctx, "some-token")
+		assert.Error(t, err)
+		assert.Nil(t, session)
+		assert.Contains(t, err.Error(), "session not found")
+
+		mockStore.AssertExpectations(t)
+	})
 }
 
 func TestService_Logout(t *testing.T) {
@@ -631,7 +659,7 @@ func TestService_ErrorPaths(t *testing.T) {
 		testUser := &User{
 			ID:                  "user-123",
 			Email:               "test@example.com",
-			PasswordResetToken:  "hashed-token",
+			PasswordResetToken:  hashSessionToken("valid-reset-token"),
 			PasswordResetExpiry: &expiry,
 			Active:              true,
 		}

internal/server/adapter_test.go

@@ -2,6 +2,8 @@ package server
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"testing"
 	"time"
 
@@ -12,6 +14,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// hashSessionTokenForTest mirrors auth.hashSessionToken (private). Session
+// tokens are stored as sha256+hex so the constant-time guard in
+// auth.Service.ValidateSession matches when both sides are derived from the
+// same raw token.
+func hashSessionTokenForTest(token string) string {
+	h := sha256.Sum256([]byte(token))
+	return hex.EncodeToString(h[:])
+}
+
 func createMockAuthService(t *testing.T) (*authServiceAdapter, *auth.MockStore) {
 	t.Helper()
 	mockStore := new(auth.MockStore)
@@ -62,9 +73,10 @@ func TestAuthServiceAdapter_ValidateSession(t *testing.T) {
 	adapter, mockStore := createMockAuthService(t)
 	ctx := context.Background()
 
-	// Token gets hashed, use mock.Anything
+	// Token gets hashed; the stored Token must match hashSessionToken("valid-token")
+	// so the constant-time guard in ValidateSession (PR #837 follow-up) succeeds.
 	mockStore.On("GetSession", ctx, mock.AnythingOfType("string")).Return(&auth.Session{
-		Token:     "hashed-token",
+		Token:     hashSessionTokenForTest("valid-token"),
 		UserID:    "user-1",
 		Email:     "test@example.com",
 		ExpiresAt: time.Now().Add(time.Hour),
@@ -235,9 +247,10 @@ func TestAuthServiceAdapter_ValidateCSRFToken(t *testing.T) {
 	adapter, mockStore := createMockAuthService(t)
 	ctx := context.Background()
 
-	// Token gets hashed, so use mock.Anything
+	// Token gets hashed; the stored Token must match hashSessionToken("session-token")
+	// so the constant-time guard in ValidateSession (PR #837 follow-up) succeeds.
 	mockStore.On("GetSession", ctx, mock.AnythingOfType("string")).Return(&auth.Session{
-		Token:     "hashed-session-token",
+		Token:     hashSessionTokenForTest("session-token"),
 		UserID:    "user-1",
 		CSRFToken: "csrf-token",
 		ExpiresAt: time.Now().Add(time.Hour),