gabotorresruiz commented on code in PR #43340:
URL: https://github.com/apache/superset/pull/43340#discussion_r3816054572
##########
superset/tasks/export_dashboard_excel.py:
##########
@@ -451,26 +451,37 @@ 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: dict[str, Any] | 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 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)
+ if user_id is not None:
+ user = security_manager.get_user_by_id(user_id)
+ elif guest_token:
+ user = security_manager.get_guest_user_from_token(guest_token)
+ else:
+ user = None
Review Comment:
Good catch, fixed in 619bd266db: user resolution now happens inside the
protected block, so a failing guest role lookup still releases the lock in the
`finally` and still records a pollable failure status via
`_handle_export_failure` (which tolerates `user=None`). Added
`test_lock_released_and_failure_recorded_when_user_resolution_fails` covering
exactly this path. Note the pre-existing code had the same latent gap with
`get_user_by_id`, which this also closes.
--
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]
