From 7f0921c82c53e38717b7de82bfb845ac7881845b Mon Sep 17 00:00:00 2001 From: Astra Date: Sun, 13 Sep 2026 22:07:28 +0100 Subject: [PATCH 1/3] tools/createuser: report which field collided on insert failure ON CONFLICT (id) DO NOTHING only catches the id itself, so every other failure (duplicate username/phone/signup_email) was reported as a generic "already exists (or insert failed)" - not useful for telling apart the four distinct causes. Co-Authored-By: Claude Sonnet 5 --- cmd/createuser/main.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/cmd/createuser/main.go b/cmd/createuser/main.go index 579a4e0c..d3557418 100644 --- a/cmd/createuser/main.go +++ b/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. From 798ea70b4eca238b71d3c417618e461bb3bb8234 Mon Sep 17 00:00:00 2001 From: Astra Date: Sun, 13 Sep 2026 22:07:54 +0100 Subject: [PATCH 2/3] rpc: return PASSWORD_MISSING for channel transfer without 2FA messages.editChatCreator unconditionally returned PASSWORD_HASH_INVALID for an account with no cloud password at all. Real Telegram Desktop's transfer- ownership flow only recognizes the distinct PASSWORD_MISSING error to show its "enable 2FA first" box; anything else falls through into the real password-entry flow, which then has nothing to check against and crashes. Co-Authored-By: Claude Sonnet 5 --- internal/domain/account.go | 1 + internal/rpc/channels_leave_rpc_test.go | 57 +++++++++++++++++++++++++ internal/rpc/channels_legacy_chat.go | 17 ++++++-- internal/rpc/errors.go | 3 ++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/internal/domain/account.go b/internal/domain/account.go index 77369673..7ca459e2 100644 --- a/internal/domain/account.go +++ b/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") diff --git a/internal/rpc/channels_leave_rpc_test.go b/internal/rpc/channels_leave_rpc_test.go index 22c09360..a2f3acee 100644 --- a/internal/rpc/channels_leave_rpc_test.go +++ b/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() diff --git a/internal/rpc/channels_legacy_chat.go b/internal/rpc/channels_legacy_chat.go index b3d653ff..999a3dbc 100644 --- a/internal/rpc/channels_legacy_chat.go +++ b/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) } diff --git a/internal/rpc/errors.go b/internal/rpc/errors.go index 564bb558..197ca0d0 100644 --- a/internal/rpc/errors.go +++ b/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): From c1c7db325fc73b3ff53992d80bc8b9374a9deeb9 Mon Sep 17 00:00:00 2001 From: Astra Date: Sun, 13 Sep 2026 22:08:06 +0100 Subject: [PATCH 3/3] 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 --- internal/app/users/service.go | 11 +++++- internal/app/users/service_test.go | 63 ++++++++++++++++++++++++++++++ internal/rpc/presence.go | 7 ++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/internal/app/users/service.go b/internal/app/users/service.go index 2fc78932..f745c175 100644 --- a/internal/app/users/service.go +++ b/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) } } diff --git a/internal/app/users/service_test.go b/internal/app/users/service_test.go index a3029039..553d2293 100644 --- a/internal/app/users/service_test.go +++ b/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() diff --git a/internal/rpc/presence.go b/internal/rpc/presence.go index 92fb5a8d..659e51f3 100644 --- a/internal/rpc/presence.go +++ b/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 }