sadpandajoe commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r4048833253
##########
superset/dashboards/api.py:
##########
@@ -354,6 +391,9 @@ class DashboardRestApi(
# menu item on it) instead of the ``can_export_xlsx`` FAB would
otherwise
# derive from the method name.
"export_xlsx": "export",
+ # Polling status of an export you already requested is the same
+ # capability as requesting it, not a distinct permission.
+ "export_xlsx_status": "export",
Review Comment:
Making this feature usable for an embedded guest requires granting
`can_export on Dashboard`, because both XLSX routes use `export`. That grant
also authorizes `/api/v1/dashboard/export/`, so the same Public-role permission
can expose dashboard bundle metadata that Excel export does not need. Could
these routes use a dedicated permission instead?
##########
superset/dashboards/excel_export/download_link.py:
##########
@@ -0,0 +1,160 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Status tracking and long-lived download links for dashboard Excel exports.
+
+The download link shared with the requester is a Superset endpoint, never a
+raw or signed storage URL: signed URLs are transferable bearer credentials
+Superset cannot observe or revoke once issued, their real lifetime is bounded
+by the signing credentials' own session (not just their nominal expiry), and
+some ambient identities (e.g. direct workload identity federation) cannot
+sign at all. The link's lifetime is enforced by this module via the
+``key_value`` store's ``expires_on``, and the file itself streams through
+Superset with the deployment's storage credentials at click time.
+
+The download endpoint (``download_xlsx``) intentionally requires no login:
+the access-control decision for the underlying dashboard was already enforced
+once, when the export was originally requested (see
+``security_manager.raise_for_access`` in
+``superset.dashboards.api.export_xlsx``); the unguessable key handed only to
+that requester is the "possession of the link is the credential" model.
+
+Every entry is keyed by ``job_id`` -- the same id the ``export_xlsx`` POST
+response hands back -- rather than a separately-generated identifier, so a
+caller that only has the job id (e.g. a polling frontend for a session with
+no email on file, such as an embedded/guest dashboard) can resolve both
+status and, once ready, a download link from that one id.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from typing import Any
+from uuid import UUID
+
+from flask import current_app
+
+from superset.daos.key_value import KeyValueDAO
+from superset.key_value.types import JsonKeyValueCodec, KeyValueResource
+from superset.utils.decorators import transaction
+from superset.utils.urls import headless_url
+
+RESOURCE = KeyValueResource.EXCEL_EXPORT_DOWNLOAD
+CODEC = JsonKeyValueCodec()
+
+DOWNLOAD_PATH = "/api/v1/dashboard/export_xlsx/download/{job_id}/"
+
+STATUS_READY = "ready"
+STATUS_ERROR = "error"
+STATUS_RUNNING = "running"
+
+
+@transaction()
+def _sweep_and_upsert(
+ job_id: UUID, value: dict[str, Any], expires_at: datetime
+) -> None:
+ # Lazily sweep expired entries each time one is written; there is no
+ # dedicated cleanup job, so this resource keeps itself tidy on write.
+ # upsert (not create) so a retried/duplicate write for the same job_id
+ # overwrites cleanly instead of colliding on the primary key.
+ # @transaction commits now; the worker session otherwise only commits when
+ # the task settles, leaving statuses invisible to polling web pods.
+ KeyValueDAO.delete_expired_entries(RESOURCE)
+ KeyValueDAO.upsert_entry(
+ resource=RESOURCE,
+ value=value,
+ codec=CODEC,
+ key=job_id,
+ expires_on=expires_at,
+ )
+
+
+def download_path(job_id: UUID) -> str:
+ """Root-relative path of the download endpoint for ``job_id``, including
+ ``APPLICATION_ROOT`` when Superset is served under a subpath. Handed to the
+ polling frontend, which resolves it against its own origin (the one host
+ the user is provably reachable at)."""
+ path = DOWNLOAD_PATH.format(job_id=job_id)
+ app_root = current_app.config.get("APPLICATION_ROOT") or "/"
+ if app_root != "/" and not path.startswith(app_root):
Review Comment:
This skips the deployment prefix when `APPLICATION_ROOT` is a prefix of
`/api` (for example, `/api`), so both the polled and emailed download URLs omit
the root and 404. Could this check the prefix on a path-segment boundary
instead?
--
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]
