Skip to content

Commit 4490518

Browse files
authored
ref(feedback): return 400 for failed validation in userreport endpoint (#90032)
Follow up to #89999
1 parent 268a19a commit 4490518

File tree

2 files changed

+53
-4
lines changed

2 files changed

+53
-4
lines changed

src/sentry/ingest/userreport.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
import uuid
66
from datetime import datetime, timedelta
77

8-
from django.core.exceptions import ValidationError
8+
from django.core.exceptions import PermissionDenied, ValidationError
99
from django.core.validators import validate_email
1010
from django.db import IntegrityError, router
1111
from django.utils import timezone
1212

1313
from sentry import eventstore, options
14+
from sentry.api.exceptions import BadRequest
1415
from sentry.constants import DataCategory
1516
from sentry.eventstore.models import Event, GroupEvent
1617
from sentry.feedback.lib.types import UserReportDict
@@ -61,9 +62,14 @@ def save_userreport(
6162
category=DataCategory.USER_REPORT_V2,
6263
quantity=1,
6364
)
65+
if (
66+
source == FeedbackCreationSource.USER_REPORT_DJANGO_ENDPOINT
67+
or source == FeedbackCreationSource.CRASH_REPORT_EMBED_FORM
68+
):
69+
raise PermissionDenied()
6470
return None
6571

66-
should_filter, metrics_reason, outcomes_reason = validate_user_report(
72+
should_filter, metrics_reason, display_reason = validate_user_report(
6773
report, project.id, source=source
6874
)
6975
if should_filter:
@@ -76,12 +82,17 @@ def save_userreport(
7682
project_id=project.id,
7783
key_id=None,
7884
outcome=Outcome.INVALID,
79-
reason=outcomes_reason,
85+
reason=display_reason,
8086
timestamp=start_time,
8187
event_id=None, # Note report["event_id"] is id of the associated event, not the report itself.
8288
category=DataCategory.USER_REPORT_V2,
8389
quantity=1,
8490
)
91+
if (
92+
source == FeedbackCreationSource.USER_REPORT_DJANGO_ENDPOINT
93+
or source == FeedbackCreationSource.CRASH_REPORT_EMBED_FORM
94+
):
95+
raise BadRequest(display_reason)
8596
return None
8697

8798
# XXX(dcramer): enforce case insensitivity by coercing this to a lowercase string
@@ -207,7 +218,7 @@ def validate_user_report(
207218
208219
Reformatting: strips whitespace from comments and dashes from event_id.
209220
210-
Returns a tuple of (should_filter, metrics_reason, outcomes_reason). XXX: ensure metrics and outcome reasons have bounded cardinality.
221+
Returns a tuple of (should_filter, metrics_reason, display_reason). XXX: ensure metrics and outcome reasons have bounded cardinality.
211222
212223
At the moment we do not raise validation errors.
213224
"""

tests/sentry/api/endpoints/test_project_user_reports.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,3 +498,41 @@ def test_simple_shim_to_feedback_no_event_should_not_call(
498498
assert report.comments == "It broke!"
499499

500500
assert len(mock_produce_occurrence_to_kafka.mock_calls) == 0
501+
502+
@patch("sentry.ingest.userreport.validate_user_report")
503+
def test_validation_error(self, mock_validate_user_report):
504+
mock_validate_user_report.return_value = (True, "data_invalid", "Data invalid")
505+
self.login_as(user=self.user)
506+
url = _make_url(self.project)
507+
508+
response = self.client.post(
509+
url,
510+
data={
511+
"event_id": self.event.event_id,
512+
"email": "foo@example.com",
513+
"name": "Foo Bar",
514+
"comments": "It broke!",
515+
},
516+
)
517+
518+
assert response.status_code == 400, response.content
519+
assert UserReport.objects.count() == 0
520+
521+
@patch("sentry.ingest.userreport.is_in_feedback_denylist")
522+
def test_denylist(self, mock_is_in_feedback_denylist):
523+
mock_is_in_feedback_denylist.return_value = True
524+
self.login_as(user=self.user)
525+
url = _make_url(self.project)
526+
527+
response = self.client.post(
528+
url,
529+
data={
530+
"event_id": self.event.event_id,
531+
"email": "foo@example.com",
532+
"name": "Foo Bar",
533+
"comments": "It broke!",
534+
},
535+
)
536+
537+
assert response.status_code == 403, response.content
538+
assert UserReport.objects.count() == 0

0 commit comments

Comments
 (0)