Repository navigation
fix(semantic-layer): check API permissions before availability - #45959
Open
mikebridge wants to merge 5 commits into
Open
mikebridge wants to merge 5 commits into
mikebridge wants to merge 5 commits into
Conversation
added 4 commits
November 1, 2026 13:14
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #45959 +/- ##
==========================================
+ Coverage 84.79% 84.87% +0.08%
==========================================
Files 3074 3075 +1
Lines 206435 207805 +1370
Branches 48651 48962 +311
==========================================
+ Hits 175047 176376 +1329
+ Misses 28089 28068 -21
- Partials 3299 3361 +62
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mikebridge
marked this pull request as ready for review
November 3, 2026 17:10
mikebridge
requested review from
geido,
michael-s-molina,
msyavuz,
rusackas,
sadpandajoe and
villebro
as code owners
November 3, 2026 17:10
Contributor
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
TL;DR: When
SEMANTIC_LAYERSwas off, the semantic-layer APIs returned 404 before checking login or permissions, so callers without access could tell whether the feature was enabled. Check permissions first: callers without access get the usual 401/403 in both states, and permitted callers still get 404 while the feature is off. This follows #45061, which is not yet in a release.Replace the blueprint
before_requestavailability check introduced by #45061 with one decorator immediately beneath each endpoint's existingprotect()orprotect_read()decorator. All sixteen exposed handlers are defined in these APIs; the SemanticView inclusion list excludes inherited ModelRestApi routes. API registration, startup configuration, and resource permissions remain unchanged. Remove ten redundant inline checks so each handler relies on the shared guard and evaluates availability once before entering its body. The feature decision still comes from the feature flag manager on each permitted request.Public-role grants continue to work through FAB: anonymous callers with
can_readon SemanticView can request a view's structure, subject to its datasource permissions. Flask's automatic OPTIONS responses return the same 200 and Allow header in either feature state, without entering a semantic handler.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI changes. Before: an anonymous caller without a Public grant received 401 when enabled and 404 when disabled; a signed-in caller without permission received 403 versus 404. After: those callers receive 401 or 403 respectively in both states. Permitted requests retain 200 when enabled and 404 when disabled, and resource-level refusals remain in place.
TESTING INSTRUCTIONS
Run:
The tests use registered Flask routes and real FAB permission checks, with in-memory principals and isolated metadata/provider lookups. They cover every exposed route, implicit HEAD, automatic OPTIONS, anonymous/Public-role/signed-in/guest callers, runtime toggle changes through the host callback, object-level refusals, and mixed bulk deletion. Removing the shared guard from each of the sixteen handlers, one at a time, makes the registered-route matrix fail. The tests do not exercise external identity providers or live semantic providers.
ADDITIONAL INFORMATION
SEMANTIC_LAYERS