[PM-12922] Disable delete if user can't manage collection (#4179)

bitwarden · Nov 6, 2024 · 87d324b · 87d324b
1 parent e397c03
commit 87d324b
Show file tree

Hide file tree

Showing 14 changed files with 503 additions and 13 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,
@@ -1589,6 +1591,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
@@ -1598,6 +1610,7 @@ class VaultAddEditViewModel @Inject constructor(
                             totpData = totpData,
                             resourceManager = resourceManager,
                             clock = clock,
+                            canDelete = canDelete,
                         )
                         ?: viewState)
                         .appendFolderAndOwnerData(
@@ -2026,6 +2039,15 @@ data class VaultAddEditState(
      */
     val isCloneMode: Boolean get() = vaultAddEditType is VaultAddEditType.CloneItem
 
+    /**
+     * Helper to determine if the UI should allow deletion of this item.
+     */
+    val canDelete: Boolean
+        get() = (viewState as? ViewState.Content)
+            ?.common
+            ?.canDelete
+            ?: false
+
     /**
      * Enum representing the main type options for the vault, such as LOGIN, CARD, etc.
      *
@@ -2085,6 +2107,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(
@@ -2101,6 +2124,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,
                         )
                     },
             )
@@ -915,6 +932,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)
 
@@ -1153,6 +1171,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.
      */
@@ -1216,6 +1242,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,
     )
-Original file line number
+Diff line change
@@ Expand Up / @@ -226,7 +226,8 @@ fun VaultItemScreen( @@
                                             )
                                         }
                                     },
-                                ),
+                                )
+                                    .takeIf { state.canDelete },
                             ),
                         )
                     },
@@ Expand Down @@