better fix
This commit is contained in:
parent
87a2a2b0e2
commit
84063d75fc
7 changed files with 128 additions and 13 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue