fix: apply per-layer datasource access checks in deck_multi charts

[sha174n]

SUMMARY

The deck_multi visualization renders a list of child layers, each bound to its own datasource referenced by id in the request (deck_slices). This applies the standard datasource access check (security_manager.raise_for_access) to every child layer before it is queried, and skips any layer the current user cannot access — so a multi-layer chart only returns data from datasources the caller is authorized for, consistent with how every other query path is gated.

TESTING INSTRUCTIONS

pytest tests/unit_tests/test_viz_get_df_payload.py

Adds test_deck_multi_skips_child_layers_without_datasource_access: a child layer whose datasource the current user cannot access is skipped rather than queried.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.18%. Comparing base (751f5eb) to head (17cd2dc).
⚠️ Report is 553 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/viz.py	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #42049      +/-   ##
==========================================
- Coverage   65.19%   65.18%   -0.01%     
==========================================
  Files        2767     2767              
  Lines      156062   156059       -3     
  Branches    35717    35715       -2     
==========================================
- Hits       101741   101732       -9     
- Misses      52361    52365       +4     
- Partials     1960     1962       +2     

Flag	Coverage Δ
hive	38.60% <0.00%> (-0.01%)	⬇️
mysql	57.86% <50.00%> (-0.01%)	⬇️
postgres	57.92% <50.00%> (-0.01%)	⬇️
presto	40.54% <0.00%> (-0.01%)	⬇️
python	59.32% <50.00%> (-0.01%)	⬇️
sqlite	57.53% <50.00%> (-0.01%)	⬇️
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.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	41adaa0
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a5bc5d13a3b600008d5f780
😎 Deploy Preview	https://deploy-preview-42049--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.

[bito-code-review]

Code Review Agent Run #a852a7

Actionable Suggestions - 0

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

• tests/integration_tests/viz_tests.py - 3

  • Incorrect security mock target · Line 1746-1746
  • Incorrect security mock target · Line 1818-1818
  • Incorrect security mock target · Line 1859-1859

Review Details

• Files reviewed - 3 · Commit Range: a903729..f467010

  • superset/viz.py
  • tests/integration_tests/viz_tests.py
  • tests/unit_tests/test_viz_get_df_payload.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

Thanks for digging into this, the deck_multi per-layer check makes sense since each child layer really does bind to its own datasource. Appreciate the test for the denied-layer case too. CI's green, so let's get this merged.

[rusackas]

Approving. can_access_datasource() wraps raise_for_access() and catches SupersetSecurityException, so the mock target on the new test is right. GitHub shows conflicts with master now though, so it'll need a rebase before it can merge.

[rusackas]

Nice fix, the per-layer check makes sense since each deck_multi layer really does bind to its own datasource, and the denied-layer test actually locks it in. LGTM, just needs a rebase off master before it's mergeable.

[rusackas]

Traced can_access_datasource down to raise_for_access, it's the real gate, not just a post-hoc filter. The continue skips querying entirely for a denied layer, not just the output. New test builds an allowed/denied layer pair and confirms only the allowed one's child viz gets constructed. LGTM, just needs a rebase.

[rusackas]

One thing before I can sign off here: this patches superset/viz.py, which #41714 removed from master recently, so the diff doesn't apply anymore and GitHub's showing conflicts. The per-layer datasource check is the right instinct, but it needs to be reworked against wherever deck_multi's layer resolution lives now. Happy to take another look once it's rebased.

Dismissing: this diff targets superset/viz.py, which was deleted from master 4 days ago by #41714 (also mine) — the PR can't apply as-is, and the per-layer access-control gap it addresses looks already closed by that refactor (each deck_multi layer now goes through its own datasource authorization via check_resource_permissions on independent explore_json requests). Left a comment on the PR with details.

[sha174n]

@rusackas You're right, and it goes further than a rebase: #41714 deleted superset/viz.py, tests/integration_tests/viz_tests.py and tests/unit_tests/test_viz_get_df_payload.py, so all three files in this diff are gone from master.

There is also nothing left to port it onto. deck_multi is fully client-side now: Multi.tsx resolves layers via GET /api/v1/chart/<id>/deck_layers/ and issues one POST /api/v1/chart/data per layer, using the authoritative datasource_id/datasource_type. Each of those runs ChartDataCommand.validate() -> QueryContext.raise_for_access() -> security_manager.raise_for_access(query_context=...), so every layer is resolved against its own datasource by construction rather than by an explicit loop. The embedded-guest path is handled separately by the parent_slice_id + _validate_child_in_parent_multilayer() check in superset/security/manager.py.

So this is obsolete rather than rebaseable, and I think closing it is the right call. Happy to open a follow-up if you want test coverage pinning the per-layer behaviour on the new path.

[rusackas]

Agreed, this is obsolete now that #41714 pulled the legacy viz.py pipeline out from under it — deck_multi's per-layer access check already happens through QueryContext on the new client-side path. Closing this out, thanks for tracking that down.