rusackas commented on code in PR #41218:
URL: https://github.com/apache/superset/pull/41218#discussion_r3440546214


##########
superset/security/manager.py:
##########
@@ -520,42 +520,87 @@ def _native_filter_request_modified(query_context: 
"QueryContext") -> bool:
     )
 
 
-def query_context_modified(query_context: "QueryContext") -> bool:
+def _collect_sortable_identifiers(
+    stored_chart: "Slice",
+    stored_query_context: Optional[dict[str, Any]],
+) -> set[str]:
     """
-    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.
+    Frozen column names and metric labels/definitions a guest may legitimately
+    sort by: every column or metric the stored chart already references.
+
+    Order-by only changes the ordering of the result, not which data is read, 
so
+    any column or metric already part of the chart is a safe sort target. A 
term
+    that is not present in the stored chart (for example a free-form 
``random()``
+    expression) cannot be validated and must be rejected. Order-by entries are
+    ``(column_or_metric, ascending)`` pairs, so only their first element is
+    collected.
     """
-    form_data = query_context.form_data
-    stored_chart = query_context.slice_
+    allowed: set[str] = set()
 
-    # Native-filter data requests have no associated chart (no slice_id). 
Rather
-    # than accepting any payload, constrain them to the column(s) the 
dashboard's
-    # native filter is allowed to target; other chartless paths keep prior
-    # behavior (see _native_filter_request_modified).
-    if stored_chart is None:
-        return _native_filter_request_modified(query_context)
+    def add(values: Any) -> None:
+        for value in values or []:
+            allowed.add(freeze_value(value))

Review Comment:
   These are tiny local closures inside an already-documented helper — a 
docstring on each feels like noise, so leaving them as-is.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to