You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/23 19:14:56 UTC
(superset) branch master updated: fix(SavedQueries): allow other admin users see "saved queries" (#20604) (#21769)
This is an automated email from the ASF dual-hosted git repository.
rusackas 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 18fea37e849 fix(SavedQueries): allow other admin users see "saved queries" (#20604) (#21769)
18fea37e849 is described below
commit 18fea37e8493be20867c01c42ce01eea5685f92e
Author: peng weikang <pe...@pengwk.com>
AuthorDate: Wed Jun 24 03:14:48 2026 +0800
fix(SavedQueries): allow other admin users see "saved queries" (#20604) (#21769)
---
.../src/pages/SavedQueryList/index.tsx | 52 ++++++++++++++-
superset/queries/saved_queries/api.py | 2 +
superset/queries/saved_queries/filters.py | 15 +++--
.../queries/saved_queries/api_tests.py | 74 +++++++++++-----------
.../queries/saved_queries/commands_tests.py | 45 ++++++++-----
5 files changed, 131 insertions(+), 57 deletions(-)
diff --git a/superset-frontend/src/pages/SavedQueryList/index.tsx b/superset-frontend/src/pages/SavedQueryList/index.tsx
index 1010585eb37..dd72c2de4c1 100644
--- a/superset-frontend/src/pages/SavedQueryList/index.tsx
+++ b/superset-frontend/src/pages/SavedQueryList/index.tsx
@@ -62,6 +62,7 @@ import { TagTypeEnum } from 'src/components/Tag/TagType';
import { loadTags } from 'src/components/Tag/utils';
import { Icons } from '@superset-ui/core/components/Icons';
import copyTextToClipboard from 'src/utils/copy';
+import type Owner from 'src/types/Owner';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal';
import { findPermission } from 'src/utils/findPermission';
@@ -91,6 +92,15 @@ interface SavedQueryListProps {
};
}
+type SavedQueryCellProps = {
+ row: {
+ original: SavedQueryObject & {
+ changed_by?: Owner | null;
+ created_by?: Owner | null;
+ };
+ };
+};
+
const StyledTableLabel = styled.div`
.count {
margin-left: 5px;
@@ -435,12 +445,30 @@ function SavedQueryList({
changed_on_delta_humanized: changedOn,
},
},
- }: any) => <ModifiedInfo user={changedBy} date={changedOn} />,
+ }: SavedQueryCellProps) => (
+ <ModifiedInfo user={changedBy ?? undefined} date={changedOn} />
+ ),
Header: t('Last modified'),
accessor: 'changed_on_delta_humanized',
size: 'xl',
id: 'changed_on_delta_humanized',
},
+ {
+ accessor: 'created_by.first_name',
+ Header: t('Created by'),
+ disableSortBy: true,
+ size: 'xl',
+ Cell: ({
+ row: {
+ original: { created_by: createdBy },
+ },
+ }: SavedQueryCellProps) =>
+ createdBy ? `${createdBy.first_name} ${createdBy.last_name}` : '',
+ },
+ {
+ accessor: 'created_by',
+ hidden: true,
+ },
{
Cell: ({ row: { original } }: any) => {
const handlePreview = () => {
@@ -589,6 +617,28 @@ function SavedQueryList({
),
paginate: true,
},
+ {
+ Header: t('Created by'),
+ key: 'created_by',
+ id: 'created_by',
+ input: 'select',
+ operator: FilterOperator.RelationOneMany,
+ unfilteredLabel: t('All'),
+ fetchSelects: createFetchRelated(
+ 'saved_query',
+ 'created_by',
+ createErrorHandler(errMsg =>
+ addDangerToast(
+ t(
+ 'An error occurred while fetching created by values: %s',
+ errMsg,
+ ),
+ ),
+ ),
+ user,
+ ),
+ paginate: true,
+ },
],
[addDangerToast],
);
diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py
index d625b906ad8..e47cc61060c 100644
--- a/superset/queries/saved_queries/api.py
+++ b/superset/queries/saved_queries/api.py
@@ -183,10 +183,12 @@ class SavedQueryRestApi(BaseSupersetModelRestApi):
related_field_filters = {
"database": "database_name",
"changed_by": RelatedFieldFilter("first_name", FilterRelatedOwners),
+ "created_by": RelatedFieldFilter("first_name", FilterRelatedOwners),
}
base_related_field_filters = {
"database": [["id", DatabaseFilter, lambda: []]],
"changed_by": [["id", BaseFilterRelatedUsers, lambda: []]],
+ "created_by": [["id", BaseFilterRelatedUsers, lambda: []]],
}
allowed_rel_fields = {"database", "changed_by", "created_by"}
allowed_distinct_fields = {"catalog", "schema"}
diff --git a/superset/queries/saved_queries/filters.py b/superset/queries/saved_queries/filters.py
index 821f42d6f11..ff57491d19b 100644
--- a/superset/queries/saved_queries/filters.py
+++ b/superset/queries/saved_queries/filters.py
@@ -16,12 +16,13 @@
# under the License.
from typing import Any
-from flask import g
+from flask import g, has_request_context, request
from flask_babel import lazy_gettext as _
from flask_sqlalchemy import BaseQuery
from sqlalchemy import or_
from sqlalchemy.orm.query import Query
+from superset import security_manager
from superset.models.sql_lab import SavedQuery
from superset.tags.filters import BaseTagIdFilter, BaseTagNameFilter
from superset.views.base import BaseFilter
@@ -82,10 +83,16 @@ class SavedQueryTagIdFilter(BaseTagIdFilter): # pylint: disable=too-few-public-
class SavedQueryFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: BaseQuery, value: Any) -> BaseQuery:
"""
- Filter saved queries to only those created by current user.
+ Filter saved queries to current user's queries unless this is a read
+ request and the user can access all queries.
:returns: flask-sqlalchemy query
"""
- return query.filter(
- SavedQuery.created_by == g.user # pylint: disable=comparison-with-callable
+ can_access_all_queries = security_manager.can_access_all_queries() and (
+ not has_request_context() or request.method == "GET"
)
+ if not can_access_all_queries:
+ query = query.filter(
+ SavedQuery.created_by == g.user # pylint: disable=comparison-with-callable
+ )
+ return query
diff --git a/tests/integration_tests/queries/saved_queries/api_tests.py b/tests/integration_tests/queries/saved_queries/api_tests.py
index 5fe597d86fc..c9ee294ed6a 100644
--- a/tests/integration_tests/queries/saved_queries/api_tests.py
+++ b/tests/integration_tests/queries/saved_queries/api_tests.py
@@ -14,13 +14,14 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+# pylint: disable=consider-using-transaction
# isort:skip_file
"""Unit tests for Superset"""
from datetime import datetime
from io import BytesIO
from typing import Optional
-from unittest.mock import patch
+from unittest.mock import Mock, patch
from zipfile import is_zipfile, ZipFile
import yaml
@@ -201,10 +202,7 @@ class TestSavedQueryApi(SupersetTestCase):
"""
Saved Query API: Test get list saved query
"""
- admin = self.get_user("admin")
- saved_queries = (
- db.session.query(SavedQuery).filter(SavedQuery.created_by == admin).all()
- )
+ saved_queries = db.session.query(SavedQuery).all()
self.login(ADMIN_USERNAME)
uri = "api/v1/saved_query/"
@@ -250,12 +248,9 @@ class TestSavedQueryApi(SupersetTestCase):
"""
Saved Query API: Test get list and sort saved query
"""
- admin = self.get_user("admin")
saved_queries = (
- db.session.query(SavedQuery)
- .filter(SavedQuery.created_by == admin)
- .order_by(SavedQuery.schema.asc())
- ).all()
+ db.session.query(SavedQuery).order_by(SavedQuery.schema.asc()).all()
+ )
self.login(ADMIN_USERNAME)
query_string = {"order_column": "schema", "order_direction": "asc"}
uri = f"api/v1/saved_query/?q={rison.dumps(query_string)}"
@@ -306,13 +301,8 @@ class TestSavedQueryApi(SupersetTestCase):
Saved Query API: Test get list and database saved query
"""
example_db = get_example_database()
- admin_user = self.get_user("admin")
-
all_db_queries = (
- db.session.query(SavedQuery)
- .filter(SavedQuery.db_id == example_db.id)
- .filter(SavedQuery.created_by_fk == admin_user.id)
- .all()
+ db.session.query(SavedQuery).filter(SavedQuery.db_id == example_db.id).all()
)
self.login(ADMIN_USERNAME)
@@ -401,12 +391,8 @@ class TestSavedQueryApi(SupersetTestCase):
Saved Query API: Test get list and custom filter (sql) saved query
"""
self.login(ADMIN_USERNAME)
- admin = self.get_user("admin")
all_queries = (
- db.session.query(SavedQuery)
- .filter(SavedQuery.created_by == admin)
- .filter(SavedQuery.sql.ilike("%table%"))
- .all()
+ db.session.query(SavedQuery).filter(SavedQuery.sql.ilike("%table%")).all()
)
query_string = {
"filters": [{"col": "label", "opr": "all_text", "value": "table"}],
@@ -423,10 +409,8 @@ class TestSavedQueryApi(SupersetTestCase):
Saved Query API: Test get list and custom filter (description) saved query
"""
self.login(ADMIN_USERNAME)
- admin = self.get_user("admin")
all_queries = (
db.session.query(SavedQuery)
- .filter(SavedQuery.created_by == admin)
.filter(SavedQuery.description.ilike("%cool%"))
.all()
)
@@ -520,12 +504,7 @@ class TestSavedQueryApi(SupersetTestCase):
# Test not favorite saves queries
expected_models = (
db.session.query(SavedQuery)
- .filter(
- and_(
- ~SavedQuery.id.in_(users_favorite_query),
- SavedQuery.created_by == admin,
- )
- )
+ .filter(and_(~SavedQuery.id.in_(users_favorite_query)))
.order_by(SavedQuery.label.asc())
.all()
)
@@ -591,9 +570,11 @@ class TestSavedQueryApi(SupersetTestCase):
"""
SavedQuery API: Test distinct schemas
"""
- admin = self.get_user("admin")
- saved_queries = (
- db.session.query(SavedQuery).filter(SavedQuery.created_by == admin).all()
+ schemas = (
+ db.session.query(SavedQuery.schema)
+ .distinct()
+ .order_by(SavedQuery.schema)
+ .all()
)
self.login(ADMIN_USERNAME)
@@ -602,11 +583,8 @@ class TestSavedQueryApi(SupersetTestCase):
assert rv.status_code == 200
data = json.loads(rv.data.decode("utf-8"))
expected_response = {
- "count": len(saved_queries),
- "result": [
- {"text": f"schema{i}", "value": f"schema{i}"}
- for i in range(len(saved_queries))
- ],
+ "count": len(schemas),
+ "result": [{"text": schema, "value": schema} for (schema,) in schemas],
}
assert data == expected_response
@@ -818,6 +796,28 @@ class TestSavedQueryApi(SupersetTestCase):
rv = self.delete_assert_metric(uri, "bulk_delete")
assert rv.status_code == 400
+ @pytest.mark.usefixtures("create_saved_queries")
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
+ def test_delete_bulk_saved_query_all_query_access_keeps_owner_filter(
+ self, mock_can_access_all_queries: Mock
+ ) -> None:
+ """
+ Saved Query API: Test all_query_access does not bypass ownership for delete
+ """
+ mock_can_access_all_queries.return_value = True
+ admin = self.get_user("admin")
+ sample_query = (
+ db.session.query(SavedQuery).filter(SavedQuery.created_by == admin).first()
+ )
+
+ self.login(GAMMA_SQLLAB_USERNAME)
+ uri = f"api/v1/saved_query/?q={rison.dumps([sample_query.id])}"
+ rv = self.delete_assert_metric(uri, "bulk_delete")
+ assert rv.status_code == 404
+ assert db.session.query(SavedQuery).get(sample_query.id) is not None
+
@pytest.mark.usefixtures("create_saved_queries")
def test_delete_bulk_saved_query_not_found(self):
"""
diff --git a/tests/integration_tests/queries/saved_queries/commands_tests.py b/tests/integration_tests/queries/saved_queries/commands_tests.py
index aea1af22060..55effc41d57 100644
--- a/tests/integration_tests/queries/saved_queries/commands_tests.py
+++ b/tests/integration_tests/queries/saved_queries/commands_tests.py
@@ -14,8 +14,9 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+# pylint: disable=consider-using-transaction
-from unittest.mock import patch
+from unittest.mock import Mock, patch
import pytest
import yaml
@@ -57,9 +58,11 @@ class TestExportSavedQueriesCommand(SupersetTestCase):
db.session.commit()
super().tearDown()
- @patch("superset.queries.saved_queries.filters.g")
- def test_export_query_command(self, mock_g):
- mock_g.user = security_manager.find_user("admin")
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
+ def test_export_query_command(self, mock_can_access_all_queries: Mock) -> None:
+ mock_can_access_all_queries.return_value = True
command = ExportSavedQueriesCommand([self.example_query.id])
contents = dict(command.run())
@@ -85,12 +88,14 @@ class TestExportSavedQueriesCommand(SupersetTestCase):
"database_uuid": str(self.example_database.uuid),
}
- @patch("superset.queries.saved_queries.filters.g")
- def test_export_query_command_no_related(self, mock_g):
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
+ def test_export_query_command_no_related(self, mock_can_access_all_queries):
"""
Test that only the query is exported when export_related=False.
"""
- mock_g.user = security_manager.find_user("admin")
+ mock_can_access_all_queries.return_value = True
command = ExportSavedQueriesCommand(
[self.example_query.id], export_related=False
@@ -103,30 +108,40 @@ class TestExportSavedQueriesCommand(SupersetTestCase):
]
assert expected == list(contents.keys())
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
@patch("superset.queries.saved_queries.filters.g")
- def test_export_query_command_no_access(self, mock_g):
+ def test_export_query_command_no_access(
+ self, mock_filter_g, mock_can_access_all_queries
+ ):
"""Test that users can't export datasets they don't have access to"""
- mock_g.user = security_manager.find_user("gamma")
+ mock_can_access_all_queries.return_value = False
+ mock_filter_g.user = security_manager.find_user("gamma")
command = ExportSavedQueriesCommand([self.example_query.id])
contents = command.run()
with self.assertRaises(SavedQueryNotFoundError): # noqa: PT027
next(contents)
- @patch("superset.queries.saved_queries.filters.g")
- def test_export_query_command_invalid_dataset(self, mock_g):
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
+ def test_export_query_command_invalid_dataset(self, mock_can_access_all_queries):
"""Test that an error is raised when exporting an invalid dataset"""
- mock_g.user = security_manager.find_user("admin")
+ mock_can_access_all_queries.return_value = True
command = ExportSavedQueriesCommand([-1])
contents = command.run()
with self.assertRaises(SavedQueryNotFoundError): # noqa: PT027
next(contents)
- @patch("superset.queries.saved_queries.filters.g")
- def test_export_query_command_key_order(self, mock_g):
+ @patch(
+ "superset.queries.saved_queries.filters.security_manager.can_access_all_queries"
+ )
+ def test_export_query_command_key_order(self, mock_can_access_all_queries):
"""Test that they keys in the YAML have the same order as export_fields"""
- mock_g.user = security_manager.find_user("admin")
+ mock_can_access_all_queries.return_value = True
command = ExportSavedQueriesCommand([self.example_query.id])
contents = dict(command.run())