eschutho commented on code in PR #26749:
URL: https://github.com/apache/superset/pull/26749#discussion_r1462649639
##########
superset/security/manager.py:
##########
@@ -1908,6 +1908,30 @@ def raise_for_access(
self.get_table_access_error_object(denied)
)
+ 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
+ )
+ ):
+ raise SupersetSecurityException(
+ SupersetError(
+
error_type=SupersetErrorType.DASHBOARD_SECURITY_ACCESS_ERROR,
+ message=_("Guest user cannot modify chart payload"),
+ level=ErrorLevel.ERROR,
Review Comment:
Similar question as here:
https://github.com/apache/superset/pull/26748/files#diff-30f4c6ffdcb1f78a9e1ebbb60e1f297b379c181534d5a185a4cd37b1b16ac6f8R301
Does this raise a 5xx error? I think these are generally user-generated,
right? We would want to know about them, but be able to distinguish them from a
system error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
