diff --git a/superset/charts/api.py b/superset/charts/api.py index 57a812fb74aba..64177950b8437 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -66,7 +66,9 @@ DashboardsForbiddenError, ) from superset.commands.chart.export import ExportChartsCommand +from superset.commands.chart.fave import AddFavoriteChartCommand from superset.commands.chart.importers.dispatcher import ImportChartsCommand +from superset.commands.chart.unfave import DelFavoriteChartCommand from superset.commands.chart.update import UpdateChartCommand from superset.commands.chart.warm_up_cache import ChartWarmUpCacheCommand from superset.commands.exceptions import CommandException, TagForbiddenError @@ -898,11 +900,13 @@ def add_favorite(self, pk: int) -> Response: 500: $ref: '#/components/responses/500' """ - chart = ChartDAO.find_by_id(pk) - if not chart: + try: + AddFavoriteChartCommand(pk).run() + except ChartNotFoundError: return self.response_404() + except ChartForbiddenError: + return self.response_403() - ChartDAO.add_favorite(chart) return self.response(200, result="OK") @expose("//favorites/", methods=("DELETE",)) @@ -941,11 +945,13 @@ def remove_favorite(self, pk: int) -> Response: 500: $ref: '#/components/responses/500' """ - chart = ChartDAO.find_by_id(pk) - if not chart: - return self.response_404() + try: + DelFavoriteChartCommand(pk).run() + except ChartNotFoundError: + self.response_404() + except ChartForbiddenError: + self.response_403() - ChartDAO.remove_favorite(chart) return self.response(200, result="OK") @expose("/warm_up_cache", methods=("PUT",)) diff --git a/superset/commands/chart/exceptions.py b/superset/commands/chart/exceptions.py index 00877aa803070..72ef71f466e8e 100644 --- a/superset/commands/chart/exceptions.py +++ b/superset/commands/chart/exceptions.py @@ -154,3 +154,11 @@ class DashboardsForbiddenError(ForbiddenError): class WarmUpCacheChartNotFoundError(CommandException): status = 404 message = _("Chart not found") + + +class ChartFaveError(CommandException): + message = _("Error faving chart") + + +class ChartUnfaveError(CommandException): + message = _("Error unfaving chart") diff --git a/superset/commands/chart/fave.py b/superset/commands/chart/fave.py new file mode 100644 index 0000000000000..d45d1694761c3 --- /dev/null +++ b/superset/commands/chart/fave.py @@ -0,0 +1,57 @@ +# 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. +import logging +from functools import partial + +from requests_cache import Optional + +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 + +logger = logging.getLogger(__name__) + + +class AddFavoriteChartCommand(BaseCommand): + def __init__(self, chart_id: int) -> None: + self._chart_id = chart_id + self._chart: Optional[Slice] = None + + @transaction(on_error=partial(on_error, reraise=ChartFaveError)) + def run(self) -> None: + self.validate() + return ChartDAO.add_favorite(self._chart) + + def validate(self) -> None: + chart = ChartDAO.find_by_id(self._chart_id) + 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 new file mode 100644 index 0000000000000..d19d2b2769993 --- /dev/null +++ b/superset/commands/chart/unfave.py @@ -0,0 +1,57 @@ +# 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. +import logging +from functools import partial + +from requests_cache import Optional + +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 + +logger = logging.getLogger(__name__) + + +class DelFavoriteChartCommand(BaseCommand): + def __init__(self, chart_id: int) -> None: + self._chart_id = chart_id + self._chart: Optional[Slice] = None + + @transaction(on_error=partial(on_error, reraise=ChartUnfaveError)) + def run(self) -> None: + self.validate() + return ChartDAO.remove_favorite(self._chart) + + def validate(self) -> None: + chart = ChartDAO.find_by_id(self._chart_id) + 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/dashboard/exceptions.py b/superset/commands/dashboard/exceptions.py index 91a4ec6832222..9281119b320bf 100644 --- a/superset/commands/dashboard/exceptions.py +++ b/superset/commands/dashboard/exceptions.py @@ -84,3 +84,11 @@ class DashboardAccessDeniedError(ForbiddenError): class DashboardCopyError(CommandInvalidError): message = _("Dashboard cannot be copied due to invalid parameters.") + + +class DashboardFaveError(CommandInvalidError): + message = _("Dashboard cannot be favorited.") + + +class DashboardUnfaveError(CommandInvalidError): + message = _("Dashboard cannot be unfavorited.") diff --git a/superset/commands/dashboard/fave.py b/superset/commands/dashboard/fave.py new file mode 100644 index 0000000000000..e3050c729dbec --- /dev/null +++ b/superset/commands/dashboard/fave.py @@ -0,0 +1,46 @@ +# 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. +import logging +from functools import partial + +from requests_cache import Optional + +from superset.commands.base import BaseCommand +from superset.commands.dashboard.exceptions import ( + DashboardFaveError, +) +from superset.daos.dashboard import DashboardDAO +from superset.models.dashboard import Dashboard +from superset.utils.decorators import on_error, transaction + +logger = logging.getLogger(__name__) + + +class AddFavoriteDashboardCommand(BaseCommand): + def __init__(self, dashboard_id: int) -> None: + self._dashboard_id = dashboard_id + self._dashboard: Optional[Dashboard] = None + + @transaction(on_error=partial(on_error, reraise=DashboardFaveError)) + def run(self) -> None: + self.validate() + return DashboardDAO.add_favorite(self._dashboard) + + def validate(self) -> None: + # Raises DashboardNotFoundError or DashboardAccessDeniedError + dashboard = DashboardDAO.get_by_id_or_slug(self._dashboard_id) + self._dashboard = dashboard diff --git a/superset/commands/dashboard/unfave.py b/superset/commands/dashboard/unfave.py new file mode 100644 index 0000000000000..811a2cdd1e68d --- /dev/null +++ b/superset/commands/dashboard/unfave.py @@ -0,0 +1,46 @@ +# 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. +import logging +from functools import partial + +from requests_cache import Optional + +from superset.commands.base import BaseCommand +from superset.commands.dashboard.exceptions import ( + DashboardUnfaveError, +) +from superset.daos.dashboard import DashboardDAO +from superset.models.dashboard import Dashboard +from superset.utils.decorators import on_error, transaction + +logger = logging.getLogger(__name__) + + +class DelFavoriteDashboardCommand(BaseCommand): + def __init__(self, dashboard_id: int) -> None: + self._dashboard_id = dashboard_id + self._dashboard: Optional[Dashboard] = None + + @transaction(on_error=partial(on_error, reraise=DashboardUnfaveError)) + def run(self) -> None: + self.validate() + return DashboardDAO.remove_favorite(self._dashboard) + + def validate(self) -> None: + # Raises DashboardNotFoundError or DashboardAccessDeniedError + dashboard = DashboardDAO.get_by_id_or_slug(self._dashboard_id) + self._dashboard = dashboard diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 43d4ecd8e8b35..9076303b20cf1 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -51,8 +51,10 @@ DashboardUpdateFailedError, ) from superset.commands.dashboard.export import ExportDashboardsCommand +from superset.commands.dashboard.fave import AddFavoriteDashboardCommand from superset.commands.dashboard.importers.dispatcher import ImportDashboardsCommand from superset.commands.dashboard.permalink.create import CreateDashboardPermalinkCommand +from superset.commands.dashboard.unfave import DelFavoriteDashboardCommand from superset.commands.dashboard.update import UpdateDashboardCommand from superset.commands.exceptions import TagForbiddenError from superset.commands.importers.exceptions import NoValidFilesFoundError @@ -1217,11 +1219,14 @@ def add_favorite(self, pk: int) -> Response: 500: $ref: '#/components/responses/500' """ - dashboard = DashboardDAO.find_by_id(pk) - if not dashboard: + try: + AddFavoriteDashboardCommand(pk).run() + + except DashboardNotFoundError: return self.response_404() + except DashboardAccessDeniedError: + return self.response_403() - DashboardDAO.add_favorite(dashboard) return self.response(200, result="OK") @expose("//favorites/", methods=("DELETE",)) @@ -1260,11 +1265,13 @@ def remove_favorite(self, pk: int) -> Response: 500: $ref: '#/components/responses/500' """ - dashboard = DashboardDAO.find_by_id(pk) - if not dashboard: + try: + DelFavoriteDashboardCommand(pk).run() + except DashboardNotFoundError: return self.response_404() + except DashboardAccessDeniedError: + return self.response_403() - DashboardDAO.remove_favorite(dashboard) return self.response(200, result="OK") @expose("/import/", methods=("POST",)) diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index d8522473d09f2..950c7cbc888d5 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -23,19 +23,24 @@ from superset import db, security_manager from superset.commands.chart.create import CreateChartCommand from superset.commands.chart.exceptions import ( + ChartForbiddenError, ChartNotFoundError, WarmUpCacheChartNotFoundError, ) from superset.commands.chart.export import ExportChartsCommand +from superset.commands.chart.fave import AddFavoriteChartCommand from superset.commands.chart.importers.v1 import ImportChartsCommand +from superset.commands.chart.unfave import DelFavoriteChartCommand from superset.commands.chart.update import UpdateChartCommand from superset.commands.chart.warm_up_cache import ChartWarmUpCacheCommand from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.exceptions import IncorrectVersionError from superset.connectors.sqla.models import SqlaTable +from superset.daos.chart import ChartDAO from superset.models.core import Database from superset.models.slice import Slice from superset.utils import json +from superset.utils.core import override_user from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, # noqa: F401 @@ -428,3 +433,58 @@ def test_warm_up_cache(self): self.assertEqual( result, {"chart_id": slc.id, "viz_error": None, "viz_status": "success"} ) + + +class TestFavoriteChartCommand(SupersetTestCase): + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_fave_unfave_chart_command(self): + """Test that a user can fave/unfave a chart""" + with self.client.application.test_request_context(): + example_chart = db.session.query(Slice).all()[0] + + # Assert that the chart exists + assert example_chart is not None + + with override_user(security_manager.find_user("admin")): + AddFavoriteChartCommand(example_chart.id).run() + + # Assert that the dashboard was faved + ids = ChartDAO.favorited_ids([example_chart]) + assert example_chart.id in ids + + DelFavoriteChartCommand(example_chart.id).run() + + # Assert that the chart was unfaved + ids = ChartDAO.favorited_ids([example_chart]) + assert example_chart.id not in ids + + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_fave_unfave_chart_command_not_found(self): + """Test that faving / unfaving a non-existing chart raises an exception""" + with self.client.application.test_request_context(): + example_chart_id = 1234 + + with override_user(security_manager.find_user("admin")): + with self.assertRaises(ChartNotFoundError): + AddFavoriteChartCommand(example_chart_id).run() + + with self.assertRaises(ChartNotFoundError): + DelFavoriteChartCommand(example_chart_id).run() + + @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""" + with self.client.application.test_request_context(): + example_chart = db.session.query(Slice).all()[0] + mock_find_by_id.return_value = example_chart + + # Assert that the chart exists + assert example_chart is not None + + with override_user(security_manager.find_user("gamma")): + with self.assertRaises(ChartForbiddenError): + AddFavoriteChartCommand(example_chart.id).run() + + with self.assertRaises(ChartForbiddenError): + DelFavoriteChartCommand(example_chart.id).run() diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py index f38774a1b4ff9..451d924cd6c6e 100644 --- a/tests/integration_tests/dashboards/commands_tests.py +++ b/tests/integration_tests/dashboards/commands_tests.py @@ -25,6 +25,7 @@ from superset.commands.dashboard.copy import CopyDashboardCommand from superset.commands.dashboard.delete import DeleteEmbeddedDashboardCommand from superset.commands.dashboard.exceptions import ( + DashboardAccessDeniedError, DashboardForbiddenError, DashboardInvalidError, DashboardNotFoundError, @@ -34,10 +35,13 @@ ExportDashboardsCommand, get_default_position, ) +from superset.commands.dashboard.fave import AddFavoriteDashboardCommand from superset.commands.dashboard.importers import v0, v1 +from superset.commands.dashboard.unfave import DelFavoriteDashboardCommand from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.exceptions import IncorrectVersionError from superset.connectors.sqla.models import SqlaTable +from superset.daos.dashboard import DashboardDAO from superset.models.core import Database from superset.models.dashboard import Dashboard from superset.models.embedded_dashboard import EmbeddedDashboard @@ -759,3 +763,67 @@ def test_delete_embedded_dashboard_command(self): .one_or_none() ) assert deleted_embedded_dashboard is None + + +class TestFavoriteDashboardCommand(SupersetTestCase): + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_fave_unfave_dashboard_command(self): + """Test that a user can fave/unfave a dashboard""" + with self.client.application.test_request_context(): + example_dashboard = ( + db.session.query(Dashboard).filter_by(slug="world_health").one() + ) + + # Assert that the dashboard exists + assert example_dashboard is not None + + with override_user(security_manager.find_user("admin")): + with patch( + "superset.daos.dashboard.DashboardDAO.get_by_id_or_slug", + return_value=example_dashboard, + ): + AddFavoriteDashboardCommand(example_dashboard.id).run() + + # Assert that the dashboard was faved + ids = DashboardDAO.favorited_ids([example_dashboard]) + assert example_dashboard.id in ids + + DelFavoriteDashboardCommand(example_dashboard.id).run() + + # Assert that the dashboard was unfaved + ids = DashboardDAO.favorited_ids([example_dashboard]) + assert example_dashboard.id not in ids + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_fave_unfave_dashboard_command_not_found(self): + """Test that faving / unfaving a non-existing dashboard raises an exception""" + with self.client.application.test_request_context(): + example_dashboard_id = 1234 + + with override_user(security_manager.find_user("admin")): + with self.assertRaises(DashboardNotFoundError): + AddFavoriteDashboardCommand(example_dashboard_id).run() + + with self.assertRaises(DashboardNotFoundError): + DelFavoriteDashboardCommand(example_dashboard_id).run() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @patch("superset.models.dashboard.Dashboard.get") + def test_fave_unfave_dashboard_command_forbidden(self, mock_get): + """Test that faving / unfaving raises an exception for a dashboard the user doesn't own""" + with self.client.application.test_request_context(): + example_dashboard = ( + db.session.query(Dashboard).filter_by(slug="world_health").one() + ) + + mock_get.return_value = example_dashboard + + # Assert that the dashboard exists + assert example_dashboard is not None + + with override_user(security_manager.find_user("gamma")): + with self.assertRaises(DashboardAccessDeniedError): + AddFavoriteDashboardCommand(example_dashboard.uuid).run() + + with self.assertRaises(DashboardAccessDeniedError): + DelFavoriteDashboardCommand(example_dashboard.uuid).run()