From 84063d75fc4453158532e4e0cdf2a67e161690fd Mon Sep 17 00:00:00 2001 From: onysd Date: Fri, 31 Jul 2026 16:57:57 +0300 Subject: [PATCH] better fix --- internal/app/auth/service.go | 29 +++++++++++++---- internal/app/auth/signup_state_test.go | 38 +++++++++++++++++++++++ internal/rpc/account.go | 2 +- internal/rpc/account_phone.go | 2 +- internal/rpc/auth.go | 27 +++++++++++++--- internal/rpc/auth_login_email_rpc_test.go | 38 +++++++++++++++++++++++ internal/rpc/rpc_testkit_auth_test.go | 5 +++ 7 files changed, 128 insertions(+), 13 deletions(-) diff --git a/internal/app/auth/service.go b/internal/app/auth/service.go index 3797fce8..da2c378a 100644 --- a/internal/app/auth/service.go +++ b/internal/app/auth/service.go @@ -863,17 +863,34 @@ func (s *Service) CancelCodeForAuthKey(ctx context.Context, authKeyID [8]byte, p return s.cancelCode(ctx, authKeyID, phone, phoneCodeHash) } +// LoginEmailResetAvailable reports whether auth.resetLoginEmail's SMS +// fallback could actually succeed on this deployment -- the exact same +// condition ConsumeLoginEmailReset enforces. The RPC layer uses this to +// decide whether to advertise reset_available_period at all, so the client +// never offers a "Can't access this email?" escape hatch that can only ever +// fail (or, before this was locked down, silently succeed with the +// well-known fixed dev code). +func (s *Service) LoginEmailResetAvailable() bool { + return s.phoneCodeSender != nil && !s.emailSignupEnabled +} + // ConsumeLoginEmailReset authorizes auth.resetLoginEmail with the exact // email-login hash previously issued for this phone owner. Possession of only // a phone number is never sufficient to remove an authentication factor. func (s *Service) ConsumeLoginEmailReset(ctx context.Context, phone, phoneCodeHash string) (int64, error) { // This flow exists to fall back to an SMS code when the login email is - // unreachable. Without a real phoneCodeSender configured, that "SMS code" - // is always the well-known TELESRV_DEV_AUTH_CODE (see createPhoneCode), - // so anyone who can call sendCode for a phone (no email access required) - // could strip the login-email requirement with a publicly known code. - // Refuse up front, before ClearLoginEmail runs, so nothing is mutated. - if s.phoneCodeSender == nil { + // unreachable. Two independent reasons it must refuse outright, before + // ClearLoginEmail runs so nothing is ever mutated on a doomed request: + // - no real phoneCodeSender: the "SMS code" is always the well-known + // TELESRV_DEV_AUTH_CODE (see createPhoneCode), so anyone who can call + // sendCode for a phone (no email access required) could strip the + // login-email requirement with a publicly known code. + // - emailSignupEnabled: this account's "phone" is a synthetic 888- + // prefixed display number (domain.NewEmailSignupDisplayPhone), never + // a real number anyone can receive SMS on. Email is the actual + // identity here regardless of whether a real SMS sender happens to + // be configured for other (real-phone) accounts on this server. + if s.phoneCodeSender == nil || s.emailSignupEnabled { return 0, ErrCodeInvalid } phone = normalizePhone(phone) diff --git a/internal/app/auth/signup_state_test.go b/internal/app/auth/signup_state_test.go index 99e58b31..0ce646ca 100644 --- a/internal/app/auth/signup_state_test.go +++ b/internal/app/auth/signup_state_test.go @@ -391,6 +391,44 @@ func TestEmailSetupVerificationAuthorizesSignUpWithWelcomeMessageOnlyNoCodeEcho( } } +// TestLoginEmailResetUnavailableForEmailSignupAccounts locks down that the +// SMS-fallback reset must stay refused for email-signup accounts even with a +// real phoneCodeSender configured: their "phone" is a synthetic display +// number nobody can receive SMS on, so email is the only real identity +// factor and must never be strippable via this escape hatch. +func TestLoginEmailResetUnavailableForEmailSignupAccounts(t *testing.T) { + ctx := context.Background() + users := memory.NewUserStore() + owner, err := users.Create(ctx, domain.User{Phone: "88800009999", FirstName: "Owner"}) + if err != nil { + t.Fatalf("create owner: %v", err) + } + codes := memory.NewCodeStore() + hash := "email-signup-reset" + if err := codes.Set(ctx, hash, store.PhoneCode{ + Version: store.PhoneCodeVersionCurrent, + IssuedUserID: owner.ID, + Phone: owner.Phone, + Code: "654321", + Channel: codeChannelEmailLogin, + MaxAttempts: 5, + }, time.Minute); err != nil { + t.Fatalf("seed code: %v", err) + } + svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", + WithPhoneCodeDelivery(&captureOTPSender{}, 5), WithEmailSignup(true)) + + if svc.LoginEmailResetAvailable() { + t.Fatalf("LoginEmailResetAvailable = true, want false for an email-signup deployment") + } + if _, err := svc.ConsumeLoginEmailReset(ctx, owner.Phone, hash); !errors.Is(err, ErrCodeInvalid) { + t.Fatalf("ConsumeLoginEmailReset err=%v, want ErrCodeInvalid", err) + } + if _, found, err := codes.Get(ctx, hash); err != nil || !found { + t.Fatalf("reset probe destroyed the seeded code found=%v err=%v", found, err) + } +} + func TestConsumeLoginEmailResetRequiresExactIssuedHash(t *testing.T) { ctx := context.Background() baseUsers := memory.NewUserStore() diff --git a/internal/rpc/account.go b/internal/rpc/account.go index 8f9c79e7..047b8a2d 100644 --- a/internal/rpc/account.go +++ b/internal/rpc/account.go @@ -792,7 +792,7 @@ func (r *Router) onAccountVerifyEmail(ctx context.Context, req *tg.AccountVerify if ClientTypeFrom(ctx) == ClientTypeAndroid { return &tg.AccountEmailVerifiedLogin{ Email: email, - SentCode: tgEmailSentCode(p.PhoneCodeHash, domain.MaskEmail(email), len(strings.TrimSpace(code))), + SentCode: tgEmailSentCode(p.PhoneCodeHash, domain.MaskEmail(email), len(strings.TrimSpace(code)), true), }, nil } u, _, needSignUp, signInErr := r.deps.Auth.SignInWithEmail(ctx, r.authzFromCtx(ctx), p.PhoneNumber, p.PhoneCodeHash, code) diff --git a/internal/rpc/account_phone.go b/internal/rpc/account_phone.go index f495a416..7d6d6cef 100644 --- a/internal/rpc/account_phone.go +++ b/internal/rpc/account_phone.go @@ -36,7 +36,7 @@ func (r *Router) onAccountSendChangePhoneCode(ctx context.Context, req *tg.Accou return nil, phoneChangeErr(err) } if delivery.Kind == domain.AuthCodeDeliveryEmail { - return tgEmailSentCode(hash, delivery.EmailPattern, delivery.Length), nil + return tgEmailSentCode(hash, delivery.EmailPattern, delivery.Length, true), nil } return tgSMSSentCode(hash, delivery.Length), nil } diff --git a/internal/rpc/auth.go b/internal/rpc/auth.go index 5529f6ff..5f9e1155 100644 --- a/internal/rpc/auth.go +++ b/internal/rpc/auth.go @@ -499,7 +499,7 @@ func tgSMSSentCode(hash string, length int) tg.AuthSentCodeClass { } } -func tgEmailSentCode(hash, emailPattern string, length int) tg.AuthSentCodeClass { +func tgEmailSentCode(hash, emailPattern string, length int, resetAvailable bool) tg.AuthSentCodeClass { if length <= 0 { length = devCodeLength } @@ -507,9 +507,14 @@ func tgEmailSentCode(hash, emailPattern string, length int) tg.AuthSentCodeClass EmailPattern: emailPattern, Length: length, } - // reset_available_period=0 表示可立即调用 auth.resetLoginEmail（开发环境无等待期）， - // 让客户端的"无法访问邮箱?"逃生入口可用。 - codeType.SetResetAvailablePeriod(0) + if resetAvailable { + // reset_available_period=0 表示可立即调用 auth.resetLoginEmail（开发环境无等待期）， + // 让客户端的"无法访问邮箱?"逃生入口可用。留空（不调用 Set）时该入口在客户端 + // 完全不显示——见 auth.Service.LoginEmailResetAvailable：这个逃生入口本来就 + // 走不通（没有真实短信通道，或邮箱本身就是身份、没有"手机"可退回)时，不应该 + // 让用户看到一个点了也没用、甚至只会用固定 dev code 顶替的按钮。 + codeType.SetResetAvailablePeriod(0) + } return &tg.AuthSentCode{ Type: codeType, PhoneCodeHash: hash, @@ -523,6 +528,14 @@ func tgEmailSetupRequiredSentCode(hash string) tg.AuthSentCodeClass { } } +// loginEmailResetAvailabilityChecker lets tgSentCodeForHash ask whether +// auth.resetLoginEmail could actually succeed right now, so the client is +// never shown a "Can't access this email?" escape hatch it cannot use (see +// auth.Service.LoginEmailResetAvailable / ConsumeLoginEmailReset). +type loginEmailResetAvailabilityChecker interface { + LoginEmailResetAvailable() bool +} + func (r *Router) tgSentCodeForHash(ctx context.Context, hash string) (tg.AuthSentCodeClass, error) { if r.deps.Auth == nil { return tgSentCode(hash), nil @@ -538,7 +551,11 @@ func (r *Router) tgSentCodeForHash(ctx context.Context, hash string) (tg.AuthSen case domain.AuthCodeDeliverySMS: return tgSMSSentCode(hash, delivery.Length), nil case domain.AuthCodeDeliveryEmail: - return tgEmailSentCode(hash, delivery.EmailPattern, delivery.Length), nil + resetAvailable := false + if checker, ok := r.deps.Auth.(loginEmailResetAvailabilityChecker); ok { + resetAvailable = checker.LoginEmailResetAvailable() + } + return tgEmailSentCode(hash, delivery.EmailPattern, delivery.Length, resetAvailable), nil case domain.AuthCodeDeliveryEmailSetupRequired: return tgEmailSetupRequiredSentCode(hash), nil default: diff --git a/internal/rpc/auth_login_email_rpc_test.go b/internal/rpc/auth_login_email_rpc_test.go index dc7a5150..033623c4 100644 --- a/internal/rpc/auth_login_email_rpc_test.go +++ b/internal/rpc/auth_login_email_rpc_test.go @@ -47,6 +47,44 @@ func TestEmailSentCodeUsesDeliveryLength(t *testing.T) { } } +// TestEmailSentCodeOmitsResetPeriodWhenUnavailable locks down the client +// signal for "this server can't service auth.resetLoginEmail" (no real SMS +// sender, or login email is this account's actual identity): the flags- +// optional reset_available_period field must be entirely absent, not merely +// 0, since 0 is also what "available right now" looks like on the wire. +func TestEmailSentCodeOmitsResetPeriodWhenUnavailable(t *testing.T) { + for _, tc := range []struct { + name string + resetAvailable bool + }{ + {name: "unavailable", resetAvailable: false}, + {name: "available", resetAvailable: true}, + } { + t.Run(tc.name, func(t *testing.T) { + authSvc := &captureAuthService{ + codeDelivery: domain.AuthCodeDelivery{ + Kind: domain.AuthCodeDeliveryEmail, + EmailPattern: "a***e@example.test", + Length: 6, + }, + resetAvailable: tc.resetAvailable, + } + r := New(Config{}, Deps{Auth: authSvc}, zaptest.NewLogger(t), fixedClock{now: time.Unix(1700000000, 0)}) + + sent, err := r.tgSentCodeForHash(context.Background(), "hash-email") + if err != nil { + t.Fatalf("tgSentCodeForHash: %v", err) + } + code := sent.(*tg.AuthSentCode) + emailType := code.Type.(*tg.AuthSentCodeTypeEmailCode) + _, ok := emailType.GetResetAvailablePeriod() + if ok != tc.resetAvailable { + t.Fatalf("reset_available_period present = %v, want %v", ok, tc.resetAvailable) + } + }) + } +} + func TestAuthSignInRoutesOfficialEmailCodeCarriers(t *testing.T) { const ( phone = "+86 188 0000 0021" diff --git a/internal/rpc/rpc_testkit_auth_test.go b/internal/rpc/rpc_testkit_auth_test.go index 8661fa0c..7c0ffe9f 100644 --- a/internal/rpc/rpc_testkit_auth_test.go +++ b/internal/rpc/rpc_testkit_auth_test.go @@ -43,6 +43,11 @@ type captureAuthService struct { signInWithEmailPhone string signInWithEmailHash string signInWithEmailCode string + resetAvailable bool +} + +func (s *captureAuthService) LoginEmailResetAvailable() bool { + return s.resetAvailable } type blockingUserAuthService struct {