diff --git a/cmd/createuser/main.go b/cmd/createuser/main.go index d3557418..579a4e0c 100644 --- a/cmd/createuser/main.go +++ b/cmd/createuser/main.go @@ -27,14 +27,11 @@ 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" @@ -127,7 +124,7 @@ func main() { var createdID int64 if err := row.Scan(&createdID); err != nil { - fmt.Fprintln(os.Stderr, describeInsertFailure(*id, *username, displayPhone, signupEmail, err)) + fmt.Fprintf(os.Stderr, "createuser: id %d already exists (or insert failed): %v\n", *id, err) os.Exit(1) } @@ -135,31 +132,6 @@ 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. diff --git a/internal/app/users/service.go b/internal/app/users/service.go index f745c175..2fc78932 100644 --- a/internal/app/users/service.go +++ b/internal/app/users/service.go @@ -769,16 +769,7 @@ 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. - // - // 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 { + if user.ID != 0 && user.EmojiStatusCollectible.Empty() { cacheable = append(cacheable, user) } } diff --git a/internal/app/users/service_test.go b/internal/app/users/service_test.go index 553d2293..a3029039 100644 --- a/internal/app/users/service_test.go +++ b/internal/app/users/service_test.go @@ -397,69 +397,6 @@ 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/domain/account.go b/internal/domain/account.go index 7ca459e2..77369673 100644 --- a/internal/domain/account.go +++ b/internal/domain/account.go @@ -7,7 +7,6 @@ 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 a2f3acee..22c09360 100644 --- a/internal/rpc/channels_leave_rpc_test.go +++ b/internal/rpc/channels_leave_rpc_test.go @@ -9,7 +9,6 @@ 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" @@ -27,10 +26,6 @@ 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() @@ -247,58 +242,6 @@ 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 999a3dbc..b3d653ff 100644 --- a/internal/rpc/channels_legacy_chat.go +++ b/internal/rpc/channels_legacy_chat.go @@ -339,20 +339,6 @@ 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() } @@ -369,6 +355,9 @@ 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 197ca0d0..564bb558 100644 --- a/internal/rpc/errors.go +++ b/internal/rpc/errors.go @@ -291,7 +291,6 @@ 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") } @@ -469,8 +468,6 @@ 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): diff --git a/internal/rpc/presence.go b/internal/rpc/presence.go index 659e51f3..92fb5a8d 100644 --- a/internal/rpc/presence.go +++ b/internal/rpc/presence.go @@ -531,13 +531,6 @@ 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 }