[PM-15970] Allow assigning collections if user has correct permissions (#4461)

This commit is contained in:
Patrick Honkonen
2024-12-20 12:33:09 -05:00
committed by GitHub
parent a7939414ae
commit 35e8cecdcf
5 changed files with 169 additions and 61 deletions

View File

@@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.vault.feature.util
import com.bitwarden.vault.CollectionView
import com.x8bit.bitwarden.ui.vault.feature.util.model.CollectionPermission
private const val COLLECTION_DIVIDER: String = "/"
@@ -110,15 +111,43 @@ fun List<CollectionView>?.hasDeletePermissionInAtLeastOneCollection(
* Checks if the user has permission to assign an item to a collection.
*
* Assigning to a collection is not allowed when the item is in a collection that the user does not
* have "manage" and "edit" permission for.
* have "manage" permission for and is also in a collection they cannot view the passwords in.
*
* E.g., If an item is in A collection with "view except passwords" or "edit except passwords"
* permission and in another with "manage" permission, the user **cannot** assign the item to other
* collections. Conversely, if an item is in a collection with "manage" permission and another with
* "view" or "edit" permission, the user **can** assign the item to other collections.
*/
fun List<CollectionView>?.canAssignToCollections(currentCollectionIds: List<String>?) =
this
?.none {
val itemIsInCollection = currentCollectionIds
?.contains(it.id)
?: false
fun List<CollectionView>?.canAssignToCollections(currentCollectionIds: List<String>?): Boolean {
if (this.isNullOrEmpty()) return true
if (currentCollectionIds.isNullOrEmpty()) return true
itemIsInCollection && (!it.manage || it.readOnly)
}
?: true
// Verify user can MANAGE at least one collection the item is in.
return this
.any {
currentCollectionIds.contains(it.id) &&
it.permission == CollectionPermission.MANAGE
} &&
// Verify user does not have "edit except password" or "view except passwords"
// permission in any collection the item is not in.
this
.none {
currentCollectionIds.contains(it.id) &&
(it.permission == CollectionPermission.EDIT_EXCEPT_PASSWORD ||
it.permission == CollectionPermission.VIEW_EXCEPT_PASSWORDS)
}
}
/**
* Determines the user's permission level for a given [CollectionView].
*/
val CollectionView.permission: CollectionPermission
get() = when {
manage -> CollectionPermission.MANAGE
readOnly && hidePasswords -> CollectionPermission.VIEW_EXCEPT_PASSWORDS
readOnly -> CollectionPermission.VIEW
!readOnly && hidePasswords -> CollectionPermission.EDIT_EXCEPT_PASSWORD
// !readOnly is the only other possible condition, which resolves to EDIT permission
else -> CollectionPermission.EDIT
}

View File

@@ -0,0 +1,12 @@
package com.x8bit.bitwarden.ui.vault.feature.util.model
/**
* Represents the permission levels a user can be assigned to a collection.
*/
enum class CollectionPermission {
VIEW,
VIEW_EXCEPT_PASSWORDS,
EDIT,
EDIT_EXCEPT_PASSWORD,
MANAGE,
}

View File

@@ -10,13 +10,64 @@ fun createMockCollectionView(
name: String? = null,
readOnly: Boolean = false,
manage: Boolean = true,
hidePasswords: Boolean = false,
): CollectionView =
CollectionView(
id = "mockId-$number",
organizationId = "mockOrganizationId-$number",
hidePasswords = false,
hidePasswords = hidePasswords,
name = name ?: "mockName-$number",
externalId = "mockExternalId-$number",
readOnly = readOnly,
manage = manage,
)
/**
* Create a [CollectionView] configured to reflect MANAGE permission.
*/
fun createManageCollectionView(number: Int) = createMockCollectionView(
number = number,
manage = true,
readOnly = false,
hidePasswords = false,
)
/**
* Create a [CollectionView] configured to reflect EDIT permission.
*/
fun createEditCollectionView(number: Int) = createMockCollectionView(
number = number,
manage = false,
readOnly = false,
hidePasswords = false,
)
/**
* Create a [CollectionView] configured to reflect EDIT_EXCEPT_PASSWORDS permission.
*/
fun createEditExceptPasswordsCollectionView(number: Int) = createMockCollectionView(
number = number,
manage = false,
readOnly = false,
hidePasswords = true,
)
/**
* Create a [CollectionView] configured to reflect VIEW permission.
*/
fun createViewCollectionView(number: Int) = createMockCollectionView(
number = number,
manage = false,
readOnly = true,
hidePasswords = false,
)
/**
* Create a [CollectionView] configured to reflect VIEW_EXCEPT_PASSWORDS permission.
*/
fun createViewExceptPasswordsCollectionView(number: Int) = createMockCollectionView(
number = number,
manage = false,
readOnly = true,
hidePasswords = true,
)

View File

@@ -43,9 +43,13 @@ import com.x8bit.bitwarden.data.tools.generator.repository.util.FakeGeneratorRep
import com.x8bit.bitwarden.data.vault.datasource.network.model.OrganizationType
import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditExceptPasswordsCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createManageCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSdkFido2CredentialList
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewExceptPasswordsCollectionView
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
import com.x8bit.bitwarden.data.vault.repository.model.DeleteCipherResult
@@ -1182,7 +1186,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = true,
canAssignToCollections = false,
)
} returns stateWithName.viewState
@@ -1190,10 +1194,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = false,
),
createEditCollectionView(number = 1),
),
),
)
@@ -1223,6 +1224,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
fun `in edit mode, canDelete should be true when cipher is in a collection the user can manage`() =
runTest {
val cipherView = createMockCipherView(1)
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
val stateWithName = createVaultAddItemState(
vaultAddEditType = vaultAddEditType,
@@ -1258,16 +1260,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = true,
readOnly = false,
),
createMockCollectionView(
number = 2,
manage = false,
readOnly = true,
),
createManageCollectionView(number = 1),
createViewCollectionView(number = 2),
),
),
)
@@ -1294,7 +1288,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user cannot manage or edit`() =
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection with view permission`() =
runTest {
val cipherView = createMockCipherView(1)
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
@@ -1332,11 +1326,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
readOnly = true,
manage = false,
),
createViewCollectionView(number = 1),
),
),
)
@@ -1363,9 +1353,10 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user cannot manage but can edit`() =
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`() =
runTest {
val cipherView = createMockCipherView(1)
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
val stateWithName = createVaultAddItemState(
vaultAddEditType = vaultAddEditType,
@@ -1392,7 +1383,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canDelete = true,
canAssignToCollections = false,
)
} returns stateWithName.viewState
@@ -1401,11 +1392,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = false,
readOnly = false,
),
createManageCollectionView(number = 1),
createEditExceptPasswordsCollectionView(number = 2),
),
),
)
@@ -1424,7 +1412,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canDelete = true,
canAssignToCollections = false,
)
}
@@ -1432,9 +1420,10 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection the user can manage but cannot edit`() =
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`() =
runTest {
val cipherView = createMockCipherView(1)
.copy(collectionIds = listOf("mockId-1", "mockId-2"))
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
val stateWithName = createVaultAddItemState(
vaultAddEditType = vaultAddEditType,
@@ -1470,11 +1459,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = true,
readOnly = true,
),
createManageCollectionView(number = 1),
createViewExceptPasswordsCollectionView(number = 2),
),
),
)

