You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "pchukwuemeka424 (via GitHub)" <gi...@apache.org> on 2026/08/26 06:26:24 UTC
[PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
pchukwuemeka424 opened a new pull request, #43536:
URL: https://github.com/apache/superset/pull/43536
### SUMMARY
Fixes #43089.
Settings → Info always links to `/user_info/`, but `UserInfoView` required `can_read on user` / `User`. That view-menu is Admin-only (`ADMIN_ONLY_VIEW_MENUS`), so Alpha/Gamma were redirected to `/superset/welcome/` while Admin worked.
This binds the SPA shell to `can_userinfo on UserInfo`. `can_userinfo` is already in `ACCESSIBLE_PERMS`, so stock Gamma and Alpha receive it on role sync without widening user-management access.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (permission / redirect behavior).
### TESTING INSTRUCTIONS
1. Log in as Alpha or Gamma (not Admin).
2. Open **Settings → Info**.
3. Confirm `/user_info/` loads the profile page instead of redirecting to `/superset/welcome/`.
4. Confirm Admin still reaches `/user_info/` and user-management (`User`) remains Admin-only.
5. After upgrade, ensure roles are synced (`superset init` / normal startup sync) so `can_userinfo on UserInfo` is granted to Gamma/Alpha.
Unit coverage: `tests/unit_tests/views/test_user_info_permissions.py`.
Integration assertion: Gamma includes `("can_userinfo", "UserInfo")` in `tests/integration_tests/security_tests.py`.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #43089
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Made with [Cursor](https://cursor.com)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-5421463923
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #43536:
URL: https://github.com/apache/superset/pull/43536#discussion_r3878625216
##########
tests/unit_tests/views/test_user_info_permissions.py:
##########
@@ -0,0 +1,58 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Permissions for the Settings → Info (/user_info/) page."""
+
+from unittest.mock import MagicMock
+
+from superset.security.manager import SupersetSecurityManager
+from superset.views.user_info import UserInfoView
+
+
+def test_user_info_view_uses_accessible_userinfo_permission() -> None:
Review Comment:
These assertions prove the decorator metadata and the role's PVM membership, but not the endpoint's FAB access check, so a registration or authorization mismatch could still redirect the Gamma/Alpha users this change targets. Could this add an authenticated Gamma/Alpha request to `/user_info/` that asserts `200`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-5612358524
This has gone silent, so I'll make some touchups to help move it along.
Codeant's backward-compat point seems real - renaming the PVM to `UserInfo` means custom roles holding the old grant won't pick up the new one automatically, so folks could lose Settings → Info access after upgrade. Might need a migration or compat check instead of a straight rename.
Also, sadpandajoe's right that the new test only checks decorator metadata and role PVM membership, not an actual authenticated request hitting `/user_info/`. Could you add a Gamma/Alpha client test asserting a 200?
Pre-commit's also red, just a one-line ruff autofix (B009) - easy fix, I reckon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-6329141223
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 20b789312fba520e6739eb1b1429cc9ed797ef24 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ad94ed9a9a47a09d79cbbee |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-43536--superset-docs-preview.netlify.app](https://deploy-preview-43536--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAwDADRu2Rt0UVR4g08RtqkWLAfWgVFvLvZDLN4L9RWUj1hBRjANcp-1w_xHq8uzcQcisklG-IUtd5L7ypZ6lGejdXOxGiFkZDsQhPygt45Efh-) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-43536--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-5421602369
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43536?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.46%. Comparing base ([`1fd763b`](https://app.codecov.io/gh/apache/superset/commit/1fd763bd29b3db73e0931f6698f8f43d6b0c184f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`04e4abf`](https://app.codecov.io/gh/apache/superset/commit/04e4abfe125aa374586e9a2f084deb73986f4164?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (1fd763b) and HEAD (04e4abf). Click for more details.
>
> <details><summary>HEAD has 51 uploads less than BASE</summary>
>
>| Flag | BASE (1fd763b) | HEAD (04e4abf) |
>|------|------|------|
>|hive|7|1|
>|python|41|2|
>|presto|7|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43536 +/- ##
===========================================
- Coverage 79.07% 57.46% -21.62%
===========================================
Files 2877 2877
Lines 165429 165429
Branches 38228 38228
===========================================
- Hits 130821 95067 -35754
- Misses 32127 69467 +37340
+ Partials 2481 895 -1586
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.99% <100.00%> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.91% <100.00%> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.96% <100.00%> (-43.69%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43536/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43536?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #43536:
URL: https://github.com/apache/superset/pull/43536#discussion_r4410400153
##########
superset/views/user_info.py:
##########
@@ -24,11 +24,21 @@
class UserInfoView(BaseSupersetView):
+ """SPA shell for the signed-in user's own profile page.
+
+ Uses ``can_userinfo on UserInfo`` rather than ``can_read on user`` /
+ ``User``. The latter is Admin-only (``ADMIN_ONLY_VIEW_MENUS``), so
+ Alpha/Gamma were redirected to ``/superset/welcome/`` when opening
+ Settings → Info even though the menu link is shown to every
+ authenticated user. ``can_userinfo`` is already in
+ ``ACCESSIBLE_PERMS``, so stock Gamma and Alpha receive it on role sync.
+ """
+
route_base = "/"
- class_permission_name = "user"
+ class_permission_name = "UserInfo"
Review Comment:
This moves `/user_info/` to `can_userinfo on UserInfo`, but `_profile_page_reachable()` in `superset/security/password_change.py:192` still checks `has_access("can_read", "user")`. With `ENABLE_FORCE_PASSWORD_CHANGE` on, a flagged Gamma/Alpha user now holds the new permission but not the old one, so the enforcement hook treats the profile page as unreachable and sends them to logout instead of the password-change page (the very flow this page serves). Should that check, its docstring, and the mocked case at `tests/unit_tests/security/test_password_change.py:355` move to the new permission in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43536:
URL: https://github.com/apache/superset/pull/43536#discussion_r3860200946
##########
superset/views/user_info.py:
##########
@@ -24,11 +24,21 @@
class UserInfoView(BaseSupersetView):
+ """SPA shell for the signed-in user's own profile page.
+
+ Uses ``can_userinfo on UserInfo`` rather than ``can_read on user`` /
+ ``User``. The latter is Admin-only (``ADMIN_ONLY_VIEW_MENUS``), so
+ Alpha/Gamma were redirected to ``/superset/welcome/`` when opening
+ Settings → Info even though the menu link is shown to every
+ authenticated user. ``can_userinfo`` is already in
+ ``ACCESSIBLE_PERMS``, so stock Gamma and Alpha receive it on role sync.
+ """
+
route_base = "/"
- class_permission_name = "user"
+ class_permission_name = "UserInfo"
Review Comment:
**Suggestion:** Changing the view-menu to `UserInfo` creates a new permission target, but existing custom roles that were granted the previous UserInfo endpoint permission will not receive or retain access to this new PVM. After upgrading and syncing, those users can be denied access to `/user_info/`. Preserve the legacy permission during migration or make the authorization accept the old target as a compatibility path. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Existing custom roles may lose Settings → Info access.
- ⚠️ Stock Alpha/Gamma roles receive the new permission.
- ⚠️ User-management permissions remain unaffected.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=434ca0d4d19b4e4a8ed0423b831838e9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=434ca0d4d19b4e4a8ed0423b831838e9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/user_info.py
**Line:** 38:38
**Comment:**
*Api Mismatch: Changing the view-menu to `UserInfo` creates a new permission target, but existing custom roles that were granted the previous UserInfo endpoint permission will not receive or retain access to this new PVM. After upgrading and syncing, those users can be denied access to `/user_info/`. Preserve the legacy permission during migration or make the authorization accept the old target as a compatibility path.
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.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43536&comment_hash=f08b9e2d04249b05df7e5945017d674986b05b412cebd9aa2e63e763b2cc3e0e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43536&comment_hash=f08b9e2d04249b05df7e5945017d674986b05b412cebd9aa2e63e763b2cc3e0e&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-6329118106
## CodeAnt PR Risk: Medium Risk
- The PR needs attention before merging.
- Changing the permission target can deny Settings → Info access to custom roles with the legacy grant; no migration or compatibility path is shown.
**Assessed commit:** `20b789312fba`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43536:
URL: https://github.com/apache/superset/pull/43536#issuecomment-5421515950
<!-- Bito Reply -->
The flagged issue is correct. Changing the `class_permission_name` from `user` to `UserInfo` creates a new permission target (`can_userinfo` on `UserInfo`), which will not be automatically granted to existing custom roles that were previously assigned `can_read` on `user` (or the previous `can_userinfo` on `UserDBModelView`).
To resolve this, you can maintain backward compatibility by allowing the security manager to accept the legacy permission target. You can implement this by overriding the `has_access` check or by ensuring the migration script grants the new permission to roles that held the old one. A concise way to handle this in the view is to check for both permissions:
```python
@expose("/user_info/")
@has_access
def list(self) -> FlaskResponse:
# Check for new or legacy permission
if not (self.appbuilder.sm.has_access("can_userinfo", "UserInfo") or
self.appbuilder.sm.has_access("can_read", "user")):
abort(403)
return super().render_app_template()
```
I have checked the PR comments, and there are no other comments to address. Would you like me to fetch all comments to validate and implement fixes for the rest of the PR?
**superset/views/user_info.py**
```
@expose("/user_info/")
@has_access
def list(self) -> FlaskResponse:
# Check for new or legacy permission
if not (self.appbuilder.sm.has_access("can_userinfo", "UserInfo") or
self.appbuilder.sm.has_access("can_read", "user")):
abort(403)
return super().render_app_template()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43536:
URL: https://github.com/apache/superset/pull/43536#discussion_r3867129086
##########
superset/views/user_info.py:
##########
@@ -24,11 +24,21 @@
class UserInfoView(BaseSupersetView):
+ """SPA shell for the signed-in user's own profile page.
+
+ Uses ``can_userinfo on UserInfo`` rather than ``can_read on user`` /
+ ``User``. The latter is Admin-only (``ADMIN_ONLY_VIEW_MENUS``), so
+ Alpha/Gamma were redirected to ``/superset/welcome/`` when opening
+ Settings → Info even though the menu link is shown to every
+ authenticated user. ``can_userinfo`` is already in
+ ``ACCESSIBLE_PERMS``, so stock Gamma and Alpha receive it on role sync.
+ """
+
route_base = "/"
- class_permission_name = "user"
+ class_permission_name = "UserInfo"
Review Comment:
Yes—the concern is valid. Changing `class_permission_name` changes the PVM required by `@has_access` from the legacy target to `can_userinfo` on `UserInfo`. Role synchronization generally adds accessible permissions but does not migrate arbitrary custom-role grants, so custom roles holding the legacy PVM may lose access.
The safest fix is an idempotent security-convergence migration that:
1. Detects roles containing the legacy Settings → Info PVM (for example, `can_userinfo` on `UserDBModelView`/the deployed legacy view target).
2. Grants those roles `can_userinfo` on `UserInfo`.
3. Retains the legacy permission for compatibility; it should not grant `can_read` on `User` or otherwise widen user management.
4. Runs during the normal role-sync/convergence path and is covered by a regression test for a custom role.
A compatibility check in `UserInfoView` could also accept either PVM, but it would leave the permission model ambiguous and make eventual cleanup harder. Migrating legacy grants to the new, purpose-specific PVM preserves existing access while keeping `User` Admin-only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): let Alpha/Gamma open Settings → Info [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #43536:
URL: https://github.com/apache/superset/pull/43536#discussion_r3867125999
##########
superset/views/user_info.py:
##########
@@ -24,11 +24,21 @@
class UserInfoView(BaseSupersetView):
+ """SPA shell for the signed-in user's own profile page.
+
+ Uses ``can_userinfo on UserInfo`` rather than ``can_read on user`` /
+ ``User``. The latter is Admin-only (``ADMIN_ONLY_VIEW_MENUS``), so
+ Alpha/Gamma were redirected to ``/superset/welcome/`` when opening
+ Settings → Info even though the menu link is shown to every
+ authenticated user. ``can_userinfo`` is already in
+ ``ACCESSIBLE_PERMS``, so stock Gamma and Alpha receive it on role sync.
+ """
+
route_base = "/"
- class_permission_name = "user"
+ class_permission_name = "UserInfo"
Review Comment:
Agreed—custom roles retain the old permission during role sync, while this endpoint now requires a different permission/view pair, so affected users lose Settings → Info after an upgrade. Could this include a security-converge migration or compatibility check for roles holding the legacy PVM?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org