diff --git a/superset/reports/api.py b/superset/reports/api.py index 5bdcccab87..6a2234f2aa 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -34,6 +34,7 @@ from superset.reports.commands.exceptions import ( ReportScheduleBulkDeleteFailedError, ReportScheduleCreateFailedError, ReportScheduleDeleteFailedError, + ReportScheduleForbiddenError, ReportScheduleInvalidError, ReportScheduleNotFoundError, ReportScheduleUpdateFailedError, @@ -192,6 +193,8 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): properties: message: type: string + 403: + $ref: '#/components/responses/403' 404: $ref: '#/components/responses/404' 422: @@ -204,6 +207,8 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): return self.response(200, message="OK") except ReportScheduleNotFoundError as ex: return self.response_404() + except ReportScheduleForbiddenError: + return self.response_403() except ReportScheduleDeleteFailedError as ex: logger.error( "Error deleting report schedule %s: %s", @@ -278,7 +283,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): @safe @statsd_metrics @permission_name("put") - def put(self, pk: int) -> Response: + def put(self, pk: int) -> Response: # pylint: disable=too-many-return-statements """Updates an Report Schedule --- put: @@ -313,6 +318,8 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): $ref: '#/components/responses/400' 401: $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' 404: $ref: '#/components/responses/404' 500: @@ -332,6 +339,8 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): return self.response_404() except ReportScheduleInvalidError as ex: return self.response_422(message=ex.normalized_messages()) + except ReportScheduleForbiddenError: + return self.response_403() except ReportScheduleUpdateFailedError as ex: logger.error( "Error updating report %s: %s", self.__class__.__name__, str(ex) @@ -368,6 +377,8 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): type: string 401: $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' 404: $ref: '#/components/responses/404' 422: @@ -388,5 +399,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): ) except ReportScheduleNotFoundError: return self.response_404() + except ReportScheduleForbiddenError: + return self.response_403() except ReportScheduleBulkDeleteFailedError as ex: return self.response_422(message=str(ex)) diff --git a/superset/reports/commands/bulk_delete.py b/superset/reports/commands/bulk_delete.py index b9dd572675..aa4898e9e5 100644 --- a/superset/reports/commands/bulk_delete.py +++ b/superset/reports/commands/bulk_delete.py @@ -21,12 +21,15 @@ from flask_appbuilder.security.sqla.models import User from superset.commands.base import BaseCommand from superset.dao.exceptions import DAODeleteFailedError +from superset.exceptions import SupersetSecurityException from superset.models.reports import ReportSchedule from superset.reports.commands.exceptions import ( ReportScheduleBulkDeleteFailedError, + ReportScheduleForbiddenError, ReportScheduleNotFoundError, ) from superset.reports.dao import ReportScheduleDAO +from superset.views.base import check_ownership logger = logging.getLogger(__name__) @@ -51,3 +54,10 @@ class BulkDeleteReportScheduleCommand(BaseCommand): self._models = ReportScheduleDAO.find_by_ids(self._model_ids) if not self._models or len(self._models) != len(self._model_ids): raise ReportScheduleNotFoundError() + + # Check ownership + for model in self._models: + try: + check_ownership(model) + except SupersetSecurityException: + raise ReportScheduleForbiddenError() diff --git a/superset/reports/commands/delete.py b/superset/reports/commands/delete.py index 79a0f4455b..8375f52c30 100644 --- a/superset/reports/commands/delete.py +++ b/superset/reports/commands/delete.py @@ -22,12 +22,15 @@ from flask_appbuilder.security.sqla.models import User from superset.commands.base import BaseCommand from superset.dao.exceptions import DAODeleteFailedError +from superset.exceptions import SupersetSecurityException from superset.models.reports import ReportSchedule from superset.reports.commands.exceptions import ( ReportScheduleDeleteFailedError, + ReportScheduleForbiddenError, ReportScheduleNotFoundError, ) from superset.reports.dao import ReportScheduleDAO +from superset.views.base import check_ownership logger = logging.getLogger(__name__) @@ -52,3 +55,9 @@ class DeleteReportScheduleCommand(BaseCommand): self._model = ReportScheduleDAO.find_by_id(self._model_id) if not self._model: raise ReportScheduleNotFoundError() + + # Check ownership + try: + check_ownership(self._model) + except SupersetSecurityException: + raise ReportScheduleForbiddenError() diff --git a/superset/reports/commands/exceptions.py b/superset/reports/commands/exceptions.py index b6fd375a91..a4aa9d9854 100644 --- a/superset/reports/commands/exceptions.py +++ b/superset/reports/commands/exceptions.py @@ -21,6 +21,7 @@ from superset.commands.exceptions import ( CommandInvalidError, CreateFailedError, DeleteFailedError, + ForbiddenError, ValidationError, ) @@ -172,3 +173,7 @@ class ReportScheduleStateNotFoundError(CommandException): class ReportScheduleUnexpectedError(CommandException): message = _("Report schedule unexpected error") + + +class ReportScheduleForbiddenError(ForbiddenError): + message = _("Changing this report is forbidden") diff --git a/superset/reports/commands/update.py b/superset/reports/commands/update.py index c8a9b6be37..d2cc9fd151 100644 --- a/superset/reports/commands/update.py +++ b/superset/reports/commands/update.py @@ -25,16 +25,19 @@ from marshmallow import ValidationError from superset.commands.utils import populate_owners from superset.dao.exceptions import DAOUpdateFailedError from superset.databases.dao import DatabaseDAO +from superset.exceptions import SupersetSecurityException from superset.models.reports import ReportSchedule, ReportScheduleType from superset.reports.commands.base import BaseReportScheduleCommand from superset.reports.commands.exceptions import ( DatabaseNotFoundValidationError, + ReportScheduleForbiddenError, ReportScheduleInvalidError, ReportScheduleNameUniquenessValidationError, ReportScheduleNotFoundError, ReportScheduleUpdateFailedError, ) from superset.reports.dao import ReportScheduleDAO +from superset.views.base import check_ownership logger = logging.getLogger(__name__) @@ -93,6 +96,12 @@ class UpdateReportScheduleCommand(BaseReportScheduleCommand): self._properties["validator_config_json"] ) + # Check ownership + try: + check_ownership(self._model) + except SupersetSecurityException: + raise ReportScheduleForbiddenError() + # Validate/Populate owner if owner_ids is None: owner_ids = [owner.id for owner in self._model.owners] diff --git a/tests/reports/api_tests.py b/tests/reports/api_tests.py index 6f374e6cd8..23736ab2a0 100644 --- a/tests/reports/api_tests.py +++ b/tests/reports/api_tests.py @@ -96,6 +96,26 @@ class TestReportSchedulesApi(SupersetTestCase): db.session.delete(report_schedule) db.session.commit() + @pytest.fixture() + def create_alpha_users(self): + with self.create_app().app_context(): + + users = [ + self.create_user( + "alpha1", "password", "Alpha", email="alpha1@superset.org" + ), + self.create_user( + "alpha2", "password", "Alpha", email="alpha2@superset.org" + ), + ] + + yield users + + # rollback changes (assuming cascade delete) + for user in users: + db.session.delete(user) + db.session.commit() + @pytest.mark.usefixtures("create_report_schedules") def test_get_report_schedule(self): """ @@ -656,6 +676,26 @@ class TestReportSchedulesApi(SupersetTestCase): data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"dashboard": "Dashboard does not exist"}} + @pytest.mark.usefixtures("create_report_schedules") + @pytest.mark.usefixtures("create_alpha_users") + def test_update_report_not_owned(self): + """ + ReportSchedule API: Test update report not owned + """ + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.name == "name2") + .one_or_none() + ) + + self.login(username="alpha2", password="password") + report_schedule_data = { + "active": False, + } + uri = f"api/v1/report/{report_schedule.id}" + rv = self.put_assert_metric(uri, report_schedule_data, "put") + self.assertEqual(rv.status_code, 403) + @pytest.mark.usefixtures("create_report_schedules") def test_delete_report_schedule(self): """ @@ -698,6 +738,23 @@ class TestReportSchedulesApi(SupersetTestCase): rv = self.client.delete(uri) assert rv.status_code == 404 + @pytest.mark.usefixtures("create_report_schedules") + @pytest.mark.usefixtures("create_alpha_users") + def test_delete_report_not_owned(self): + """ + ReportSchedule API: Test delete try not owned + """ + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.name == "name2") + .one_or_none() + ) + + self.login(username="alpha2", password="password") + uri = f"api/v1/report/{report_schedule.id}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 403) + @pytest.mark.usefixtures("create_report_schedules") def test_bulk_delete_report_schedule(self): """ @@ -737,6 +794,24 @@ class TestReportSchedulesApi(SupersetTestCase): rv = self.client.delete(uri) assert rv.status_code == 404 + @pytest.mark.usefixtures("create_report_schedules") + @pytest.mark.usefixtures("create_alpha_users") + def test_bulk_delete_report_not_owned(self): + """ + ReportSchedule API: Test bulk delete try not owned + """ + report_schedule = ( + db.session.query(ReportSchedule) + .filter(ReportSchedule.name == "name2") + .one_or_none() + ) + report_schedules_ids = [report_schedule.id] + + self.login(username="alpha2", password="password") + uri = f"api/v1/report/?q={prison.dumps(report_schedules_ids)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 403) + @pytest.mark.usefixtures("create_report_schedules") def test_get_list_report_schedule_logs(self): """