gabotorresruiz commented on code in PR #43340:
URL: https://github.com/apache/superset/pull/43340#discussion_r3867273240
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -451,31 +491,45 @@ def _handle_export_failure(
def export_dashboard_excel(
self: Any, # pylint: disable=unused-argument
dashboard_id: int,
- user_id: int,
+ user_id: int | None,
active_data_mask: dict[str, Any],
job_id: str,
mode: str = EXPORT_MODE_DATA,
+ guest_token: GuestToken | None = None,
) -> None:
"""
Export a dashboard's charts to an ``.xlsx`` and record a download link.
:param dashboard_id: The dashboard to export
- :param user_id: The requesting user (the task runs with their permissions)
+ :param user_id: The requesting user (the task runs with their permissions),
+ or ``None`` for a guest/embedded requester
:param active_data_mask: Live dashboard filter state keyed by native
filter id
- :param job_id: Correlation id, also the Celery task id and S3 object name
+ :param job_id: Correlation id, also the Celery task id and storage object
name
:param mode: ``"data"`` streams every chart's tabular result; ``"images"``
embeds non-table charts as rendered images and keeps tables tabular
+ :param guest_token: The guest token payload when the requester is an
+ embedded guest; the guest user is reconstructed from it so the export
+ runs under the token's RLS rules and resource claims, never under an
+ elevated identity
"""
# pylint: disable=import-outside-toplevel
from superset.models.dashboard import Dashboard
requested_at = datetime.now(tz=timezone.utc)
- user = security_manager.get_user_by_id(user_id)
+ user = None
dashboard_title = ""
tmp_path: str | None = None
ttl = current_app.config["EXCEL_EXPORT_LINK_TTL_SECONDS"]
try:
+ # Resolve the user inside the protected block: if this raises (e.g. the
+ # guest role lookup fails), the ``finally`` below must still release
the
+ # lock the API acquired, and the failure status must still be recorded
+ # for pollers.
+ if user_id is not None:
+ user = security_manager.get_user_by_id(user_id)
+ elif guest_token:
Review Comment:
Fixed in c75abca73a: the no token branch now loads the anonymous principal,
mirroring superset.tasks.async_queries, so Public role permissions apply to
each chart's access check. Covered by
test_anonymous_export_runs_under_the_anonymous_principal.
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -315,35 +337,37 @@ export const useDownloadMenuItems = (
...(userCanExport
? [
{
- key: "export-xlsx",
- label: t("Export Data to Excel"),
- onClick: () => onExportXlsx("data"),
+ key: 'export-xlsx',
+ label: t('Export Data to Excel'),
+ onClick: () => onExportXlsx('data'),
},
// Image export renders charts through the headless webdriver, so
only
// offer it where that infrastructure is available (same signal as
the
// PDF/PNG image downloads above); otherwise non-table charts would
- // silently come back empty.
- ...(isWebDriverScreenshotEnabled
+ // silently come back empty. Embedded sessions are excluded too: the
+ // webdriver cannot render Explore under a guest identity, so the
+ // export would burn its whole task budget and produce nothing.
+ ...(isWebDriverScreenshotEnabled && !isEmbedded
Review Comment:
Fixed in c75abca73a: export_xlsx now returns 403 for mode=images when
security_manager.is_guest_user(), before the lock is acquired, so a direct POST
cannot hold the shared guest slot. Covered by
test_export_xlsx_images_403_for_guest.
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -83,9 +89,28 @@ export const useDownloadMenuItems = (
canExportImage,
} = props;
- const { addDangerToast, addSuccessToast } = useToasts();
+ const { addDangerToast, addSuccessToast, addInfoToast } = useToasts();
const dataMask = useSelector((state: RootState) => state.dataMask);
- const SCREENSHOT_NODE_SELECTOR = ".dashboard";
+ // Embedded (iframe) sessions may have no email address, so they get
+ // delivery-neutral copy and a poll window that outlives the task budget.
+ const isEmbedded = isEmbeddedDashboard();
Review Comment:
Fixed in c75abca73a: the branch now keys off the session principal (userId
and email from the user state) instead of iframe detection. An authenticated
user in an iframe keeps image export and the email copy; an anonymous session
at the top level gets the neutral copy and the long poll window. isEmbedded is
no longer imported there.
--
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]
