Skip to content

fix(BA-5277): clean up RBAC permissions on vfolder ownership transfer#10480

Open
fregataa wants to merge 7 commits into25.15from
BA-5277-25.15
Open

fix(BA-5277): clean up RBAC permissions on vfolder ownership transfer#10480
fregataa wants to merge 7 commits into25.15from
BA-5277-25.15

Conversation

@fregataa
Copy link
Member

Summary

  • Clean up ObjectPermissionRow RBAC records for both old and new owners during change_vfolder_ownership, preventing unique constraint violations on re-invite after A→B→A round-trip ownership transfer
  • Fetch old owner UUID alongside folder host in the pre-transfer query
  • Add regression tests verifying RBAC cleanup and round-trip transfer scenarios

Test plan

  • pants test tests/manager/repositories/vfolder/test_vfolder_ownership_transfer.py
  • Verify A→B→A ownership transfer followed by re-invite of B does not raise unique constraint violation

Resolves BA-5277

Previously, change_vfolder_ownership only cleaned up legacy tables
(vfolder_invitations, vfolder_permissions) but not the RBAC table
(ObjectPermissionRow). This caused unique constraint violations when
re-inviting a user after A→B→A round-trip ownership transfer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 10:29
@github-actions github-actions bot added size:XL 500~ LoC comp:manager Related to Manager component labels Mar 24, 2026
Copy link
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

Cleans up RBAC ObjectPermissionRow records during vfolder ownership transfer to prevent unique-constraint conflicts on re-invite after A→B→A round-trip transfers.

Changes:

  • Fetch old owner UUID during the pre-transfer query in change_vfolder_ownership.
  • Delete RBAC object-permission records for both the new owner (invitee records) and old owner during transfer.
  • Add regression tests covering RBAC cleanup and round-trip A→B→A behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
tests/manager/repositories/vfolder/test_vfolder_ownership_transfer.py Adds regression tests validating RBAC cleanup behavior during ownership transfer scenarios.
src/ai/backend/manager/api/vfolder.py Fetches old owner and performs RBAC object-permission cleanup for both owners during ownership transfer.
changes/10480.fix.md Documents the RBAC cleanup fix in the changelog.

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

@github-actions github-actions bot added size:M 30~100 LoC and removed size:XL 500~ LoC labels Mar 24, 2026
fregataa and others added 2 commits March 24, 2026 19:41
… transfer

The try/except IntegrityError in map_entity_to_scope does not recover
PostgreSQL's aborted transaction state. When a stale scope-entity mapping
exists, the INSERT triggers a constraint violation that aborts the
transaction, causing subsequent queries (SELECT permission_groups) to
fail with InFailedSQLTransactionError.

Add unmap_entity_from_scope calls for both old and new owners alongside
the existing delete_object_permission_of_user calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Mar 24, 2026
fregataa and others added 2 commits March 24, 2026 19:52
Old owner cleanup alone is sufficient — the scope_id-based deletion
covers both owner and invitee records since they share the same
user UUID as scope_id.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verifies that old owner RBAC cleanup during each transfer removes
scope-entity mappings and object permissions, so that re-inviting
the user after a round-trip does not hit IntegrityError.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fregataa fregataa requested review from a team and removed request for a team March 24, 2026 10:55
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