(superset) branch master updated: fix: catalog permission check (#29581)

[beto]

This is an automated email from the ASF dual-hosted git repository.

beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new fb15278f97 fix: catalog permission check (#29581)
fb15278f97 is described below

commit fb15278f973d37e5fbed1a7674346b84b1fa5af8
Author: Beto Dealmeida <robe...@dealmeida.net>
AuthorDate: Fri Jul 12 21:00:13 2024 -0400

    fix: catalog permission check (#29581)
---
 superset/security/manager.py              |  9 ++++-
 tests/unit_tests/security/manager_test.py | 66 ++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/superset/security/manager.py b/superset/security/manager.py
index 53fc9aa232..30cf37f117 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -860,7 +860,7 @@ class SupersetSecurityManager(  # pylint: 
disable=too-many-public-methods
             if len(parts) == 2 and default_catalog:
                 accessible_catalogs.add(default_catalog)
             elif len(parts) == 3:
-                accessible_catalogs.add(parts[2])
+                accessible_catalogs.add(parts[1])
 
         # datasource_access
         if perms := self.user_view_menu_names("datasource_access"):
@@ -911,7 +911,12 @@ class SupersetSecurityManager(  # pylint: 
disable=too-many-public-methods
                 return datasource_names
 
         if schema:
-            schema_perm = self.get_schema_perm(database, catalog, schema)
+            default_catalog = database.get_default_catalog()
+            schema_perm = self.get_schema_perm(
+                database.database_name,
+                catalog or default_catalog,
+                schema,
+            )
             if schema_perm and self.can_access("schema_access", schema_perm):
                 return datasource_names
 
diff --git a/tests/unit_tests/security/manager_test.py 
b/tests/unit_tests/security/manager_test.py
index a83e415529..924e2cbf28 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -32,7 +32,7 @@ from superset.security.manager import (
 )
 from superset.sql_parse import Table
 from superset.superset_typing import AdhocColumn, AdhocMetric
-from superset.utils.core import override_user
+from superset.utils.core import DatasourceName, override_user
 
 
 def test_security_manager(app_context: None) -> None:
@@ -760,3 +760,67 @@ def test_raise_for_access_catalog(
         == """You need access to the following tables: `db2.public.ab_user`,
             `all_database_access` or `all_datasource_access` permission"""
     )
+
+
+def test_get_datasources_accessible_by_user_schema_access(
+    mocker: MockerFixture,
+    app_context: None,
+) -> None:
+    """
+    Test that `get_datasources_accessible_by_user` works with schema 
permissions.
+    """
+    sm = SupersetSecurityManager(appbuilder)
+    mocker.patch.object(sm, "can_access_database", return_value=False)
+
+    database = mocker.MagicMock()
+    database.database_name = "db1"
+    database.get_default_catalog.return_value = "catalog2"
+
+    can_access = mocker.patch.object(sm, "can_access", return_value=True)
+
+    datasource_names = [
+        DatasourceName("table1", "schema1", "catalog2"),
+        DatasourceName("table2", "schema1", "catalog2"),
+    ]
+
+    assert sm.get_datasources_accessible_by_user(
+        database,
+        datasource_names,
+        catalog=None,
+        schema="schema1",
+    ) == [
+        DatasourceName("table1", "schema1", "catalog2"),
+        DatasourceName("table2", "schema1", "catalog2"),
+    ]
+
+    # Even though we passed `catalog=None,` the schema check uses the default 
catalog
+    # when building the schema permission, since the DB supports catalog.
+    can_access.assert_called_with("schema_access", 
"[db1].[catalog2].[schema1]")
+
+
+def test_get_catalogs_accessible_by_user_schema_access(
+    mocker: MockerFixture,
+    app_context: None,
+) -> None:
+    """
+    Test that `get_catalogs_accessible_by_user` works with schema permissions.
+    """
+    sm = SupersetSecurityManager(appbuilder)
+    mocker.patch.object(sm, "can_access_database", return_value=False)
+    mocker.patch.object(
+        sm,
+        "user_view_menu_names",
+        side_effect=[
+            set(),  # catalog_access
+            {"[db1].[catalog2].[schema1]"},  # schema_access
+            set(),  # datasource_access
+        ],
+    )
+
+    database = mocker.MagicMock()
+    database.database_name = "db1"
+    database.get_default_catalog.return_value = "catalog2"
+
+    catalogs = {"catalog1", "catalog2"}
+
+    assert sm.get_catalogs_accessible_by_user(database, catalogs) == 
{"catalog2"}