geido commented on code in PR #45060:
URL: https://github.com/apache/superset/pull/45060#discussion_r4231089005
##########
superset/common/query_context_factory.py:
##########
@@ -72,6 +75,27 @@ def create( # pylint: disable=too-many-arguments
result_type = result_type or ChartDataResultType.FULL
result_format = result_format or ChartDataResultFormat.JSON
+ if (
+ self._authorize_semantic_before_metadata
+ and datasource_model_instance is not None
+ and DatasourceType(datasource["type"]) ==
DatasourceType.SEMANTIC_VIEW
+ ):
+ # Guest dashboard and payload checks need the completed query
context,
+ # and an operator EXTRA_RAISE_FOR_ACCESS_BYPASS hook may read the
+ # request's queries; keep their authorization path and timing
unchanged.
+ if not security_manager.is_guest_user() and not
current_app.config.get(
Review Comment:
I think this needs a guard before merge. A custom `raise_for_access` that
checks the requested metrics/columns gets `queries=[]` here, so a previously
valid semantic chart request can fail before the full check. Could we skip the
preflight when the security manager overrides this method? The stock-manager
tests won’t cover that path.
--
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]
