diff --git a/src/sentry/api/serializers/rest_framework/dashboard.py b/src/sentry/api/serializers/rest_framework/dashboard.py index f9aae095b48a..041a3a4b2b4c 100644 --- a/src/sentry/api/serializers/rest_framework/dashboard.py +++ b/src/sentry/api/serializers/rest_framework/dashboard.py @@ -13,7 +13,7 @@ from sentry import features, options from sentry.api.serializers.rest_framework import CamelSnakeSerializer from sentry.api.serializers.rest_framework.base import convert_dict_key_case, snake_to_camel_case -from sentry.constants import ALL_ACCESS_PROJECTS +from sentry.constants import ALL_ACCESS_PROJECTS, ObjectStatus from sentry.discover.arithmetic import ArithmeticError, categorize_columns from sentry.exceptions import InvalidSearchQuery from sentry.issues.issue_search import parse_search_query @@ -30,6 +30,7 @@ DatasetSourcesTypes, get_max_widget_limit, ) +from sentry.models.project import Project from sentry.models.team import Team from sentry.relay.config.metric_extraction import get_current_widget_specs, widget_exceeds_max_specs from sentry.search.eap.trace_metrics.validator import extract_trace_metric_from_aggregate @@ -44,6 +45,7 @@ set_or_create_on_demand_state, ) from sentry.utils.dates import parse_stats_period +from sentry.utils.snuba import UnqualifiedQueryError from sentry.utils.strings import oxfordize_list from sentry.utils.tracing import set_span_data, start_span @@ -51,6 +53,8 @@ AGGREGATE_BASE = r".*(\w+)\((.*)?\)" EQUATION_PREFIX = "equation|" +_FALLBACK_PROJECT_IDS_KEY = "_validation_fallback_project_ids" + OnDemandExtractionState = DashboardWidgetQueryOnDemand.OnDemandExtractionState DATASET_SOURCE_MAP = {source[1]: source[0] for source in DatasetSourcesTypes.as_choices()} @@ -285,7 +289,7 @@ def validate(self, data): params: ParamsType = { "start": datetime.now() - timedelta(days=1), "end": datetime.now(), - "project_id": [p.id for p in self.context["projects"]], + "project_id": self._get_validation_project_ids(), "organization_id": self.context["organization"].id, "environment": self.context.get("environment", []), } @@ -333,6 +337,13 @@ def validate(self, data): except InvalidSearchQuery as err: data["discover_query_error"] = {"conditions": [f"Invalid conditions: {err}"]} return data + except UnqualifiedQueryError: + # Fixed message: sibling SnubaError types carry Snuba/ClickHouse + # internals that must not reach API consumers. + data["discover_query_error"] = { + "conditions": ["Could not validate query: no project available."] + } + return data # TODO(dam): Add validation for metrics fields/queries try: @@ -351,6 +362,36 @@ def validate(self, data): return data + def _get_validation_project_ids(self) -> list[int]: + """Project ids used only to satisfy the query builder during validation. + + Which projects are in scope doesn't affect the outcome, so any active + one in the organization will do when the context list is empty. That + happens routinely: `get_projects` filters on team membership, while + `allow_joinleave` orgs grant project access without it, so any teamless + member of an open-membership org lands here despite seeing everything. + Not an access check — no query runs against the borrowed project. + """ + project_ids = [p.id for p in self.context["projects"]] + if project_ids: + return project_ids + + # Cached in the (request-scoped, tree-wide) serializer context because + # this runs once per widget query, up to MAX_WIDGETS times per request. + if _FALLBACK_PROJECT_IDS_KEY not in self.context: + fallback_id = ( + Project.objects.filter( + organization_id=self.context["organization"].id, status=ObjectStatus.ACTIVE + ) + .values_list("id", flat=True) + .first() + ) + self.context[_FALLBACK_PROJECT_IDS_KEY] = ( + [fallback_id] if fallback_id is not None else [] + ) + + return self.context[_FALLBACK_PROJECT_IDS_KEY] + def _get_attr(self, data, attr, empty_value=None): value = data.get(attr) if value is not None: diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py index 5cb2f709936a..6fdae266eb76 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py @@ -4,6 +4,8 @@ from typing import Any from unittest.mock import patch +from django.db import connection +from django.test.utils import CaptureQueriesContext from django.urls import reverse from sentry.dashboards.endpoints.organization_dashboards import ( @@ -2666,3 +2668,138 @@ def test_post_validate_only_error_for_invalid_dashboard(self) -> None: assert not Dashboard.objects.filter( organization=self.organization, title="Invalid Dashboard" ).exists() + + # resolve_params only raises on an empty project list outside the test + # environment, so these patch in_test_environment to reach that path. + + def _single_widget_dashboard(self, title: str) -> dict[str, Any]: + return { + "title": title, + "widgets": [ + { + "displayType": "line", + "interval": "5m", + "title": "Transaction count()", + "queries": [ + { + "name": "Transactions", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "event.type:transaction", + } + ], + }, + ], + } + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_succeeds_for_user_without_team_membership( + self, mock_in_test_environment + ) -> None: + """get_projects filters by team membership, so this resolves to [].""" + assert self.project # the fixture is lazy; the org needs a project + teamless_user = self.create_user() + self.create_member( + organization=self.organization, user=teamless_user, role="member", teams=[] + ) + self.login_as(teamless_user) + + response = self.do_request( + "post", self.url + "?validateOnly=1", data=self._single_widget_dashboard("Teamless") + ) + assert response.status_code == 200, response.data + assert not Dashboard.objects.filter( + organization=self.organization, title="Teamless" + ).exists() + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_creates_dashboard_for_teamless_member_of_open_membership_org( + self, mock_in_test_environment + ) -> None: + """allow_joinleave grants project access without team membership. + + These users resolve to an empty list but can see every project, so + creation must still work — not just validation. + """ + org = self.create_organization(owner=self.user, flags=1) # allow_joinleave + team = self.create_team(organization=org) + self.create_project(organization=org, teams=[team]) + member = self.create_user() + self.create_member(organization=org, user=member, role="member", teams=[]) + self.login_as(member) + + url = reverse( + "sentry-api-0-organization-dashboards", + kwargs={"organization_id_or_slug": org.slug}, + ) + response = self.do_request("post", url, data=self._single_widget_dashboard("Open Org")) + assert response.status_code == 201, response.data + assert Dashboard.objects.filter(organization=org, title="Open Org").exists() + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_looks_up_fallback_project_once( + self, mock_in_test_environment + ) -> None: + """The fallback runs per widget query, so its lookup must be cached.""" + assert self.project # the fixture is lazy; the org needs a project + teamless_user = self.create_user() + self.create_member( + organization=self.organization, user=teamless_user, role="member", teams=[] + ) + self.login_as(teamless_user) + + data = { + "title": "Many Widgets", + "widgets": [ + { + "displayType": "line", + "interval": "5m", + "title": f"Widget {i}", + "queries": [ + { + "name": f"q{j}", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "", + } + for j in range(2) + ], + } + for i in range(5) + ], + } + + with CaptureQueriesContext(connection) as queries: + response = self.do_request("post", self.url + "?validateOnly=1", data=data) + assert response.status_code == 200, response.data + + fallback_lookups = [ + q["sql"] + for q in queries.captured_queries + if 'FROM "sentry_project"' in q["sql"] and "LIMIT 1" in q["sql"] + ] + assert len(fallback_lookups) == 1, fallback_lookups + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_does_not_error_for_organization_without_projects( + self, mock_in_test_environment + ) -> None: + """With no projects there is no fallback, so this hits the backstop.""" + empty_org = self.create_organization(owner=self.user) + url = reverse( + "sentry-api-0-organization-dashboards", + kwargs={"organization_id_or_slug": empty_org.slug}, + ) + self.login_as(self.user) + + response = self.do_request( + "post", url + "?validateOnly=1", data=self._single_widget_dashboard("No Projects") + ) + assert response.status_code == 400, response.data + # Exact match: the exception text must not be echoed back. + assert response.data["widgets"][0]["queries"][0]["conditions"] == [ + "Could not validate query: no project available." + ] + assert not Dashboard.objects.filter(organization=empty_org, title="No Projects").exists()