Repository navigation
test(mcp): pin is_guest_user False in the RLS tool test fixture - #46125
Open
sadpandajoe wants to merge 2 commits into
Open
sadpandajoe wants to merge 2 commits into
sadpandajoe wants to merge 2 commits into
Conversation
The autouse mock_auth fixture built the request user as a bare Mock(), so `is_guest_user` resolved to a truthy auto-attribute. Since #45644, SecurityManager.is_guest_user() no longer short-circuits on the username lookup, so with EMBEDDED_SUPERSET enabled the RLS tools' guest guard treated the fixture user as an embedded guest and returned Forbidden. Pin the attribute so the fixture models the regular user it is meant to be. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #46125 +/- ##
===========================================
+ Coverage 65.35% 85.21% +19.85%
===========================================
Files 3083 3083
Lines 211224 211224
Branches 49803 49803
===========================================
+ Hits 138054 180002 +41948
+ Misses 69992 27817 -42175
- Partials 3178 3405 +227
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:
|
9 tasks
2 of 6 tasks
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
Since #45644 (5a0e884),
SecurityManager.is_guest_user()no longer gates on the current user's username, so it returnsuser.is_guest_userfor whateverg.useris. The RLS tool tests' autousemock_authfixture builds that user as a bareMock(), whose auto-createdis_guest_userattribute is truthy. WhenEMBEDDED_SUPERSETis enabled in the test worker, the guest guard inlist_rls_filters/get_rls_filter_infotreats the fixture user as an embedded guest and returnsForbidden, failing 10 tests intests/unit_tests/mcp_service/rls/tool/test_rls_tools.pyon master.The production behaviour from #45644 is correct (real
Userobjects have nois_guest_user;GuestUsersets it toTrue). This change pinsmock_user.is_guest_user = Falsein the fixture so it models the regular user it is meant to be. Guest denial stays covered by the existing tests that patchis_guest_usertoTrue.TESTING INSTRUCTIONS
FEATURE_FLAGS = {"EMBEDDED_SUPERSET": True}and pointSUPERSET_CONFIG_PATHat it.pytest tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py: on master 10 tests fail withRLS filters are not available to embedded guests.; with this change they all pass.SUPERSET_CONFIG_PATHand run it again: all tests still pass.ADDITIONAL INFORMATION
Notes:
EMBEDDED_SUPERSETleaks across tests in the same xdist worker:build_appintests/unit_tests/conftest.pyloads config viafrom_object("superset.config"), soapp.config["DEFAULT_FEATURE_FLAGS"]is the module-level dict, andfeature_flag_manager.init_app(superset/utils/feature_flag_manager.py:44-45) updates it in place. A parametrizedFEATURE_FLAGS = {"EMBEDDED_SUPERSET": True}therefore stays on for later apps in that worker. Follow-up, not addressed here.Mock()user withoutis_guest_user(e.g.chart/tool/test_get_chart_info.py,test_dashboard_scope.py,test_dataset_scope.py) have the same exposure but did not fail.🤖 Generated with Claude Code