feat(BA-2274): Apply RBAC validation to VFolder#6360
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces RBAC (Role-Based Access Control) validation to VFolder operations by integrating validators into the processor layer. The changes ensure that user permissions are checked before VFolder operations are executed.
Key changes:
- Added RBAC validators (Single Entity, Scope, and Batch) to the VFolder processor initialization
- Implemented permission checking logic in the repository layer
- Introduced new error handling for RBAC-related authorization failures
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/ai/backend/manager/services/vfolder/processors/vfolder.py |
Updated VFolderProcessors to accept and use ValidatorArgs for RBAC validation |
src/ai/backend/manager/services/processors.py |
Modified ProcessorArgs to include action_validators and refactored Processors.create() to pass validators to VFolderProcessors |
src/ai/backend/manager/server.py |
Instantiated RBAC validators and wired them into ProcessorArgs |
src/ai/backend/manager/repositories/repositories.py |
Added PermissionControllerRepository to the Repositories dataclass |
src/ai/backend/manager/repositories/permission_controller/repository.py |
Added factory method and simplified permission checking logic with documentation |
src/ai/backend/manager/repositories/permission_controller/db_source.py |
Removed unused get_entity_mapped_scopes method |
src/ai/backend/manager/errors/rbac.py |
Created new RBACForbidden error class for permission denial scenarios |
src/ai/backend/manager/actions/validators/rbac/single_entity.py |
Updated validator to check permission result and raise RBACForbidden when invalid |
src/ai/backend/manager/actions/validators/rbac/scope.py |
Updated validator to check permission result and raise RBACForbidden when invalid |
src/ai/backend/manager/actions/validator/args.py |
Created ValidatorArgs dataclass to bundle validator lists |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return await self._db_source.check_object_permission_exist( | ||
| data.user_id, data.target_object_id, data.operation | ||
| ) |
There was a problem hiding this comment.
The refactored check_permission_of_entity method now directly delegates to _db_source.check_object_permission_exist, removing the inline permission checking logic. However, the original inline implementation iterated through roles, object permissions, and permission groups to check permissions comprehensively. Verify that check_object_permission_exist in db_source.py fully replicates this logic, including checking both direct object permissions and scope-based permission groups, to avoid breaking existing permission checks.
| return await self._db_source.check_object_permission_exist( | |
| data.user_id, data.target_object_id, data.operation | |
| ) | |
| # Comprehensive permission check: direct object permissions and scope-based permission groups | |
| # 1. Get all roles assigned to the user | |
| roles = await self._db_source.get_roles_of_user(data.user_id) | |
| for role in roles: | |
| # 2. Check direct object permissions for the role | |
| if await self._db_source.check_object_permission_exist( | |
| role.id, data.target_object_id, data.operation | |
| ): | |
| return True | |
| # 3. Check permission groups (scope-based) | |
| permission_groups = await self._db_source.get_permission_groups_of_role(role.id) | |
| for group in permission_groups: | |
| if await self._db_source.check_scope_permission_exist( | |
| group.scope_id, data.target_object_id, data.operation | |
| ): | |
| return True | |
| return False |
3243144 to
0734b78
Compare
| raise UserNotFound("User not found in context") | ||
|
|
||
| await self._repository.check_permission_in_scope( | ||
| is_valid = await self._repository.check_permission_in_scope( |
There was a problem hiding this comment.
How about juwt valid rather than is_valid.
1a10632 to
2d8c9ea
Compare
9552aac to
4af738e
Compare
|
Closed this PR since it is outdate |
resolves #5760 (BA-2274)
Checklist: (if applicable)
ai.backend.testdocsdirectory