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:07 UTC
[superset] 01/01: fix: table schema permissions
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"""
+ )