diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index 9828ceab7e4..3f5ae5f17c0 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -1016,17 +1016,18 @@ const DatasetList: FunctionComponent = ({ ); } - Promise.all(promises).then( - () => { - refreshData(); + Promise.allSettled(promises).then(results => { + const failures = results.filter(r => r.status === 'rejected'); + // Always refresh so the list reflects whatever actually got deleted. + refreshData(); + if (failures.length === 0) { addSuccessToast(t('Deleted %s item(s)', datasetsToDelete.length)); - }, - createErrorHandler(errMsg => + } else { addDangerToast( - t('There was an issue deleting the selected datasets: %s', errMsg), - ), - ), - ); + t('There was an issue deleting the selected items'), + ); + } + }); }; const handleDatasetDuplicate = (newDatasetName: string) => { diff --git a/superset/commands/semantic_layer/delete.py b/superset/commands/semantic_layer/delete.py index bdc13502f4d..0093be42514 100644 --- a/superset/commands/semantic_layer/delete.py +++ b/superset/commands/semantic_layer/delete.py @@ -21,14 +21,17 @@ from functools import partial from sqlalchemy.exc import SQLAlchemyError +from superset import security_manager from superset.commands.base import BaseCommand from superset.commands.semantic_layer.exceptions import ( SemanticLayerDeleteFailedError, SemanticLayerNotFoundError, SemanticViewDeleteFailedError, + SemanticViewForbiddenError, SemanticViewNotFoundError, ) from superset.daos.semantic_layer import SemanticLayerDAO, SemanticViewDAO +from superset.exceptions import SupersetSecurityException from superset.semantic_layers.models import SemanticLayer, SemanticView from superset.utils.decorators import on_error, transaction @@ -79,6 +82,10 @@ class DeleteSemanticViewCommand(BaseCommand): self._model = SemanticViewDAO.find_by_id(self._pk, id_column="id") if not self._model: raise SemanticViewNotFoundError() + try: + security_manager.raise_for_ownership(self._model) + except SupersetSecurityException as ex: + raise SemanticViewForbiddenError() from ex class BulkDeleteSemanticViewCommand(BaseCommand): @@ -101,3 +108,8 @@ class BulkDeleteSemanticViewCommand(BaseCommand): self._models = SemanticViewDAO.find_by_ids(self._model_ids, id_column="id") if len(self._models) != len(self._model_ids): raise SemanticViewNotFoundError() + for model in self._models: + try: + security_manager.raise_for_ownership(model) + except SupersetSecurityException as ex: + raise SemanticViewForbiddenError() from ex diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index b1935065b27..b5d5bdc6bf2 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -362,6 +362,8 @@ class SemanticViewRestApi(BaseSupersetModelRestApi): return self.response(200, message="OK") except SemanticViewNotFoundError: return self.response_404() + except SemanticViewForbiddenError: + return self.response_403() except SemanticViewDeleteFailedError as ex: logger.error( "Error deleting semantic view: %s", @@ -422,6 +424,8 @@ class SemanticViewRestApi(BaseSupersetModelRestApi): ) except SemanticViewNotFoundError: return self.response_404() + except SemanticViewForbiddenError: + return self.response_403() except SemanticViewDeleteFailedError as ex: logger.error( "Error bulk deleting semantic views: %s", diff --git a/tests/unit_tests/commands/semantic_layer/delete_test.py b/tests/unit_tests/commands/semantic_layer/delete_test.py index 36f39895eb1..e5b6e5e8549 100644 --- a/tests/unit_tests/commands/semantic_layer/delete_test.py +++ b/tests/unit_tests/commands/semantic_layer/delete_test.py @@ -59,6 +59,11 @@ def test_delete_semantic_view_success(mocker: MockerFixture) -> None: ) dao.find_by_id.return_value = mock_model + # Admin is owner of everything — no exception raised + mocker.patch( + "superset.commands.semantic_layer.delete.security_manager" + ).raise_for_ownership.return_value = None + from superset.commands.semantic_layer.delete import DeleteSemanticViewCommand DeleteSemanticViewCommand(42).run() @@ -67,6 +72,26 @@ def test_delete_semantic_view_success(mocker: MockerFixture) -> None: dao.delete.assert_called_once_with([mock_model]) +def test_delete_semantic_view_forbidden(mocker: MockerFixture) -> None: + """Test that SemanticViewForbiddenError is raised for non-owners.""" + from superset.commands.semantic_layer.delete import DeleteSemanticViewCommand + from superset.commands.semantic_layer.exceptions import SemanticViewForbiddenError + from superset.exceptions import SupersetSecurityException + + dao = mocker.patch( + "superset.commands.semantic_layer.delete.SemanticViewDAO", + ) + dao.find_by_id.return_value = MagicMock() + + mocker.patch( + "superset.security_manager.raise_for_ownership", + side_effect=SupersetSecurityException(MagicMock()), + ) + + with pytest.raises(SemanticViewForbiddenError): + DeleteSemanticViewCommand(42).run() + + def test_delete_semantic_view_not_found(mocker: MockerFixture) -> None: """Test that SemanticViewNotFoundError is raised when view is missing.""" dao = mocker.patch( @@ -92,6 +117,10 @@ def test_bulk_delete_semantic_view_success(mocker: MockerFixture) -> None: ) dao.find_by_ids.return_value = mock_models + mocker.patch( + "superset.commands.semantic_layer.delete.security_manager" + ).raise_for_ownership.return_value = None + from superset.commands.semantic_layer.delete import BulkDeleteSemanticViewCommand BulkDeleteSemanticViewCommand([1, 2]).run() @@ -100,6 +129,26 @@ def test_bulk_delete_semantic_view_success(mocker: MockerFixture) -> None: dao.delete.assert_called_once_with(mock_models) +def test_bulk_delete_semantic_view_forbidden(mocker: MockerFixture) -> None: + """Test that SemanticViewForbiddenError is raised for non-owners.""" + from superset.commands.semantic_layer.delete import BulkDeleteSemanticViewCommand + from superset.commands.semantic_layer.exceptions import SemanticViewForbiddenError + from superset.exceptions import SupersetSecurityException + + dao = mocker.patch( + "superset.commands.semantic_layer.delete.SemanticViewDAO", + ) + dao.find_by_ids.return_value = [MagicMock(), MagicMock()] + + mocker.patch( + "superset.security_manager.raise_for_ownership", + side_effect=SupersetSecurityException(MagicMock()), + ) + + with pytest.raises(SemanticViewForbiddenError): + BulkDeleteSemanticViewCommand([1, 2]).run() + + def test_bulk_delete_semantic_view_not_found(mocker: MockerFixture) -> None: """Test that SemanticViewNotFoundError is raised when any id is missing.""" dao = mocker.patch( diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index 5dfcb6e25e6..f7acfb47e34 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -1907,7 +1907,7 @@ def test_get_views_exception( ) assert response.status_code == 400 - assert "Connection failed" in response.json["message"] + assert "Unable to fetch semantic views" in response.json["message"] @SEMANTIC_LAYERS_APP