You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/03/14 00:38:06 UTC

[superset] branch schema_restriction created (now 216e7efea2)

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

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


      at 216e7efea2 fix: table schema permissions

This branch includes the following new commits:

     new 216e7efea2 fix: table schema permissions

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[superset] 01/01: fix: table schema permissions

Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 216e7efea23e0cefc638c52b82cafcaf0f8fe3f1
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Mar 13 17:28:39 2023 -0700

    fix: table schema permissions
---
 scripts/benchmark_migration.py            |  2 +-
 superset/security/manager.py              |  8 ++-
 tests/unit_tests/security/__init__.py     | 16 ++++++
 tests/unit_tests/security/manager_test.py | 91 +++++++++++++++++++++++++++++++
 4 files changed, 115 insertions(+), 2 deletions(-)

diff --git a/scripts/benchmark_migration.py b/scripts/benchmark_migration.py
index baae8befec..e3e838bd5e 100644
--- a/scripts/benchmark_migration.py
+++ b/scripts/benchmark_migration.py
@@ -120,7 +120,7 @@ def find_models(module: ModuleType) -> List[Type[Model]]:
     # sort topologically so we can create entities in order and
     # maintain relationships (eg, create a database before creating
     # a slice)
-    sorter = TopologicalSorter()
+    sorter = TopologicalSorter()  # type: ignore
     for model in models:
         inspector = inspect(model)
         dependent_tables: List[str] = []
diff --git a/superset/security/manager.py b/superset/security/manager.py
index f068d2baec..8d9974db45 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1823,8 +1823,14 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
                 return
 
             if query:
+                # if no schema is specified, use the database default; in the future we
+                # should pass the query schema to the SQLAlchemy URI so that it's set as
+                # the default one.
+                with database.get_sqla_engine_with_context() as engine:
+                    default_schema = inspect(engine).default_schema_name
+
                 tables = {
-                    Table(table_.table, table_.schema or query.schema)
+                    Table(table_.table, table_.schema or default_schema)
                     for table_ in sql_parse.ParsedQuery(query.sql).tables
                 }
             elif table:
diff --git a/tests/unit_tests/security/__init__.py b/tests/unit_tests/security/__init__.py
new file mode 100644
index 0000000000..13a83393a9
--- /dev/null
+++ b/tests/unit_tests/security/__init__.py
@@ -0,0 +1,16 @@
+# 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.
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
new file mode 100644
index 0000000000..fdf6fb3a2c
--- /dev/null
+++ b/tests/unit_tests/security/manager_test.py
@@ -0,0 +1,91 @@
+# 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 pytest
+from pytest_mock import MockFixture
+
+from superset.exceptions import SupersetSecurityException
+from superset.extensions import appbuilder
+from superset.security.manager import SupersetSecurityManager
+
+
+def test_security_manager(app_context: None) -> None:
+    """
+    Test that the security manager can be built.
+    """
+    sm = SupersetSecurityManager(appbuilder)
+    assert sm
+
+
+def test_raise_for_access_query_default_schema(
+    mocker: MockFixture,
+    app_context: None,
+) -> None:
+    """
+    Test that the DB default schema is used in non-qualified table names.
+
+    For example, in Postgres, for the following query:
+
+        > SELECT * FROM foo;
+
+    We should check that the user has access to the `public` schema, regardless of the
+    schema set in the query.
+    """
+    sm = SupersetSecurityManager(appbuilder)
+    mocker.patch.object(sm, "can_access_database", return_value=False)
+    mocker.patch.object(sm, "get_schema_perm", return_value="[PostgreSQL].[public]")
+    SqlaTable = mocker.patch("superset.connectors.sqla.models.SqlaTable")
+    SqlaTable.query_datasources_by_name.return_value = []
+
+    inspect = mocker.patch("superset.security.manager.inspect")
+    inspect().default_schema_name = "public"
+    database = mocker.MagicMock()
+    query = mocker.MagicMock()
+    query.database = database
+    query.sql = "SELECT * FROM ab_user"
+
+    # user has access to `public` schema
+    mocker.patch.object(sm, "can_access", return_value=True)
+    assert (
+        sm.raise_for_access(  # type: ignore
+            database=None,
+            datasource=None,
+            query=query,
+            query_context=None,
+            table=None,
+            viz=None,
+        )
+        is None
+    )
+    sm.can_access.assert_called_with("schema_access", "[PostgreSQL].[public]")  # type: ignore
+
+    # user has only access to `secret` schema
+    mocker.patch.object(sm, "can_access", return_value=False)
+    with pytest.raises(SupersetSecurityException) as excinfo:
+        sm.raise_for_access(
+            database=None,
+            datasource=None,
+            query=query,
+            query_context=None,
+            table=None,
+            viz=None,
+        )
+    assert (
+        str(excinfo.value)
+        == """You need access to the following tables: `public.ab_user`,
+            `all_database_access` or `all_datasource_access` permission"""
+    )