From cf654d3332f361de67a7abf8356bdf892723529a Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:44:03 -0800 Subject: [PATCH] ref(typing): Let issue search take a service account Type the issue search helpers with RequestPrincipal, dropping five files from the request.user allowlist. get_teams_for_users skips callers with no user, so a service account searching assigned:my_teams matches no teams instead of failing with 403. --- src/sentry/api/helpers/group_index/index.py | 7 ++- .../issues/endpoints/group_event_details.py | 5 +-- src/sentry/issues/issue_search.py | 44 +++++++++---------- src/sentry/search/utils.py | 14 +++--- .../endpoints/organization_detector_index.py | 5 ++- tests/sentry/auth/test_principal.py | 2 +- .../test_organization_group_index.py | 38 ++++++++++++++++ tools/mypy_helpers/request_user_allowlist.txt | 5 --- 8 files changed, 75 insertions(+), 45 deletions(-) diff --git a/src/sentry/api/helpers/group_index/index.py b/src/sentry/api/helpers/group_index/index.py index 0f72c13839fc..db56b9534c7d 100644 --- a/src/sentry/api/helpers/group_index/index.py +++ b/src/sentry/api/helpers/group_index/index.py @@ -5,7 +5,6 @@ from typing import Any, ParamSpec, TypeVar import sentry_sdk -from django.contrib.auth.models import AnonymousUser from rest_framework.authentication import SessionAuthentication from rest_framework.exceptions import ParseError from rest_framework.request import Request @@ -15,6 +14,7 @@ from sentry.api.event_search import AggregateFilter, SearchFilter from sentry.api.helpers.environments import get_environment from sentry.api.serializers import serialize +from sentry.auth.principal import RequestPrincipal from sentry.constants import DEFAULT_SORT_OPTION from sentry.exceptions import InvalidSearchQuery from sentry.issues.issue_search import convert_query_values, parse_search_query @@ -25,7 +25,6 @@ from sentry.models.release import Release from sentry.signals import advanced_search_feature_gated from sentry.snuba.referrer import Referrer -from sentry.users.models.user import User from sentry.utils import metrics from sentry.utils.cursors import Cursor, CursorResult @@ -51,7 +50,7 @@ def parse_and_convert_issue_search_query( organization: Organization, projects: Sequence[Project], environments: Sequence[Environment] | None, - user: User | AnonymousUser, + user: RequestPrincipal, ) -> Sequence[SearchFilter | AggregateFilter]: try: search_filters = convert_query_values( @@ -129,7 +128,7 @@ def build_query_params_from_request( def validate_search_filter_permissions( organization: Organization, search_filters: Sequence[AggregateFilter | SearchFilter], - user: User | AnonymousUser, + user: RequestPrincipal, ) -> None: """ Verifies that an organization is allowed to perform the query that they diff --git a/src/sentry/issues/endpoints/group_event_details.py b/src/sentry/issues/endpoints/group_event_details.py index 714a4c095aed..68086e3e7d2b 100644 --- a/src/sentry/issues/endpoints/group_event_details.py +++ b/src/sentry/issues/endpoints/group_event_details.py @@ -7,7 +7,6 @@ from typing import Any, cast import sentry_sdk -from django.contrib.auth.models import AnonymousUser from drf_spectacular.utils import OpenApiParameter, extend_schema from rest_framework.exceptions import ParseError from rest_framework.request import Request @@ -45,6 +44,7 @@ from sentry.apidocs.parameters import EventParams, GlobalParams, IssueParams from sentry.apidocs.response_types import DetailResponse from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import RequestPrincipal from sentry.constants import CELL_API_DEPRECATION_DATE from sentry.exceptions import InvalidParams, InvalidSearchQuery from sentry.issues.endpoints.bases.group import GroupEndpoint @@ -76,7 +76,6 @@ from sentry.services.eventstore.models import Event, GroupEvent from sentry.snuba.dataset import Dataset from sentry.types.ratelimit import RateLimit, RateLimitCategory -from sentry.users.models.user import User from sentry.utils import metrics from sentry.utils.snuba import get_snuba_column_name @@ -92,7 +91,7 @@ def format_search_filter(self, term: SearchFilter) -> WhereType | None: def issue_search_query_to_conditions( - query: str, group: Group, user: User | AnonymousUser, environments: Sequence[Environment] + query: str, group: Group, user: RequestPrincipal, environments: Sequence[Environment] ) -> tuple[list[Condition], list[Any]]: from sentry.utils.snuba import resolve_conditions diff --git a/src/sentry/issues/issue_search.py b/src/sentry/issues/issue_search.py index 32f7d2802141..5a6c29b94e8e 100644 --- a/src/sentry/issues/issue_search.py +++ b/src/sentry/issues/issue_search.py @@ -5,8 +5,6 @@ from functools import partial from typing import cast, overload -from django.contrib.auth.models import AnonymousUser - from sentry.api.event_search import ( AggregateFilter, ParenExpression, @@ -19,6 +17,7 @@ default_config, ) from sentry.api.event_search import parse_search_query as base_parse_query +from sentry.auth.principal import RequestPrincipal from sentry.exceptions import InvalidSearchQuery from sentry.issues.grouptype import ( GroupCategory, @@ -45,7 +44,6 @@ from sentry.seer.autofix.constants import FixabilityScoreThresholds from sentry.seer.autofix.issue_search import AUTOFIX_STATE_VALUES from sentry.types.group import SUBSTATUS_UPDATE_CHOICES, PriorityLevel -from sentry.users.models.user import User from sentry.users.services.user import RpcUser is_filter_translation: dict[str, tuple[str, int]] = { @@ -89,7 +87,7 @@ [ Iterable[str], Sequence[Project], - User | RpcUser | AnonymousUser | None, + RequestPrincipal | None, Sequence[Environment] | None, ], str | list[str] | list[RpcUser | Team | None] | list[RpcUser] | list[int] | list[float], @@ -99,7 +97,7 @@ def convert_actor_or_none_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[RpcUser | Team | None]: # TODO: This will make N queries. This should be ok, we don't typically have large @@ -116,7 +114,7 @@ def convert_actor_or_none_value( def convert_user_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[RpcUser]: # TODO: This will make N queries. This should be ok, we don't typically have large @@ -127,7 +125,7 @@ def convert_user_value( def convert_release_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> str | list[str]: # TODO: This will make N queries. This should be ok, we don't typically have large @@ -144,7 +142,7 @@ def convert_release_value( def convert_first_release_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[str]: # TODO: This will make N queries. This should be ok, we don't typically have large @@ -158,7 +156,7 @@ def convert_first_release_value( def convert_substatus_value( value: Iterable[str | int], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[int]: parsed = [] @@ -173,7 +171,7 @@ def convert_substatus_value( def convert_status_value( value: Iterable[str | int], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[int]: parsed = [] @@ -188,7 +186,7 @@ def convert_status_value( def convert_category_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[int]: """Convert a value like 'error' or 'performance' to the GroupType value for issue lookup""" @@ -204,7 +202,7 @@ def convert_category_value( def convert_priority_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[int]: """Convert a value like 'high' or 'medium' to the Priority value for issue lookup""" @@ -220,7 +218,7 @@ def convert_priority_value( def convert_type_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[int]: """Convert a value like 'error' or 'performance_n_plus_one_db_queries' to the GroupType value for issue lookup""" @@ -236,7 +234,7 @@ def convert_type_value( def convert_device_class_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[str]: """Convert high, medium, and low to the underlying device class values""" @@ -252,7 +250,7 @@ def convert_device_class_value( def convert_seer_actionability_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[float]: """Convert high, medium, and low to fixability score thresholds""" @@ -268,7 +266,7 @@ def convert_seer_actionability_value( def convert_issue_progress_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[str]: results: list[str] = [] @@ -284,7 +282,7 @@ def convert_issue_progress_value( def convert_autofix_state_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[str]: results: list[str] = [] @@ -298,7 +296,7 @@ def convert_autofix_state_value( def convert_detector_value( value: Iterable[str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, ) -> list[str]: """Convert detector ID(s) to a list of strings for consistent handling""" @@ -332,7 +330,7 @@ def convert_detector_value( def convert_query_values( search_filters: list[SearchFilter], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, value_converters: Mapping[str, ValueConverter] = value_converters, allow_aggregate_filters: bool = False, @@ -344,7 +342,7 @@ def convert_query_values( def convert_query_values( search_filters: Sequence[AggregateFilter | SearchFilter], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, value_converters: Mapping[str, ValueConverter] = value_converters, allow_aggregate_filters: bool = False, @@ -355,7 +353,7 @@ def convert_query_values( def convert_query_values( search_filters: Sequence[QueryToken], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, value_converters: Mapping[str, ValueConverter] = value_converters, allow_aggregate_filters: bool = False, @@ -366,7 +364,7 @@ def convert_query_values( def convert_query_values( search_filters: Sequence[ParsedTerm], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, value_converters: Mapping[str, ValueConverter] = value_converters, allow_aggregate_filters: bool = False, @@ -376,7 +374,7 @@ def convert_query_values( def convert_query_values( search_filters: Sequence[QueryToken | str], projects: Sequence[Project], - user: User | RpcUser | AnonymousUser | None, + user: RequestPrincipal | None, environments: Sequence[Environment] | None, value_converters: Mapping[str, ValueConverter] = value_converters, allow_aggregate_filters: bool = False, diff --git a/src/sentry/search/utils.py b/src/sentry/search/utils.py index df54c4effe89..15f0cbdff075 100644 --- a/src/sentry/search/utils.py +++ b/src/sentry/search/utils.py @@ -6,11 +6,10 @@ from enum import Enum from typing import Any, Optional -from django.contrib.auth.models import AnonymousUser from django.db import DataError, connections, router from django.utils import timezone as django_timezone -from sentry.auth.principal import RequestPrincipal +from sentry.auth.principal import RequestPrincipal, has_user from sentry.exceptions import InvalidSearchQuery from sentry.models.environment import Environment from sentry.models.group import STATUS_QUERY_CHOICES, Group @@ -318,12 +317,11 @@ def parse_team_value(projects: Sequence[Project], value: Sequence[str]) -> Team: def get_teams_for_users( - projects: Sequence[Project], - users: Sequence[User | RpcUser | AnonymousUser | None], + projects: Sequence[Project], users: Sequence[RequestPrincipal | None] ) -> list[Team]: if not projects: return [] - user_ids = [u.id for u in users if u is not None] + user_ids = [u.id for u in users if u is not None and has_user(u)] teams = Team.objects.filter( id__in=OrganizationMemberTeam.objects.filter( organizationmember__in=OrganizationMember.objects.filter( @@ -336,7 +334,7 @@ def get_teams_for_users( def parse_actor_value( - projects: Sequence[Project], value: str, user: User | RpcUser | AnonymousUser | None + projects: Sequence[Project], value: str, user: RequestPrincipal | None ) -> RpcUser | Team: if value.startswith("#"): return parse_team_value(projects, value) @@ -344,7 +342,7 @@ def parse_actor_value( def parse_actor_or_none_value( - projects: Sequence[Project], value: str, user: User | RpcUser | AnonymousUser | None + projects: Sequence[Project], value: str, user: RequestPrincipal | None ) -> RpcUser | Team | None: if value == "none": return None @@ -741,7 +739,7 @@ def split_query_into_tokens(query: str) -> Sequence[str]: def parse_query( projects: Sequence[Project], query: str, - user: User | RpcUser | AnonymousUser, + user: RequestPrincipal, environments: Sequence[Environment], ) -> dict[str, Any]: """| Parses the query string and returns a dict of structured query term values: diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_index.py b/src/sentry/workflow_engine/endpoints/organization_detector_index.py index b8811a5b9145..d3acf16e6e62 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -40,6 +40,7 @@ ) from sentry.apidocs.response_types import DetailResponse from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.auth.principal import RequestPrincipal from sentry.constants import ObjectStatus from sentry.deletions.models.scheduleddeletion import CellScheduledDeletion from sentry.exceptions import InvalidSearchQuery @@ -86,7 +87,9 @@ parse_detector_query = partial(base_parse_search_query, config=detector_search_config) -def convert_assignee_values(value: Iterable[str], projects: Sequence[Project], user: User) -> Q: +def convert_assignee_values( + value: Iterable[str], projects: Sequence[Project], user: RequestPrincipal +) -> Q: """ Convert an assignee search value to a Django Q object for filtering detectors. """ diff --git a/tests/sentry/auth/test_principal.py b/tests/sentry/auth/test_principal.py index b36d7147802f..f5ff015ce1c1 100644 --- a/tests/sentry/auth/test_principal.py +++ b/tests/sentry/auth/test_principal.py @@ -277,7 +277,7 @@ def _is_request_user_type_check(node: ast.AST) -> bool: # Lower these as callers move off. Exact, so the baseline can't drift back up. REQUEST_USER_ID_BASELINE = 133 REQUEST_USER_TYPE_CHECK_BASELINE = 29 -REQUEST_USER_ALLOWLIST_BASELINE = 22 +REQUEST_USER_ALLOWLIST_BASELINE = 17 def test_request_user_id_uses_only_go_down() -> None: diff --git a/tests/sentry/issues/endpoints/test_organization_group_index.py b/tests/sentry/issues/endpoints/test_organization_group_index.py index 851e2ba66195..6274658f5cd9 100644 --- a/tests/sentry/issues/endpoints/test_organization_group_index.py +++ b/tests/sentry/issues/endpoints/test_organization_group_index.py @@ -12,6 +12,7 @@ from django.db import connection from django.db.utils import OperationalError +from django.test import override_settings from django.test.utils import CaptureQueriesContext from django.urls import reverse from django.utils import timezone @@ -79,6 +80,7 @@ from sentry.users.models.user_option import UserOption from sentry.utils import json from sentry.utils.snuba import SnubaQueryParams +from sentry.viewer_context import Actor, ActorType, ViewerContext, encode_viewer_context from tests.sentry.feedback import mock_feedback_event from tests.sentry.issues.test_utils import SearchIssueTestMixin @@ -421,6 +423,42 @@ def test_sort_by_inbox_my_teams_without_teams(self) -> None: ) assert response.data == [] + @override_settings(SEER_API_SHARED_SECRET="service-account-secret") + @with_feature("organizations:service-accounts") + def test_service_account_has_no_teams_or_self_to_search_by(self) -> None: + group = self.store_event( + data={"timestamp": before_now(seconds=1).isoformat(), "fingerprint": ["group-1"]}, + project_id=self.project.id, + ).group + assert group is not None + GroupAssignee.objects.assign(group, self.team) + service_account = self.create_service_account( + organization=self.organization, + scope_list=["event:read"], + project_ids=[self.project.id], + ) + viewer_context = encode_viewer_context( + ViewerContext( + actor=Actor(type=ActorType.SERVICE_ACCOUNT, actor_id=service_account.id), + organization_id=self.organization.id, + ), + key="service-account-secret", + ) + + def search(query: str) -> list[str]: + response = self.client.get( + reverse(self.endpoint, args=[self.organization.slug]), + {"query": query, "project": [self.project.id]}, + HTTP_X_VIEWER_CONTEXT=viewer_context, + ) + assert response.status_code == 200, response.content + return [item["id"] for item in response.data] + + assert search("is:unresolved") == [str(group.id)] + assert search("assigned:my_teams") == [] + assert search("!assigned:my_teams") == [str(group.id)] + assert search("assigned:me") == [] + def test_trace_search(self) -> None: event = self.store_event( data={ diff --git a/tools/mypy_helpers/request_user_allowlist.txt b/tools/mypy_helpers/request_user_allowlist.txt index 11fc42541604..96a7e10f7035 100644 --- a/tools/mypy_helpers/request_user_allowlist.txt +++ b/tools/mypy_helpers/request_user_allowlist.txt @@ -5,18 +5,13 @@ src/sentry/api/endpoints/organization_events_stats.py src/sentry/api/endpoints/organization_releases.py src/sentry/api/endpoints/organization_trace_item_attributes.py src/sentry/api/endpoints/prompts_activity.py -src/sentry/api/helpers/group_index/index.py src/sentry/core/endpoints/organization_member_details.py src/sentry/core/endpoints/organization_member_index.py src/sentry/core/endpoints/organization_member_requests_invite_index.py src/sentry/dashboards/endpoints/organization_dashboards.py src/sentry/data_export/endpoints/data_export.py -src/sentry/issues/endpoints/group_event_details.py -src/sentry/issues/endpoints/group_events.py src/sentry/issues/endpoints/group_integration_details.py -src/sentry/issues/endpoints/organization_issues_count.py src/sentry/notebooks/endpoints/base.py src/sentry/seer/endpoints/organization_agent_token.py src/sentry/users/api/endpoints/user_authenticator_details.py src/sentry/web/client_config.py -src/sentry/workflow_engine/endpoints/organization_detector_index.py