PM-24245: Remove the restrict-item-deletion-to-can-manage-permission feature flag (#5606)

This commit is contained in:
David Perez 2025-07-29 09:31:13 -05:00 committed by GitHub
parent a70f441064
commit 4ffd41c33f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 3 additions and 303 deletions

View File

@ -29,7 +29,6 @@ fun <T : Any> FlagKey<T>.ListItemContent(
FlagKey.CredentialExchangeProtocolImport,
FlagKey.CredentialExchangeProtocolExport,
FlagKey.CipherKeyEncryption,
FlagKey.RestrictCipherItemDeletion,
FlagKey.UserManagedPrivilegedApps,
FlagKey.RemoveCardPolicy,
-> {
@ -76,7 +75,6 @@ private fun <T : Any> FlagKey<T>.getDisplayLabel(): String = when (this) {
FlagKey.CredentialExchangeProtocolImport -> stringResource(BitwardenString.cxp_import)
FlagKey.CredentialExchangeProtocolExport -> stringResource(BitwardenString.cxp_export)
FlagKey.CipherKeyEncryption -> stringResource(BitwardenString.cipher_key_encryption)
FlagKey.RestrictCipherItemDeletion -> stringResource(BitwardenString.restrict_item_deletion)
FlagKey.UserManagedPrivilegedApps -> {
stringResource(BitwardenString.user_trusted_privileged_app_management)
}

View File

@ -7,7 +7,6 @@ import androidx.credentials.provider.ProviderCreateCredentialRequest
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.bitwarden.core.DateTime
import com.bitwarden.core.data.manager.model.FlagKey
import com.bitwarden.core.data.repository.model.DataState
import com.bitwarden.core.data.repository.util.takeUntilLoaded
import com.bitwarden.network.model.PolicyTypeJson
@ -30,7 +29,6 @@ import com.x8bit.bitwarden.data.credentials.model.CreateCredentialRequest
import com.x8bit.bitwarden.data.credentials.model.Fido2RegisterCredentialResult
import com.x8bit.bitwarden.data.credentials.model.UserVerificationRequirement
import com.x8bit.bitwarden.data.credentials.util.getCreatePasskeyCredentialRequestOrNull
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager
@ -128,7 +126,6 @@ class VaultAddEditViewModel @Inject constructor(
private val organizationEventManager: OrganizationEventManager,
private val networkConnectionManager: NetworkConnectionManager,
private val firstTimeActionManager: FirstTimeActionManager,
private val featureFlagManager: FeatureFlagManager,
) : BaseViewModel<VaultAddEditState, VaultAddEditEvent, VaultAddEditAction>(
// We load the state from the savedStateHandle for testing purposes.
initialState = savedStateHandle[KEY_STATE]
@ -1784,10 +1781,6 @@ class VaultAddEditViewModel @Inject constructor(
vaultData: VaultData?,
userData: UserState?,
): VaultAddEditState {
val restrictCipherItemDeletionEnabled = featureFlagManager
.getFeatureFlag(
FlagKey.RestrictCipherItemDeletion,
)
val internalVaultData = vaultData
?: VaultData(
decryptCipherListResult = DecryptCipherListResult(
@ -1826,9 +1819,7 @@ class VaultAddEditViewModel @Inject constructor(
currentAccount = userData?.activeAccount,
vaultAddEditType = vaultAddEditType,
) { currentAccount, cipherView ->
val canDelete = if (restrictCipherItemDeletionEnabled &&
cipherView?.permissions?.delete != null
) {
val canDelete = if (cipherView?.permissions?.delete != null) {
cipherView.permissions?.delete == true
} else {
val needsManagePermission = cipherView

View File

@ -4,7 +4,6 @@ import android.net.Uri
import android.os.Parcelable
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.bitwarden.core.data.manager.model.FlagKey
import com.bitwarden.core.data.repository.model.DataState
import com.bitwarden.core.data.repository.util.combineDataStates
import com.bitwarden.core.data.repository.util.mapNullable
@ -21,7 +20,6 @@ import com.bitwarden.vault.CipherView
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
@ -75,7 +73,6 @@ class VaultItemViewModel @Inject constructor(
private val organizationEventManager: OrganizationEventManager,
private val environmentRepository: EnvironmentRepository,
private val settingsRepository: SettingsRepository,
private val featureFlagManager: FeatureFlagManager,
private val snackbarRelayManager: SnackbarRelayManager,
) : BaseViewModel<VaultItemState, VaultItemEvent, VaultItemAction>(
// We load the state from the savedStateHandle for testing purposes.
@ -112,10 +109,6 @@ class VaultItemViewModel @Inject constructor(
vaultRepository.collectionsStateFlow,
vaultRepository.foldersStateFlow,
) { cipherViewState, userState, authCodeState, collectionsState, folderState ->
val restrictCipherItemDeletionEnabled = featureFlagManager
.getFeatureFlag(
FlagKey.RestrictCipherItemDeletion,
)
val totpCodeData = authCodeState.data?.let {
TotpCodeItemData(
periodSeconds = it.periodSeconds,
@ -136,9 +129,7 @@ class VaultItemViewModel @Inject constructor(
}
.mapNullable {
val cipherView = cipherViewState.data
val canDelete = if (restrictCipherItemDeletionEnabled &&
cipherView?.permissions?.delete != null
) {
val canDelete = if (cipherView?.permissions?.delete != null) {
cipherView.permissions?.delete == true
} else {
val needsManagePermission = cipherView
@ -156,9 +147,7 @@ class VaultItemViewModel @Inject constructor(
)
}
val canRestore = if (restrictCipherItemDeletionEnabled &&
cipherView?.permissions?.restore != null
) {
val canRestore = if (cipherView?.permissions?.restore != null) {
cipherView.permissions?.restore == true &&
cipherView.deletedDate != null
} else {

View File

@ -146,7 +146,6 @@ class DebugMenuViewModelTest : BaseViewModelTest() {
private val DEFAULT_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf(
FlagKey.CredentialExchangeProtocolImport to true,
FlagKey.CredentialExchangeProtocolExport to true,
FlagKey.RestrictCipherItemDeletion to true,
FlagKey.UserManagedPrivilegedApps to true,
FlagKey.RemoveCardPolicy to true,
)
@ -154,7 +153,6 @@ private val DEFAULT_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf
private val UPDATED_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf(
FlagKey.CredentialExchangeProtocolImport to false,
FlagKey.CredentialExchangeProtocolExport to false,
FlagKey.RestrictCipherItemDeletion to false,
FlagKey.UserManagedPrivilegedApps to false,
FlagKey.RemoveCardPolicy to false,
)

View File

@ -40,7 +40,6 @@ import com.x8bit.bitwarden.data.credentials.model.CreateCredentialRequest
import com.x8bit.bitwarden.data.credentials.model.Fido2RegisterCredentialResult
import com.x8bit.bitwarden.data.credentials.model.UserVerificationRequirement
import com.x8bit.bitwarden.data.credentials.model.createMockCreateCredentialRequest
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager
@ -49,7 +48,6 @@ import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardMan
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.CoachMarkTourType
import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
import com.bitwarden.core.data.manager.model.FlagKey
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance
import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager
@ -204,10 +202,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
every { markCoachMarkTourCompleted(CoachMarkTourType.ADD_LOGIN) } just runs
every { shouldShowAddLoginCoachMarkFlow } returns mutableShouldShowAddLoginCoachMarkFlow
}
private val featureFlagManager: FeatureFlagManager = mockk {
every { getFeatureFlag(key = FlagKey.RestrictCipherItemDeletion) } returns false
}
private val mutableSnackbarDataFlow: MutableSharedFlow<BitwardenSnackbarData> =
bufferedMutableSharedFlow()
private val snackbarRelayManager: SnackbarRelayManager = mockk {
@ -1349,89 +1343,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `in edit mode, canDelete should be false when cipher permission is false`() =
runTest {
val cipherListView = createMockCipherListView(
number = 1,
permissions = createMockSdkCipherPermissions(
delete = false,
restore = false,
),
)
val cipherView = createMockCipherView(1)
.copy(
permissions = createMockSdkCipherPermissions(
delete = false,
restore = false,
),
)
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 {
featureFlagManager.getFeatureFlag(FlagKey.RestrictCipherItemDeletion)
} returns true
coEvery {
vaultRepository.getCipher("mockId-1")
} returns GetCipherResult.Success(cipherView)
every {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = true,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
data = createVaultData(
cipherListView = cipherListView,
collectionViewList = listOf(
createEditCollectionView(number = 1),
),
),
)
createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = vaultAddEditType,
vaultItemCipherType = VaultItemCipherType.LOGIN,
),
)
verify {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = true,
)
}
}
@Test
fun `in edit mode, canDelete should be true when cipher permission is true`() =
runTest {
@ -1463,9 +1374,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
canDelete = true,
),
)
every {
featureFlagManager.getFeatureFlag(FlagKey.RestrictCipherItemDeletion)
} returns true
coEvery {
vaultRepository.getCipher("mockId-1")
@ -1512,92 +1420,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canDelete should be false when cipher is in a collection the user cannot manage and org has limitItemDeletion`() =
runTest {
val userState = createUserState().copy(
accounts = listOf(
createUserState().accounts.first().copy(
organizations = listOf(
Organization(
id = "mockOrganizationId-1",
name = "Mock Organization Name 1",
shouldManageResetPassword = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
keyConnectorUrl = null,
userIsClaimedByOrganization = false,
limitItemDeletion = true,
),
),
),
),
)
mutableUserStateFlow.value = userState
val cipherListView = createMockCipherListView(number = 1)
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,
canAssignToCollections = true,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
data = createVaultData(
cipherListView = cipherListView,
collectionViewList = listOf(
createEditCollectionView(number = 1),
),
),
)
createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = vaultAddEditType,
vaultItemCipherType = VaultItemCipherType.LOGIN,
),
)
verify {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = true,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canDelete should be true when cipher is in a collection the user can manage`() =
@ -1671,73 +1493,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `in edit mode, canAssociateToCollections should be false when cipher is in a collection with view permission`() =
runTest {
val cipherListView = createMockCipherListView(number = 1)
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,
canAssociateToCollections = false,
),
)
every {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = false,
)
} returns stateWithName.viewState
mutableVaultDataFlow.value = DataState.Loaded(
data = createVaultData(
cipherListView = cipherListView,
collectionViewList = listOf(
createViewCollectionView(number = 1),
),
),
)
createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = vaultAddEditType,
vaultItemCipherType = VaultItemCipherType.LOGIN,
),
)
verify {
cipherView.toViewState(
isClone = false,
isIndividualVaultDisabled = false,
totpData = null,
resourceManager = resourceManager,
clock = fixedClock,
canDelete = false,
canAssignToCollections = false,
)
}
}
@Suppress("MaxLineLength")
@Test
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`() =
@ -3553,7 +3308,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
organizationEventManager = organizationEventManager,
networkConnectionManager = networkConnectionManager,
firstTimeActionManager = firstTimeActionManager,
featureFlagManager = featureFlagManager,
)
}
@ -4943,7 +4697,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
organizationEventManager = organizationEventManager,
networkConnectionManager = networkConnectionManager,
firstTimeActionManager = firstTimeActionManager,
featureFlagManager = featureFlagManager,
)
private fun createVaultData(

View File

@ -23,11 +23,9 @@ import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult
import com.x8bit.bitwarden.data.auth.repository.model.Organization
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
import com.bitwarden.core.data.manager.model.FlagKey
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
@ -119,9 +117,6 @@ class VaultItemViewModelTest : BaseViewModelTest() {
every { isIconLoadingDisabled } returns false
every { isIconLoadingDisabledFlow } returns mutableIsIconLoadingDisabledFlow
}
private val featureFlagManager: FeatureFlagManager = mockk {
every { getFeatureFlag(key = FlagKey.RestrictCipherItemDeletion) } returns false
}
private val mutableSnackbarDataFlow: MutableSharedFlow<BitwardenSnackbarData> =
bufferedMutableSharedFlow()
private val snackbarRelayManager: SnackbarRelayManager = mockk {
@ -2154,9 +2149,6 @@ class VaultItemViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
fun `on VaultDataReceive with Loaded and nonnull false permission data should update the ViewState with cipher permissions`() {
val viewState = mockk<VaultItemState.ViewState>()
every {
featureFlagManager.getFeatureFlag(FlagKey.RestrictCipherItemDeletion)
} returns true
every { mockCipherView.organizationId } returns "mockOrganizationId"
every { mockCipherView.collectionIds } returns listOf("mockId-1")
every { mockCipherView.folderId } returns "mockId-1"
@ -2219,9 +2211,6 @@ class VaultItemViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
fun `on VaultDataReceive with Loaded and nonnull true permission data should update the ViewState with cipher permissions`() {
val viewState = mockk<VaultItemState.ViewState>()
every {
featureFlagManager.getFeatureFlag(FlagKey.RestrictCipherItemDeletion)
} returns true
every { mockCipherView.organizationId } returns "mockOrganizationId"
every { mockCipherView.deletedDate } returns Instant.MIN
every { mockCipherView.collectionIds } returns listOf("mockId-1")
@ -2466,7 +2455,6 @@ class VaultItemViewModelTest : BaseViewModelTest() {
organizationEventManager = eventManager,
environmentRepository = environmentRepository,
settingsRepository = settingsRepository,
featureFlagManager = featureFlagManager,
snackbarRelayManager = snackbarRelayManager,
)

View File

@ -27,7 +27,6 @@ fun <T : Any> FlagKey<T>.ListItemContent(
FlagKey.CredentialExchangeProtocolExport,
FlagKey.CredentialExchangeProtocolImport,
FlagKey.RemoveCardPolicy,
FlagKey.RestrictCipherItemDeletion,
FlagKey.UserManagedPrivilegedApps,
-> BooleanFlagItem(
label = flagKey.getDisplayLabel(),
@ -69,7 +68,6 @@ private fun <T : Any> FlagKey<T>.getDisplayLabel(): String = when (this) {
FlagKey.CredentialExchangeProtocolImport -> stringResource(BitwardenString.cxp_import)
FlagKey.CredentialExchangeProtocolExport -> stringResource(BitwardenString.cxp_export)
FlagKey.CipherKeyEncryption -> stringResource(BitwardenString.cipher_key_encryption)
FlagKey.RestrictCipherItemDeletion -> stringResource(BitwardenString.restrict_item_deletion)
FlagKey.UserManagedPrivilegedApps -> {
stringResource(BitwardenString.user_trusted_privileged_app_management)
}

View File

@ -32,7 +32,6 @@ sealed class FlagKey<out T : Any> {
listOf(
CredentialExchangeProtocolImport,
CredentialExchangeProtocolExport,
RestrictCipherItemDeletion,
UserManagedPrivilegedApps,
RemoveCardPolicy,
)
@ -65,14 +64,6 @@ sealed class FlagKey<out T : Any> {
override val defaultValue: Boolean = false
}
/**
* Data object holding the feature flag key to enable the restriction of cipher item deletion
*/
data object RestrictCipherItemDeletion : FlagKey<Boolean>() {
override val keyName: String = "pm-15493-restrict-item-deletion-to-can-manage-permission"
override val defaultValue: Boolean = false
}
/**
* Data object holding the feature flag key to enabled user-managed privileged apps.
*/

View File

@ -20,10 +20,6 @@ class FlagKeyTest {
FlagKey.CipherKeyEncryption.keyName,
"cipher-key-encryption",
)
assertEquals(
FlagKey.RestrictCipherItemDeletion.keyName,
"pm-15493-restrict-item-deletion-to-can-manage-permission",
)
assertEquals(
FlagKey.UserManagedPrivilegedApps.keyName,
"pm-18970-user-managed-privileged-apps",
@ -45,7 +41,6 @@ class FlagKeyTest {
FlagKey.CredentialExchangeProtocolImport,
FlagKey.CredentialExchangeProtocolExport,
FlagKey.CipherKeyEncryption,
FlagKey.RestrictCipherItemDeletion,
FlagKey.UserManagedPrivilegedApps,
FlagKey.RemoveCardPolicy,
FlagKey.BitwardenAuthenticationEnabled,

View File

@ -18,7 +18,6 @@
<string name="restart_onboarding_carousel_details">This will force the change to app state which will cause the first time carousel to show. The carousel will continue to show for any \"new\" account until a login is completed. May need to exit debug menu manually.</string>
<string name="cipher_key_encryption">Cipher Key Encryption</string>
<string name="reset_coach_mark_tour_status">Reset all coach mark tours</string>
<string name="restrict_item_deletion">Restrict item deletion</string>
<string name="generate_crash">Generate crash</string>
<string name="generate_error_report">Generate error report</string>
<string name="error_reports">Error reports</string>