fix: update internal labels endpoints to work with RBAC (#5099)

# What this PR does Related to: - grafana/oncall-private#2943 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --------- Co-authored-by: Vadim Stepanov <[email protected]>
grafana · Oct 2, 2024 · f39a755 · f39a755
1 parent 784b7e5
commit f39a755
Show file tree

Hide file tree

Showing 10 changed files with 95 additions and 240 deletions.
diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py
@@ -30,6 +30,7 @@
 from apps.metrics_exporter.tasks import update_metrics_for_alert_group
 from apps.slack.slack_formatter import SlackFormatter
 from apps.user_management.models import User
+from common.constants.plugin_ids import PluginID
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 from common.utils import clean_markup, str_or_backup
 
@@ -556,7 +557,7 @@ def web_link(self) -> str:
     @property
     def declare_incident_link(self) -> str:
         """Generate a link for AlertGroup to declare Grafana Incident by click"""
-        incident_link = urljoin(self.channel.organization.grafana_url, "a/grafana-incident-app/incidents/declare/")
+        incident_link = urljoin(self.channel.organization.grafana_url, f"a/{PluginID.INCIDENT}/incidents/declare/")
         caption = urllib.parse.quote_plus("OnCall Alert Group")
         title = urllib.parse.quote_plus(self.web_title_cache) if self.web_title_cache else DEFAULT_BACKUP_TITLE
         title = title[:2000]  # set max title length to avoid exceptions with too long declare incident link

diff --git a/engine/apps/api/permissions.py b/engine/apps/api/permissions.py
@@ -9,15 +9,14 @@
 from rest_framework.views import APIView
 from rest_framework.viewsets import ViewSet, ViewSetMixin
 
+from common.constants.plugin_ids import PluginID
 from common.utils import getattrd
 
 if typing.TYPE_CHECKING:
     from apps.user_management.models import User
 
-ACTION_PREFIX = "grafana-oncall-app"
 RBAC_PERMISSIONS_ATTR = "rbac_permissions"
 RBAC_OBJECT_PERMISSIONS_ATTR = "rbac_object_permissions"
-BASIC_ROLE_PERMISSIONS_ATTR = "basic_role_permissions"
 
 ViewSetOrAPIView = typing.Union[ViewSet, APIView]
 
@@ -67,6 +66,7 @@ class Resources(enum.Enum):
     OTHER_SETTINGS = "other-settings"
 
     ADMIN = "admin"
+    LABEL = "label"
 
 
 class Actions(enum.Enum):
@@ -78,6 +78,8 @@ class Actions(enum.Enum):
     UPDATE_SETTINGS = "update-settings"
     DIRECT_PAGING = "direct-paging"
 
+    CREATE = "create"
+
 
 class LegacyAccessControlRole(enum.IntEnum):
     ADMIN = 0
@@ -91,15 +93,20 @@ def choices(cls):
 
 
 class LegacyAccessControlCompatiblePermission:
-    def __init__(self, resource: Resources, action: Actions, fallback_role: LegacyAccessControlRole) -> None:
-        self.value = f"{ACTION_PREFIX}.{resource.value}:{action.value}"
+    def __init__(
+        self,
+        resource: Resources,
+        action: Actions,
+        fallback_role: LegacyAccessControlRole,
+        prefix: str = PluginID.ONCALL,
+    ) -> None:
+        self.value = f"{prefix}.{resource.value}:{action.value}"
         self.fallback_role = fallback_role
 
 
 LegacyAccessControlCompatiblePermissions = typing.List[LegacyAccessControlCompatiblePermission]
 RBACPermissionsAttribute = typing.Dict[str, LegacyAccessControlCompatiblePermissions]
 RBACObjectPermissionsAttribute = typing.Dict[permissions.BasePermission, typing.List[str]]
-BasicRolePermissionsAttribute = typing.Dict[str, LegacyAccessControlRole]
 
 
 def get_view_action(request: AuthenticatedRequest, view: ViewSetOrAPIView) -> str:
@@ -119,24 +126,14 @@ def get_most_authorized_role(permissions: LegacyAccessControlCompatiblePermissio
     return min({p.fallback_role for p in permissions}, key=lambda r: r.value)
 
 
-def user_is_authorized(
-    user: "User",
-    required_permissions: LegacyAccessControlCompatiblePermissions,
-    required_basic_role_permission: LegacyAccessControlRole = None,
-) -> bool:
+def user_is_authorized(user: "User", required_permissions: LegacyAccessControlCompatiblePermissions) -> bool:
     """
-    This function checks whether `user` has all necessary permissions. If `required_basic_role_permission` is set,
-    it only checks the basic user role, otherwise it checks whether `user` has all permissions in
-    `required_permissions`.
+    This function checks whether `user` has all necessary permissions specified in `required_permissions`.
     RBAC permissions are used if RBAC is enabled for the organization, otherwise the fallback basic role is checked.
 
-    user - The user to check permissions for
-    required_permissions - A list of permissions that a user must have to be considered authorized
-    required_basic_role_permission - Min basic role user must have to be considered authorized (used in cases when
-    it's needed to check ONLY the basic user role, otherwise `required_permissions` should be used)
+    `user` - The user to check permissions for
+    `required_permissions` - A list of permissions that a user must have to be considered authorized
     """
-    if required_basic_role_permission is not None:
-        return user.role <= required_basic_role_permission.value
     if user.organization.is_rbac_permissions_enabled:
         user_permissions = [u["action"] for u in user.permissions]
         required_permission_values = [p.value for p in required_permissions]
@@ -250,6 +247,17 @@ class Permissions:
             Resources.OTHER_SETTINGS, Actions.WRITE, LegacyAccessControlRole.ADMIN
         )
 
+        # NOTE: we don't currently add the label delete permission here because we don't currently use this in OnCall
+        LABEL_CREATE = LegacyAccessControlCompatiblePermission(
+            Resources.LABEL, Actions.CREATE, LegacyAccessControlRole.EDITOR, prefix=PluginID.LABELS
+        )
+        LABEL_READ = LegacyAccessControlCompatiblePermission(
+            Resources.LABEL, Actions.READ, LegacyAccessControlRole.VIEWER, prefix=PluginID.LABELS
+        )
+        LABEL_WRITE = LegacyAccessControlCompatiblePermission(
+            Resources.LABEL, Actions.WRITE, LegacyAccessControlRole.EDITOR, prefix=PluginID.LABELS
+        )
+
     # 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
@@ -301,40 +309,6 @@ def has_object_permission(self, request: AuthenticatedRequest, view: ViewSetOrAP
         return True
 
 
-class BasicRolePermission(permissions.BasePermission):
-    """Checks only basic user role permissions, regardless of whether RBAC is enabled for the organization"""
-
-    # 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
-    def has_permission(self, request: AuthenticatedRequest, view: ViewSetOrAPIView) -> bool:  # type: ignore[override]
-        # the django-debug-toolbar UI makes OPTIONS calls. Without this statement the debug UI can't gather the
-        # necessary info it needs to work properly
-        if settings.DEBUG and request.method == "OPTIONS":
-            return True
-        action = get_view_action(request, view)
-
-        basic_role_permissions: typing.Optional[BasicRolePermissionsAttribute] = getattr(
-            view, BASIC_ROLE_PERMISSIONS_ATTR, None
-        )
-
-        # first check that the basic_role_permissions dict attribute is defined
-        assert (
-            basic_role_permissions is not None
-        ), f"Must define a {BASIC_ROLE_PERMISSIONS_ATTR} dict on the ViewSet that is consuming the role class"
-
-        action_required_permissions: LegacyAccessControlRole = basic_role_permissions.get(action, None)
-
-        # next check that the action in question is defined within the basic_role_permissions dict attribute
-        assert (
-            action_required_permissions is not None
-        ), f"""Each action must be defined within the {BASIC_ROLE_PERMISSIONS_ATTR} dict on the ViewSet"""
-
-        return user_is_authorized(
-            request.user, required_permissions=[], required_basic_role_permission=action_required_permissions
-        )
-
-
 ALL_PERMISSION_NAMES = [perm for perm in dir(RBACPermission.Permissions) if not perm.startswith("_")]
 ALL_PERMISSION_CLASSES = [
     getattr(RBACPermission.Permissions, permission_name) for permission_name in ALL_PERMISSION_NAMES

diff --git a/engine/apps/api/tests/test_labels.py b/engine/apps/api/tests/test_labels.py
@@ -23,9 +23,8 @@ def test_labels_get_keys(
     mocked_get_labels_keys,
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
-    make_alert_receive_channel,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:get_keys")
     response = client.get(url, format="json", **make_user_auth_headers(user, token))
@@ -49,7 +48,7 @@ def test_get_update_key_get(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:get_update_key", kwargs={"key_id": "keyid123"})
     response = client.get(url, format="json", **make_user_auth_headers(user, token))
@@ -73,7 +72,7 @@ def test_get_update_key_put(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:get_update_key", kwargs={"key_id": "keyid123"})
     data = {"name": "team"}
@@ -98,7 +97,7 @@ def test_add_value(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:add_value", kwargs={"key_id": "keyid123"})
     data = {"name": "yolo"}
@@ -123,7 +122,7 @@ def test_rename_value(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:get_update_value", kwargs={"key_id": "keyid123", "value_id": "valueid123"})
     data = {"name": "yolo"}
@@ -148,7 +147,7 @@ def test_get_value(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:get_update_value", kwargs={"key_id": "keyid123", "value_id": "valueid123"})
     response = client.get(url, format="json", **make_user_auth_headers(user, token))
@@ -172,7 +171,7 @@ def test_labels_create_label(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
     url = reverse("api-internal:create_label")
     data = {"key": {"name": "team"}, "values": [{"name": "yolo"}]}
@@ -192,7 +191,7 @@ def test_labels_feature_false(
 ):
     settings.FEATURE_LABELS_ENABLED_FOR_ALL = False
 
-    organization, user, token = make_organization_and_user_with_plugin_token()
+    _, user, token = make_organization_and_user_with_plugin_token()
     client = APIClient()
 
     url = reverse("api-internal:get_keys")
@@ -240,7 +239,7 @@ def test_labels_permissions_get_actions(
     role,
     expected_status,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token(role)
+    _, user, token = make_organization_and_user_with_plugin_token(role)
     client = APIClient()
     with patch("apps.api.views.labels.LabelsViewSet.get_keys", return_value=Response(status=status.HTTP_200_OK)):
         url = reverse("api-internal:get_keys")
@@ -274,7 +273,7 @@ def test_labels_permissions_create_update_actions(
     role,
     expected_status,
 ):
-    organization, user, token = make_organization_and_user_with_plugin_token(role)
+    _, user, token = make_organization_and_user_with_plugin_token(role)
     client = APIClient()
     with patch("apps.api.views.labels.LabelsViewSet.rename_key", return_value=Response(status=status.HTTP_200_OK)):
         url = reverse("api-internal:get_update_key", kwargs={"key_id": "keyid123"})