From 2c40a7f105c7d01a1daafb2bbe52355a762040eb Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Mon, 11 Nov 2024 11:52:53 -0500 Subject: [PATCH] [PM-14589] Prevent SSH key item creation (#4251) --- .../feature/addedit/VaultAddEditViewModel.kt | 49 ++------ .../addedit/VaultAddEditViewModelTest.kt | 109 +++--------------- 2 files changed, 25 insertions(+), 133 deletions(-) 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 index 1f6b45979..f34ab583d 100644 --- 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 @@ -16,12 +16,10 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialRequest import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager 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.FlagKey import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent import com.x8bit.bitwarden.data.platform.manager.util.toAutofillSaveItemOrNull import com.x8bit.bitwarden.data.platform.manager.util.toAutofillSelectionDataOrNull @@ -103,7 +101,6 @@ class VaultAddEditViewModel @Inject constructor( private val resourceManager: ResourceManager, private val clock: Clock, private val organizationEventManager: OrganizationEventManager, - private val featureFlagManager: FeatureFlagManager, ) : BaseViewModel( // We load the state from the savedStateHandle for testing purposes. initialState = savedStateHandle[KEY_STATE] @@ -170,11 +167,7 @@ class VaultAddEditViewModel @Inject constructor( // Set special conditions for autofill and fido2 save shouldShowCloseButton = autofillSaveItem == null && fido2AttestationOptions == null, shouldExitOnSave = shouldExitOnSave, - supportedItemTypes = getSupportedItemTypeOptions( - isSshKeyVaultItemSupported = featureFlagManager.getFeatureFlag( - key = FlagKey.SshKeyCipherItems, - ), - ), + supportedItemTypes = getSupportedItemTypeOptions(), ) }, ) { @@ -216,11 +209,6 @@ class VaultAddEditViewModel @Inject constructor( } .onEach(::sendAction) .launchIn(viewModelScope) - - featureFlagManager.getFeatureFlagFlow(FlagKey.SshKeyCipherItems) - .map { VaultAddEditAction.Internal.SshKeyCipherItemsFeatureFlagReceive(it) } - .onEach(::sendAction) - .launchIn(viewModelScope) } override fun handleAction(action: VaultAddEditAction) { @@ -1447,10 +1435,6 @@ class VaultAddEditViewModel @Inject constructor( is VaultAddEditAction.Internal.ValidateFido2PinResultReceive -> { handleValidateFido2PinResultReceive(action) } - - is VaultAddEditAction.Internal.SshKeyCipherItemsFeatureFlagReceive -> { - handleSshKeyCipherItemsFeatureFlagReceive(action) - } } } @@ -1785,19 +1769,6 @@ class VaultAddEditViewModel @Inject constructor( getRequestAndRegisterCredential() } - - private fun handleSshKeyCipherItemsFeatureFlagReceive( - action: VaultAddEditAction.Internal.SshKeyCipherItemsFeatureFlagReceive, - ) { - mutableStateFlow.update { - it.copy( - supportedItemTypes = getSupportedItemTypeOptions( - isSshKeyVaultItemSupported = action.enabled, - ), - ) - } - } - //endregion Internal Type Handlers //region Utility Functions @@ -3112,13 +3083,6 @@ sealed class VaultAddEditAction { val generatorResult: GeneratorResult, ) : Internal() - /** - * Indicates that the the SSH key vault item feature flag state has been received. - */ - data class SshKeyCipherItemsFeatureFlagReceive( - val enabled: Boolean, - ) : Internal() - /** * Indicates that the vault item data has been received. */ @@ -3173,7 +3137,10 @@ sealed class VaultAddEditAction { } } -private fun getSupportedItemTypeOptions( - isSshKeyVaultItemSupported: Boolean, -) = VaultAddEditState.ItemTypeOption.entries - .filter { isSshKeyVaultItemSupported || it != VaultAddEditState.ItemTypeOption.SSH_KEYS } +/** + * Returns a list of item type options that can be selected during item creation. + * + * TODO: [PM-10413] Allow SSH key creation when the SDK supports it. + */ +private fun getSupportedItemTypeOptions() = VaultAddEditState.ItemTypeOption.entries + .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt index f7e9d7815..7333f069b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt @@ -25,14 +25,12 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CredentialRe import com.x8bit.bitwarden.data.autofill.model.AutofillSaveItem import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManagerImpl 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.x8bit.bitwarden.data.platform.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.repository.SettingsRepository @@ -155,15 +153,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { private val organizationEventManager = mockk { every { trackEvent(event = any()) } just runs } - private val mutableSshVaultItemsFeatureFlagFlow = MutableStateFlow(true) - private val featureFlagManager = mockk { - every { - getFeatureFlagFlow(key = FlagKey.SshKeyCipherItems) - } returns mutableSshVaultItemsFeatureFlagFlow - every { - getFeatureFlag(key = FlagKey.SshKeyCipherItems) - } returns mutableSshVaultItemsFeatureFlagFlow.value - } @BeforeEach fun setup() { @@ -180,6 +169,20 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { @Test fun `initial state should be correct when state is null`() = runTest { + val expectedState = VaultAddEditState( + vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN), + viewState = VaultAddEditState.ViewState.Content( + common = VaultAddEditState.ViewState.Content.Common(), + isIndividualVaultDisabled = false, + type = VaultAddEditState.ViewState.Content.ItemType.Login(), + ), + dialog = null, + totpData = null, + shouldShowCloseButton = true, + shouldExitOnSave = false, + supportedItemTypes = VaultAddEditState.ItemTypeOption.entries + .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }, + ) val viewModel = createAddVaultItemViewModel( savedStateHandle = createSavedStateHandleWithState( state = null, @@ -188,10 +191,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { ) viewModel.stateFlow.test { assertEquals( - createVaultAddItemState( - commonContentViewState = VaultAddEditState.ViewState.Content.Common(), - typeContentViewState = createLoginTypeContentViewState(), - ), + expectedState, awaitItem(), ) } @@ -262,7 +262,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { type = VaultAddEditState.ViewState.Content.ItemType.Login(), ), dialog = null, - supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + supportedItemTypes = VaultAddEditState.ItemTypeOption.entries + .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }, ), viewModel.stateFlow.value, ) @@ -378,56 +379,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { } } - @Test - fun `initial add state should be correct when SSH key feature flag is enabled`() { - mutableSshVaultItemsFeatureFlagFlow.value = true - val vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN) - val initState = createVaultAddItemState(vaultAddEditType = vaultAddEditType) - val viewModel = createAddVaultItemViewModel( - savedStateHandle = createSavedStateHandleWithState( - state = initState, - vaultAddEditType = vaultAddEditType, - ), - ) - assertEquals( - initState, - viewModel.stateFlow.value, - ) - } - - @Test - fun `initial add state should be correct when SSH key feature flag is disabled`() { - mutableSshVaultItemsFeatureFlagFlow.value = false - every { - featureFlagManager.getFeatureFlag(key = FlagKey.SshKeyCipherItems) - } returns false - val vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN) - val expectedState = VaultAddEditState( - vaultAddEditType = vaultAddEditType, - viewState = VaultAddEditState.ViewState.Content( - common = VaultAddEditState.ViewState.Content.Common(), - isIndividualVaultDisabled = false, - type = VaultAddEditState.ViewState.Content.ItemType.Login(), - ), - dialog = null, - totpData = null, - shouldShowCloseButton = true, - shouldExitOnSave = false, - supportedItemTypes = VaultAddEditState.ItemTypeOption.entries - .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }, - ) - val viewModel = createAddVaultItemViewModel( - savedStateHandle = createSavedStateHandleWithState( - state = null, - vaultAddEditType = vaultAddEditType, - ), - ) - assertEquals( - expectedState, - viewModel.stateFlow.value, - ) - } - @Test fun `initial edit state should be correct`() = runTest { val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID) @@ -3175,7 +3126,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { resourceManager = resourceManager, clock = fixedClock, organizationEventManager = organizationEventManager, - featureFlagManager = featureFlagManager, ) } @@ -4271,30 +4221,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { ) } } - - @Suppress("MaxLineLength") - @Test - fun `SshKeyCipherItemsFeatureFlagReceive should update supportedItemTypes`() = runTest { - // Verify SSH keys is supported when feature flag is enabled. - viewModel.trySendAction( - VaultAddEditAction.Internal.SshKeyCipherItemsFeatureFlagReceive(enabled = true), - ) - assertEquals( - VaultAddEditState.ItemTypeOption.entries, - viewModel.stateFlow.value.supportedItemTypes, - ) - - // Verify SSH keys is not supported when feature flag is disabled. - viewModel.trySendAction( - VaultAddEditAction.Internal.SshKeyCipherItemsFeatureFlagReceive(enabled = false), - ) - assertEquals( - VaultAddEditState.ItemTypeOption.entries.filterNot { - it == VaultAddEditState.ItemTypeOption.SSH_KEYS - }, - viewModel.stateFlow.value.supportedItemTypes, - ) - } } //region Helper functions @@ -4429,7 +4355,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { resourceManager = bitwardenResourceManager, clock = clock, organizationEventManager = organizationEventManager, - featureFlagManager = featureFlagManager, ) private fun createVaultData(