fix(reports): apply owners security validation (#12035)

* fix(reports): apply owners security validation

* fix pylint

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))

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()

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()

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")

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]

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):
         """