Skip to content

feat(BA-3696): Apply RBAC validator for User actions#10055

Merged
HyeockJinKim merged 22 commits into
mainfrom
BA-3696
Mar 19, 2026
Merged

feat(BA-3696): Apply RBAC validator for User actions#10055
HyeockJinKim merged 22 commits into
mainfrom
BA-3696

Conversation

@fregataa
Copy link
Copy Markdown
Member

@fregataa fregataa commented Mar 13, 2026

Summary

  • Implemented RBAC validator integration for User service following the established pattern from Group, VFolder, and Session services
  • Scope actions (create, search) now use BaseScopeAction base class with ScopeActionProcessor
  • Single-entity actions (get, update, delete, purge) now use BaseSingleEntityAction base class with SingleEntityActionProcessor
  • Added RBAC-aware base classes UserScopeAction and UserSingleEntityAction to define entity type and required methods
  • Updated all action result classes to inherit from RBAC result bases and implement scope/entity ID methods
  • Wired RBAC validators from ActionValidators to processors for all user actions

Test plan

  • pants fmt passes with no changes
  • pants fix passes with no changes
  • pants lint --changed-since=origin/main passes
  • CI type checks pass
  • CI tests pass

Resolves BA-3696

Copilot AI review requested due to automatic review settings March 13, 2026 03:02
@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component labels Mar 13, 2026
fregataa added a commit that referenced this pull request Mar 13, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Integrates RBAC validation into the User service action pipeline by migrating user actions to RBAC-aware base action/result types and wiring RBAC validators into the corresponding processors, aligning User service behavior with other RBAC-enabled services.

Changes:

  • Switched User scope actions (create/search) to BaseScopeAction + ScopeActionProcessor and single-entity actions (get/update/delete/purge) to BaseSingleEntityAction + SingleEntityActionProcessor.
  • Added User-specific RBAC base action/result classes and updated action results to implement required scope/entity ID methods.
  • Updated REST + legacy GQL call sites and service methods to supply required scope/entity identifiers in action results.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/ai/backend/manager/services/user/service.py Populates RBAC-required scope/entity IDs into action results (domain/project/role/user).
src/ai/backend/manager/services/user/processors.py Wires RBAC validators into scope and single-entity processors for user actions.
src/ai/backend/manager/services/user/actions/base.py Introduces UserScopeAction / UserSingleEntityAction and RBAC result base wrappers.
src/ai/backend/manager/services/user/actions/create_user.py Converts create action/result to scope-based RBAC action/result.
src/ai/backend/manager/services/user/actions/get_user.py Converts get action/result to single-entity RBAC action/result.
src/ai/backend/manager/services/user/actions/modify_user.py Converts modify action/result to single-entity RBAC action/result.
src/ai/backend/manager/services/user/actions/delete_user.py Converts delete action/result to single-entity RBAC action/result.
src/ai/backend/manager/services/user/actions/purge_user.py Converts purge action/result to single-entity RBAC action/result.
src/ai/backend/manager/services/user/actions/search_users_by_domain.py Converts scoped search action/result to RBAC scope action/result (domain).
src/ai/backend/manager/services/user/actions/search_users_by_project.py Converts scoped search action/result to RBAC scope action/result (project).
src/ai/backend/manager/services/user/actions/search_users_by_role.py Converts scoped search action/result to RBAC scope action/result (role).
src/ai/backend/manager/services/user/actions/init.py Exports newly added RBAC base types and role-scoped search action/result.
src/ai/backend/manager/api/rest/user/handler.py Passes required domain scope info when constructing CreateUserAction.
src/ai/backend/manager/api/gql_legacy/user.py Passes required domain scope info when constructing CreateUserAction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ai/backend/manager/services/user/actions/delete_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/delete_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/modify_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/modify_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/purge_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/purge_user.py Outdated
Comment thread src/ai/backend/manager/services/user/actions/create_user.py
fregataa added a commit that referenced this pull request Mar 13, 2026
@github-actions github-actions Bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Mar 13, 2026
fregataa added a commit that referenced this pull request Mar 13, 2026
@fregataa fregataa marked this pull request as draft March 13, 2026 08:44
@github-actions github-actions Bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Mar 13, 2026
@fregataa fregataa marked this pull request as ready for review March 13, 2026 17:47
fregataa added a commit that referenced this pull request Mar 15, 2026
@fregataa fregataa requested a review from a team March 15, 2026 01:10
@fregataa fregataa added this to the 26.4 milestone Mar 16, 2026
fregataa added a commit that referenced this pull request Mar 16, 2026
@fregataa fregataa removed the request for review from a team March 16, 2026 06:09
@fregataa fregataa marked this pull request as draft March 16, 2026 06:09
@fregataa fregataa requested a review from a team March 16, 2026 06:22
@fregataa fregataa marked this pull request as ready for review March 16, 2026 06:22
fregataa and others added 2 commits March 16, 2026 17:31
Added UserScopeAction and UserSingleEntityAction base classes following
the group service pattern. These will be used to apply RBAC validators
to user service actions.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Scope actions (create, search by domain/project/role) now extend UserScopeAction
- Single entity actions (get, modify, delete, purge) now extend UserSingleEntityAction
- Implemented required RBAC methods:
  - scope_type(), scope_id(), target_element() for scope actions
  - target_entity_id(), target_element() for single entity actions
- Updated action result classes to include scope/entity data where needed
- Stats and admin search actions remain on UserAction (no RBAC)
- Bulk actions remain on UserAction (special handling)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
fregataa and others added 18 commits March 16, 2026 17:31
- Use ScopeActionProcessor for scope actions (create, search_by_domain, search_by_project, search_by_role)
- Use SingleEntityActionProcessor for single entity actions (get, modify, delete, purge)
- Pass RBAC validators: validators.rbac.scope and validators.rbac.single_entity
- Keep bulk and internal/stats actions on plain ActionProcessor
- Follow group processors pattern

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…om RBAC result bases

