fix for phone generation on email signup

This commit is contained in:
onysd 2026-07-14 00:30:52 +03:00
parent cee960fea0
commit 3409f190b9
20 changed files with 457 additions and 54 deletions

View file

@ -75,8 +75,14 @@ func TestEmailSignupChangePhoneRoutesCodeToDecodedEmail(t *testing.T) {
if err != nil {
t.Fatalf("ChangePhone: %v", err)
}
if result.User.Phone != newPhone {
t.Fatalf("result.User.Phone = %q, want %q", result.User.Phone, newPhone)
// Rebinding to a new email assigns a fresh short "888" display number
// (see auth.Service.SignUp's identical treatment) rather than storing
// the long wire value directly; SignupEmail carries the association.
if result.User.Phone == newPhone || !domain.ValidPhone(result.User.Phone) || domain.IsEmailSignupPhone(result.User.Phone) {
t.Fatalf("result.User.Phone = %q, want a short all-digit 888 display number distinct from the wire value %q", result.User.Phone, newPhone)
}
if result.User.SignupEmail != "newmail@owpengram.local" {
t.Fatalf("result.User.SignupEmail = %q, want newmail@owpengram.local", result.User.SignupEmail)
}
}

View file

@ -81,6 +81,15 @@ func (s *Service) sendChangePhoneCodeByEmail(ctx context.Context, userID int64,
if !ok {
return "", domain.AuthCodeDelivery{}, domain.ErrPhoneNumberInvalid
}
// The wire value never equals any stored users.phone once assigned (see
// ChangePhone), so the generic ByPhone occupied check above is a no-op
// for this path; the real "is this email already someone else's
// account" guard is by SignupEmail instead.
if existing, found, err := s.users.ByEmail(ctx, email); err != nil {
return "", domain.AuthCodeDelivery{}, err
} else if found && existing.ID != userID {
return "", domain.AuthCodeDelivery{}, domain.ErrPhoneNumberOccupied
}
if s.loginEmailSender == nil {
return "", domain.AuthCodeDelivery{}, fmt.Errorf("email signup sender is not configured")
}
@ -162,7 +171,7 @@ func (s *Service) ChangePhone(ctx context.Context, userID int64, authKeyID, orig
if date == 0 {
date = int(time.Now().Unix())
}
result, err := s.phoneChanges.ChangePhone(ctx, domain.PhoneChangeRequest{
req := domain.PhoneChangeRequest{
UserID: userID,
Phone: phone,
Date: date,
@ -171,7 +180,31 @@ func (s *Service) ChangePhone(ctx context.Context, userID int64, authKeyID, orig
// echoes updateUserPhone back to the initiating device and suppresses the wrong session.
ExcludeAuthKeyID: originRawAuthKeyID,
ExcludeSessionID: sessionID,
})
}
// Rebinding to a different email: the account keeps its short "888"
// display number (or gets a fresh one), never the long wire value —
// same reasoning as SignUp. ByPhone above is a no-op for this case since
// the wire value never equals a stored users.phone; the real
// already-bound-elsewhere guard is by SignupEmail.
if s.emailSignupEnabled && domain.IsEmailSignupPhone(phone) {
email, ok := domain.DecodeEmailPhone(phone)
if !ok {
return domain.PhoneChangeResult{}, domain.ErrPhoneNumberInvalid
}
email = domain.NormalizeEmailForPhone(email)
if existing, found, err := s.users.ByEmail(ctx, email); err != nil {
return domain.PhoneChangeResult{}, err
} else if found && existing.ID != userID {
return domain.PhoneChangeResult{}, domain.ErrPhoneNumberOccupied
}
displayPhone, err := s.assignEmailSignupDisplayPhone(ctx)
if err != nil {
return domain.PhoneChangeResult{}, err
}
req.Phone = displayPhone
req.SignupEmail = email
}
result, err := s.phoneChanges.ChangePhone(ctx, req)
if err != nil {
return domain.PhoneChangeResult{}, err
}
@ -205,6 +238,30 @@ func (s *Service) phoneChangeCaller(ctx context.Context, userID int64, authKeyID
return u, nil
}
// maxEmailSignupPhoneAttempts bounds assignEmailSignupDisplayPhone's
// collision-retry loop so a store failure can't spin forever.
const maxEmailSignupPhoneAttempts = 20
// assignEmailSignupDisplayPhone generates a short "888" display number for
// an email-signup account rebinding to a new email (see
// domain.NewEmailSignupDisplayPhone / auth.Service's SignUp counterpart),
// re-rolling on the astronomically unlikely collision with an existing
// account's phone.
func (s *Service) assignEmailSignupDisplayPhone(ctx context.Context) (string, error) {
for i := 0; i < maxEmailSignupPhoneAttempts; i++ {
candidate, err := domain.NewEmailSignupDisplayPhone()
if err != nil {
return "", err
}
if _, found, err := s.users.ByPhone(ctx, candidate); err != nil {
return "", err
} else if !found {
return candidate, nil
}
}
return "", fmt.Errorf("assign email signup display phone: exhausted %d attempts", maxEmailSignupPhoneAttempts)
}
func phoneChangeHash() (string, error) {
var raw [8]byte
if _, err := rand.Read(raw[:]); err != nil {

View file

@ -47,14 +47,20 @@ func TestEmailSignupSendCodeRoutesFreshSignupToEmail(t *testing.T) {
t.Fatalf("needSignUp = false, want true for a brand-new email-signup account")
}
// SignUp itself is completely untouched by email-signup: same call, same
// phone (the 888-encoded value), no email-specific parameter anywhere.
// SignUp itself is called exactly like the plain-phone path (same phone
// argument, no email-specific parameter anywhere), but the account it
// creates gets a short, normal-looking "888" display number instead of
// storing the long email-encoded wire value as its permanent phone; the
// email->user association lives in SignupEmail instead.
created, _, err := svc.SignUp(ctx, domain.Authorization{}, phone, hash, "New", "User")
if err != nil {
t.Fatalf("SignUp: %v", err)
}
if created.Phone != phone {
t.Fatalf("created.Phone = %q, want %q", created.Phone, phone)
if created.Phone == phone || !domain.ValidPhone(created.Phone) || domain.IsEmailSignupPhone(created.Phone) {
t.Fatalf("created.Phone = %q, want a short all-digit 888 display number distinct from the wire value %q", created.Phone, phone)
}
if created.SignupEmail != "newuser@owpengram.local" {
t.Fatalf("created.SignupEmail = %q, want newuser@owpengram.local", created.SignupEmail)
}
}
@ -88,8 +94,11 @@ func TestEmailSignupSignUpSucceedsWithLoginEmailRequireSetupAlsoOn(t *testing.T)
if err != nil {
t.Fatalf("SignUp: %v (this is the loop bug if it fails with ErrCodeInvalid)", err)
}
if created.Phone != phone {
t.Fatalf("created.Phone = %q, want %q", created.Phone, phone)
if created.Phone == phone || !domain.ValidPhone(created.Phone) || domain.IsEmailSignupPhone(created.Phone) {
t.Fatalf("created.Phone = %q, want a short all-digit 888 display number distinct from the wire value %q", created.Phone, phone)
}
if created.SignupEmail != "requiresetup@owpengram.local" {
t.Fatalf("created.SignupEmail = %q, want requiresetup@owpengram.local", created.SignupEmail)
}
}

View file

@ -298,7 +298,7 @@ func (s *Service) CompletePasswordSignIn(ctx context.Context, authKeyID [8]byte)
// deliberately skipped the welcome message while password_pending.
if a, found, err := s.auths.ByAuthKey(ctx, authKeyID); err == nil && found {
if u, found, err := s.users.ByID(ctx, a.UserID); err == nil && found {
s.recordWelcomeMessage(ctx, u.ID, u.Phone)
s.recordWelcomeMessage(ctx, u)
}
}
return nil
@ -345,10 +345,25 @@ func (s *Service) SendCode(ctx context.Context, phone string) (string, error) {
return s.createPhoneCode(ctx, phone, issuedUserID)
}
// currentPhoneOwner resolves the account currently identified by a wire
// phone value. For email-signup phones (see domain.EncodeEmailPhone) the
// account's real users.phone is a short, unrelated "888" display number
// (domain.NewEmailSignupDisplayPhone) assigned at SignUp — the wire value
// itself is never stored — so lookup goes through the decoded email and
// User.SignupEmail instead of ByPhone. Every owner-drift/idempotency
// invariant elsewhere in this file (issuedOwnerMatches, verifyLoginCode,
// SignUp's occupied checks, ...) is expressed purely in terms of "the
// account currentPhoneOwner resolves to", so fixing this one lookup keeps
// all of them correct for email-signup phones with no further changes.
func (s *Service) currentPhoneOwner(ctx context.Context, phone string) (domain.User, bool, error) {
if s == nil || s.users == nil {
return domain.User{}, false, fmt.Errorf("user store is not configured")
}
if s.emailSignupEnabled {
if email, ok := domain.DecodeEmailPhone(phone); ok {
return s.users.ByEmail(ctx, email)
}
}
return s.users.ByPhone(ctx, phone)
}
@ -906,7 +921,7 @@ func (s *Service) finishSignIn(ctx context.Context, auth domain.Authorization, e
// 2FA accounts only really finish authorizing in CompletePasswordSignIn;
// firing the welcome message here too would notify about an attempt that
// never actually got past the password check.
s.recordWelcomeMessage(ctx, existing.ID, existing.Phone)
s.recordWelcomeMessage(ctx, existing)
return existing, domain.Message{}, false, nil
}
@ -993,6 +1008,23 @@ func (s *Service) SignUp(ctx context.Context, auth domain.Authorization, phone,
FirstName: firstName,
LastName: lastName,
}
// Email-signup accounts don't keep the long email-encoded wire value as
// their permanent phone — that only ever needed to travel on the wire to
// get here. Assign a short, normal-looking "888" number instead and
// record the email separately (SignupEmail), which is what
// currentPhoneOwner uses to find this account again on a later login.
if s.emailSignupEnabled && domain.IsEmailSignupPhone(phone) {
email, ok := domain.DecodeEmailPhone(phone)
if !ok {
return domain.User{}, domain.Message{}, ErrPhoneNumberInvalid
}
displayPhone, err := s.assignEmailSignupDisplayPhone(ctx)
if err != nil {
return domain.User{}, domain.Message{}, err
}
newUser.Phone = displayPhone
newUser.SignupEmail = domain.NormalizeEmailForPhone(email)
}
// 新账号默认赠送会员:到期时间 = 注册时刻 + N 个月(与迁移 0094 对存量
// 账号的 backfill 同一语义)。premium 状态由下发路径按该时间即时派生。
if s.premiumGrantMonths > 0 {
@ -1021,7 +1053,7 @@ func (s *Service) SignUp(ctx context.Context, auth domain.Authorization, phone,
return domain.User{}, domain.Message{}, err
}
}
s.recordWelcomeMessage(ctx, u.ID, phone)
s.recordWelcomeMessage(ctx, u)
return u, loginMessage, nil
}
@ -1329,11 +1361,11 @@ func (s *Service) recordLoginMessage(ctx context.Context, userID int64, code str
// SignIn/SignInWithEmail), regardless of channel. Best-effort: a failure here
// must never fail the sign-in itself, since unlike recordLoginMessage it
// carries no secret the caller needs.
func (s *Service) recordWelcomeMessage(ctx context.Context, userID int64, phone string) {
func (s *Service) recordWelcomeMessage(ctx context.Context, u domain.User) {
if s == nil || s.messages == nil || s.dialogs == nil {
return
}
msg, err := domain.OfficialWelcomeMessage(userID, domain.SignInMethodLabel(phone), int(time.Now().Unix()))
msg, err := domain.OfficialWelcomeMessage(u.ID, domain.SignInMethodLabel(u), int(time.Now().Unix()))
if err != nil {
return
}
@ -1341,7 +1373,7 @@ func (s *Service) recordWelcomeMessage(ctx context.Context, userID int64, phone
if err != nil {
return
}
_ = s.dialogs.UpsertInbox(ctx, userID, domain.Dialog{
_ = s.dialogs.UpsertInbox(ctx, u.ID, domain.Dialog{
Peer: created.Peer,
TopMessage: created.ID,
TopMessageDate: created.Date,
@ -1445,6 +1477,28 @@ func normalizePhone(phone string) string {
return domain.NormalizePhone(phone)
}
// assignEmailSignupDisplayPhone generates a short "888" display number for a
// new email-signup account (see domain.NewEmailSignupDisplayPhone),
// re-rolling on the astronomically unlikely collision with an existing
// account's phone. maxEmailSignupPhoneAttempts bounds the loop so a store
// failure can't spin forever.
const maxEmailSignupPhoneAttempts = 20
func (s *Service) assignEmailSignupDisplayPhone(ctx context.Context) (string, error) {
for i := 0; i < maxEmailSignupPhoneAttempts; i++ {
candidate, err := domain.NewEmailSignupDisplayPhone()
if err != nil {
return "", err
}
if _, found, err := s.users.ByPhone(ctx, candidate); err != nil {
return "", err
} else if !found {
return candidate, nil
}
}
return "", fmt.Errorf("assign email signup display phone: exhausted %d attempts", maxEmailSignupPhoneAttempts)
}
func randomHex(n int) (string, error) {
b := make([]byte, n)
if _, err := rand.Read(b); err != nil {