fix(embedding): add optional dataset allowlist to guest tokens#39302
fix(embedding): add optional dataset allowlist to guest tokens#39302sha174n wants to merge 6 commits intoapache:masterfrom
Conversation
Code Review Agent Run #6df525Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (99.81%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #39302 +/- ##
==========================================
- Coverage 64.45% 64.41% -0.04%
==========================================
Files 2555 2553 -2
Lines 132721 132573 -148
Branches 30802 30752 -50
==========================================
- Hits 85539 85400 -139
+ Misses 45696 45684 -12
- Partials 1486 1489 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Introduces a `datasets` field to the guest token JWT and the `/api/v1/security/guest_token/` API schema. When `datasets` is present in the token, `raise_for_access()` enforces it as a strict allowlist — only datasource IDs listed in the claim are accessible to the embedded guest. When the field is absent the behaviour is unchanged: all datasets linked to the dashboard remain accessible, preserving full backwards compatibility. Changes: - `GuestToken` TypedDict split into required base + optional extension so the new field is correctly typed as `list[int] | absent` - `create_guest_access_token()` accepts an optional `datasets` kwarg and includes it in the JWT claims only when provided - `GuestTokenCreateSchema` exposes a nullable `datasets` list field - `raise_for_access()` checks the claim when present and raises `SupersetSecurityException` for any datasource not in the allowlist - 8 new unit tests covering JWT claim presence/absence, schema loading, and allowlist enforcement (allow, block, empty-list blocks all) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sha174n
force-pushed
the
feat/guest-token-dataset-allowlist
branch
from
f32ea0b to
75cf184
Compare
sha174n
changed the title
… g.user `g.user` is set by Flask-Login on every real request but is absent in unit test contexts that only push an app context. Switch to `getattr(g, "user", None)` so the method returns None gracefully instead of raising AttributeError, matching the expected semantics of "return the guest user if one exists". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #d5d8caActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
- get_current_guest_user_if_guest: use isinstance(GuestUser) instead of
trusting is_guest_user(), fixing AttributeError when tests mock
is_guest_user but g.user is a regular FAB User object
- api.py: use body.get("user", {}) to avoid KeyError when user field omitted
- api.py: pass datasets kwarg conditionally to preserve backwards compat
with custom security manager overrides that lack the parameter
- manager.py: use isinstance(allowed_datasets, list) guard to avoid
TypeError on malformed JWT claims
- guest_token.py: add docstring to _GuestTokenRequired
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #fcd79fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…non-guest path - Test get_current_guest_user_if_guest() directly for both the GuestUser and regular-user branches (previously only exercised through mocks) - Test that the dataset allowlist block is skipped entirely for non-guest users, covering the is_guest_user()=False branch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #3efc24Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…type guard for allowlist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #053ca6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…dataset-allowlist
1 participant
SUMMARY
Adds an optional
datasetsfield to the guest token API and JWT claims, giving embedding operators fine-grained control over which datasets a guest user can access.How it works:
datasetsis omitted from the token request, all datasets linked to the embedded dashboard remain accessible — exactly the existing behaviour (full backwards compatibility).datasetsis provided (e.g.[7, 8]),raise_for_access()enforces it as a strict allowlist: any datasource not in the list raisesSupersetSecurityException. An explicit empty list ([]) blocks all datasource access.Implementation details:
GuestTokenTypedDict split into required-fields base (_GuestTokenRequired) + optional-fields extension (GuestToken,total=False) so the new field is typed correctly as present-or-absent rather thanOptionalcreate_guest_access_token()acceptsdatasets: Optional[list[int]] = None; the claim is included in the JWT only when the value is notNoneGuestTokenCreateSchemaexposes a nullabledatasets: List[Integer]field (load_default=None)raise_for_access()BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API-only change
TESTING INSTRUCTIONS
# Run the new unit tests pytest tests/unit_tests/security/test_guest_token_dataset_allowlist.py -vManual API test:
datasets→ embedded dashboard loads all charts normallydatasets: [<id>]→ only charts using that dataset load; others return 403datasets: []→ all charts return 403ADDITIONAL INFORMATION
🤖 Generated with Claude Code