feat(annotations): security permissions simplification (#12014)

* Changed security permissions for annotations and annotation layers

* Updated permissions in annotation layers list

* Created test for retrieving premissions info. Updated uris from f-strings to strings

* Updated annotations in security_tests and added annotations to NEW_SECURITY_CONVERGE_VIEWS

* Added migration for annotations security converge

* Updated current revision after rebase master

* Updated migration name to annotations security converge

* Updated annotations permissions names in AnnotationLayersList and updated test since 'can_write' has wider permissions

* Updated annotations migration to current

superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx

@@ -61,7 +61,7 @@ const mockUser = {
 };
 
 fetchMock.get(layersInfoEndpoint, {
-  permissions: ['can_delete'],
+  permissions: ['can_write'],
 });
 fetchMock.get(layersEndpoint, {
   result: mocklayers,
@@ -156,7 +156,7 @@ describe('AnnotationLayersList', () => {
   });
 
   it('shows/hides bulk actions when bulk actions is clicked', async () => {
-    const button = wrapper.find(Button).at(0);
+    const button = wrapper.find(Button).at(1);
     act(() => {
       button.props().onClick();
     });

superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx

@@ -114,9 +114,9 @@ function AnnotationLayersList({
     );
   };
 
-  const canCreate = hasPerm('can_add');
-  const canEdit = hasPerm('can_edit');
-  const canDelete = hasPerm('can_delete');
+  const canCreate = hasPerm('can_write');
+  const canEdit = hasPerm('can_write');
+  const canDelete = hasPerm('can_write');
 
   function handleAnnotationLayerEdit(layer: AnnotationLayerObject | null) {
     setCurrentAnnotationLayer(layer);

superset/annotation_layers/annotations/api.py

@@ -52,7 +52,7 @@ from superset.annotation_layers.annotations.schemas import (
     openapi_spec_methods_override,
 )
 from superset.annotation_layers.commands.exceptions import AnnotationLayerNotFoundError
-from superset.constants import RouteMethod
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.models.annotations import Annotation
 from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
 
@@ -65,7 +65,9 @@ class AnnotationRestApi(BaseSupersetModelRestApi):
     include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | {
         "bulk_delete",  # not using RouteMethod since locally defined
     }
-    class_permission_name = "AnnotationLayerModelView"
+    class_permission_name = "Annotation"
+    method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
     resource_name = "annotation_layer"
     allow_browser_login = True
 

superset/annotation_layers/api.py

@@ -46,7 +46,7 @@ from superset.annotation_layers.schemas import (
     get_delete_ids_schema,
     openapi_spec_methods_override,
 )
-from superset.constants import RouteMethod
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.extensions import event_logger
 from superset.models.annotations import AnnotationLayer
 from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
@@ -61,7 +61,9 @@ class AnnotationLayerRestApi(BaseSupersetModelRestApi):
         RouteMethod.RELATED,
         "bulk_delete",  # not using RouteMethod since locally defined
     }
-    class_permission_name = "AnnotationLayerModelView"
+    class_permission_name = "Annotation"
+    method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
     resource_name = "annotation_layer"
     allow_browser_login = True
 

superset/migrations/versions/c25cb2c78727_security_converge_annotations.py

@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""security converge annotations
+
+Revision ID: c25cb2c78727
+Revises: ccb74baaa89b
+Create Date: 2020-12-11 17:02:21.213046
+
+"""
+
+from alembic import op
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+
+# revision identifiers, used by Alembic.
+from superset.migrations.shared.security_converge import (
+    add_pvms,
+    get_reversed_new_pvms,
+    get_reversed_pvm_map,
+    migrate_roles,
+    Pvm,
+)
+
+revision = "c25cb2c78727"
+down_revision = "ccb74baaa89b"
+
+
+NEW_PVMS = {"Annotation": ("can_read", "can_write",)}
+PVM_MAP = {
+    Pvm("AnnotationLayerModelView", "can_delete"): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationLayerModelView", "can_list"): (Pvm("Annotation", "can_read"),),
+    Pvm("AnnotationLayerModelView", "can_show",): (Pvm("Annotation", "can_read"),),
+    Pvm("AnnotationLayerModelView", "can_add",): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationLayerModelView", "can_edit",): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationModelView", "can_annotation",): (Pvm("Annotation", "can_read"),),
+    Pvm("AnnotationModelView", "can_show",): (Pvm("Annotation", "can_read"),),
+    Pvm("AnnotationModelView", "can_add",): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationModelView", "can_delete",): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationModelView", "can_edit",): (Pvm("Annotation", "can_write"),),
+    Pvm("AnnotationModelView", "can_list",): (Pvm("Annotation", "can_read"),),
+}
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = Session(bind=bind)
+
+    # Add the new permissions on the migration itself
+    add_pvms(session, NEW_PVMS)
+    migrate_roles(session, PVM_MAP)
+    try:
+        session.commit()
+    except SQLAlchemyError as ex:
+        print(f"An error occurred while upgrading annotation permissions: {ex}")
+        session.rollback()
+
+
+def downgrade():
+    bind = op.get_bind()
+    session = Session(bind=bind)
+
+    # Add the old permissions on the migration itself
+    add_pvms(session, get_reversed_new_pvms(PVM_MAP))
+    migrate_roles(session, get_reversed_pvm_map(PVM_MAP))
+    try:
+        session.commit()
+    except SQLAlchemyError as ex:
+        print(f"An error occurred while downgrading annotation permissions: {ex}")
+        session.rollback()
+    pass

superset/views/annotations.py

@@ -24,7 +24,7 @@ from flask_babel import lazy_gettext as _
 from wtforms.validators import StopValidation
 
 from superset import is_feature_enabled
-from superset.constants import RouteMethod
+from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.models.annotations import Annotation, AnnotationLayer
 from superset.typing import FlaskResponse
 from superset.views.base import SupersetModelView
@@ -54,6 +54,9 @@ class AnnotationModelView(
     datamodel = SQLAInterface(Annotation)
     include_route_methods = RouteMethod.CRUD_SET | {"annotation"}
 
+    class_permission_name = "Annotation"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     list_title = _("Annotations")
     show_title = _("Show Annotation")
     add_title = _("Add Annotation")
@@ -109,6 +112,10 @@ class AnnotationLayerModelView(SupersetModelView):  # pylint: disable=too-many-a
     datamodel = SQLAInterface(AnnotationLayer)
     include_route_methods = RouteMethod.CRUD_SET | {RouteMethod.API_READ}
     related_views = [AnnotationModelView]
+
+    class_permission_name = "Annotation"
+    method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
     list_title = _("Annotation Layers")
     show_title = _("Show Annotation Layer")
     add_title = _("Add Annotation Layer")

tests/annotation_layers/api_tests.py

@@ -75,10 +75,24 @@ class TestAnnotationLayerApi(SupersetTestCase):
         Annotation API: Test info
         """
         self.login(username="admin")
-        uri = f"api/v1/annotation_layer/_info"
+        uri = "api/v1/annotation_layer/_info"
         rv = self.get_assert_metric(uri, "info")
         assert rv.status_code == 200
 
+    def test_info_security_query(self):
+        """
+        Annotation API: Test info security
+        """
+        self.login(username="admin")
+        params = {"keys": ["permissions"]}
+        uri = f"api/v1/annotation_layer/_info?q={prison.dumps(params)}"
+        rv = self.get_assert_metric(uri, "info")
+        data = json.loads(rv.data.decode("utf-8"))
+        assert rv.status_code == 200
+        assert "can_read" in data["permissions"]
+        assert "can_write" in data["permissions"]
+        assert len(data["permissions"]) == 2
+
     @pytest.mark.usefixtures("create_annotation_layers")
     def test_get_annotation_layer_not_found(self):
         """
@@ -96,7 +110,7 @@ class TestAnnotationLayerApi(SupersetTestCase):
         Annotation Api: Test get list annotation layers
         """
         self.login(username="admin")
-        uri = f"api/v1/annotation_layer/"
+        uri = "api/v1/annotation_layer/"
         rv = self.get_assert_metric(uri, "get_list")
 
         expected_fields = [
@@ -120,7 +134,7 @@ class TestAnnotationLayerApi(SupersetTestCase):
         Annotation Api: Test sorting on get list annotation layers
         """
         self.login(username="admin")
-        uri = f"api/v1/annotation_layer/"
+        uri = "api/v1/annotation_layer/"
 
         order_columns = [
             "name",

tests/security_tests.py

@@ -48,7 +48,7 @@ from .dashboard_utils import (
 from .fixtures.energy_dashboard import load_energy_table_with_slice
 from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice
 
-NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery", "Chart")
+NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery", "Chart", "Annotation")
 
 
 def get_perm_tuples(role_name):
@@ -672,7 +672,7 @@ class TestRolePermission(SupersetTestCase):
         self.assert_can_menu("Dashboards", perm_set)
 
     def assert_can_alpha(self, perm_set):
-        self.assert_can_all("AnnotationLayerModelView", perm_set)
+        self.assert_can_all("Annotation", perm_set)
         self.assert_can_all("CssTemplate", perm_set)
         self.assert_can_all("TableModelView", perm_set)
         self.assert_can_read("QueryView", perm_set)