mirror of
https://github.com/bitwarden/android.git
synced 2026-10-21 10:40:49 -05:00
[PM-40533] feat: Validate recipient email domains against SendControls allowed domains (#7269)
This commit is contained in:
1 parent
7affa3d8a7
commit
2fd9ce8820
3 files changed
+256
-2
No files matched your search
+56
-2
@@ -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<String>): 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<String> = 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<AuthEmail>,
|
||||
): 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<String>
|
||||
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
|
||||
|
||||
+199
@@ -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<SendView>()
|
||||
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<String>): 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].
|
||||
*/
|
||||
|
||||
@@ -1250,6 +1250,7 @@ Do you want to switch to this account?</string>
|
||||
<string name="invalid_email_addresses">Invalid email addresses</string>
|
||||
<string name="one_or_more_email_addresses_are_incorrect">One or more email addresses are incorrect. Please review and try again.</string>
|
||||
<string name="no_email_addresses_entered">No email addresses entered</string>
|
||||
<string name="only_include_the_following_domains_x_please_review_and_try_again">Only include the following domains: %1$s. Please review and try again.</string>
|
||||
<string name="enter_at_least_one_valid_email_address_to_share_this_send">Enter at least one valid email address to share this Send.</string>
|
||||
<string name="sync_with_browser">Sync with browser</string>
|
||||
<string name="sync_with_browser_description">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.</string>
|
||||
|
||||
Reference in new issue
Block a user