PM-19404: Improve email validation and validation error parsing (#4945)

app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/SendVerificationEmailResponseJson.kt

@@ -1,7 +1,9 @@
 package com.x8bit.bitwarden.data.auth.datasource.network.model
 
+import kotlinx.serialization.ExperimentalSerializationApi
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.JsonNames
 
 /**
  * The response body for sending a verification email.
@@ -26,20 +28,24 @@ sealed class SendVerificationEmailResponseJson {
      * The values in the array should be used for display to the user, since the keys tend to come
      * back as nonsense. (eg: empty string key)
      */
+    @OptIn(ExperimentalSerializationApi::class)
     @Serializable
     data class Invalid(
-        @SerialName("message")
-        private val invalidMessage: String? = null,
-
+        @JsonNames("message")
         @SerialName("Message")
         private val errorMessage: String? = null,
 
         @SerialName("validationErrors")
-        val validationErrors: Map<String, List<String>>?,
+        private val validationErrors: Map<String, List<String>>?,
     ) : SendVerificationEmailResponseJson() {
         /**
          * A generic error message.
          */
-        val message: String? get() = invalidMessage ?: errorMessage
+        val message: String?
+            get() = validationErrors
+                ?.values
+                ?.firstOrNull()
+                ?.firstOrNull()
+                ?: errorMessage
     }
 }

app/src/main/java/com/x8bit/bitwarden/ui/platform/base/util/StringExtensions.kt

@@ -43,9 +43,16 @@ fun String?.orZeroWidthSpace(): String = this.orNullIfBlank() ?: ZERO_WIDTH_CHAR
 /**
  * Whether or not string is a valid email address.
  *
- * This just checks if the string contains the "@" symbol.
+ * This validates that the email is valid by asserting that:
+ * * The string starts with a string of characters including periods, underscores, percent symbols,
+ * plus's, minus's, and alphanumeric characters.
+ * * Followed by an '@' symbol.
+ * * Followed by a string of characters including periods, minus's, and alphanumeric characters.
+ * * Followed by a period.
+ * * Followed by at least 2 more alphanumeric characters.
  */
-fun String.isValidEmail(): Boolean = contains("@")
+fun String.isValidEmail(): Boolean =
+    this.matches(regex = "^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$".toRegex())
 
 /**
  * Returns `true` if the given [String] is a non-blank, valid URI and `false` otherwise.

app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt

@@ -5474,7 +5474,6 @@ class AuthRepositoryTest {
         assertEquals(PrevalidateSsoResult.Success(token = "token"), result)
     }
 
-    @Suppress("MaxLineLength")
     @Test
     fun `logout for an inactive account should call logout on the UserLogoutManager`() {
         val userId = USER_ID_2
@@ -6229,7 +6228,7 @@ class AuthRepositoryTest {
                 ),
             )
         } returns SendVerificationEmailResponseJson
-            .Invalid(invalidMessage = errorMessage, validationErrors = null)
+            .Invalid(errorMessage = errorMessage, validationErrors = null)
             .asSuccess()
 
         val result = repository.sendVerificationEmail(

app/src/test/java/com/x8bit/bitwarden/ui/platform/base/util/StringExtensionsTest.kt

@@ -9,26 +9,26 @@ import org.junit.jupiter.api.Test
 class StringExtensionsTest {
 
     @Test
-    fun `emails without an @ character should be invalid`() {
+    fun `isValidEmail should return appropriate value for possible email addresses`() {
         val invalidEmails = listOf(
-            "",
-            " ",
-            "test.com",
+            "" to false,
+            " " to false,
+            "test.com" to false,
+            "@" to false,
+            "@." to false,
+            "@.aa" to false,
+            "a@.aa" to false,
+            "test@test.com" to true,
+            " test@test " to false,
+            "test@test.c" to false,
+            "a@a.aa" to true,
+            "test@test.com" to true,
+            "test@test.test.com" to true,
+            "test.test@test.com" to true,
+            "test.test@test.test.com" to true,
         )
         invalidEmails.forEach {
-            assertFalse(it.isValidEmail())
-        }
-    }
-
-    @Test
-    fun `emails with an @ character should be valid`() {
-        val validEmails = listOf(
-            "@",
-            "test@test.com",
-            " test@test ",
-        )
-        validEmails.forEach {
-            assertTrue(it.isValidEmail())
+            assertEquals(it.first.isValidEmail(), it.second)
         }
     }