feat(user_info): include Groups in user data payload when include_perms is True and show Groups on user_info page#39450
feat(user_info): include Groups in user data payload when include_perms is True and show Groups on user_info page#39450declan-zhao wants to merge 3 commits intoapache:masterfrom
Conversation
Code Review Agent Run #3159feActionable 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 |
| roles, permissions = get_permissions(user) | ||
| payload["roles"] = roles | ||
| payload["permissions"] = permissions | ||
| payload["groups"] = [group.name for group in user.groups or []] |
There was a problem hiding this comment.
Suggestion: Accessing user.groups unconditionally can raise an AttributeError for anonymous users created with AnonymousUserMixin (for example in embedded/dashboard routes), because those objects don't define a groups attribute. This will cause SPA bootstrap to fail at runtime. Use a safe attribute lookup and default to an empty list. [null pointer]
Severity Level: Critical 🚨
- ❌ Login view (`/login`) SPA bootstrap fails for anonymous users.
- ❌ Registration views using `render_app_template` error for anonymous sessions.
- ❌ Any anonymous SPA route using `get_spa_payload` crashes on bootstrap.
- ⚠️ Frontend cannot reliably read `groups` from user payload.| payload["groups"] = [group.name for group in user.groups or []] | |
| payload["groups"] = [ | |
| group.name for group in (getattr(user, "groups", None) or []) | |
| ] |
Steps of Reproduction ✅
1. Start Superset with this PR code and ensure authentication is enabled so
unauthenticated visitors are represented by `AnonymousUserMixin` (see
`superset/security/manager.py:2858-2865`, where `get_anonymous_user` returns
`AnonymousUserMixin()`).
2. In a fresh browser session (not logged in), navigate to the login URL handled by
`SupersetAuthView.login` in `superset/views/auth.py:34-56`. Because `g.user` is anonymous
(`is_authenticated` is False), the view executes `return super().render_app_template()`.
3. `render_app_template` in `BaseSupersetView` (`superset/views/base.py:15-38`) calls
`get_spa_template_context(entry, extra_bootstrap_data, **template_kwargs)`, which in turn
calls `get_spa_payload(extra_bootstrap_data)` (`superset/views/base.py:30-46`).
4. `get_spa_payload` constructs the SPA bootstrap payload and sets `"user":
bootstrap_user_data(g.user, include_perms=True)`. Inside `bootstrap_user_data`
(`superset/views/utils.py:107-138`), the anonymous branch runs, and with
`include_perms=True` it reaches line 136: `payload["groups"] = [group.name for group in
user.groups or []]`. Since the anonymous user instance (`AnonymousUserMixin`) has no
`groups` attribute, this line raises `AttributeError: 'AnonymousUserMixin' object has no
attribute 'groups'`, causing SPA bootstrap (and thus the login page) to fail with HTTP
500.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/views/utils.py
**Line:** 136:136
**Comment:**
*Null Pointer: Accessing `user.groups` unconditionally can raise an `AttributeError` for anonymous users created with `AnonymousUserMixin` (for example in embedded/dashboard routes), because those objects don't define a `groups` attribute. This will cause SPA bootstrap to fail at runtime. Use a safe attribute lookup and default to an empty list.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39450 +/- ##
=======================================
Coverage 64.49% 64.49%
=======================================
Files 2557 2557
Lines 133191 133193 +2
Branches 30935 30936 +1
=======================================
+ Hits 85897 85899 +2
Misses 45804 45804
Partials 1490 1490
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:
|
Code Review Agent Run #c558e8Actionable Suggestions - 0Additional Suggestions - 1
Review 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 |
SUMMARY
Currently, Group info (which Groups a user belongs to) is missing on the
user_infopage. This change includes Groups in payload wheninclude_permsis True and show Groups onuser_infopage.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION