fix(dashboard): resolve guest users in Excel export instead of crashing on g.user.id

[gabotorresruiz]

SUMMARY

Targets fix/excel-export-guest-sessions-and-s3-link-expiry (#43336), not master. It fixes a bug in that PR's guest path and is meant to merge into it, so the guard removal and this fix travel together.

#43336 removes the isinstance(g.user, GuestUser) guard that returned 400 so embedded guests can export, but export_xlsx still calls g.user.id (for the throttle lock params and the Celery task's user_id). GuestUser extends AnonymousUserMixin and has no id attribute, so a guest POST crashes with AttributeError and returns 500 {"message": "Fatal error"} before the task is ever enqueued. Verified empirically on a staging deployment carrying #43336's changes: the embedded export button returned exactly that 500; with this fix applied the export goes through.

The fix mirrors _load_user_from_job_metadata in superset/tasks/async_queries.py:

• The endpoint passes user_id=get_user_id() (None for guests) plus guest_token=getattr(g.user, "guest_token", None).
• The task reconstructs the guest via security_manager.get_guest_user_from_token(...), so the export runs under the token's RLS rules and resource claims, never under an elevated identity.
• Guests share throttle lock slot 0 per dashboard, acquired and released with the same key.

ADDITIONAL INFORMATION

• Has associated issue: fixes the guest path of fix(dashboard): decouple Excel export link lifetime from S3 credential expiry, and support guest/embedded sessions #43336
• Required feature flags:
• Changes UI
• Includes DB Migration

[bito-code-review]

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	4ad2fcd
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a8600bd7b85590008a68281
😎 Deploy Preview	https://deploy-preview-43340--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 17 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (fix/excel-export-guest-sessions-and-s3-link-expiry@26fabcf). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...hboard/components/menu/DownloadMenuItems/index.tsx	76.92%	9 Missing ⚠️
superset/tasks/export_dashboard_excel.py	11.11%	8 Missing ⚠️

Additional details and impacted files

@@                                  Coverage Diff                                  @@
##             fix/excel-export-guest-sessions-and-s3-link-expiry   #43340   +/-   ##
=====================================================================================
  Coverage                                                      ?   66.66%           
=====================================================================================
  Files                                                         ?     2874           
  Lines                                                         ?   163763           
  Branches                                                      ?    37788           
=====================================================================================
  Hits                                                          ?   109178           
  Misses                                                        ?    52455           
  Partials                                                      ?     2130           

Flag	Coverage Δ
hive	38.18% <16.66%> (?)
javascript	73.78% <76.92%> (?)
mysql	57.86% <33.33%> (?)
postgres	57.89% <33.33%> (?)
presto	40.12% <16.66%> (?)
python	59.27% <33.33%> (?)
sqlite	57.53% <33.33%> (?)
unit	100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codeant-ai-for-open-source]

Suggestion: The guest reconstruction runs before the try/finally that releases the distributed lock. If get_guest_user_from_token raises—for example because the guest role lookup or metadata access fails—the task exits without releasing the lock acquired by the API, leaving every guest export for that dashboard blocked until the TTL expires. Move user reconstruction inside the protected block or explicitly release the lock on this failure path. [missing cleanup]

Severity Level: Major ⚠️

- ❌ Guest exports remain blocked until lock TTL expiry.
- ⚠️ All guests share the affected dashboard's slot-0 lock.
- ⚠️ Failure status is not recorded for this path.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/tasks/export_dashboard_excel.py
**Line:** 479:484
**Comment:**
	*Missing Cleanup: The guest reconstruction runs before the `try`/`finally` that releases the distributed lock. If `get_guest_user_from_token` raises—for example because the guest role lookup or metadata access fails—the task exits without releasing the lock acquired by the API, leaving every guest export for that dashboard blocked until the TTL expires. Move user reconstruction inside the protected block or explicitly release the lock on this failure path.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[gabotorresruiz]

Good catch, fixed in 619bd26: 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.

[bito-code-review]

The flagged issue is correct. In superset/tasks/export_dashboard_excel.py, the guest user reconstruction (security_manager.get_guest_user_from_token(guest_token)) occurs before the try/finally block that ensures the distributed lock is released. If this reconstruction fails, the task terminates without releasing the lock, causing subsequent exports for that dashboard to be blocked until the TTL expires.

To resolve this, move the user reconstruction logic inside the try block or ensure the lock is released in a finally block that covers the entire execution path. Given the current structure, wrapping the user initialization in the try block is the most straightforward fix.

superset/tasks/export_dashboard_excel.py

# ... (previous code)
    requested_at = datetime.now(tz=timezone.utc)
    try:
        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
        # ... (rest of the logic)
    finally:
        # ... (lock release logic)