diff --git a/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx index 7a4847007e07e..fa6adddae69af 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx +++ b/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(); }); diff --git a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx index d066d4dd12779..92bf9bd586e07 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx +++ b/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); diff --git a/superset/annotation_layers/annotations/api.py b/superset/annotation_layers/annotations/api.py index d7241b650e696..1c63f699d51c6 100644 --- a/superset/annotation_layers/annotations/api.py +++ b/superset/annotation_layers/annotations/api.py @@ -52,7 +52,7 @@ 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 diff --git a/superset/annotation_layers/api.py b/superset/annotation_layers/api.py index c608e30157d56..b47bf87ecb891 100644 --- a/superset/annotation_layers/api.py +++ b/superset/annotation_layers/api.py @@ -46,7 +46,7 @@ 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 diff --git a/superset/migrations/versions/45731db65d9c_security_converge_datasets.py b/superset/migrations/versions/45731db65d9c_security_converge_datasets.py index 30cc83987684c..5b4670857faf4 100644 --- a/superset/migrations/versions/45731db65d9c_security_converge_datasets.py +++ b/superset/migrations/versions/45731db65d9c_security_converge_datasets.py @@ -24,7 +24,7 @@ # revision identifiers, used by Alembic. revision = "45731db65d9c" -down_revision = "ccb74baaa89b" +down_revision = "c25cb2c78727" from alembic import op from sqlalchemy.exc import SQLAlchemyError diff --git a/superset/migrations/versions/c25cb2c78727_security_converge_annotations.py b/superset/migrations/versions/c25cb2c78727_security_converge_annotations.py new file mode 100644 index 0000000000000..33099dd2e74b2 --- /dev/null +++ b/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 diff --git a/superset/views/annotations.py b/superset/views/annotations.py index 03cc26006ff35..345fd2c15a1f0 100644 --- a/superset/views/annotations.py +++ b/superset/views/annotations.py @@ -24,7 +24,7 @@ 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") diff --git a/tests/annotation_layers/api_tests.py b/tests/annotation_layers/api_tests.py index 0ee361bcfbd1b..b53624394ef46 100644 --- a/tests/annotation_layers/api_tests.py +++ b/tests/annotation_layers/api_tests.py @@ -75,10 +75,24 @@ def test_info_annotation(self): 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 @@ def test_get_list_annotation_layer(self): 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 @@ def test_get_list_annotation_layer_sorting(self): 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", diff --git a/tests/security_tests.py b/tests/security_tests.py index 3d4ce77053cf9..5625fd9636b1c 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -48,7 +48,13 @@ from .fixtures.energy_dashboard import load_energy_table_with_slice from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice -NEW_SECURITY_CONVERGE_VIEWS = ("Dataset", "CssTemplate", "Chart", "SavedQuery") +NEW_SECURITY_CONVERGE_VIEWS = ( + "Annotation", + "Dataset", + "CssTemplate", + "Chart", + "SavedQuery", +) def get_perm_tuples(role_name): @@ -672,7 +678,7 @@ def assert_can_gamma(self, perm_set): 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("Dataset", perm_set) self.assert_can_read("QueryView", perm_set)