Skip to content

Commit 6d976be

Browse files
authored
[PM-21577] Handle organization limitItemDeletion from sync response. (#5244)
1 parent d5c0412 commit 6d976be

File tree

10 files changed

+127
-13
lines changed

10 files changed

+127
-13
lines changed

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/model/Organization.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import com.bitwarden.network.model.OrganizationType
1212
* @property shouldUseKeyConnector Indicates that the organization uses a key connector.
1313
* @property role The user's role in the organization.
1414
* @property keyConnectorUrl The key connector domain (if applicable).
15+
* @property userIsClaimedByOrganization Indicates that the user is claimed by the organization.
16+
* @property limitItemDeletion Indicates that the organization limits item deletion.
1517
*/
1618
data class Organization(
1719
val id: String,
@@ -21,4 +23,5 @@ data class Organization(
2123
val role: OrganizationType,
2224
val keyConnectorUrl: String?,
2325
val userIsClaimedByOrganization: Boolean,
26+
val limitItemDeletion: Boolean = false,
2427
)

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/util/SyncResponseJsonExtensions.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ fun SyncResponseJson.Profile.Organization.toOrganization(): Organization =
2424
shouldManageResetPassword = this.permissions.shouldManageResetPassword,
2525
keyConnectorUrl = this.keyConnectorUrl,
2626
userIsClaimedByOrganization = this.userIsClaimedByOrganization,
27+
limitItemDeletion = this.limitItemDeletion,
2728
)
2829

2930
/**

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1754,9 +1754,21 @@ class VaultAddEditViewModel @Inject constructor(
17541754
) {
17551755
cipherView.permissions?.delete == true
17561756
} else {
1757+
val needsManagePermission = cipherView
1758+
?.organizationId
1759+
?.let { orgId ->
1760+
currentAccount
1761+
.organizations
1762+
.firstOrNull { it.id == orgId }
1763+
?.limitItemDeletion
1764+
}
1765+
17571766
internalVaultData
17581767
.collectionViewList
1759-
.hasDeletePermissionInAtLeastOneCollection(cipherView?.collectionIds)
1768+
.hasDeletePermissionInAtLeastOneCollection(
1769+
collectionIds = cipherView?.collectionIds,
1770+
needsManagePermission = needsManagePermission == true,
1771+
)
17601772
}
17611773

17621774
val canAssignToCollections = internalVaultData

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,18 @@ class VaultItemViewModel @Inject constructor(
138138
) {
139139
cipherView.permissions?.delete == true
140140
} else {
141+
val needsManagePermission = cipherView
142+
?.organizationId
143+
?.let { orgId ->
144+
userState
145+
?.activeAccount
146+
?.organizations
147+
?.firstOrNull { it.id == orgId }
148+
?.limitItemDeletion
149+
}
141150
collectionsState.data.hasDeletePermissionInAtLeastOneCollection(
142151
collectionIds = cipherView?.collectionIds,
152+
needsManagePermission = needsManagePermission == true,
143153
)
144154
}
145155

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CollectionViewExtensions.kt

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,17 +93,32 @@ fun String.toCollectionDisplayName(list: List<CollectionView>): String {
9393
/**
9494
* Checks if the user has delete permission in at least one collection.
9595
*
96-
* Deletion is allowed when the item is in any collection that the user has "manage" permission for.
96+
* Deletion is allowed when the item is in any collection that the user has "manage" permission for
97+
* if [needsManagePermission] is true. Otherwise, deletion is allowed when the item is in any with
98+
* manage or edit permission.
9799
*/
98100
fun List<CollectionView>?.hasDeletePermissionInAtLeastOneCollection(
99101
collectionIds: List<String>?,
102+
needsManagePermission: Boolean = false,
100103
): Boolean {
101104
if (this.isNullOrEmpty() || collectionIds.isNullOrEmpty()) return true
102105
return this
103106
.any { collectionView ->
104107
collectionIds
105108
.contains(collectionView.id)
106-
.let { isInCollection -> isInCollection && collectionView.manage }
109+
.let { isInCollection ->
110+
if (!isInCollection) {
111+
return false
112+
}
113+
114+
if (needsManagePermission) {
115+
return collectionView.manage
116+
}
117+
118+
return collectionView.manage ||
119+
collectionView.permission == CollectionPermission.EDIT ||
120+
collectionView.permission == CollectionPermission.EDIT_EXCEPT_PASSWORD
121+
}
107122
}
108123
}
109124

app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4752,6 +4752,7 @@ class AuthRepositoryTest {
47524752
every { type } returns OrganizationType.USER
47534753
every { keyConnectorUrl } returns null
47544754
every { userIsClaimedByOrganization } returns false
4755+
every { limitItemDeletion } returns false
47554756
},
47564757
)
47574758
fakeAuthDiskSource.storeOrganizations(userId = USER_ID_1, organizations = organizations)
@@ -4782,6 +4783,7 @@ class AuthRepositoryTest {
47824783
every { type } returns OrganizationType.USER
47834784
every { keyConnectorUrl } returns url
47844785
every { userIsClaimedByOrganization } returns false
4786+
every { limitItemDeletion } returns false
47854787
},
47864788
)
47874789
fakeAuthDiskSource.storeOrganizations(userId = USER_ID_1, organizations = organizations)
@@ -4820,6 +4822,7 @@ class AuthRepositoryTest {
48204822
every { type } returns OrganizationType.USER
48214823
every { keyConnectorUrl } returns url
48224824
every { userIsClaimedByOrganization } returns false
4825+
every { limitItemDeletion } returns false
48234826
},
48244827
)
48254828
fakeAuthDiskSource.storeOrganizations(userId = USER_ID_1, organizations = organizations)
@@ -4861,6 +4864,7 @@ class AuthRepositoryTest {
48614864
every { type } returns OrganizationType.USER
48624865
every { keyConnectorUrl } returns url
48634866
every { userIsClaimedByOrganization } returns false
4867+
every { limitItemDeletion } returns false
48644868
},
48654869
)
48664870
fakeAuthDiskSource.storeOrganizations(userId = USER_ID_1, organizations = organizations)
@@ -4901,6 +4905,7 @@ class AuthRepositoryTest {
49014905
every { type } returns OrganizationType.USER
49024906
every { keyConnectorUrl } returns url
49034907
every { userIsClaimedByOrganization } returns false
4908+
every { limitItemDeletion } returns false
49044909
},
49054910
)
49064911
fakeAuthDiskSource.storeOrganizations(userId = USER_ID_1, organizations = organizations)

