ref(service-accounts): Remove the ServiceAccountId runtime guard

src/sentry/hybridcloud/rpc/sig.py

@@ -7,7 +7,6 @@
 
 import pydantic
 from django.utils.functional import LazyObject
-from rest_framework.exceptions import APIException
 
 from sentry.hybridcloud.rpc import ArgumentDict
 
@@ -132,10 +131,6 @@ def serialize_arguments(self, raw_arguments: ArgumentDict) -> ArgumentDict:
 
         try:
             model_instance = self._parameter_model(**raw_arguments)
-        except APIException:
-            # e.g. UserRequired from a service account's id: already an API response.
-            # Temporary, like ServiceAccountId: goes once mypy enforces request.user's type.
-            raise
         except Exception as e:
             raise SerializableFunctionValueException(self, "Could not serialize arguments") from e
         return model_instance.dict()

src/sentry/service_accounts/principal.py

@@ -1,33 +1,6 @@
 from __future__ import annotations
 
 import dataclasses
-from typing import TYPE_CHECKING, NoReturn
-
-from sentry.api.exceptions import UserRequired
-
-
-@dataclasses.dataclass(frozen=True)
-class ServiceAccountId:
-    """
-    A service account's ``request.user.id``.
-
-    It never equals a user id, and turning it into an int raises ``UserRequired``, so a
-    lookup or write keyed on ``request.user.id`` fails instead of reaching the user with
-    the same number.
-
-    A runtime guard for code mypy's ``request.user`` typing doesn't cover, such as
-    getsentry.
-    """
-
-    service_account_id: int
-
-    def __int__(self) -> NoReturn:
-        raise UserRequired()
-
-    __index__ = __int__
-
-    def __str__(self) -> str:
-        return f"service_account:{self.service_account_id}"
 
 
 @dataclasses.dataclass(frozen=True)
@@ -51,12 +24,5 @@ class ServiceAccountPrincipal:
     is_sentry_app = False
     is_suspended = False
 
-    # Hidden from mypy so request.user.id needs narrowing; kept at runtime as a guard.
-    if not TYPE_CHECKING:
-
-        @property
-        def id(self) -> ServiceAccountId:
-            return ServiceAccountId(self.service_account_id)
-
     def get_display_name(self) -> str:
         return self.name

tests/sentry/service_accounts/test_principal.py

@@ -1,9 +1,7 @@
-import pytest
 from django.contrib.auth.models import AnonymousUser
 from django.test import RequestFactory
 from rest_framework.request import Request
 
-from sentry.api.exceptions import UserRequired
 from sentry.auth.principal import (
     RequestPrincipal,
     acting_user_id,
@@ -12,28 +10,11 @@
     set_request_principal,
 )
 from sentry.auth.services.auth import AuthenticatedToken
-from sentry.models.organizationmember import OrganizationMember
 from sentry.service_accounts.auth import SERVICE_ACCOUNT_TOKEN_KIND
-from sentry.service_accounts.principal import ServiceAccountId, ServiceAccountPrincipal
-from sentry.testutils.cases import TestCase
-from sentry.testutils.silo import cell_silo_test
-from sentry.users.services.user.service import user_service
+from sentry.service_accounts.principal import ServiceAccountPrincipal
 from sentry.viewer_context import Actor, ActorType
 
 
-def test_id_never_equals_a_user_id() -> None:
-    account_id: object = ServiceAccountId(42)
-    assert account_id != 42
-    assert account_id not in {42}
-    assert account_id == ServiceAccountId(42)
-    assert str(account_id) == "service_account:42"
-
-
-def test_id_refuses_int_conversion() -> None:
-    with pytest.raises(UserRequired):
-        int(ServiceAccountId(42))
-
-
 def test_principal() -> None:
     principal: RequestPrincipal = ServiceAccountPrincipal(
         service_account_id=42, organization_id=1, name="autofix"
@@ -45,6 +26,7 @@ def test_principal() -> None:
     assert principal.get_display_name() == "autofix"
     assert not hasattr(principal, "email")
     assert not hasattr(principal, "pk")
+    assert not hasattr(principal, "id")
 
 
 def _principal() -> ServiceAccountPrincipal:
@@ -74,20 +56,3 @@ def test_django_request_stays_anonymous() -> None:
     request = RequestFactory().get("/")
     set_request_principal(request, _principal(), _token())
     assert isinstance(request.user, AnonymousUser)
-
-
-@cell_silo_test
-class ServiceAccountIdCollisionTest(TestCase):
-    def test_orm_lookup_by_id_raises(self) -> None:
-        principal = ServiceAccountPrincipal(
-            service_account_id=self.user.id, organization_id=self.organization.id, name="a"
-        )
-        with pytest.raises(UserRequired):
-            OrganizationMember.objects.filter(user_id=principal.id).exists()  # type: ignore[attr-defined]
-
-    def test_rpc_lookup_by_id_raises(self) -> None:
-        principal = ServiceAccountPrincipal(
-            service_account_id=self.user.id, organization_id=self.organization.id, name="a"
-        )
-        with pytest.raises(UserRequired):
-            user_service.get_user(user_id=principal.id)  # type: ignore[attr-defined]