Browse Source

fix(discover) Allow users to view events in non-member projects (#15292)

If a user gets a link to an event they should be able to see it if they
have access to the project.

Add permissions check for project access in event_details. This 
endpoint wasn't previously checking project access properly.

Fixes SENTRY-CX0
Mark Story 5 years ago
parent
commit
60256231ce

+ 1 - 1
src/sentry/api/bases/organization_events.py

@@ -54,7 +54,7 @@ class OrganizationEventsEndpointBase(OrganizationEndpoint):
         reference_event_id = request.GET.get("referenceEvent")
         reference_event_id = request.GET.get("referenceEvent")
         if reference_event_id:
         if reference_event_id:
             snuba_args["conditions"] = get_reference_event_conditions(
             snuba_args["conditions"] = get_reference_event_conditions(
-                snuba_args, reference_event_id
+                organization, snuba_args, reference_event_id
             )
             )
 
 
         return snuba_args
         return snuba_args

+ 11 - 3
src/sentry/api/endpoints/organization_event_details.py

@@ -5,7 +5,7 @@ from rest_framework.response import Response
 from sentry.api.bases import OrganizationEventsEndpointBase, OrganizationEventsError, NoProjects
 from sentry.api.bases import OrganizationEventsEndpointBase, OrganizationEventsError, NoProjects
 from sentry.api.event_search import get_reference_event_conditions
 from sentry.api.event_search import get_reference_event_conditions
 from sentry import eventstore, features
 from sentry import eventstore, features
-from sentry.models.project import Project
+from sentry.models.project import Project, ProjectStatus
 from sentry.api.serializers import serialize
 from sentry.api.serializers import serialize
 
 
 
 
@@ -23,9 +23,15 @@ class OrganizationEventDetailsEndpoint(OrganizationEventsEndpointBase):
             return Response(status=404)
             return Response(status=404)
 
 
         try:
         try:
-            project = Project.objects.get(slug=project_slug, organization_id=organization.id)
+            project = Project.objects.get(
+                slug=project_slug, organization_id=organization.id, status=ProjectStatus.VISIBLE
+            )
         except Project.DoesNotExist:
         except Project.DoesNotExist:
             return Response(status=404)
             return Response(status=404)
+        # Check access to the project as this endpoint doesn't use membership checks done
+        # get_filter_params().
+        if not request.access.has_project_access(project):
+            return Response(status=404)
 
 
         # We return the requested event if we find a match regardless of whether
         # We return the requested event if we find a match regardless of whether
         # it occurred within the range specified
         # it occurred within the range specified
@@ -38,7 +44,9 @@ class OrganizationEventDetailsEndpoint(OrganizationEventsEndpointBase):
         # This ensure that if a field list/groupby conditions were provided
         # This ensure that if a field list/groupby conditions were provided
         # that we constrain related events to the query + current event values
         # that we constrain related events to the query + current event values
         event_slug = u"{}:{}".format(project.slug, event_id)
         event_slug = u"{}:{}".format(project.slug, event_id)
-        snuba_args["conditions"].extend(get_reference_event_conditions(snuba_args, event_slug))
+        snuba_args["conditions"].extend(
+            get_reference_event_conditions(organization, snuba_args, event_slug)
+        )
 
 
         data = serialize(event)
         data = serialize(event)
         data["nextEventID"] = self.next_event_id(snuba_args, event)
         data["nextEventID"] = self.next_event_id(snuba_args, event)

+ 5 - 5
src/sentry/api/event_search.py

@@ -13,7 +13,7 @@ from parsimonious.nodes import Node
 from parsimonious.grammar import Grammar, NodeVisitor
 from parsimonious.grammar import Grammar, NodeVisitor
 
 
 from sentry import eventstore
 from sentry import eventstore
-from sentry.models import Project
+from sentry.models import Project, ProjectStatus
 from sentry.search.utils import (
 from sentry.search.utils import (
     parse_datetime_range,
     parse_datetime_range,
     parse_datetime_string,
     parse_datetime_string,
@@ -813,14 +813,14 @@ def resolve_field_list(fields, snuba_args):
     }
     }
 
 
 
 
-def find_reference_event(snuba_args, reference_event_slug, fields):
+def find_reference_event(organization, snuba_args, reference_event_slug, fields):
     try:
     try:
         project_slug, event_id = reference_event_slug.split(":")
         project_slug, event_id = reference_event_slug.split(":")
     except ValueError:
     except ValueError:
         raise InvalidSearchQuery("Invalid reference event")
         raise InvalidSearchQuery("Invalid reference event")
     try:
     try:
         project = Project.objects.get(
         project = Project.objects.get(
-            slug=project_slug, id__in=snuba_args["filter_keys"]["project_id"]
+            slug=project_slug, organization=organization, status=ProjectStatus.VISIBLE
         )
         )
     except Project.DoesNotExist:
     except Project.DoesNotExist:
         raise InvalidSearchQuery("Invalid reference event")
         raise InvalidSearchQuery("Invalid reference event")
@@ -834,7 +834,7 @@ def find_reference_event(snuba_args, reference_event_slug, fields):
 TAG_KEY_RE = re.compile(r"^tags\[(.*)\]$")
 TAG_KEY_RE = re.compile(r"^tags\[(.*)\]$")
 
 
 
 
-def get_reference_event_conditions(snuba_args, event_slug):
+def get_reference_event_conditions(organization, snuba_args, event_slug):
     """
     """
     Returns a list of additional conditions/filter_keys to
     Returns a list of additional conditions/filter_keys to
     scope a query by the groupby fields using values from the reference event
     scope a query by the groupby fields using values from the reference event
@@ -848,7 +848,7 @@ def get_reference_event_conditions(snuba_args, event_slug):
 
 
     # Fetch the reference event ensuring the fields in the groupby
     # Fetch the reference event ensuring the fields in the groupby
     # clause are present.
     # clause are present.
-    event_data = find_reference_event(snuba_args, event_slug, columns)
+    event_data = find_reference_event(organization, snuba_args, event_slug, columns)
 
 
     conditions = []
     conditions = []
     tags = {}
     tags = {}

+ 15 - 13
tests/sentry/api/test_event_search.py

@@ -1194,7 +1194,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
 
 
     def test_bad_slug_format(self):
     def test_bad_slug_format(self):
         with pytest.raises(InvalidSearchQuery):
         with pytest.raises(InvalidSearchQuery):
-            get_reference_event_conditions(self.conditions, "lol")
+            get_reference_event_conditions(self.organization, self.conditions, "lol")
 
 
     def test_unknown_project(self):
     def test_unknown_project(self):
         event = self.store_event(
         event = self.store_event(
@@ -1203,12 +1203,14 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["filter_keys"]["project_id"] = [-1]
         self.conditions["filter_keys"]["project_id"] = [-1]
         with pytest.raises(InvalidSearchQuery):
         with pytest.raises(InvalidSearchQuery):
-            get_reference_event_conditions(self.conditions, "nope:{}".format(event.event_id))
+            get_reference_event_conditions(
+                self.organization, self.conditions, "nope:{}".format(event.event_id)
+            )
 
 
     def test_unknown_event(self):
     def test_unknown_event(self):
         with pytest.raises(InvalidSearchQuery):
         with pytest.raises(InvalidSearchQuery):
             slug = "{}:deadbeef".format(self.project.slug)
             slug = "{}:deadbeef".format(self.project.slug)
-            get_reference_event_conditions(self.conditions, slug)
+            get_reference_event_conditions(self.organization, self.conditions, slug)
 
 
     def test_no_fields(self):
     def test_no_fields(self):
         event = self.store_event(
         event = self.store_event(
@@ -1221,7 +1223,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = []
         self.conditions["groupby"] = []
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert len(result) == 0
         assert len(result) == 0
 
 
     def test_basic_fields(self):
     def test_basic_fields(self):
@@ -1235,7 +1237,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["message", "transaction", "unknown-field"]
         self.conditions["groupby"] = ["message", "transaction", "unknown-field"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [
         assert result == [
             ["message", "=", "oh no! /issues/{issue_id}"],
             ["message", "=", "oh no! /issues/{issue_id}"],
             ["transaction", "=", "/issues/{issue_id}"],
             ["transaction", "=", "/issues/{issue_id}"],
@@ -1256,7 +1258,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["geo.city", "geo.region", "geo.country_code"]
         self.conditions["groupby"] = ["geo.city", "geo.region", "geo.country_code"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [
         assert result == [
             ["geo.city", "=", "San Francisco"],
             ["geo.city", "=", "San Francisco"],
             ["geo.region", "=", "CA"],
             ["geo.region", "=", "CA"],
@@ -1275,7 +1277,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["sdk.version", "sdk.name"]
         self.conditions["groupby"] = ["sdk.version", "sdk.name"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [["sdk.version", "=", "5.0.12"], ["sdk.name", "=", "sentry-python"]]
         assert result == [["sdk.version", "=", "5.0.12"], ["sdk.name", "=", "sentry-python"]]
 
 
     def test_error_field(self):
     def test_error_field(self):
@@ -1284,7 +1286,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         event = self.store_event(data=data, project_id=self.project.id)
         event = self.store_event(data=data, project_id=self.project.id)
         self.conditions["groupby"] = ["error.value", "error.type", "error.handled"]
         self.conditions["groupby"] = ["error.value", "error.type", "error.handled"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [
         assert result == [
             ["error.value", "=", "This is a test exception sent from the Raven CLI."],
             ["error.value", "=", "This is a test exception sent from the Raven CLI."],
             ["error.type", "=", "Exception"],
             ["error.type", "=", "Exception"],
@@ -1296,7 +1298,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         event = self.store_event(data=data, project_id=self.project.id)
         event = self.store_event(data=data, project_id=self.project.id)
         self.conditions["groupby"] = ["stack.filename", "stack.function"]
         self.conditions["groupby"] = ["stack.filename", "stack.function"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [
         assert result == [
             ["stack.filename", "=", "/Users/example/Development/raven-php/bin/raven"],
             ["stack.filename", "=", "/Users/example/Development/raven-php/bin/raven"],
             ["stack.function", "=", "raven_cli_test"],
             ["stack.function", "=", "raven_cli_test"],
@@ -1313,7 +1315,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["nope", "color", "customer_id"]
         self.conditions["groupby"] = ["nope", "color", "customer_id"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [["color", "=", "red"], ["customer_id", "=", "1"]]
         assert result == [["color", "=", "red"], ["customer_id", "=", "1"]]
 
 
     def test_context_value(self):
     def test_context_value(self):
@@ -1331,7 +1333,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["gpu.name", "browser.name"]
         self.conditions["groupby"] = ["gpu.name", "browser.name"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [
         assert result == [
             ["gpu.name", "=", "nvidia 8600"],
             ["gpu.name", "=", "nvidia 8600"],
             ["browser.name", "=", "Firefox"],
             ["browser.name", "=", "Firefox"],
@@ -1352,7 +1354,7 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         )
         )
         self.conditions["groupby"] = ["issue.id"]
         self.conditions["groupby"] = ["issue.id"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [["issue.id", "=", event.group_id]]
         assert result == [["issue.id", "=", event.group_id]]
 
 
     @pytest.mark.xfail(reason="This requires eventstore.get_event_by_id to work with transactions")
     @pytest.mark.xfail(reason="This requires eventstore.get_event_by_id to work with transactions")
@@ -1361,5 +1363,5 @@ class GetReferenceEventConditionsTest(SnubaTestCase, TestCase):
         event = self.store_event(data=data, project_id=self.project.id)
         event = self.store_event(data=data, project_id=self.project.id)
         self.conditions["groupby"] = ["transaction.op", "transaction.duration"]
         self.conditions["groupby"] = ["transaction.op", "transaction.duration"]
         slug = "{}:{}".format(self.project.slug, event.event_id)
         slug = "{}:{}".format(self.project.slug, event.event_id)
-        result = get_reference_event_conditions(self.conditions, slug)
+        result = get_reference_event_conditions(self.organization, self.conditions, slug)
         assert result == [["transaction.op", "=", "db"], ["transaction.duration", "=", 2]]
         assert result == [["transaction.op", "=", "db"], ["transaction.duration", "=", 2]]

+ 32 - 1
tests/snuba/api/endpoints/test_organization_event_details.py

@@ -92,7 +92,7 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
             response = self.client.get(url, format="json")
             response = self.client.get(url, format="json")
         assert response.status_code == 200
         assert response.status_code == 200
 
 
-    def test_no_access(self):
+    def test_no_access_missing_feature(self):
         url = reverse(
         url = reverse(
             "sentry-api-0-organization-event-details",
             "sentry-api-0-organization-event-details",
             kwargs={
             kwargs={
@@ -103,7 +103,38 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
         )
         )
 
 
         response = self.client.get(url, format="json")
         response = self.client.get(url, format="json")
+        assert response.status_code == 404, response.content
+
+    def test_access_non_member_project(self):
+        # Add a new user to a project and then access events on project they are not part of.
+        member_user = self.create_user()
+        team = self.create_team(members=[member_user])
+        self.create_project(organization=self.organization, teams=[team])
+
+        # Enable open membership
+        self.organization.flags.allow_joinleave = True
+        self.organization.save()
+
+        self.login_as(member_user)
+
+        url = reverse(
+            "sentry-api-0-organization-event-details",
+            kwargs={
+                "organization_slug": self.organization.slug,
+                "project_slug": self.project.slug,
+                "event_id": "a" * 32,
+            },
+        )
+        with self.feature("organizations:events-v2"):
+            response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+
+        # When open membership is off, access should be denied to non owner users
+        self.organization.flags.allow_joinleave = False
+        self.organization.save()
 
 
+        with self.feature("organizations:events-v2"):
+            response = self.client.get(url, format="json")
         assert response.status_code == 404, response.content
         assert response.status_code == 404, response.content
 
 
     def test_no_event(self):
     def test_no_event(self):