fix(auth): reject API keys minted before the owner's last password change

internal/auth/errors.go

@@ -143,4 +143,8 @@ var (
 	ErrAPIKeyExpiresAtRequired = errors.New("expires_at is required")
 	ErrAPIKeyExpiresAtTooFar   = errors.New("expires_at exceeds the maximum API key lifetime")
 	ErrAPIKeyExpiresAtInPast   = errors.New("expires_at must be in the future")
+
+	// ErrAPIKeyPasswordRotated: the owner's password changed after the key was
+	// minted (issue #402).
+	ErrAPIKeyPasswordRotated = errors.New("API key predates the owner's last password change")
 )

internal/auth/service_apikeys.go

@@ -123,6 +123,9 @@ func (s *Service) CreateAPIKey(ctx context.Context, userID, name, password strin
 		CreatedAt:   now,
 		LastUsedAt:  nil,
 		IsActive:    true,
+		// The version of the row whose password was just verified, so a
+		// rotation that commits before this insert still invalidates the key.
+		PasswordVersion: user.PasswordVersion,
 	}
 
 	// Store the API key
@@ -396,6 +399,9 @@ func (s *Service) ValidateUserAPIKey(ctx context.Context, apiKey string) (*UserA
 	if err != nil {
 		return nil, nil, err
 	}
+	if key.PasswordVersion != user.PasswordVersion {
+		return nil, nil, ErrAPIKeyPasswordRotated
+	}
 
 	return key, user, nil
 }

internal/auth/service_apikeys_rotation_db_test.go

@@ -0,0 +1,104 @@
+//go:build integration
+
+package auth
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	rotationPassword    = "SyntheticPassword123!"
+	rotationNewPassword = "RotatedPassword456#"
+)
+
+// failingKeyListStore makes the best-effort revocation scan after a password
+// change fail, as a transient database error would.
+type failingKeyListStore struct{ *PostgresStore }
+
+func (failingKeyListStore) ListAPIKeysByUser(context.Context, string) ([]*UserAPIKey, error) {
+	return nil, errors.New("synthetic list failure")
+}
+
+// userReadHook runs onRead once, right after the first GetUserByID returns.
+type userReadHook struct {
+	*PostgresStore
+	onRead func()
+}
+
+func (s *userReadHook) GetUserByID(ctx context.Context, id string) (*User, error) {
+	user, err := s.PostgresStore.GetUserByID(ctx, id)
+	if hook := s.onRead; hook != nil {
+		s.onRead = nil
+		hook()
+	}
+	return user, err
+}
+
+func newRotationService(store StoreInterface) *Service {
+	svc := NewService(ServiceConfig{Store: store})
+	svc.bcryptCostOverride = 4
+	return svc
+}
+
+func createRotationUser(t *testing.T, svc *Service, store *PostgresStore, email string) *User {
+	t.Helper()
+	hash, err := svc.hashPassword(rotationPassword)
+	require.NoError(t, err)
+	user := &User{Email: email, PasswordHash: hash, Active: true, GroupIDs: []string{DefaultPurchaserGroupID}}
+	require.NoError(t, store.CreateUser(t.Context(), user))
+	return user
+}
+
+func TestIntegration_APIKeyRejectedAfterPasswordRotation(t *testing.T) {
+	store := NewPostgresStore(setupAuthTestDB(t))
+	svc := newRotationService(store)
+	ctx := t.Context()
+	expiry := time.Now().Add(time.Hour)
+	perms := []Permission{{Action: ActionView, Resource: ResourceRecommendations}}
+	change := ChangePasswordRequest{CurrentPassword: rotationPassword, NewPassword: rotationNewPassword}
+
+	t.Run("revocation-scan-fails", func(t *testing.T) {
+		user := createRotationUser(t, svc, store, "rotation-scan-fails@example.com")
+		apiKey, _, err := svc.CreateAPIKey(ctx, user.ID, "ci", rotationPassword, perms, &expiry)
+		require.NoError(t, err)
+		require.NoError(t, newRotationService(failingKeyListStore{store}).ChangePassword(ctx, user.ID, change))
+
+		_, _, err = svc.ValidateUserAPIKey(ctx, apiKey)
+		require.ErrorIs(t, err, ErrAPIKeyPasswordRotated)
+	})
+
+	t.Run("key-minted-across-rotation", func(t *testing.T) {
+		user := createRotationUser(t, svc, store, "rotation-mint-race@example.com")
+		racing := &userReadHook{PostgresStore: store, onRead: func() {
+			require.NoError(t, svc.ChangePassword(ctx, user.ID, change))
+		}}
+		apiKey, _, err := newRotationService(racing).CreateAPIKey(ctx, user.ID, "ci", rotationPassword, perms, &expiry)
+		require.NoError(t, err, "the mint verified the password it read before the rotation")
+
+		_, _, err = svc.ValidateUserAPIKey(ctx, apiKey)
+		require.ErrorIs(t, err, ErrAPIKeyPasswordRotated)
+	})
+
+	t.Run("key-minted-after-rotation", func(t *testing.T) {
+		user := createRotationUser(t, svc, store, "rotation-fresh-key@example.com")
+		before, _, err := svc.CreateAPIKey(ctx, user.ID, "old", rotationPassword, perms, &expiry)
+		require.NoError(t, err)
+		require.NoError(t, svc.UpdateUserProfile(ctx, user.ID, "rotation-fresh-key-renamed@example.com", rotationPassword, ""))
+		_, _, err = svc.ValidateUserAPIKey(ctx, before)
+		require.NoError(t, err, "a write that keeps the password must not invalidate keys")
+
+		require.NoError(t, svc.ChangePassword(ctx, user.ID, change))
+		after, _, err := svc.CreateAPIKey(ctx, user.ID, "new", rotationNewPassword, perms, &expiry)
+		require.NoError(t, err)
+		key, owner, err := svc.ValidateUserAPIKey(ctx, after)
+		require.NoError(t, err)
+		assert.Equal(t, user.ID, owner.ID)
+		assert.Equal(t, "new", key.Name)
+	})
+}