View File

@@ -1,7 +1,13 @@
package com.x8bit.bitwarden.ui.vault.feature.util
import com.bitwarden.vault.CollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createEditExceptPasswordsCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createManageCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewCollectionView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createViewExceptPasswordsCollectionView
import com.x8bit.bitwarden.ui.vault.feature.util.model.CollectionPermission
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
@@ -73,11 +79,14 @@ class CollectionViewExtensionsTest {
@Test
fun `hasDeletePermissionInAtLeastOneCollection should return true if the user has manage permission in at least one collection`() {
val collectionList: List<CollectionView> = listOf(
createMockCollectionView(number = 1, manage = true),
createMockCollectionView(number = 2, manage = false),
createManageCollectionView(number = 1),
createEditCollectionView(number = 2),
createEditExceptPasswordsCollectionView(number = 3),
createViewCollectionView(number = 4),
createViewExceptPasswordsCollectionView(number = 5),
)
val collectionIds = listOf("mockId-1", "mockId-2")
val collectionIds = collectionList.mapNotNull { it.id }
assertTrue(collectionList.hasDeletePermissionInAtLeastOneCollection(collectionIds))
}
@@ -86,10 +95,12 @@ class CollectionViewExtensionsTest {
@Test
fun `hasDeletePermissionInAtLeastOneCollection should return false if the user does not have manage permission in at least one collection`() {
val collectionList: List<CollectionView> = listOf(
createMockCollectionView(number = 1, manage = false),
createMockCollectionView(number = 2, manage = false),
createEditCollectionView(number = 1),
createEditExceptPasswordsCollectionView(number = 2),
createViewCollectionView(number = 3),
createViewExceptPasswordsCollectionView(number = 4),
)
val collectionIds = listOf("mockId-1", "mockId-2")
val collectionIds = collectionList.mapNotNull { it.id }
assertFalse(collectionList.hasDeletePermissionInAtLeastOneCollection(collectionIds))
}
@@ -104,8 +115,8 @@ class CollectionViewExtensionsTest {
@Test
fun `hasDeletePermissionInAtLeastOneCollection should return true if the collectionIds list is null`() {
val collectionList: List<CollectionView> = listOf(
createMockCollectionView(number = 1, manage = true),
createMockCollectionView(number = 2, manage = false),
createManageCollectionView(number = 1),
createEditCollectionView(number = 2),
)
assertTrue(collectionList.hasDeletePermissionInAtLeastOneCollection(null))
}
@@ -135,20 +146,24 @@ class CollectionViewExtensionsTest {
@Test
fun `canAssociateToCollections should return true if the user has edit and manage permission`() {
val collectionList: List<CollectionView> = listOf(
createMockCollectionView(number = 1, manage = true, readOnly = false),
createEditCollectionView(number = 1),
createManageCollectionView(number = 2),
)
val collectionIds = listOf("mockId-1", "mockId-2")
val collectionIds = collectionList.mapNotNull { it.id }
assertTrue(collectionList.canAssignToCollections(collectionIds))
}
@Suppress("MaxLineLength")
@Test
fun `canAssociateToCollections should return false if the user does not have manage or edit permission in at least one collection`() {
fun `canAssociateToCollections should return false if the user has except password permission at least one collection`() {
val collectionList: List<CollectionView> = listOf(
createMockCollectionView(number = 1, manage = false, readOnly = true),
createMockCollectionView(number = 2, manage = false, readOnly = true),
createEditExceptPasswordsCollectionView(number = 1),
createViewCollectionView(number = 2),
createViewExceptPasswordsCollectionView(number = 3),
createManageCollectionView(number = 4),
createEditCollectionView(number = 5),
)
val collectionIds = listOf("mockId-1", "mockId-2")
val collectionIds = collectionList.mapNotNull { it.id }
assertFalse(collectionList.canAssignToCollections(collectionIds))
}
@@ -166,4 +181,19 @@ class CollectionViewExtensionsTest {
)
assertTrue(collectionList.canAssignToCollections(null))
}
@Test
fun `permission should return correct value`() {
assertEquals(CollectionPermission.MANAGE, createManageCollectionView(number = 1).permission)
assertEquals(CollectionPermission.EDIT, createEditCollectionView(number = 1).permission)
assertEquals(
CollectionPermission.EDIT_EXCEPT_PASSWORD,
createEditExceptPasswordsCollectionView(number = 1).permission,
)
assertEquals(CollectionPermission.VIEW, createViewCollectionView(number = 1).permission)
assertEquals(
CollectionPermission.VIEW_EXCEPT_PASSWORDS,
createViewExceptPasswordsCollectionView(number = 1).permission,
)
}
}