msyavuz commented on code in PR #45451:
URL: https://github.com/apache/superset/pull/45451#discussion_r4323296943
##########
superset/reports/filters.py:
##########
@@ -88,3 +93,35 @@ def apply(self, query: Query, value: Any) -> Query:
ReportSchedule.sql.ilike(ilike_value, escape="\\"),
)
)
+
+
+class ReportTargetChartFilter(ChartFilter): # pylint:
disable=too-few-public-methods
+ """Charts selectable as the target of a scheduled report.
+
+ A scheduled report is not rendered by the user who creates it: it runs
later,
+ as whichever principal the deployment configures as its executor. So a
+ deployment can hold assets a user may read themselves but may not schedule
a
+ report on, and that is a narrower question than ordinary chart visibility.
+
+ Only the exclusion key differs; every other rule is inherited. The
exclusion
+ is applied before delegating so it survives the parent's short-circuits.
+ """
+
+ def apply(self, query: Query, value: Any) -> Query:
+ query = apply_extra_access_exclusions(query, "report_target_charts",
Slice.id)
Review Comment:
The `report_target_*` exclusion only narrows the related-field dropdown:
`_check_object_access` in `commands/report/base.py` resolves the target through
`ChartDAO`/`DashboardDAO.find_by_id`, whose base filter applies only the plain
`charts`/`dashboards` key, so a POST or PUT with an excluded chart or dashboard
id is still accepted.
##########
superset/commands/explore/get.py:
##########
@@ -63,8 +63,21 @@ def _authorize_datasource(
routing below is testable without a request context.
"""
if slc:
+ # Chart access, and then the dataset in its own right. Access to a
+ # chart is not access to everything else in its dataset: Explore is a
+ # query builder, so opening it over a dataset the user cannot read
+ # hands them its full column and metric list and a UI for composing
+ # queries the saved chart never ran. Deployments that grant chart
+ # access by other means -- dashboard RBAC, or an
+ # ``EXTRA_RAISE_FOR_ACCESS_BYPASS`` hook -- reach this path with no
+ # dataset grant of their own, and the chart check alone let it pass.
security_manager.raise_for_access(chart=slc)
- elif isinstance(datasource, Query):
+ _raise_for_datasource(datasource)
Review Comment:
Saved charts now also need dataset access, so dashboard-only viewers
(dashboard RBAC or `EXTRA_RAISE_FOR_ACCESS_BYPASS`) who could open "View in
Explore" before will now get a 403; this is a user-visible behaviour change and
has no UPDATING.md entry.
##########
superset/utils/filters.py:
##########
@@ -176,3 +177,33 @@ def guest_embedded_dashboard_filter() ->
Optional[ColumnElement[bool]]:
# dashboards, never a plain internal id.
conditions.append(and_(Dashboard.id.in_(int_ids),
Dashboard.embedded.any()))
return or_(*conditions) if conditions else false()
+
+
+def apply_extra_access_exclusions(
+ query: "Query",
+ asset_kind: str,
+ id_column: "ColumnElement[Any]",
+) -> "Query":
+ """Withhold the asset IDs an ``EXTRA_ACCESS_QUERY_EXCLUSIONS`` hook names.
+
+ ``EXTRA_ACCESS_QUERY_FILTERS`` can only widen a result set -- its
conditions
+ are OR'd with the role paths -- so a deployment with its own notion of a
+ withheld asset cannot express that through it. This is the matching AND
NOT.
+
+ Returns ``query`` unchanged when no hook is configured or there is no
+ authenticated user, so the default build is unaffected.
+ """
+ from flask import current_app
+
+ from superset.utils.core import get_user_id
+
+ exclusions = current_app.config.get("EXTRA_ACCESS_QUERY_EXCLUSIONS") or {}
+ hook = exclusions.get(asset_kind)
+ if not hook:
+ return query
+
+ user_id = get_user_id()
+ if not user_id:
+ return query
+
+ return query.filter(id_column.notin_(hook(user_id)))
Review Comment:
`NOT IN` against a hook subquery that yields any NULL id evaluates to NULL
for every row, so a single NULL silently empties the whole chart or dashboard
list, including for admins; filter out NULLs or use `NOT EXISTS`.
--
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]
