diff --git a/src/sentry/auth/access.py b/src/sentry/auth/access.py index a93d97f77eb0f4..c5d2e76f2c40f3 100644 --- a/src/sentry/auth/access.py +++ b/src/sentry/auth/access.py @@ -1079,12 +1079,12 @@ def _from_sentry_app(user: User, organization: Organization | None = None) -> Ac def _from_rpc_sentry_app(context: RpcUserOrganizationContext | None = None) -> Access: - from sentry.sentry_apps.services.app import app_service + from sentry.sentry_apps.services.app.service import get_installation_by_proxy_user if not context or context.user_id is None: return NoAccess() - installation = app_service.find_installation_by_proxy_user( + installation = get_installation_by_proxy_user( proxy_user_id=context.user_id, organization_id=context.organization.id ) if installation is None: diff --git a/src/sentry/ratelimits/utils.py b/src/sentry/ratelimits/utils.py index b8e21727248bc0..c62c6ea63be81e 100644 --- a/src/sentry/ratelimits/utils.py +++ b/src/sentry/ratelimits/utils.py @@ -141,9 +141,9 @@ def get_rate_limit_key( def get_organization_id_from_token(token_id: int) -> int | None: - from sentry.sentry_apps.services.app import app_service + from sentry.sentry_apps.services.app.service import get_installation_org_id_by_token_id - organization_id = app_service.get_installation_org_id_by_token_id(token_id=token_id) + organization_id = get_installation_org_id_by_token_id(token_id) # Return None to avoid collisions caused by tokens not being associated with # a SentryAppInstallation. We fallback to IP address rate limiting in this case. if not organization_id: diff --git a/src/sentry/sentry_apps/models/sentry_app_installation.py b/src/sentry/sentry_apps/models/sentry_app_installation.py index 78a7e48589f489..389a5f35f60f9e 100644 --- a/src/sentry/sentry_apps/models/sentry_app_installation.py +++ b/src/sentry/sentry_apps/models/sentry_app_installation.py @@ -167,6 +167,7 @@ def outboxes_for_delete(self) -> list[ControlOutbox]: payload={ "sentry_app_id": self.sentry_app_id, "organization_id": self.organization_id, + "proxy_user_id": self.sentry_app.proxy_user_id, }, ) for cell_name in find_all_cell_names() @@ -184,7 +185,10 @@ def prepare_ui_component( def handle_async_replication(self, cell_name: str, shard_identifier: int) -> None: from sentry.hybridcloud.rpc.caching import cell_caching_service - from sentry.sentry_apps.services.app.service import get_installation + from sentry.sentry_apps.services.app.service import ( + clear_installation_by_proxy_user_cache, + get_installation, + ) if self.api_token is not None: # ApiTokens replicate the organization_id they are associated with. @@ -193,6 +197,18 @@ def handle_async_replication(self, cell_name: str, shard_identifier: int) -> Non ob.save() cell_caching_service.clear_key(key=get_installation.key_from(self.id), cell_name=cell_name) + # Invalidate the proxy-user installation cache so that permission checks and rate-limit + # key lookups for Sentry Apps don't serve stale data after an install is updated. + try: + proxy_user_id = self.sentry_app.proxy_user_id + if proxy_user_id is not None: + clear_installation_by_proxy_user_cache( + proxy_user_id=proxy_user_id, + organization_id=self.organization_id, + ) + except Exception: + pass + @classmethod def handle_async_deletion( cls, @@ -202,6 +218,7 @@ def handle_async_deletion( payload: Mapping[str, Any] | None, ) -> None: from sentry.models.apitoken import ApiToken + from sentry.sentry_apps.services.app.service import clear_installation_by_proxy_user_cache if payload: api_token_id = payload.get("api_token_id", None) @@ -211,6 +228,15 @@ def handle_async_deletion( for ob in ApiToken(id=api_token_id, user_id=user_id).outboxes_for_update(): ob.save() + # Invalidate the proxy-user installation cache on deletion. + proxy_user_id = payload.get("proxy_user_id") + organization_id = payload.get("organization_id") + if isinstance(proxy_user_id, int) and isinstance(organization_id, int): + clear_installation_by_proxy_user_cache( + proxy_user_id=proxy_user_id, + organization_id=organization_id, + ) + def payload_for_update(self) -> dict[str, Any] | None: from sentry.models.apitoken import ApiToken diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index 2dced307d297f7..cbb39a3e34d5ac 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -266,4 +266,71 @@ def get_by_application_id(application_id: int) -> RpcSentryApp | None: return app_service.find_service_hook_sentry_app(api_application_id=application_id) +def get_installation_org_id_by_token_id(token_id: int) -> int | None: + """ + Cached wrapper around app_service.get_installation_org_id_by_token_id. + + Avoids a synchronous cross-silo RPC call on every Sentry App request + hitting the rate-limiter by caching the result in the cell silo's Django cache. + """ + from django.core.cache import cache + + cache_key = f"sentry-app-install-token:{token_id}" + result = cache.get(cache_key) + if result is not None: + if result == _INSTALLATION_NOT_FOUND_SENTINEL: + return None + return result + + organization_id = app_service.get_installation_org_id_by_token_id(token_id=token_id) + # Cache both found and not-found results for 5 minutes. + cache.set( + cache_key, + organization_id if organization_id is not None else _INSTALLATION_NOT_FOUND_SENTINEL, + 300, + ) + return organization_id + + +def get_installation_by_proxy_user( + proxy_user_id: int, organization_id: int +) -> "RpcSentryAppInstallation | None": + """ + Cached wrapper around app_service.find_installation_by_proxy_user. + + Uses a short-lived Django cache entry to avoid a synchronous cross-silo RPC + call on every Sentry App request during the permission check. The cache is + invalidated whenever the installation changes via handle_async_replication / + handle_async_deletion on SentryAppInstallation. + """ + from django.core.cache import cache + + cache_key = f"sentry-app-install-proxy:{proxy_user_id}:{organization_id}" + result = cache.get(cache_key) + if result is not None: + # Sentinel: we cached a "not found" result + if result == _INSTALLATION_NOT_FOUND_SENTINEL: + return None + return result + + installation = app_service.find_installation_by_proxy_user( + proxy_user_id=proxy_user_id, organization_id=organization_id + ) + # Cache both found and not-found results for 5 minutes to avoid repeated RPCs. + cache.set(cache_key, installation if installation is not None else _INSTALLATION_NOT_FOUND_SENTINEL, 300) + return installation + + +# Sentinel value used to distinguish a cached "not found" result from a cache miss. +_INSTALLATION_NOT_FOUND_SENTINEL = "NOT_FOUND" + + +def clear_installation_by_proxy_user_cache(proxy_user_id: int, organization_id: int) -> None: + """Invalidate the find_installation_by_proxy_user cache entry for the given pair.""" + from django.core.cache import cache + + cache_key = f"sentry-app-install-proxy:{proxy_user_id}:{organization_id}" + cache.delete(cache_key) + + app_service = AppService.create_delegation()