feat: split organization leave vs deletion with owner validation #2344
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change separates organization leave and deletion into distinct operations with proper owner validation to prevent accidental organization loss. - Users can only leave if there's another owner remaining - Validates sole ownership before allowing leave - Never automatically deletes organizations - New explicit `chainloop organization delete` command - Requires organization owner permissions - Includes confirmation dialog for safety - Blocks user deletion if they are sole owner of any organization - Requires users to add other owners or delete orgs first - Prevents accidental organization loss during account cleanup - Added `OrganizationService.Delete()` RPC for explicit deletion - Updated `MembershipService.Leave()` behavior with validation - Removed automatic organization deletion from leave operations - Clear error messages guide users on required actions - Validation prevents unsafe operations - Helpful suggestions for resolving ownership conflicts Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
migmartri
force-pushed
the
872-org-leave
branch
from
09d4069 to
705fb19
Compare
- Update user integration test to expect account deletion to be blocked when user is sole owner - Update membership integration test to expect no cleanup when sole owner cannot leave - Update copyright headers to 2024-2025 for modified test files - Set proper owner roles in test setup to trigger validation logic Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Split TestDeleteUser into two scenarios: blocked and allowed deletion - Test validates that users can be deleted when they are not sole owners - Ensures both the safety validation and normal deletion flow work correctly Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Added TestLeaveHappyPath to demonstrate successful leave scenarios - Test validates that owners can leave when other owners remain - Test confirms that after successful leave, sole remaining owner cannot leave - Provides clear documentation of both allowed and blocked leave operations Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Format integration test files for proper Go style - Format new CLI command and action files - Ensure consistent code formatting across all new and modified files Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Replace s.T().Run with s.Run to avoid unused testing.T parameter - Fix unused parameter warnings in TestLeaveHappyPath test functions - Address revive linter warnings for better code quality Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Update role expectation from RoleViewer to RoleOwner in test assertion - Fix 'no memberships' test case to work with sole owner validation - Create proper test scenario where user can leave without being sole owner - Use organization deletion for cleanup when user is sole owner Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
- Format all Go code to ensure consistent formatting - Fix any remaining formatting issues across the codebase Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements the requested feature to split organization leave vs organization deletion into separate processes with proper owner validation to prevent accidental organization loss.
Behavior Changes
Before
After
Key Features
Safe Organization Leave
Explicit Organization Deletion
chainloop organization delete --name <org>CLI commandAccount Deletion Safety
API Changes
OrganizationService.Delete()RPC for explicit organization deletionMembershipService.Leave()with owner validation (never auto-deletes)UserService.DeleteUser()with ownership checksTesting
Files Changed
app/controlplane/api/controlplane/v1/organization.proto- New Delete RPCapp/controlplane/pkg/biz/membership.go- New Leave() function with validationapp/controlplane/pkg/biz/organization.go- DeleteByUser() with owner checksapp/controlplane/pkg/biz/user.go- Safe account deletionapp/cli/cmd/organization_delete.go- New delete command