Skip to content
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

Change mapping of permission NOTIFICATIONS_READ to legacy role #5358

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions engine/apps/api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,29 @@ class Permissions:
Resources.LABEL, Actions.WRITE, LegacyAccessControlRole.EDITOR, prefix=PluginID.LABELS
)

class ViewerOnCallPermissions(Permissions):
"""
This class is used to define permissions for the "Viewer on Call" role. This role is used in the context of
the "Viewer on Call" feature flag.
The role is a subset of the "Viewer" role, and is used to define permissions for users who
are allowed be OnCall having only READ role in grafana.
"""

ALERT_GROUPS_WRITE = LegacyAccessControlCompatiblePermission(
Resources.ALERT_GROUPS, Actions.WRITE, LegacyAccessControlRole.VIEWER
)
ALERT_GROUPS_DIRECT_PAGING = LegacyAccessControlCompatiblePermission(
Resources.ALERT_GROUPS, Actions.DIRECT_PAGING, LegacyAccessControlRole.VIEWER
)
SCHEDULES_WRITE = LegacyAccessControlCompatiblePermission(
Resources.SCHEDULES, Actions.WRITE, LegacyAccessControlRole.VIEWER
)
NOTIFICATIONS_READ = LegacyAccessControlCompatiblePermission(
Resources.NOTIFICATIONS, Actions.READ, LegacyAccessControlRole.VIEWER
)

permissions: Permissions = Permissions if not settings.FEATURE_ALLOW_VIEWERS_ON_CALL else ViewerOnCallPermissions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure, what happens with the other permissions when the setting is enabled? This seems like it could break if combined with an RBAC-based setup?
OTOH, what if permissions are added or changed? I think we would prefer a more general/reusable approach to avoid potential unexpected behaviors in the future (I still think allowing viewers to be on-call may make sense in some cases, but it would be nice to make the change as simple and future-proof as possible)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure, what happens with the other permissions when the setting is enabled? This seems like it could break if combined with an RBAC-based setup?

I don't see how the changes may break something. When FEATURE_ALLOW_VIEWERS_ON_CALL == false - previously existing logic is being used. Class Permissions doesn't have any modifications. ViewerOnCallPermissions simply redefines several permissions. Do I miss something?


# mypy complains about "Liskov substitution principle" here because request is `AuthenticatedRequest` object
# and not rest_framework.request.Request
# https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Expand Down Expand Up @@ -350,9 +373,9 @@ def has_object_permission(self, request: AuthenticatedRequest, view: ViewSetOrAP
return True


ALL_PERMISSION_NAMES = [perm for perm in dir(RBACPermission.Permissions) if not perm.startswith("_")]
ALL_PERMISSION_NAMES = [perm for perm in dir(RBACPermission.permissions) if not perm.startswith("_")]
ALL_PERMISSION_CLASSES: LegacyAccessControlCompatiblePermissions = [
getattr(RBACPermission.Permissions, permission_name) for permission_name in ALL_PERMISSION_NAMES
getattr(RBACPermission.permissions, permission_name) for permission_name in ALL_PERMISSION_NAMES
]
ALL_PERMISSION_CHOICES: typing.List[typing.Tuple[str, str]] = []
for permission_class, permission_name in zip(ALL_PERMISSION_CLASSES, ALL_PERMISSION_NAMES):
Expand Down
162 changes: 82 additions & 80 deletions engine/apps/api/tests/test_permissions.py

Large diffs are not rendered by default.

