users: never cache a deleted user's base row

redisstore.userBaseValue has no Deleted/DeletedAt/Status field, so caching a
deleted user silently reset Deleted back to false (and Status to the zero
UserStatusUnknown) on every round trip. That never self-healed: each later
cache miss reloaded the correctly tombstoned DB row and immediately
re-corrupted it on write, so once anyone looked a deleted account up, it kept
showing a blank name with "last seen recently" instead of "Deleted Account".

Keep deleted users off the base cache entirely so lookups always hit the
authoritative store, and stop presence overlay from touching a Deleted user's
Status at all as defense in depth.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

cmd/createuser/main.go

@@ -27,11 +27,14 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/binary"
+	"errors"
 	"flag"
 	"fmt"
 	"os"
 	"time"
 
+	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/jackc/pgx/v5/pgxpool"
 
 	"telesrv/internal/config"
@@ -124,7 +127,7 @@ func main() {
 
 	var createdID int64
 	if err := row.Scan(&createdID); err != nil {
-		fmt.Fprintf(os.Stderr, "createuser: id %d already exists (or insert failed): %v\n", *id, err)
+		fmt.Fprintln(os.Stderr, describeInsertFailure(*id, *username, displayPhone, signupEmail, err))
 		os.Exit(1)
 	}
 
@@ -132,6 +135,31 @@ func main() {
 		createdID, accessHash, *firstName, *lastName, *username, displayPhone, signupEmail)
 }
 
+// describeInsertFailure turns the INSERT's failure into a message naming the
+// actual thing that collided, instead of "id already exists" for every case:
+// ON CONFLICT (id) DO NOTHING only covers the id itself, so a duplicate
+// username/phone/signup_email surfaces here as a distinct unique-violation
+// error (pgx.ErrNoRows only means the id itself was the conflict).
+func describeInsertFailure(id int64, username, phone, signupEmail string, err error) string {
+	var pgErr *pgconn.PgError
+	if errors.As(err, &pgErr) && pgErr.Code == "23505" {
+		switch pgErr.ConstraintName {
+		case "users_username_lower_unique_idx":
+			return fmt.Sprintf("createuser: username %q is already taken", username)
+		case "users_phone_unique_idx":
+			return fmt.Sprintf("createuser: phone %q is already in use", phone)
+		case "users_signup_email_lower_unique_idx":
+			return fmt.Sprintf("createuser: email %q is already in use by another account", signupEmail)
+		default:
+			return fmt.Sprintf("createuser: unique constraint %q violated: %v", pgErr.ConstraintName, err)
+		}
+	}
+	if errors.Is(err, pgx.ErrNoRows) {
+		return fmt.Sprintf("createuser: id %d already exists", id)
+	}
+	return fmt.Sprintf("createuser: insert failed: %v", err)
+}
+
 // assignEmailSignupDisplayPhone mirrors internal/app/auth/service.go's method
 // of the same name: pick a random "888"-prefixed display phone and re-roll on
 // the astronomically unlikely collision with an existing account's phone.

internal/app/users/service.go

@@ -769,7 +769,16 @@ func (s *Service) putCachedUsers(ctx context.Context, users ...domain.User) {
 		// Collectible ownership may change inside the star-gift aggregate. Keep
 		// these uncommon users on the authoritative store path so the database
 		// lifecycle trigger can never be masked by a stale base-user cache entry.
-		if user.ID != 0 && user.EmojiStatusCollectible.Empty() {
+		//
+		// redisstore.userBaseValue has no field for Deleted/DeletedAt/Status: caching
+		// a deleted user silently resets Deleted to false (and Status to the zero
+		// UserStatusUnknown) on every round trip, which never self-heals -- each
+		// subsequent miss reloads the correctly tombstoned DB row and immediately
+		// re-corrupts it on write. That regressed a deleted account back to looking
+		// live (blank name, but "last seen recently" instead of "Deleted Account").
+		// Keep deleted users off the cache so lookups always hit the authoritative
+		// store, which encodes the tombstone correctly.
+		if user.ID != 0 && user.EmojiStatusCollectible.Empty() && !user.Deleted {
 			cacheable = append(cacheable, user)
 		}
 	}

internal/app/users/service_test.go

@@ -397,6 +397,69 @@ func TestServiceUsesBaseCacheWithoutCachingViewerOverlay(t *testing.T) {
 	}
 }
 
