From 0a4f71d3a303437e4d53e3d3e5b965956651f4af Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Fri, 26 Sep 2025 16:49:10 -0400 Subject: [PATCH 1/5] fix: make /api/queries/{id}/results db_role-aware --- redash/handlers/query_results.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 4dd3cfd6ca..0078963e00 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -318,11 +318,16 @@ def get(self, query_id=None, query_result_id=None, filetype="json"): if query_id is not None: query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) - if query_result is None and query is not None and query.latest_query_data_id is not None: - query_result = get_object_or_404( - models.QueryResult.get_by_id_and_org, - query.latest_query_data_id, - self.current_org, + if query_result is None and query is not None: + # query.latest_query_data_id isn't db_role-specific, so + # ignore it and fetch the latest results for the current + # user's role. + query_result = models.QueryResult.get_latest( + data_source=query.data_source, + query=query.query_hash, + max_age=-1, + is_hash=True, + db_role=getattr(self.current_user, "db_role", None), ) if query is not None and query_result is not None and self.current_user.is_api_user(): From a19b166099ea2c7399e7d4a01a23525ed3e603ce Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Fri, 26 Sep 2025 20:11:16 -0400 Subject: [PATCH 2/5] test query result api responses by db role --- tests/handlers/test_query_results.py | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/handlers/test_query_results.py b/tests/handlers/test_query_results.py index 1170d6941e..3ce01f6ce4 100644 --- a/tests/handlers/test_query_results.py +++ b/tests/handlers/test_query_results.py @@ -329,6 +329,41 @@ def test_signed_in_user_and_different_query_result(self): rv = self.make_request("get", "/api/queries/{}/results/{}.json".format(query.id, query_result2.id)) self.assertEqual(rv.status_code, 403) + def test_query_results_by_db_role(self): + query = self.factory.create_query() + admin_result = self.factory.create_query_result( + query_text=query.query_text, query_hash=query.query_hash + ) + limited_result = self.factory.create_query_result( + query_text=query.query_text, + query_hash=query.query_hash, + db_role="limited", + ) + query.latest_query_data = limited_result + admin_user = self.factory.create_user() + limited_user = self.factory.create_user() + limited_user.db_role = "limited" + + admin_response = self.make_request( + "get", + "/api/queries/{}/results.json".format(query.id), + user=admin_user, + ) + limited_response = self.make_request( + "get", + "/api/queries/{}/results.json".format(query.id), + user=limited_user, + ) + self.assertEqual(query.latest_query_data_id, limited_result.id) + self.assertEqual(admin_response.status_code, 200) + self.assertEqual( + admin_response.get_json()["query_result"]["id"], admin_result.id + ) + self.assertEqual(limited_response.status_code, 200) + self.assertEqual( + limited_response.get_json()["query_result"]["id"], limited_result.id + ) + class TestQueryResultDropdownResource(BaseTestCase): def test_checks_for_access_to_the_query(self): From 4193cbb3237af749e18aa86ec0c9f70841f53c47 Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 16 Oct 2025 15:12:17 -0400 Subject: [PATCH 3/5] fix: enforce db_role for the query results data source When referencing a cached query using the query results data source (QRDS), fetch results specific to the caller's db_role. Note that this will only apply to new query runs. --- redash/query_runner/query_results.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/redash/query_runner/query_results.py b/redash/query_runner/query_results.py index 3fdc40c294..99349c0ba6 100644 --- a/redash/query_runner/query_results.py +++ b/redash/query_runner/query_results.py @@ -66,18 +66,24 @@ def replace_query_parameters(query_text, params): def get_query_results(user, query_id, bring_from_cache, params=None): query = _load_query(user, query_id) if bring_from_cache: - if query.latest_query_data_id is not None: - results = query.latest_query_data.data - else: + results = models.QueryResult.get_latest( + data_source=query.data_source, + query=query.query_hash, + max_age=-1, + is_hash=True, + db_role=getattr(user, "db_role", None), + ) + if not results: raise Exception("No cached result available for query {}.".format(query.id)) - else: - query_text = query.query_text - if params is not None: - query_text = replace_query_parameters(query_text, params) + return results.data + + query_text = query.query_text + if params is not None: + query_text = replace_query_parameters(query_text, params) - results, error = query.data_source.query_runner.run_query(query_text, user) - if error: - raise Exception("Failed loading results for query id {}.".format(query.id)) + results, error = query.data_source.query_runner.run_query(query_text, user) + if error: + raise Exception("Failed loading results for query id {}.".format(query.id)) return results From 750ae8a9d53e4f182331b22af8e675b15bbb75b4 Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 16 Oct 2025 15:41:23 -0400 Subject: [PATCH 4/5] bake a db_role into saved query_results runs --- redash/query_runner/query_results.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/redash/query_runner/query_results.py b/redash/query_runner/query_results.py index 99349c0ba6..3293a2149e 100644 --- a/redash/query_runner/query_results.py +++ b/redash/query_runner/query_results.py @@ -206,7 +206,19 @@ def run_query(self, query, user): rows.append(dict(zip(column_names, row))) - data = {"columns": columns, "rows": rows} + # Track the db role associated with the cached query result. + # Without this we know the db_role context of the user running + # a query against the Query Results Data Source, but not necessarily + # the role of the cached query data it referenced. + # + # TODO: Potentially use this data, either as an extra check + # when fetching query results or to clean up cached QRDS + # results that don't specify a db_role (via migration perhaps) + data = { + "columns": columns, + "rows": rows, + "db_role": getattr(user, "db_role", None), + } error = None else: error = "Query completed but it returned no data." From 2b210d6cd2f287207a7e9dd83c97966ec54cd1bc Mon Sep 17 00:00:00 2001 From: AJ Kerrigan Date: Thu, 16 Oct 2025 16:07:16 -0400 Subject: [PATCH 5/5] ok, ok... include the 404 wrapper --- redash/handlers/query_results.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 0078963e00..a9c922ecf4 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -322,7 +322,8 @@ def get(self, query_id=None, query_result_id=None, filetype="json"): # query.latest_query_data_id isn't db_role-specific, so # ignore it and fetch the latest results for the current # user's role. - query_result = models.QueryResult.get_latest( + query_result = get_object_or_404( + models.QueryResult.get_latest, data_source=query.data_source, query=query.query_hash, max_age=-1,