sadpandajoe commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r4077189792
##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -167,31 +213,134 @@ export const useDownloadMenuItems = (
}
};
+ const triggerExportDownload = (downloadUrl: string) => {
+ // Stream the file straight to disk via a hidden iframe. The endpoint sends
+ // Content-Disposition: attachment, so the browser saves it without
+ // navigating the dashboard away (fatal inside an embedded iframe) and
+ // without buffering the whole workbook in tab memory the way
fetch().blob()
+ // would. The status endpoint already confirmed the link is ready and
+ // backend-matched, so the only failure left is the narrow race where the
+ // object is removed between that check and this click; such an error
+ // response loads invisibly in the iframe and leaves the page untouched.
+ const iframe = document.createElement('iframe');
+ iframe.style.display = 'none';
+ iframe.src = downloadUrl;
Review Comment:
This download is now subject to the dashboard document’s `frame-src` policy.
Deployments with `frame-src 'none'` will block the request while the next line
still reports a successful download, leaving guest/Public users with no
fallback. Could this use a non-frame delivery mechanism that preserves the page
without requiring a CSP frame exception?
##########
superset/dashboards/api.py:
##########
@@ -1844,21 +1914,162 @@ def export_xlsx(self, pk: int) -> WerkzeugResponse:
export_dashboard_excel.apply_async(
kwargs={
"dashboard_id": dashboard.id,
- "user_id": g.user.id,
+ "user_id": user_id,
"active_data_mask": payload.get("active_data_mask", {}),
"job_id": job_id,
"mode": payload.get("mode", "data"),
+ "guest_token": guest_token_payload,
+ "lock_token": acquire.token,
},
task_id=job_id,
)
except Exception:
# If enqueuing fails (e.g. broker down) the task will never run to
# release the lock, so free it now rather than block exports until
# the TTL expires.
- ReleaseDistributedLock(EXPORT_LOCK_NAMESPACE, lock_params).run()
+ ReleaseDistributedLock(
+ EXPORT_LOCK_NAMESPACE, lock_params, token=acquire.token
+ ).run()
raise
return self.response(202, job_id=job_id)
+ @expose("/export_xlsx/status/<uuid:job_id>/", methods=("GET",))
+ @protect()
+ @safe
+ @statsd_metrics
+ def export_xlsx_status(self, job_id: uuid.UUID) -> WerkzeugResponse:
+ """Poll the status of an in-flight or completed Excel export.
+ ---
+ get:
+ summary: Poll the status of a dashboard Excel export job
+ description: >-
+ For a session with no email address to be notified at (e.g. an
+ embedded/guest session), the frontend polls this endpoint with the
+ job_id from the export_xlsx response instead of waiting for an
+ email. Behind the same @protect() as the export request itself,
+ unlike the login-free download_xlsx stream (which also has to
+ work when clicked from a plain email link, possibly with no
+ active session at all).
+ parameters:
+ - in: path
+ schema:
+ type: string
+ format: uuid
+ name: job_id
+ description: The job_id from the export_xlsx response
+ responses:
+ 200:
+ description: >-
+ Job status: {"status": "pending"} while queued,
+ {"status": "running"} once a worker has started executing,
+ {"status": "ready", "download_url": "..."} once the file is
+ available, or {"status": "error", "message": "..."} if the
+ export failed.
+ 401:
+ $ref: '#/components/responses/401'
+ """
+ payload = get_export_status(job_id)
+ if payload is None:
+ return self.response(200, status="pending")
+ if payload.get("status") == STATUS_READY:
+ # Never report ready for a link the download endpoint will refuse:
+ # an unset backend (config cleared since upload) 501s there, and a
+ # mismatched backend 410s. Both mean the file cannot be served.
+ storage_backend =
current_app.config["EXPORT_STORAGE"].get("backend")
+ if storage_backend is None or not _link_backend_matches(
+ payload.get("backend")
+ ):
+ return self.response(
+ 200, status=STATUS_ERROR, message="This download has
expired."
+ )
+ return self.response(
+ 200, status=STATUS_READY, download_url=download_path(job_id)
+ )
+ if payload.get("status") == STATUS_ERROR:
+ return self.response(
+ 200, status=STATUS_ERROR, message=payload.get("message")
+ )
+ if payload.get("status") == STATUS_RUNNING:
+ return self.response(200, status=STATUS_RUNNING)
+ return self.response(200, status="pending")
+
+ def get_method_permission(self, method_name: str) -> str:
+ # download_xlsx is intentionally login-free (no @protect): the
+ # unguessable job_id is the credential, and dashboard access was
+ # already checked when the export was requested. Map it to no
+ # permission so FAB neither requires auth nor advertises a security
+ # requirement for it in the OpenAPI spec.
+ if method_name == "download_xlsx":
+ return ""
+ return super().get_method_permission(method_name)
+
+ @expose("/export_xlsx/download/<uuid:job_id>/", methods=("GET",))
+ @safe
+ @statsd_metrics
+ def download_xlsx(self, job_id: uuid.UUID) -> WerkzeugResponse:
+ """Stream a completed Excel export from storage.
+ ---
+ get:
+ summary: Download a completed dashboard Excel export
+ security: []
+ description: >-
+ Intentionally requires no login: the unguessable job_id, emailed
+ only to the original requester (or handed to their own session
+ via export_xlsx_status), is the credential. The dashboard access
+ check already ran once, when the export was requested -- see
+ security_manager.raise_for_access in export_xlsx. The file
+ streams through Superset with the deployment's own storage
+ credentials instead of redirecting to a signed storage URL, so
+ it works for ambient identities that cannot sign (e.g. workload
+ identity federation) and never mints a bearer URL Superset
+ cannot observe or revoke.
+ parameters:
+ - in: path
+ schema:
+ type: string
+ format: uuid
+ name: job_id
+ description: The job_id from the export_xlsx response
+ responses:
+ 200:
+ description: The .xlsx file as an attachment
+ 410:
+ description: The link is unknown, expired, or the export failed
+ 501:
+ description: Excel export is not configured on this server
+ """
+ resolved = resolve_download_link(job_id)
+ if resolved is None:
+ return self.response(410, message="This download link has
expired.")
+ bucket, key, uploaded_backend = resolved
+ storage_backend = current_app.config["EXPORT_STORAGE"].get("backend")
+ if storage_backend is None:
+ # A link can only exist if a backend was configured when the export
+ # ran, so reaching this means the config was cleared since then.
+ return self.response(
+ 501, message="Excel export is not configured on this server."
+ )
+ if not _link_backend_matches(uploaded_backend):
+ # Don't read another backend's upload; expire the link instead.
+ return self.response(410, message="This download link has
expired.")
+ try:
+ # Existence is checked eagerly, so a missing object is a clean 410.
+ size, chunks = storage_backend.download(bucket, key)
+ except FileNotFoundError:
+ return self.response(410, message="This download link has
expired.")
+ return Response(
+ stream_with_context(chunks),
+ mimetype=(
+
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
+ ),
+ headers={
Review Comment:
This login-free URL is a bearer credential, but the streamed response is
cacheable by default. A shared proxy can retain the workbook and serve it after
the link record expires, bypassing the configured TTL. Could this set
`Cache-Control: private, no-store` (and related no-cache directives) on the
attachment response?
--
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]
