Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 89 additions & 34 deletions src/sentry/api/bases/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

import sentry_sdk
from django.core.cache import cache
from django.http.request import HttpRequest
from django.db.models import Q
from django.http.request import HttpRequest, QueryDict
from rest_framework.exceptions import ParseError, PermissionDenied
from rest_framework.permissions import BasePermission
from rest_framework.request import Request
Expand All @@ -15,11 +16,12 @@
from sentry.api.base import Endpoint
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.api.helpers.environments import get_environments
from sentry.api.helpers.projects import ParsedProjectIdOrSlugParams, parse_id_or_slug_params
from sentry.api.permissions import DemoSafePermission, StaffPermissionMixin
from sentry.api.utils import get_date_range_from_params, is_member_disabled_from_limit
from sentry.auth.staff import is_active_staff
from sentry.auth.superuser import is_active_superuser
from sentry.constants import ALL_ACCESS_PROJECT_ID, ALL_ACCESS_PROJECTS_SLUG, ObjectStatus
from sentry.constants import ObjectStatus
from sentry.exceptions import InvalidParams
from sentry.models.apikey import is_api_key_auth
from sentry.models.apitoken import is_api_token_auth
Expand Down Expand Up @@ -349,8 +351,11 @@ def _validate_fetched_projects(
"""
Validates that user has access to the specific projects they are requesting.
"""
missing_project_ids = ids and ids != {p.id for p in filtered_projects}
missing_project_slugs = slugs and slugs != {p.slug for p in filtered_projects}
fetched_ids = {p.id for p in filtered_projects}
fetched_slugs = {p.slug for p in filtered_projects}

missing_project_ids = ids and not ids.issubset(fetched_ids)
missing_project_slugs = slugs and not slugs.issubset(fetched_slugs)

if missing_project_ids or missing_project_slugs:
raise PermissionDenied
Expand Down Expand Up @@ -389,32 +394,30 @@ 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: If both project_ids and project_slugs are passed, we will default
to fetching projects via project_id list.
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")

slugs = project_slugs or set(filter(None, request.GET.getlist("projectSlug")))
ids = project_ids or self.get_requested_project_ids_unchecked(request)

if project_ids is None and slugs:
# If we're querying for project slugs specifically
if ALL_ACCESS_PROJECTS_SLUG in slugs:
# All projects I have access to
include_all_accessible = True
else:
qs = qs.filter(slug__in=slugs)
if project_ids:
requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set())
Comment thread
cursor[bot] marked this conversation as resolved.
elif project_slugs:
requested_projects = ParsedProjectIdOrSlugParams(ids=set(), slugs=set(project_slugs))
else:
# If we are explicitly querying for projects via id
# Or we're querying for an empty set of ids
if ALL_ACCESS_PROJECT_ID in ids:
# All projects i have access to
include_all_accessible = True
elif ids:
qs = qs.filter(id__in=ids)
# No project ids === `all projects I am a member of`
requested_projects = self.get_requested_project_params_unchecked(request)
ids = requested_projects.ids
slugs = requested_projects.slugs

if requested_projects.has_all_projects_sentinel:
include_all_accessible = True
elif ids and slugs:
qs = qs.filter(Q(id__in=ids) | Q(slug__in=slugs))
elif slugs:
qs = qs.filter(slug__in=slugs)
elif ids:
qs = qs.filter(id__in=ids)
# No project ids or slugs === `all projects I am a member of`

