[PM-12922] Disable delete if user can't manage collection

Disables the delete button for items in collections where the user does not have "manage" permission. This change ensures that users cannot delete items from collections they are not authorized to manage. It updates the UI to reflect the user's permissions and prevents accidental or unauthorized deletions.
bitwarden · Oct 31, 2024 · 4d2f1c2 · 4d2f1c2
1 parent ce180f1
commit 4d2f1c2
Show file tree

Hide file tree

Showing 15 changed files with 493 additions and 14 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt
@@ -314,7 +314,7 @@ fun VaultAddEditScreen(
                                 text = stringResource(id = R.string.delete),
                                 onClick = { pendingDeleteCipher = true },
                             )
-                                .takeUnless { state.isAddItemMode },
+                                .takeUnless { state.isAddItemMode || !state.canDelete },
                         ),
                     )
                 },

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt
@@ -149,7 +149,9 @@ class VaultAddEditViewModel @Inject constructor(
                                 attestationOptions = fido2AttestationOptions,
                                 isIndividualVaultDisabled = isIndividualVaultDisabled,
                             )
-                            ?: totpData?.toDefaultAddTypeContent(isIndividualVaultDisabled)
+                            ?: totpData?.toDefaultAddTypeContent(
+                                isIndividualVaultDisabled = isIndividualVaultDisabled,
+                            )
                             ?: VaultAddEditState.ViewState.Content(
                                 common = VaultAddEditState.ViewState.Content.Common(),
                                 isIndividualVaultDisabled = isIndividualVaultDisabled,
@@ -1619,6 +1621,16 @@ class VaultAddEditViewModel @Inject constructor(
                     currentAccount = userData?.activeAccount,
                     vaultAddEditType = vaultAddEditType,
                 ) { currentAccount, cipherView ->
+                    val canDelete = vaultData.collectionViewList
+                        .none {
+                            val itemIsInCollection = cipherView
+                                ?.collectionIds
+                                ?.contains(it.id) == true
+
+                            val canManageCollection = it.manage
+
+                            itemIsInCollection && !canManageCollection
+                        }
                     // Derive the view state from the current Cipher for Edit mode
                     // or use the current state for Add
                     (cipherView
@@ -1628,6 +1640,7 @@ class VaultAddEditViewModel @Inject constructor(
                             totpData = totpData,
                             resourceManager = resourceManager,
                             clock = clock,
+                            canDelete = canDelete,
                         )
                         ?: viewState)
                         .appendFolderAndOwnerData(
@@ -2115,6 +2128,7 @@ data class VaultAddEditState(
              * @property selectedOwnerId The ID of the owner associated with the item.
              * @property availableOwners A list of available owners.
              * @property hasOrganizations Indicates if the user is part of any organizations.
+             * @property canDelete Indicates whether the current user can delete the item.
              */
             @Parcelize
             data class Common(
@@ -2131,6 +2145,7 @@ data class VaultAddEditState(
                 val selectedOwnerId: String? = null,
                 val availableOwners: List<Owner> = emptyList(),
                 val hasOrganizations: Boolean = false,
+                val canDelete: Boolean = true,
             ) : Parcelable {
 
                 /**

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt
@@ -35,13 +35,14 @@ private const val PASSKEY_CREATION_TIME_PATTERN: String = "hh:mm a"
 /**
  * Transforms [CipherView] into [VaultAddEditState.ViewState].
  */
-@Suppress("LongMethod")
+@Suppress("LongMethod", "LongParameterList")
 fun CipherView.toViewState(
     isClone: Boolean,
     isIndividualVaultDisabled: Boolean,
     totpData: TotpData?,
     resourceManager: ResourceManager,
     clock: Clock,
+    canDelete: Boolean,
 ): VaultAddEditState.ViewState =
     VaultAddEditState.ViewState.Content(
         type = when (type) {
@@ -108,6 +109,7 @@ fun CipherView.toViewState(
             availableOwners = emptyList(),
             hasOrganizations = false,
             customFieldData = this.fields.orEmpty().map { it.toCustomField() },
+            canDelete = canDelete,
         ),
         isIndividualVaultDisabled = isIndividualVaultDisabled,
     )

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt
@@ -226,7 +226,8 @@ fun VaultItemScreen(
                                         )
                                     }
                                 },
-                            ),
+                            )
+                                .takeIf { state.canDelete },
                         ),
                     )
                 },

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt
@@ -82,7 +82,8 @@ class VaultItemViewModel @Inject constructor(
             vaultRepository.getVaultItemStateFlow(state.vaultItemId),
             authRepository.userStateFlow,
             vaultRepository.getAuthCodeFlow(state.vaultItemId),
-        ) { cipherViewState, userState, authCodeState ->
+            vaultRepository.collectionsStateFlow,
+        ) { cipherViewState, userState, authCodeState, collectionsState ->
             val totpCodeData = authCodeState.data?.let {
                 TotpCodeItemData(
                     periodSeconds = it.periodSeconds,
@@ -96,14 +97,30 @@ class VaultItemViewModel @Inject constructor(
                 vaultDataState = combineDataStates(
                     cipherViewState,
                     authCodeState,
-                ) { _, _ ->
+                    collectionsState,
+                ) { _, _, _ ->
                     // We are only combining the DataStates to know the overall state,
                     // we map it to the appropriate value below.
                 }
                     .mapNullable {
+                        // Deletion is not allowed when the item is in a collection that the user
+                        // does not have "manage" permission for.
+                        val canDelete = collectionsState.data
+                            ?.none {
+                                val itemIsInCollection = cipherViewState.data
+                                    ?.collectionIds
+                                    ?.contains(it.id) == true
+
+                                val canManageCollection = it.manage
+
+                                itemIsInCollection && !canManageCollection
+                            }
+                            ?: true
+
                         VaultItemStateData(
                             cipher = cipherViewState.data,
                             totpCodeItemData = totpCodeData,
+                            canDelete = canDelete,
                         )
                     },
             )
@@ -899,6 +916,7 @@ class VaultItemViewModel @Inject constructor(
             isPremiumUser = account.isPremium,
             hasMasterPassword = account.hasMasterPassword,
             totpCodeItemData = this.data?.totpCodeItemData,
+            canDelete = this.data?.canDelete == true,
         )
         ?: VaultItemState.ViewState.Error(message = errorText)
 
@@ -1137,6 +1155,14 @@ data class VaultItemState(
             ?.isNotEmpty()
             ?: false
 
+    /**
+     * Whether or not the cipher can be deleted.
+     */
+    val canDelete: Boolean
+        get() = viewState.asContentOrNull()
+            ?.common
+            ?.canDelete == true
+
     /**
      * The text to display on the deletion confirmation dialog.
      */
@@ -1200,6 +1226,7 @@ data class VaultItemState(
                 @IgnoredOnParcel
                 val currentCipher: CipherView? = null,
                 val attachments: List<AttachmentItem>?,
+                val canDelete: Boolean,
             ) : Parcelable {
 
                 /**

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/model/VaultItemStateData.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/model/VaultItemStateData.kt
@@ -11,4 +11,5 @@ import com.bitwarden.vault.CipherView
 data class VaultItemStateData(
     val cipher: CipherView?,
     val totpCodeItemData: TotpCodeItemData?,
+    val canDelete: Boolean,
 )
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt
@@ -32,13 +32,14 @@ private const val FIDO2_CREDENTIAL_CREATION_TIME_PATTERN: String = "h:mm a"
 /**
  * Transforms [VaultData] into [VaultState.ViewState].
  */
-@Suppress("CyclomaticComplexMethod", "LongMethod")
+@Suppress("CyclomaticComplexMethod", "LongMethod", "LongParameterList")
 fun CipherView.toViewState(
     previousState: VaultItemState.ViewState.Content?,
     isPremiumUser: Boolean,
     hasMasterPassword: Boolean,
     totpCodeItemData: TotpCodeItemData?,
     clock: Clock = Clock.systemDefaultZone(),
+    canDelete: Boolean,
 ): VaultItemState.ViewState =
     VaultItemState.ViewState.Content(
         common = VaultItemState.ViewState.Content.Common(
@@ -79,6 +80,7 @@ fun CipherView.toViewState(
                     }
                 }
                 .orEmpty(),
+            canDelete = canDelete,
         ),
         type = when (type) {
             CipherType.LOGIN -> {

diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/CollectionViewUtil.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/CollectionViewUtil.kt
@@ -5,13 +5,17 @@ import com.bitwarden.vault.CollectionView
 /**
  * Create a mock [CollectionView] with a given [number].
  */
-fun createMockCollectionView(number: Int, name: String? = null): CollectionView =
+fun createMockCollectionView(
+    number: Int,
+    name: String? = null,
+    manage: Boolean = true,
+): CollectionView =
     CollectionView(
         id = "mockId-$number",
         organizationId = "mockOrganizationId-$number",
         hidePasswords = false,
         name = name ?: "mockName-$number",
         externalId = "mockExternalId-$number",
         readOnly = false,
-        manage = true,
+        manage = manage,
     )