-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(aci): Disable error monitor create button #103544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(aci): Disable error monitor create button #103544
Conversation
| return hasEveryAccess(['alerts:write'], {organization}); | ||
| } | ||
|
|
||
| return detectorTypeIsUserCreateable(detectorType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent authorization allows monitor creation.
When detectorType is provided and user-createable, the hook returns true without checking the user's alerts:write permission. This allows users without proper permissions to see an enabled "Create Monitor" button for user-createable detector types. The permission check is only performed when detectorType is null, creating an inconsistent authorization check.
| ), | ||
| } | ||
| ); | ||
| const permissionTooltipText = detectorTypeIsUserCreateable(detector.type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this only show if you're unable to edit the monitor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh canEdit inherited behavior huh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm I see I think
| } | ||
| ) | ||
| : t( | ||
| 'This monitor is managed by Sentry. Only organization owners and managers can edit it.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, is this right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now yeah, only org owners/managers can access the edit screen for error monitors
Closes ID-1104