Skip to content

Commit 5f9ca1a

Browse files
SaintPatrckSaintPatrck
committed
[PM-15970] Allow assigning item if user can edit
Allows assigning items to collections if the user has manage permissions and is not restricted from viewing or editing password in the collection. This fixes an issue where collection assignment would be blocked if the user could view the item and its passwords.
1 parent 3329dfa commit 5f9ca1a

File tree

5 files changed

+170
-62
lines changed

5 files changed

+170
-62
lines changed

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

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.x8bit.bitwarden.ui.vault.feature.util
22

33
import com.bitwarden.vault.CollectionView
4+
import com.x8bit.bitwarden.ui.vault.feature.util.model.CollectionPermission
45

56
private const val COLLECTION_DIVIDER: String = "/"
67

@@ -110,15 +111,43 @@ fun List<CollectionView>?.hasDeletePermissionInAtLeastOneCollection(
110111
* Checks if the user has permission to assign an item to a collection.
111112
*
112113
* Assigning to a collection is not allowed when the item is in a collection that the user does not
113-
* have "manage" and "edit" permission for.
114+
* have "manage" permission for and is also in a collection they cannot view the passwords in.
115+
*
116+
* E.g., If an item is in A collection with "view except passwords" or "edit except passwords"
117+
* permission and in another with "manage" permission, the user **cannot** assign the item to other
118+
* collections. Conversely, if an item is in a collection with "manage" permission and another with
119+
* "view" or "edit" permission, the user **can** assign the item to other collections.
114120
*/
115-
fun List<CollectionView>?.canAssignToCollections(currentCollectionIds: List<String>?) =
116-
this
117-
?.none {
118-
val itemIsInCollection = currentCollectionIds
119-
?.contains(it.id)
120-
?: false
121-
122-
itemIsInCollection && (!it.manage || it.readOnly)
123-
}
124-
?: true
121+
fun List<CollectionView>?.canAssignToCollections(currentCollectionIds: List<String>?): Boolean {
122+
if (this.isNullOrEmpty()) return true
123+
if (currentCollectionIds.isNullOrEmpty()) return true
124+
125+
// Verify user can MANAGE at least one collection the item is in.
126+
return this
127+
.any {
128+
currentCollectionIds.contains(it.id) &&
129+
it.permission == CollectionPermission.MANAGE
130+
} &&
131+
132+
// Verify user does not have "edit except password" or "view except passwords"
133+
// permission in any collection the item is not in.
134+
this
135+
.none {
136+
currentCollectionIds.contains(it.id) &&
137+
(it.permission == CollectionPermission.EDIT_EXCEPT_PASSWORD ||
138+
it.permission == CollectionPermission.VIEW_EXCEPT_PASSWORDS)
139+
}
140+
}
141+
142+
/**
143+
* Determines the user's permission level for a given [CollectionView].
144+
*/
145+
val CollectionView.permission: CollectionPermission
146+
get() = when {
147+
manage -> CollectionPermission.MANAGE
148+
readOnly && hidePasswords -> CollectionPermission.VIEW_EXCEPT_PASSWORDS
149+
readOnly -> CollectionPermission.VIEW
150+
!readOnly && hidePasswords -> CollectionPermission.EDIT_EXCEPT_PASSWORD
151+
// !readOnly is the only other possible condition, which resolves to EDIT permission
152+
else -> CollectionPermission.EDIT
153+
}

‎app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/util/model/CollectionPermission.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package com.x8bit.bitwarden.ui.vault.feature.util.model
2+
3+
/**
4+
* Represents the permission levels a user can be assigned to a collection.
5+
*/
6+
enum class CollectionPermission {
7+
VIEW,
8+
VIEW_EXCEPT_PASSWORDS,
9+
EDIT,
10+
EDIT_EXCEPT_PASSWORD,
11+
MANAGE,
12+
}

‎app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/CollectionViewUtil.kt

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,64 @@ fun createMockCollectionView(
1010
name: String? = null,
1111
readOnly: Boolean = false,
1212
manage: Boolean = true,
13+
hidePasswords: Boolean = false,
1314
): CollectionView =
1415
CollectionView(
1516
id = "mockId-$number",
1617
organizationId = "mockOrganizationId-$number",
17-
hidePasswords = false,
18+
hidePasswords = hidePasswords,
1819
name = name ?: "mockName-$number",
1920
externalId = "mockExternalId-$number",
2021
readOnly = readOnly,
2122
manage = manage,
2223
)
24+
25+
/**
26+
* Create a [CollectionView] configured to reflect MANAGE permission.
27+
*/
28+
fun createManageCollectionView(number: Int) = createMockCollectionView(
29+
number = number,
30+
manage = true,
31+
readOnly = false,
32+
hidePasswords = false,
33+
)
34+
35+
/**
36+
* Create a [CollectionView] configured to reflect EDIT permission.
37+
*/
38+
fun createEditCollectionView(number: Int) = createMockCollectionView(
39+
number = number,
40+
manage = false,
41+
readOnly = false,
42+
hidePasswords = false,
43+
)
44+
45+
/**
46+
* Create a [CollectionView] configured to reflect EDIT_EXCEPT_PASSWORDS permission.
47+
*/
48+
fun createEditExceptPasswordsCollectionView(number: Int) = createMockCollectionView(
49+
number = number,
50+
manage = false,
51+
readOnly = false,
52+
hidePasswords = true,
53+
)
54+
55+
/**
56+
* Create a [CollectionView] configured to reflect VIEW permission.
57+
*/
58+
fun createViewCollectionView(number: Int) = createMockCollectionView(
59+
number = number,
60+
manage = false,
61+
readOnly = true,
62+
hidePasswords = false,
63+
)
64+
65+
/**
66+
* Create a [CollectionView] configured to reflect VIEW_EXCEPT_PASSWORDS permission.
67+
*/
68+
fun createViewExceptPasswordsCollectionView(number: Int) = createMockCollectionView(
69+
number = number,
70+
manage = false,
71+
readOnly = true,
72+
hidePasswords = true,
73+
)

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

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,13 @@ import com.x8bit.bitwarden.data.tools.generator.repository.util.FakeGeneratorRep
4343
import com.x8bit.bitwarden.data.vault.datasource.network.model.OrganizationType
4444
import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
4545
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
46+
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditCollectionView
47+
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditExceptPasswordsCollectionView
48+
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createManageCollectionView
4649
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView
47-
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView
4850
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSdkFido2CredentialList
51+
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewCollectionView
52+
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewExceptPasswordsCollectionView
4953
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
5054
import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
5155
import com.x8bit.bitwarden.data.vault.repository.model.DeleteCipherResult
@@ -1182,18 +1186,15 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
11821186
resourceManager = resourceManager,
11831187
clock = fixedClock,
11841188
canDelete = false,
1185-
canAssignToCollections = true,
1189+
canAssignToCollections = false,
11861190
)
11871191
} returns stateWithName.viewState
11881192

