feat(replays): allow org:write to change granular replay permission settings instead of org:admin #105036
Conversation
…ettings instead of org:admin
| def test_granular_replay_permissions_admin_cannot_edit(self) -> None: | ||
| owner = self.create_user() | ||
| org = self.create_organization(owner=owner) | ||
| admin = self.create_user() | ||
| self.create_member(organization=org, user=admin, role="admin") | ||
| member = self.create_member(organization=org, user=self.create_user(), role="member") | ||
| self.login_as(admin) | ||
|
|
||
| self.get_error_response( | ||
| org.slug, hasGranularReplayPermissions=True, replayAccessMembers=[member.user_id] | ||
| ) |
There was a problem hiding this comment.
Bug: Changing the permission check from org:admin to org:write causes a loss of functionality for users with the deprecated "admin" role, as this role lacks the org:write scope.
Severity: MEDIUM | Confidence: High
🔍 Detailed Analysis
The pull request changes the permission check for modifying granular replay permissions from requiring the org:admin scope to requiring the org:write scope. However, the deprecated "admin" role, defined in src/sentry/conf/server.py, was never granted the org:write scope. As a result, users with the "admin" role will no longer be able to edit these settings, which they could previously. This creates an unintended loss of functionality and an inconsistent permission hierarchy where "admin" users have fewer privileges than "manager" users for this specific feature.
💡 Suggested Fix
To maintain backward compatibility for users with the deprecated "admin" role, either add the org:write scope to the "admin" role definition or update the permission check to accept both org:write and org:admin scopes.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: tests/sentry/core/endpoints/test_organization_details.py#L1766-L1776
Potential issue: The pull request changes the permission check for modifying granular
replay permissions from requiring the `org:admin` scope to requiring the `org:write`
scope. However, the deprecated "admin" role, defined in `src/sentry/conf/server.py`, was
never granted the `org:write` scope. As a result, users with the "admin" role will no
longer be able to edit these settings, which they could previously. This creates an
unintended loss of functionality and an inconsistent permission hierarchy where "admin"
users have fewer privileges than "manager" users for this specific feature.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7584428
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #105036 +/- ##
========================================
Coverage 80.54% 80.54%
========================================
Files 9398 9398
Lines 403154 403154
Branches 25622 25622
========================================
+ Hits 324734 324738 +4
+ Misses 77972 77968 -4
Partials 448 448 |
org:writeto change the granular replay permission settings instead oforg:admin