fix: allowed username characters [WPB-4365] (#2305)

* fix: allowed username characters [WPB-4365] * fix tests
wireapp · Dec 13, 2023 · 568fde8 · 568fde8
1 parent b131620
commit 568fde8
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 14 deletions.
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LoginUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LoginUseCase.kt
@@ -122,7 +122,7 @@ internal class LoginUseCaseImpl internal constructor(
                 )
             }
 
-            validateUserHandleUseCase(cleanUserIdentifier).isValidAllowingDots -> {
+            validateUserHandleUseCase(cleanUserIdentifier).isValid -> {
                 loginRepository.loginWithHandle(
                     handle = cleanUserIdentifier,
                     password = password,

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/ValidateUserHandleUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/ValidateUserHandleUseCase.kt
@@ -52,7 +52,7 @@ internal class ValidateUserHandleUseCaseImpl : ValidateUserHandleUseCase {
     }
 
     private companion object {
-        private const val HANDLE_FORBIDDEN_CHARACTERS_REGEX = "[^a-z0-9_]"
+        private const val HANDLE_FORBIDDEN_CHARACTERS_REGEX = "[^a-z0-9._-]"
         private const val HANDLE_MIN_LENGTH = 2
         private const val HANDLE_MAX_LENGTH = 255
     }
@@ -71,8 +71,4 @@ sealed class ValidateUserHandleResult(val handle: String) {
     }
 
     val isValid: Boolean get() = this is Valid
-
-    // in some cases there is still possible to create a handle with dots so we have to allow it in some cases, e.g. login
-    val isValidAllowingDots: Boolean
-        get() = this is Valid || (this is Invalid.InvalidCharacters && this.invalidCharactersUsed == listOf('.'))
 }
diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/LoginUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/LoginUseCaseTest.kt
@@ -416,16 +416,16 @@ class LoginUseCaseTest {
     }
 
     @Test
-    fun givenUserHandleWithDots_whenLoggingInUsingUserHandle_thenReturnSuccess() = runTest {
-        val handle = "cool.user"
+    fun givenUserHandleWithValidCharacters_whenLoggingInUsingUserHandle_thenReturnSuccess() = runTest {
+        val handle = "-cool.user_"
 
         val (arrangement, loginUseCase) = Arrangement()
             .withEmailValidationSucceeding(
                 isSucceeding = false,
                 email = handle
             )
             .withHandleValidationReturning(
-                handleValidationResult = ValidateUserHandleResult.Invalid.InvalidCharacters("cooluser", listOf('.')),
+                handleValidationResult = ValidateUserHandleResult.Valid(handle),
                 handle = handle
             )
             .withLoginUsingHandleResulting(Either.Right(TEST_AUTH_TOKENS to TEST_SSO_ID))
@@ -454,6 +454,42 @@ class LoginUseCaseTest {
             .wasNotInvoked()
     }
 
+    @Test
+    fun givenUserHandleWithInvalidCharacters_whenLoggingInUsingUserHandle_thenReturnInvalidUserIdentifier() = runTest {
+        val handle = "!cool:user?"
+
+        val (arrangement, loginUseCase) = Arrangement()
+            .withEmailValidationSucceeding(
+                isSucceeding = false,
+                email = handle
+            )
+            .withHandleValidationReturning(
+                handleValidationResult = ValidateUserHandleResult.Invalid.InvalidCharacters("cooluser", listOf('!', ':', '?')),
+                handle = handle
+            )
+            .withLoginUsingHandleResulting(Either.Right(TEST_AUTH_TOKENS to TEST_SSO_ID))
+            .arrange()
+
+        val loginUserCaseResult = loginUseCase(handle, TEST_PASSWORD, TEST_PERSIST_CLIENT, TEST_LABEL)
+
+        assertEquals(AuthenticationResult.Failure.InvalidUserIdentifier, loginUserCaseResult)
+
+        verify(arrangement.validateEmailUseCase)
+            .invocation { invoke(handle) }
+            .wasInvoked(exactly = once)
+        verify(arrangement.validateUserHandleUseCase)
+            .invocation { invoke(handle) }
+            .wasInvoked(exactly = once)
+        verify(arrangement.loginRepository)
+            .suspendFunction(arrangement.loginRepository::loginWithHandle)
+            .with(any(), any(), any(), any())
+            .wasNotInvoked()
+        verify(arrangement.loginRepository)
+            .suspendFunction(arrangement.loginRepository::loginWithEmail)
+            .with(any(), any(), any(), any(), any())
+            .wasNotInvoked()
+    }
+
     private class Arrangement {
 
         @Mock

diff --git a/...src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/ValidateUserHandleUseCaseTest.kt b/...src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/ValidateUserHandleUseCaseTest.kt
@@ -72,31 +72,46 @@ class ValidateUserHandleUseCaseTest {
     fun givenUserHandleContainsDots_whenValidating_thenReturnProperValues() {
         val handleWithDot = "user.name"
         val result = validateUserHandleUseCase(handleWithDot)
-        assertFalse { result.isValid }
-        assertTrue { result.isValidAllowingDots }
+        assertTrue { result.isValid }
+    }
+
+    @Test
+    fun givenUserHandleContainsUnderline_whenValidating_thenReturnProperValues() {
+        val handleWithDot = "user_name"
+        val result = validateUserHandleUseCase(handleWithDot)
+        assertTrue { result.isValid }
+    }
+
+    @Test
+    fun givenUserHandleContainsDash_whenValidating_thenReturnProperValues() {
+        val handleWithDot = "user-name"
+        val result = validateUserHandleUseCase(handleWithDot)
+        assertTrue { result.isValid }
     }
 
     @Test
     fun givenUserHandleContainsInvalidCharacters_whenValidating_thenReturnListOfInvalidCharacters() {
         val handleWithDot = "user.name!with?invalid,characters"
         val result = validateUserHandleUseCase(handleWithDot)
         assertIs<ValidateUserHandleResult.Invalid.InvalidCharacters>(result)
-        assertTrue { result.invalidCharactersUsed.toSet() == listOf('.', '!', '?', ',').toSet() }
+        assertTrue { result.invalidCharactersUsed.toSet() == listOf('!', '?', ',').toSet() }
     }
 
     private companion object {
         val VALID_HANDLES = listOf(
             "cm",
             "hadle_",
             "user_99",
-            "1_user"
+            "1-user",
+            "user.name",
         )
 
         val INVALID_HANDLES = listOf(
             "c",
             "@hadle",
             "User_99",
-            "1_uSer"
+            "1-uSer",
+            "user,name",
         )
     }