Revert "fix: Allow dataset owners to explore their datasets (#20382)"

This reverts commit f910958.

superset/charts/filters.py

@@ -20,8 +20,7 @@
 from sqlalchemy import and_, or_
 from sqlalchemy.orm.query import Query
 
-from superset import db, security_manager
-from superset.connectors.sqla import models
+from superset import security_manager
 from superset.connectors.sqla.models import SqlaTable
 from superset.models.slice import Slice
 from superset.views.base import BaseFilter
@@ -78,18 +77,6 @@ def apply(self, query: Query, value: Any) -> Query:
             return query
         perms = security_manager.user_view_menu_names("datasource_access")
         schema_perms = security_manager.user_view_menu_names("schema_access")
-        owner_ids_query = (
-            db.session.query(models.SqlaTable.id)
-            .join(models.SqlaTable.owners)
-            .filter(
-                security_manager.user_model.id
-                == security_manager.user_model.get_user_id()
-            )
-        )
         return query.filter(
-            or_(
-                self.model.perm.in_(perms),
-                self.model.schema_perm.in_(schema_perms),
-                models.SqlaTable.id.in_(owner_ids_query),
-            )
+            or_(self.model.perm.in_(perms), self.model.schema_perm.in_(schema_perms))
         )

superset/security/manager.py

@@ -1093,7 +1093,6 @@ def raise_for_access(
         from superset.connectors.sqla.models import SqlaTable
         from superset.extensions import feature_flag_manager
         from superset.sql_parse import Table
-        from superset.views.utils import is_owner
 
         if database and table or query:
             if query:
@@ -1124,9 +1123,7 @@ def raise_for_access(
 
                     # Access to any datasource is suffice.
                     for datasource_ in datasources:
-                        if self.can_access(
-                            "datasource_access", datasource_.perm
-                        ) or is_owner(datasource_, getattr(g, "user", None)):
+                        if self.can_access("datasource_access", datasource_.perm):
                             break
                     else:
                         denied.add(table_)
@@ -1152,7 +1149,6 @@ def raise_for_access(
             if not (
                 self.can_access_schema(datasource)
                 or self.can_access("datasource_access", datasource.perm or "")
-                or is_owner(datasource, getattr(g, "user", None))
                 or (
                     should_check_dashboard_access
                     and self.can_access_based_on_dashboard(datasource)

superset/views/utils.py

@@ -32,7 +32,6 @@
 import superset.models.core as models
 from superset import app, dataframe, db, result_set, viz
 from superset.common.db_query_status import QueryStatus
-from superset.connectors.sqla.models import SqlaTable
 from superset.datasource.dao import DatasourceDAO
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
@@ -427,7 +426,7 @@ def is_slice_in_container(
     return False
 
 
-def is_owner(obj: Union[Dashboard, Slice, SqlaTable], user: User) -> bool:
+def is_owner(obj: Union[Dashboard, Slice], user: User) -> bool:
     """Check if user is owner of the slice"""
     return obj and user in obj.owners
 

tests/integration_tests/base_tests.py

@@ -21,7 +21,7 @@
 import json
 from contextlib import contextmanager
 from typing import Any, Dict, Union, List, Optional
-from unittest.mock import Mock, patch, MagicMock
+from unittest.mock import Mock, patch
 
 import pandas as pd
 import pytest
@@ -252,7 +252,7 @@ def get_database_by_name(database_name: str = "main") -> Database:
 
     @staticmethod
     def get_datasource_mock() -> BaseDatasource:
-        datasource = MagicMock()
+        datasource = Mock()
         results = Mock()
         results.query = Mock()
         results.status = Mock()
@@ -266,7 +266,6 @@ def get_datasource_mock() -> BaseDatasource:
         datasource.database = Mock()
         datasource.database.db_engine_spec = Mock()
         datasource.database.db_engine_spec.mutate_expression_label = lambda x: x
-        datasource.owners = MagicMock()
         return datasource
 
     def get_resp(

tests/integration_tests/security_tests.py

@@ -906,25 +906,20 @@ def test_can_access_table(self, mock_raise_for_access):
 
     @patch("superset.security.SupersetSecurityManager.can_access")
     @patch("superset.security.SupersetSecurityManager.can_access_schema")
-    @patch("superset.views.utils.is_owner")
-    def test_raise_for_access_datasource(
-        self, mock_can_access_schema, mock_can_access, mock_is_owner
-    ):
+    def test_raise_for_access_datasource(self, mock_can_access_schema, mock_can_access):
         datasource = self.get_datasource_mock()
 
         mock_can_access_schema.return_value = True
         security_manager.raise_for_access(datasource=datasource)
 
         mock_can_access.return_value = False
         mock_can_access_schema.return_value = False
-        mock_is_owner.return_value = False
 
         with self.assertRaises(SupersetSecurityException):
             security_manager.raise_for_access(datasource=datasource)
 
     @patch("superset.security.SupersetSecurityManager.can_access")
-    @patch("superset.views.utils.is_owner")
-    def test_raise_for_access_query(self, mock_can_access, mock_is_owner):
+    def test_raise_for_access_query(self, mock_can_access):
         query = Mock(
             database=get_example_database(), schema="bar", sql="SELECT * FROM foo"
         )
@@ -933,7 +928,6 @@ def test_raise_for_access_query(self, mock_can_access, mock_is_owner):
         security_manager.raise_for_access(query=query)
 
         mock_can_access.return_value = False
-        mock_is_owner.return_value = False
 
         with self.assertRaises(SupersetSecurityException):
             security_manager.raise_for_access(query=query)

tests/unit_tests/explore/utils_test.py

@@ -271,7 +271,7 @@ def test_query_has_access(mocker: MockFixture, app_context: AppContext) -> None:
     )
 
 
-def test_query_no_access(mocker: MockFixture, client, app_context: AppContext) -> None:
+def test_query_no_access(mocker: MockFixture, app_context: AppContext) -> None:
     from superset.connectors.sqla.models import SqlaTable
     from superset.explore.utils import check_datasource_access
     from superset.models.core import Database
@@ -282,9 +282,7 @@ def test_query_no_access(mocker: MockFixture, client, app_context: AppContext) -
             query_find_by_id,
             return_value=Query(database=Database(), sql="select * from foo"),
         )
-        table = SqlaTable()
-        table.owners = []
-        mocker.patch(query_datasources_by_name, return_value=[table])
+        mocker.patch(query_datasources_by_name, return_value=[SqlaTable()])
         mocker.patch(is_user_admin, return_value=False)
         mocker.patch(is_owner, return_value=False)
         mocker.patch(can_access, return_value=False)