From 3fb37a08737ed94381da2c0001d6122c47e7c2b4 Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 7 Nov 2023 16:54:59 +0100 Subject: [PATCH] Use `EmailAddressParser` for validating email address in account setup --- .../domain/usecase/ValidateEmailAddress.kt | 66 +++++++++++++++---- .../AutoDiscoveryStringMapper.kt | 20 ++++-- .../setup/src/main/res/values/strings.xml | 4 +- .../usecase/ValidateEmailAddressTest.kt | 63 ++++++++++++++++++ 4 files changed, 134 insertions(+), 19 deletions(-) diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddress.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddress.kt index 3370de6bedf..061b91daf6b 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddress.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddress.kt @@ -2,30 +2,72 @@ package app.k9mail.feature.account.setup.domain.usecase import app.k9mail.core.common.domain.usecase.validation.ValidationError import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.core.common.mail.EmailAddressParserError +import app.k9mail.core.common.mail.EmailAddressParserException +import app.k9mail.core.common.mail.toEmailAddressOrNull +import app.k9mail.core.common.mail.toUserEmailAddress import app.k9mail.feature.account.setup.domain.DomainContract.UseCase +import com.fsck.k9.logging.Timber +/** + * Validate an email address that the user wants to add to an account. + * + * This only allows a subset of all valid email addresses. We currently don't support international email addresses + * and don't allow quoted local parts, or email addresses exceeding length restrictions. + * + * Note: Do NOT use this to validate recipients in incoming or outgoing messages. Use [String.toEmailAddressOrNull] + * instead. + */ class ValidateEmailAddress : UseCase.ValidateEmailAddress { - // TODO replace by new email validation override fun execute(emailAddress: String): ValidationResult { - return when { - emailAddress.isBlank() -> ValidationResult.Failure(ValidateEmailAddressError.EmptyEmailAddress) + if (emailAddress.isBlank()) { + return ValidationResult.Failure(ValidateEmailAddressError.EmptyEmailAddress) + } + + return try { + val parsedEmailAddress = emailAddress.toUserEmailAddress() + + if (parsedEmailAddress.warnings.isEmpty()) { + ValidationResult.Success + } else { + ValidationResult.Failure(ValidateEmailAddressError.NotAllowed) + } + } catch (e: EmailAddressParserException) { + Timber.v(e, "Error parsing email address: %s", emailAddress) - !EMAIL_ADDRESS.matches(emailAddress) -> ValidationResult.Failure( - ValidateEmailAddressError.InvalidEmailAddress, - ) + val validationError = when (e.error) { + EmailAddressParserError.AddressLiteralsNotSupported, + EmailAddressParserError.LocalPartLengthExceeded, + EmailAddressParserError.DnsLabelLengthExceeded, + EmailAddressParserError.DomainLengthExceeded, + EmailAddressParserError.TotalLengthExceeded, + EmailAddressParserError.QuotedStringInLocalPart, + EmailAddressParserError.LocalPartRequiresQuotedString, + EmailAddressParserError.EmptyLocalPart, + -> { + ValidateEmailAddressError.NotAllowed + } - else -> ValidationResult.Success + else -> { + if ('@' in emailAddress) { + // We currently don't support or recognize international email addresses. So if the string + // contains an "@" character, we assume it's a valid email address that we don't support. + ValidateEmailAddressError.InvalidOrNotSupported + } else { + ValidateEmailAddressError.InvalidEmailAddress + } + } + } + + ValidationResult.Failure(validationError) } } sealed interface ValidateEmailAddressError : ValidationError { object EmptyEmailAddress : ValidateEmailAddressError + object NotAllowed : ValidateEmailAddressError + object InvalidOrNotSupported : ValidateEmailAddressError object InvalidEmailAddress : ValidateEmailAddressError } - - private companion object { - val EMAIL_ADDRESS = - "[a-zA-Z0-9+._%\\-]{1,256}@[a-zA-Z0-9][a-zA-Z0-9\\-]{0,64}(\\.[a-zA-Z0-9][a-zA-Z0-9\\-]{0,25})+".toRegex() - } } diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/autodiscovery/AutoDiscoveryStringMapper.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/autodiscovery/AutoDiscoveryStringMapper.kt index 9ec6756f5f9..e7117c2bb1b 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/autodiscovery/AutoDiscoveryStringMapper.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/autodiscovery/AutoDiscoveryStringMapper.kt @@ -39,13 +39,21 @@ internal fun ValidationError.toResourceString(resources: Resources): String { private fun ValidateEmailAddress.ValidateEmailAddressError.toEmailAddressErrorString(resources: Resources): String { return when (this) { - is ValidateEmailAddress.ValidateEmailAddressError.EmptyEmailAddress -> resources.getString( - R.string.account_setup_auto_discovery_validation_error_email_address_required, - ) + ValidateEmailAddress.ValidateEmailAddressError.EmptyEmailAddress -> { + resources.getString(R.string.account_setup_auto_discovery_validation_error_email_address_required) + } - is ValidateEmailAddress.ValidateEmailAddressError.InvalidEmailAddress -> resources.getString( - R.string.account_setup_auto_discovery_validation_error_email_address_invalid, - ) + ValidateEmailAddress.ValidateEmailAddressError.NotAllowed -> { + resources.getString(R.string.account_setup_auto_discovery_validation_error_email_address_not_allowed) + } + + ValidateEmailAddress.ValidateEmailAddressError.InvalidOrNotSupported -> { + resources.getString(R.string.account_setup_auto_discovery_validation_error_email_address_not_supported) + } + + ValidateEmailAddress.ValidateEmailAddressError.InvalidEmailAddress -> { + resources.getString(R.string.account_setup_auto_discovery_validation_error_email_address_invalid) + } } } diff --git a/feature/account/setup/src/main/res/values/strings.xml b/feature/account/setup/src/main/res/values/strings.xml index 1291caca638..fa429793dbe 100644 --- a/feature/account/setup/src/main/res/values/strings.xml +++ b/feature/account/setup/src/main/res/values/strings.xml @@ -7,7 +7,9 @@ Unknown error Email address is required. - Email address is invalid. + This email address is not allowed. + This email address is not supported. + This is not recognized as a valid email address. StartTLS SSL/TLS diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddressTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddressTest.kt index c00d35ed701..431cfffdccc 100644 --- a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddressTest.kt +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateEmailAddressTest.kt @@ -27,6 +27,69 @@ class ValidateEmailAddressTest { .isInstanceOf() } + @Test + fun `should fail when email address is using unnecessary quoting in local part`() { + val result = testSubject.execute("\"local-part\"@domain.example") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when email address requires quoted local part`() { + val result = testSubject.execute("\"local part\"@domain.example") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when local part is empty`() { + val result = testSubject.execute("\"\"@domain.example") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when domain part contains IPv4 literal`() { + val result = testSubject.execute("user@[255.0.100.23]") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when domain part contains IPv6 literal`() { + val result = testSubject.execute("user@[IPv6:2001:0db8:0000:0000:0000:ff00:0042:8329]") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when local part contains non-ASCII character`() { + val result = testSubject.execute("töst@domain.example") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + + @Test + fun `should fail when domain contains non-ASCII character`() { + val result = testSubject.execute("test@dömain.example") + + assertThat(result).isInstanceOf() + .prop(ValidationResult.Failure::error) + .isInstanceOf() + } + @Test fun `should fail when email address is invalid`() { val result = testSubject.execute("test")