mirrors/bitwarden-android
1
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2024-11-27 03:49:36 +03:00
Code Issues Projects Releases Packages Wiki Activity

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

Browse source
This commit is contained in:
Patrick Honkonen 2024-11-06 18:42:06 -05:00 committed by GitHub
parent e397c036e4
commit 87d324b063
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 503 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt
View file

@ -314,7 +314,7 @@ fun VaultAddEditScreen(
text = stringResource(id = R.string.delete),
onClick = { pendingDeleteCipher = true },
)
.takeUnless { state.isAddItemMode },
.takeUnless { state.isAddItemMode || !state.canDelete },
),
)
},

26
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt
View file

@ -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 {
/**

4
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt
View file

@ -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,
)

3
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt
View file

@ -226,7 +226,8 @@ fun VaultItemScreen(
)
}
},
),
)
.takeIf { state.canDelete },
),
)
},

31
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt
View file

@ -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 {
/**

1
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/model/VaultItemStateData.kt
View file

@ -11,4 +11,5 @@ import com.bitwarden.vault.CipherView
data class VaultItemStateData(
val cipher: CipherView?,
val totpCodeItemData: TotpCodeItemData?,
val canDelete: Boolean,
)

4
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt
View file

@ -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 -> {

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

@ -5,7 +5,11 @@ 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",
@ -13,5 +17,5 @@ fun createMockCollectionView(number: Int, name: String? = null): CollectionView
name = name ?: "mockName-$number",
externalId = "mockExternalId-$number",
readOnly = false,
manage = true,
manage = manage,
)

140
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt
View file

@ -45,6 +45,7 @@ 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.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.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
@ -1195,6 +1196,136 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canDelete should be false when cipher is in a collection the user cannot manage`() =
runTest {
val cipherView = createMockCipherView(1)
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
val stateWithName = createVaultAddItemState(
vaultAddEditType = vaultAddEditType,
commonContentViewState = createCommonContentViewState(
name = "mockName-1",
originalCipher = cipherView,
customFieldData = listOf(
VaultAddEditState.Custom.HiddenField(
itemId = "testId",
name = "mockName-1",
value = "mockValue-1",
),
),
notes = "mockNotes-1",
canDelete = false,
),
)
every {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = false,
),
),
),
)
createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = vaultAddEditType,
),
)
verify {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canDelete should be true when cipher is in a collection the user can manage`() =
runTest {
val cipherView = createMockCipherView(1)
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
val stateWithName = createVaultAddItemState(
vaultAddEditType = vaultAddEditType,
commonContentViewState = createCommonContentViewState(
name = "mockName-1",
originalCipher = cipherView,
customFieldData = listOf(
VaultAddEditState.Custom.HiddenField(
itemId = "testId",
name = "mockName-1",
value = "mockValue-1",
),
),
notes = "mockNotes-1",
canDelete = true,
),
)
every {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
data = createVaultData(
cipherView = cipherView,
collectionViewList = listOf(
createMockCollectionView(
number = 1,
manage = true,
),
),
),
)
createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = vaultAddEditType,
),
)
verify {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
}
}
@Test
fun `in edit mode, updateCipher success should ShowToast and NavigateBack`() =
runTest {
@ -1310,6 +1441,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
@ -1341,6 +1473,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
}
@ -1375,6 +1508,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
coEvery {
@ -1437,6 +1571,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
coEvery {
@ -1502,6 +1637,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
@ -1558,6 +1694,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
every { fido2CredentialManager.isUserVerified } returns true
@ -1619,6 +1756,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = true,
)
} returns stateWithName.viewState
every { fido2CredentialManager.isUserVerified } returns false
@ -3982,6 +4120,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
availableOwners: List<VaultAddEditState.Owner> = createOwnerList(),
selectedOwnerId: String? = null,
hasOrganizations: Boolean = true,
canDelete: Boolean = true,
): VaultAddEditState.ViewState.Content.Common =
VaultAddEditState.ViewState.Content.Common(
name = name,
@ -3995,6 +4134,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
availableFolders = availableFolders,
availableOwners = availableOwners,
hasOrganizations = hasOrganizations,
canDelete = canDelete,
)
@Suppress("LongParameterList")

7
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensionsTest.kt
View file

@ -75,6 +75,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -121,6 +122,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -172,6 +174,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -232,6 +235,7 @@ class CipherViewExtensionsTest {
totpData = mockk { every { uri } returns totp },
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -289,6 +293,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -324,6 +329,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(
@ -368,6 +374,7 @@ class CipherViewExtensionsTest {
totpData = null,
resourceManager = resourceManager,
clock = FIXED_CLOCK,
canDelete = true,
)
assertEquals(

28
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt
View file

@ -780,22 +780,39 @@ class VaultItemScreenTest : BaseComposeTest() {
}
@Test
fun `menu Delete option click should be displayed`() {
// Confirm dropdown version of item is absent
fun `menu Delete option should be displayed based on state`() {
// Confirm overflow is closed on initial load
composeTestRule
.onAllNodesWithText("Delete")
.filter(hasAnyAncestor(isPopup()))
.assertCountEquals(0)
// Open the overflow menu
composeTestRule
.onNodeWithContentDescription("More")
.performClick()
// Click on the delete item in the dropdown
// Confirm Delete option is present
mutableStateFlow.update { it.copy(viewState = DEFAULT_LOGIN_VIEW_STATE) }
composeTestRule
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isPopup()))
.assertIsDisplayed()
// Confirm Delete option is not present when canDelete is false
mutableStateFlow.update {
it.copy(
viewState = DEFAULT_LOGIN_VIEW_STATE
.copy(
common = DEFAULT_COMMON
.copy(canDelete = false),
),
)
}
composeTestRule
.onAllNodesWithText("Delete")
.filter(hasAnyAncestor(isPopup()))
.assertCountEquals(0)
}
@Test
@ -1166,6 +1183,7 @@ class VaultItemScreenTest : BaseComposeTest() {
@Test
fun `Menu should display correct items when cipher is not in a collection`() {
mutableStateFlow.update { it.copy(viewState = DEFAULT_LOGIN_VIEW_STATE) }
composeTestRule
.onNodeWithContentDescription("More")
.performClick()
@ -2374,6 +2392,7 @@ private val DEFAULT_COMMON: VaultItemState.ViewState.Content.Common =
title = "test.mp4",
),
),
canDelete = true,
)
private val DEFAULT_PASSKEY = R.string.created_xy.asText(
@ -2455,6 +2474,7 @@ private val EMPTY_COMMON: VaultItemState.ViewState.Content.Common =
requiresReprompt = true,
requiresCloneConfirmation = false,
attachments = emptyList(),
canDelete = true,
)
private val EMPTY_LOGIN_TYPE: VaultItemState.ViewState.Content.ItemType.Login =

247
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt
View file

File diff suppressed because it is too large Load diff

13
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensionsTest.kt
View file

@ -45,6 +45,7 @@ class CipherViewExtensionsTest {
totpCode = "testCode",
),
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -80,6 +81,7 @@ class CipherViewExtensionsTest {
totpCode = "testCode",
),
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -108,6 +110,7 @@ class CipherViewExtensionsTest {
totpCode = "testCode",
),
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -136,6 +139,7 @@ class CipherViewExtensionsTest {
totpCode = "testCode",
),
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -170,6 +174,7 @@ class CipherViewExtensionsTest {
totpCode = "testCode",
),
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -194,6 +199,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -216,6 +222,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -237,6 +244,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -268,6 +276,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -304,6 +313,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -342,6 +352,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
@ -364,6 +375,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
val expectedState = VaultItemState.ViewState.Content(
@ -384,6 +396,7 @@ class CipherViewExtensionsTest {
hasMasterPassword = true,
totpCodeItemData = null,
clock = fixedClock,
canDelete = true,
)
assertEquals(
VaultItemState.ViewState.Content(

2
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/util/VaultItemTestUtil.kt
View file

@ -170,6 +170,7 @@ fun createCommonContent(
requiresReprompt = true,
requiresCloneConfirmation = false,
attachments = emptyList(),
canDelete = true,
)
} else {
VaultItemState.ViewState.Content.Common(
@ -213,6 +214,7 @@ fun createCommonContent(
title = "test.mp4",
),
),
canDelete = true,
)
}