Conversation
Add RoleSearchScope that filters roles visible to a user through direct assignment (user_roles) or RBAC scope chain (association_scopes_entities). Non-admin users now see only their assigned roles; admin users retain full listing capability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates role search to support RBAC-based visibility, enabling non-superadmin users to query only roles they can see (via direct assignment or RBAC associations).
Changes:
- Introduces
RoleSearchScopeand threads it through db_source → repository → service → action. - Updates REST (and GQL fetcher API) to support scoped role search for non-superadmin users.
- Adds unit tests covering RBAC visibility scenarios.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/repositories/permission_controller/test_search_roles.py | Adds RBAC visibility tests for search_roles() using direct and association-based role visibility. |
| src/ai/backend/manager/services/permission_contoller/service.py | Passes action.scope down to repository search. |
| src/ai/backend/manager/services/permission_contoller/actions/search_roles.py | Extends SearchRolesAction with optional scope. |
| src/ai/backend/manager/repositories/permission_controller/types.py | Adds RoleSearchScope that constrains visible roles via RBAC tables. |
| src/ai/backend/manager/repositories/permission_controller/repository.py | Adds scope parameter to search_roles() and forwards it to db_source. |
| src/ai/backend/manager/repositories/permission_controller/db_source/db_source.py | Accepts scope and applies it when listing roles from the query. |
| src/ai/backend/manager/api/rest/rbac/handler.py | Removes superadmin-only restriction; applies RoleSearchScope for non-superadmin requests. |
| src/ai/backend/manager/api/gql/rbac/fetcher/role.py | Adds optional scope parameter and forwards it into SearchRolesAction. |
| changes/9555.feature.md | Adds changelog entry for scoped role search behavior. |
Comments suppressed due to low confidence (1)
src/ai/backend/manager/api/gql/rbac/fetcher/role.py:1
fetch_roles()defaultsscope=None, which will return all roles unless every caller explicitly passes a scope. If this fetcher is reachable by non-superadmin users, it risks bypassing the RBAC visibility constraint added in REST. Consider deriving a defaultRoleSearchScopefrominfo.context(unless superadmin) or makingscoperequired for non-privileged contexts.
"""Role fetcher functions."""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| scope_association = sa.select( | ||
| sa.cast(AssociationScopesEntitiesRow.scope_id, sa.Uuid), | ||
| ).where( | ||
| sa.and_( | ||
| AssociationScopesEntitiesRow.entity_type == EntityType.USER, | ||
| AssociationScopesEntitiesRow.entity_id == user_id_str, | ||
| AssociationScopesEntitiesRow.scope_type == ScopeType.ROLE, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
The RBAC association subquery does not constrain relation_type. If association_scopes_entities contains other relation types (e.g., REF edges), this can broaden visibility and expose roles unintentionally. Consider explicitly filtering to the intended relation type(s) (e.g., RelationType.AUTO) to match the visibility rules.
| """Search roles with filters, orders, and pagination. | ||
|
|
||
| Admin users see all roles. Non-admin users see only roles | ||
| assigned to them via RBAC (user_roles or association_scopes_entities). | ||
| """ | ||
| scope: RoleSearchScope | None = None | ||
| if not ctx.is_superadmin: | ||
| raise NotEnoughPermission("Only superadmin can search roles.") | ||
| scope = RoleSearchScope(user_id=ctx.user_uuid) |
There was a problem hiding this comment.
Docstring says 'Admin users see all roles' but the actual condition is ctx.is_superadmin. If 'admin' and 'superadmin' are distinct in this codebase, the documentation is misleading. Either update the docstring to say 'superadmin' or adjust the condition to match the intended admin behavior.
| class RoleSearchScope(SearchScope): | ||
| """Scope for searching roles visible to a specific user via RBAC. | ||
|
|
||
| A role is visible to a user if: | ||
| - The user has a direct role assignment via user_roles, OR | ||
| - The user is associated with the role scope via association_scopes_entities | ||
| (entity_type=USER, scope_type=ROLE) |
There was a problem hiding this comment.
The PR description mentions an RBAC 'scope chain', but this scope implementation/documentation describes (and implements) only direct visibility via user_roles or a direct association_scopes_entities link to a ROLE scope. If transitive visibility via scope hierarchy is intended, this should be implemented here (or the wording adjusted) to avoid future confusion and incorrect assumptions.
…earchScope RoleSearchScope should filter roles by user_roles table only, not by association_scopes_entities. Simplifies the query to a single subquery on UserRoleRow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep BA-4813 focused on the repository/service layer scope parameter. REST handler changes should be handled separately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Closed this PR since it has already been addressed by #9554 |
Summary
RoleSearchScopethat filters roles visible to a user through direct assignment (user_roles) or RBAC scope chain (association_scopes_entities)search_roles()across all layers (db_source, repository, service, action) to accept optional RBAC scopeTest plan
Resolves BA-4813