12 changes: 7 additions & 5 deletions engine/apps/api/tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ def test_list_users_filtered_by_granted_permission(
make_token_for_organization,
make_user_auth_headers,
):
permission = permissions.RBACPermission.Permissions.NOTIFICATIONS_READ
admin_perm_required_to_call_endpoint = permissions.RBACPermission.Permissions.USER_SETTINGS_READ
permission = permissions.RBACPermission.permissions.NOTIFICATIONS_READ
admin_perm_required_to_call_endpoint = permissions.RBACPermission.permissions.USER_SETTINGS_READ
perm_to_filter_on = (
permissions.convert_oncall_permission_to_irm(permission) if is_grafana_irm_enabled else permission.value
)
Expand All @@ -340,9 +340,11 @@ def test_list_users_filtered_by_granted_permission(
# make_user_for_organization fixture will only grant the oncall flavour of the permission
permissions=permissions.GrafanaAPIPermissions.construct_permissions(
[
permissions.convert_oncall_permission_to_irm(admin_perm_required_to_call_endpoint)
if is_grafana_irm_enabled
else admin_perm_required_to_call_endpoint.value
(
permissions.convert_oncall_permission_to_irm(admin_perm_required_to_call_endpoint)
if is_grafana_irm_enabled
else admin_perm_required_to_call_endpoint.value
)
]
),
)
Expand Down
40 changes: 20 additions & 20 deletions engine/apps/api/views/alert_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,26 +263,26 @@ class AlertGroupView(
permission_classes = (IsAuthenticated, RBACPermission)

rbac_permissions = {
"metadata": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"list": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"retrieve": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"stats": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"filters": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"silence_options": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"bulk_action_options": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"destroy": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"acknowledge": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"unacknowledge": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"resolve": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"unresolve": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"attach": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"unattach": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"silence": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"unsilence": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"unpage_user": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"bulk_action": [RBACPermission.Permissions.ALERT_GROUPS_WRITE],
"preview_template": [RBACPermission.Permissions.INTEGRATIONS_TEST],
"escalation_snapshot": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"metadata": [RBACPermission.permissions.ALERT_GROUPS_READ],
"list": [RBACPermission.permissions.ALERT_GROUPS_READ],
"retrieve": [RBACPermission.permissions.ALERT_GROUPS_READ],
"stats": [RBACPermission.permissions.ALERT_GROUPS_READ],
"filters": [RBACPermission.permissions.ALERT_GROUPS_READ],
"silence_options": [RBACPermission.permissions.ALERT_GROUPS_READ],
"bulk_action_options": [RBACPermission.permissions.ALERT_GROUPS_READ],
"destroy": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"acknowledge": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"unacknowledge": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"resolve": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"unresolve": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"attach": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"unattach": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"silence": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"unsilence": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"unpage_user": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"bulk_action": [RBACPermission.permissions.ALERT_GROUPS_WRITE],
"preview_template": [RBACPermission.permissions.INTEGRATIONS_TEST],
"escalation_snapshot": [RBACPermission.permissions.ALERT_GROUPS_READ],
}

queryset = AlertGroup.objects.none() # needed for drf-spectacular introspection
Expand Down
8 changes: 4 additions & 4 deletions engine/apps/api/views/alert_group_table_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class AlertGroupTableColumnsViewSet(LabelsFeatureFlagViewSet):
permission_classes = (IsAuthenticated, RBACPermission)

rbac_permissions = {
"get_columns": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"update_user_columns": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"reset_user_columns": [RBACPermission.Permissions.ALERT_GROUPS_READ],
"update_organization_columns": [RBACPermission.Permissions.OTHER_SETTINGS_WRITE],
"get_columns": [RBACPermission.permissions.ALERT_GROUPS_READ],
"update_user_columns": [RBACPermission.permissions.ALERT_GROUPS_READ],
"reset_user_columns": [RBACPermission.permissions.ALERT_GROUPS_READ],
"update_organization_columns": [RBACPermission.permissions.OTHER_SETTINGS_WRITE],
}

def get_columns(self, request: Request) -> Response:
Expand Down
80 changes: 41 additions & 39 deletions engine/apps/api/views/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,42 +150,42 @@ class AlertReceiveChannelView(
pagination_class = FifteenPageSizePaginator

rbac_permissions = {
"metadata": [RBACPermission.Permissions.INTEGRATIONS_READ],
"list": [RBACPermission.Permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.Permissions.INTEGRATIONS_READ],
"integration_options": [RBACPermission.Permissions.INTEGRATIONS_READ],
"counters": [RBACPermission.Permissions.INTEGRATIONS_READ],
"counters_per_integration": [RBACPermission.Permissions.INTEGRATIONS_READ],
"send_demo_alert": [RBACPermission.Permissions.INTEGRATIONS_TEST],
"preview_template": [RBACPermission.Permissions.INTEGRATIONS_TEST],
"create": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"destroy": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"change_team": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"filters": [RBACPermission.Permissions.INTEGRATIONS_READ],
"start_maintenance": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"stop_maintenance": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"validate_name": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"migrate": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"connected_contact_points": [RBACPermission.Permissions.INTEGRATIONS_READ],
"contact_points": [RBACPermission.Permissions.INTEGRATIONS_READ],
"connect_contact_point": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"create_contact_point": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"disconnect_contact_point": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"test_connection_create": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"test_connection": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"status_options": [RBACPermission.Permissions.INTEGRATIONS_READ],
"webhooks_get": [RBACPermission.Permissions.INTEGRATIONS_READ],
"webhooks_post": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"webhooks_put": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"webhooks_delete": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_get": [RBACPermission.Permissions.INTEGRATIONS_READ],
"connected_alert_receive_channels_post": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_put": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_delete": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"backsync_token_get": [RBACPermission.Permissions.INTEGRATIONS_READ],
"backsync_token_post": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"metadata": [RBACPermission.permissions.INTEGRATIONS_READ],
"list": [RBACPermission.permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.permissions.INTEGRATIONS_READ],
"integration_options": [RBACPermission.permissions.INTEGRATIONS_READ],
"counters": [RBACPermission.permissions.INTEGRATIONS_READ],
"counters_per_integration": [RBACPermission.permissions.INTEGRATIONS_READ],
"send_demo_alert": [RBACPermission.permissions.INTEGRATIONS_TEST],
"preview_template": [RBACPermission.permissions.INTEGRATIONS_TEST],
"create": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"destroy": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"change_team": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"filters": [RBACPermission.permissions.INTEGRATIONS_READ],
"start_maintenance": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"stop_maintenance": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"validate_name": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"migrate": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"connected_contact_points": [RBACPermission.permissions.INTEGRATIONS_READ],
"contact_points": [RBACPermission.permissions.INTEGRATIONS_READ],
"connect_contact_point": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"create_contact_point": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"disconnect_contact_point": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"test_connection_create": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"test_connection": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"status_options": [RBACPermission.permissions.INTEGRATIONS_READ],
"webhooks_get": [RBACPermission.permissions.INTEGRATIONS_READ],
"webhooks_post": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"webhooks_put": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"webhooks_delete": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_get": [RBACPermission.permissions.INTEGRATIONS_READ],
"connected_alert_receive_channels_post": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_put": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"connected_alert_receive_channels_delete": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"backsync_token_get": [RBACPermission.permissions.INTEGRATIONS_READ],
"backsync_token_post": [RBACPermission.permissions.INTEGRATIONS_WRITE],
}

def perform_update(self, serializer):
Expand Down Expand Up @@ -375,9 +375,11 @@ def integration_options(self, request):
"display_name": integration_title,
"short_description": AlertReceiveChannel.INTEGRATION_SHORT_DESCRIPTION[integration_id],
"featured": integration_id in AlertReceiveChannel.INTEGRATION_FEATURED,
"featured_tag_name": AlertReceiveChannel.INTEGRATION_FEATURED_TAG_NAME[integration_id]
if integration_id in AlertReceiveChannel.INTEGRATION_FEATURED_TAG_NAME
else None,
"featured_tag_name": (
AlertReceiveChannel.INTEGRATION_FEATURED_TAG_NAME[integration_id]
if integration_id in AlertReceiveChannel.INTEGRATION_FEATURED_TAG_NAME
else None
),
}
# if integration is featured we show it in the beginning
if choice["featured"]:
Expand Down
10 changes: 5 additions & 5 deletions engine/apps/api/views/alert_receive_channel_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ class AlertReceiveChannelTemplateView(
permission_classes = (IsAuthenticated, RBACPermission)

rbac_permissions = {
"metadata": [RBACPermission.Permissions.INTEGRATIONS_READ],
"list": [RBACPermission.Permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.Permissions.INTEGRATIONS_READ],
"update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"metadata": [RBACPermission.permissions.INTEGRATIONS_READ],
"list": [RBACPermission.permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.permissions.INTEGRATIONS_READ],
"update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
}

model = AlertReceiveChannel
Expand Down
18 changes: 9 additions & 9 deletions engine/apps/api/views/channel_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ class ChannelFilterView(
authentication_classes = (PluginAuthentication,)
permission_classes = (IsAuthenticated, RBACPermission)
rbac_permissions = {
"metadata": [RBACPermission.Permissions.INTEGRATIONS_READ],
"list": [RBACPermission.Permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.Permissions.INTEGRATIONS_READ],
"create": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"destroy": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"move_to_position": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"convert_from_regex_to_jinja2": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
"metadata": [RBACPermission.permissions.INTEGRATIONS_READ],
"list": [RBACPermission.permissions.INTEGRATIONS_READ],
"retrieve": [RBACPermission.permissions.INTEGRATIONS_READ],
"create": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"partial_update": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"destroy": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"move_to_position": [RBACPermission.permissions.INTEGRATIONS_WRITE],
"convert_from_regex_to_jinja2": [RBACPermission.permissions.INTEGRATIONS_WRITE],
}

queryset = ChannelFilter.objects.none() # needed for drf-spectacular introspection
Expand Down
2 changes: 1 addition & 1 deletion engine/apps/api/views/direct_paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class DirectPagingAPIView(APIView):
permission_classes = (IsAuthenticated, RBACPermission)

rbac_permissions = {
"post": [RBACPermission.Permissions.ALERT_GROUPS_DIRECT_PAGING],
"post": [RBACPermission.permissions.ALERT_GROUPS_DIRECT_PAGING],
}

def post(self, request):
Expand Down
18 changes: 9 additions & 9 deletions engine/apps/api/views/escalation_chain.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ class EscalationChainViewSet(
permission_classes = (IsAuthenticated, RBACPermission)

rbac_permissions = {
"metadata": [RBACPermission.Permissions.ESCALATION_CHAINS_READ],
"list": [RBACPermission.Permissions.ESCALATION_CHAINS_READ],
"retrieve": [RBACPermission.Permissions.ESCALATION_CHAINS_READ],
"details": [RBACPermission.Permissions.ESCALATION_CHAINS_READ],
"create": [RBACPermission.Permissions.ESCALATION_CHAINS_WRITE],
"update": [RBACPermission.Permissions.ESCALATION_CHAINS_WRITE],
"destroy": [RBACPermission.Permissions.ESCALATION_CHAINS_WRITE],
"copy": [RBACPermission.Permissions.ESCALATION_CHAINS_WRITE],
"filters": [RBACPermission.Permissions.ESCALATION_CHAINS_READ],
"metadata": [RBACPermission.permissions.ESCALATION_CHAINS_READ],
"list": [RBACPermission.permissions.ESCALATION_CHAINS_READ],
"retrieve": [RBACPermission.permissions.ESCALATION_CHAINS_READ],
"details": [RBACPermission.permissions.ESCALATION_CHAINS_READ],
"create": [RBACPermission.permissions.ESCALATION_CHAINS_WRITE],
"update": [RBACPermission.permissions.ESCALATION_CHAINS_WRITE],
"destroy": [RBACPermission.permissions.ESCALATION_CHAINS_WRITE],
"copy": [RBACPermission.permissions.ESCALATION_CHAINS_WRITE],
"filters": [RBACPermission.permissions.ESCALATION_CHAINS_READ],
}

queryset = EscalationChain.objects.none() # needed for drf-spectacular introspection
Expand Down
Loading