Skip to content

Commit 74dfdaf

Browse files
authored
ref(browser reporting): Use option to control Reporting-Endpoints header (#93560)
This switches our check for whether to add the `Reporting-Endpoints` header from using `staff.is_active` (which seems to be false even if the user has `is_staff = True` in the database) to using a new option, `issues.browser_reporting.reporting_endpoints_header_enabled`.
1 parent 87ac189 commit 74dfdaf

File tree

3 files changed

+52
-60
lines changed

3 files changed

+52
-60
lines changed

src/sentry/middleware/reporting_endpoint.py

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

6+
from sentry import options
7+
68

79
class ReportingEndpointMiddleware:
810
"""
@@ -19,10 +21,14 @@ def __call__(self, request: HttpRequest) -> HttpResponseBase:
1921
def process_response(
2022
self, request: HttpRequest, response: HttpResponseBase
2123
) -> HttpResponseBase:
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
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
2632
# They always report to the default endpoint
2733
response["Reporting-Endpoints"] = (
2834
"default=https://sentry.my.sentry.io/api/0/reporting-api-experiment/"

src/sentry/options/defaults.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3458,6 +3458,15 @@
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+
34613470
# Enable the collection of Reporting API reports via the `/api/0/reporting-api-experiment/`
34623471
# endpoint. When this is false, the endpoint will just 404.
34633472
register(
Lines changed: 33 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,45 @@
1-
from unittest.mock import Mock
1+
from unittest.mock import MagicMock, patch
22

3-
from django.http import HttpResponse, HttpResponseBase
3+
from django.http import HttpResponse
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
89

910

1011
class ReportingEndpointMiddlewareTestCase(TestCase):
1112
def setUp(self) -> None:
1213
self.middleware = ReportingEndpointMiddleware(lambda request: HttpResponse())
1314
self.factory = RequestFactory()
1415

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)
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

0 commit comments

Comments
 (0)