internal/auth/service_apikeys_test.go

@@ -720,6 +720,27 @@ func TestService_ValidateUserAPIKey(t *testing.T) {
 		mockStore.AssertExpectations(t)
 	})
 
+	t.Run("fail when owner's password changed after minting", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := &Service{store: mockStore}
+
+		apiKey := "test-api-key-123456"
+		hash := sha256.Sum256([]byte(apiKey))
+		keyHash := base64.RawURLEncoding.EncodeToString(hash[:])
+
+		mockStore.On("GetAPIKeyByHash", ctx, keyHash).Return(&UserAPIKey{
+			ID: "key-1", UserID: "user-123", KeyHash: keyHash, IsActive: true, PasswordVersion: 2,
+		}, nil)
+		mockStore.On("GetUserByID", ctx, "user-123").Return(&User{ID: "user-123", Active: true, PasswordVersion: 3}, nil)
+
+		resultKey, resultUser, err := service.ValidateUserAPIKey(ctx, apiKey)
+
+		require.ErrorIs(t, err, ErrAPIKeyPasswordRotated)
+		assert.Nil(t, resultUser)
+		assert.Nil(t, resultKey)
+		mockStore.AssertExpectations(t)
+	})
+
 	t.Run("fail when API key not found", func(t *testing.T) {
 		mockStore := new(MockStore)
 		service := &Service{store: mockStore}

internal/auth/service_credentials_db_test.go

@@ -168,6 +168,7 @@ func TestIntegration_CredentialWritesPreserveConcurrentMFA(t *testing.T) {
 		assert.Equal(t, []string{f.user.PasswordHash}, stored.PasswordHistory)
 		assertOnlyChanged(t, enrolled(), stored, func(w *User) {
 			w.Email, w.PasswordHash, w.Salt, w.PasswordHistory = "profile-both-new@example.com", stored.PasswordHash, "", stored.PasswordHistory
+			w.PasswordVersion++
 		})
 		_, err := f.svc.ValidateSession(ctx, f.session)
 		require.Error(t, err, "a password change must revoke existing sessions")
@@ -185,6 +186,7 @@ func TestIntegration_CredentialWritesPreserveConcurrentMFA(t *testing.T) {
 		assert.Equal(t, []string{f.user.PasswordHash}, stored.PasswordHistory)
 		assertOnlyChanged(t, enrolled(), stored, func(w *User) {
 			w.PasswordHash, w.Salt, w.PasswordHistory = stored.PasswordHash, "", stored.PasswordHistory
+			w.PasswordVersion++
 		})
 		_, err := f.svc.ValidateSession(ctx, f.session)
 		require.Error(t, err, "a password change must revoke existing sessions")

internal/auth/store_postgres.go

@@ -48,7 +48,7 @@ func (s *PostgresStore) GetUserByID(ctx context.Context, userID string) (*User,
 		       mfa_enabled, mfa_secret, mfa_pending_secret, mfa_pending_secret_expires_at,
 		       mfa_recovery_codes, password_reset_token, password_reset_expiry,
 		       failed_login_attempts, locked_until, password_history,
-		       created_at, updated_at, last_login_at, deactivated_at
+		       created_at, updated_at, last_login_at, deactivated_at, password_version
 		FROM users
 		WHERE id = $1
 	`
@@ -63,7 +63,7 @@ func (s *PostgresStore) GetUserByEmail(ctx context.Context, email string) (*User
 		       mfa_enabled, mfa_secret, mfa_pending_secret, mfa_pending_secret_expires_at,
 		       mfa_recovery_codes, password_reset_token, password_reset_expiry,
 		       failed_login_attempts, locked_until, password_history,
-		       created_at, updated_at, last_login_at, deactivated_at
+		       created_at, updated_at, last_login_at, deactivated_at, password_version
 		FROM users
 		WHERE email = $1
 	`
@@ -277,7 +277,7 @@ func (s *PostgresStore) ListUsers(ctx context.Context) ([]User, error) {
 		       mfa_enabled, mfa_secret, mfa_pending_secret, mfa_pending_secret_expires_at,
 		       mfa_recovery_codes, password_reset_token, password_reset_expiry,
 		       failed_login_attempts, locked_until, password_history,
-		       created_at, updated_at, last_login_at, deactivated_at
+		       created_at, updated_at, last_login_at, deactivated_at, password_version
 		FROM users
 		ORDER BY created_at DESC
 		LIMIT 10000
@@ -316,7 +316,7 @@ func (s *PostgresStore) GetUserByResetToken(ctx context.Context, token string) (
 		       mfa_enabled, mfa_secret, mfa_pending_secret, mfa_pending_secret_expires_at,
 		       mfa_recovery_codes, password_reset_token, password_reset_expiry,
 		       failed_login_attempts, locked_until, password_history,
-		       created_at, updated_at, last_login_at, deactivated_at
+		       created_at, updated_at, last_login_at, deactivated_at, password_version
 		FROM users
 		WHERE password_reset_token = $1
 	`
@@ -777,6 +777,7 @@ func (s *PostgresStore) scanUser(scanner Scanner) (*User, error) {
 		&user.UpdatedAt,
 		&lastLoginAt,
 		&deactivatedAt,
+		&user.PasswordVersion,
 	)
 
 	if err != nil {

internal/auth/store_postgres_apikeys.go

@@ -39,8 +39,8 @@ func (s *PostgresStore) CreateAPIKey(ctx context.Context, key *UserAPIKey) error
 	query := `
 		INSERT INTO api_keys (
 			id, user_id, name, key_prefix, key_hash, permissions,
-			is_active, expires_at, created_at, last_used_at
-		) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
+			is_active, expires_at, created_at, last_used_at, password_version
+		) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
 	`
 
 	_, err = s.db.Exec(ctx, query,
@@ -54,6 +54,7 @@ func (s *PostgresStore) CreateAPIKey(ctx context.Context, key *UserAPIKey) error
 		key.ExpiresAt,
 		key.CreatedAt,
 		key.LastUsedAt,
+		key.PasswordVersion,
 	)
 
 	if err != nil {
@@ -68,7 +69,8 @@ func (s *PostgresStore) GetAPIKeyByID(ctx context.Context, keyID string) (*UserA
 	query := `
 		SELECT id, user_id, name, key_prefix, key_hash, permissions,
 		       is_active, expires_at, created_at, last_used_at,
-		       request_count_total, request_count_window, request_count_window_start
+		       request_count_total, request_count_window, request_count_window_start,
+		       password_version
 		FROM api_keys
 		WHERE id = $1
 	`
@@ -81,7 +83,8 @@ func (s *PostgresStore) GetAPIKeyByHash(ctx context.Context, keyHash string) (*U
 	query := `
 		SELECT id, user_id, name, key_prefix, key_hash, permissions,
 		       is_active, expires_at, created_at, last_used_at,
-		       request_count_total, request_count_window, request_count_window_start
+		       request_count_total, request_count_window, request_count_window_start,
+		       password_version
 		FROM api_keys
 		WHERE key_hash = $1 AND is_active = true
 		  AND (expires_at IS NULL OR expires_at > NOW())
@@ -95,7 +98,8 @@ func (s *PostgresStore) ListAPIKeysByUser(ctx context.Context, userID string) ([
 	query := `
 		SELECT id, user_id, name, key_prefix, key_hash, permissions,
 		       is_active, expires_at, created_at, last_used_at,
-		       request_count_total, request_count_window, request_count_window_start
+		       request_count_total, request_count_window, request_count_window_start,
+		       password_version
 		FROM api_keys
 		WHERE user_id = $1
 		ORDER BY created_at DESC
@@ -274,6 +278,7 @@ func (s *PostgresStore) scanAPIKey(scanner Scanner) (*UserAPIKey, error) {
 		&key.RequestCountTotal,
 		&key.RequestCountWindow,
 		&windowStart,
+		&key.PasswordVersion,
 	)
 
 	if err != nil {

internal/auth/store_postgres_pgxmock_test.go

@@ -319,7 +319,7 @@ var userColumns = []string{
 	"mfa_enabled", "mfa_secret", "mfa_pending_secret", "mfa_pending_secret_expires_at",
 	"mfa_recovery_codes", "password_reset_token", "password_reset_expiry",
 	"failed_login_attempts", "locked_until", "password_history",
-	"created_at", "updated_at", "last_login_at", "deactivated_at",
+	"created_at", "updated_at", "last_login_at", "deactivated_at", "password_version",
 }
 
 func TestPGXMock_ListUsers_Success(t *testing.T) {
@@ -337,15 +337,15 @@ func TestPGXMock_ListUsers_Success(t *testing.T) {
 			true, "mfa-secret", nil, nil,
 			[]string{"code1"}, nil, nil,
 			0, nil, []string{"old-hash"},
-			created, created, lastLogin, nil,
+			created, created, lastLogin, nil, int64(4),
 		).
 		AddRow(
 			"user-2", "user@example.com", "hash2", "salt2",
 			[]string{}, false,
 			false, nil, nil, nil,
 			[]string{}, nil, nil,
 			2, nil, []string{},
-			created, created, nil, deactivated,
+			created, created, nil, deactivated, int64(0),
 		)
 
 	mock.ExpectQuery(`(?s)SELECT id, email, password_hash, salt, group_ids, active,.*FROM users\s+ORDER BY created_at DESC\s+LIMIT 10000`).
@@ -364,6 +364,7 @@ func TestPGXMock_ListUsers_Success(t *testing.T) {
 	assert.Equal(t, lastLogin, *users[0].LastLoginAt)
 
 	assert.Nil(t, users[0].DeactivatedAt)
+	assert.Equal(t, int64(4), users[0].PasswordVersion)
 
 	assert.Equal(t, "user-2", users[1].ID)
 	assert.False(t, users[1].Active)
@@ -412,7 +413,7 @@ func TestPGXMock_ListUsers_RowError(t *testing.T) {
 			false, nil, nil, nil,
 			[]string{}, nil, nil,
 			0, nil, []string{},
-			created, created, nil, nil,
+			created, created, nil, nil, int64(0),
 		).
 		RowError(0, errors.New("connection reset mid-iteration"))
 
@@ -429,6 +430,7 @@ var apiKeyColumns = []string{
 	"id", "user_id", "name", "key_prefix", "key_hash", "permissions",
 	"is_active", "expires_at", "created_at", "last_used_at",
 	"request_count_total", "request_count_window", "request_count_window_start",
+	"password_version",
 }
 
 func TestPGXMock_ListAPIKeysByUser_Success(t *testing.T) {
@@ -444,12 +446,12 @@ func TestPGXMock_ListAPIKeysByUser_Success(t *testing.T) {
 		AddRow(
 			"key-1", "user-1", "ci key", "cudly_ab", "hash-1",
 			[]byte(`[{"action":"view","resource":"recommendations"}]`), true, expires, created, lastUsed,
-			int64(42), int64(3), windowStart,
+			int64(42), int64(3), windowStart, int64(4),
 		).
 		AddRow(
 			"key-2", "user-1", "old key", "cudly_cd", "hash-2",
 			[]byte(`[]`), false, nil, created, nil,
-			int64(0), int64(0), nil,
+			int64(0), int64(0), nil, int64(0),
 		)
 
 	mock.ExpectQuery(`(?s)SELECT id, user_id, name, key_prefix, key_hash, permissions,.*FROM api_keys\s+WHERE user_id = \$1\s+ORDER BY created_at DESC`).
@@ -470,6 +472,7 @@ func TestPGXMock_ListAPIKeysByUser_Success(t *testing.T) {
 	assert.Equal(t, lastUsed, *keys[0].LastUsedAt)
 	assert.Equal(t, int64(42), keys[0].RequestCountTotal)
 	assert.Equal(t, int64(3), keys[0].RequestCountWindow)
+	assert.Equal(t, int64(4), keys[0].PasswordVersion)
 	require.NotNil(t, keys[0].RequestCountWindowStart)
 	assert.Equal(t, windowStart, *keys[0].RequestCountWindowStart)
 

internal/auth/types.go

@@ -30,6 +30,9 @@ type User struct {
 	FailedLoginAttempts int        `json:"-" dynamodbav:"FailedLoginAttempts,omitempty"`
 	Active              bool       `json:"active" dynamodbav:"Active"`
 	MFAEnabled          bool       `json:"mfa_enabled" dynamodbav:"MFAEnabled"`
+	// PasswordVersion is bumped by a database trigger on every password_hash
+	// change; API keys minted under an older version are rejected (#402).
+	PasswordVersion int64 `json:"-" dynamodbav:"PasswordVersion"`
 }
 
 // Group represents a permission group.
@@ -85,6 +88,8 @@ type UserAPIKey struct {
 	KeyHash     string       `json:"-" dynamodbav:"KeyHash"`
 	Permissions []Permission `json:"permissions,omitempty" dynamodbav:"Permissions"`
 	IsActive    bool         `json:"is_active" dynamodbav:"IsActive"`
+	// PasswordVersion is the owner's User.PasswordVersion read when the key was minted.
+	PasswordVersion int64 `json:"-" dynamodbav:"PasswordVersion"`
 	// Usage counters (issue #340/#344 deferred sub-task -- migration 000094).
 	// Both default to 0 for legacy rows. RequestCountWindow is a
 	// FIXED/TUMBLING window count, not a true trailing-24h rolling count:

internal/database/postgres/migrations/000103_api_key_password_version.down.sql

@@ -0,0 +1,5 @@
+-- 000103 down: drop the password-version binding of API keys.
+DROP TRIGGER IF EXISTS users_bump_password_version ON users;
+DROP FUNCTION IF EXISTS bump_user_password_version();
+ALTER TABLE api_keys DROP COLUMN IF EXISTS password_version;
+ALTER TABLE users DROP COLUMN IF EXISTS password_version;