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 2024/03/12 17:36:22 UTC
(superset) branch sc-79188 created (now f74908e8ae)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch sc-79188
in repository https://gitbox.apache.org/repos/asf/superset.git
at f74908e8ae fix: check if guest user modified query
This branch includes the following new commits:
new f74908e8ae fix: check if guest user modified query
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix: check if guest user modified query
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch sc-79188
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f74908e8aeb9a707f6c531ba526aa990ce3d40cb
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Mar 12 12:38:40 2024 -0400
fix: check if guest user modified query
---
superset/security/manager.py | 52 +++++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 83e12fb2dc..66bf4c623a 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -67,6 +67,7 @@ from superset.security.guest_token import (
GuestTokenUser,
GuestUser,
)
+from superset.superset_typing import AdhocMetric
from superset.utils.core import (
DatasourceName,
DatasourceType,
@@ -143,6 +144,43 @@ RoleModelView.edit_columns = ["name", "permissions", "user"]
RoleModelView.related_views = []
+def query_context_modified(query_context: "QueryContext") -> bool:
+ """
+ Check if a query context has been modified.
+
+ This is used to ensure guest users don't modify the payload and fetch data
+ different from what was shared with them in dashboards.
+ """
+ form_data = query_context.form_data
+ stored_chart = query_context.slice_
+
+ # sanity checks
+ if form_data is None or stored_chart is None:
+ return True
+
+ # cannot request a different chart
+ if form_data.get("slice_id") != stored_chart.id:
+ return True
+
+ # compare form_data
+ requested_metrics = set(form_data.get("metrics") or [])
+ stored_metrics = set(stored_chart.params_dict.get("metrics") or [])
+ if requested_metrics != stored_metrics:
+ return True
+
+ # compare queries in query_context
+ queries_metrics: set[Union[AdhocMetric, str]] = set()
+ for query in query_context.queries:
+ queries_metrics.update(query.metrics or [])
+
+ if stored_chart.query_context:
+ stored_query_context = json.loads(cast(str, stored_chart.query_context))
+ for query in stored_query_context.get("queries") or []:
+ stored_metrics.update(query.get("metrics") or []) # type: ignore
+
+ return queries_metrics != stored_metrics
+
+
class SupersetSecurityManager( # pylint: disable=too-many-public-methods
SecurityManager
):
@@ -1944,19 +1982,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
if self.is_guest_user() and query_context:
# Guest users MUST not modify the payload so it's requesting a different
# chart or different ad-hoc metrics from what's saved.
- form_data = query_context.form_data
- stored_chart = query_context.slice_
-
- if (
- form_data is None
- or stored_chart is None
- or form_data.get("slice_id") != stored_chart.id
- or form_data.get("metrics", []) != stored_chart.params_dict["metrics"]
- or any(
- query.metrics != stored_chart.params_dict["metrics"]
- for query in query_context.queries
- )
- ):
+ if query_context_modified(query_context):
raise SupersetSecurityException(
SupersetError(
error_type=SupersetErrorType.DASHBOARD_SECURITY_ACCESS_ERROR,