From cad1a4b6a7e8d036e959fc970bda5239119ad95c Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Tue, 20 Oct 2026 14:22:43 +0100
Subject: [PATCH 1/2] fix(auth): take the admin invariant lock before the row
 on demote, deactivate and delete

UpdateUser, UpdateUserAdminFields and DeleteUser now run in a transaction
whose first statement takes adminInvariantAdvisoryLockKey, the same point
CreateAdminIfNone takes it, so every write that can lower the active-admin
count orders the advisory lock before the users row lock. Before, these
writes locked the row first and only reached the lock in the deferred
min-one-admin trigger at commit. The trigger already kept the floor, so
this is defense in depth: a mix of the two orders would deadlock, and the
new real-Postgres deadlock test catches a partial conversion (40P01 in
round 0).

Closes #63

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/auth/store_postgres.go               | 127 ++++---
 .../auth/store_postgres_admin_lock_db_test.go | 349 ++++++++++++++++++
 .../auth/store_postgres_admin_lock_test.go    | 239 ++++++++++++
 internal/auth/store_postgres_credentials.go   |  29 +-
 internal/auth/store_postgres_test.go          | 110 ------
 5 files changed, 689 insertions(+), 165 deletions(-)
 create mode 100644 internal/auth/store_postgres_admin_lock_db_test.go
 create mode 100644 internal/auth/store_postgres_admin_lock_test.go

diff --git a/internal/auth/store_postgres.go b/internal/auth/store_postgres.go
index 052ac8131..1253f4b75 100644
--- a/internal/auth/store_postgres.go
+++ b/internal/auth/store_postgres.go
@@ -186,7 +186,8 @@ func isLastAdminConstraintViolation(err error) bool {
 	return strings.HasPrefix(err.Error(), sentinel)
 }
 
-// UpdateUser updates an existing user.
+// UpdateUser updates an existing user. It rewrites group_ids and active, so it
+// takes the admin-invariant lock first (see adminInvariantAdvisoryLockKey).
 func (s *PostgresStore) UpdateUser(ctx context.Context, user *User) error {
 	user.UpdatedAt = time.Now()
 
@@ -219,53 +220,54 @@ func (s *PostgresStore) UpdateUser(ctx context.Context, user *User) error {
 		recoveryCodes = []string{}
 	}
 
-	result, err := s.db.Exec(ctx, query,
-		user.ID,
-		user.Email,
-		user.PasswordHash,
-		user.Salt,
-		user.GroupIDs,
-		user.Active,
-		user.MFAEnabled,
-		user.MFASecret,
-		user.MFAPendingSecret,
-		user.MFAPendingSecretExpiresAt,
-		recoveryCodes,
-		user.PasswordResetToken,
-		user.PasswordResetExpiry,
-		user.FailedLoginAttempts,
-		user.LockedUntil,
-		user.PasswordHistory,
-		user.UpdatedAt,
-		user.LastLoginAt,
-		user.DeactivatedAt,
-	)
-
-	if err != nil {
-		return fmt.Errorf("failed to update user: %w", err)
-	}
-
-	if result.RowsAffected() == 0 {
-		return fmt.Errorf("user not found: %s", user.ID)
-	}
-
-	return nil
+	return s.withAdminInvariantLock(ctx, "update user", func(tx pgx.Tx) error {
+		result, err := tx.Exec(ctx, query,
+			user.ID,
+			user.Email,
+			user.PasswordHash,
+			user.Salt,
+			user.GroupIDs,
+			user.Active,
+			user.MFAEnabled,
+			user.MFASecret,
+			user.MFAPendingSecret,
+			user.MFAPendingSecretExpiresAt,
+			recoveryCodes,
+			user.PasswordResetToken,
+			user.PasswordResetExpiry,
+			user.FailedLoginAttempts,
+			user.LockedUntil,
+			user.PasswordHistory,
+			user.UpdatedAt,
+			user.LastLoginAt,
+			user.DeactivatedAt,
+		)
+		if err != nil {
+			return fmt.Errorf("failed to update user: %w", err)
+		}
+		if result.RowsAffected() == 0 {
+			return fmt.Errorf("user not found: %s", user.ID)
+		}
+		return nil
+	})
 }
 
-// DeleteUser deletes a user.
+// DeleteUser deletes a user. Deleting an Administrators-group member lowers
+// the admin count, so it takes the admin-invariant lock first (see
+// adminInvariantAdvisoryLockKey).
 func (s *PostgresStore) DeleteUser(ctx context.Context, userID string) error {
 	query := `DELETE FROM users WHERE id = $1`
 
-	result, err := s.db.Exec(ctx, query, userID)
-	if err != nil {
-		return fmt.Errorf("failed to delete user: %w", err)
-	}
-
-	if result.RowsAffected() == 0 {
-		return fmt.Errorf("user not found: %s", userID)
-	}
-
-	return nil
+	return s.withAdminInvariantLock(ctx, "delete user", func(tx pgx.Tx) error {
+		result, err := tx.Exec(ctx, query, userID)
+		if err != nil {
+			return fmt.Errorf("failed to delete user: %w", err)
+		}
+		if result.RowsAffected() == 0 {
+			return fmt.Errorf("user not found: %s", userID)
+		}
+		return nil
+	})
 }
 
 // ListUsers lists all users.
