aminghadersohi commented on code in PR #44339:
URL: https://github.com/apache/superset/pull/44339#discussion_r4034983392
##########
superset/mcp_service/utils/permissions_utils.py:
##########
@@ -30,3 +31,38 @@ def get_current_user() -> Optional[User]:
return getattr(g, "user", None)
except Exception:
return None
+
+
+def get_user_role_names(user: Any) -> list[str]:
+ """Return the names of every role a user holds, directly or through a
group.
+
+ Follows ``SecurityManager.get_user_roles``, which grants a user the roles
of
+ each of their groups on top of the ones assigned to them, so reading
+ ``User.roles`` alone under-reports what the user can do. Each name is kept
+ once, direct roles first.
+
+ Roles that cannot be read (detached ORM instances, a non-string ``name``)
+ are skipped. A ``groups`` relationship that cannot be read still leaves the
+ direct roles in place.
+ """
+ names: list[str] = []
+
+ def add(roles: Any) -> None:
+ for role in roles or []:
+ try:
+ name = role.name
+ except (AttributeError, DetachedInstanceError):
+ continue
Review Comment:
Skipping here is silent: the code this replaces logged at debug, and
`user/schemas.py`'s own validator still does. A user then under-reports roles
with no diagnostic. Suggest `logger.debug` here and in the `groups` arm — not a
fence, it needs a module logger across two spans.
--
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]
