Skip to content

feat(BA-4811): update association_scopes_entities on role-user mapping changes#9553

Closed
fregataa wants to merge 2 commits intomainfrom
BA-4811
Closed

feat(BA-4811): update association_scopes_entities on role-user mapping changes#9553
fregataa wants to merge 2 commits intomainfrom
BA-4811

Conversation

@fregataa
Copy link
Copy Markdown
Member

@fregataa fregataa commented Mar 1, 2026

Summary

  • When a role is assigned to a user (assign_role, map_user_to_role), create an association_scopes_entities record (USER scope → ROLE entity) so the role becomes visible to the user through RBAC
  • When a role is revoked from a user (revoke_role), delete the corresponding association_scopes_entities record
  • Add tests verifying association creation on assign, deletion on revoke, correct scope/entity types, and isolation between users

Test plan

  • test_assign_role_creates_association — verifies association record is created
  • test_revoke_role_deletes_association — verifies association record is deleted
  • test_assign_role_association_has_correct_types — verifies USER scope and ROLE entity types
  • test_revoke_preserves_other_associations — verifies revoking one user's role doesn't affect another

Resolves BA-4811

…g changes

When a role is assigned to or unassigned from a user, create/delete the
corresponding association_scopes_entities record so the role becomes
visible to the user through RBAC scope chain traversal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 1, 2026 05:37
@github-actions github-actions Bot added the size:L 100~500 LoC label Mar 1, 2026
@github-actions github-actions Bot added the comp:manager Related to Manager component label Mar 1, 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

This PR implements synchronization between user_roles mutations and association_scopes_entities records. When a role is assigned to a user, a corresponding AssociationScopesEntitiesRow (USER scope → ROLE entity) is now created so the role becomes visible through the RBAC scope chain. When a role is revoked, the association record is deleted. Tests are added to verify the create/delete behaviour.

Changes:

  • assign_role in PermissionDBSource now inserts an AssociationScopesEntitiesRow atomically alongside the UserRoleRow
  • revoke_role in PermissionDBSource now deletes the corresponding AssociationScopesEntitiesRow when removing a role
  • map_user_to_role in RoleManager is updated to also create the AssociationScopesEntitiesRow, mirroring the new assign_role behaviour

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/ai/backend/manager/repositories/permission_controller/db_source/db_source.py Adds AssociationScopesEntitiesRow creation in assign_role and deletion in revoke_role
src/ai/backend/manager/repositories/permission_controller/role_manager.py Updates map_user_to_role to create the AssociationScopesEntitiesRow after the UserRoleRow
tests/unit/manager/repositories/permission_controller/test_user_role_association.py New test file verifying association creation on assign, deletion on revoke, correct types, and user isolation

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

relation_type=RelationType.AUTO,
),
)

Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The updated map_user_to_role method now creates an AssociationScopesEntitiesRow, but this method is never called anywhere in the codebase. The user creation paths in user/db_source/db_source.py (which are the primary paths for creating user-role mappings) call execute_creator directly instead of _role_manager.map_user_to_role, meaning newly created users will not get the AssociationScopesEntitiesRow record that makes the role visible through RBAC. The change to this method will have no effect until its callers are updated to invoke it instead of execute_creator.

Suggested change
await db_session.flush()

Copilot uses AI. Check for mistakes.
) -> int:
async with db.begin_readonly_session_read_committed() as db_sess:
result = await db_sess.scalar(
sa.select(sa.func.count()).where(
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The _count_associations helper uses sa.select(sa.func.count()).where(...) without an explicit .select_from() call. Throughout the codebase, the established convention is to always pair sa.select(sa.func.count()) with .select_from(TableRow) when counting rows (e.g., sa.select(sa.func.count()).select_from(AssociationScopesEntitiesRow).where(...)). The query happens to work because SQLAlchemy infers the table from the ORM column references in the WHERE clause, but adding .select_from(AssociationScopesEntitiesRow) explicitly before .where(...) would align with the rest of the codebase.

Suggested change
sa.select(sa.func.count()).where(
sa.select(sa.func.count())
.select_from(AssociationScopesEntitiesRow)
.where(

Copilot uses AI. Check for mistakes.
@fregataa fregataa marked this pull request as draft March 1, 2026 05:52
@fregataa
Copy link
Copy Markdown
Member Author

Closing this PR. Currently, roles are not managed as RBAC entities, and only superadmins are allowed to CRUD roles other than their own. Therefore, scope chain traversal for role-user mappings is unnecessary, and there is no need to add association_scopes_entities records for this case.

@fregataa fregataa closed this Mar 23, 2026
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.

2 participants