(superset) 09/09: fix: pass valid SQL to SM (#27464)

[mi...@apache.org]

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

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

commit ae942dddc39e0c485e870a3b16dc3db8eebd2884
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Mar 18 15:38:58 2024 -0400

    fix: pass valid SQL to SM (#27464)
    
    (cherry picked from commit 376bfd05bdba2bbc4bde2d209324105d0d408ee4)
---
 superset/security/manager.py                  |  6 ++++-
 tests/unit_tests/commands/dataset/__init__.py | 16 ++++++++++++
 tests/unit_tests/security/manager_test.py     | 35 +++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/superset/security/manager.py b/superset/security/manager.py
index 38f4e4bdfa..f125269fb9 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -59,6 +59,7 @@ from superset.exceptions import (
     DatasetInvalidPermissionEvaluationException,
     SupersetSecurityException,
 )
+from superset.jinja_context import get_template_processor
 from superset.security.guest_token import (
     GuestToken,
     GuestTokenResources,
@@ -1956,11 +1957,14 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
                 return
 
             if query:
+                # make sure the quuery is valid SQL by rendering any Jinja
+                processor = get_template_processor(database=query.database)
+                rendered_sql = processor.process_template(query.sql)
                 default_schema = database.get_default_schema_for_query(query)
                 tables = {
                     Table(table_.table, table_.schema or default_schema)
                     for table_ in sql_parse.ParsedQuery(
-                        query.sql,
+                        rendered_sql,
                         engine=database.db_engine_spec.engine,
                     ).tables
                 }
diff --git a/tests/unit_tests/commands/dataset/__init__.py b/tests/unit_tests/commands/dataset/__init__.py
new file mode 100644
index 0000000000..13a83393a9
--- /dev/null
+++ b/tests/unit_tests/commands/dataset/__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
index 325531c25b..22ec0dda4a 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -27,6 +27,7 @@ from superset.exceptions import SupersetSecurityException
 from superset.extensions import appbuilder
 from superset.models.slice import Slice
 from superset.security.manager import SupersetSecurityManager
+from superset.sql_parse import Table
 from superset.superset_typing import AdhocMetric
 from superset.utils.core import override_user
 
@@ -245,6 +246,40 @@ def test_raise_for_access_query_default_schema(
     )
 
 
+def test_raise_for_access_jinja_sql(mocker: MockFixture, app_context: None) -> None:
+    """
+    Test that Jinja gets rendered to SQL.
+    """
+    sm = SupersetSecurityManager(appbuilder)
+    mocker.patch.object(sm, "can_access_database", return_value=False)
+    mocker.patch.object(sm, "get_schema_perm", return_value="[PostgreSQL].[public]")
+    mocker.patch.object(sm, "can_access", return_value=False)
+    mocker.patch.object(sm, "is_guest_user", return_value=False)
+    get_table_access_error_object = mocker.patch.object(
+        sm, "get_table_access_error_object"
+    )
+    SqlaTable = mocker.patch("superset.connectors.sqla.models.SqlaTable")
+    SqlaTable.query_datasources_by_name.return_value = []
+
+    database = mocker.MagicMock()
+    database.get_default_schema_for_query.return_value = "public"
+    query = mocker.MagicMock()
+    query.database = database
+    query.sql = "SELECT * FROM {% if True %}ab_user{% endif %} WHERE 1=1"
+
+    with pytest.raises(SupersetSecurityException):
+        sm.raise_for_access(
+            database=None,
+            datasource=None,
+            query=query,
+            query_context=None,
+            table=None,
+            viz=None,
+        )
+
+    get_table_access_error_object.assert_called_with({Table("ab_user", "public")})
+
+
 def test_raise_for_access_chart_for_datasource_permission(
     mocker: MockFixture,
     app_context: None,