fix: table schema permissions

superset/security/manager.py

@@ -1823,8 +1823,14 @@ def raise_for_access(
                 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:

tests/integration_tests/sqllab_tests.py

@@ -312,15 +312,15 @@ def test_sql_json_schema_access(self):
         )
         self.assertEqual(1, len(data["data"]))
 
-        # postgres needs a schema as a part of the table name.
-        if db_backend == "mysql":
-            data = self.run_sql(
-                "SELECT * FROM test_table",
-                "5",
-                username="SchemaUser",
-                schema=CTAS_SCHEMA_NAME,
-            )
-            self.assertEqual(1, len(data["data"]))
+        # without a schema the query will fail, since it uses the default schema instead
+        # of CTAS_SCHEMA_NAME
+        data = self.run_sql(
+            "SELECT * FROM test_table",
+            "5",
+            username="SchemaUser",
+            schema=CTAS_SCHEMA_NAME,
+        )
+        self.assertTrue("errors" in data)
 
         db.session.query(Query).delete()
         with get_example_database().get_sqla_engine_with_context() as engine:

tests/unit_tests/explore/utils_test.py

@@ -274,15 +274,20 @@ def test_query_no_access(mocker: MockFixture, client) -> None:
     from superset.models.core import Database
     from superset.models.sql_lab import Query
 
+    inspect = mocker.patch("superset.security.manager.inspect")
+    inspect().default_schema_name = "public"
+
+    database = mocker.MagicMock()
+    mocker.patch(
+        query_find_by_id,
+        return_value=Query(database=database, sql="select * from foo"),
+    )
+    mocker.patch(query_datasources_by_name, return_value=[SqlaTable()])
+    mocker.patch(is_admin, return_value=False)
+    mocker.patch(is_owner, return_value=False)
+    mocker.patch(can_access, return_value=False)
+
     with raises(SupersetSecurityException):
-        mocker.patch(
-            query_find_by_id,
-            return_value=Query(database=Database(), sql="select * from foo"),
-        )
-        mocker.patch(query_datasources_by_name, return_value=[SqlaTable()])
-        mocker.patch(is_admin, return_value=False)
-        mocker.patch(is_owner, return_value=False)
-        mocker.patch(can_access, return_value=False)
         check_datasource_access(
             datasource_id=1,
             datasource_type=DatasourceType.QUERY,

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.

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