fix for security bug

internal/app/account/service.go

@@ -411,13 +411,47 @@ 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
+	// A fixed recovery code was used here regardless of whether it was
+	// actually emailed anywhere, which let anyone reset 2FA on any account
+	// with a recovery email set. Recovery now requires a real sender and a
+	// freshly generated code delivered to it -- no sender, no recovery.
+	if 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
+	}
+	expiresAtUnix := time.Now().Unix() + recoveryCodeTTL
+	expiresAt := time.Unix(expiresAtUnix, 0)
+	settings.RecoveryCode = code
+	settings.RecoveryCodeExpiresAt = expiresAtUnix
 	if s.passwords != nil {
 		if err := s.passwords.Save(ctx, userID, settings); 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 {
+		if s.passwords != nil {
+			cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second)
+			defer cancel()
+			settings.RecoveryCode = ""
+			settings.RecoveryCodeExpiresAt = 0
+			_ = s.passwords.Save(cleanupCtx, userID, settings)
+		}
+		return "", err
+	}
 	return emailPattern(settings.RecoveryEmail), nil
 }
 
@@ -535,10 +569,9 @@ func (s *Service) CancelPasswordEmail(ctx context.Context, userID int64) error {
 }
 
 func checkRecoveryCode(settings domain.PasswordSettings, code string) error {
+	// No standing fixed-code fallback: an unrequested (or already consumed)
+	// recovery must not be satisfiable by any code at all.
 	if settings.RecoveryCode == "" {
-		if code == recoveryCode {
-			return nil
-		}
 		return domain.ErrPasswordRecoveryNA
 	}
 	if settings.RecoveryCodeExpiresAt > 0 && time.Now().Unix() > settings.RecoveryCodeExpiresAt {

internal/app/account/service_test.go

@@ -69,7 +69,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 +95,10 @@ 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 == "" {
+		t.Fatalf("sender = %+v, want delivered code to bob@example.com", sender)
+	}
+	if err := svc.RecoverPassword(ctx, userID, sender.code, nil); err != nil {
 		t.Fatalf("RecoverPassword clear: %v", err)
 	}
 	cleared, err := svc.GetPassword(ctx, userID)

internal/app/account/srp.go

@@ -12,7 +12,6 @@ import (
 
 const (
 	passwordHashSize = 256
-	recoveryCode     = "12345"
 	recoveryCodeTTL  = 15 * 60
 )
 

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)
 	}

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),
+		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("sender = %+v, want delivered recovery code to alice@example.com", 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 {