fix: require verified primary email before adding 2fa (#81982)

A user could add 2FA with a verified secondary email address, but not a
verified primary address. This leads to some confusing edge cases with
security impact.

This PR adds a new decorator `primary_email_verification_required` that
specifically ensures the account's primary email is verified for the
particular endpoint it is wrapping. The endpoint used to enroll new 2FA
interfaces has been switched over to this new decorator.

src/sentry/api/decorators.py

@@ -4,7 +4,11 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry.api.exceptions import EmailVerificationRequired, SudoRequired
+from sentry.api.exceptions import (
+    EmailVerificationRequired,
+    PrimaryEmailVerificationRequired,
+    SudoRequired,
+)
 from sentry.models.apikey import is_api_key_auth
 from sentry.models.apitoken import is_api_token_auth
 from sentry.models.orgauthtoken import is_org_auth_token_auth
@@ -45,3 +49,13 @@ def wrapped(self, request: Request, *args, **kwargs) -> Response:
         return func(self, request, *args, **kwargs)
 
     return wrapped
+
+
+def primary_email_verification_required(func):
+    @wraps(func)
+    def wrapped(self, request: Request, *args, **kwargs) -> Response:
+        if isinstance(request.user, AnonymousUser) or not request.user.has_verified_primary_email():
+            raise PrimaryEmailVerificationRequired(request.user)
+        return func(self, request, *args, **kwargs)
+
+    return wrapped

src/sentry/api/exceptions.py

@@ -118,6 +118,15 @@ def __init__(self, user):
         super().__init__(username=user.username)
 
 
+class PrimaryEmailVerificationRequired(SentryAPIException):
+    status_code = status.HTTP_401_UNAUTHORIZED
+    code = "primary-email-verification-required"
+    message = "Primary email verification required."
+
+    def __init__(self, user):
+        super().__init__(username=user.username)
+
+
 class TwoFactorRequired(SentryAPIException):
     status_code = status.HTTP_401_UNAUTHORIZED
     code = "2fa-required"

src/sentry/users/api/endpoints/user_authenticator_enroll.py

@@ -13,7 +13,7 @@
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import control_silo_endpoint
-from sentry.api.decorators import email_verification_required, sudo_required
+from sentry.api.decorators import primary_email_verification_required, sudo_required
 from sentry.api.invite_helper import ApiInviteHelper, remove_invite_details_from_session
 from sentry.api.serializers import serialize
 from sentry.auth.authenticators.base import EnrollmentStatus, NewEnrollmentDisallowed
@@ -175,7 +175,7 @@ def get(self, request: Request, user: User, interface_id: str) -> HttpResponse:
         return Response(response)
 
     @sudo_required
-    @email_verification_required
+    @primary_email_verification_required
     def post(self, request: Request, user: User, interface_id: str) -> HttpResponse:
         """
         Enroll in authenticator interface

src/sentry/users/models/user.py

@@ -244,6 +244,9 @@ def has_verified_emails(self) -> bool:
     def has_unverified_emails(self) -> bool:
         return self.get_unverified_emails().exists()
 
+    def has_verified_primary_email(self) -> bool:
+        return self.emails.filter(is_verified=True, email=self.email).exists()
+
     def has_usable_password(self) -> bool:
         if self.password == "" or self.password is None:
             # This is the behavior we've been relying on from Django 1.6 - 2.0.

src/sentry/users/services/user/model.py

@@ -87,6 +87,9 @@ def has_unverified_emails(self) -> bool:
     def has_verified_emails(self) -> bool:
         return len(self.get_verified_emails()) > 0
 
+    def has_verified_primary_email(self) -> bool:
+        return bool([e for e in self.useremails if e.is_verified and e.email == self.email])
+
     def get_unverified_emails(self) -> list[RpcUserEmail]:
         return [e for e in self.useremails if not e.is_verified]
 

tests/sentry/users/api/endpoints/test_user_authenticator_enroll.py

@@ -84,6 +84,47 @@ def test_totp_can_enroll(self, validate_otp):
         assert interface.secret == "secret56"
         assert interface.config == {"secret": "secret56"}
 
+    @mock.patch("sentry.auth.authenticators.TotpInterface.validate_otp", return_value=True)
+    def test_totp_no_verified_primary_email(self, validate_otp):
+        from urllib.parse import quote
+
+        user = self.create_user()
+        UserEmail.objects.filter(user=user, email=user.email).update(is_verified=False)
+        self.login_as(user)
+
+        # XXX: Pretend an unbound function exists.
+        validate_otp.__func__ = None
+
+        with mock.patch(
+            "sentry.auth.authenticators.base.generate_secret_key", return_value="Z" * 32
+        ):
+            resp = self.get_success_response("me", "totp")
+
+        assert resp.data["secret"] == "Z" * 32
+        assert (
+            resp.data["qrcode"]
+            == f"otpauth://totp/{quote(user.email)}?issuer=Sentry&secret=ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ"
+        )
+        assert resp.data["form"]
+        assert resp.data["secret"]
+
+        # try to enroll
+        with self.tasks():
+            resp = self.get_error_response(
+                "me",
+                "totp",
+                method="post",
+                status_code=401,
+                **{"secret": "secret12", "otp": "1234"},
+            )
+            assert resp.data == {
+                "detail": {
+                    "code": "primary-email-verification-required",
+                    "message": "Primary email verification required.",
+                    "extra": {"username": user.email},
+                }
+            }
+
     @override_options({"totp.disallow-new-enrollment": True})
     def test_totp_disallow_new_enrollment(self):
         self.get_error_response(
@@ -188,8 +229,8 @@ def test_sms_no_verified_email(self):
         )
         assert resp.data == {
             "detail": {
-                "code": "email-verification-required",
-                "message": "Email verification required.",
+                "code": "primary-email-verification-required",
+                "message": "Primary email verification required.",
                 "extra": {"username": user.email},
             }
         }