From 2fd9ce8820714ea9de43c9ced1d2c8abfc28e299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Wed, 19 Aug 2026 14:37:11 +0000 Subject: [PATCH] [PM-40533] feat: Validate recipient email domains against SendControls allowed domains (#7269) --- .../send/addedit/AddEditSendViewModel.kt | 58 ++++- .../send/addedit/AddEditSendViewModelTest.kt | 199 ++++++++++++++++++ ui/src/main/res/values/strings.xml | 1 + 3 files changed, 256 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt index f713ebea01..2dd333d91d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt @@ -51,6 +51,7 @@ import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.toViewState import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType import com.x8bit.bitwarden.ui.tools.feature.send.util.toSendUrl import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.flow.combine @@ -79,6 +80,28 @@ private const val KEY_STATE = "state" */ private const val MAX_FILE_SIZE_BYTES: Long = 100 * 1024 * 1024 +/** + * Whether this email address belongs to one of [allowedDomains], compared case-insensitively. + */ +private fun String.hasAllowedDomain(allowedDomains: List): Boolean { + val domain = substringAfterLast(delimiter = '@', missingDelimiterValue = "") + return allowedDomains.any { it.equals(other = domain, ignoreCase = true) } +} + +/** + * Splits this comma-separated policy value into trimmed, non-blank domains. + * + * Returns an empty list when no domain is configured, which includes a value that holds nothing + * usable such as `","`. An empty result is treated as no restriction rather than as a restriction + * nothing can satisfy, since the latter would block saving with no way for the user to recover. + */ +private fun String?.splitToDomains(): ImmutableList = this + ?.split(",") + ?.map { it.trim() } + ?.filter { it.isNotBlank() } + .orEmpty() + .toImmutableList() + /** * Returns the [SendAuth] this access type restricts a Send to, preserving [current] when it already * matches the restricted type so that any emails already entered are kept. @@ -494,6 +517,24 @@ class AddEditSendViewModel @Inject constructor( return clock.instant().plus(hours, ChronoUnit.HOURS) } + /** + * Returns the error to show when any of [emails] falls outside the recipient domains the + * SendControls policy allows, or `null` when every recipient is acceptable. + */ + private fun AddEditSendState.disallowedDomainErrorOrNull( + emails: List, + ): AddEditSendState.DialogState.Error? { + val enforced = enforcedAllowedDomains + if (enforced.isEmpty()) return null + if (emails.all { it.value.hasAllowedDomain(enforced) }) return null + return AddEditSendState.DialogState.Error( + title = BitwardenString.invalid_email_addresses.asText(), + message = BitwardenString + .only_include_the_following_domains_x_please_review_and_try_again + .asText(enforced.joinToString(separator = ", ")), + ) + } + @Suppress("LongMethod") private fun handleSendDataReceive(action: AddEditSendAction.Internal.SendDataReceive) { when (val sendDataState = action.sendDataState) { @@ -800,6 +841,11 @@ class AddEditSendViewModel @Inject constructor( } return@onContent } + + state.disallowedDomainErrorOrNull(emails = nonBlankEmails)?.let { error -> + mutableStateFlow.update { it.copy(dialogState = error) } + return@onContent + } } (content.selectedType as? AddEditSendState.ViewState.Content.SendType.File) @@ -996,8 +1042,8 @@ class AddEditSendViewModel @Inject constructor( /** * Models state for the add/edit send screen. * - * @property allowedDomains The allowed recipient email domains, sourced from - * [EffectiveSendPolicy.allowedDomains]. Currently unused by the UI. + * @property allowedDomains The allowed recipient email domains as a comma-separated list, sourced + * from [EffectiveSendPolicy.allowedDomains]. * @property allowedSendTypes The types of Sends that are allowed to be created, sourced from * [EffectiveSendPolicy.allowedSendTypes]. Currently unused by the UI. * @property deletionHours The enforced Send deletion window in hours, sourced from @@ -1023,6 +1069,14 @@ data class AddEditSendState( val isPremium: Boolean, ) : Parcelable { + /** + * Helper to determine the recipient email domains the SendControls policy restricts a Send to, + * or an empty list when recipients may use any domain. The legacy send options policy has no + * equivalent enforcement, so this is only in effect alongside the SendControls feature flag. + */ + val enforcedAllowedDomains: ImmutableList + get() = allowedDomains.takeIf { isSendControlsEnabled }.splitToDomains() + /** * Helper to determine the Send deletion window enforced by the SendControls policy, or `null` * when the deletion date is left to the user. The legacy send options policy has no equivalent diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt index 7c72933939..4f4c97379f 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt @@ -55,6 +55,7 @@ import io.mockk.runs import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach @@ -928,6 +929,178 @@ class AddEditSendViewModelTest : BaseViewModelTest() { ) } + @Test + fun `SaveClick with a recipient outside the allowed domains should show error dialog`() { + val viewState = emailViewState(emails = listOf("recipient@example.com")) + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = "bitwarden.com", + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + assertEquals( + domainPolicyState(viewState = viewState, allowedDomains = "bitwarden.com").copy( + dialogState = AddEditSendState.DialogState.Error( + title = BitwardenString.invalid_email_addresses.asText(), + message = BitwardenString + .only_include_the_following_domains_x_please_review_and_try_again + .asText("bitwarden.com"), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `SaveClick outside the allowed domains should list every allowed domain`() { + val viewState = emailViewState(emails = listOf("recipient@example.com")) + val domains = "bitwarden.com,bitwarden.eu" + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = domains, + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + assertEquals( + domainPolicyState(viewState = viewState, allowedDomains = domains).copy( + dialogState = AddEditSendState.DialogState.Error( + title = BitwardenString.invalid_email_addresses.asText(), + message = BitwardenString + .only_include_the_following_domains_x_please_review_and_try_again + .asText("bitwarden.com, bitwarden.eu"), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `SaveClick outside the allowed domains should normalize the policy value`() { + val viewState = emailViewState(emails = listOf("recipient@example.com")) + val domains = " bitwarden.com , , bitwarden.eu " + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = domains, + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + // Padding and the empty entry are dropped for both matching and display. + assertEquals( + domainPolicyState(viewState = viewState, allowedDomains = domains).copy( + dialogState = AddEditSendState.DialogState.Error( + title = BitwardenString.invalid_email_addresses.asText(), + message = BitwardenString + .only_include_the_following_domains_x_please_review_and_try_again + .asText("bitwarden.com, bitwarden.eu"), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `SaveClick with every recipient inside the allowed domains should save`() { + val viewState = emailViewState( + emails = listOf("one@bitwarden.com", "two@BITWARDEN.EU"), + ) + val mockSendView = stubCreateSend(viewState = viewState) + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = "bitwarden.com, bitwarden.eu", + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + // Matching is case-insensitive, so the save proceeds to the repository. + coVerify(exactly = 1) { + vaultRepository.createSend(sendView = mockSendView, fileUri = null) + } + } + + @Test + fun `SaveClick should skip domain validation when send controls is disabled`() { + val viewState = emailViewState(emails = listOf("recipient@example.com")) + val mockSendView = stubCreateSend(viewState = viewState) + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = "bitwarden.com", + isSendControlsEnabled = false, + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + coVerify(exactly = 1) { + vaultRepository.createSend(sendView = mockSendView, fileUri = null) + } + } + + @Test + fun `SaveClick should skip domain validation when the policy sets no domains`() { + val viewState = emailViewState(emails = listOf("recipient@example.com")) + val mockSendView = stubCreateSend(viewState = viewState) + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = null, + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + coVerify(exactly = 1) { + vaultRepository.createSend(sendView = mockSendView, fileUri = null) + } + } + + @Test + fun `SaveClick should skip domain validation for other access types`() { + val viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy(name = "test", sendAuth = SendAuth.Password), + ) + val mockSendView = stubCreateSend(viewState = viewState) + val viewModel = createViewModelWithDomainPolicy( + viewState = viewState, + allowedDomains = "bitwarden.com", + ) + + viewModel.trySendAction(AddEditSendAction.SaveClick) + + coVerify(exactly = 1) { + vaultRepository.createSend(sendView = mockSendView, fileUri = null) + } + } + + /** + * Stubs out a successful conversion and creation of the Send backing [viewState], returning the + * [SendView] the repository is expected to be called with. + */ + private fun stubCreateSend(viewState: AddEditSendState.ViewState.Content): SendView { + val mockSendView = mockk() + every { viewState.toSendView(clock) } returns mockSendView + coEvery { + vaultRepository.createSend(sendView = mockSendView, fileUri = null) + } returns CreateSendResult.Error(message = null, error = null) + return mockSendView + } + + /** + * Creates a view model whose SendControls policy restricts recipients to [allowedDomains]. + * + * The policy is applied through the flows rather than the initial state, since the view model + * overwrites its own policy fields as soon as it subscribes to them. + */ + private fun createViewModelWithDomainPolicy( + viewState: AddEditSendState.ViewState.Content, + allowedDomains: String?, + isSendControlsEnabled: Boolean = true, + ): AddEditSendViewModel { + mutableSendControlsFlagFlow.value = isSendControlsEnabled + mutableEffectiveSendPolicyFlow.value = + DEFAULT_EFFECTIVE_SEND_POLICY.copy(allowedDomains = allowedDomains) + return createViewModel(DEFAULT_STATE.copy(viewState = viewState)) + } + @Test fun `SaveClick for file without Premium show error dialog`() { mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( @@ -1990,6 +2163,32 @@ private val DEFAULT_STATE = AddEditSendState( isPremium = true, ) +/** + * Builds the state expected once the SendControls policy restricting recipients to + * [allowedDomains] has been applied on top of [viewState]. + */ +private fun domainPolicyState( + viewState: AddEditSendState.ViewState.Content, + allowedDomains: String?, +): AddEditSendState = DEFAULT_STATE.copy( + viewState = viewState, + isSendControlsEnabled = true, + allowedDomains = allowedDomains, +) + +/** + * Builds a named content view state whose recipients are [emails]. + */ +private fun emailViewState(emails: List): AddEditSendState.ViewState.Content = + DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy( + name = "test", + sendAuth = SendAuth.Email( + emails = emails.map { AuthEmail(value = it) }.toImmutableList(), + ), + ), + ) + /** * Builds the state expected when [whoCanAccess] is enforced and has forced [sendAuth]. */ diff --git a/ui/src/main/res/values/strings.xml b/ui/src/main/res/values/strings.xml index 97386ddf30..f84bee8379 100644 --- a/ui/src/main/res/values/strings.xml +++ b/ui/src/main/res/values/strings.xml @@ -1250,6 +1250,7 @@ Do you want to switch to this account? Invalid email addresses One or more email addresses are incorrect. Please review and try again. No email addresses entered + Only include the following domains: %1$s. Please review and try again. Enter at least one valid email address to share this Send. Sync with browser To connect to %1$s, sync your default browser with the Bitwarden mobile app. When your browser opens, complete sign-in if prompted.\nOnce syncing is complete, you’ll be returned to this app.