with sentry_sdk.start_span(op="fetch_organization_projects") as span:
projects = list(qs)
Expand Down Expand Up @@ -474,15 +477,65 @@ def _filter_projects_by_permissions(

def get_requested_project_ids_unchecked(self, request: HttpRequest) -> set[int]:
"""
Returns the project ids that were requested by the request.
Returns the numeric project ids that were requested by the request.
Non-numeric values (slugs) are silently ignored.

To determine the projects to filter this endpoint by with full
permission checking, use ``get_projects``, instead.
"""
try:
return set(map(int, request.GET.getlist("project")))
except ValueError:
raise ParseError(detail="Invalid project parameter. Values must be numbers.")
return self.get_requested_project_ids_and_slugs_unchecked(request).ids
Comment thread
cursor[bot] marked this conversation as resolved.

def get_requested_project_ids_and_slugs_unchecked(
self, request: HttpRequest
) -> ParsedProjectIdOrSlugParams:
"""
Returns the project ids and slugs requested via the project query param.

To determine the projects to filter this endpoint by with full
permission checking, use ``get_projects``, instead.
"""
return parse_id_or_slug_params(request.GET.getlist("project"))

def get_requested_project_params_unchecked(
self, request: HttpRequest
) -> ParsedProjectIdOrSlugParams:
"""
Returns requested project ids and slugs from projectSlug or project query params.

To determine the projects to filter this endpoint by with full
permission checking, use ``get_projects``, instead.
"""
project_slugs = set(filter(None, request.GET.getlist("projectSlug")))
if project_slugs:
return ParsedProjectIdOrSlugParams(ids=set(), slugs=project_slugs)
return self.get_requested_project_ids_and_slugs_unchecked(request)

def get_query_params_without_empty_project_params(self, request: HttpRequest) -> QueryDict:
"""
Return query params with blank project filters treated as absent.
"""
query_params = request.GET.copy()
if "project" in query_params:
query_params.setlist(
"project", [project for project in query_params.getlist("project") if project]
)
return query_params

def get_query_params_with_project_slug_precedence(self, request: HttpRequest) -> QueryDict:
"""
Return query params for serializers that accept project and projectSlug.

This mirrors get_projects() precedence: non-empty legacy projectSlug
filters win over project filters, and blank project filters are treated
as absent.
"""
query_params = self.get_query_params_without_empty_project_params(request)
project_slug_params = [slug for slug in query_params.getlist("projectSlug") if slug]
if "projectSlug" in query_params:
query_params.setlist("projectSlug", project_slug_params)
if project_slug_params:
query_params.pop("project", None)
return query_params

def get_environments(
self, request: Request, organization: Organization | RpcOrganization
Expand Down Expand Up @@ -723,7 +776,7 @@ def has_release_permission(
they cannot access. The all-projects check respects Open Membership
via has_global_access.

Results are cached for 60s per actor/org/release/project-ids/mode.
Results are cached for 60s per actor/org/release/effective-project-scope/mode.
"""
actor_id = None
has_perms = None
Expand All @@ -733,17 +786,19 @@ def has_release_permission(
elif request.auth is not None:
actor_id = "apikey:%s" % request.auth.entity_id
if actor_id is not None:
requested_project_ids = project_ids
if requested_project_ids is None:
requested_project_ids = self.get_requested_project_ids_unchecked(request)
if project_ids:
requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set())
else:
requested_projects = self.get_requested_project_params_unchecked(request)
key = "release_perms:1:%s" % hash_values(
[
actor_id,
organization.id,
release.id if release is not None else 0,
int(require_all_projects),
]
+ sorted(requested_project_ids)
+ sorted(requested_projects.ids)
+ sorted(requested_projects.slugs)
)
has_perms = cache.get(key)
if has_perms is None:
Expand Down
8 changes: 1 addition & 7 deletions src/sentry/api/endpoints/organization_stats_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from sentry.apidocs.examples.organization_examples import OrganizationExamples
from sentry.apidocs.parameters import GlobalParams
from sentry.apidocs.utils import inline_sentry_response_serializer
from sentry.constants import ALL_ACCESS_PROJECTS
from sentry.exceptions import InvalidParams
from sentry.models.organization import Organization
from sentry.models.project import Project
Expand Down Expand Up @@ -188,19 +187,14 @@ def build_outcomes_query(self, request: Request, organization):
return QueryDefinition.from_query_dict(query_dict, params)

def _get_projects_for_orgstats_query(self, request: Request, organization):
req_proj_ids = self.get_requested_project_ids_unchecked(request)

# the projects table always filters by project
# the projects in the table should be those the user has access to

projects = self.get_projects(request, organization, project_ids=req_proj_ids)
projects = self.get_projects(request, organization)
if not projects:
raise NoProjects("No projects available")
return [p.id for p in projects]

def _is_org_total_query(self, project_ids):
return all([not project_ids or project_ids == ALL_ACCESS_PROJECTS])

def _generate_csv(self, projects):
if not len(projects):
return
Expand Down
21 changes: 11 additions & 10 deletions src/sentry/api/endpoints/organization_stats_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from sentry.apidocs.examples.organization_examples import OrganizationExamples
from sentry.apidocs.parameters import GlobalParams
from sentry.apidocs.utils import inline_sentry_response_serializer
from sentry.constants import ALL_ACCESS_PROJECTS
from sentry.constants import ALL_ACCESS_PROJECTS, ALL_ACCESS_PROJECTS_SLUG
from sentry.exceptions import InvalidParams
from sentry.models.organization import Organization
from sentry.ratelimits.config import RateLimitConfig
Expand Down Expand Up @@ -205,21 +205,22 @@ def _get_projects_for_orgstats_query(self, request: Request, organization):
# look at the raw project_id filter passed in, if its empty
# and project_id is not in groupBy filter, treat it as an
# org wide query and don't pass project_id in to QueryDefinition
req_proj_ids = self.get_requested_project_ids_unchecked(request)
if self._is_org_total_query(request, req_proj_ids):
requested_projects = self.get_requested_project_params_unchecked(request)
if self._is_org_total_query(request, requested_projects):
return None
else:
projects = self.get_projects(request, organization, project_ids=req_proj_ids)
projects = self.get_projects(request, organization)
if not projects:
raise NoProjects("No projects available")
return [p.id for p in projects]

def _is_org_total_query(self, request: Request, project_ids):
return all(
[
not project_ids or project_ids == ALL_ACCESS_PROJECTS,
"project" not in request.GET.get("groupBy", []),
]
def _is_org_total_query(self, request: Request, requested_projects):
no_project_filter = not requested_projects.has_values
all_access_filter = (
requested_projects.ids == ALL_ACCESS_PROJECTS and not requested_projects.slugs
) or (requested_projects.slugs == {ALL_ACCESS_PROJECTS_SLUG} and not requested_projects.ids)
return (no_project_filter or all_access_filter) and "project" not in request.GET.getlist(
"groupBy"
)

@contextmanager
Expand Down
5 changes: 3 additions & 2 deletions src/sentry/api/helpers/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from collections.abc import Iterable
from typing import NamedTuple, TypeAlias

from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import extend_schema_field
from rest_framework import serializers

Expand All @@ -12,6 +11,8 @@

ProjectIdOrSlug: TypeAlias = int | str

PROJECT_ID_OR_SLUG_SCHEMA = {"anyOf": [{"type": "integer"}, {"type": "string"}]}


class ParsedProjectIdOrSlugParams(NamedTuple):
ids: set[int]
Expand Down Expand Up @@ -52,7 +53,7 @@ def parse_id_or_slug_params(
return ParsedProjectIdOrSlugParams(ids=ids, slugs=slugs)


@extend_schema_field(field=OpenApiTypes.STR)
@extend_schema_field(field=PROJECT_ID_OR_SLUG_SCHEMA)
class ProjectIdOrSlugField(serializers.Field[ProjectIdOrSlug, object, ProjectIdOrSlug, object]):
default_error_messages = {
"invalid": "Expected a project ID or slug.",
Expand Down
21 changes: 17 additions & 4 deletions src/sentry/apidocs/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from drf_spectacular.utils import OpenApiParameter

from sentry import constants
from sentry.api.helpers.projects import PROJECT_ID_OR_SLUG_SCHEMA
from sentry.snuba.dataset import Dataset
from sentry.snuba.sessions import STATS_PERIODS

Expand Down Expand Up @@ -44,7 +45,7 @@ class GlobalParams:
)
PROJECT_ID_OR_SLUG = OpenApiParameter(
name="project_id_or_slug",
description="The ID or slug of the project the resource belongs to.",
description="The ID or slug of the project the resource belongs to. Project slugs are unique within each organization.",
required=True,
type=str,
location="path",
Expand Down Expand Up @@ -143,7 +144,18 @@ class OrganizationParams:
required=False,
many=True,
type=str,
description="""The project slugs to filter by. Use `$all` to include all available projects. For example, the following are valid parameters:
description="""The legacy project slug filter. Prefer `project`, which accepts project IDs or slugs. Use `$all` to include all available projects. For example, the following are valid parameters:
- `/?projectSlug=$all`
- `/?projectSlug=android&projectSlug=javascript-react`
""",
)
PROJECT_SLUG = OpenApiParameter(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're intending to drop this it we can probably just remove this from the docs

name="projectSlug",
location="query",
required=False,
many=True,
type=str,
description="""The project slugs to filter by. This legacy parameter takes precedence over `project` if both are provided. Prefer `project`, which accepts project IDs or slugs. Use `$all` to include all available projects. For example, the following are valid parameters:
- `/?projectSlug=$all`
- `/?projectSlug=android&projectSlug=javascript-react`
""",
Expand All @@ -153,10 +165,11 @@ class OrganizationParams:
location="query",
required=False,
many=True,
type=int,
description="""The IDs of projects to filter by. `-1` means all available projects.
type=PROJECT_ID_OR_SLUG_SCHEMA,
description="""The IDs or slugs of projects to filter by. Project slugs are unique within each organization. Omit this parameter to include all accessible projects. `-1` is also accepted to include all accessible projects.
For example, the following are valid parameters:
- `/?project=1234&project=56789`
- `/?project=android&project=javascript-react`
- `/?project=-1`
""",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
from sentry.middleware import is_frontend_request
from sentry.models.groupopenperiod import GroupOpenPeriod
from sentry.models.organization import Organization
from sentry.models.organizationmemberteam import OrganizationMemberTeam
from sentry.models.project import Project
from sentry.models.team import Team
from sentry.monitors.models import (
Expand Down Expand Up @@ -525,31 +524,10 @@ def get(self, request: Request, organization: Organization) -> Response:
"""
Fetches metric, issue, crons, and uptime alert rules for an organization
"""
# Common setup: project resolution
project_ids = self.get_requested_project_ids_unchecked(request) or None
if project_ids == {-1}: # All projects for org:
project_ids = set(
Project.objects.filter(
organization=organization, status=ObjectStatus.ACTIVE
).values_list("id", flat=True)
)
elif project_ids is None: # All projects for user
org_team_list = Team.objects.filter(organization=organization).values_list(
"id", flat=True
)
user_team_list = OrganizationMemberTeam.objects.filter(
organizationmember__user_id=request.user.id, team__in=org_team_list
).values_list("team", flat=True)
project_ids = set(
Project.objects.filter(
teams__in=user_team_list, status=ObjectStatus.ACTIVE
).values_list("id", flat=True)
)

# Materialize the project ids here. This helps us to not overwhelm the query planner with
# overcomplicated subqueries. Previously, this was causing Postgres to use a suboptimal
# index to filter on. Also enforces permission checks.
projects = self.get_projects(request, organization, project_ids=project_ids)
projects = self.get_projects(request, organization)

# Common setup: team parsing
teams = request.GET.getlist("team", [])
Expand Down
Loading
Loading