From 359cff138c9096de5cf74079de5be02a7385ede9 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 30 Nov 2020 18:11:02 +0000 Subject: [PATCH 1/5] feat: security converge css templates --- .../CRUD/csstemplates/CssTemplatesList_spec.jsx | 2 +- .../views/CRUD/csstemplates/CssTemplatesList.tsx | 6 +++--- superset/css_templates/api.py | 6 ++++-- superset/views/css_templates.py | 8 +++++++- tests/css_templates/api_tests.py | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx index afb53d30a6cf0..d0ca8c5c62b19 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx @@ -58,7 +58,7 @@ const mockUser = { }; fetchMock.get(templatesInfoEndpoint, { - permissions: ['can_delete'], + permissions: ['can_write'], }); fetchMock.get(templatesEndpoint, { result: mocktemplates, diff --git a/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx b/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx index 5769fee058678..bcd6ebdac0d31 100644 --- a/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx +++ b/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx @@ -74,9 +74,9 @@ function CssTemplatesList({ setCurrentCssTemplate, ] = useState(null); - 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'); const [ templateCurrentlyDeleting, diff --git a/superset/css_templates/api.py b/superset/css_templates/api.py index 8e6f07ab3b5d4..f443b002c1e66 100644 --- a/superset/css_templates/api.py +++ b/superset/css_templates/api.py @@ -22,7 +22,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext -from superset.constants import RouteMethod +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.css_templates.commands.bulk_delete import BulkDeleteCssTemplateCommand from superset.css_templates.commands.exceptions import ( CssTemplateBulkDeleteFailedError, @@ -47,7 +47,9 @@ class CssTemplateRestApi(BaseSupersetModelRestApi): RouteMethod.RELATED, "bulk_delete", # not using RouteMethod since locally defined } - class_permission_name = "CssTemplateModelView" + class_permission_name = "CssTemplate" + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP + resource_name = "css_template" allow_browser_login = True diff --git a/superset/views/css_templates.py b/superset/views/css_templates.py index 99dd51f9f94c1..c998aabc2d411 100644 --- a/superset/views/css_templates.py +++ b/superset/views/css_templates.py @@ -20,7 +20,7 @@ from flask_babel import lazy_gettext as _ 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 import core as models from superset.typing import FlaskResponse from superset.views.base import DeleteMixin, SupersetModelView @@ -32,6 +32,9 @@ class CssTemplateModelView( # pylint: disable=too-many-ancestors datamodel = SQLAInterface(models.CssTemplate) include_route_methods = RouteMethod.CRUD_SET + class_permission_name = "CssTemplate" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP + list_title = _("CSS Templates") show_title = _("Show CSS Template") add_title = _("Add CSS Template") @@ -55,4 +58,7 @@ class CssTemplateAsyncModelView( # pylint: disable=too-many-ancestors CssTemplateModelView ): include_route_methods = {RouteMethod.API_READ} + class_permission_name = "CssTemplate" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP + list_columns = ["template_name", "css"] diff --git a/tests/css_templates/api_tests.py b/tests/css_templates/api_tests.py index 23a6c7559cfcb..f4e1f65f85945 100644 --- a/tests/css_templates/api_tests.py +++ b/tests/css_templates/api_tests.py @@ -159,6 +159,20 @@ def test_info_css_template(self): rv = self.get_assert_metric(uri, "info") assert rv.status_code == 200 + def test_info_security_css_template(self): + """ + CssTemplate API: Test info security + """ + self.login(username="admin") + params = {"keys": ["permissions"]} + uri = f"api/v1/css_template/_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_css_templates") def test_get_css_template(self): """ From 1021e466c78d51a6ce6c934cfdaa25ce118d512e Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 30 Nov 2020 22:26:11 +0000 Subject: [PATCH 2/5] fix security tests --- tests/security_tests.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/security_tests.py b/tests/security_tests.py index c8e3eeacb5504..c640518223332 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -47,6 +47,8 @@ ) from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice +NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery") + def get_perm_tuples(role_name): perm_set = set() @@ -611,18 +613,27 @@ def test_sqllab_gamma_user_schema_access_to_sqllab(self): self.logout() def assert_can_read(self, view_menu, permissions_set): - self.assertIn(("can_list", view_menu), permissions_set) + if view_menu in NEW_SECURITY_CONVERGE_VIEWS: + self.assertIn(("can_read", view_menu), permissions_set) + else: + self.assertIn(("can_list", view_menu), permissions_set) def assert_can_write(self, view_menu, permissions_set): - self.assertIn(("can_add", view_menu), permissions_set) - self.assertIn(("can_delete", view_menu), permissions_set) - self.assertIn(("can_edit", view_menu), permissions_set) + if view_menu in NEW_SECURITY_CONVERGE_VIEWS: + self.assertIn(("can_write", view_menu), permissions_set) + else: + self.assertIn(("can_add", view_menu), permissions_set) + self.assertIn(("can_delete", view_menu), permissions_set) + self.assertIn(("can_edit", view_menu), permissions_set) def assert_cannot_write(self, view_menu, permissions_set): - self.assertNotIn(("can_add", view_menu), permissions_set) - self.assertNotIn(("can_delete", view_menu), permissions_set) - self.assertNotIn(("can_edit", view_menu), permissions_set) - self.assertNotIn(("can_save", view_menu), permissions_set) + if view_menu in NEW_SECURITY_CONVERGE_VIEWS: + self.assertNotIn(("can_write", view_menu), permissions_set) + else: + self.assertNotIn(("can_add", view_menu), permissions_set) + self.assertNotIn(("can_delete", view_menu), permissions_set) + self.assertNotIn(("can_edit", view_menu), permissions_set) + self.assertNotIn(("can_save", view_menu), permissions_set) def assert_can_all(self, view_menu, permissions_set): self.assert_can_read(view_menu, permissions_set) @@ -661,7 +672,7 @@ def assert_can_gamma(self, perm_set): def assert_can_alpha(self, perm_set): self.assert_can_all("AnnotationLayerModelView", perm_set) - self.assert_can_all("CssTemplateModelView", perm_set) + self.assert_can_all("CssTemplate", perm_set) self.assert_can_all("TableModelView", perm_set) self.assert_can_read("QueryView", perm_set) self.assertIn(("can_import_dashboards", "Superset"), perm_set) From e7c23a12f87d79df9aa304e689d0d65b68071f5b Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 30 Nov 2020 23:40:13 +0000 Subject: [PATCH 3/5] fix JS test --- .../views/CRUD/csstemplates/CssTemplatesList_spec.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx index d0ca8c5c62b19..f293bf1eca15c 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx @@ -158,7 +158,7 @@ describe('CssTemplatesList', () => { }); 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(); }); From 5eda6d3e4e74af4b3603fb141cbc7f8c166099b9 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 1 Dec 2020 00:23:57 +0000 Subject: [PATCH 4/5] add migration --- ...9739cf9_security_converge_css_templates.py | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py diff --git a/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py b/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py new file mode 100644 index 0000000000000..dfab64b74926a --- /dev/null +++ b/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py @@ -0,0 +1,83 @@ +# 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 css templates + +Revision ID: 8ee129739cf9 +Revises: e38177dbf641 +Create Date: 2020-11-30 17:54:09.118630 + +""" + +# revision identifiers, used by Alembic. +revision = '8ee129739cf9' +down_revision = 'e38177dbf641' + + +from alembic import op +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset.migrations.shared.security_converge import ( + add_pvms, + get_reversed_new_pvms, + get_reversed_pvm_map, + migrate_roles, + Pvm, +) + + +NEW_PVMS = {"CssTemplate": ("can_read", "can_write",)} +PVM_MAP = { + Pvm("CssTemplateModelView", "can_list"): (Pvm("CssTemplate", "can_read"),), + Pvm("CssTemplateModelView", "can_show"): (Pvm("CssTemplate", "can_read"),), + Pvm("CssTemplateModelView", "can_add",): (Pvm("CssTemplate", "can_write"),), + Pvm("CssTemplateModelView", "can_edit",): (Pvm("CssTemplate", "can_write"),), + Pvm("CssTemplateModelView", "can_delete",): (Pvm("CssTemplate", "can_write"),), + Pvm("CssTemplateModelView", "muldelete",): (Pvm("CssTemplate", "can_write"),), + Pvm("CssTemplateModelView", "can_mulexport",): (Pvm("CssTemplate", "can_read"),), + Pvm("CssTemplateAsyncModelView", "can_list",): (Pvm("CssTemplate", "can_read"),), + Pvm("CssTemplateAsyncModelView", "muldelete",): (Pvm("CssTemplate", "can_write"),), +} + + +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 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 permissions: {ex}") + session.rollback() + pass From c4d84f349eb3f53ae2ac2897064bc57766a8e8fc Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 1 Dec 2020 00:35:11 +0000 Subject: [PATCH 5/5] black and fix migration --- .../8ee129739cf9_security_converge_css_templates.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py b/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py index dfab64b74926a..cc641006d6afb 100644 --- a/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py +++ b/superset/migrations/versions/8ee129739cf9_security_converge_css_templates.py @@ -23,8 +23,8 @@ """ # revision identifiers, used by Alembic. -revision = '8ee129739cf9' -down_revision = 'e38177dbf641' +revision = "8ee129739cf9" +down_revision = "e38177dbf641" from alembic import op @@ -39,7 +39,6 @@ Pvm, ) - NEW_PVMS = {"CssTemplate": ("can_read", "can_write",)} PVM_MAP = { Pvm("CssTemplateModelView", "can_list"): (Pvm("CssTemplate", "can_read"),), @@ -48,7 +47,6 @@ Pvm("CssTemplateModelView", "can_edit",): (Pvm("CssTemplate", "can_write"),), Pvm("CssTemplateModelView", "can_delete",): (Pvm("CssTemplate", "can_write"),), Pvm("CssTemplateModelView", "muldelete",): (Pvm("CssTemplate", "can_write"),), - Pvm("CssTemplateModelView", "can_mulexport",): (Pvm("CssTemplate", "can_read"),), Pvm("CssTemplateAsyncModelView", "can_list",): (Pvm("CssTemplate", "can_read"),), Pvm("CssTemplateAsyncModelView", "muldelete",): (Pvm("CssTemplate", "can_write"),), }