Copilot commented on code in PR #44339:
URL: https://github.com/apache/superset/pull/44339#discussion_r4023752408
##########
superset/mcp_service/system/schemas.py:
##########
@@ -151,23 +151,49 @@ class UserInfo(BaseModel):
roles: list[str] = Field(
default_factory=list,
description=(
- "Role names assigned to the user (e.g., Admin, Alpha, Gamma,
Viewer). "
- "Use this to determine what actions the user can perform."
+ "Role names the user holds, whether assigned directly or through a
"
+ "group (e.g., Admin, Alpha, Gamma, Viewer). Use this to determine "
+ "what actions the user can perform."
),
)
+def _role_names(user: Any) -> list[str]:
+ """Return the names of every role the user holds, direct or through a
group.
+
+ ``User.roles`` only holds directly assigned roles, so a user whose access
+ comes from a group would otherwise look role-less — while the security
+ manager, which reads both, lets them through. Group roles are appended
+ after the direct ones, each name kept once.
+ """
+ names: list[str] = []
+
+ def add(roles: Any) -> None:
+ for role in roles or []:
+ name = getattr(role, "name", None)
+ if name is not None and name not in names:
+ names.append(name)
+
+ try:
+ add(getattr(user, "roles", None))
+ except TypeError:
+ return []
+ try:
+ for group in getattr(user, "groups", None) or []:
+ add(getattr(group, "roles", None))
+ except TypeError:
+ # Group roles are additive: a user object without a usable ``groups``
+ # relationship still reports the roles assigned to it directly.
+ pass
+ return names
+
+
def serialize_user_object(user: Any) -> UserInfo | None:
"""Serialize a user ORM object to UserInfo, extracting role names as
strings."""
if not user:
return None
- user_roles: list[str] = []
- if (raw_roles := getattr(user, "roles", None)) is not None:
- try:
- user_roles = [role.name for role in raw_roles if hasattr(role,
"name")]
- except TypeError:
- user_roles = []
+ user_roles = _role_names(user)
Review Comment:
This helper is only used by the system-level `get_instance_info` serializer.
`get_user_info` and `list_users` import the separate
`superset.mcp_service.user.schemas.serialize_user_object`, which still reads
only `user.roles`, so group-only users will continue to receive `roles: []`
from those tools despite the stated scope of this change. Please share this
effective-role serialization or update the user-directory serializer and its
tests as well.
--
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]