app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
416416
specialCircumstanceManager.specialCircumstance =
417417
SpecialCircumstance.ProviderCreateCredential(
418418
createCredentialRequest = createCredentialRequest,
419-
)
419+
)
420420
val fido2ContentState = createCredentialRequest.toDefaultAddTypeContent(
421421
attestationOptions = createMockPasskeyAttestationOptions(number = 1),
422422
isIndividualVaultDisabled = false,
@@ -1467,8 +1467,28 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
14671467

14681468
@Suppress("MaxLineLength")
14691469
@Test
1470-
fun `in edit mode, canDelete should be false when cipher is in a collection the user cannot manage`() =
1470+
fun `in edit mode, canDelete should be false when cipher is in a collection the user cannot manage and org has limitItemDeletion`() =
14711471
runTest {
1472+
val userState = createUserState().copy(
1473+
accounts = listOf(
1474+
createUserState().accounts.first().copy(
1475+
organizations = listOf(
1476+
Organization(
1477+
id = "mockOrganizationId-1",
1478+
name = "Mock Organization Name 1",
1479+
shouldManageResetPassword = false,
1480+
shouldUseKeyConnector = false,
1481+
role = OrganizationType.ADMIN,
1482+
keyConnectorUrl = null,
1483+
userIsClaimedByOrganization = false,
1484+
limitItemDeletion = true,
1485+
),
1486+
),
1487+
),
1488+
),
1489+
)
1490+
mutableUserStateFlow.value = userState
1491+
14721492
val cipherView = createMockCipherView(1)
14731493
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
14741494
val stateWithName = createVaultAddItemState(
@@ -2115,7 +2135,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
21152135
specialCircumstanceManager.specialCircumstance =
21162136
SpecialCircumstance.ProviderCreateCredential(
21172137
createCredentialRequest = mockFido2CredentialRequest,
2118-
)
2138+
)
21192139

21202140
setupFido2CreateRequest()
21212141

@@ -2174,7 +2194,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
21742194
specialCircumstanceManager.specialCircumstance =
21752195
SpecialCircumstance.ProviderCreateCredential(
21762196
createCredentialRequest = mockFidoRequest,
2177-
)
2197+
)
21782198
setupFido2CreateRequest(
21792199
mockCallingAppInfo = mockCallingAppInfo,
21802200
mockCreatePublicKeyCredentialRequest = mockCreatePublicKeyCredentialRequest,
@@ -2254,7 +2274,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
22542274
specialCircumstanceManager.specialCircumstance =
22552275
SpecialCircumstance.ProviderCreateCredential(
22562276
createCredentialRequest = mockFidoRequest,
2257-
)
2277+
)
22582278

22592279
setupFido2CreateRequest()
22602280

@@ -4501,7 +4521,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
45014521
specialCircumstanceManager.specialCircumstance =
45024522
SpecialCircumstance.ProviderCreateCredential(
45034523
createCredentialRequest = mockRequest,
4504-
)
4524+
)
45054525
setupFido2CreateRequest()
45064526
every { authRepository.activeUserId } returns "activeUserId"
45074527
coEvery {
@@ -4529,7 +4549,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
45294549
specialCircumstanceManager.specialCircumstance =
45304550
SpecialCircumstance.ProviderCreateCredential(
45314551
createCredentialRequest = mockRequest,
4532-
)
4552+
)
45334553
every { authRepository.activeUserId } returns "activeUserId"
45344554
coEvery {
45354555
bitwardenCredentialManager.registerFido2Credential(
@@ -4577,7 +4597,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
45774597
specialCircumstanceManager.specialCircumstance =
45784598
SpecialCircumstance.ProviderCreateCredential(
45794599
createCredentialRequest = mockRequest,
4580-
)
4600+
)
45814601
setupFido2CreateRequest()
45824602
every { authRepository.activeUserId } returns "activeUserId"
45834603
coEvery {
@@ -4780,6 +4800,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
47804800
role = OrganizationType.ADMIN,
47814801
keyConnectorUrl = null,
47824802
userIsClaimedByOrganization = false,
4803+
limitItemDeletion = false,
47834804
),
47844805
),
47854806
isBiometricsEnabled = true,