@@ -362,8 +364,47 @@ func (s *PostgresStore) CountGroupMembers(ctx context.Context, groupID string) (
 // check_min_one_admin() in migration 000065 so bootstrap inserts and
 // admin-demoting commits serialize against each other as well as among
 // themselves.
+//
+// Convention (issue #63): any transaction that may demote, deactivate or
+// delete an Administrators-group member, or create the first admin, must
+// acquire this lock as its first statement, before it locks any users row.
+// Use withAdminInvariantLock. The deferred trigger takes the same key again at
+// commit, which is then a no-op re-acquire. A write that locked a users row
+// first and only reached this lock in the trigger would hold the row while
+// waiting for the lock, and deadlock against a writer holding the lock while
+// waiting for that row. Writes that cannot take admin standing away (login
+// counters, MFA and password columns) do not take the lock; they only ever
+// hold the row lock, so they cannot complete such a cycle.
 const adminInvariantAdvisoryLockKey = 8059058058580001
 
+// withAdminInvariantLock runs fn in a transaction whose first statement takes
+// adminInvariantAdvisoryLockKey, then commits. The min-one-admin trigger runs
+// at that commit, so its last_admin_constraint_violation comes back wrapped
+// from here. op names the write in begin, lock and commit errors; errors from
+// fn are returned unchanged. The lock auto-releases at COMMIT or ROLLBACK.
+func (s *PostgresStore) withAdminInvariantLock(ctx context.Context, op string, fn func(tx pgx.Tx) error) error {
+	tx, err := s.db.Begin(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to %s: begin transaction: %w", op, err)
+	}
+	defer func() {
+		if rbErr := tx.Rollback(ctx); rbErr != nil {
+			_ = rbErr // no-op after Commit; Postgres logs real rollback failures
+		}
+	}()
+
+	if _, err := tx.Exec(ctx, "SELECT pg_advisory_xact_lock($1)", adminInvariantAdvisoryLockKey); err != nil {
+		return fmt.Errorf("failed to %s: acquire admin invariant lock: %w", op, err)
+	}
+	if err := fn(tx); err != nil {
+		return err
+	}
+	if err := tx.Commit(ctx); err != nil {
+		return fmt.Errorf("failed to %s: commit: %w", op, err)
+	}
+	return nil
+}
+
 // CreateAdminIfNone atomically inserts user as the first admin in the
 // system. Returns (true, nil) when the insert succeeded; (false, nil)
 // when an admin already existed (TOCTOU race — both callers passed
diff --git a/internal/auth/store_postgres_admin_lock_db_test.go b/internal/auth/store_postgres_admin_lock_db_test.go
new file mode 100644
index 000000000..547b1eca8
--- /dev/null
+++ b/internal/auth/store_postgres_admin_lock_db_test.go
@@ -0,0 +1,349 @@
+//go:build integration
+
+package auth
+
+// store_postgres_admin_lock_db_test.go - real-PostgreSQL tests for the
+// admin-invariant advisory lock ordering (issue #63). Every write that can
+// lower the active-admin count must take adminInvariantAdvisoryLockKey as the
+// first statement of its transaction, before any users row lock, the same as
+// CreateAdminIfNone. The min-one-admin deferred trigger (migration 000065)
+// takes the same key again at commit, which is then a no-op re-acquire.
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgconn"
+	"github.com/jackc/pgx/v5/pgxpool"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
+)
+
+// adminDemotingWrite is one store-level write that can lower the active-admin
+// count. target is the row as the caller read it.
+type adminDemotingWrite struct {
+	name string
+	run  func(ctx context.Context, store *PostgresStore, target *User) error
+}
+
+func adminDemotingWrites() []adminDemotingWrite {
+	return []adminDemotingWrite{
+		{"UpdateUserAdminFields deactivate", func(ctx context.Context, store *PostgresStore, target *User) error {
+			next := *target
+			now := time.Now()
+			next.Active, next.DeactivatedAt = false, &now
+			return store.UpdateUserAdminFields(ctx, &next, target.Email, target.GroupIDs, target.Active)
+		}},
+		{"UpdateUserAdminFields drop Administrators group", func(ctx context.Context, store *PostgresStore, target *User) error {
+			next := *target
+			next.GroupIDs = []string{DefaultPurchaserGroupID}
+			return store.UpdateUserAdminFields(ctx, &next, target.Email, target.GroupIDs, target.Active)
+		}},
+		{"UpdateUser deactivate", func(ctx context.Context, store *PostgresStore, target *User) error {
+			next := *target
+			next.Active = false
+			return store.UpdateUser(ctx, &next)
+		}},
+		{"DeleteUser", func(ctx context.Context, store *PostgresStore, target *User) error {
+			return store.DeleteUser(ctx, target.ID)
+		}},
+	}
+}
+
+// seedActiveAdmins inserts n active Administrators-group members. CreateUser
+// is a plain INSERT, which the min-one-admin triggers do not watch.
+func seedActiveAdmins(t *testing.T, store *PostgresStore, prefix string, n int) []*User {
+	t.Helper()
+	admins := make([]*User, n)
+	for i := range admins {
+		u := newBootstrapCandidate(fmt.Sprintf("%s-%d@example.com", prefix, i))
+		u.GroupIDs = []string{DefaultAdminGroupID}
+		require.NoError(t, store.CreateUser(context.Background(), u))
+		admins[i] = u
+	}
+	return admins
+}
+
+func countActiveAdmins(t *testing.T, db DBConnection) int {
+	t.Helper()
+	var n int
+	require.NoError(t, db.QueryRow(context.Background(),
+		`SELECT COUNT(*) FROM users WHERE `+adminGroupContainsClause+` AND active = true`,
+		DefaultAdminGroupID).Scan(&n))
+	return n
+}
+
+// waitForAdminLockWaiter blocks until some backend is queued on the
+// admin-invariant advisory lock. A bigint advisory key is reported in
+// pg_locks split into classid (high 32 bits) and objid (low 32 bits).
+func waitForAdminLockWaiter(t *testing.T, db DBConnection) {
+	t.Helper()
+	deadline := time.Now().Add(15 * time.Second)
+	for time.Now().Before(deadline) {
+		var waiting int
+		require.NoError(t, db.QueryRow(context.Background(), `
+			SELECT COUNT(*) FROM pg_locks
+			WHERE locktype = 'advisory' AND NOT granted
+			  AND ((classid::bigint << 32) | objid::bigint) = $1`,
+			int64(adminInvariantAdvisoryLockKey)).Scan(&waiting))
+		if waiting > 0 {
+			return
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatal("no backend queued on the admin-invariant advisory lock")
+}
+
+// TestIntegration_AdminDemotingWrites_LockBeforeRowLock pins the lock order.
+// A test transaction holds the admin-invariant lock, as a bootstrap would,
+// while each write runs. The write must queue on that lock before touching
+// the target row, so the holder can still lock the row with NOWAIT. Before
+// issue #63 the write updated or deleted the row first and only reached the
+// lock in the deferred trigger at commit, so NOWAIT failed with 55P03.
+func TestIntegration_AdminDemotingWrites_LockBeforeRowLock(t *testing.T) {
+	db := setupAuthTestDB(t)
+	store := NewPostgresStore(db)
+
+	for _, write := range adminDemotingWrites() {
+		t.Run(write.name, func(t *testing.T) {
+			resetUsers(t, db)
+			// Two admins, so the write is allowed once the holder lets go.
+			target := seedActiveAdmins(t, store, "lock-order", 2)[0]
+			ctx := context.Background()
+
+			holder, err := db.Pool().Begin(ctx)
+			require.NoError(t, err)
+			defer func() { _ = holder.Rollback(ctx) }()
+			_, err = holder.Exec(ctx, "SELECT pg_advisory_xact_lock($1)", adminInvariantAdvisoryLockKey)
+			require.NoError(t, err)
+
+			done := make(chan error, 1)
+			go func() { done <- write.run(ctx, store, target) }()
+			waitForAdminLockWaiter(t, db)
+
+			_, err = holder.Exec(ctx, "SELECT 1 FROM users WHERE id = $1 FOR UPDATE NOWAIT", target.ID)
+			require.NoError(t, err,
+				"the write locked the users row before the admin-invariant advisory lock")
+			require.NoError(t, holder.Commit(ctx))
+
+			select {
+			case err := <-done:
+				require.NoError(t, err)
+			case <-time.After(30 * time.Second):
+				t.Fatal("write did not finish after the advisory lock was released")
+			}
+			assert.Equal(t, 1, countActiveAdmins(t, db))
+		})
+	}
+}
+
+// serviceDemotion is one service-level way of taking admin standing away.
+type serviceDemotion struct {
+	name string
+	run  func(ctx context.Context, svc *Service, userID string) error
+}
+
+func serviceDemotions() []serviceDemotion {
+	inactive := false
+	return []serviceDemotion{
+		{"deactivate", func(ctx context.Context, svc *Service, id string) error {
+			_, err := svc.UpdateUser(ctx, "", id, UpdateUserRequest{Active: &inactive})
+			return err
+		}},
+		{"drop group", func(ctx context.Context, svc *Service, id string) error {
+			_, err := svc.UpdateUser(ctx, "", id, UpdateUserRequest{GroupIDs: []string{DefaultPurchaserGroupID}})
+			return err
+		}},
+		{"delete", func(ctx context.Context, svc *Service, id string) error {
+			return svc.DeleteUser(ctx, id)
+		}},
+	}
+}
+
+// runConcurrently starts every fn at once behind a barrier and returns their
+// errors in order.
+func runConcurrently(fns ...func() error) []error {
+	errs := make([]error, len(fns))
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	for i, fn := range fns {
+		wg.Add(1)
+		go func(i int, fn func() error) {
+			defer wg.Done()
+			<-start
+			errs[i] = fn()
+		}(i, fn)
+	}
+	close(start)
+	wg.Wait()
+	return errs
+}
+
+// TestIntegration_ConcurrentDemotionOfLastTwoAdmins races every pair of
+// demote, deactivate and delete against the two remaining admins: exactly one
+// of the pair succeeds, the other gets ErrLastAdmin, and one active admin is
+// left.
+func TestIntegration_ConcurrentDemotionOfLastTwoAdmins(t *testing.T) {
+	db := setupAuthTestDB(t)
+	store := NewPostgresStore(db)
+	svc := NewService(ServiceConfig{Store: store})
+	ctx := context.Background()
+	const iterations = 15
+
+	for _, first := range serviceDemotions() {
+		for _, second := range serviceDemotions() {
+			t.Run(first.name+" vs "+second.name, func(t *testing.T) {
+				for i := 0; i < iterations; i++ {
+					resetUsers(t, db)
+					admins := seedActiveAdmins(t, store, fmt.Sprintf("last-two-%d", i), 2)
+
+					errs := runConcurrently(
+						func() error { return first.run(ctx, svc, admins[0].ID) },
+						func() error { return second.run(ctx, svc, admins[1].ID) },
+					)
+
+					succeeded := 0
+					for _, err := range errs {
+						if err == nil {
+							succeeded++
+							continue
+						}
+						require.ErrorIs(t, err, ErrLastAdmin, "iteration %d", i)
+					}
+					require.Equal(t, 1, succeeded, "iteration %d: exactly one demotion must win", i)
+					require.Equal(t, 1, countActiveAdmins(t, db), "iteration %d", i)
+				}
+			})
+		}
+	}
+}
+
+// TestIntegration_ConcurrentBootstrapVersusDemotion races CreateAdminIfNone
+// against demoting an existing admin. An admin exists throughout, so the
+// bootstrap must never insert; the demotion succeeds only when another admin
+// remains.
+func TestIntegration_ConcurrentBootstrapVersusDemotion(t *testing.T) {
+	db := setupAuthTestDB(t)
+	store := NewPostgresStore(db)
+	svc := NewService(ServiceConfig{Store: store})
+	ctx := context.Background()
+	const iterations = 15
+
+	for _, admins := range []int{1, 2} {
+		for _, demotion := range serviceDemotions() {
+			t.Run(fmt.Sprintf("%d admins, %s", admins, demotion.name), func(t *testing.T) {
+				for i := 0; i < iterations; i++ {
+					resetUsers(t, db)
+					seeded := seedActiveAdmins(t, store, fmt.Sprintf("bootstrap-race-%d", i), admins)
+
+					var created bool
+					errs := runConcurrently(
+						func() error {
+							var err error
+							created, err = store.CreateAdminIfNone(ctx, newBootstrapCandidate(fmt.Sprintf("bootstrap-%d@example.com", i)))
+							return err
+						},
+						func() error { return demotion.run(ctx, svc, seeded[0].ID) },
+					)
+
+					require.NoError(t, errs[0], "iteration %d", i)
+					assert.False(t, created, "iteration %d: an admin existed, the bootstrap must not insert", i)
+					if admins == 1 {
+						require.ErrorIs(t, errs[1], ErrLastAdmin, "iteration %d", i)
+					} else {
+						require.NoError(t, errs[1], "iteration %d", i)
+					}
+					require.Equal(t, 1, countActiveAdmins(t, db), "iteration %d", i)
+				}
+			})
+		}
+	}
+}
+
+// lockTimeoutPool opens a second pool on the test database whose sessions
+// give up on any lock wait after lockTimeout, so a hang fails fast instead of
+// stalling the suite.
+func lockTimeoutPool(t *testing.T, db *database.Connection, lockTimeout string) *pgxpool.Pool {
+	t.Helper()
+	cfg := db.Pool().Config().Copy()
+	cfg.MaxConns = 16
+	cfg.ConnConfig.RuntimeParams["lock_timeout"] = lockTimeout
+	pool, err := pgxpool.NewWithConfig(context.Background(), cfg)
+	require.NoError(t, err)
+	t.Cleanup(pool.Close)
+	return pool
+}
+
+// expectedContentionError reports whether err is an outcome the mixed
+// workload can legitimately produce: the min-one-admin trigger refusing the
+// last admin, a compare-and-set losing to another write, or the row having
+// been deleted by another write.
+func expectedContentionError(err error) bool {
+	if err == nil || isLastAdminConstraintViolation(err) || errors.Is(err, ErrUserChanged) {
+		return true
+	}
+	var pgErr *pgconn.PgError
+	return !errors.As(err, &pgErr) && strings.HasPrefix(err.Error(), "user not found: ")
+}
+
+// mixedAdminWorkload returns one round of concurrent writes against three
+// admins: every admin-demoting store path, plus the bootstrap and the login
+// paths that lock users rows without the advisory lock.
+func mixedAdminWorkload(ctx context.Context, store *PostgresStore, admins []*User) []func() error {
+	var fns []func() error
+	for i, write := range adminDemotingWrites() {
+		target := admins[i%len(admins)]
+		other := admins[(i+1)%len(admins)]
+		fns = append(fns,
+			func() error { return write.run(ctx, store, target) },
+			func() error { return write.run(ctx, store, other) },
+		)
+	}
+	for _, a := range admins {
+		id := a.ID
+		fns = append(fns,
+			func() error { return store.RecordSuccessfulLogin(ctx, id) },
+			func() error { return store.RecordFailedLogin(ctx, id) },
+		)
+	}
+	fns = append(fns, func() error {
+		_, err := store.CreateAdminIfNone(ctx, newBootstrapCandidate("mixed-bootstrap@example.com"))
+		return err
+	})
+	return fns
+}
+
+// TestIntegration_AdminWrites_NoDeadlock runs many rounds of the mixed
+// workload with a lock_timeout and asserts no transaction ever deadlocks
+// (40P01) or times out on a lock (55P03), and that an active admin always
+// remains. A write path that still locked the row before the advisory lock
+// would deadlock against one that takes the advisory lock first.
+func TestIntegration_AdminWrites_NoDeadlock(t *testing.T) {
+	db := setupAuthTestDB(t)
+	pool := lockTimeoutPool(t, db, "10s")
+	store := NewPostgresStore(pool)
+	ctx := context.Background()
+	const rounds = 60
+
+	for round := 0; round < rounds; round++ {
+		resetUsers(t, db)
+		admins := seedActiveAdmins(t, store, fmt.Sprintf("mixed-%d", round), 3)
+
+		for _, err := range runConcurrently(mixedAdminWorkload(ctx, store, admins)...) {
+			var pgErr *pgconn.PgError
+			if errors.As(err, &pgErr) {
+				require.NotEqual(t, "40P01", pgErr.Code, "round %d: deadlock: %v", round, err)
+				require.NotEqual(t, "55P03", pgErr.Code, "round %d: lock wait timed out: %v", round, err)
+			}
+			require.True(t, expectedContentionError(err), "round %d: unexpected error: %v", round, err)
+		}
+		require.GreaterOrEqual(t, countActiveAdmins(t, pool), 1, "round %d", round)
+	}
+}
diff --git a/internal/auth/store_postgres_admin_lock_test.go b/internal/auth/store_postgres_admin_lock_test.go
new file mode 100644
index 000000000..4ab953f65
--- /dev/null
+++ b/internal/auth/store_postgres_admin_lock_test.go
@@ -0,0 +1,239 @@
+package auth
+
+// store_postgres_admin_lock_test.go - pgxmock tests pinning that every
+// PostgresStore write able to lower the active-admin count runs in a
+// transaction whose first statement takes adminInvariantAdvisoryLockKey
+// (issue #63). The lock-order and concurrency behavior itself is proven on a
+// real PostgreSQL in store_postgres_admin_lock_db_test.go.
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgconn"
+	"github.com/pashagolub/pgxmock/v4"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	updateUserSQLRegex = `(?s)UPDATE users SET\s+email = \$2,.*deactivated_at = \$19\s+WHERE id = \$1`
+
+	updateUserAdminFieldsSQLRegex = `(?s)UPDATE users SET email = \$2, group_ids = \$3, active = \$4, ` +
+		`deactivated_at = \$5, updated_at = NOW\(\)\s+` +
+		`WHERE id = \$1 AND email = \$6 AND group_ids = \$7 AND active = \$8\s+RETURNING updated_at`
+
+	deleteUserSQLRegex = `DELETE FROM users WHERE id = \$1`
+)
+
+const lockedWriteUserID = "22222222-3333-4444-8555-666666666666"
+
+// writeOutcome selects what the mocked write statement returns.
+type writeOutcome int
+
+const (
+	writeOK writeOutcome = iota
+	writeNoRow
+	writeFails
+)
+
+// adminLockedWrite describes one admin-affecting store write: how to mock its
+// statement for an outcome, how to call it, and the error a missing row gives.
+type adminLockedWrite struct {
+	name        string
+	expect      func(mock pgxmock.PgxPoolIface, outcome writeOutcome)
+	call        func(ctx context.Context, store *PostgresStore) error
+	assertNoRow func(t *testing.T, err error)
+}
+
+func expectLockedExec(mock pgxmock.PgxPoolIface, sqlRegex, verb string, args []any, outcome writeOutcome) {
+	e := mock.ExpectExec(sqlRegex).WithArgs(args...)
+	switch outcome {
+	case writeOK:
+		e.WillReturnResult(pgxmock.NewResult(verb, 1))
+	case writeNoRow:
+		e.WillReturnResult(pgxmock.NewResult(verb, 0))
+	case writeFails:
+		e.WillReturnError(errors.New("connection reset"))
+	}
+}
+
+func anyArgs(n int) []any {
+	args := make([]any, n)
+	for i := range args {
+		args[i] = pgxmock.AnyArg()
+	}
+	return args
+}
+
+func adminLockedWrites() []adminLockedWrite {
+	assertUserNotFound := func(t *testing.T, err error) {
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "user not found: "+lockedWriteUserID)
+	}
+	return []adminLockedWrite{
+		{
+			name: "UpdateUser",
+			expect: func(mock pgxmock.PgxPoolIface, outcome writeOutcome) {
+				args := append([]any{lockedWriteUserID}, anyArgs(18)...)
+				expectLockedExec(mock, updateUserSQLRegex, "UPDATE", args, outcome)
+			},
+			call: func(ctx context.Context, store *PostgresStore) error {
+				return store.UpdateUser(ctx, &User{ID: lockedWriteUserID, GroupIDs: []string{DefaultPurchaserGroupID}})
+			},
+			assertNoRow: assertUserNotFound,
+		},
+		{
+			name: "UpdateUserAdminFields",
+			expect: func(mock pgxmock.PgxPoolIface, outcome writeOutcome) {
+				e := mock.ExpectQuery(updateUserAdminFieldsSQLRegex).
+					WithArgs(lockedWriteUserID, "new@example.com", []string{DefaultPurchaserGroupID}, false,
+						pgxmock.AnyArg(), "old@example.com", []string{DefaultAdminGroupID}, true)
+				switch outcome {
+				case writeOK:
+					e.WillReturnRows(mock.NewRows([]string{"updated_at"}).AddRow(time.Unix(1700000000, 0)))
+				case writeNoRow:
+					e.WillReturnRows(mock.NewRows([]string{"updated_at"}))
+				case writeFails:
+					e.WillReturnError(errors.New("connection reset"))
+				}
+			},
+			call: func(ctx context.Context, store *PostgresStore) error {
+				user := &User{ID: lockedWriteUserID, Email: "new@example.com", GroupIDs: []string{DefaultPurchaserGroupID}}
+				return store.UpdateUserAdminFields(ctx, user, "old@example.com", []string{DefaultAdminGroupID}, true)
+			},
+			assertNoRow: func(t *testing.T, err error) { assert.ErrorIs(t, err, ErrUserChanged) },
+		},
+		{
+			name: "DeleteUser",
+			expect: func(mock pgxmock.PgxPoolIface, outcome writeOutcome) {
+				expectLockedExec(mock, deleteUserSQLRegex, "DELETE", []any{lockedWriteUserID}, outcome)
+			},
+			call: func(ctx context.Context, store *PostgresStore) error {
+				return store.DeleteUser(ctx, lockedWriteUserID)
+			},
+			assertNoRow: assertUserNotFound,
+		},
+	}
+}
+
+// expectAdminLock registers the transaction open and the advisory lock that
+// must precede every admin-affecting write. pgxmock matches in order, so a
+// write issued before the lock fails the test.
+func expectAdminLock(mock pgxmock.PgxPoolIface) {
+	mock.ExpectBegin()
+	mock.ExpectExec(advisoryLockSQLRegex).
+		WithArgs(adminInvariantAdvisoryLockKey).
+		WillReturnResult(pgxmock.NewResult("SELECT", 1))
+}
+
+func TestPGXMock_AdminWrites_TakeLockBeforeWriteAndCommit(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			expectAdminLock(mock)
+			w.expect(mock, writeOK)
+			mock.ExpectCommit()
+
+			require.NoError(t, w.call(context.Background(), NewPostgresStore(mock)))
+		})
+	}
+}
+
+func TestPGXMock_AdminWrites_BeginError(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			mock.ExpectBegin().WillReturnError(errors.New("pool exhausted"))
+
+			err := w.call(context.Background(), NewPostgresStore(mock))
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "begin transaction")
+		})
+	}
+}
+
+func TestPGXMock_AdminWrites_LockErrorSkipsWrite(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			mock.ExpectBegin()
+			mock.ExpectExec(advisoryLockSQLRegex).
+				WithArgs(adminInvariantAdvisoryLockKey).
+				WillReturnError(errors.New("canceling statement due to lock timeout"))
+			mock.ExpectRollback()
+
+			err := w.call(context.Background(), NewPostgresStore(mock))
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "acquire admin invariant lock")
+		})
+	}
+}
+
+func TestPGXMock_AdminWrites_NoRowRollsBack(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			expectAdminLock(mock)
+			w.expect(mock, writeNoRow)
+			mock.ExpectRollback()
+
+			w.assertNoRow(t, w.call(context.Background(), NewPostgresStore(mock)))
+		})
+	}
+}
+
+func TestPGXMock_AdminWrites_WriteErrorRollsBack(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			expectAdminLock(mock)
+			w.expect(mock, writeFails)
+			mock.ExpectRollback()
+
+			err := w.call(context.Background(), NewPostgresStore(mock))
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "connection reset")
+		})
+	}
+}
+
+// TestPGXMock_AdminWrites_CommitTriggerViolationStaysDetectable covers the
+// deferred min-one-admin trigger, which now raises at the explicit COMMIT. The
+// service maps it to ErrLastAdmin through isLastAdminConstraintViolation, so
+// the PgError must survive the wrapping.
+func TestPGXMock_AdminWrites_CommitTriggerViolationStaysDetectable(t *testing.T) {
+	for _, w := range adminLockedWrites() {
+		t.Run(w.name, func(t *testing.T) {
+			mock := newAuthPgxMock(t)
+			expectAdminLock(mock)
+			w.expect(mock, writeOK)
+			mock.ExpectCommit().WillReturnError(&pgconn.PgError{
+				Code:    "P0001",
+				Message: "last_admin_constraint_violation: at least one active member of the Administrators group must remain",
+			})
+			mock.ExpectRollback()
+
+			err := w.call(context.Background(), NewPostgresStore(mock))
+			require.Error(t, err)
+			assert.True(t, isLastAdminConstraintViolation(err), "lost the trigger error: %v", err)
+			assert.Contains(t, err.Error(), "commit")
+		})
+	}
+}
+
+func TestPGXMock_UpdateUserAdminFields_ReturnsStoredUpdatedAt(t *testing.T) {
+	mock := newAuthPgxMock(t)
+	stored := time.Unix(1700000000, 0)
+	expectAdminLock(mock)
+	mock.ExpectQuery(updateUserAdminFieldsSQLRegex).
+		WithArgs(anyArgs(8)...).
+		WillReturnRows(mock.NewRows([]string{"updated_at"}).AddRow(stored))
+	mock.ExpectCommit()
+
+	user := &User{ID: lockedWriteUserID}
+	require.NoError(t, NewPostgresStore(mock).UpdateUserAdminFields(context.Background(), user, "", nil, true))
+	assert.True(t, user.UpdatedAt.Equal(stored))
+}
diff --git a/internal/auth/store_postgres_credentials.go b/internal/auth/store_postgres_credentials.go
index e0dfd6324..ca716bbf9 100644
--- a/internal/auth/store_postgres_credentials.go
+++ b/internal/auth/store_postgres_credentials.go
@@ -14,19 +14,24 @@ import (
 // the row still holds the email, groups and active state the caller read, so a
 // stale admin save cannot erase MFA fields or restore an old password hash
 // (issue #493).
+//
+// The write can deactivate a member or drop the Administrators group, so it
+// takes the admin-invariant lock first (see adminInvariantAdvisoryLockKey).
 func (s *PostgresStore) UpdateUserAdminFields(ctx context.Context, user *User, readEmail string, readGroupIDs []string, readActive bool) error {
-	err := s.db.QueryRow(ctx, `
-		UPDATE users SET email = $2, group_ids = $3, active = $4, deactivated_at = $5, updated_at = NOW()
-		WHERE id = $1 AND email = $6 AND group_ids = $7 AND active = $8
-		RETURNING updated_at
-	`, user.ID, user.Email, user.GroupIDs, user.Active, user.DeactivatedAt, readEmail, readGroupIDs, readActive).Scan(&user.UpdatedAt)
-	if errors.Is(err, pgx.ErrNoRows) {
-		return ErrUserChanged
-	}
-	if err != nil {
-		return fmt.Errorf("failed to update user: %w", err)
-	}
-	return nil
+	return s.withAdminInvariantLock(ctx, "update user", func(tx pgx.Tx) error {
+		err := tx.QueryRow(ctx, `
+			UPDATE users SET email = $2, group_ids = $3, active = $4, deactivated_at = $5, updated_at = NOW()
+			WHERE id = $1 AND email = $6 AND group_ids = $7 AND active = $8
+			RETURNING updated_at
+		`, user.ID, user.Email, user.GroupIDs, user.Active, user.DeactivatedAt, readEmail, readGroupIDs, readActive).Scan(&user.UpdatedAt)
+		if errors.Is(err, pgx.ErrNoRows) {
+			return ErrUserChanged
+		}
+		if err != nil {
+			return fmt.Errorf("failed to update user: %w", err)
+		}
+		return nil
+	})
 }
 
 // UpdateUserCredentials writes only the email and password columns, and only
diff --git a/internal/auth/store_postgres_test.go b/internal/auth/store_postgres_test.go
index 5e21b0de1..b270bc97a 100644
--- a/internal/auth/store_postgres_test.go
+++ b/internal/auth/store_postgres_test.go
@@ -275,93 +275,6 @@ func TestPostgresStore_CreateUser(t *testing.T) {
 	})
 }
 
-func TestPostgresStore_UpdateUser(t *testing.T) {
-	ctx := context.Background()
-
-	t.Run("successfully update user", func(t *testing.T) {
-		mockDB := new(MockDBConnection)
-		store := &PostgresStore{db: mockDB}
-
-		user := &User{
-			ID:       "user-123",
-			Email:    "updated@example.com",
-			GroupIDs: []string{},
-		}
-
-		// Create a CommandTag that indicates 1 row was affected
-		tag := pgconn.NewCommandTag("UPDATE 1")
-		mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(tag, nil)
-
-		err := store.UpdateUser(ctx, user)
-		require.NoError(t, err)
-		assert.False(t, user.UpdatedAt.IsZero())
-
-		mockDB.AssertExpectations(t)
-	})
-
-	t.Run("return error when update fails", func(t *testing.T) {
-		mockDB := new(MockDBConnection)
-		store := &PostgresStore{db: mockDB}
-
-		user := &User{
-			ID:       "user-123",
-			GroupIDs: []string{},
-		}
-
-		mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(pgconn.CommandTag{}, fmt.Errorf("update failed"))
-
-		err := store.UpdateUser(ctx, user)
-		assert.Error(t, err)
-
-		mockDB.AssertExpectations(t)
-	})
-}
-
-func TestPostgresStore_DeleteUser(t *testing.T) {
-	ctx := context.Background()
-
-	t.Run("successfully delete user", func(t *testing.T) {
-		mockDB := new(MockDBConnection)
-		store := &PostgresStore{db: mockDB}
-
-		// Create a CommandTag that indicates 1 row was affected
-		tag := pgconn.NewCommandTag("DELETE 1")
-		mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(tag, nil)
-
-		err := store.DeleteUser(ctx, "user-123")
-		require.NoError(t, err)
-
-		mockDB.AssertExpectations(t)
-	})
-
-	t.Run("return error when delete fails", func(t *testing.T) {
-		mockDB := new(MockDBConnection)
-		store := &PostgresStore{db: mockDB}
-
-		mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(pgconn.CommandTag{}, fmt.Errorf("delete failed"))
-
-		err := store.DeleteUser(ctx, "user-123")
-		assert.Error(t, err)
-
-		mockDB.AssertExpectations(t)
-	})
-
-	t.Run("return error when user not found", func(t *testing.T) {
-		mockDB := new(MockDBConnection)
-		store := &PostgresStore{db: mockDB}
-
-		// Return 0 rows affected
-		tag := pgconn.NewCommandTag("DELETE 0")
-		mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(tag, nil)
-
-		err := store.DeleteUser(ctx, "nonexistent")
-		assert.Error(t, err)
-		assert.Contains(t, err.Error(), "user not found")
-
-		mockDB.AssertExpectations(t)
-	})
-}
-
 func TestNewPostgresStore(t *testing.T) {
 	mockDB := new(MockDBConnection)
 	store := NewPostgresStore(mockDB)
@@ -470,29 +383,6 @@ func TestPostgresStore_AdminExists(t *testing.T) {
 	})
 }
 
-func TestPostgresStore_UpdateUser_NotFound(t *testing.T) {
-	ctx := context.Background()
-
-	mockDB := new(MockDBConnection)
-	store := &PostgresStore{db: mockDB}
-
-	user := &User{
-		ID:       "nonexistent",
-		Email:    "test@example.com",
-		GroupIDs: []string{},
-	}
-
-	// Return 0 rows affected
-	tag := pgconn.NewCommandTag("UPDATE 0")
-	mockDB.On("Exec", ctx, mock.AnythingOfType("string"), mock.Anything).Return(tag, nil)
-
-	err := store.UpdateUser(ctx, user)
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "user not found")
-
-	mockDB.AssertExpectations(t)
-}
-
 // ==========================================
 // SESSION TESTS
 // ==========================================

From c1d39b0c60d5ea40c6bb136214215b0f1be3cc14 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Tue, 20 Oct 2026 15:58:48 +0100
Subject: [PATCH 2/2] chore(auth): remove the unused whole-row store UpdateUser

After #581 nothing in production called PostgresStore.UpdateUser(ctx,
*User): every service path writes through a column-scoped,
compare-and-set writer. Keeping a 19-column writer in the store
interface lets a future caller bring back the stale whole-row save that
overwrote concurrent MFA, password and membership changes.

