diff --git a/internal/app/auth/email_signup_test.go b/internal/app/auth/email_signup_test.go index 8316b7a2..872884b2 100644 --- a/internal/app/auth/email_signup_test.go +++ b/internal/app/auth/email_signup_test.go @@ -2,6 +2,7 @@ package auth import ( "context" + "errors" "testing" "telesrv/internal/domain" @@ -126,3 +127,40 @@ func TestEmailSignupSendCodeIgnoredWhenPhoneIsNotEncoded(t *testing.T) { t.Fatalf("delivery.Kind = Email, want a non-email fallback for a real phone number") } } + +// This server has no real SMS delivery, so a real phone number is only +// reachable via the client's explicit "log in by phone number" fallback +// (see EmailSignupWidget). That account must get a login email bound +// immediately: EmailSignupEnable alone must force the setup-required gate +// even though neither LOGIN_EMAIL_ENABLE nor LOGIN_EMAIL_REQUIRE_SETUP is on. +func TestEmailSignupForcesLoginEmailSetupForRealPhoneNumbers(t *testing.T) { + ctx := context.Background() + users := memory.NewUserStore() + authz := memory.NewAuthorizationStore() + sender := &testMailSender{} + svc := NewService(users, authz, memory.NewCodeStore(), nil, nil, "12345", + WithLoginEmail(LoginEmailOptions{Store: &testLoginEmailStore{emails: map[string]string{}}, Sender: sender}), + WithEmailSignup(true)) + + hash, err := svc.SendCode(ctx, "+15550029999") + if err != nil { + t.Fatalf("SendCode: %v", err) + } + if sender.to != "" { + t.Fatalf("sender.to = %q, want empty (setup-required never sends a code itself)", sender.to) + } + delivery, found, err := svc.CodeDelivery(ctx, hash) + if err != nil || !found { + t.Fatalf("CodeDelivery found=%v err=%v", found, err) + } + if delivery.Kind != domain.AuthCodeDeliveryEmailSetupRequired { + t.Fatalf("delivery.Kind = %v, want AuthCodeDeliveryEmailSetupRequired", delivery.Kind) + } + + // Signing up straight off this code without ever completing email setup + // (the account.sendVerifyEmailCode/verifyEmail round trip) must be + // rejected: the hash was never marked SignUpVerified. + if _, _, err := svc.SignUp(ctx, domain.Authorization{}, "+15550029999", hash, "No", "Email"); !errors.Is(err, ErrCodeInvalid) { + t.Fatalf("SignUp err = %v, want ErrCodeInvalid (email setup was never completed)", err) + } +} diff --git a/internal/app/auth/service.go b/internal/app/auth/service.go index 49002bf4..7fdc009f 100644 --- a/internal/app/auth/service.go +++ b/internal/app/auth/service.go @@ -330,7 +330,13 @@ func (s *Service) SendCode(ctx context.Context, phone string) (string, error) { return s.createEmailLoginCode(ctx, phone, email, issuedUserID) } } - if s.loginEmailEnabled && s.loginEmails != nil { + // A real (non-encoded) phone number reaching here on an email-signup + // server is someone using the client's explicit "log in by phone number" + // fallback (see EmailSignupWidget) — a real phone is otherwise + // unreachable from the intro flow. This server never sends real SMS, so + // that account must get a login email bound immediately: force the setup + // gate on regardless of the raw TELESRV_LOGIN_EMAIL_REQUIRE_SETUP config. + if (s.loginEmailEnabled || s.emailSignupEnabled) && s.loginEmails != nil { email, found, err := s.loginEmails.LoginEmailByPhone(ctx, phone) if err != nil { return "", err @@ -338,13 +344,22 @@ func (s *Service) SendCode(ctx context.Context, phone string) (string, error) { if found && strings.TrimSpace(email) != "" { return s.createEmailLoginCode(ctx, phone, email, issuedUserID) } - if s.loginEmailRequireSetup { + if s.requireLoginEmailSetup() { return s.createSetupRequiredCode(ctx, phone, issuedUserID) } } return s.createPhoneCode(ctx, phone, issuedUserID) } +// requireLoginEmailSetup reports whether a phone-number account without a +// bound login email must set one up before it can finish signing in/up. This +// is forced on for every real phone number once email-signup is enabled — +// see SendCode — independent of the raw TELESRV_LOGIN_EMAIL_REQUIRE_SETUP +// config value, since this server has no real SMS delivery to fall back to. +func (s *Service) requireLoginEmailSetup() bool { + return s.loginEmailRequireSetup || s.emailSignupEnabled +} + // 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 @@ -972,7 +987,7 @@ func (s *Service) SignUp(ctx context.Context, auth domain.Authorization, phone, // email via the legacy VerifiedEmail/PendingEmail flow; it does not apply // here and would otherwise permanently block SignUp for every // email-signup account. - if s.loginEmailRequireSetup && !rec.VerifiedEmail && strings.TrimSpace(rec.PendingEmail) == "" && !domain.IsEmailSignupPhone(phone) { + if s.requireLoginEmailSetup() && !rec.VerifiedEmail && strings.TrimSpace(rec.PendingEmail) == "" && !domain.IsEmailSignupPhone(phone) { return domain.User{}, domain.Message{}, ErrCodeInvalid } if current, currentFound, err := s.currentPhoneOwner(ctx, phone); err != nil {