app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/util/CollectionViewExtensionsTest.kt

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,56 @@ class CollectionViewExtensionsTest {
9393

9494
@Suppress("MaxLineLength")
9595
@Test
96-
fun `hasDeletePermissionInAtLeastOneCollection should return false if the user does not have manage permission in at least one collection`() {
96+
fun `hasDeletePermissionInAtLeastOneCollection should return false if the user does not have manage permission in at least one collection and org has limitItemDeletion active`() {
9797
val collectionList: List<CollectionView> = listOf(
9898
createEditCollectionView(number = 1),
9999
createEditExceptPasswordsCollectionView(number = 2),
100100
createViewCollectionView(number = 3),
101101
createViewExceptPasswordsCollectionView(number = 4),
102102
)
103103
val collectionIds = collectionList.mapNotNull { it.id }
104-
assertFalse(collectionList.hasDeletePermissionInAtLeastOneCollection(collectionIds))
104+
assertFalse(
105+
collectionList.hasDeletePermissionInAtLeastOneCollection(
106+
collectionIds = collectionIds,
107+
needsManagePermission = true,
108+
),
109+
)
110+
}
111+
112+
@Suppress("MaxLineLength")
113+
@Test
114+
fun `hasDeletePermissionInAtLeastOneCollection should return true if the user does not have manage permission in at least one collection and org has limitItemDeletion off and has edit permissions`() {
115+
val collectionList: List<CollectionView> = listOf(
116+
createEditCollectionView(number = 1),
117+
createEditExceptPasswordsCollectionView(number = 2),
118+
createViewCollectionView(number = 3),
119+
createViewExceptPasswordsCollectionView(number = 4),
120+
)
121+
val collectionIds = collectionList.mapNotNull { it.id }
122+
assertTrue(
123+
collectionList.hasDeletePermissionInAtLeastOneCollection(
124+
collectionIds = collectionIds,
125+
needsManagePermission = false,
126+
),
127+
)
128+
}
129+
130+
@Suppress("MaxLineLength")
131+
@Test
132+
fun `hasDeletePermissionInAtLeastOneCollection should return false if ids don't match any collection`() {
133+
val collectionList: List<CollectionView> = listOf(
134+
createEditCollectionView(number = 1),
135+
createEditExceptPasswordsCollectionView(number = 2),
136+
createViewCollectionView(number = 3),
137+
createViewExceptPasswordsCollectionView(number = 4),
138+
)
139+
val collectionIds = listOf("notInCollectionId")
140+
assertFalse(
141+
collectionList.hasDeletePermissionInAtLeastOneCollection(
142+
collectionIds = collectionIds,
143+
needsManagePermission = false,
144+
),
145+
)
105146
}
106147

107148
@Suppress("MaxLineLength")

network/src/main/kotlin/com/bitwarden/network/model/SyncResponseJson.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ data class SyncResponseJson(
250250
* @property familySponsorshipValidUntil The family sponsorship valid until
251251
* of the organization (nullable).
252252
* @property status The status of the organization.
253+
* @property limitItemDeletion If the organization limits item deletion.
253254
*/
254255
@Serializable
255256
data class Organization(
@@ -347,6 +348,9 @@ data class SyncResponseJson(
347348

348349
@SerialName("userIsClaimedByOrganization")
349350
val userIsClaimedByOrganization: Boolean = false,
351+
352+
@SerialName("limitItemDeletion")
353+
val limitItemDeletion: Boolean = false,
350354
)
351355

352356
/**

network/src/testFixtures/kotlin/com/bitwarden/network/model/SyncResponseProfileUtil.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ fun createMockOrganization(
9393
familySponsorshipValidUntil: ZonedDateTime? = ZonedDateTime.parse("2023-10-27T12:00:00Z"),
9494
status: OrganizationStatusType = OrganizationStatusType.ACCEPTED,
9595
userIsClaimedByOrganization: Boolean = false,
96+
limitItemDeletion: Boolean = false,
9697
): SyncResponseJson.Profile.Organization =
9798
SyncResponseJson.Profile.Organization(
9899
shouldUsePolicies = shouldUsePolicies,
@@ -126,6 +127,7 @@ fun createMockOrganization(
126127
familySponsorshipValidUntil = familySponsorshipValidUntil,
127128
status = status,
128129
userIsClaimedByOrganization = userIsClaimedByOrganization,
130+
limitItemDeletion = limitItemDeletion,
129131
)
130132

131133
/**

0 commit comments

Comments
 (0)