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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