+// deletedOverrideUserStore reports one user id as an already-tombstoned
+// domain.User regardless of what the underlying memory store holds, so the
+// test doesn't need a memory.UserStore deletion helper to exercise the
+// base-user cache's handling of deleted users.
+type deletedOverrideUserStore struct {
+	*memory.UserStore
+	deletedID  int64
+	byIDsCalls int
+}
+
+func (s *deletedOverrideUserStore) ByIDs(ctx context.Context, ids []int64) ([]domain.User, error) {
+	s.byIDsCalls++
+	users, err := s.UserStore.ByIDs(ctx, ids)
+	if err != nil {
+		return nil, err
+	}
+	for i, u := range users {
+		if u.ID == s.deletedID {
+			users[i] = domain.User{ID: u.ID, Deleted: true, Status: domain.UserStatus{Kind: domain.UserStatusEmpty}}
+		}
+	}
+	return users, nil
+}
+
+// TestServiceNeverCachesDeletedUser guards the redisstore.UserCache cache
+// schema gap: userBaseValue carries no Deleted/DeletedAt/Status field, so
+// caching a deleted user silently resets Deleted to false (and Status to the
+// zero UserStatusUnknown) on every round trip -- a bug that never self-heals,
+// since each subsequent cache miss reloads the correctly tombstoned row and
+// immediately re-corrupts it on write. It regressed a deleted account back to
+// looking live: blank name (still blank, that part survives), but "last seen
+// recently" instead of "Deleted Account". The service must keep deleted users
+// off the base cache entirely so every lookup hits the authoritative store.
+func TestServiceNeverCachesDeletedUser(t *testing.T) {
+	ctx := context.Background()
+	base := memory.NewUserStore()
+	owner, err := base.Create(ctx, domain.User{AccessHash: 1, Phone: "15550000031", FirstName: "Owner"})
+	if err != nil {
+		t.Fatalf("create owner: %v", err)
+	}
+	target, err := base.Create(ctx, domain.User{AccessHash: 2, Phone: "15550000032", FirstName: "Target"})
+	if err != nil {
+		t.Fatalf("create target: %v", err)
+	}
+	store := &deletedOverrideUserStore{UserStore: base, deletedID: target.ID}
+	cache := newMemoryBaseUserCache()
+	svc := NewService(store, WithBaseUserCache(cache))
+
+	got, found, err := svc.ByID(ctx, owner.ID, target.ID)
+	if err != nil || !found || !got.Deleted {
+		t.Fatalf("ByID = %+v found=%v err=%v, want a deleted user", got, found, err)
+	}
+	if _, cached := cache.users[target.ID]; cached {
+		t.Fatalf("cache holds deleted user %d, want it kept off the cache entirely", target.ID)
+	}
+	if _, found, err := svc.ByID(ctx, owner.ID, target.ID); err != nil || !found {
+		t.Fatalf("second ByID found=%v err=%v", found, err)
+	}
+	if store.byIDsCalls != 2 {
+		t.Fatalf("store ByIDs calls = %d, want 2 (deleted user must never be served from cache)", store.byIDsCalls)
+	}
+}
+
 func TestServiceRefreshesBaseCacheAfterProfileUpdate(t *testing.T) {
 	ctx := context.Background()
 	base := memory.NewUserStore()

internal/domain/account.go

@@ -7,6 +7,7 @@ import (
 
 var (
 	ErrPasswordHashInvalid     = errors.New("password hash invalid")
+	ErrPasswordMissing         = errors.New("password missing")
 	ErrSRPIDInvalid            = errors.New("srp id invalid")
 	ErrSRPPasswordChanged      = errors.New("srp password changed")
 	ErrNewSettingsInvalid      = errors.New("new password settings invalid")

internal/rpc/channels_leave_rpc_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/iamxvbaba/td/tgerr"
 	"go.uber.org/zap/zaptest"
 
+	appaccount "telesrv/internal/app/account"
 	appchannels "telesrv/internal/app/channels"
 	appusers "telesrv/internal/app/users"
 	"telesrv/internal/domain"
@@ -26,6 +27,10 @@ func (acceptPasswordAccountService) CheckPassword(_ context.Context, _ int64, ch
 	return nil
 }
 
+func (acceptPasswordAccountService) GetPassword(_ context.Context, _ int64) (domain.PasswordSettings, error) {
+	return domain.PasswordSettings{HasPassword: true}, nil
+}
+
 func TestMessagesGetFutureChatCreatorAfterLeaveAndCreatorLeaveTransfers(t *testing.T) {
 	ctx := context.Background()
 	userStore := memory.NewUserStore()
@@ -242,6 +247,58 @@ func TestMessagesEditChatCreatorTransfersWithoutChannelPts(t *testing.T) {
 	}
 }
 
+// TestMessagesEditChatCreatorRequiresPasswordSetup covers an owner who has never
+// enabled two-step verification: the client's transfer-ownership probe
+// (inputUserEmpty + inputCheckPasswordEmpty) must get PASSWORD_MISSING, not
+// PASSWORD_HASH_INVALID -- the desktop client only recognizes PASSWORD_MISSING
+// to show its "enable 2FA first" box, and otherwise falls through into a real
+// password-entry flow it can't satisfy.
+func TestMessagesEditChatCreatorRequiresPasswordSetup(t *testing.T) {
+	ctx := context.Background()
+	userStore := memory.NewUserStore()
+	owner, err := userStore.Create(ctx, domain.User{AccessHash: 9221, Phone: "15550009221", FirstName: "Owner"})
+	if err != nil {
+		t.Fatalf("create owner: %v", err)
+	}
+	member, err := userStore.Create(ctx, domain.User{AccessHash: 9222, Phone: "15550009222", FirstName: "Member"})
+	if err != nil {
+		t.Fatalf("create member: %v", err)
+	}
+	channelStore := memory.NewChannelStore()
+	channelService := appchannels.NewService(channelStore)
+	r := New(Config{}, Deps{
+		Account:  appaccount.NewService(memory.NewPasswordStore()),
+		Users:    appusers.NewService(userStore),
+		Channels: channelService,
+	}, zaptest.NewLogger(t), fixedClock{now: time.Unix(1700009130, 0)})
+	created, err := channelService.CreateChannel(ctx, owner.ID, domain.CreateChannelRequest{
+		CreatorUserID: owner.ID,
+		Title:         "no password owner",
+		Megagroup:     true,
+		MemberUserIDs: []int64{member.ID},
+		Date:          1700009130,
+	})
+	if err != nil {
+		t.Fatalf("create channel: %v", err)
+	}
+	ownerCtx := WithUserID(ctx, owner.ID)
+	peer := &tg.InputPeerChannel{ChannelID: created.Channel.ID, AccessHash: created.Channel.AccessHash}
+	if _, err := r.onMessagesEditChatCreator(ownerCtx, &tg.MessagesEditChatCreatorRequest{
+		Peer:     peer,
+		UserID:   &tg.InputUserEmpty{},
+		Password: &tg.InputCheckPasswordEmpty{},
+	}); err == nil || !tgerr.Is(err, "PASSWORD_MISSING") {
+		t.Fatalf("editChatCreator probe err = %v, want PASSWORD_MISSING", err)
+	}
+	if _, err := r.onMessagesEditChatCreator(ownerCtx, &tg.MessagesEditChatCreatorRequest{
+		Peer:     peer,
+		UserID:   &tg.InputUser{UserID: member.ID, AccessHash: member.AccessHash},
+		Password: &tg.InputCheckPasswordSRP{SRPID: 1, A: []byte{1}, M1: []byte{2}},
+	}); err == nil || !tgerr.Is(err, "PASSWORD_MISSING") {
+		t.Fatalf("editChatCreator transfer err = %v, want PASSWORD_MISSING", err)
+	}
+}
+
 func TestMessagesGetFutureChatCreatorAfterLeaveNoCandidate(t *testing.T) {
 	ctx := context.Background()
 	userStore := memory.NewUserStore()

internal/rpc/channels_legacy_chat.go

@@ -339,6 +339,20 @@ func (r *Router) onMessagesEditChatCreator(ctx context.Context, req *tg.Messages
 	if req.Password == nil {
 		return nil, passwordHashInvalidErr()
 	}
+	if r.deps.Account == nil {
+		return nil, passwordHashInvalidErr()
+	}
+	// 转让所有权无条件要求已开启两步验证：先探测账号是否设有密码，
+	// 让 messages.editChatCreator 的探测请求（inputUserEmpty + inputCheckPasswordEmpty）
+	// 拿到 PASSWORD_MISSING 而非 PASSWORD_HASH_INVALID —— 客户端只识别前者来展示
+	// “请先开启两步验证”提示，否则会误入真实密码校验流程并在没有密码可核对时崩溃。
+	passwordSettings, err := r.deps.Account.GetPassword(ctx, userID)
+	if err != nil {
+		return nil, internalErr()
+	}
+	if !passwordSettings.HasPassword {
+		return nil, passwordMissingErr()
+	}
 	if _, ok := req.UserID.(*tg.InputUserEmpty); ok {
 		return nil, passwordHashInvalidErr()
 	}
@@ -355,9 +369,6 @@ func (r *Router) onMessagesEditChatCreator(ctx context.Context, req *tg.Messages
 	if target.Bot {
 		return nil, userIDInvalidErr()
 	}
-	if r.deps.Account == nil {
-		return nil, passwordHashInvalidErr()
-	}
 	if err := r.deps.Account.CheckPassword(ctx, userID, domainPasswordCheck(req.Password)); err != nil {
 		return nil, passwordErr(err)
 	}

internal/rpc/errors.go

@@ -291,6 +291,7 @@ func scoreInvalidErr() error { return tgerr.New(400, "SCORE_INVALID") }
 
 func sessionPasswordNeededErr() error { return tgerr.New(401, "SESSION_PASSWORD_NEEDED") }
 func passwordHashInvalidErr() error   { return tgerr.New(400, "PASSWORD_HASH_INVALID") }
+func passwordMissingErr() error       { return tgerr.New(400, "PASSWORD_MISSING") }
 func srpIDInvalidErr() error          { return tgerr.New(400, "SRP_ID_INVALID") }
 func srpPasswordChangedErr() error    { return tgerr.New(400, "SRP_PASSWORD_CHANGED") }
 func newSettingsInvalidErr() error    { return tgerr.New(400, "NEW_SETTINGS_INVALID") }
@@ -468,6 +469,8 @@ func passwordErr(err error) error {
 	switch {
 	case errors.Is(err, domain.ErrPasswordHashInvalid):
 		return passwordHashInvalidErr()
+	case errors.Is(err, domain.ErrPasswordMissing):
+		return passwordMissingErr()
 	case errors.Is(err, domain.ErrSRPIDInvalid):
 		return srpIDInvalidErr()
 	case errors.Is(err, domain.ErrSRPPasswordChanged):

internal/rpc/presence.go

@@ -531,6 +531,13 @@ func (r *Router) withUserPresence(u domain.User) domain.User {
 	if u.Bot {
 		return u
 	}
+	// 已注销账号同理不参与 presence：tgUser/tgSelfUser 对 Deleted 用户直接短路输出
+	// 精简 tombstone，从不读取 Status，这里覆盖与否本应无观测差异——但只靠那一层
+	// 短路里应外合：任何上游把 Deleted 弄丢的 bug（例如缓存往返丢字段）都会让这里
+	// 覆盖出的 Status 变成可见的“最近上线”，掩盖真正的 bug 而不是让它更早炸出来。
+	if u.Deleted {
+		return u
+	}
 	u.Status = r.userPresenceStatusForUser(u)
 	return u
 }