11891193
mutableVaultDataFlow.value = DataState.Loaded(
11901194
data = createVaultData(
11911195
cipherView = cipherView,
11921196
collectionViewList = listOf(
1193-
createMockCollectionView(
1194-
number = 1,
1195-
manage = false,
1196-
),
1197+
createEditCollectionView(number = 1),
11971198
),
11981199
),
11991200
)
@@ -1223,6 +1224,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
12231224
fun `in edit mode, canDelete should be true when cipher is in a collection the user can manage`() =
12241225
runTest {
12251226
val cipherView = createMockCipherView(1)
1227+
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
12261228
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
12271229
val stateWithName = createVaultAddItemState(
12281230
vaultAddEditType = vaultAddEditType,
@@ -1258,16 +1260,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
12581260
data = createVaultData(
12591261
cipherView = cipherView,
12601262
collectionViewList = listOf(
1261-
createMockCollectionView(
1262-
number = 1,
1263-
manage = true,
1264-
readOnly = false,
1265-
),
1266-
createMockCollectionView(
1267-
number = 2,
1268-
manage = false,
1269-
readOnly = true,
1270-
),
1263+
createManageCollectionView(number = 1),
1264+
createViewCollectionView(number = 2),
12711265
),
12721266
),
12731267
)
@@ -1294,7 +1288,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
12941288

12951289
@Suppress("MaxLineLength")
12961290
@Test
1297-
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user cannot manage or edit`() =
1291+
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection with view permission`() =
12981292
runTest {
12991293
val cipherView = createMockCipherView(1)
13001294
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
@@ -1332,11 +1326,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
13321326
data = createVaultData(
13331327
cipherView = cipherView,
13341328
collectionViewList = listOf(
1335-
createMockCollectionView(
1336-
number = 1,
1337-
readOnly = true,
1338-
manage = false,
1339-
),
1329+
createViewCollectionView(number = 1),
13401330
),
13411331
),
13421332
)
@@ -1363,9 +1353,10 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
13631353

13641354
@Suppress("MaxLineLength")
13651355
@Test
1366-
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user cannot manage but can edit`() =
1356+
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection with manage permission and a collection with edit, except password permission`() =
13671357
runTest {
13681358
val cipherView = createMockCipherView(1)
1359+
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
13691360
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
13701361
val stateWithName = createVaultAddItemState(
13711362
vaultAddEditType = vaultAddEditType,
@@ -1392,7 +1383,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
13921383
totpData = null,
13931384
resourceManager = resourceManager,
13941385
clock = fixedClock,
1395-
canDelete = false,
1386+
canDelete = true,
13961387
canAssignToCollections = false,
13971388
)
13981389
} returns stateWithName.viewState
@@ -1401,11 +1392,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
14011392
data = createVaultData(
14021393
cipherView = cipherView,
14031394
collectionViewList = listOf(
1404-
createMockCollectionView(
1405-
number = 1,
1406-
manage = false,
1407-
readOnly = false,
1408-
),
1395+
createManageCollectionView(number = 1),
1396+
createEditExceptPasswordsCollectionView(number = 2),
14091397
),
14101398
),
14111399
)
@@ -1424,17 +1412,18 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
14241412
totpData = null,
14251413
resourceManager = resourceManager,
14261414
clock = fixedClock,
1427-
canDelete = false,
1415+
canDelete = true,
14281416
canAssignToCollections = false,
14291417
)
14301418
}
14311419
}
14321420

14331421
@Suppress("MaxLineLength")
14341422
@Test
1435-
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user can manage but cannot edit`() =
1423+
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection with manage permission and a collection with view, except password permission`() =
14361424
runTest {
14371425
val cipherView = createMockCipherView(1)
1426+
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
14381427
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
14391428
val stateWithName = createVaultAddItemState(
14401429
vaultAddEditType = vaultAddEditType,
@@ -1470,11 +1459,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
14701459
data = createVaultData(
14711460
cipherView = cipherView,
14721461
collectionViewList = listOf(
1473-
createMockCollectionView(
1474-
number = 1,
1475-
manage = true,
1476-
readOnly = true,
1477-
),
1462+
createManageCollectionView(number = 1),
1463+
createViewExceptPasswordsCollectionView(number = 2),
14781464
),
14791465
),
14801466
)

0 commit comments

Comments
 (0)