Remove it from StoreInterface, PostgresStore, MockStore, MockAuthStore
and the server health-test fake. The #63 lock-order tests drop their
UpdateUser cases; UpdateUserAdminFields and DeleteUser are now the only
store writes that can lower the active-admin count, and both are still
covered by the pgxmock order tests, the NOWAIT lock-before-row test, the
last-two-admins races and the mixed no-deadlock workload.

Fixtures that used UpdateUser as the concurrent writer now go through
UpdateUserAdminFields (the credential race tests), or through a
test-only statement that writes the same columns under the admin
invariant lock (the login test, whose competing change spans more
columns than any one store writer). AssertNotCalled("UpdateUser")
checks, which could no longer fail, now name the narrow writer each
path would use, or are dropped where the mock already panics on any
unstubbed write. Comments that named the store UpdateUser now name the
real writer.

TestStore_HasNoWholeRowUserWriter pins the removal.

Closes #583

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/auth/interfaces.go                   |  1 -
 internal/auth/membership_ceiling_test.go      |  2 +-
 internal/auth/self_account_scope_test.go      |  2 +-
 .../auth/self_escalation_carveout_test.go     |  2 +-
 internal/auth/service_api_test.go             |  6 +-
 internal/auth/service_credentials_db_test.go  | 40 ++++++-----
 .../auth/service_group_only_authz_test.go     |  2 +-
 internal/auth/service_lockout_test.go         |  2 -
 internal/auth/service_login_db_test.go        | 34 +++++++++-
 internal/auth/service_mfa.go                  |  3 +-
 internal/auth/service_mfa_test.go             |  4 +-
 internal/auth/service_password.go             |  2 +-
 .../auth/service_password_callback_test.go    |  4 +-
 internal/auth/service_password_test.go        | 10 +--
 internal/auth/service_security_test.go        |  4 +-
 internal/auth/service_user_test.go            |  8 +--
 internal/auth/store_postgres.go               | 66 -------------------
 .../auth/store_postgres_admin_lock_db_test.go |  5 --
 .../auth/store_postgres_admin_lock_test.go    | 30 +++++----
 internal/auth/test_helpers.go                 |  5 --
 internal/mocks/stores.go                      |  6 --
 internal/server/adapter_test.go               |  2 +-
 internal/server/health_test.go                |  4 --
 23 files changed, 98 insertions(+), 146 deletions(-)

