-
Notifications
You must be signed in to change notification settings - Fork 929
[PM-28470] Implement revoke from organization #6383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add new endpoint to revoke user's membership from organization. This endpoint will be used when users decline vault migration and choose to leave the organization. - Add revokeFromOrganization to AuthenticatedOrganizationApi - Add service method to OrganizationService - Add repository method to AuthRepository 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6383 +/- ##
========================================
Coverage 85.51% 85.52%
========================================
Files 764 764
Lines 55174 55309 +135
Branches 7991 8016 +25
========================================
+ Hits 47182 47301 +119
- Misses 5232 5239 +7
- Partials 2760 2769 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| collectionIds = listOfNotNull(defaultUserCollection.id), | ||
| ).getOrElse { return MigratePersonalVaultResult.Failure(it) } | ||
|
|
||
| mutableVaultMigrationDataStateFlow.update { VaultMigrationData.NoMigrationRequired } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there another spot that can be updated with this function call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I understood your question, we need to update the StateFlow on migration and on leaveOrganization (and when user is trying to do this operation but is offline)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if there are other places in this file that can use the new helper method?
| */ | ||
| suspend fun revokeFromOrganization( | ||
| organizationId: String, | ||
| ): LeaveOrganizationResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a different result for this?
What is the actual difference between leaving an org and revoking yourself from an org?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are different, leaving an org you are out of the org, you need to be reinvited.
Revoking an user, they do not have access to anything related to the org but admins can restore their access immediately.
And it should has a different result, it was an oversight.
| ) | ||
|
|
||
| @Suppress("MaxLineLength") | ||
| override suspend fun revokeFromOrganization(organizationId: String): RevokeFromOrganizationResult = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this suppression if you format this line.
override suspend fun revokeFromOrganization(
organizationId: String,
): RevokeFromOrganizationResult =|
|
||
| if (personalCiphers.isEmpty()) { | ||
| mutableVaultMigrationDataStateFlow.update { VaultMigrationData.NoMigrationRequired } | ||
| clearMigrationState() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍

🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-28470
📔 Objective
Added new Revoke from Organization endpoint, replaced leaveOrganization with RevokeFromOrganization endpoint.
Wired navigation from MigrateToMyItemsViewModel to the LeaveOrganizationViewModel
AuthenticatedOrganizationApi.kt - Added revokeFromOrganization endpoint
OrganizationService.kt - Added service interface method
OrganizationServiceImpl.kt - Added service implementation
AuthRepository.kt - Added revokeFromOrganization interface method
AuthRepositoryImpl.kt - Added repository implementation (no longer has leaveOrganization removed - both exist!)
VaultMigrationManager.kt - Added clearMigrationState() method
VaultMigrationManagerImpl.kt - Implemented clearMigrationState()
LeaveOrganizationViewModel.kt - Calls clearMigrationState() on success
MigrateToMyItemsViewModel.kt - Removed unused OrganizationService dependency
MigrateToMyItemsScreen.kt - Updated callback signature to accept org params
MigrateToMyItemsNavigation.kt - Updated destination signature
VaultUnlockedNavigation.kt - Wired up navigation to LeaveOrganization with params
VaultMigrationManagerTest.kt - Added test for clearMigrationState()
LeaveOrganizationViewModelTest.kt - Added verification for state clearing
MigrateToMyItemsViewModelTest.kt - Fixed event structure tests
MigrateToMyItemsScreenTest.kt - Updated callback signatures
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes