diff --git a/superset-frontend/spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx index d919bc033..a816a6091 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx @@ -58,7 +58,7 @@ const mockUser = { }; fetchMock.get(datasetsInfoEndpoint, { - permissions: ['can_list', 'can_edit', 'can_add', 'can_delete'], + permissions: ['can_read', 'can_write'], }); fetchMock.get(datasetsOwnersEndpoint, { result: [], diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 565541eff..752fb44cb 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -141,10 +141,10 @@ const DatasetList: FunctionComponent = ({ refreshData(); }; - const canEdit = hasPerm('can_edit'); - const canDelete = hasPerm('can_delete'); - const canCreate = hasPerm('can_add'); - const canExport = hasPerm('can_mulexport'); + const canEdit = hasPerm('can_write'); + const canDelete = hasPerm('can_write'); + const canCreate = hasPerm('can_write'); + const canExport = hasPerm('can_read'); const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 23efa86e1..a9ae564e3 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -33,7 +33,7 @@ from wtforms.validators import Regexp from superset import app, db, is_feature_enabled from superset.connectors.base.views import DatasourceModelView from superset.connectors.sqla import models -from superset.constants import RouteMethod +from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.typing import FlaskResponse from superset.utils import core as utils from superset.views.base import ( @@ -55,6 +55,8 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors ): datamodel = SQLAInterface(models.TableColumn) # TODO TODO, review need for this on related_views + class_permission_name = "Dataset" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP include_route_methods = RouteMethod.RELATED_VIEW_SET | RouteMethod.API_SET list_title = _("Columns") @@ -174,6 +176,8 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors CompactCRUDMixin, SupersetModelView ): datamodel = SQLAInterface(models.SqlMetric) + class_permission_name = "Dataset" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP include_route_methods = RouteMethod.RELATED_VIEW_SET | RouteMethod.API_SET list_title = _("Metrics") @@ -327,6 +331,8 @@ class TableModelView( # pylint: disable=too-many-ancestors DatasourceModelView, DeleteMixin, YamlExportMixin ): datamodel = SQLAInterface(models.SqlaTable) + class_permission_name = "Dataset" + method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP include_route_methods = RouteMethod.CRUD_SET list_title = _("Tables") diff --git a/superset/constants.py b/superset/constants.py index 16cd4e64a..dc56939dd 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -82,6 +82,7 @@ MODEL_VIEW_RW_METHOD_PERMISSION_MAP = { "list": "read", "muldelete": "write", "show": "read", + "yaml_export": "read", } MODEL_API_RW_METHOD_PERMISSION_MAP = { @@ -95,8 +96,10 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = { "post": "write", "put": "write", "related": "read", - "favorite_status": "write", + "refresh": "read", + "related_objects": "read", "import_": "write", + "favorite_status": "write", "cache_screenshot": "read", "screenshot": "read", "data": "read", diff --git a/superset/datasets/api.py b/superset/datasets/api.py index a9a210e6a..68cfdbb85 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -33,7 +33,7 @@ from superset import event_logger, is_feature_enabled from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.v1.utils import remove_root from superset.connectors.sqla.models import SqlaTable -from superset.constants import RouteMethod +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.databases.filters import DatabaseFilter from superset.datasets.commands.bulk_delete import BulkDeleteDatasetCommand from superset.datasets.commands.create import CreateDatasetCommand @@ -79,7 +79,8 @@ class DatasetRestApi(BaseSupersetModelRestApi): resource_name = "dataset" allow_browser_login = True - class_permission_name = "TableModelView" + class_permission_name = "Dataset" + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | { RouteMethod.EXPORT, RouteMethod.IMPORT, diff --git a/superset/migrations/versions/45731db65d9c_security_converge_datasets.py b/superset/migrations/versions/45731db65d9c_security_converge_datasets.py new file mode 100644 index 000000000..5b4670857 --- /dev/null +++ b/superset/migrations/versions/45731db65d9c_security_converge_datasets.py @@ -0,0 +1,91 @@ +# 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 datasets + +Revision ID: 45731db65d9c +Revises: ccb74baaa89b +Create Date: 2020-12-10 15:05:44.928020 + +""" + +# revision identifiers, used by Alembic. +revision = "45731db65d9c" +down_revision = "c25cb2c78727" + +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 = {"Dataset": ("can_read", "can_write",)} +PVM_MAP = { + Pvm("SqlMetricInlineView", "can_add"): (Pvm("Dataset", "can_write"),), + Pvm("SqlMetricInlineView", "can_delete"): (Pvm("Dataset", "can_write"),), + Pvm("SqlMetricInlineView", "can_edit"): (Pvm("Dataset", "can_write"),), + Pvm("SqlMetricInlineView", "can_list"): (Pvm("Dataset", "can_read"),), + Pvm("SqlMetricInlineView", "can_show"): (Pvm("Dataset", "can_read"),), + Pvm("TableColumnInlineView", "can_add"): (Pvm("Dataset", "can_write"),), + Pvm("TableColumnInlineView", "can_delete"): (Pvm("Dataset", "can_write"),), + Pvm("TableColumnInlineView", "can_edit"): (Pvm("Dataset", "can_write"),), + Pvm("TableColumnInlineView", "can_list"): (Pvm("Dataset", "can_read"),), + Pvm("TableColumnInlineView", "can_show"): (Pvm("Dataset", "can_read"),), + Pvm("TableModelView", "can_add",): (Pvm("Dataset", "can_write"),), + Pvm("TableModelView", "can_delete",): (Pvm("Dataset", "can_write"),), + Pvm("TableModelView", "can_edit",): (Pvm("Dataset", "can_write"),), + Pvm("TableModelView", "can_list"): (Pvm("Dataset", "can_read"),), + Pvm("TableModelView", "can_mulexport"): (Pvm("Dataset", "can_read"),), + Pvm("TableModelView", "can_show"): (Pvm("Dataset", "can_read"),), + Pvm("TableModelView", "muldelete",): (Pvm("Dataset", "can_write"),), + Pvm("TableModelView", "refresh",): (Pvm("Dataset", "can_write"),), + Pvm("TableModelView", "yaml_export",): (Pvm("Dataset", "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 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 diff --git a/superset/security/manager.py b/superset/security/manager.py index 3bc097433..047448799 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -120,9 +120,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods } GAMMA_READ_ONLY_MODEL_VIEWS = { - "SqlMetricInlineView", - "TableColumnInlineView", - "TableModelView", + "Dataset", "DruidColumnInlineView", "DruidDatasourceModelView", "DruidMetricInlineView", @@ -160,7 +158,13 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "all_query_access", } - READ_ONLY_PERMISSION = {"can_show", "can_list", "can_get", "can_external_metadata"} + READ_ONLY_PERMISSION = { + "can_show", + "can_list", + "can_get", + "can_external_metadata", + "can_read", + } ALPHA_ONLY_PERMISSIONS = { "muldelete", diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py index 65ea2c327..02e2266b7 100644 --- a/tests/datasets/api_tests.py +++ b/tests/datasets/api_tests.py @@ -353,6 +353,20 @@ class TestDatasetApi(SupersetTestCase): rv = self.get_assert_metric(uri, "info") assert rv.status_code == 200 + def test_info_security_dataset(self): + """ + Dataset API: Test info security + """ + self.login(username="admin") + params = {"keys": ["permissions"]} + uri = f"api/v1/dataset/_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 + def test_create_dataset_item(self): """ Dataset API: Test create dataset item @@ -455,23 +469,23 @@ class TestDatasetApi(SupersetTestCase): expected_result = {"message": {"owners": ["Owners are invalid"]}} assert data == expected_result + @pytest.mark.usefixtures("load_energy_table_with_slice") def test_create_dataset_validate_uniqueness(self): """ Dataset API: Test create dataset validate table uniqueness """ - example_db = get_example_database() + energy_usage_ds = self.get_energy_usage_dataset() self.login(username="admin") table_data = { - "database": example_db.id, - "schema": "", - "table_name": "birth_names", + "database": energy_usage_ds.database_id, + "table_name": energy_usage_ds.table_name, } uri = "api/v1/dataset/" rv = self.post_assert_metric(uri, table_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == { - "message": {"table_name": ["Datasource birth_names already exists"]} + "message": {"table_name": ["Datasource energy_usage already exists"]} } def test_create_dataset_same_name_different_schema(self): @@ -1104,7 +1118,7 @@ class TestDatasetApi(SupersetTestCase): self.login(username="gamma") rv = self.client.get(uri) - assert rv.status_code == 401 + assert rv.status_code == 404 @patch.dict( "superset.extensions.feature_flag_manager._feature_flags", @@ -1165,8 +1179,8 @@ class TestDatasetApi(SupersetTestCase): self.login(username="gamma") rv = self.client.get(uri) - - assert rv.status_code == 401 + # gamma users by default do not have access to this dataset + assert rv.status_code == 404 def test_get_dataset_related_objects(self): """ diff --git a/tests/security_tests.py b/tests/security_tests.py index 2836b4850..5625fd963 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -48,7 +48,13 @@ 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", "Annotation") +NEW_SECURITY_CONVERGE_VIEWS = ( + "Annotation", + "Dataset", + "CssTemplate", + "Chart", + "SavedQuery", +) def get_perm_tuples(role_name): @@ -644,7 +650,7 @@ class TestRolePermission(SupersetTestCase): self.assertIn(("menu_access", view_menu), permissions_set) def assert_can_gamma(self, perm_set): - self.assert_can_read("TableModelView", perm_set) + self.assert_can_read("Dataset", perm_set) # make sure that user can create slices and dashboards self.assert_can_all("Chart", perm_set) @@ -674,7 +680,7 @@ class TestRolePermission(SupersetTestCase): def assert_can_alpha(self, 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_all("Dataset", perm_set) self.assert_can_read("QueryView", perm_set) self.assertIn(("can_import_dashboards", "Superset"), perm_set) self.assertIn(("can_this_form_post", "CsvToDatabaseView"), perm_set) @@ -711,7 +717,7 @@ class TestRolePermission(SupersetTestCase): def test_is_admin_only(self): self.assertFalse( security_manager._is_admin_only( - security_manager.find_permission_view_menu("can_list", "TableModelView") + security_manager.find_permission_view_menu("can_read", "Dataset") ) ) self.assertFalse( @@ -759,15 +765,13 @@ class TestRolePermission(SupersetTestCase): def test_is_alpha_only(self): self.assertFalse( security_manager._is_alpha_only( - security_manager.find_permission_view_menu("can_list", "TableModelView") + security_manager.find_permission_view_menu("can_read", "Dataset") ) ) self.assertTrue( security_manager._is_alpha_only( - security_manager.find_permission_view_menu( - "muldelete", "TableModelView" - ) + security_manager.find_permission_view_menu("can_write", "Dataset") ) ) self.assertTrue( @@ -788,7 +792,7 @@ class TestRolePermission(SupersetTestCase): def test_is_gamma_pvm(self): self.assertTrue( security_manager._is_gamma_pvm( - security_manager.find_permission_view_menu("can_list", "TableModelView") + security_manager.find_permission_view_menu("can_read", "Dataset") ) ) @@ -837,7 +841,7 @@ class TestRolePermission(SupersetTestCase): gamma_perm_set.add((perm.permission.name, perm.view_menu.name)) # check read only perms - self.assert_can_read("TableModelView", gamma_perm_set) + self.assert_can_read("Dataset", gamma_perm_set) # make sure that user can create slices and dashboards self.assert_can_all("Chart", gamma_perm_set)