diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 0636c6561eb187..e7cd168a83a4f1 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -394,16 +394,12 @@ def get_projects( :return: A list of Project objects, or raises PermissionDenied. When project_ids or project_slugs are explicitly provided, the returned list is guaranteed non-empty (or PermissionDenied is raised). - NOTE: Passing both project_ids and project_slugs raises ``ParseError``. """ qs = Project.objects.filter(organization_id=organization.id, status=ObjectStatus.ACTIVE) - if project_slugs and project_ids: - raise ParseError(detail="Cannot query for both ids and slugs") - - if project_ids: - requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set()) - elif project_slugs: - requested_projects = ParsedProjectIdOrSlugParams(ids=set(), slugs=set(project_slugs)) + if project_ids or project_slugs: + requested_projects = ParsedProjectIdOrSlugParams( + ids=project_ids or set(), slugs=set(project_slugs or ()) + ) else: requested_projects = self.get_requested_project_params_unchecked(request) ids = requested_projects.ids diff --git a/src/sentry/api/endpoints/release_thresholds/release_threshold_index.py b/src/sentry/api/endpoints/release_thresholds/release_threshold_index.py index f5f1b3234e02ce..b6d1827dd23327 100644 --- a/src/sentry/api/endpoints/release_thresholds/release_threshold_index.py +++ b/src/sentry/api/endpoints/release_thresholds/release_threshold_index.py @@ -10,24 +10,29 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import cell_silo_endpoint from sentry.api.bases.organization import OrganizationEndpoint +from sentry.api.helpers.projects import ProjectIdOrSlug, ProjectIdOrSlugField from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import serialize +from sentry.constants import ALL_ACCESS_PROJECTS_SLUG from sentry.models.organization import Organization from sentry.models.release_threshold.release_threshold import ReleaseThreshold class ReleaseThresholdIndexGETData(TypedDict, total=False): environment: list[str] - project: list[int] + project: list[ProjectIdOrSlug] class ReleaseThresholdIndexGETValidator(serializers.Serializer[ReleaseThresholdIndexGETData]): environment = serializers.ListField( required=False, allow_empty=True, child=serializers.CharField() ) - project = serializers.ListField( - required=True, allow_empty=False, child=serializers.IntegerField() - ) + project = serializers.ListField(required=True, allow_empty=False, child=ProjectIdOrSlugField()) + + def validate_project(self, value: list[ProjectIdOrSlug]) -> list[ProjectIdOrSlug]: + if ALL_ACCESS_PROJECTS_SLUG in value: + raise serializers.ValidationError("Invalid project") + return value @cell_silo_endpoint @@ -39,7 +44,7 @@ class ReleaseThresholdIndexEndpoint(OrganizationEndpoint): def get(self, request: Request, organization: Organization) -> HttpResponse: validator = ReleaseThresholdIndexGETValidator( - data=request.query_params, + data=self.get_query_params_without_empty_project_params(request), ) if not validator.is_valid(): return Response(validator.errors, status=400) diff --git a/src/sentry/api/endpoints/release_thresholds/release_threshold_status_index.py b/src/sentry/api/endpoints/release_thresholds/release_threshold_status_index.py index 7b4a47f02d3267..e04421aec00311 100644 --- a/src/sentry/api/endpoints/release_thresholds/release_threshold_status_index.py +++ b/src/sentry/api/endpoints/release_thresholds/release_threshold_status_index.py @@ -28,6 +28,11 @@ get_errors_counts_timeseries_by_project_and_release, get_new_issue_counts, ) +from sentry.api.helpers.projects import ( + ProjectIdOrSlug, + ProjectIdOrSlugField, + parse_id_or_slug_params, +) from sentry.api.serializers import serialize from sentry.apidocs.constants import RESPONSE_BAD_REQUEST from sentry.apidocs.examples.release_threshold_examples import ReleaseThresholdExamples @@ -56,6 +61,7 @@ class ReleaseThresholdStatusIndexData(TypedDict, total=False): environment: list[str] projectSlug: list[str] release: list[str] + project: list[ProjectIdOrSlug] class ReleaseThresholdStatusIndexSerializer( @@ -82,7 +88,7 @@ class ReleaseThresholdStatusIndexSerializer( projectSlug = serializers.ListField( required=False, allow_empty=True, - child=serializers.CharField(), + child=serializers.CharField(allow_blank=True), help_text=("A list of project slugs to filter your results by."), ) release = serializers.ListField( @@ -91,6 +97,12 @@ class ReleaseThresholdStatusIndexSerializer( child=serializers.CharField(), help_text=("A list of release versions to filter your results by."), ) + project = serializers.ListField( + required=False, + allow_empty=True, + child=ProjectIdOrSlugField(), + help_text=("A list of project IDs or slugs to filter your results by."), + ) def validate(self, data: ReleaseThresholdStatusIndexData) -> ReleaseThresholdStatusIndexData: if data["start"] >= data["end"]: @@ -136,27 +148,36 @@ def get( # NOTE: start/end parameters determine window to query for releases # This is NOT the window to query snuba for event data - nor the individual threshold windows # ======================================================================== - serializer = ReleaseThresholdStatusIndexSerializer( - data=request.query_params, - ) + query_params = self.get_query_params_with_project_slug_precedence(request) + + serializer = ReleaseThresholdStatusIndexSerializer(data=query_params) if not serializer.is_valid(): return Response(as_validation_errors(serializer), status=400) - environments_list = serializer.validated_data.get( - "environment" - ) # list of environment names - project_slug_list = serializer.validated_data.get("projectSlug") - releases_list = serializer.validated_data.get("release") # list of release versions + validated_data = serializer.validated_data + environments_list = validated_data.get("environment") # list of environment names + releases_list = validated_data.get("release") # list of release versions + + project_ids: set[int] | None = None + project_slugs = {slug for slug in validated_data.get("projectSlug", []) if slug} or None + if project_slugs is None: + requested_project = parse_id_or_slug_params(validated_data.get("project", [])) + project_ids = requested_project.ids or None + project_slugs = requested_project.slugs or None + try: filter_params = self.get_filter_params( - request, organization, date_filter_optional=True, project_slugs=project_slug_list + request, + organization, + date_filter_optional=True, + project_ids=project_ids, + project_slugs=project_slugs, ) except NoProjects: raise NoProjects("No projects available") - # Use validated project IDs from get_filter_params instead of raw user input. - # The raw project_slug_list could contain slugs for projects the user doesn't - # have access to, bypassing the permission checks in get_projects(). + # Use project IDs from get_filter_params instead of raw project filters so + # project access is checked before fetching threshold data. validated_project_ids = set(filter_params["project_id"]) start: datetime | None = filter_params["start"] @@ -203,7 +224,7 @@ def get( "Fetched releases", extra={ "results": len(queryset), - "project_slugs": project_slug_list, + "project_slugs": project_slugs, "releases": releases_list, "environments": environments_list, }, diff --git a/src/sentry/releases/endpoints/organization_release_details.py b/src/sentry/releases/endpoints/organization_release_details.py index bc4ebd3370ec72..c86d2d249a3508 100644 --- a/src/sentry/releases/endpoints/organization_release_details.py +++ b/src/sentry/releases/endpoints/organization_release_details.py @@ -1,7 +1,7 @@ import sentry_sdk from django.db.models import Q -from drf_spectacular.utils import extend_schema, extend_schema_serializer -from rest_framework.exceptions import ParseError +from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_serializer +from rest_framework.exceptions import ParseError, ValidationError from rest_framework.request import Request from rest_framework.response import Response from rest_framework.serializers import ListField @@ -17,6 +17,10 @@ get_stats_period_detail, ) from sentry.api.exceptions import ConflictError, InvalidRepository, ResourceDoesNotExist +from sentry.api.helpers.projects import ( + PROJECT_ID_OR_SLUG_SCHEMA, + ProjectIdOrSlugField, +) from sentry.api.serializers import serialize from sentry.api.serializers.rest_framework import ( ReleaseHeadCommitSerializer, @@ -38,6 +42,7 @@ as_validation_errors, ) from sentry.apidocs.utils import inline_sentry_response_serializer +from sentry.constants import ALL_ACCESS_PROJECT_ID, ALL_ACCESS_PROJECTS_SLUG from sentry.models.activity import Activity from sentry.models.organization import Organization from sentry.models.release import Release, ReleaseStatus @@ -314,7 +319,21 @@ class OrganizationReleaseDetailsEndpoint( parameters=[ GlobalParams.ORG_ID_OR_SLUG, ReleaseParams.VERSION, - ReleaseParams.PROJECT_ID, + OpenApiParameter( + name="project_id", + location="query", + required=False, + type=str, + deprecated=True, + description="Deprecated. Use project instead.", + ), + OpenApiParameter( + name="project", + location="query", + required=False, + type=PROJECT_ID_OR_SLUG_SCHEMA, + description="The project ID or slug to filter by. Overrides project_id when both are sent.", + ), ReleaseParams.HEALTH, ReleaseParams.ADOPTION_STAGES, ReleaseParams.SUMMARY_STATS_PERIOD, @@ -340,7 +359,7 @@ def get( """ # Dictionary responsible for storing selected project meta data current_project_meta = {} - project_id = request.GET.get("project") + project_id = request.GET.get("project") or request.GET.get("project_id") with_health = request.GET.get("health") == "1" with_adoption_stages = request.GET.get("adoptionStages") == "1" summary_stats_period = request.GET.get("summaryStatsPeriod") or "14d" @@ -362,15 +381,30 @@ def get( if not self.has_release_permission(request, organization, release): raise ResourceDoesNotExist - # Validate project access when project_id is provided + # Validate project access when a project identifier is provided. project = None + project_ids: set[int] | None = None + project_slugs: set[str] | None = None if project_id: try: - project_id_int = int(project_id) - except ValueError: + project_id_or_slug = ProjectIdOrSlugField().run_validation(project_id) + except ValidationError: raise ParseError(detail="Invalid project") + + if isinstance(project_id_or_slug, int): + if project_id_or_slug == ALL_ACCESS_PROJECT_ID: + raise ParseError(detail="Invalid project") + project_ids = {project_id_or_slug} + else: + if project_id_or_slug == ALL_ACCESS_PROJECTS_SLUG: + raise ParseError(detail="Invalid project") + project_slugs = {project_id_or_slug} + validated_projects = self.get_projects( - request, organization, project_ids={project_id_int} + request, + organization, + project_ids=project_ids, + project_slugs=project_slugs, ) if not validated_projects: raise ResourceDoesNotExist @@ -395,7 +429,12 @@ def get( # Get prev and next release to current release try: - filter_params = self.get_filter_params(request, organization) + filter_params = self.get_filter_params( + request, + organization, + project_ids=project_ids, + project_slugs=project_slugs, + ) current_project_meta.update( { **self.get_adjacent_releases_to_current_release( diff --git a/tests/sentry/api/bases/test_organization.py b/tests/sentry/api/bases/test_organization.py index 5c856fbf1db6ad..ff4e26baa87bb7 100644 --- a/tests/sentry/api/bases/test_organization.py +++ b/tests/sentry/api/bases/test_organization.py @@ -666,6 +666,19 @@ def test_project_param_with_mixed_ids_and_slugs(self) -> None: assert {p.id for p in result} == {self.project_1.id, self.project_2.id} + def test_explicit_project_ids_and_slugs(self) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + request = self.build_request() + + result = self.endpoint.get_projects( + request, + self.org, + project_ids={self.project_1.id}, + project_slugs={self.project_2.slug}, + ) + + assert {p.id for p in result} == {self.project_1.id, self.project_2.id} + def test_project_param_with_nonexistent_slug(self) -> None: self.create_team_membership(user=self.user, team=self.team_1) request = self.build_request(project=["nonexistent-slug"]) diff --git a/tests/sentry/api/endpoints/release_thresholds/test_release_threshold_status.py b/tests/sentry/api/endpoints/release_thresholds/test_release_threshold_status.py index 6cd07fe3d90059..cde3b874356c02 100644 --- a/tests/sentry/api/endpoints/release_thresholds/test_release_threshold_status.py +++ b/tests/sentry/api/endpoints/release_thresholds/test_release_threshold_status.py @@ -407,6 +407,64 @@ def test_get_success_project_slug_filter(self) -> None: r2_keys = [k for k, v in data.items() if k.split("-")[1] == self.release2.version] assert len(r2_keys) == 0 + def test_get_success_project_param_slug_filter(self) -> None: + now = datetime.now(UTC) + yesterday = now - timedelta(hours=24) + + project_slug_response = self.get_success_response( + self.organization.slug, start=yesterday, end=now, projectSlug=[self.project2.slug] + ) + project_response = self.get_success_response( + self.organization.slug, start=yesterday, end=now, project=[self.project2.slug] + ) + + assert project_response.data == project_slug_response.data + + def test_get_success_project_param_mixed_id_and_slug_filter(self) -> None: + now = datetime.now(UTC) + yesterday = now - timedelta(hours=24) + + response = self.get_success_response( + self.organization.slug, + start=yesterday, + end=now, + project=[str(self.project1.id), self.project2.slug], + ) + + assert set(response.data.keys()) == { + f"{self.project1.slug}-{self.release1.version}", + f"{self.project1.slug}-{self.release2.version}", + f"{self.project2.slug}-{self.release1.version}", + } + + def test_get_success_project_slug_takes_precedence_over_project_id(self) -> None: + now = datetime.now(UTC) + yesterday = now - timedelta(hours=24) + + response = self.get_success_response( + self.organization.slug, + start=yesterday, + end=now, + projectSlug=[self.project2.slug], + project=[str(self.project1.id)], + ) + + assert set(response.data.keys()) == {f"{self.project2.slug}-{self.release1.version}"} + + def test_get_success_empty_project_slug_falls_back_to_project_filter(self) -> None: + now = datetime.now(UTC) + yesterday = now - timedelta(hours=24) + + response = self.get_success_response( + self.organization.slug, + start=yesterday, + end=now, + projectSlug=[""], + project=[self.project2.slug], + ) + + assert set(response.data.keys()) == {f"{self.project2.slug}-{self.release1.version}"} + @patch( "sentry.api.endpoints.release_thresholds.release_threshold_status_index.fetch_sessions_data" ) diff --git a/tests/sentry/api/endpoints/release_thresholds/test_release_thresholds_index.py b/tests/sentry/api/endpoints/release_thresholds/test_release_thresholds_index.py index 0bc3882fbb255b..fa7bcc7e0917b6 100644 --- a/tests/sentry/api/endpoints/release_thresholds/test_release_thresholds_index.py +++ b/tests/sentry/api/endpoints/release_thresholds/test_release_thresholds_index.py @@ -22,6 +22,11 @@ def setUp(self) -> None: def test_get_invalid_project(self) -> None: self.get_error_response(self.organization.slug, project="foo bar") + def test_get_all_projects_slug_is_invalid(self) -> None: + response = self.get_error_response(self.organization.slug, project="$all") + + assert response.status_code == 400 + def test_get_no_project(self) -> None: self.get_error_response(self.organization.slug) @@ -49,6 +54,21 @@ def test_get_valid_project(self) -> None: assert created_threshold["environment"]["id"] == str(self.canary_environment.id) assert created_threshold["environment"]["name"] == self.canary_environment.name + def test_get_valid_project_slug(self) -> None: + ReleaseThreshold.objects.create( + threshold_type=0, + trigger_type=0, + value=100, + window_in_seconds=1800, + project=self.project, + environment=self.canary_environment, + ) + + response = self.get_success_response(self.organization.slug, project=self.project.slug) + + assert len(response.data) == 1 + assert response.data[0]["project"]["id"] == str(self.project.id) + def test_get_invalid_environment(self) -> None: self.get_error_response(self.organization.slug, environment="foo bar", project="-1") diff --git a/tests/sentry/releases/endpoints/test_organization_release_details.py b/tests/sentry/releases/endpoints/test_organization_release_details.py index 5c34b43ef7425e..10c3b0593c8703 100644 --- a/tests/sentry/releases/endpoints/test_organization_release_details.py +++ b/tests/sentry/releases/endpoints/test_organization_release_details.py @@ -130,6 +130,12 @@ def test_wrong_project(self) -> None: response = self.client.get(url, {"project": self.project1.id}) assert response.status_code == 200 + response = self.client.get(url, {"project_id": self.project1.id}) + assert response.status_code == 200 + + response = self.client.get(url, {"project": self.project1.slug}) + assert response.status_code == 200 + def test_project_from_another_org_is_rejected(self) -> None: """Supplying a project_id belonging to a different organization must not leak session health data (IDOR check).""" @@ -171,6 +177,38 @@ def test_project_user_has_no_access_to_is_rejected(self) -> None: response = self.client.get(url, {"project": no_access_project.id}) assert response.status_code in (403, 404) + def test_all_project_id_sentinel_is_rejected(self) -> None: + release = Release.objects.create(organization_id=self.organization.id, version="abcabcabc") + release.add_project(self.project1) + + self.create_member(teams=[self.team1], user=self.user1, organization=self.organization) + self.login_as(user=self.user1) + + url = reverse( + "sentry-api-0-organization-release-details", + kwargs={"organization_id_or_slug": self.organization.slug, "version": release.version}, + ) + + response = self.client.get(url, {"project": "-1"}) + assert response.status_code == 400 + assert response.data["detail"] == "Invalid project" + + def test_all_project_slug_sentinel_is_rejected(self) -> None: + release = Release.objects.create(organization_id=self.organization.id, version="abcabcabc") + release.add_project(self.project1) + + self.create_member(teams=[self.team1], user=self.user1, organization=self.organization) + self.login_as(user=self.user1) + + url = reverse( + "sentry-api-0-organization-release-details", + kwargs={"organization_id_or_slug": self.organization.slug, "version": release.version}, + ) + + response = self.client.get(url, {"project": "$all"}) + assert response.status_code == 400 + assert response.data["detail"] == "Invalid project" + def test_correct_project_contains_current_project_meta(self) -> None: """ Test that shows when correct project id is passed to the request, `sessionsLowerBound`, @@ -276,6 +314,48 @@ def test_get_prev_and_next_release_to_current_release_on_date_sort(self) -> None assert response.data["currentProjectMeta"]["nextReleaseVersion"] == "foobar@2.0.0" assert response.data["currentProjectMeta"]["prevReleaseVersion"] is None + def test_project_id_scopes_adjacent_releases(self) -> None: + other_project = self.create_project(teams=[self.team1], organization=self.organization) + now = datetime.now(UTC) + + other_newer_release = Release.objects.create( + date_added=now, + organization_id=self.organization.id, + version="other@2.0.0", + ) + other_newer_release.add_project(other_project) + + current_release = Release.objects.create( + date_added=now - timedelta(days=1), + organization_id=self.organization.id, + version="project@2.0.0", + ) + current_release.add_project(self.project1) + + previous_release = Release.objects.create( + date_added=now - timedelta(days=2), + organization_id=self.organization.id, + version="project@1.0.0", + ) + previous_release.add_project(self.project1) + + self.create_member(teams=[self.team1], user=self.user1, organization=self.organization) + self.login_as(user=self.user1) + + url = reverse( + "sentry-api-0-organization-release-details", + kwargs={ + "organization_id_or_slug": self.organization.slug, + "version": current_release.version, + }, + ) + + response = self.client.get(url, {"project_id": self.project1.id}) + + assert response.status_code == 200 + assert response.data["currentProjectMeta"]["nextReleaseVersion"] is None + assert response.data["currentProjectMeta"]["prevReleaseVersion"] == previous_release.version + def test_get_prev_and_next_release_to_current_release_on_date_sort_with_same_date(self) -> None: """ Test that ensures that in the case we are trying to get prev and next release to a current