- Add _domain_name field to CreateUserAction (following group pattern)
- Update all result classes to inherit from UserScopeActionResult or UserSingleEntityActionResult
- Implement required scope_type()/scope_id() or target_entity_id() methods on results
- Update service.py to pass required scope/entity IDs to result constructors
- Update API layer (REST, GraphQL) to pass _domain_name when creating CreateUserAction
- Add _user_id field to DeleteUserActionResult and PurgeUserActionResult
- Simplify search result classes to use specific field names (_domain_name, _project_id, _role_id)

All user service files now pass pants fmt, fix, and lint checks.
Type check errors in processors.py are pre-existing (111 errors on main branch).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…g _domain_name parameter

- Remove _domain_name as a required field from CreateUserAction and CreateUserActionResult
- Derive domain_name from creator.spec.domain_name (UserCreatorSpec)
- Use TYPE_CHECKING and cast() to satisfy mypy without runtime imports
- Fixes 8 test call sites that were missing _domain_name parameter

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove the _domain_name keyword from CreateUserAction calls in both
GQL legacy and REST handlers — the field was dropped in favour of
deriving scope info from creator.spec.  Update test conftest files
(user, infra, resource_preset) to properly mock ActionValidators with
async RBAC validator attributes so processor init succeeds.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import UserCreatorSpec at module level (services→repositories is a
valid import direction) and use cast() directly, eliminating the
if-TYPE_CHECKING runtime/static split in scope_id() and
target_element().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit resolves three critical issues:

1. **Circular Import**: Moved `UserCreateSpec` and `UserUpdateSpec` from
   action files to `services/user/types.py` to break circular dependency
   between actions and repository layers.

2. **Email-as-entity-ID in RBAC**: Updated `ModifyUserAction`,
   `DeleteUserAction`, and `PurgeUserAction` to use UUID instead of email
   for RBAC validation. RBAC associations are keyed by user UUID, not email.
   - REST API handler now passes both UUID and email
   - Legacy GraphQL API fetches UUID from email before creating actions
   - Made `user_uuid` field optional with runtime validation for legacy API

3. **Failing Tests**: Fixed import errors in test_action_types.py and
   test_resource.py by resolving the circular import.

Changes:
- services/user/types.py: Added UserCreateSpec, UserUpdateSpec
- services/user/actions/*: Remove duplicate specs, add UUID parameter
- api/rest/user/handler.py: Pass UUID to single-entity actions
- api/gql_legacy/user.py: Fetch UUID from email for RBAC validation
- services/user/service.py: Return UUID in DeleteUserActionResult, PurgeUserActionResult
- repositories/user/*: Import specs from types.py instead of actions

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…constructors

Modified GraphQL legacy user handlers to pass user_uuid parameter to the
to_action() methods instead of trying to set it after construction (which
fails because actions are frozen dataclasses).

Changes:
- ModifyUserInput.to_action(): Accept user_uuid parameter and pass to ModifyUserAction constructor
- PurgeUserInput.to_action(): Accept user_uuid parameter and pass to PurgeUserAction constructor
- ModifyUser.mutate(): Pass user_uuid to props.to_action() call
- PurgeUser.mutate(): Pass user_uuid to props.to_action() call

This ensures RBAC validators receive correct user UUIDs instead of email addresses.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ng user_uuid to DeleteUserAction

Import UserCreateSpec and UserUpdateSpec from their canonical location
(services/user/types) instead of action submodules that don't re-export
them. Add required user_uuid argument to DeleteUserAction calls in tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Search actions are excluded from RBAC validator scope per BA-2946.
Search results are already filtered by scope through the existing
SearchScope mechanism.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert the user_uuid DB lookups added to ModifyUser, DeleteUser, and
PurgeUser mutations in gql_legacy/user.py back to main state. The API
handler layer should not perform direct DB queries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert DeleteUserAction and DeleteUserActionResult back to their
original main branch definitions (simple UserAction/BaseActionResult
base classes with email-only parameter).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove user_uuid kwarg from DeleteUserAction calls (field was removed in revert)
- Revert delete_user processor from SingleEntityActionProcessor to ActionProcessor
  (DeleteUserAction no longer extends BaseSingleEntityAction)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nd add keypair processors

- Move UserCreateSpec back to create_user.py, UserUpdateSpec back to modify_user.py
  (fixes mypy attr-defined error with strict no_implicit_reexport)
- Add missing issue_my_keypair, revoke_my_keypair, switch_my_main_access_key
  processors to UserProcessors
- Update __init__.py re-exports and all import paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fregataa and others added 2 commits March 16, 2026 17:45
…eSpec in types.py

UserCreateSpec/UserUpdateSpec must live in types.py to break circular
dependency: actions/create_user.py → repositories/user/creators.py →
repositories/user/__init__.py → db_source.py → actions/create_user.py.

Add __all__ to create_user.py and modify_user.py to explicitly re-export
the types, satisfying mypy strict no_implicit_reexport.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…epository layer

Repository layer must import from types.py (not action files) to avoid
circular import: actions/create_user.py → repositories/user/ →
db_source.py → actions/create_user.py (still initializing).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@override
def target_entity_id(self) -> str:
if self.user_uuid is None:
raise ValueError("user_uuid must be set for RBAC validation")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that a BackendAIError should be raised.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's handle this as a follow-up task.

@HyeockJinKim HyeockJinKim merged commit 5821d9e into main Mar 19, 2026
33 checks passed
@HyeockJinKim HyeockJinKim deleted the BA-3696 branch March 19, 2026 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants