diff --git a/superset/commands/chart/fave.py b/superset/commands/chart/fave.py index c39873cbeed..f1bf190bc4f 100644 --- a/superset/commands/chart/fave.py +++ b/superset/commands/chart/fave.py @@ -17,15 +17,12 @@ import logging from functools import partial -from superset import security_manager from superset.commands.base import BaseCommand from superset.commands.chart.exceptions import ( ChartFaveError, - ChartForbiddenError, ChartNotFoundError, ) from superset.daos.chart import ChartDAO -from superset.exceptions import SupersetSecurityException from superset.models.slice import Slice from superset.utils.decorators import on_error, transaction @@ -48,9 +45,4 @@ class AddFavoriteChartCommand(BaseCommand): if not chart: raise ChartNotFoundError() - try: - security_manager.raise_for_ownership(chart) - except SupersetSecurityException as ex: - raise ChartForbiddenError() from ex - self._chart = chart diff --git a/superset/commands/chart/unfave.py b/superset/commands/chart/unfave.py index e6d0cace39c..0346b433e38 100644 --- a/superset/commands/chart/unfave.py +++ b/superset/commands/chart/unfave.py @@ -17,15 +17,12 @@ import logging from functools import partial -from superset import security_manager from superset.commands.base import BaseCommand from superset.commands.chart.exceptions import ( - ChartForbiddenError, ChartNotFoundError, ChartUnfaveError, ) from superset.daos.chart import ChartDAO -from superset.exceptions import SupersetSecurityException from superset.models.slice import Slice from superset.utils.decorators import on_error, transaction @@ -48,9 +45,4 @@ class DelFavoriteChartCommand(BaseCommand): if not chart: raise ChartNotFoundError() - try: - security_manager.raise_for_ownership(chart) - except SupersetSecurityException as ex: - raise ChartForbiddenError() from ex - self._chart = chart diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index 7b0824972ed..b5fa05b7264 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -25,7 +25,6 @@ from flask import g # noqa: F401 from superset import db, security_manager from superset.commands.chart.create import CreateChartCommand from superset.commands.chart.exceptions import ( - ChartForbiddenError, ChartNotFoundError, WarmUpCacheChartNotFoundError, ) @@ -668,8 +667,8 @@ class TestFavoriteChartCommand(SupersetTestCase): @pytest.mark.usefixtures("load_energy_table_with_slice") @patch("superset.daos.base.BaseDAO.find_by_id") - def test_fave_unfave_chart_command_forbidden(self, mock_find_by_id): - """Test that faving / unfaving raises an exception for a chart the user doesn't own""" # noqa: E501 + def test_fave_unfave_chart_command_non_owner(self, mock_find_by_id): + """Test that faving / unfaving a chart the user doesn't own works properly""" # noqa: E501 with self.client.application.test_request_context(): example_chart = db.session.query(Slice).all()[0] mock_find_by_id.return_value = example_chart @@ -678,8 +677,12 @@ class TestFavoriteChartCommand(SupersetTestCase): assert example_chart is not None with override_user(security_manager.find_user("gamma")): - with self.assertRaises(ChartForbiddenError): # noqa: PT027 - AddFavoriteChartCommand(example_chart.id).run() + AddFavoriteChartCommand(example_chart.id).run() + ids = ChartDAO.favorited_ids([example_chart]) - with self.assertRaises(ChartForbiddenError): # noqa: PT027 - DelFavoriteChartCommand(example_chart.id).run() + assert example_chart.id in ids + + DelFavoriteChartCommand(example_chart.id).run() + ids = ChartDAO.favorited_ids([example_chart]) + + assert example_chart.id not in ids