feat(BA-2274): apply RBAC validation to VFolder operations#9628
feat(BA-2274): apply RBAC validation to VFolder operations#9628
Conversation
…pe/_scope_id fields Change VFolderScopeAction pattern to compute scope from business context instead of storing it in separate fields: - Remove _scope_type and _scope_id fields from VFolder scope actions: * CreateVFolderAction * ListVFolderAction * ListVFolderActionResult - Each concrete class now computes scope directly from its business fields: * scope_type() always returns ScopeType.USER * scope_id() returns str(self.user_uuid) * target_element() uses USER scope with user_uuid - Remove scope calculation logic from API handlers: * handler.py: Remove scope_type/scope_id computation * vfolder.py: Remove scope_type/scope_id computation - Update VFolder service to not pass removed fields to ActionResult Benefits: - Eliminates redundant fields (_scope_type, _scope_id) - Scope derivation logic co-located with action definition - Enforces "always USER scope" requirement at type level - API handlers now only need to provide user_uuid, not compute scope This follows the RBAC validator pattern established in BA-2946: "always use user id for scope, never use GLOBAL scope". Related: BA-2946 (parent issue), BA-2946-RBAC-validator-implementation-guide.md Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
github-actions
bot
added
size:XL
There was a problem hiding this comment.
Pull request overview
Refactors VFolder scope actions to remove stored _scope_type/_scope_id and derive RBAC scope directly from user_uuid, with corresponding API handler simplifications.
Changes:
- Removed
_scope_type/_scope_idfields from VFolder scope actions/results and stopped passing them from services/handlers. - Updated VFolder action RBAC methods to always return
ScopeType.USERwithuser_uuid. - Added an RBAC validator implementation guide proposal document.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/manager/services/vfolder/services/vfolder.py | Stops including scope fields in ListVFolderActionResult. |
| src/ai/backend/manager/services/vfolder/actions/base.py | Removes _scope_* fields; hard-codes USER scope + USER RBAC element derivation. |
| src/ai/backend/manager/api/vfolder.py | Simplifies REST handlers by removing scope computation/passing. |
| src/ai/backend/manager/api/rest/vfolder/handler.py | Simplifies REST handlers by removing scope computation/passing. |
| proposals/BA-2946-RBAC-validator-implementation-guide.md | Adds a how-to guide for implementing RBAC validators. |
| changes/9628.enhance.md | Changelog entry for the refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def scope_type(self) -> ScopeType: | ||
| return self._scope_type | ||
| return ScopeType.USER | ||
|
|
||
| @override | ||
| def scope_id(self) -> str: | ||
| return self._scope_id | ||
| return str(self.user_uuid) | ||
|
|
||
| @override | ||
| def target_element(self) -> RBACElementRef: | ||
| return RBACElementRef( | ||
| element_type=RBACElementType(self._scope_type.value), | ||
| element_id=self._scope_id, | ||
| element_type=RBACElementType.USER, | ||
| element_id=str(self.user_uuid), | ||
| ) |
There was a problem hiding this comment.
This change makes VFolder RBAC scope/target always USER, but CreateVFolderAction still carries group_id_or_name (project context) and the APIs still expose group_id for listing. Previously, the handler derived PROJECT scope when a group was provided; hard-coding USER here can weaken authorization and/or mis-scope permission checks for project-scoped VFolders. A safer refactor that still removes redundant fields is to derive scope_type()/scope_id()/target_element() from existing action inputs (e.g., return PROJECT when group_id_or_name/group_id is set, else USER), rather than storing _scope_* on the action.
| def scope_type(self) -> ScopeType: | ||
| return self._scope_type | ||
| return ScopeType.USER | ||
|
|
||
| @override | ||
| def scope_id(self) -> str: | ||
| return self._scope_id | ||
| return str(self.user_uuid) | ||
|
|
||
| @override | ||
| def target_element(self) -> RBACElementRef: | ||
| return RBACElementRef( | ||
| element_type=RBACElementType(self._scope_type.value), | ||
| element_id=self._scope_id, | ||
| element_type=RBACElementType.USER, | ||
| element_id=str(self.user_uuid), | ||
| ) |
There was a problem hiding this comment.
This change makes VFolder RBAC scope/target always USER, but CreateVFolderAction still carries group_id_or_name (project context) and the APIs still expose group_id for listing. Previously, the handler derived PROJECT scope when a group was provided; hard-coding USER here can weaken authorization and/or mis-scope permission checks for project-scoped VFolders. A safer refactor that still removes redundant fields is to derive scope_type()/scope_id()/target_element() from existing action inputs (e.g., return PROJECT when group_id_or_name/group_id is set, else USER), rather than storing _scope_* on the action.
| result = await root_ctx.processors.vfolder.list_vfolder.wait_for_complete( | ||
| ListVFolderAction( | ||
| user_uuid=owner_user_uuid, | ||
| _scope_type=scope_type, | ||
| _scope_id=scope_id, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The list endpoint still accepts a group_id (removed scope derivation logic just above), but ListVFolderAction is now instantiated with only user_uuid, so the service/validators have no way to distinguish user-scope vs project-scope listing. If project filtering is still supported, include the group identifier in ListVFolderAction (and derive scope from it), or explicitly remove/disable the group_id behavior at the API layer to avoid silently changing semantics.
Apply RBAC permission validation to VFolder scope and single-entity actions:
Changes:
- Add ScopeActionRBACValidator and SingleEntityActionRBACValidator to VFolderProcessors
- Add permission_repository parameter to VFolderProcessors.__init__
- Connect validators to ActionProcessor instances:
* Scope actions (2): create_vfolder, list_vfolder
* Single entity actions (8): get_vfolder, update_vfolder_attribute,
move_to_trash_vfolder, restore_vfolder_from_trash, delete_forever_vfolder,
purge_vfolder, force_delete_vfolder, clone_vfolder
- Update Processors.create() to pass permission_repository to VFolderProcessors
- Replace ScopeActionProcessor/SingleEntityActionProcessor with ActionProcessor
Implementation follows the pattern established in BA-2946 session RBAC work:
- Three-tier organization: RBAC scope actions, RBAC single-entity actions,
internal/legacy actions without validation
- Validators check USER scope permissions before allowing operations
- Storage ops and internal actions remain without RBAC validation
Benefits:
- Enforces permission checks at processor level
- Prevents unauthorized vfolder access/modification
- Consistent with session RBAC implementation
- Separates permission logic from business logic
Related: BA-2946, BA-2946-RBAC-validator-implementation-guide.md
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
fregataa
changed the title
…scope_type/_scope_id fields" This reverts commit 37f4ec8.
VFolderProcessors previously created RBAC validators internally from PermissionControllerRepository, violating dependency injection principles. Changes: - Create RBACValidators dataclass in rbac/__init__.py - Update VFolderProcessors to accept RBACValidators via constructor - Add rbac_validators field to ProcessorArgs - Inject validators from server.py and composer.py This follows the same pattern as action_monitors injection, moving validator instantiation to the dependency injection layer. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
This follows the RBAC validator pattern established in BA-2946 session implementation.
Test plan
Resolves BA-2274