fix(trace_related): Use global_views whitelist for events endpoint #75174
Conversation
github-actions
bot
added
the
Scope: Backend
armenzg
changed the title
| @@ -165,6 +165,17 @@ def test_performance_view_feature(self): | |||
| assert response.status_code == 200 | |||
| assert len(response.data["data"]) == 1 | |||
|
|
|||
| def test_multi_project_feature_gate_rejection(self): | |||
There was a problem hiding this comment.
This adds the original test back.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #75174 +/- ##
===========================================
+ Coverage 57.02% 78.05% +21.03%
===========================================
Files 6749 6759 +10
Lines 301279 301671 +392
Branches 51856 51910 +54
===========================================
+ Hits 171809 235477 +63668
+ Misses 124697 59848 -64849
- Partials 4773 6346 +1573
|
- This updates events-meta to use the snuba params dataclass instead of the dict - Tests failing until #75174 merges
There was a problem hiding this comment.
As discussed, since there's currently a DACI to change this behaviour lets not go with the original suggestion to add new endpoints yet.
Instead please add (with a TODO to remove and a comment explaining why its a bad pattern to use more) a check that the referrer is in an allowlist before allowing global-views
| @@ -130,9 +130,6 @@ def get_snuba_dataclass( | |||
| has_global_views, | |||
| len(params.projects) <= 1, | |||
| fetching_replay_data, | |||
| # If a developer can view issues of a project they do not belong to | |||
| # via open membership, we will also allow the endpoint to return events for it | |||
| organization.flags.allow_joinleave, | |||
There was a problem hiding this comment.
This was added to my original PR.
This has been moved down below:
armenzg
changed the title
Co-authored-by: William Mak <william@wmak.io>
|
let's make sure we revisit this within a few weeks since that's the current expectation for the DACI to have a decision |
- This updates events-meta to use the snuba params dataclass instead of the dict - Tests failing until #75174 merges
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
In #72961 I added a change to a base class which I should have not.
This PR reverts my original change and only adjusts the events endpoint and requires a referrer whitelist.