[PM-27119] Prevent import card data when ITEM_RESTRICT_TYPES policy is active (#6123)

This commit is contained in:
aj-rosado
2025-11-11 19:24:38 +00:00
committed by GitHub
parent b2d94fae40
commit 5eb56cafaa
8 changed files with 299 additions and 25 deletions
@@ -43,3 +43,9 @@ inline fun <reified T : PolicyInformation> getPolicyTypeJson(): PolicyTypeJson =
)
}
}
/**
* Helper method for verifying if user has enabled the restrict item policy.
*/
fun PolicyManager.hasRestrictItemTypes(): Boolean =
getActivePolicies(type = PolicyTypeJson.RESTRICT_ITEM_TYPES)
.any { it.isEnabled }
@@ -12,6 +12,9 @@ import com.bitwarden.network.model.ImportCiphersJsonRequest
import com.bitwarden.network.model.ImportCiphersResponseJson
import com.bitwarden.network.service.CiphersService
import com.bitwarden.network.util.base64UrlDecodeOrNull
import com.bitwarden.vault.CipherType
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.util.hasRestrictItemTypes
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
import com.x8bit.bitwarden.data.vault.manager.model.ImportCxfPayloadResult
import com.x8bit.bitwarden.data.vault.manager.model.SyncVaultDataResult
@@ -34,6 +37,7 @@ class CredentialExchangeImportManagerImpl(
private val vaultSdkSource: VaultSdkSource,
private val ciphersService: CiphersService,
private val vaultSyncManager: VaultSyncManager,
private val policyManager: PolicyManager,
private val json: Json,
) : CredentialExchangeImportManager {
@@ -83,7 +87,9 @@ class CredentialExchangeImportManagerImpl(
}
val accountsJson = try {
json.encodeToString(exportResponse.accounts.firstOrNull())
json.encodeToString(
value = exportResponse.accounts.firstOrNull(),
)
} catch (_: SerializationException) {
return ImportCxfPayloadResult.Error(
ImportCredentialsInvalidJsonException("Unable to re-encode accounts."),
@@ -95,7 +101,14 @@ class CredentialExchangeImportManagerImpl(
payload = accountsJson,
)
.flatMap { cipherList ->
if (cipherList.isEmpty()) {
// Filter out card ciphers if RESTRICT_ITEM_TYPES policy is active
val filteredCipherList = if (policyManager.hasRestrictItemTypes()) {
cipherList.filter { cipher -> cipher.type != CipherType.CARD }
} else {
cipherList
}
if (filteredCipherList.isEmpty()) {
// If no ciphers were returned, we can skip the remaining steps and return the
// appropriate result.
return ImportCxfPayloadResult.NoItems
@@ -103,7 +116,7 @@ class CredentialExchangeImportManagerImpl(
ciphersService
.importCiphers(
request = ImportCiphersJsonRequest(
ciphers = cipherList.map {
ciphers = filteredCipherList.map {
it.toEncryptedNetworkCipher(
encryptedFor = userId,
)
@@ -120,7 +133,7 @@ class CredentialExchangeImportManagerImpl(
ImportCiphersResponseJson.Success -> {
ImportCxfPayloadResult
.Success(itemCount = cipherList.size)
.Success(itemCount = filteredCipherList.size)
.asSuccess()
}
}
@@ -17,6 +17,7 @@ import com.x8bit.bitwarden.data.auth.manager.UserStateManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.AppStateManager
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
@@ -210,11 +211,13 @@ object VaultManagerModule {
vaultSdkSource: VaultSdkSource,
ciphersService: CiphersService,
vaultSyncManager: VaultSyncManager,
policyManager: PolicyManager,
json: Json,
): CredentialExchangeImportManager = CredentialExchangeImportManagerImpl(
vaultSdkSource = vaultSdkSource,
ciphersService = ciphersService,
vaultSyncManager = vaultSyncManager,
policyManager = policyManager,
json = json,
)
}
@@ -21,6 +21,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.VerifyOtpResult
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
import com.x8bit.bitwarden.data.platform.manager.util.hasRestrictItemTypes
import com.x8bit.bitwarden.data.vault.manager.FileManager
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.ExportVaultDataResult
@@ -464,10 +465,7 @@ class ExportVaultViewModel @Inject constructor(
}
private fun getRestrictedItemTypes(): List<CipherType> {
val hasActiveRestrictItemTypesPolicy = policyManager
.getActivePolicies(type = PolicyTypeJson.RESTRICT_ITEM_TYPES)
.isNotEmpty()
return if (!hasActiveRestrictItemTypesPolicy) {
return if (!policyManager.hasRestrictItemTypes()) {
emptyList()
} else {
listOf(CipherType.CARD)
@@ -13,6 +13,8 @@ import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.Text
import com.bitwarden.ui.util.asPluralsText
import com.bitwarden.ui.util.asText
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.util.hasRestrictItemTypes
import com.x8bit.bitwarden.data.vault.manager.model.SyncVaultDataResult
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.ImportCredentialsResult
@@ -32,6 +34,7 @@ private const val KEY_STATE = "state"
class ImportItemsViewModel @Inject constructor(
savedStateHandle: SavedStateHandle,
private val vaultRepository: VaultRepository,
private val policyManager: PolicyManager,
) : BaseViewModel<ImportItemsState, ImportItemsEvent, ImportItemsAction>(
initialState = savedStateHandle[KEY_STATE] ?: ImportItemsState(),
) {
@@ -117,24 +120,29 @@ class ImportItemsViewModel @Inject constructor(
}
private fun handleImportFromAnotherAppClick() {
val credentialTypes = buildList {
add(CredentialTypes.CREDENTIAL_TYPE_BASIC_AUTH)
add(CredentialTypes.CREDENTIAL_TYPE_PUBLIC_KEY)
add(CredentialTypes.CREDENTIAL_TYPE_ADDRESS)
add(CredentialTypes.CREDENTIAL_TYPE_API_KEY)
// Only include credit card type if policy doesn't restrict it
if (!policyManager.hasRestrictItemTypes()) {
add(CredentialTypes.CREDENTIAL_TYPE_CREDIT_CARD)
}
add(CredentialTypes.CREDENTIAL_TYPE_CUSTOM_FIELDS)
add(CredentialTypes.CREDENTIAL_TYPE_DRIVERS_LICENSE)
add(CredentialTypes.CREDENTIAL_TYPE_IDENTITY_DOCUMENT)
add(CredentialTypes.CREDENTIAL_TYPE_NOTE)
add(CredentialTypes.CREDENTIAL_TYPE_PASSPORT)
add(CredentialTypes.CREDENTIAL_TYPE_PERSON_NAME)
add(CredentialTypes.CREDENTIAL_TYPE_SSH_KEY)
add(CredentialTypes.CREDENTIAL_TYPE_TOTP)
add(CredentialTypes.CREDENTIAL_TYPE_WIFI)
}
sendEvent(
ImportItemsEvent.ShowRegisteredImportSources(
credentialTypes = listOf(
CredentialTypes.CREDENTIAL_TYPE_BASIC_AUTH,
CredentialTypes.CREDENTIAL_TYPE_PUBLIC_KEY,
CredentialTypes.CREDENTIAL_TYPE_ADDRESS,
CredentialTypes.CREDENTIAL_TYPE_API_KEY,
CredentialTypes.CREDENTIAL_TYPE_CREDIT_CARD,
CredentialTypes.CREDENTIAL_TYPE_CUSTOM_FIELDS,
CredentialTypes.CREDENTIAL_TYPE_DRIVERS_LICENSE,
CredentialTypes.CREDENTIAL_TYPE_IDENTITY_DOCUMENT,
CredentialTypes.CREDENTIAL_TYPE_NOTE,
CredentialTypes.CREDENTIAL_TYPE_PASSPORT,
CredentialTypes.CREDENTIAL_TYPE_PERSON_NAME,
CredentialTypes.CREDENTIAL_TYPE_SSH_KEY,
CredentialTypes.CREDENTIAL_TYPE_TOTP,
CredentialTypes.CREDENTIAL_TYPE_WIFI,
),
credentialTypes = credentialTypes,
),
)
}
@@ -9,9 +9,12 @@ import com.bitwarden.cxf.model.CredentialExchangeProtocolMessage
import com.bitwarden.cxf.model.CredentialExchangeVersion
import com.bitwarden.network.model.ImportCiphersJsonRequest
import com.bitwarden.network.model.ImportCiphersResponseJson
import com.bitwarden.network.model.PolicyTypeJson
import com.bitwarden.network.model.SyncResponseJson
import com.bitwarden.network.service.CiphersService
import com.bitwarden.network.util.base64UrlDecodeOrNull
import com.bitwarden.vault.Cipher
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSdkCipher
import com.x8bit.bitwarden.data.vault.manager.model.ImportCxfPayloadResult
@@ -41,6 +44,11 @@ class CredentialExchangeImportManagerTest {
private val vaultSdkSource: VaultSdkSource = mockk()
private val ciphersService: CiphersService = mockk(relaxed = true)
private val vaultSyncManager: VaultSyncManager = mockk()
private val policyManager: PolicyManager = mockk {
every {
getActivePolicies(any())
} returns emptyList()
}
private val json = mockk<Json> {
every {
decodeFromStringOrNull<CredentialExchangeProtocolMessage>(any())
@@ -57,6 +65,7 @@ class CredentialExchangeImportManagerTest {
vaultSdkSource = vaultSdkSource,
ciphersService = ciphersService,
vaultSyncManager = vaultSyncManager,
policyManager = policyManager,
json = json,
)
@@ -342,6 +351,187 @@ class CredentialExchangeImportManagerTest {
assertTrue(result is ImportCxfPayloadResult.Error)
}
@Test
fun `when user has restrict item types policy, card ciphers should be filtered out`() =
runTest {
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns listOf(
SyncResponseJson.Policy(
id = "mockId-1",
organizationId = "mockId-1",
type = PolicyTypeJson.RESTRICT_ITEM_TYPES,
isEnabled = true,
data = null,
),
)
val loginCipher = createMockSdkCipher(number = 1)
val cardCipher = createMockSdkCipher(number = 2).copy(
type = com.bitwarden.vault.CipherType.CARD,
)
val mixedCipherList = listOf(loginCipher, cardCipher)
coEvery {
vaultSdkSource.importCxf(
userId = DEFAULT_USER_ID,
payload = DEFAULT_ACCOUNT_JSON,
)
} returns mixedCipherList.asSuccess()
val capturedRequest = slot<ImportCiphersJsonRequest>()
coEvery {
ciphersService.importCiphers(capture(capturedRequest))
} returns ImportCiphersResponseJson.Success.asSuccess()
coEvery {
vaultSyncManager.syncForResult(forced = true)
} returns SyncVaultDataResult.Success(itemsAvailable = true)
val result = importManager.importCxfPayload(DEFAULT_USER_ID, DEFAULT_PAYLOAD)
assertEquals(ImportCxfPayloadResult.Success(itemCount = 1), result)
// Verify only the login cipher was imported, card was filtered out
assertEquals(1, capturedRequest.captured.ciphers.size)
coVerify(exactly = 1) {
vaultSdkSource.importCxf(DEFAULT_USER_ID, DEFAULT_ACCOUNT_JSON)
ciphersService.importCiphers(any())
vaultSyncManager.syncForResult(forced = true)
}
}
@Test
fun `when user has no restrict item types policy, card ciphers should not be filtered`() =
runTest {
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns emptyList()
val loginCipher = createMockSdkCipher(number = 1)
val cardCipher = createMockSdkCipher(number = 2).copy(
type = com.bitwarden.vault.CipherType.CARD,
)
val mixedCipherList = listOf(loginCipher, cardCipher)
coEvery {
vaultSdkSource.importCxf(
userId = DEFAULT_USER_ID,
payload = DEFAULT_ACCOUNT_JSON,
)
} returns mixedCipherList.asSuccess()
val capturedRequest = slot<ImportCiphersJsonRequest>()
coEvery {
ciphersService.importCiphers(capture(capturedRequest))
} returns ImportCiphersResponseJson.Success.asSuccess()
coEvery {
vaultSyncManager.syncForResult(forced = true)
} returns SyncVaultDataResult.Success(itemsAvailable = true)
val result = importManager.importCxfPayload(DEFAULT_USER_ID, DEFAULT_PAYLOAD)
assertEquals(ImportCxfPayloadResult.Success(itemCount = 2), result)
// Verify both ciphers were imported
assertEquals(2, capturedRequest.captured.ciphers.size)
coVerify(exactly = 1) {
vaultSdkSource.importCxf(DEFAULT_USER_ID, DEFAULT_ACCOUNT_JSON)
ciphersService.importCiphers(any())
vaultSyncManager.syncForResult(forced = true)
}
}
@Test
fun `when user has restrict policy disabled, card ciphers should not be filtered`() =
runTest {
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns listOf(
SyncResponseJson.Policy(
id = "mockId-1",
organizationId = "mockId-1",
type = PolicyTypeJson.RESTRICT_ITEM_TYPES,
isEnabled = false,
data = null,
),
)
val loginCipher = createMockSdkCipher(number = 1)
val cardCipher = createMockSdkCipher(number = 2).copy(
type = com.bitwarden.vault.CipherType.CARD,
)
val mixedCipherList = listOf(loginCipher, cardCipher)
coEvery {
vaultSdkSource.importCxf(
userId = DEFAULT_USER_ID,
payload = DEFAULT_ACCOUNT_JSON,
)
} returns mixedCipherList.asSuccess()
val capturedRequest = slot<ImportCiphersJsonRequest>()
coEvery {
ciphersService.importCiphers(capture(capturedRequest))
} returns ImportCiphersResponseJson.Success.asSuccess()
coEvery {
vaultSyncManager.syncForResult(forced = true)
} returns SyncVaultDataResult.Success(itemsAvailable = true)
val result = importManager.importCxfPayload(DEFAULT_USER_ID, DEFAULT_PAYLOAD)
assertEquals(ImportCxfPayloadResult.Success(itemCount = 2), result)
// Verify both ciphers were imported when policy is disabled
assertEquals(2, capturedRequest.captured.ciphers.size)
coVerify(exactly = 1) {
vaultSdkSource.importCxf(DEFAULT_USER_ID, DEFAULT_ACCOUNT_JSON)
ciphersService.importCiphers(any())
vaultSyncManager.syncForResult(forced = true)
}
}
@Test
fun `when user has restrict policy and all ciphers are cards, should return NoItems`() =
runTest {
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns listOf(
SyncResponseJson.Policy(
id = "mockId-1",
organizationId = "mockId-1",
type = PolicyTypeJson.RESTRICT_ITEM_TYPES,
isEnabled = true,
data = null,
),
)
val cardCipher1 = createMockSdkCipher(number = 1).copy(
type = com.bitwarden.vault.CipherType.CARD,
)
val cardCipher2 = createMockSdkCipher(number = 2).copy(
type = com.bitwarden.vault.CipherType.CARD,
)
val allCardsList = listOf(cardCipher1, cardCipher2)
coEvery {
vaultSdkSource.importCxf(
userId = DEFAULT_USER_ID,
payload = DEFAULT_ACCOUNT_JSON,
)
} returns allCardsList.asSuccess()
val result = importManager.importCxfPayload(DEFAULT_USER_ID, DEFAULT_PAYLOAD)
assertEquals(ImportCxfPayloadResult.NoItems, result)
coVerify(exactly = 1) {
vaultSdkSource.importCxf(DEFAULT_USER_ID, DEFAULT_ACCOUNT_JSON)
}
// Verify importCiphers was never called since all items were filtered
coVerify(exactly = 0) {
ciphersService.importCiphers(any())
}
}
}
private const val DEFAULT_USER_ID = "mockId-1"
@@ -379,6 +569,7 @@ private val DEFAULT_CXF_EXPORT_RESPONSE: CredentialExchangeExportResponse =
timestamp = 0,
accounts = listOf(DEFAULT_ACCOUNT),
)
private val DEFAULT_ACCOUNT_JSON = """
{
"id": "$DEFAULT_USER_ID",
@@ -143,7 +143,7 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
} returns ValidatePasswordResult.Success(isValid = true)
every {
policyManager.getActivePolicies(type = PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns listOf(createMockPolicy())
} returns listOf(createMockPolicy(isEnabled = true))
val viewModel = createViewModel()
viewModel.trySendAction(ExportVaultAction.PasswordInputChanged(password))
@@ -9,12 +9,16 @@ import com.bitwarden.ui.platform.base.BaseViewModelTest
import com.bitwarden.ui.platform.components.snackbar.model.BitwardenSnackbarData
import com.bitwarden.ui.platform.resource.BitwardenPlurals
import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.network.model.PolicyTypeJson
import com.bitwarden.network.model.SyncResponseJson
import com.bitwarden.ui.util.asPluralsText
import com.bitwarden.ui.util.asText
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.vault.manager.model.SyncVaultDataResult
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.ImportCredentialsResult
import io.mockk.awaits
import io.mockk.every
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.just
@@ -26,6 +30,7 @@ import org.junit.jupiter.api.Test
class ImportItemsViewModelTest : BaseViewModelTest() {
private val vaultRepository = mockk<VaultRepository>()
private val policyManager = mockk<PolicyManager>()
@Test
fun `BackClick sends NavigateBack event`() = runTest {
@@ -55,6 +60,10 @@ class ImportItemsViewModelTest : BaseViewModelTest() {
@Test
fun `ImportFromAnotherAppClick sends ShowRegisteredImportSources event`() {
runTest {
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns emptyList()
val viewModel = createViewModel()
viewModel.trySendAction(ImportItemsAction.ImportFromAnotherAppClick)
viewModel.eventFlow.test {
@@ -83,6 +92,51 @@ class ImportItemsViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `ImportFromAnotherAppClick sends ShowRegisteredImportSources event without CREDIT_CARD when policy enabled`() {
runTest {
// Policy is active and enabled
every {
policyManager.getActivePolicies(PolicyTypeJson.RESTRICT_ITEM_TYPES)
} returns listOf(
SyncResponseJson.Policy(
organizationId = "org-id",
id = "policy-id",
type = PolicyTypeJson.RESTRICT_ITEM_TYPES,
isEnabled = true,
data = null,
),
)
val viewModel = createViewModel()
viewModel.trySendAction(ImportItemsAction.ImportFromAnotherAppClick)
viewModel.eventFlow.test {
assertEquals(
ImportItemsEvent.ShowRegisteredImportSources(
listOf(
CredentialTypes.CREDENTIAL_TYPE_BASIC_AUTH,
CredentialTypes.CREDENTIAL_TYPE_PUBLIC_KEY,
CredentialTypes.CREDENTIAL_TYPE_ADDRESS,
CredentialTypes.CREDENTIAL_TYPE_API_KEY,
// CREDENTIAL_TYPE_CREDIT_CARD is excluded
CredentialTypes.CREDENTIAL_TYPE_CUSTOM_FIELDS,
CredentialTypes.CREDENTIAL_TYPE_DRIVERS_LICENSE,
CredentialTypes.CREDENTIAL_TYPE_IDENTITY_DOCUMENT,
CredentialTypes.CREDENTIAL_TYPE_NOTE,
CredentialTypes.CREDENTIAL_TYPE_PASSPORT,
CredentialTypes.CREDENTIAL_TYPE_PERSON_NAME,
CredentialTypes.CREDENTIAL_TYPE_SSH_KEY,
CredentialTypes.CREDENTIAL_TYPE_TOTP,
CredentialTypes.CREDENTIAL_TYPE_WIFI,
),
),
awaitItem(),
)
}
}
}
@Test
fun `ImportCredentialSelectionReceive and Cancelled result updates state`() = runTest {
val viewModel = createViewModel()
@@ -342,5 +396,6 @@ class ImportItemsViewModelTest : BaseViewModelTest() {
private fun createViewModel(): ImportItemsViewModel = ImportItemsViewModel(
vaultRepository = vaultRepository,
savedStateHandle = SavedStateHandle(),
policyManager = policyManager,
)
}