diff --git a/internal/auth/interfaces.go b/internal/auth/interfaces.go
index 610874183..b9683311f 100644
--- a/internal/auth/interfaces.go
+++ b/internal/auth/interfaces.go
@@ -11,7 +11,6 @@ type StoreInterface interface {
 	GetUserByID(ctx context.Context, userID string) (*User, error)
 	GetUserByEmail(ctx context.Context, email string) (*User, error)
 	CreateUser(ctx context.Context, user *User) error
-	UpdateUser(ctx context.Context, user *User) error
 	UpdateUserAdminFields(ctx context.Context, user *User, readEmail string, readGroupIDs []string, readActive bool) error
 	UpdateUserCredentials(ctx context.Context, user *User, readEmail, readPasswordHash string) error
 	SetPasswordResetToken(ctx context.Context, user *User, readExpiry *time.Time) error
diff --git a/internal/auth/membership_ceiling_test.go b/internal/auth/membership_ceiling_test.go
index 2c7d99986..f413629b0 100644
--- a/internal/auth/membership_ceiling_test.go
+++ b/internal/auth/membership_ceiling_test.go
@@ -8,7 +8,7 @@ package auth
 // newCeilingService) are shared with the permission-ceiling tests; see
 // group_ceiling_fixtures_test.go.
 //
-// Every refusal case stubs NO CreateUser / UpdateUser expectation on the
+// Every refusal case stubs NO CreateUser / UpdateUserAdminFields expectation on the
 // mock store, so a removed guard fails the test loudly (testify panics on an
 // un-stubbed call) rather than passing silently.
 
diff --git a/internal/auth/self_account_scope_test.go b/internal/auth/self_account_scope_test.go
index 03609cab3..d3be92caa 100644
--- a/internal/auth/self_account_scope_test.go
+++ b/internal/auth/self_account_scope_test.go
@@ -61,7 +61,7 @@ func acctAViewersGroup() *Group {
 
 // stubScopedActor wires the actor's user row and the groups the change touches.
 //
-// Refusal cases add a permissive UpdateUser stub on purpose: without one, a
+// Refusal cases add a permissive UpdateUserAdminFields stub on purpose: without one, a
 // guard removal kills the test by PANICKING on an unstubbed write, and that
 // kill evaporates the moment someone adds a stub while tidying fixtures. With
 // it, the removal is caught by an assertion instead.
diff --git a/internal/auth/self_escalation_carveout_test.go b/internal/auth/self_escalation_carveout_test.go
index 14c53ff35..b831bc802 100644
--- a/internal/auth/self_escalation_carveout_test.go
+++ b/internal/auth/self_escalation_carveout_test.go
@@ -17,7 +17,7 @@ import (
 // admin:* grants, so it passed. Closing only the group-permission write path
 // would have left this open and #1550 would have auto-closed over it.
 //
-// As elsewhere in this package, refusal cases stub NO UpdateUser expectation:
+// As elsewhere in this package, refusal cases stub NO UpdateUserAdminFields expectation:
 // testify panics if the write lands, so the assertions cannot be vacuous.
 
 const (
diff --git a/internal/auth/service_api_test.go b/internal/auth/service_api_test.go
index 984465cdb..220754381 100644
--- a/internal/auth/service_api_test.go
+++ b/internal/auth/service_api_test.go
@@ -213,7 +213,7 @@ func TestService_UpdateUserAPI(t *testing.T) {
 	// toast would fire, but the users.email column was never touched. We
 	// assert that (a) the email-uniqueness lookup happens (proves the
 	// admin path runs the same validation as self-edit), and (b) the
-	// user persisted via store.UpdateUser carries the new email value,
+	// user persisted via store.UpdateUserAdminFields carries the new email value,
 	// NOT just that the call succeeded.
 	t.Run("successful email update persists to store", func(t *testing.T) {
 		mockStore := new(MockStore)
@@ -229,7 +229,7 @@ func TestService_UpdateUserAPI(t *testing.T) {
 		mockStore.On("GetUserByID", ctx, "user-123").Return(existingUser, nil).Once()
 		// Uniqueness check: new email not yet in use.
 		mockStore.On("GetUserByEmail", ctx, "new@example.com").Return(nil, nil).Once()
-		// Capture the User passed to UpdateUser and assert its email is
+		// Capture the User passed to UpdateUserAdminFields and assert its email is
 		// the NEW one. Without the fix, this fails because the User would
 		// still carry old@example.com.
 		mockStore.On("UpdateUserAdminFields", ctx, mock.MatchedBy(func(u *User) bool {
@@ -296,7 +296,7 @@ func TestService_UpdateUserAPI(t *testing.T) {
 	// Regression for issue #89: before this fix, APIUpdateUserRequest had no
 	// Active field at all, so a PUT /api/users/{id} body could never
 	// deactivate a user -- the same field-silently-dropped defect class
-	// issue #892 fixed for Email. We assert the User passed to store.UpdateUser
+	// issue #892 fixed for Email. We assert the User passed to store.UpdateUserAdminFields
 	// carries Active=false, not just that the call succeeds, so the test
 	// would have failed on the pre-fix struct (which drops the field on the
 	// floor and always persists whatever GetUserByID returned, i.e. true).
diff --git a/internal/auth/service_credentials_db_test.go b/internal/auth/service_credentials_db_test.go
index 8eae7d460..4e9bc4668 100644
--- a/internal/auth/service_credentials_db_test.go
+++ b/internal/auth/service_credentials_db_test.go
@@ -5,6 +5,7 @@ package auth
 import (
 	"context"
 	"errors"
+	"slices"
 	"testing"
 	"time"
 
@@ -132,6 +133,17 @@ func (f *credentialRaceFixture) stored() *User {
 	return u
 }
 
+// adminEdit applies edit to the stored row through UpdateUserAdminFields, the
+// column-scoped writer an admin's email or active change goes through, so the
+// competing write is the one production would make.
+func (f *credentialRaceFixture) adminEdit(ctx context.Context, edit func(u *User)) {
+	u, err := f.store.GetUserByID(ctx, f.user.ID)
+	require.NoError(f.t, err)
+	readEmail, readGroupIDs, readActive := u.Email, slices.Clone(u.GroupIDs), u.Active
+	edit(u)
+	require.NoError(f.t, f.store.UpdateUserAdminFields(ctx, u, readEmail, readGroupIDs, readActive))
+}
+
 // requireFactorEnforced proves the victim's factor survived: login without it
 // is refused and login with it succeeds.
 func (f *credentialRaceFixture) requireFactorEnforced(email, password string, enrolled *User) {
@@ -247,10 +259,7 @@ func TestIntegration_CredentialWritesRejectStaleCredentials(t *testing.T) {
 	t.Run("profile-after-concurrent-email-change", func(t *testing.T) {
 		f := newCredentialRaceFixture(t, store, "stale-profile@example.com")
 		winner := f.onRead(func(ctx context.Context, _ *Service) {
-			u, err := store.GetUserByID(ctx, f.user.ID)
-			require.NoError(t, err)
-			u.Email = "stale-profile-admin@example.com"
-			require.NoError(t, store.UpdateUser(ctx, u))
+			f.adminEdit(ctx, func(u *User) { u.Email = "stale-profile-admin@example.com" })
 		})
 		err := f.svc.UpdateUserProfile(ctx, f.user.ID, "stale-profile-self@example.com", credentialRacePassword, credentialRaceNew)
 		require.ErrorIs(t, err, ErrUserChanged)
@@ -262,11 +271,10 @@ func TestIntegration_CredentialWritesRejectStaleCredentials(t *testing.T) {
 	t.Run("reset-after-concurrent-deactivation", func(t *testing.T) {
 		f := newCredentialRaceFixture(t, store, "stale-reset-deactivated@example.com")
 		winner := f.onRead(func(ctx context.Context, _ *Service) {
-			u, err := store.GetUserByID(ctx, f.user.ID)
-			require.NoError(t, err)
-			now := time.Now()
-			u.Active, u.DeactivatedAt = false, &now
-			require.NoError(t, store.UpdateUser(ctx, u))
+			f.adminEdit(ctx, func(u *User) {
+				now := time.Now()
+				u.Active, u.DeactivatedAt = false, &now
+			})
 		})
 		require.NoError(t, f.svc.RequestPasswordReset(ctx, f.user.Email))
 		assert.Empty(t, f.mail.resets)
@@ -276,10 +284,7 @@ func TestIntegration_CredentialWritesRejectStaleCredentials(t *testing.T) {
 	t.Run("reset-after-concurrent-email-change", func(t *testing.T) {
 		f := newCredentialRaceFixture(t, store, "stale-reset-email@example.com")
 		winner := f.onRead(func(ctx context.Context, _ *Service) {
-			u, err := store.GetUserByID(ctx, f.user.ID)
-			require.NoError(t, err)
-			u.Email = "stale-reset-email-new@example.com"
-			require.NoError(t, store.UpdateUser(ctx, u))
+			f.adminEdit(ctx, func(u *User) { u.Email = "stale-reset-email-new@example.com" })
 		})
 		require.NoError(t, f.svc.RequestPasswordReset(ctx, f.user.Email))
 		assert.Empty(t, f.mail.resets, "a token must not be mailed to the address the user just left")
@@ -404,11 +409,10 @@ func TestIntegration_ResetConfirmRejectsStaleRead(t *testing.T) {
 		f := newCredentialRaceFixture(t, store, "reset-after-deactivation@example.com")
 		token := f.issueResetToken()
 		winner := f.onRead(func(ctx context.Context, _ *Service) {
-			u, err := store.GetUserByID(ctx, f.user.ID)
-			require.NoError(t, err)
-			now := time.Now()
-			u.Active, u.DeactivatedAt = false, &now
-			require.NoError(t, store.UpdateUser(ctx, u))
+			f.adminEdit(ctx, func(u *User) {
+				now := time.Now()
+				u.Active, u.DeactivatedAt = false, &now
+			})
 		})
 		err := f.svc.ConfirmPasswordReset(ctx, PasswordResetConfirm{Token: token, NewPassword: credentialRaceNew})
 		require.ErrorIs(t, err, ErrUserChanged)
diff --git a/internal/auth/service_group_only_authz_test.go b/internal/auth/service_group_only_authz_test.go
index a47da9178..20ebd495b 100644
--- a/internal/auth/service_group_only_authz_test.go
+++ b/internal/auth/service_group_only_authz_test.go
@@ -248,7 +248,7 @@ func TestUpdateUser_SelfEscalationDenied(t *testing.T) {
 	mockStore.On("GetGroup", ctx, viewerGroup().ID).Return(viewerGroup(), nil)
 	// Permissive write stub, deliberately: without it, removing the
 	// self-escalation guard kills this test by panicking on an unstubbed
-	// UpdateUser -- a kill that disappears the moment someone adds a stub
+	// UpdateUserAdminFields -- a kill that disappears the moment someone adds a stub
 	// while tidying fixtures. With it, the removal is caught by the
 	// AssertNotCalled below, which is the actual security property.
 	mockStore.On("UpdateUserAdminFields", ctx, mock.AnythingOfType("*auth.User"), mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
diff --git a/internal/auth/service_lockout_test.go b/internal/auth/service_lockout_test.go
index 5efc859b4..ce7406607 100644
--- a/internal/auth/service_lockout_test.go
+++ b/internal/auth/service_lockout_test.go
@@ -60,7 +60,6 @@ func TestLogin_AccountLockout_Bookkeeping(t *testing.T) {
 					assert.EqualError(t, err, genericLoginError)
 				}
 			}
-			store.AssertNotCalled(t, "UpdateUser", mock.Anything, mock.Anything)
 			store.AssertExpectations(t)
 		})
 	}
@@ -108,7 +107,6 @@ func TestLogin_BookkeepingErrors(t *testing.T) {
 					assert.EqualError(t, err, genericLoginError)
 				}
 			}
-			store.AssertNotCalled(t, "UpdateUser", mock.Anything, mock.Anything)
 			store.AssertExpectations(t)
 		})
 	}
diff --git a/internal/auth/service_login_db_test.go b/internal/auth/service_login_db_test.go
index 0774bbf20..c3280ccbe 100644
--- a/internal/auth/service_login_db_test.go
+++ b/internal/auth/service_login_db_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/jackc/pgx/v5"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -26,6 +27,37 @@ func (s *loginReadBarrierStore) GetUserByEmail(ctx context.Context, email string
 	return u, err
 }
 
+// overwriteSecurityColumns stands in for every security change that can land
+// while a login is in flight (credentials, MFA, reset token, password history,
+// membership and active state), all in one statement. No store writer covers
+// them all: the whole-row UpdateUser that did was removed in #583. It writes
+// group_ids and active, so like the store's own writers it takes the
+// admin-invariant lock first (see adminInvariantAdvisoryLockKey).
+func overwriteSecurityColumns(ctx context.Context, store *PostgresStore, u *User) error {
+	return store.withAdminInvariantLock(ctx, "overwrite security columns", func(tx pgx.Tx) error {
+		tag, err := tx.Exec(ctx, `
+			UPDATE users SET
+				email = $2, password_hash = $3, salt = $4, group_ids = $5,
+				active = $6, deactivated_at = $7, mfa_enabled = $8, mfa_secret = $9,
+				mfa_pending_secret = $10, mfa_pending_secret_expires_at = $11,
+				mfa_recovery_codes = $12, password_reset_token = $13,
+				password_reset_expiry = $14, password_history = $15, updated_at = NOW()
+			WHERE id = $1`,
+			u.ID, u.Email, u.PasswordHash, u.Salt, u.GroupIDs,
+			u.Active, u.DeactivatedAt, u.MFAEnabled, u.MFASecret,
+			u.MFAPendingSecret, u.MFAPendingSecretExpiresAt,
+			u.MFARecoveryCodes, u.PasswordResetToken,
+			u.PasswordResetExpiry, u.PasswordHistory)
+		if err != nil {
+			return err
+		}
+		if tag.RowsAffected() != 1 {
+			return fmt.Errorf("overwrite security columns: %d rows for user %s", tag.RowsAffected(), u.ID)
+		}
+		return nil
+	})
+}
+
 func TestIntegration_LoginPreservesSecurityChanges(t *testing.T) {
 	store := NewPostgresStore(setupAuthTestDB(t))
 	ctx := t.Context()
@@ -55,7 +87,7 @@ func TestIntegration_LoginPreservesSecurityChanges(t *testing.T) {
 				current.Active, current.DeactivatedAt = false, &stamp
 				current.PasswordResetToken, current.PasswordResetExpiry = "new-reset-token", &stamp
 				current.PasswordHistory = []string{"new-history"}
-				if updateErr := store.UpdateUser(ctx, current); updateErr != nil {
+				if updateErr := overwriteSecurityColumns(ctx, store, current); updateErr != nil {
 					return updateErr
 				}
 				changed, readErr = store.GetUserByID(ctx, current.ID)
diff --git a/internal/auth/service_mfa.go b/internal/auth/service_mfa.go
index 9c32c58bf..1fe269ccf 100644
--- a/internal/auth/service_mfa.go
+++ b/internal/auth/service_mfa.go
@@ -243,7 +243,8 @@ func (s *Service) hashRecoveryCode(code string) (string, error) {
 // consumeRecoveryCode returns true and removes the matching hash from
 // user.MFARecoveryCodes if entered matches any stored hash. The
 // removal is what makes recovery codes single-use; the caller must
-// persist the user afterwards via UpdateUser.
+// persist the shortened slice through its own compare-and-set write
+// (ConsumeMFARecoveryCode on login).
 //
 // Fails closed on empty input: an empty string normalizes to "", which
 // bcrypt.CompareHashAndPassword would compare against every stored hash.
diff --git a/internal/auth/service_mfa_test.go b/internal/auth/service_mfa_test.go
index dccb8dfd3..d7ebc9814 100644
--- a/internal/auth/service_mfa_test.go
+++ b/internal/auth/service_mfa_test.go
@@ -235,7 +235,7 @@ func TestMFASetup_WrongPassword(t *testing.T) {
 	_, err := service.MFASetup(ctx, user.ID, "WrongPassword!@#")
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "invalid password")
-	mockStore.AssertNotCalled(t, "UpdateUser", mock.Anything, mock.Anything)
+	mockStore.AssertNotCalled(t, "SetPendingMFASecret", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestMFAEnable_HappyPath(t *testing.T) {
@@ -364,7 +364,7 @@ func TestMFADisable_WrongPassword(t *testing.T) {
 	err := service.MFADisable(ctx, user.ID, "wrong", "000000")
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "invalid password")
-	mockStore.AssertNotCalled(t, "UpdateUser", mock.Anything, mock.Anything)
+	mockStore.AssertNotCalled(t, "DisableMFA", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestMFADisable_IdempotentWhenAlreadyDisabled(t *testing.T) {
diff --git a/internal/auth/service_password.go b/internal/auth/service_password.go
index 3be2d3027..e4d83ce6a 100644
--- a/internal/auth/service_password.go
+++ b/internal/auth/service_password.go
@@ -277,7 +277,7 @@ func (s *Service) ChangePassword(ctx context.Context, userID string, req ChangeP
 
 	// Invalidate sessions and API keys only after the new password is
 	// persisted (non-critical, best-effort -- issue #102). Invalidating
-	// first would mean a failed UpdateUser leaves the caller with revoked
+	// first would mean a failed UpdateUserCredentials leaves the caller with revoked
 	// credentials for a password that never actually changed (CodeRabbit
 	// finding on #392).
 	s.invalidateUserCredentialsBestEffort(ctx, userID, "password change")
diff --git a/internal/auth/service_password_callback_test.go b/internal/auth/service_password_callback_test.go
index 3fa5d3295..cb49061f3 100644
--- a/internal/auth/service_password_callback_test.go
+++ b/internal/auth/service_password_callback_test.go
@@ -84,10 +84,10 @@ func TestService_OnPasswordChange_ChangePassword(t *testing.T) {
 
 		testUser := createTestUser(t, "OldSecure123!")
 
-		// Invalidation now runs only after UpdateUser succeeds (issue #102
+		// Invalidation now runs only after UpdateUserCredentials succeeds (issue #102
 		// CodeRabbit finding on #392: invalidating first would revoke
 		// credentials for a password that never actually changed), so a
-		// failed UpdateUser must never reach DeleteUserSessions/ListAPIKeysByUser.
+		// failed UpdateUserCredentials must never reach DeleteUserSessions/ListAPIKeysByUser.
 		mockStore.On("GetUserByID", ctx, "user-123").Return(testUser, nil).Once()
 		mockStore.On("UpdateUserCredentials", ctx, mock.AnythingOfType("*auth.User"), mock.Anything, mock.Anything).Return(assert.AnError).Once()
 
diff --git a/internal/auth/service_password_test.go b/internal/auth/service_password_test.go
index 6c85eddfc..56080cbf1 100644
--- a/internal/auth/service_password_test.go
+++ b/internal/auth/service_password_test.go
@@ -334,7 +334,7 @@ func TestService_RequestPasswordReset(t *testing.T) {
 	// Regression test for issue #89 / audit finding A03-006: an admin-
 	// deactivated account must not receive a working reset link. It must be
 	// refused with the exact same no-error, no-email response as "email
-	// does not exist" -- neither UpdateUser (which would mint a fresh reset
+	// does not exist" -- neither SetPasswordResetToken (which would mint a fresh reset
 	// token) nor SendPasswordResetEmail may be called, or the response shape
 	// would let a caller distinguish deactivated from missing.
 	t.Run("deactivated account gets no reset link, same response as missing email", func(t *testing.T) {
@@ -353,7 +353,7 @@ func TestService_RequestPasswordReset(t *testing.T) {
 		}
 
 		mockStore.On("GetUserByEmail", ctx, "deactivated@example.com").Return(deactivatedUser, nil).Once()
-		// UpdateUser and SendPasswordResetEmail must NOT be called.
+		// SetPasswordResetToken and SendPasswordResetEmail must NOT be called.
 
 		err := service.RequestPasswordReset(ctx, "deactivated@example.com")
 		require.NoError(t, err, "deactivated-account reset request must not error (enumeration protection)")
@@ -362,7 +362,7 @@ func TestService_RequestPasswordReset(t *testing.T) {
 
 // TestService_RequestPasswordReset_RateLimit is the regression test for 03-M5.
 // An active token issued within PasswordResetRateLimit must be preserved: the
-// second call must return nil without calling UpdateUser or sending an email.
+// second call must return nil without calling SetPasswordResetToken or sending an email.
 func TestService_RequestPasswordReset_RateLimit(t *testing.T) {
 	ctx := context.Background()
 	mockStore := new(MockStore)
@@ -382,11 +382,11 @@ func TestService_RequestPasswordReset_RateLimit(t *testing.T) {
 	}
 
 	mockStore.On("GetUserByEmail", ctx, "ratelimit@example.com").Return(userWithFreshToken, nil).Once()
-	// UpdateUser and SendPasswordResetEmail must NOT be called.
+	// SetPasswordResetToken and SendPasswordResetEmail must NOT be called.
 
 	err := service.RequestPasswordReset(ctx, "ratelimit@example.com")
 	require.NoError(t, err, "rate-limited reset must not return an error (enumeration protection)")
-	// Verify by asserting no UpdateUser or email calls happened (handled by AssertExpectations).
+	// Verify by asserting no SetPasswordResetToken or email calls happened (handled by AssertExpectations).
 }
 
 func TestService_ConfirmPasswordReset(t *testing.T) {
diff --git a/internal/auth/service_security_test.go b/internal/auth/service_security_test.go
index 2d12041d3..242c4853e 100644
--- a/internal/auth/service_security_test.go
+++ b/internal/auth/service_security_test.go
@@ -114,8 +114,8 @@ func TestMFARegenerateRecoveryCodes_EmptyCode(t *testing.T) {
 	_, err := service.MFARegenerateRecoveryCodes(ctx, user.ID, "")
 	require.Error(t, err, "MFARegenerateRecoveryCodes with empty code must return an error")
 
-	// UpdateUser must NOT have been called.
-	mockStore.AssertNotCalled(t, "UpdateUser", mock.Anything, mock.Anything)
+	// The codes must NOT have been replaced.
+	mockStore.AssertNotCalled(t, "ReplaceMFARecoveryCodes", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 
 	// The pre-existing codes must be untouched.
 	assert.Equal(t, []string{"$2a$04$preexistingstub"}, user.MFARecoveryCodes,
diff --git a/internal/auth/service_user_test.go b/internal/auth/service_user_test.go
index 2207ab298..055faf2dc 100644
--- a/internal/auth/service_user_test.go
+++ b/internal/auth/service_user_test.go
@@ -460,7 +460,7 @@ func TestService_DeleteUser_ConcurrentLastTwoAdmins(t *testing.T) {
 // simultaneously deactivate the last two active admins. Both read count == 2
 // and pass the soft check. The 000065 deferred trigger now counts only *active*
 // members and serializes via an advisory xact lock, so exactly one commit is
-// rejected. We simulate the DB outcome by having the second UpdateUser return
+// rejected. We simulate the DB outcome by having the second UpdateUserAdminFields return
 // the trigger violation, and assert the invariant: never zero active admins.
 func TestService_UpdateUser_ConcurrentDeactivateLastTwoAdmins(t *testing.T) {
 	ctx := context.Background()
@@ -476,7 +476,7 @@ func TestService_UpdateUser_ConcurrentDeactivateLastTwoAdmins(t *testing.T) {
 	mockStore.On("GetUserByID", ctx, "admin-b").Return(adminB, nil).Once()
 	mockStore.On("CountGroupMembers", ctx, DefaultAdminGroupID).Return(2, nil).Twice()
 
-	// Session revocation now happens only after store.UpdateUser succeeds,
+	// Session revocation now happens only after store.UpdateUserAdminFields succeeds,
 	// so only admin-a (whose commit succeeds below) gets DeleteUserSessions;
 	// admin-b hits the deferred trigger and must not be logged out for a
 	// deactivation that never actually took effect.
@@ -899,7 +899,7 @@ func TestService_UpdateUser(t *testing.T) {
 		// Soft check passes: two admins visible at read time.
 		mockStore.On("GetUserByID", ctx, "admin-1").Return(adminUser, nil).Once()
 		mockStore.On("CountGroupMembers", ctx, DefaultAdminGroupID).Return(2, nil).Once()
-		// UpdateUser hits the deferred trigger at commit time.
+		// UpdateUserAdminFields hits the deferred trigger at commit time.
 		triggerErr := fmt.Errorf("last_admin_constraint_violation: at least one member of the Administrators group must remain")
 		mockStore.On("UpdateUserAdminFields", ctx, mock.AnythingOfType("*auth.User"), mock.Anything, mock.Anything, mock.Anything).Return(triggerErr).Once()
 
@@ -986,7 +986,7 @@ func TestService_UpdateUser(t *testing.T) {
 		triggerErr := fmt.Errorf("last_admin_constraint_violation: at least one active member of the Administrators group must remain")
 		mockStore.On("UpdateUserAdminFields", ctx, mock.AnythingOfType("*auth.User"), mock.Anything, mock.Anything, mock.Anything).Return(triggerErr).Once()
 		// DeleteUserSessions must NOT be called: session revocation now
-		// happens only after store.UpdateUser succeeds, so a request that
+		// happens only after store.UpdateUserAdminFields succeeds, so a request that
 		// hits the deferred trigger must not log the user out for a change
 		// that never actually took effect.
 
diff --git a/internal/auth/store_postgres.go b/internal/auth/store_postgres.go
index 1253f4b75..15848f8c6 100644
--- a/internal/auth/store_postgres.go
+++ b/internal/auth/store_postgres.go
@@ -186,72 +186,6 @@ func isLastAdminConstraintViolation(err error) bool {
 	return strings.HasPrefix(err.Error(), sentinel)
 }
 
-// UpdateUser updates an existing user. It rewrites group_ids and active, so it
-// takes the admin-invariant lock first (see adminInvariantAdvisoryLockKey).
-func (s *PostgresStore) UpdateUser(ctx context.Context, user *User) error {
-	user.UpdatedAt = time.Now()
-
-	query := `
-		UPDATE users SET
-			email = $2,
-			password_hash = $3,
-			salt = $4,
-			group_ids = $5,
-			active = $6,
-			mfa_enabled = $7,
-			mfa_secret = $8,
-			mfa_pending_secret = $9,
-			mfa_pending_secret_expires_at = $10,
-			mfa_recovery_codes = $11,
-			password_reset_token = $12,
-			password_reset_expiry = $13,
-			failed_login_attempts = $14,
-			locked_until = $15,
-			password_history = $16,
-			updated_at = $17,
-			last_login_at = $18,
-			deactivated_at = $19
-		WHERE id = $1
-	`
-
-	// See CreateUser for the nil-slice substitution rationale.
-	recoveryCodes := user.MFARecoveryCodes
-	if recoveryCodes == nil {
-		recoveryCodes = []string{}
-	}
-
-	return s.withAdminInvariantLock(ctx, "update user", func(tx pgx.Tx) error {
-		result, err := tx.Exec(ctx, query,
-			user.ID,
-			user.Email,
-			user.PasswordHash,
-			user.Salt,
-			user.GroupIDs,
-			user.Active,
-			user.MFAEnabled,
-			user.MFASecret,
-			user.MFAPendingSecret,
-			user.MFAPendingSecretExpiresAt,
-			recoveryCodes,
-			user.PasswordResetToken,
-			user.PasswordResetExpiry,
-			user.FailedLoginAttempts,
-			user.LockedUntil,
-			user.PasswordHistory,
-			user.UpdatedAt,
-			user.LastLoginAt,
-			user.DeactivatedAt,
-		)
-		if err != nil {
-			return fmt.Errorf("failed to update user: %w", err)
-		}
-		if result.RowsAffected() == 0 {
-			return fmt.Errorf("user not found: %s", user.ID)
-		}
-		return nil
-	})
-}
-
 // DeleteUser deletes a user. Deleting an Administrators-group member lowers
 // the admin count, so it takes the admin-invariant lock first (see
 // adminInvariantAdvisoryLockKey).
diff --git a/internal/auth/store_postgres_admin_lock_db_test.go b/internal/auth/store_postgres_admin_lock_db_test.go
index 547b1eca8..cbcfd7828 100644
--- a/internal/auth/store_postgres_admin_lock_db_test.go
+++ b/internal/auth/store_postgres_admin_lock_db_test.go
@@ -46,11 +46,6 @@ func adminDemotingWrites() []adminDemotingWrite {
 			next.GroupIDs = []string{DefaultPurchaserGroupID}
 			return store.UpdateUserAdminFields(ctx, &next, target.Email, target.GroupIDs, target.Active)
 		}},
-		{"UpdateUser deactivate", func(ctx context.Context, store *PostgresStore, target *User) error {
-			next := *target
-			next.Active = false
-			return store.UpdateUser(ctx, &next)
-		}},
 		{"DeleteUser", func(ctx context.Context, store *PostgresStore, target *User) error {
 			return store.DeleteUser(ctx, target.ID)
 		}},
diff --git a/internal/auth/store_postgres_admin_lock_test.go b/internal/auth/store_postgres_admin_lock_test.go
index 4ab953f65..bfa53c5b1 100644
--- a/internal/auth/store_postgres_admin_lock_test.go
+++ b/internal/auth/store_postgres_admin_lock_test.go
@@ -9,6 +9,7 @@ package auth
 import (
 	"context"
 	"errors"
+	"reflect"
 	"testing"
 	"time"
 
@@ -19,8 +20,6 @@ import (
 )
 
 const (
-	updateUserSQLRegex = `(?s)UPDATE users SET\s+email = \$2,.*deactivated_at = \$19\s+WHERE id = \$1`
-
 	updateUserAdminFieldsSQLRegex = `(?s)UPDATE users SET email = \$2, group_ids = \$3, active = \$4, ` +
 		`deactivated_at = \$5, updated_at = NOW\(\)\s+` +
 		`WHERE id = \$1 AND email = \$6 AND group_ids = \$7 AND active = \$8\s+RETURNING updated_at`
@@ -74,17 +73,6 @@ func adminLockedWrites() []adminLockedWrite {
 		assert.Contains(t, err.Error(), "user not found: "+lockedWriteUserID)
 	}
 	return []adminLockedWrite{
-		{
-			name: "UpdateUser",
-			expect: func(mock pgxmock.PgxPoolIface, outcome writeOutcome) {
-				args := append([]any{lockedWriteUserID}, anyArgs(18)...)
-				expectLockedExec(mock, updateUserSQLRegex, "UPDATE", args, outcome)
-			},
-			call: func(ctx context.Context, store *PostgresStore) error {
-				return store.UpdateUser(ctx, &User{ID: lockedWriteUserID, GroupIDs: []string{DefaultPurchaserGroupID}})
-			},
-			assertNoRow: assertUserNotFound,
-		},
 		{
 			name: "UpdateUserAdminFields",
 			expect: func(mock pgxmock.PgxPoolIface, outcome writeOutcome) {
@@ -224,6 +212,22 @@ func TestPGXMock_AdminWrites_CommitTriggerViolationStaysDetectable(t *testing.T)
 	}
 }
 
+// TestStore_HasNoWholeRowUserWriter pins issue #583: the whole-row
+// UpdateUser(ctx, *User) is gone from the store, so adminLockedWrites above
+// lists every store write that can lower the active-admin count. A new caller
+// has to pick a column-scoped writer, and cannot bring back a stale whole-row
+// save that overwrites MFA, password or membership changes made since its
+// read.
+func TestStore_HasNoWholeRowUserWriter(t *testing.T) {
+	for _, typ := range []reflect.Type{
+		reflect.TypeFor[StoreInterface](),
+		reflect.TypeFor[*PostgresStore](),
+	} {
+		_, found := typ.MethodByName("UpdateUser")
+		assert.False(t, found, "%s still has the whole-row UpdateUser", typ)
+	}
+}
+
 func TestPGXMock_UpdateUserAdminFields_ReturnsStoredUpdatedAt(t *testing.T) {
 	mock := newAuthPgxMock(t)
 	stored := time.Unix(1700000000, 0)
diff --git a/internal/auth/test_helpers.go b/internal/auth/test_helpers.go
index c09ce3d55..3384aaf9d 100644
--- a/internal/auth/test_helpers.go
+++ b/internal/auth/test_helpers.go
@@ -45,11 +45,6 @@ func (m *MockStore) CreateUser(ctx context.Context, user *User) error {
 	return args.Error(0)
 }
 
-func (m *MockStore) UpdateUser(ctx context.Context, user *User) error {
-	args := m.Called(ctx, user)
-	return args.Error(0)
-}
-
 func (m *MockStore) UpdateUserAdminFields(ctx context.Context, user *User, readEmail string, readGroupIDs []string, readActive bool) error {
 	return m.Called(ctx, user, readEmail, readGroupIDs, readActive).Error(0)
 }
diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go
index 38b64862f..296db3f4c 100644
--- a/internal/mocks/stores.go
+++ b/internal/mocks/stores.go
@@ -1069,12 +1069,6 @@ func (m *MockAuthStore) CreateUser(ctx context.Context, user *auth.User) error {
 	return args.Error(0)
 }
 
-// UpdateUser mocks the UpdateUser operation.
-func (m *MockAuthStore) UpdateUser(ctx context.Context, user *auth.User) error {
-	args := m.Called(ctx, user)
-	return args.Error(0)
-}
-
 func (m *MockAuthStore) UpdateUserAdminFields(ctx context.Context, user *auth.User, readEmail string, readGroupIDs []string, readActive bool) error {
 	return m.Called(ctx, user, readEmail, readGroupIDs, readActive).Error(0)
 }
diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go
index a1dfd4fee..2009aa7eb 100644
--- a/internal/server/adapter_test.go
+++ b/internal/server/adapter_test.go
@@ -354,7 +354,7 @@ func TestAuthServiceAdapter_UpdateUserAPI(t *testing.T) {
 	// (issue #907 self-escalation guard). Actor != target, so the
 	// self-escalation guard skips; the group change does not remove the
 	// Administrators group, so the last-admin guard skips; the store
-	// UpdateUser call must therefore fire. Without that, the test silently
+	// UpdateUserAdminFields call must therefore fire. Without that, the test silently
 	// passes even if the adapter stopped forwarding actorUserID, which is
 	// exactly the regression CR flagged.
 	_, err := adapter.UpdateUserAPI(ctx, "actor-1", "user-1", auth.APIUpdateUserRequest{
diff --git a/internal/server/health_test.go b/internal/server/health_test.go
index 53727f78c..a67e3dfa3 100644
--- a/internal/server/health_test.go
+++ b/internal/server/health_test.go
@@ -28,10 +28,6 @@ func (m *mockAuthStoreForHealth) CreateUser(ctx context.Context, user *auth.User
 	return nil
 }
 
-func (m *mockAuthStoreForHealth) UpdateUser(ctx context.Context, user *auth.User) error {
-	return nil
-}
-
 func (m *mockAuthStoreForHealth) UpdateUserAdminFields(context.Context, *auth.User, string, []string, bool) error {
 	return nil
 }