From 111ecb92978ba0b76387ca887952f2e2dedbf18e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 16 Jun 2023 12:00:14 +0200 Subject: [PATCH] remove legacy permission strings (#2269) # What this PR does Remove deprecated `permissions` `List[str]` from internal API user response. These permission strings are no longer used and AFAICT are not referenced anywhere in the UI. ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 3 ++- engine/apps/api/permissions.py | 27 --------------------------- engine/apps/api/serializers/user.py | 12 ++---------- engine/apps/api/tests/test_user.py | 10 +--------- 4 files changed, 5 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ca77442..3f80630a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Change .Values.externalRabbitmq.passwordKey from `password` to `""` (default value `rabbitmq-password`) [864](https://github.com/grafana/oncall/pull/864) +- Change .Values.externalRabbitmq.passwordKey from `password` to `""` (default value `rabbitmq-password`) ([#864](https://github.com/grafana/oncall/pull/864)) +- Remove deprecated `permissions` string array from the internal API user serializer by @joeyorlando ([#2269](https://github.com/grafana/oncall/pull/2269)) ### Added diff --git a/engine/apps/api/permissions.py b/engine/apps/api/permissions.py index a1bebd91..0d77ec8d 100644 --- a/engine/apps/api/permissions.py +++ b/engine/apps/api/permissions.py @@ -298,30 +298,3 @@ class IsStaff(permissions.BasePermission): RBACPermissionsAttribute = typing.Dict[str, typing.List[LegacyAccessControlCompatiblePermission]] RBACObjectPermissionsAttribute = typing.Dict[permissions.BasePermission, typing.List[str]] - - -# The below is legacy, it is only needed currently for backward compatibility w/ users running -# older "pinned" version of Grafana in Grafana Cloud -_DONT_USE_LEGACY_VIEWER_PERMISSIONS = [] -_DONT_USE_LEGACY_EDITOR_PERMISSIONS = ["update_incidents", "update_own_settings", "view_other_users"] -_DONT_USE_LEGACY_ADMIN_PERMISSIONS = _DONT_USE_LEGACY_EDITOR_PERMISSIONS + [ - "update_alert_receive_channels", - "update_escalation_policies", - "update_notification_policies", - "update_general_log_channel_id", - "update_other_users_settings", - "update_integrations", - "update_schedules", - "update_custom_actions", - "update_api_tokens", - "update_teams", - "update_maintenances", - "update_global_settings", - "send_demo_alert", -] - -DONT_USE_LEGACY_PERMISSION_MAPPING: typing.Dict[LegacyAccessControlRole, typing.List[str]] = { - LegacyAccessControlRole.VIEWER: _DONT_USE_LEGACY_VIEWER_PERMISSIONS, - LegacyAccessControlRole.EDITOR: _DONT_USE_LEGACY_EDITOR_PERMISSIONS, - LegacyAccessControlRole.ADMIN: _DONT_USE_LEGACY_ADMIN_PERMISSIONS, -} diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index 77d8cb9e..1164954f 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -1,11 +1,9 @@ import math import time -import typing from django.conf import settings from rest_framework import serializers -from apps.api.permissions import DONT_USE_LEGACY_PERMISSION_MAPPING from apps.api.serializers.telegram import TelegramToUserConnectorSerializer from apps.base.messaging import get_messaging_backends from apps.base.models import UserNotificationPolicy @@ -37,7 +35,6 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): timezone = TimeZoneField(allow_null=True, required=False) avatar = serializers.URLField(source="avatar_url", read_only=True) avatar_full = serializers.URLField(source="avatar_full_url", read_only=True) - permissions = serializers.SerializerMethodField() notification_chain_verbal = serializers.SerializerMethodField() cloud_connection_status = serializers.SerializerMethodField() @@ -52,7 +49,7 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): "email", "username", "name", - "role", # LEGACY.. this should get removed eventually + "role", "avatar", "avatar_full", "timezone", @@ -62,7 +59,6 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): "slack_user_identity", "telegram_configuration", "messaging_backends", - "permissions", # LEGACY.. this should get removed eventually "notification_chain_verbal", "cloud_connection_status", "hide_phone_number", @@ -71,7 +67,7 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): "email", "username", "name", - "role", # LEGACY.. this should get removed eventually + "role", "verified_phone_number", ] @@ -128,9 +124,6 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): serialized_data[backend_id] = backend.serialize_user(obj) return serialized_data - def get_permissions(self, obj) -> typing.List[str]: - return DONT_USE_LEGACY_PERMISSION_MAPPING[obj.role] - def get_notification_chain_verbal(self, obj): default, important = UserNotificationPolicy.get_short_verbals_for_user(user=obj) return {"default": " - ".join(default), "important": " - ".join(important)} @@ -173,7 +166,6 @@ class UserHiddenFieldsSerializer(UserSerializer): "timezone", "working_hours", "notification_chain_verbal", - "permissions", ] def to_representation(self, instance): diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index 29eda039..ec30db63 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -10,12 +10,7 @@ from rest_framework import status from rest_framework.response import Response from rest_framework.test import APIClient -from apps.api.permissions import ( - DONT_USE_LEGACY_PERMISSION_MAPPING, - GrafanaAPIPermission, - LegacyAccessControlRole, - RBACPermission, -) +from apps.api.permissions import GrafanaAPIPermission, LegacyAccessControlRole, RBACPermission from apps.base.models import UserNotificationPolicy from apps.phone_notifications.exceptions import FailedToFinishVerification from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb @@ -96,7 +91,6 @@ def test_update_user_cant_change_email_and_username( } }, "cloud_connection_status": None, - "permissions": DONT_USE_LEGACY_PERMISSION_MAPPING[admin.role], "notification_chain_verbal": {"default": "", "important": ""}, "slack_user_identity": None, "avatar": admin.avatar_url, @@ -147,7 +141,6 @@ def test_list_users( "user": admin.username, } }, - "permissions": DONT_USE_LEGACY_PERMISSION_MAPPING[admin.role], "notification_chain_verbal": {"default": "", "important": ""}, "slack_user_identity": None, "avatar": admin.avatar_url, @@ -173,7 +166,6 @@ def test_list_users( "user": editor.username, } }, - "permissions": DONT_USE_LEGACY_PERMISSION_MAPPING[editor.role], "notification_chain_verbal": {"default": "", "important": ""}, "slack_user_identity": None, "avatar": editor.avatar_url,