Skip to content

Commit 135b60a

Browse files
getsentry-botasottile-sentry
authored andcommitted
Revert "ref(browser reporting): Use option to control Reporting-Endpoints header (#93560)"
This reverts commit 74dfdaf. Co-authored-by: asottile-sentry <[email protected]>
1 parent eb07cc7 commit 135b60a

File tree

3 files changed

+60
-52
lines changed

3 files changed

+60
-52
lines changed

src/sentry/middleware/reporting_endpoint.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
from django.http.request import HttpRequest
44
from django.http.response import HttpResponseBase
55

6-
from sentry import options
7-
86

97
class ReportingEndpointMiddleware:
108
"""
@@ -21,14 +19,10 @@ def __call__(self, request: HttpRequest) -> HttpResponseBase:
2119
def process_response(
2220
self, request: HttpRequest, response: HttpResponseBase
2321
) -> HttpResponseBase:
24-
# There are some Relay endpoints which need to be able to run without touching the database
25-
# (so the `options` check below is no good), and besides, Relay has no use for a
26-
# browser-specific header, so we need to (but also can) bail early.
27-
if "api/0/relays" in request.path:
28-
return response
29-
30-
if options.get("issues.browser_reporting.reporting_endpoints_header_enabled"):
31-
# This will enable crashes and intervention and deprecation reports
22+
# Check if the request has staff attribute and if staff is active
23+
staff = getattr(request, "staff", None)
24+
if staff and staff.is_active:
25+
# This will enable crashes, intervention and deprecation warnings
3226
# They always report to the default endpoint
3327
response["Reporting-Endpoints"] = (
3428
"default=https://sentry.my.sentry.io/api/0/reporting-api-experiment/"

src/sentry/options/defaults.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3458,15 +3458,6 @@
34583458
flags=FLAG_ALLOW_EMPTY | FLAG_AUTOMATOR_MODIFIABLE,
34593459
)
34603460

3461-
# Enable adding the `Reporting-Endpoints` header, which will in turn enable the sending of Reporting
3462-
# API reports from the browser (as long as it's Chrome).
3463-
register(
3464-
"issues.browser_reporting.reporting_endpoints_header_enabled",
3465-
type=Bool,
3466-
default=False,
3467-
flags=FLAG_AUTOMATOR_MODIFIABLE,
3468-
)
3469-
34703461
# Enable the collection of Reporting API reports via the `/api/0/reporting-api-experiment/`
34713462
# endpoint. When this is false, the endpoint will just 404.
34723463
register(
Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,68 @@
1-
from unittest.mock import MagicMock, patch
1+
from unittest.mock import Mock
22

3-
from django.http import HttpResponse
3+
from django.http import HttpResponse, HttpResponseBase
44
from django.test import RequestFactory
55

66
from sentry.middleware.reporting_endpoint import ReportingEndpointMiddleware
77
from sentry.testutils.cases import TestCase
8-
from sentry.testutils.helpers.options import override_options
98

109

1110
class ReportingEndpointMiddlewareTestCase(TestCase):
1211
def setUp(self) -> None:
1312
self.middleware = ReportingEndpointMiddleware(lambda request: HttpResponse())
1413
self.factory = RequestFactory()
1514

16-
def test_obeys_option(self) -> None:
17-
with override_options(
18-
{"issues.browser_reporting.reporting_endpoints_header_enabled": True}
19-
):
20-
request = self.factory.get("/")
21-
response = self.middleware.process_response(request, HttpResponse())
22-
23-
assert (
24-
response.get("Reporting-Endpoints")
25-
== "default=https://sentry.my.sentry.io/api/0/reporting-api-experiment/"
26-
)
27-
28-
with override_options(
29-
{"issues.browser_reporting.reporting_endpoints_header_enabled": False}
30-
):
31-
request = self.factory.get("/")
32-
response = self.middleware.process_response(request, HttpResponse())
33-
34-
assert response.get("Reporting-Endpoints") is None
35-
36-
@patch("src.sentry.middleware.reporting_endpoint.options.get")
37-
def test_no_options_check_in_relay_endpoints(self, mock_options_get: MagicMock) -> None:
38-
with override_options(
39-
{"issues.browser_reporting.reporting_endpoints_header_enabled": True}
40-
):
41-
request = self.factory.get("/api/0/relays/register/challenge/")
42-
response = self.middleware.process_response(request, HttpResponse())
43-
44-
mock_options_get.assert_not_called()
45-
assert response.get("Reporting-Endpoints") is None
15+
def _no_header_set(self, result: HttpResponseBase) -> None:
16+
assert "Reporting-Endpoints" not in result
17+
18+
def test_adds_header_for_staff_user(self) -> None:
19+
"""Test that the ReportingEndpoint header is added when user is Sentry staff."""
20+
request = self.factory.get("/")
21+
22+
# Mock staff object with is_active = True
23+
staff_mock = Mock()
24+
staff_mock.is_active = True
25+
setattr(request, "staff", staff_mock)
26+
27+
response = HttpResponse()
28+
result = self.middleware.process_response(request, response)
29+
30+
assert "Reporting-Endpoints" in result
31+
assert (
32+
result["Reporting-Endpoints"]
33+
== "default=https://sentry.my.sentry.io/api/0/reporting-api-experiment/"
34+
)
35+
36+
def test_no_header_for_non_staff_user(self) -> None:
37+
"""Test that the ReportingEndpoint header is not added when user is not Sentry staff."""
38+
request = self.factory.get("/")
39+
40+
# Mock staff object with is_active = False
41+
staff_mock = Mock()
42+
staff_mock.is_active = False
43+
setattr(request, "staff", staff_mock)
44+
45+
response = HttpResponse()
46+
result = self.middleware.process_response(request, response)
47+
48+
self._no_header_set(result)
49+
50+
def test_no_header_when_no_staff_attribute(self) -> None:
51+
"""Test that the ReportingEndpoint header is not added when request has no staff attribute."""
52+
request = self.factory.get("/")
53+
54+
# No staff attribute on request
55+
response = HttpResponse()
56+
result = self.middleware.process_response(request, response)
57+
58+
self._no_header_set(result)
59+
60+
def test_no_header_when_staff_is_none(self) -> None:
61+
"""Test that the ReportingEndpoint header is not added when staff is None."""
62+
request = self.factory.get("/")
63+
setattr(request, "staff", None)
64+
65+
response = HttpResponse()
66+
result = self.middleware.process_response(request, response)
67+
68+
self._no_header_set(result)

0 commit comments

Comments
 (0)