diff --git a/deploy/migrations/0001_init.up.sql b/deploy/migrations/0001_init.up.sql index 2ef5e6b0..db244937 100644 --- a/deploy/migrations/0001_init.up.sql +++ b/deploy/migrations/0001_init.up.sql @@ -1237,9 +1237,7 @@ CREATE TABLE public.account_passwords ( srp_verifier bytea DEFAULT '\x'::bytea NOT NULL, srp_b_secret bytea DEFAULT '\x'::bytea NOT NULL, srp_b bytea DEFAULT '\x'::bytea NOT NULL, - recovery_email character varying(256) DEFAULT ''::character varying NOT NULL, - recovery_code character varying(32) DEFAULT ''::character varying NOT NULL, - recovery_code_expires_at timestamp with time zone + recovery_email character varying(256) DEFAULT ''::character varying NOT NULL ); diff --git a/internal/app/account/service.go b/internal/app/account/service.go index d3202c78..94d04e74 100644 --- a/internal/app/account/service.go +++ b/internal/app/account/service.go @@ -3,7 +3,9 @@ package account import ( "context" "crypto/rand" + "crypto/sha256" "crypto/subtle" + "encoding/hex" "errors" "fmt" "strings" @@ -26,6 +28,10 @@ const ( codeChannelEmailChange = "email_change" codeChannelEmailLogin = "email_login" codeChannelEmailSetupRequired = "email_setup_required" + codeChannelPasswordRecovery = "password_recovery" + passwordRecoveryCodePrefix = "password-recovery:" + passwordRecoveryCodeTTL = 15 * time.Minute + passwordRecoveryCASRetries = 32 ) // Service 提供账号安全配置查询。 @@ -388,12 +394,48 @@ func (s *Service) RequestPasswordRecovery(ctx context.Context, userID int64) (st if !settings.HasPassword || settings.RecoveryEmail == "" { return "", domain.ErrPasswordRecoveryNA } - settings.RecoveryCode = recoveryCode - settings.RecoveryCodeExpiresAt = time.Now().Unix() + recoveryCodeTTL - if s.passwords != nil { - if err := s.passwords.Save(ctx, userID, settings); err != nil { - return "", err + // Recovery has no development-code fallback. If the server cannot both + // persist and deliver a fresh code, it must report the flow unavailable. + if s == nil || s.codes == nil || s.loginEmailSender == nil { + return "", domain.ErrPasswordRecoveryNA + } + code, err := randomDigits(s.loginEmailCodeLength) + if err != nil { + return "", err + } + deliveryID, err := otpdelivery.NewDeliveryID() + if err != nil { + return "", err + } + expiresAt := time.Now().Add(passwordRecoveryCodeTTL) + key := passwordRecoveryCodeKey(userID) + rec := store.PhoneCode{ + Version: store.PhoneCodeVersionCurrent, + UserID: userID, + Email: normalizeLoginEmail(settings.RecoveryEmail), + Code: code, + DeliveryID: deliveryID, + Channel: codeChannelPasswordRecovery, + MaxAttempts: s.loginEmailCodeMaxAttempts, + RecoveryBinding: passwordRecoveryBinding(settings), + } + if err := s.codes.Set(ctx, key, rec, passwordRecoveryCodeTTL); err != nil { + return "", err + } + if err := deliverOTP(ctx, s.loginEmailSender, otpdelivery.Request{ + DeliveryID: deliveryID, + Purpose: otpdelivery.PurposePasswordRecovery, + Channel: otpdelivery.ChannelEmail, + Recipient: settings.RecoveryEmail, + Code: code, + ExpiresAt: expiresAt, + }); err != nil { + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second) + defer cancel() + if cleanupErr := s.deletePasswordRecoveryCode(cleanupCtx, key, deliveryID); cleanupErr != nil { + return "", fmt.Errorf("deliver password recovery code: %w (cleanup failed: %v)", err, cleanupErr) } + return "", fmt.Errorf("deliver password recovery code: %w", err) } return emailPattern(settings.RecoveryEmail), nil } @@ -403,23 +445,35 @@ func (s *Service) CheckRecoveryPassword(ctx context.Context, userID int64, code if err != nil { return err } - return checkRecoveryCode(settings, code) + if !settings.HasPassword || settings.RecoveryEmail == "" { + return domain.ErrPasswordRecoveryExpired + } + return s.verifyPasswordRecoveryCode(ctx, userID, passwordRecoveryBinding(settings), code, false) } func (s *Service) RecoverPassword(ctx context.Context, userID int64, code string, input *domain.PasswordInputSettings) error { if s == nil || s.passwords == nil || userID == 0 { - return nil + return domain.ErrPasswordRecoveryExpired } settings, err := s.GetPasswordWithoutRefresh(ctx, userID) if err != nil { return err } - if err := checkRecoveryCode(settings, code); err != nil { - return err + if !settings.HasPassword || settings.RecoveryEmail == "" { + return domain.ErrPasswordRecoveryExpired } + binding := passwordRecoveryBinding(settings) if input == nil || len(input.NewPasswordHash) == 0 { - settings = defaultPasswordSettings() - return s.passwords.Save(ctx, userID, settings) + if err := s.verifyPasswordRecoveryCode(ctx, userID, binding, code, true); err != nil { + return err + } + return s.passwords.Save(ctx, userID, defaultPasswordSettings()) + } + // Reject invalid proofs before doing the comparatively expensive SRP + // verifier/challenge work. The final consuming check below still decides + // the single winner if the code changes concurrently. + if err := s.verifyPasswordRecoveryCode(ctx, userID, binding, code, false); err != nil { + return err } if err := validateNewPasswordSettings(*input); err != nil { return err @@ -438,8 +492,9 @@ func (s *Service) RecoverPassword(ctx context.Context, userID int64, code string if input.HasHint { settings.Hint = input.Hint } - settings.RecoveryCode = "" - settings.RecoveryCodeExpiresAt = 0 + if err := s.verifyPasswordRecoveryCode(ctx, userID, binding, code, true); err != nil { + return err + } return s.passwords.Save(ctx, userID, normalizePasswordSettings(settings)) } @@ -498,33 +553,123 @@ func (s *Service) ResendPasswordEmail(ctx context.Context, userID int64) error { } func (s *Service) CancelPasswordEmail(ctx context.Context, userID int64) error { + if s != nil && s.codes != nil && userID != 0 { + if err := s.codes.Del(ctx, passwordRecoveryCodeKey(userID)); err != nil { + return err + } + } settings, err := s.GetPasswordWithoutRefresh(ctx, userID) if err != nil { return err } settings.EmailUnconfirmedPattern = "" - settings.RecoveryCode = "" - settings.RecoveryCodeExpiresAt = 0 if s.passwords != nil { return s.passwords.Save(ctx, userID, settings) } return nil } -func checkRecoveryCode(settings domain.PasswordSettings, code string) error { - if settings.RecoveryCode == "" { - if code == recoveryCode { +func passwordRecoveryCodeKey(userID int64) string { + return passwordRecoveryCodePrefix + fmt.Sprint(userID) +} + +func passwordRecoveryBinding(settings domain.PasswordSettings) string { + sum := sha256.Sum256([]byte(fmt.Sprintf("%d\x00%s", settings.SRPID, normalizeLoginEmail(settings.RecoveryEmail)))) + return hex.EncodeToString(sum[:]) +} + +func (s *Service) deletePasswordRecoveryCode(ctx context.Context, key, deliveryID string) error { + for attempt := 0; attempt < passwordRecoveryCASRetries; attempt++ { + snapshot, found, err := s.codes.GetSnapshot(ctx, key) + if err != nil || !found { + return err + } + if snapshot.Record.Channel != codeChannelPasswordRecovery || snapshot.Record.DeliveryID != deliveryID { return nil } - return domain.ErrPasswordRecoveryNA + deleted, err := s.codes.CompareAndDelete(ctx, key, snapshot.Revision) + if err != nil { + return err + } + if deleted { + return nil + } + if err := ctx.Err(); err != nil { + return err + } } - if settings.RecoveryCodeExpiresAt > 0 && time.Now().Unix() > settings.RecoveryCodeExpiresAt { - return domain.ErrEmailCodeInvalid + return fmt.Errorf("delete password recovery code: concurrent state did not settle") +} + +// verifyPasswordRecoveryCode keeps check non-consuming while making the final +// recovery a single-winner CAS. Wrong attempts are counted atomically and the +// code is removed at the configured threshold. +func (s *Service) verifyPasswordRecoveryCode(ctx context.Context, userID int64, binding, code string, consume bool) error { + code = strings.TrimSpace(code) + if code == "" { + return domain.ErrRecoveryCodeEmpty } - if subtle.ConstantTimeCompare([]byte(settings.RecoveryCode), []byte(code)) != 1 { - return domain.ErrEmailCodeInvalid + if s == nil || s.codes == nil || userID == 0 { + return domain.ErrPasswordRecoveryExpired } - return nil + key := passwordRecoveryCodeKey(userID) + for attempt := 0; attempt < passwordRecoveryCASRetries; attempt++ { + snapshot, found, err := s.codes.GetSnapshot(ctx, key) + if err != nil { + return err + } + if !found { + return domain.ErrPasswordRecoveryExpired + } + rec := snapshot.Record + if rec.Channel != codeChannelPasswordRecovery || rec.UserID != userID || rec.RecoveryBinding == "" || rec.RecoveryBinding != binding { + deleted, err := s.codes.CompareAndDelete(ctx, key, snapshot.Revision) + if err != nil { + return err + } + if deleted { + return domain.ErrPasswordRecoveryExpired + } + continue + } + if subtle.ConstantTimeCompare([]byte(rec.Code), []byte(code)) == 1 { + if !consume { + return nil + } + deleted, err := s.codes.CompareAndDelete(ctx, key, snapshot.Revision) + if err != nil { + return err + } + if deleted { + return nil + } + continue + } + maxAttempts := rec.MaxAttempts + if maxAttempts <= 0 { + maxAttempts = s.loginEmailCodeMaxAttempts + } + if maxAttempts <= 0 { + maxAttempts = 1 + } + rec.Attempts++ + var applied bool + if rec.Attempts >= maxAttempts { + applied, err = s.codes.CompareAndDelete(ctx, key, snapshot.Revision) + } else { + applied, err = s.codes.CompareAndUpdate(ctx, key, snapshot.Revision, rec) + } + if err != nil { + return err + } + if applied { + return domain.ErrRecoveryCodeInvalid + } + if err := ctx.Err(); err != nil { + return err + } + } + return fmt.Errorf("verify password recovery code: concurrent state did not settle") } func randomBytesOrDefault(n int, fallback []byte) []byte { @@ -557,14 +702,24 @@ func randomDigits(n int) (string, error) { if n <= 0 { n = 6 } - b := make([]byte, n) - if _, err := rand.Read(b); err != nil { - return "", err - } var out strings.Builder out.Grow(n) - for _, v := range b { - out.WriteByte(byte('0') + v%10) + var buf [32]byte + for out.Len() < n { + if _, err := rand.Read(buf[:]); err != nil { + return "", err + } + for _, v := range buf { + // Reject the top six values so every digit has exactly 25 source + // byte values instead of inheriting modulo bias from 256 %% 10. + if v >= 250 { + continue + } + out.WriteByte(byte('0') + v%10) + if out.Len() == n { + break + } + } } return out.String(), nil } diff --git a/internal/app/account/service_test.go b/internal/app/account/service_test.go index 0d5b2831..1689cf9e 100644 --- a/internal/app/account/service_test.go +++ b/internal/app/account/service_test.go @@ -6,12 +6,14 @@ import ( "crypto/sha512" "errors" "math/big" + "sync" "testing" "time" "golang.org/x/crypto/pbkdf2" "telesrv/internal/domain" + "telesrv/internal/otpdelivery" "telesrv/internal/store/memory" ) @@ -69,7 +71,9 @@ func TestPasswordSRPRoundTrip(t *testing.T) { func TestRecoverPasswordClearsTwoFactorPassword(t *testing.T) { ctx := context.Background() const userID int64 = 1002 - svc := NewService(memory.NewPasswordStore()) + sender := &captureMailSender{} + svc := NewService(memory.NewPasswordStore(), + WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 3, 6)) initial, err := svc.GetPassword(ctx, userID) if err != nil { @@ -93,7 +97,13 @@ func TestRecoverPasswordClearsTwoFactorPassword(t *testing.T) { if pattern != "b***b@example.com" { t.Fatalf("recovery pattern = %q, want masked email", pattern) } - if err := svc.RecoverPassword(ctx, userID, recoveryCode, nil); err != nil { + if sender.to != "bob@example.com" || sender.code == "" || len(sender.requests) != 1 || sender.requests[0].Purpose != otpdelivery.PurposePasswordRecovery { + t.Fatalf("recovery delivery = %+v, want one password-recovery email", sender) + } + if err := svc.CheckRecoveryPassword(ctx, userID, sender.code); err != nil { + t.Fatalf("CheckRecoveryPassword: %v", err) + } + if err := svc.RecoverPassword(ctx, userID, sender.code, nil); err != nil { t.Fatalf("RecoverPassword clear: %v", err) } cleared, err := svc.GetPassword(ctx, userID) @@ -105,6 +115,155 @@ func TestRecoverPasswordClearsTwoFactorPassword(t *testing.T) { } } +func TestPasswordRecoveryFailsClosedWithoutSenderOrIssuedCode(t *testing.T) { + ctx := context.Background() + const userID int64 = 1012 + passwords := memory.NewPasswordStore() + if err := passwords.Save(ctx, userID, domain.PasswordSettings{ + HasPassword: true, HasRecovery: true, RecoveryEmail: "owner@example.test", + SRPID: 7, SRPVerifier: []byte{1, 2, 3}, + }); err != nil { + t.Fatal(err) + } + svc := NewService(passwords) + if _, err := svc.RequestPasswordRecovery(ctx, userID); !errors.Is(err, domain.ErrPasswordRecoveryNA) { + t.Fatalf("RequestPasswordRecovery err=%v, want unavailable", err) + } + if err := svc.RecoverPassword(ctx, userID, "12345", nil); !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("standing fixed code err=%v, want expired", err) + } + if err := NewService(nil).RecoverPassword(ctx, userID, "12345", nil); !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("missing password store err=%v, want fail-closed expiry", err) + } +} + +func TestPasswordRecoveryAttemptLimitAndStateBinding(t *testing.T) { + ctx := context.Background() + const userID int64 = 1013 + passwords := memory.NewPasswordStore() + settings := domain.PasswordSettings{ + HasPassword: true, HasRecovery: true, RecoveryEmail: "owner@example.test", + SRPID: 8, SRPVerifier: []byte{4, 5, 6}, + } + if err := passwords.Save(ctx, userID, settings); err != nil { + t.Fatal(err) + } + sender := &captureMailSender{} + svc := NewService(passwords, + WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 3, 6)) + if _, err := svc.RequestPasswordRecovery(ctx, userID); err != nil { + t.Fatal(err) + } + for attempt := 1; attempt <= 3; attempt++ { + if err := svc.CheckRecoveryPassword(ctx, userID, "000000"); !errors.Is(err, domain.ErrRecoveryCodeInvalid) { + t.Fatalf("wrong attempt %d err=%v, want invalid", attempt, err) + } + } + if err := svc.CheckRecoveryPassword(ctx, userID, sender.code); !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("code after attempt limit err=%v, want expired", err) + } + + if _, err := svc.RequestPasswordRecovery(ctx, userID); err != nil { + t.Fatal(err) + } + issued := sender.code + settings.SRPID++ + if err := passwords.Save(ctx, userID, settings); err != nil { + t.Fatal(err) + } + if err := svc.CheckRecoveryPassword(ctx, userID, issued); !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("code after 2FA state change err=%v, want expired", err) + } +} + +func TestConcurrentPasswordRecoveryHasSingleConsumer(t *testing.T) { + ctx := context.Background() + const userID int64 = 1014 + passwords := memory.NewPasswordStore() + if err := passwords.Save(ctx, userID, domain.PasswordSettings{ + HasPassword: true, HasRecovery: true, RecoveryEmail: "owner@example.test", + SRPID: 9, SRPVerifier: []byte{7, 8, 9}, + }); err != nil { + t.Fatal(err) + } + sender := &captureMailSender{} + svc := NewService(passwords, + WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 5, 6)) + if _, err := svc.RequestPasswordRecovery(ctx, userID); err != nil { + t.Fatal(err) + } + const workers = 24 + start := make(chan struct{}) + errs := make(chan error, workers) + var wg sync.WaitGroup + for i := 0; i < workers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + errs <- svc.RecoverPassword(ctx, userID, sender.code, nil) + }() + } + close(start) + wg.Wait() + close(errs) + successes := 0 + for err := range errs { + if err == nil { + successes++ + continue + } + if !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("concurrent recovery err=%v", err) + } + } + if successes != 1 { + t.Fatalf("successful recoveries=%d, want 1", successes) + } +} + +func TestPasswordRecoveryDeliveryFailureRemovesIssuedCode(t *testing.T) { + ctx := context.Background() + const userID int64 = 1015 + passwords := memory.NewPasswordStore() + if err := passwords.Save(ctx, userID, domain.PasswordSettings{ + HasPassword: true, HasRecovery: true, RecoveryEmail: "owner@example.test", + SRPID: 10, SRPVerifier: []byte{10}, + }); err != nil { + t.Fatal(err) + } + sender := &captureMailSender{err: errors.New("provider rejected request")} + svc := NewService(passwords, + WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 5, 6)) + if _, err := svc.RequestPasswordRecovery(ctx, userID); err == nil { + t.Fatal("RequestPasswordRecovery succeeded after known delivery failure") + } + if err := svc.CheckRecoveryPassword(ctx, userID, sender.code); !errors.Is(err, domain.ErrPasswordRecoveryExpired) { + t.Fatalf("undelivered code err=%v, want expired", err) + } +} + +func TestPasswordRecoveryUnknownDeliveryOutcomeKeepsIssuedCode(t *testing.T) { + ctx := context.Background() + const userID int64 = 1016 + passwords := memory.NewPasswordStore() + if err := passwords.Save(ctx, userID, domain.PasswordSettings{ + HasPassword: true, HasRecovery: true, RecoveryEmail: "owner@example.test", + SRPID: 11, SRPVerifier: []byte{11}, + }); err != nil { + t.Fatal(err) + } + sender := &captureMailSender{err: &otpdelivery.OutcomeUnknownError{Cause: errors.New("provider ACK lost")}} + svc := NewService(passwords, + WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 5, 6)) + if _, err := svc.RequestPasswordRecovery(ctx, userID); err != nil { + t.Fatalf("RequestPasswordRecovery outcome-unknown err=%v", err) + } + if err := svc.CheckRecoveryPassword(ctx, userID, sender.code); err != nil { + t.Fatalf("outcome-unknown code was discarded: %v", err) + } +} + func TestResetPasswordWaitAndDecline(t *testing.T) { ctx := context.Background() const userID int64 = 1003 diff --git a/internal/app/account/srp.go b/internal/app/account/srp.go index accd809b..e725a11b 100644 --- a/internal/app/account/srp.go +++ b/internal/app/account/srp.go @@ -12,8 +12,6 @@ import ( const ( passwordHashSize = 256 - recoveryCode = "12345" - recoveryCodeTTL = 15 * 60 ) var ( diff --git a/internal/app/auth/service.go b/internal/app/auth/service.go index c1789de8..250f48cc 100644 --- a/internal/app/auth/service.go +++ b/internal/app/auth/service.go @@ -775,10 +775,23 @@ func (s *Service) CancelCodeForAuthKey(ctx context.Context, authKeyID [8]byte, p return s.cancelCode(ctx, authKeyID, phone, phoneCodeHash) } +// LoginEmailResetAvailable reports whether this deployment can complete the +// SMS fallback promised by auth.resetLoginEmail. Fixed development codes are +// deliberately not a recovery channel. +func (s *Service) LoginEmailResetAvailable() bool { + return s != nil && s.phoneCodeSender != nil && s.codes != nil && s.users != nil +} + // 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) { + // Refuse before consuming the email proof or clearing any account state. If + // there is no real SMS sender, the successor code would be the public + // development code and could strip the login-email factor. + if !s.LoginEmailResetAvailable() || s.codes == nil { + return 0, ErrCodeInvalid + } phone = normalizePhone(phone) rec, found, err := s.codes.Get(ctx, phoneCodeHash) if err != nil { diff --git a/internal/app/auth/signup_state_test.go b/internal/app/auth/signup_state_test.go index f3c9eec1..079fca28 100644 --- a/internal/app/auth/signup_state_test.go +++ b/internal/app/auth/signup_state_test.go @@ -399,7 +399,12 @@ func TestConsumeLoginEmailResetRequiresExactIssuedHash(t *testing.T) { users := &switchablePhoneOwnerStore{UserStore: baseUsers} codes := memory.NewCodeStore() delivery := &captureLoginCodeDelivery{} - svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", WithLoginCodeDelivery(delivery)) + otp := &captureOTPSender{} + svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", + WithLoginCodeDelivery(delivery), WithPhoneCodeDelivery(otp, 5)) + if !svc.LoginEmailResetAvailable() { + t.Fatal("LoginEmailResetAvailable=false with real SMS sender") + } seed := func(hash, channel string) { t.Helper() if err := codes.Set(ctx, hash, store.PhoneCode{ @@ -451,11 +456,38 @@ func TestConsumeLoginEmailResetRequiresExactIssuedHash(t *testing.T) { if len(delivery.requests) != 1 || delivery.requests[0].UserID != owner.ID || delivery.requests[0].PhoneCodeHash != replacementHash { t.Fatalf("replacement delivery=%+v", delivery.requests) } - if rec, found, err := codes.Get(ctx, replacementHash); err != nil || !found || rec.Version != store.PhoneCodeVersionCurrent || rec.IssuedUserID != owner.ID || rec.Channel != codeChannelPhone { + if rec, found, err := codes.Get(ctx, replacementHash); err != nil || !found || rec.Version != store.PhoneCodeVersionCurrent || rec.IssuedUserID != owner.ID || rec.Channel != codeChannelSMS { t.Fatalf("replacement code=%+v found=%v err=%v", rec, found, err) } } +func TestLoginEmailResetUnavailableWithoutRealSMSSender(t *testing.T) { + ctx := context.Background() + users := memory.NewUserStore() + owner, err := users.Create(ctx, domain.User{Phone: "15550009339", FirstName: "Owner"}) + if err != nil { + t.Fatal(err) + } + codes := memory.NewCodeStore() + const hash = "unavailable-email-reset" + if err := codes.Set(ctx, hash, store.PhoneCode{ + Version: store.PhoneCodeVersionCurrent, IssuedUserID: owner.ID, + Phone: owner.Phone, Code: "654321", Channel: codeChannelEmailLogin, + }, time.Minute); err != nil { + t.Fatal(err) + } + svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345") + if svc.LoginEmailResetAvailable() { + t.Fatal("LoginEmailResetAvailable=true without real SMS sender") + } + if _, err := svc.ConsumeLoginEmailReset(ctx, owner.Phone, hash); !errors.Is(err, ErrCodeInvalid) { + t.Fatalf("ConsumeLoginEmailReset err=%v, want invalid", err) + } + if _, found, err := codes.Get(ctx, hash); err != nil || !found { + t.Fatalf("unavailable reset consumed proof found=%v err=%v", found, err) + } +} + func TestConcurrentLoginEmailResetHasSingleConsumer(t *testing.T) { ctx := context.Background() users := memory.NewUserStore() @@ -475,7 +507,8 @@ func TestConcurrentLoginEmailResetHasSingleConsumer(t *testing.T) { }, time.Minute); err != nil { t.Fatalf("seed code: %v", err) } - svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345") + svc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", + WithPhoneCodeDelivery(&captureOTPSender{}, 5)) const workers = 24 start := make(chan struct{}) errs := make(chan error, workers) @@ -536,7 +569,8 @@ func TestLoginEmailResetLocksUserAcrossOwnerTransfer(t *testing.T) { t.Fatalf("seed reset code: %v", err) } delivery := &captureLoginCodeDelivery{} - authSvc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", WithLoginCodeDelivery(delivery)) + authSvc := NewService(users, memory.NewAuthorizationStore(), codes, nil, nil, "12345", + WithLoginCodeDelivery(delivery), WithPhoneCodeDelivery(&captureOTPSender{}, 5)) resetUserID, err := authSvc.ConsumeLoginEmailReset(ctx, ownerA.Phone, hash) if err != nil || resetUserID != ownerA.ID { t.Fatalf("ConsumeLoginEmailReset uid=%d err=%v", resetUserID, err) diff --git a/internal/domain/account.go b/internal/domain/account.go index 40c7cf6a..e7235234 100644 --- a/internal/domain/account.go +++ b/internal/domain/account.go @@ -6,24 +6,27 @@ import ( ) var ( - ErrPasswordHashInvalid = errors.New("password hash invalid") - ErrSRPIDInvalid = errors.New("srp id invalid") - ErrSRPPasswordChanged = errors.New("srp password changed") - ErrNewSettingsInvalid = errors.New("new password settings invalid") - ErrNewSaltInvalid = errors.New("new password salt invalid") - ErrPasswordRecoveryNA = errors.New("password recovery not available") - ErrEmailCodeInvalid = errors.New("email code invalid") - ErrEmailInvalid = errors.New("email invalid") - ErrEmailNotAllowed = errors.New("email not allowed") - ErrEmailOccupied = errors.New("email occupied") - ErrSessionPasswordNeeded = errors.New("session password needed") - ErrPhoneNumberInvalid = errors.New("phone number invalid") - ErrPhoneNumberOccupied = errors.New("phone number occupied") - ErrPhoneCodeEmpty = errors.New("phone code empty") - ErrPhoneCodeInvalid = errors.New("phone code invalid") - ErrPhoneCodeExpired = errors.New("phone code expired") - ErrPhoneChangeAuthInvalid = errors.New("phone change auth invalid") - ErrPhoneChangeForbidden = errors.New("phone change forbidden") + ErrPasswordHashInvalid = errors.New("password hash invalid") + ErrSRPIDInvalid = errors.New("srp id invalid") + ErrSRPPasswordChanged = errors.New("srp password changed") + ErrNewSettingsInvalid = errors.New("new password settings invalid") + ErrNewSaltInvalid = errors.New("new password salt invalid") + ErrPasswordRecoveryNA = errors.New("password recovery not available") + ErrRecoveryCodeEmpty = errors.New("recovery code empty") + ErrRecoveryCodeInvalid = errors.New("recovery code invalid") + ErrPasswordRecoveryExpired = errors.New("password recovery expired") + ErrEmailCodeInvalid = errors.New("email code invalid") + ErrEmailInvalid = errors.New("email invalid") + ErrEmailNotAllowed = errors.New("email not allowed") + ErrEmailOccupied = errors.New("email occupied") + ErrSessionPasswordNeeded = errors.New("session password needed") + ErrPhoneNumberInvalid = errors.New("phone number invalid") + ErrPhoneNumberOccupied = errors.New("phone number occupied") + ErrPhoneCodeEmpty = errors.New("phone code empty") + ErrPhoneCodeInvalid = errors.New("phone code invalid") + ErrPhoneCodeExpired = errors.New("phone code expired") + ErrPhoneChangeAuthInvalid = errors.New("phone change auth invalid") + ErrPhoneChangeForbidden = errors.New("phone change forbidden") ) type AuthCodeDeliveryKind string @@ -115,9 +118,6 @@ type PasswordSettings struct { // Server-only SRP fields. They are persisted but never exposed to rpc/tg conversion. SRPVerifier []byte SRPBSecret []byte - - RecoveryCode string - RecoveryCodeExpiresAt int64 } // ReactionNotifyFrom stores one account-level reaction notification scope. diff --git a/internal/mtprotoedge/login_email_e2e_test.go b/internal/mtprotoedge/login_email_e2e_test.go index f694a327..0632f97e 100644 --- a/internal/mtprotoedge/login_email_e2e_test.go +++ b/internal/mtprotoedge/login_email_e2e_test.go @@ -51,7 +51,7 @@ func TestLoginEmailEndToEnd(t *testing.T) { dc = 2 phone = "+8613800138777" wantPhone = "8613800138777" - code = "12345" + devCode = "12345" email = "owner@example.com" wantMask = "o***r@example.com" ) @@ -79,9 +79,10 @@ func TestLoginEmailEndToEnd(t *testing.T) { accountService := account.NewService(passwordStore, account.WithUsers(userStore), account.WithLoginEmailVerification(codeStore, emailSender, 5*time.Minute, 5, 6)) - authService := auth.NewService(userStore, authzStore, codeStore, authKeyStore, memory.NewTempAuthKeyBindingStore(authKeyStore), code, + authService := auth.NewService(userStore, authzStore, codeStore, authKeyStore, memory.NewTempAuthKeyBindingStore(authKeyStore), devCode, auth.WithLoginMessages(messageStore, dialogStore), auth.WithLoginCodeDelivery(memory.NewLoginCodeDeliveryStore(messageStore, updateEventStore)), + auth.WithPhoneCodeDelivery(emailSender, 5), auth.WithPasswords(passwordStore), auth.WithLoginEmail(auth.LoginEmailOptions{ Enabled: true, @@ -135,7 +136,7 @@ func TestLoginEmailEndToEnd(t *testing.T) { return err } hash := sent.(*tg.AuthSentCode).PhoneCodeHash - if _, err := raw.AuthSignIn(ctx, &tg.AuthSignInRequest{PhoneNumber: phone, PhoneCodeHash: hash, PhoneCode: code}); err != nil { + if _, err := raw.AuthSignIn(ctx, &tg.AuthSignInRequest{PhoneNumber: phone, PhoneCodeHash: hash, PhoneCode: emailSender.code}); err != nil { return err } if _, err := raw.AuthSignUp(ctx, &tg.AuthSignUpRequest{PhoneNumber: phone, PhoneCodeHash: hash, FirstName: "Owner"}); err != nil { @@ -234,11 +235,15 @@ func TestLoginEmailEndToEnd(t *testing.T) { return err } sentCode := sent.(*tg.AuthSentCode) - if _, ok := sentCode.Type.(*tg.AuthSentCodeTypeEmailCode); !ok { + emailType, ok := sentCode.Type.(*tg.AuthSentCodeTypeEmailCode) + if !ok { return fmt.Errorf("pre-reset sendCode type = %T, want email code", sentCode.Type) } + if _, ok := emailType.GetResetAvailablePeriod(); !ok { + return fmt.Errorf("pre-reset email code omitted reset_available_period with real SMS sender") + } - // 重置登录邮箱:返回一个新的手机验证码 sentCode(sentCodeTypeApp)。 + // 重置登录邮箱:真实 SMS sender 签发随机手机验证码并返回 sentCodeTypeSms。 resetRes, err := raw.AuthResetLoginEmail(ctx, &tg.AuthResetLoginEmailRequest{PhoneNumber: phone, PhoneCodeHash: sentCode.PhoneCodeHash}) if err != nil { return err @@ -247,12 +252,12 @@ func TestLoginEmailEndToEnd(t *testing.T) { if !ok { return fmt.Errorf("resetLoginEmail result = %T, want *tg.AuthSentCode", resetRes) } - if _, ok := resetSent.Type.(*tg.AuthSentCodeTypeApp); !ok { - return fmt.Errorf("resetLoginEmail sentCode type = %T, want *tg.AuthSentCodeTypeApp (back to phone)", resetSent.Type) + if _, ok := resetSent.Type.(*tg.AuthSentCodeTypeSMS); !ok { + return fmt.Errorf("resetLoginEmail sentCode type = %T, want *tg.AuthSentCodeTypeSMS (real phone fallback)", resetSent.Type) } - // 用手机验证码完成登录。 - signInRes, err := raw.AuthSignIn(ctx, &tg.AuthSignInRequest{PhoneNumber: phone, PhoneCodeHash: resetSent.PhoneCodeHash, PhoneCode: code}) + // 用 sender 捕获的动态手机验证码完成登录,禁止回退固定 development code。 + signInRes, err := raw.AuthSignIn(ctx, &tg.AuthSignInRequest{PhoneNumber: phone, PhoneCodeHash: resetSent.PhoneCodeHash, PhoneCode: emailSender.code}) if err != nil { return err } diff --git a/internal/otpdelivery/delivery.go b/internal/otpdelivery/delivery.go index 9a681c2f..7ad7eafb 100644 --- a/internal/otpdelivery/delivery.go +++ b/internal/otpdelivery/delivery.go @@ -30,6 +30,7 @@ const ( PurposeLoginEmailChange Purpose = "login_email_change" PurposeChangePhone Purpose = "change_phone" PurposeConfirmPhone Purpose = "confirm_phone" + PurposePasswordRecovery Purpose = "password_recovery" ) type Request struct { @@ -47,7 +48,7 @@ func (r Request) Validate(now time.Time) error { return fmt.Errorf("delivery id is empty or too long") } switch r.Purpose { - case PurposeLoginEmail, PurposeLoginSMS, PurposeLoginEmailSetup, PurposeLoginEmailChange, PurposeChangePhone, PurposeConfirmPhone: + case PurposeLoginEmail, PurposeLoginSMS, PurposeLoginEmailSetup, PurposeLoginEmailChange, PurposeChangePhone, PurposeConfirmPhone, PurposePasswordRecovery: default: return fmt.Errorf("unsupported delivery purpose %q", r.Purpose) } diff --git a/internal/rpc/account.go b/internal/rpc/account.go index 8150bed9..837cb481 100644 --- a/internal/rpc/account.go +++ b/internal/rpc/account.go @@ -746,7 +746,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)), r.loginEmailResetAvailable()), }, nil } u, _, needSignUp, signInErr := r.deps.Auth.SignInWithEmail(ctx, r.authzFromCtx(ctx), p.PhoneNumber, p.PhoneCodeHash, code) diff --git a/internal/rpc/auth.go b/internal/rpc/auth.go index 2d664438..31f827ce 100644 --- a/internal/rpc/auth.go +++ b/internal/rpc/auth.go @@ -433,7 +433,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 } @@ -441,15 +441,26 @@ func tgEmailSentCode(hash, emailPattern string, length int) tg.AuthSentCodeClass EmailPattern: emailPattern, Length: length, } - // reset_available_period=0 表示可立即调用 auth.resetLoginEmail(开发环境无等待期), - // 让客户端的"无法访问邮箱?"逃生入口可用。 - codeType.SetResetAvailablePeriod(0) + if resetAvailable { + // 0 means the SMS fallback is available immediately. Absence means this + // deployment cannot safely service auth.resetLoginEmail. + codeType.SetResetAvailablePeriod(0) + } return &tg.AuthSentCode{ Type: codeType, PhoneCodeHash: hash, } } +type loginEmailResetAvailabilityChecker interface { + LoginEmailResetAvailable() bool +} + +func (r *Router) loginEmailResetAvailable() bool { + checker, ok := r.deps.Auth.(loginEmailResetAvailabilityChecker) + return ok && checker.LoginEmailResetAvailable() +} + func tgEmailSetupRequiredSentCode(hash string) tg.AuthSentCodeClass { return &tg.AuthSentCode{ Type: &tg.AuthSentCodeTypeSetUpEmailRequired{}, @@ -472,7 +483,7 @@ 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 + return tgEmailSentCode(hash, delivery.EmailPattern, delivery.Length, r.loginEmailResetAvailable()), 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..cb5971fd 100644 --- a/internal/rpc/auth_login_email_rpc_test.go +++ b/internal/rpc/auth_login_email_rpc_test.go @@ -47,6 +47,35 @@ func TestEmailSentCodeUsesDeliveryLength(t *testing.T) { } } +func TestEmailSentCodeAdvertisesResetOnlyWhenAvailable(t *testing.T) { + for _, tc := range []struct { + name string + available bool + }{ + {name: "unavailable"}, + {name: "available", available: 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.available, + } + r := New(Config{}, Deps{Auth: authSvc}, zaptest.NewLogger(t), fixedClock{now: time.Unix(1700000000, 0)}) + sent, err := r.tgSentCodeForHash(context.Background(), "hash-email-reset") + if err != nil { + t.Fatal(err) + } + emailType := sent.(*tg.AuthSentCode).Type.(*tg.AuthSentCodeTypeEmailCode) + _, present := emailType.GetResetAvailablePeriod() + if present != tc.available { + t.Fatalf("reset_available_period present=%v, want %v", present, tc.available) + } + }) + } +} + func TestAuthSignInRoutesOfficialEmailCodeCarriers(t *testing.T) { const ( phone = "+86 188 0000 0021" diff --git a/internal/rpc/auth_password_pending_test.go b/internal/rpc/auth_password_pending_test.go index 69fb57c5..1dad9bc4 100644 --- a/internal/rpc/auth_password_pending_test.go +++ b/internal/rpc/auth_password_pending_test.go @@ -3,6 +3,7 @@ package rpc import ( "context" "testing" + "time" "github.com/iamxvbaba/td/clock" "github.com/iamxvbaba/td/tg" @@ -10,9 +11,21 @@ import ( appaccount "telesrv/internal/app/account" "telesrv/internal/domain" + "telesrv/internal/otpdelivery" "telesrv/internal/store/memory" ) +type capturePasswordRecoveryMailSender struct { + to string + code string +} + +func (s *capturePasswordRecoveryMailSender) Deliver(_ context.Context, req otpdelivery.Request) (otpdelivery.Result, error) { + s.to = req.Recipient + s.code = req.Code + return otpdelivery.Result{}, nil +} + func TestAccountGetPasswordUsesPendingPasswordUser(t *testing.T) { ctx := pendingPasswordContext() const userID int64 = 42 @@ -65,9 +78,11 @@ func TestAuthRecoverPasswordCompletesPendingSignIn(t *testing.T) { pendingPassword: true, } sessions := &captureSessions{} + sender := &capturePasswordRecoveryMailSender{} router := New(Config{}, Deps{ - Auth: auth, - Account: appaccount.NewService(passwords), + Auth: auth, + Account: appaccount.NewService(passwords, + appaccount.WithLoginEmailVerification(memory.NewCodeStore(), sender, time.Minute, 3, 6)), Users: staticUsersService{user: domain.User{ID: userID, AccessHash: 7, Phone: "15550000042", FirstName: "Alice"}}, Sessions: sessions, }, zaptest.NewLogger(t), clock.System) @@ -75,7 +90,10 @@ func TestAuthRecoverPasswordCompletesPendingSignIn(t *testing.T) { if _, err := router.onAuthRequestPasswordRecovery(ctx); err != nil { t.Fatalf("auth.requestPasswordRecovery: %v", err) } - if _, err := router.onAuthRecoverPassword(ctx, &tg.AuthRecoverPasswordRequest{Code: "12345"}); err != nil { + if sender.to != "alice@example.com" || sender.code == "" { + t.Fatalf("recovery delivery=%+v", sender) + } + if _, err := router.onAuthRecoverPassword(ctx, &tg.AuthRecoverPasswordRequest{Code: sender.code}); err != nil { t.Fatalf("auth.recoverPassword: %v", err) } if auth.completePasswordCount != 1 || auth.completedPasswordKey != authKeyID { diff --git a/internal/rpc/errors.go b/internal/rpc/errors.go index a552faeb..d045d1dc 100644 --- a/internal/rpc/errors.go +++ b/internal/rpc/errors.go @@ -479,6 +479,12 @@ func passwordErr(err error) error { return emailNotAllowedErr() case errors.Is(err, domain.ErrEmailCodeInvalid): return emailCodeInvalidErr() + case errors.Is(err, domain.ErrRecoveryCodeEmpty): + return tgerr.New(400, "CODE_EMPTY") + case errors.Is(err, domain.ErrRecoveryCodeInvalid): + return tgerr.New(400, "CODE_INVALID") + case errors.Is(err, domain.ErrPasswordRecoveryExpired): + return tgerr.New(400, "PASSWORD_RECOVERY_EXPIRED") case errors.Is(err, domain.ErrPasswordRecoveryNA): return passwordRecoveryNAErr() default: diff --git a/internal/rpc/errors_test.go b/internal/rpc/errors_test.go index 45fe746f..16bd2af8 100644 --- a/internal/rpc/errors_test.go +++ b/internal/rpc/errors_test.go @@ -15,6 +15,22 @@ func TestPasswordErrMapsOccupiedLoginEmailToNotAllowed(t *testing.T) { } } +func TestPasswordErrMapsRecoveryState(t *testing.T) { + tests := []struct { + err error + want string + }{ + {domain.ErrRecoveryCodeEmpty, "CODE_EMPTY"}, + {domain.ErrRecoveryCodeInvalid, "CODE_INVALID"}, + {domain.ErrPasswordRecoveryExpired, "PASSWORD_RECOVERY_EXPIRED"}, + } + for _, tc := range tests { + if err := passwordErr(tc.err); !tgerr.Is(err, tc.want) { + t.Fatalf("passwordErr(%v)=%v, want %s", tc.err, err, tc.want) + } + } +} + func TestBindTempAuthKeyErrPreservesRecoverableRotationErrors(t *testing.T) { tests := []struct { err error 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 { diff --git a/internal/rpc/users.go b/internal/rpc/users.go index 412d09d7..d10c7682 100644 --- a/internal/rpc/users.go +++ b/internal/rpc/users.go @@ -249,15 +249,13 @@ func applyContactNoteToUserFull(user domain.User, full *tg.UserFull) bool { } func (r *Router) buildUserFullProjection(ctx context.Context, currentUserID int64, u domain.User) (tg.UserFull, error) { + visibility, err := r.userFullPrivacyVisibility(ctx, currentUserID, u.ID) + if err != nil { + return tg.UserFull{}, internalErr() + } about := u.About - if r.deps.Privacy != nil && u.ID != currentUserID { - allowed, err := r.deps.Privacy.CanSee(ctx, u.ID, currentUserID, domain.PrivacyKeyAbout) - if err != nil { - return tg.UserFull{}, internalErr() - } - if !allowed { - about = "" - } + if !visibility[domain.PrivacyKeyAbout] { + about = "" } full := tg.UserFull{ ID: u.ID, @@ -279,24 +277,9 @@ func (r *Router) buildUserFullProjection(ctx context.Context, currentUserID int6 // 通话入口:客户端不见 phone_calls_available=true 不显示通话按钮(P1 前置项)。 // phone_calls_private 标记对端禁 P2P(p2p_allowed 真值在通话确认时另行计算)。 if !u.Bot && u.ID != currentUserID { - callsAllowed, p2pAllowed, voiceAllowed := true, true, true - if r.deps.Privacy != nil { - allowed, err := r.deps.Privacy.CanSee(ctx, u.ID, currentUserID, domain.PrivacyKeyPhoneCall) - if err != nil { - return tg.UserFull{}, internalErr() - } - callsAllowed = allowed - allowed, err = r.deps.Privacy.CanSee(ctx, u.ID, currentUserID, domain.PrivacyKeyPhoneP2P) - if err != nil { - return tg.UserFull{}, internalErr() - } - p2pAllowed = allowed - allowed, err = r.deps.Privacy.CanSee(ctx, u.ID, currentUserID, domain.PrivacyKeyVoiceMessages) - if err != nil { - return tg.UserFull{}, internalErr() - } - voiceAllowed = allowed - } + callsAllowed := visibility[domain.PrivacyKeyPhoneCall] + p2pAllowed := visibility[domain.PrivacyKeyPhoneP2P] + voiceAllowed := visibility[domain.PrivacyKeyVoiceMessages] full.PhoneCallsAvailable = callsAllowed full.VideoCallsAvailable = callsAllowed full.PhoneCallsPrivate = !p2pAllowed @@ -326,15 +309,11 @@ func (r *Router) buildUserFullProjection(ctx context.Context, currentUserID int6 break } } - if err := r.fillUserFullPhotos(ctx, currentUserID, u.ID, &full); err != nil { + if err := r.fillUserFullPhotos(ctx, currentUserID, u.ID, visibility[domain.PrivacyKeyProfilePhoto], &full); err != nil { return tg.UserFull{}, err } if r.deps.Account != nil { - allowed, err := r.canSeeSavedMusic(ctx, currentUserID, u.ID) - if err != nil { - return tg.UserFull{}, err - } - if allowed { + if visibility[domain.PrivacyKeySavedMusic] { music, err := r.deps.Account.ListSavedMusic(ctx, u.ID, 0, 1) if err != nil { return tg.UserFull{}, internalErr() @@ -406,15 +385,7 @@ func (r *Router) buildUserFullProjection(ctx context.Context, currentUserID int6 // 生日(account.updateBirthday):落 userFull.birthday,按 PrivacyKeyBirthday 对他人裁剪, // 本人恒可见。 if u.Birthday.IsSet() { - birthdayVisible := true - if r.deps.Privacy != nil && u.ID != currentUserID { - allowed, err := r.deps.Privacy.CanSee(ctx, u.ID, currentUserID, domain.PrivacyKeyBirthday) - if err != nil { - return tg.UserFull{}, internalErr() - } - birthdayVisible = allowed - } - if birthdayVisible { + if visibility[domain.PrivacyKeyBirthday] { full.SetBirthday(tgBirthday(u.Birthday)) } } @@ -425,6 +396,51 @@ func (r *Router) buildUserFullProjection(ctx context.Context, currentUserID int6 return full, nil } +// userFullPrivacyVisibility evaluates every privacy-controlled UserFull field +// from one owner snapshot when the service supports batching. Older test or +// alternate implementations retain scalar parity; a missing batch key fails +// closed instead of exposing that field. +func (r *Router) userFullPrivacyVisibility(ctx context.Context, viewerUserID, ownerUserID int64) (map[domain.PrivacyKey]bool, error) { + keys := []domain.PrivacyKey{ + domain.PrivacyKeyAbout, + domain.PrivacyKeyPhoneCall, + domain.PrivacyKeyPhoneP2P, + domain.PrivacyKeyVoiceMessages, + domain.PrivacyKeyProfilePhoto, + domain.PrivacyKeySavedMusic, + domain.PrivacyKeyBirthday, + } + out := make(map[domain.PrivacyKey]bool, len(keys)) + if r.deps.Privacy == nil || ownerUserID == viewerUserID { + for _, key := range keys { + out[key] = true + } + return out, nil + } + if batch, ok := r.deps.Privacy.(batchPrivacyEvaluator); ok { + matrix, err := batch.CanSeeBatch(ctx, []int64{ownerUserID}, viewerUserID, keys) + if err != nil { + return nil, err + } + ownerVisibility, found := matrix[ownerUserID] + if !found { + return out, nil + } + for _, key := range keys { + out[key] = ownerVisibility[key] + } + return out, nil + } + for _, key := range keys { + allowed, err := r.deps.Privacy.CanSee(ctx, ownerUserID, viewerUserID, key) + if err != nil { + return nil, err + } + out[key] = allowed + } + return out, nil +} + // applyAccountRatingToUserFull projects gramsrv's stored composite rating through // the rating fields official clients already render. This is a gramsrv policy // score, not a promise that its inputs or thresholds match Telegram's service. @@ -724,7 +740,7 @@ func savedMusicDocumentIDs(docs []domain.Document) []int64 { return ids } -func (r *Router) fillUserFullPhotos(ctx context.Context, viewerUserID, ownerUserID int64, full *tg.UserFull) error { +func (r *Router) fillUserFullPhotos(ctx context.Context, viewerUserID, ownerUserID int64, profileAllowed bool, full *tg.UserFull) error { if r.deps.Files == nil || full == nil || ownerUserID == 0 { return nil } @@ -756,14 +772,6 @@ func (r *Router) fillUserFullPhotos(ctx context.Context, viewerUserID, ownerUser } } } - profileAllowed := true - if r.deps.Privacy != nil { - var err error - profileAllowed, err = r.deps.Privacy.CanSee(ctx, ownerUserID, viewerUserID, domain.PrivacyKeyProfilePhoto) - if err != nil { - return internalErr() - } - } if profileAllowed { if photo, found, err := r.deps.Files.CurrentProfilePhotoKind(ctx, domain.PeerTypeUser, ownerUserID, domain.ProfilePhotoKindProfile); err != nil { return internalErr() diff --git a/internal/rpc/users_full_privacy_batch_test.go b/internal/rpc/users_full_privacy_batch_test.go new file mode 100644 index 00000000..2d929e43 --- /dev/null +++ b/internal/rpc/users_full_privacy_batch_test.go @@ -0,0 +1,122 @@ +package rpc + +import ( + "context" + "testing" + + "github.com/iamxvbaba/td/clock" + "go.uber.org/zap/zaptest" + + "telesrv/internal/domain" +) + +type userFullBatchPrivacy struct { + stubPrivacy + visible map[domain.PrivacyKey]bool + batchCalls int + scalarCalls int + keys []domain.PrivacyKey +} + +func (p *userFullBatchPrivacy) CanSee(_ context.Context, _, _ int64, key domain.PrivacyKey) (bool, error) { + p.scalarCalls++ + return p.visible[key], nil +} + +func (p *userFullBatchPrivacy) CanSeeBatch(_ context.Context, ownerUserIDs []int64, _ int64, keys []domain.PrivacyKey) (map[int64]map[domain.PrivacyKey]bool, error) { + p.batchCalls++ + p.keys = append([]domain.PrivacyKey(nil), keys...) + out := make(map[int64]map[domain.PrivacyKey]bool, len(ownerUserIDs)) + for _, ownerID := range ownerUserIDs { + owner := make(map[domain.PrivacyKey]bool, len(p.visible)) + for key, allowed := range p.visible { + owner[key] = allowed + } + out[ownerID] = owner + } + return out, nil +} + +type userFullScalarPrivacy struct { + stubPrivacy + visible map[domain.PrivacyKey]bool + calls int +} + +func (p *userFullScalarPrivacy) CanSee(_ context.Context, _, _ int64, key domain.PrivacyKey) (bool, error) { + p.calls++ + return p.visible[key], nil +} + +func TestBuildUserFullProjectionBatchesPrivacyAndFailsClosedOnMissingKeys(t *testing.T) { + privacy := &userFullBatchPrivacy{visible: map[domain.PrivacyKey]bool{ + domain.PrivacyKeyAbout: false, + domain.PrivacyKeyPhoneCall: true, + domain.PrivacyKeyPhoneP2P: false, + domain.PrivacyKeyVoiceMessages: false, + domain.PrivacyKeyBirthday: true, + // ProfilePhoto and SavedMusic are deliberately absent: batch omissions + // must stay denied rather than becoming a privacy bypass. + }} + r := New(Config{}, Deps{Privacy: privacy}, zaptest.NewLogger(t), clock.System) + full, err := r.buildUserFullProjection(context.Background(), 10, domain.User{ + ID: 20, FirstName: "Target", About: "private about", + Birthday: domain.Birthday{Day: 2, Month: 8, Year: 2000}, + }) + if err != nil { + t.Fatal(err) + } + if privacy.batchCalls != 1 || privacy.scalarCalls != 0 { + t.Fatalf("privacy calls batch=%d scalar=%d, want 1/0", privacy.batchCalls, privacy.scalarCalls) + } + if len(privacy.keys) != 7 { + t.Fatalf("batch keys=%v, want seven UserFull privacy keys", privacy.keys) + } + if full.About != "" || !full.PhoneCallsAvailable || !full.PhoneCallsPrivate || !full.VoiceMessagesForbidden { + t.Fatalf("privacy projection=%+v", full) + } + if _, ok := full.GetBirthday(); !ok { + t.Fatal("allowed birthday omitted") + } + if _, ok := full.GetProfilePhoto(); ok { + t.Fatal("missing profile-photo visibility defaulted to visible") + } + if _, ok := full.GetSavedMusic(); ok { + t.Fatal("missing saved-music visibility defaulted to visible") + } +} + +func TestBuildUserFullProjectionRetainsScalarPrivacyFallback(t *testing.T) { + privacy := &userFullScalarPrivacy{visible: map[domain.PrivacyKey]bool{ + domain.PrivacyKeyAbout: true, + domain.PrivacyKeyPhoneCall: true, + domain.PrivacyKeyPhoneP2P: true, + domain.PrivacyKeyVoiceMessages: true, + domain.PrivacyKeyProfilePhoto: true, + domain.PrivacyKeySavedMusic: true, + domain.PrivacyKeyBirthday: true, + }} + r := New(Config{}, Deps{Privacy: privacy}, zaptest.NewLogger(t), clock.System) + full, err := r.buildUserFullProjection(context.Background(), 10, domain.User{ID: 20, About: "visible"}) + if err != nil { + t.Fatal(err) + } + if privacy.calls != 7 { + t.Fatalf("scalar privacy calls=%d, want 7", privacy.calls) + } + if full.About != "visible" || !full.PhoneCallsAvailable || full.PhoneCallsPrivate || full.VoiceMessagesForbidden { + t.Fatalf("scalar privacy projection=%+v", full) + } +} + +func TestBuildUserFullProjectionSelfSkipsPrivacyEvaluation(t *testing.T) { + privacy := &userFullBatchPrivacy{visible: map[domain.PrivacyKey]bool{}} + r := New(Config{}, Deps{Privacy: privacy}, zaptest.NewLogger(t), clock.System) + full, err := r.buildUserFullProjection(context.Background(), 20, domain.User{ID: 20, About: "self"}) + if err != nil { + t.Fatal(err) + } + if privacy.batchCalls != 0 || privacy.scalarCalls != 0 || full.About != "self" { + t.Fatalf("self projection calls=%d/%d full=%+v", privacy.batchCalls, privacy.scalarCalls, full) + } +} diff --git a/internal/store/code.go b/internal/store/code.go index 08355a7e..a2363933 100644 --- a/internal/store/code.go +++ b/internal/store/code.go @@ -76,6 +76,11 @@ type PhoneCode struct { VerifiedEmail bool RequireSignUp bool LoginEmailHash string + // RecoveryBinding ties a password-recovery code to the exact 2FA state + // which existed when the code was issued. A password or recovery-email + // change makes an older code unusable without relying on cross-store + // best-effort invalidation. + RecoveryBinding string // AccountDeletionHash is the hex-encoded SHA-256 digest of the validated // confirmphone link token. It binds account.confirmPhone to one pending // deletion without persisting the raw link credential in the code record. diff --git a/internal/store/postgres/account.go b/internal/store/postgres/account.go index 7272094d..075a69db 100644 --- a/internal/store/postgres/account.go +++ b/internal/store/postgres/account.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "strings" - "time" "github.com/jackc/pgerrcode" "github.com/jackc/pgx/v5" @@ -36,18 +35,17 @@ SELECT email_unconfirmed_pattern, login_email_pattern, secure_random, current_algo_salt1, current_algo_salt2, current_algo_g, current_algo_p, srp_id, srp_verifier, srp_b_secret, srp_b, - recovery_email, recovery_code, recovery_code_expires_at, login_email + recovery_email, login_email FROM account_passwords WHERE user_id = $1`, userID) var settings domain.PasswordSettings var salt1, salt2, p []byte - var recoveryExpires sql.NullTime if err := row.Scan( &settings.HasRecovery, &settings.HasSecureValues, &settings.HasPassword, &settings.Hint, &settings.EmailUnconfirmedPattern, &settings.LoginEmailPattern, &settings.SecureRandom, &salt1, &salt2, &settings.NewAlgo.G, &p, &settings.SRPID, &settings.SRPVerifier, &settings.SRPBSecret, &settings.SRPB, - &settings.RecoveryEmail, &settings.RecoveryCode, &recoveryExpires, &settings.LoginEmail, + &settings.RecoveryEmail, &settings.LoginEmail, ); err != nil { if errors.Is(err, pgx.ErrNoRows) { return domain.PasswordSettings{}, false, nil @@ -65,9 +63,6 @@ WHERE user_id = $1`, userID) settings.NewAlgo.Salt1 = append([]byte(nil), salt1...) settings.NewAlgo.Salt2 = append([]byte(nil), salt2...) settings.NewAlgo.P = append([]byte(nil), p...) - if recoveryExpires.Valid { - settings.RecoveryCodeExpiresAt = recoveryExpires.Time.Unix() - } settings.SecureRandom = append([]byte(nil), settings.SecureRandom...) settings.SRPVerifier = append([]byte(nil), settings.SRPVerifier...) settings.SRPBSecret = append([]byte(nil), settings.SRPBSecret...) @@ -102,19 +97,15 @@ func (s *PasswordStore) Save(ctx context.Context, userID int64, settings domain. if settings.CurrentAlgo != nil { algo = *settings.CurrentAlgo } - var recoveryExpires any - if settings.RecoveryCodeExpiresAt > 0 { - recoveryExpires = time.Unix(settings.RecoveryCodeExpiresAt, 0) - } _, err := s.db.Exec(ctx, ` INSERT INTO account_passwords ( user_id, has_recovery, has_secure_values, has_password, hint, email_unconfirmed_pattern, login_email_pattern, secure_random, current_algo_salt1, current_algo_salt2, current_algo_g, current_algo_p, srp_id, srp_verifier, srp_b_secret, srp_b, - recovery_email, recovery_code, recovery_code_expires_at, login_email + recovery_email, login_email ) -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20) +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18) ON CONFLICT (user_id) DO UPDATE SET has_recovery = EXCLUDED.has_recovery, has_secure_values = EXCLUDED.has_secure_values, @@ -132,8 +123,6 @@ ON CONFLICT (user_id) DO UPDATE SET srp_b_secret = EXCLUDED.srp_b_secret, srp_b = EXCLUDED.srp_b, recovery_email = EXCLUDED.recovery_email, - recovery_code = EXCLUDED.recovery_code, - recovery_code_expires_at = EXCLUDED.recovery_code_expires_at, login_email = EXCLUDED.login_email, updated_at = now()`, userID, @@ -141,7 +130,7 @@ ON CONFLICT (user_id) DO UPDATE SET settings.EmailUnconfirmedPattern, settings.LoginEmailPattern, nonNilBytea(settings.SecureRandom), nonNilBytea(algo.Salt1), nonNilBytea(algo.Salt2), algo.G, nonNilBytea(algo.P), settings.SRPID, nonNilBytea(settings.SRPVerifier), nonNilBytea(settings.SRPBSecret), nonNilBytea(settings.SRPB), - settings.RecoveryEmail, settings.RecoveryCode, recoveryExpires, settings.LoginEmail, + settings.RecoveryEmail, settings.LoginEmail, ) if err != nil { if isAccountPasswordLoginEmailUnique(err) { diff --git a/internal/store/postgres/contiguous_pts_integration_test.go b/internal/store/postgres/contiguous_pts_integration_test.go index 05bef6c1..263cfd76 100644 --- a/internal/store/postgres/contiguous_pts_integration_test.go +++ b/internal/store/postgres/contiguous_pts_integration_test.go @@ -8,6 +8,88 @@ import ( "telesrv/internal/domain" ) +func TestReserveUserPtsRejectsZeroBeforeQuery(t *testing.T) { + if _, err := reserveUserPts(context.Background(), nil, 0, 1); err == nil { + t.Fatal("reserveUserPts user=0 succeeded, want fail-fast before DB access") + } +} + +// TestAppendAllocatedFirstPtsRangeAndRollback covers both branches of the +// single-statement watermark upsert through a legal durable update. A rolled +// back first allocation must leave no watermark; the committed retry must +// create one range ending at pts=3 with pts_count=3. +func TestAppendAllocatedFirstPtsRangeAndRollback(t *testing.T) { + pool := testPool(t) + ctx := context.Background() + suffix := randomSuffix(t) + owner, err := NewUserStore(pool).Create(ctx, domain.User{ + AccessHash: 4, + Phone: "+1555" + suffix + "01", + FirstName: "FirstPtsRange", + }) + if err != nil { + t.Fatalf("create user: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(ctx, "DELETE FROM users WHERE id = $1", owner.ID) + }) + + event := domain.UpdateEvent{ + Type: domain.UpdateEventDeleteMessages, + PtsCount: 3, + Date: 1700000003, + MessageIDs: []int{101, 102, 103}, + } + tx, err := pool.Begin(ctx) + if err != nil { + t.Fatalf("begin rollback allocation: %v", err) + } + allocated, err := NewUpdateEventStore(tx).AppendAllocated(ctx, owner.ID, event) + if err != nil { + _ = tx.Rollback(ctx) + t.Fatalf("append allocated before rollback: %v", err) + } + if allocated.Pts != 3 || allocated.PtsCount != 3 { + _ = tx.Rollback(ctx) + t.Fatalf("allocated before rollback = pts %d count %d, want 3/3", allocated.Pts, allocated.PtsCount) + } + if err := tx.Rollback(ctx); err != nil { + t.Fatalf("rollback first allocation: %v", err) + } + + var rows int + if err := pool.QueryRow(ctx, `SELECT count(*)::int FROM user_update_watermarks WHERE user_id=$1`, owner.ID).Scan(&rows); err != nil { + t.Fatalf("count watermark after rollback: %v", err) + } + if rows != 0 { + t.Fatalf("watermark rows after rollback = %d, want 0", rows) + } + if err := pool.QueryRow(ctx, `SELECT count(*)::int FROM user_update_events WHERE user_id=$1`, owner.ID).Scan(&rows); err != nil { + t.Fatalf("count events after rollback: %v", err) + } + if rows != 0 { + t.Fatalf("event rows after rollback = %d, want 0", rows) + } + + allocated, err = NewUpdateEventStore(pool).AppendAllocated(ctx, owner.ID, event) + if err != nil { + t.Fatalf("append allocated after rollback: %v", err) + } + if allocated.Pts != 3 || allocated.PtsCount != 3 { + t.Fatalf("committed allocation = pts %d count %d, want 3/3", allocated.Pts, allocated.PtsCount) + } + if pts, err := NewUpdateEventStore(pool).MaxContiguousPts(ctx, owner.ID); err != nil || pts != 3 { + t.Fatalf("MaxContiguousPts = %d err=%v, want 3", pts, err) + } + events, err := NewUpdateEventStore(pool).ListAfter(ctx, owner.ID, 0, 10) + if err != nil { + t.Fatalf("ListAfter: %v", err) + } + if len(events) != 1 || events[0].Pts != 3 || events[0].PtsCount != 3 || len(events[0].MessageIDs) != 3 { + t.Fatalf("events = %+v, want one delete range ending at 3", events) + } +} + // TestAppendRejectsPtsHole 用真实 PG 验证显式 pts 写入不能制造空洞。 func TestAppendRejectsPtsHole(t *testing.T) { pool := testPool(t) diff --git a/internal/store/postgres/message_concurrent_integration_test.go b/internal/store/postgres/message_concurrent_integration_test.go index fd20f983..623e9c41 100644 --- a/internal/store/postgres/message_concurrent_integration_test.go +++ b/internal/store/postgres/message_concurrent_integration_test.go @@ -45,6 +45,16 @@ func TestSendPrivateTextConcurrentNoPtsGap(t *testing.T) { _, _ = pool.Exec(ctx, "DELETE FROM dialogs WHERE user_id = ANY($1::bigint[])", ids) _, _ = pool.Exec(ctx, "DELETE FROM users WHERE id = ANY($1::bigint[])", ids) }) + var initialWatermarks int + if err := pool.QueryRow(ctx, ` +SELECT count(*)::int +FROM user_update_watermarks +WHERE user_id = ANY($1::bigint[])`, ids).Scan(&initialWatermarks); err != nil { + t.Fatalf("count initial watermarks: %v", err) + } + if initialWatermarks != 0 { + t.Fatalf("initial watermarks = %d, want 0 so concurrency covers first upsert", initialWatermarks) + } messages := NewMessageStore(pool, WithMessageAllocators(&perUserCounterAllocator{})) diff --git a/internal/store/postgres/sqlcgen/models.go b/internal/store/postgres/sqlcgen/models.go index ef36e34a..e440e208 100644 --- a/internal/store/postgres/sqlcgen/models.go +++ b/internal/store/postgres/sqlcgen/models.go @@ -53,8 +53,6 @@ type AccountPassword struct { SrpBSecret []byte SrpB []byte RecoveryEmail string - RecoveryCode string - RecoveryCodeExpiresAt pgtype.Timestamptz LoginEmail string PasswordChangedAt pgtype.Timestamptz } diff --git a/internal/store/postgres/user_pts.go b/internal/store/postgres/user_pts.go index b1d41fb0..7bf6c29f 100644 --- a/internal/store/postgres/user_pts.go +++ b/internal/store/postgres/user_pts.go @@ -30,15 +30,16 @@ ON CONFLICT (user_id) DO NOTHING`, userID) func reserveUserPts(ctx context.Context, db sqlcgen.DBTX, userID int64, count int) (int, error) { count = normalizePtsCount(count) - if err := ensureUserUpdateWatermark(ctx, db, userID); err != nil { - return 0, err + if userID == 0 { + return 0, fmt.Errorf("user pts: missing user id") } var pts int if err := db.QueryRow(ctx, ` -UPDATE user_update_watermarks -SET contiguous_pts = contiguous_pts + $2, +INSERT INTO user_update_watermarks (user_id, contiguous_pts) +VALUES ($1, $2) +ON CONFLICT (user_id) DO UPDATE +SET contiguous_pts = user_update_watermarks.contiguous_pts + EXCLUDED.contiguous_pts, updated_at = now() -WHERE user_id = $1 RETURNING contiguous_pts`, userID, count).Scan(&pts); err != nil { return 0, fmt.Errorf("reserve user pts: %w", err) }