alexandrusoare commented on code in PR #45273:
URL: https://github.com/apache/superset/pull/45273#discussion_r4270445044


##########
tests/unit_tests/views/datasource/views_test.py:
##########
@@ -888,6 +888,36 @@ def test_samples_proceeds_for_supported_datasource_type(
     view.json_response.assert_called_once_with({"result": {"rows": []}})
 
 
+@patch("superset.views.datasource.views.redact_sensitive_query_fields")

Review Comment:
   This mocks the helper and sets is_guest_user = False, so it'd pass even if 
the redaction were a no-op for guests — worth one case with the real helper and 
a guest?



##########
superset/charts/data/api.py:
##########
@@ -563,18 +564,11 @@ def _process_data(query_data: Any) -> Any:
                 # their guest token has been granted "can view query on 
Dashboard",
                 # mirroring the permission the frontend uses to expose the
                 # "View query" action. Stacktraces and driver errors stay 
redacted
-                # regardless, as those leak details of the deployment itself.
-                can_view_query = security_manager.can_access(
-                    "can_view_query", "Dashboard"
-                )
-                for query in queries:
-                    if not can_view_query:
-                        query.pop("query", None)
-                    query.pop("stacktrace", None)
-                    if query.get("error"):
-                        query["error"] = sanitize_error_message(
-                            query["error"], required=sanitize_required
-                        )
+                # regardless, as those leak details of the deployment itself. 
The
+                # same helper redacts the ``/datasource/samples`` (Drill to
+                # detail) response so the two guest-facing endpoints cannot
+                # diverge.
+                redact_sensitive_query_fields(queries, 
required=sanitize_required)

Review Comment:
   The if sanitize_required := is_sanitization_required() guard is redundant 
now — the helper resolves required and returns early itself, so a bare 
redact_sensitive_query_fields(queries) is identical (and retires the 
is_sanitization_required import here).



-- 
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