[PM-12739] adjusted generator length to not be lower than minimum len…

…gth (#4016)
bitwarden · Oct 3, 2024 · e2e5042 · e2e5042
1 parent 0c83a10
commit e2e5042
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 16 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt
@@ -507,7 +507,7 @@ private fun ColumnScope.PasswordTypeContent(
     BitwardenSlider(
         value = passwordTypeState.length,
         onValueChange = passwordHandlers.onPasswordSliderLengthChange,
-        range = passwordTypeState.minLength..passwordTypeState.maxLength,
+        range = passwordTypeState.computedMinimumLength..passwordTypeState.maxLength,
         sliderTag = "PasswordLengthSlider",
         valueTag = "PasswordLengthLabel",
     )

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt
@@ -578,7 +578,7 @@ class GeneratorViewModel @Inject constructor(
             uppercase = password.useCapitals,
             numbers = password.useNumbers,
             special = password.useSpecialChars,
-            length = password.length.toUByte(),
+            length = max(password.computedMinimumLength, password.length).toUByte(),
             avoidAmbiguous = password.avoidAmbiguousChars,
             minLowercase = null,
             minUppercase = null,
@@ -829,7 +829,7 @@ class GeneratorViewModel @Inject constructor(
 
         updatePasswordType { currentPasswordType ->
             currentPasswordType.copy(
-                length = max(adjustedLength, currentPasswordType.minimumLength),
+                length = max(adjustedLength, currentPasswordType.computedMinimumLength),
                 isUserInteracting = action.isUserInteracting,
             )
         }
@@ -1477,7 +1477,10 @@ class GeneratorViewModel @Inject constructor(
     private fun updatePasswordLength() {
         updatePasswordType { currentPasswordType ->
             currentPasswordType.copy(
-                length = max(currentPasswordType.length, currentPasswordType.minimumLength),
+                length = max(
+                    currentPasswordType.length,
+                    currentPasswordType.computedMinimumLength,
+                ),
             )
         }
     }
@@ -1853,6 +1856,22 @@ data class GeneratorState(
                     override val displayStringResId: Int
                         get() = PasscodeTypeOption.PASSWORD.labelRes
 
+                    /**
+                     * The computed minimum length for the generated Password
+                     * based on what characters must be included.
+                     */
+                    val computedMinimumLength: Int
+                        get() {
+                            val minLowercase = if (useLowercase) 1 else 0
+                            val minUppercase = if (useCapitals) 1 else 0
+                            val minimumNumbers = if (useNumbers) max(1, minNumbers) else 0
+                            val minimumSpecial = if (useSpecialChars) max(1, minSpecial) else 0
+                            return max(
+                                minLength,
+                                minLowercase + minUppercase + minimumNumbers + minimumSpecial,
+                            )
+                        }
+
                     @Suppress("UndocumentedPublicClass")
                     companion object {
                         private const val DEFAULT_PASSWORD_LENGTH: Int = 14
@@ -2625,18 +2644,6 @@ private fun Password.enforceAtLeastOneToggleOn(): Password =
         this
     }
 
-/**
- * The computed minimum length for the generated Password based on what characters must be included.
- */
-private val Password.minimumLength: Int
-    get() {
-        val minLowercase = if (useLowercase) 1 else 0
-        val minUppercase = if (useCapitals) 1 else 0
-        val minimumNumbers = if (useNumbers) max(1, minNumbers) else 0
-        val minimumSpecial = if (useSpecialChars) max(1, minSpecial) else 0
-        return minLowercase + minUppercase + minimumNumbers + minimumSpecial
-    }
-
 private val PasscodeGenerationOptions?.passcodeType: Passcode.PasscodeType
     get() = when (this?.type) {
         PasscodeGenerationOptions.PasscodeType.PASSWORD -> Password()

diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt
@@ -2304,6 +2304,53 @@ class GeneratorViewModelTest : BaseViewModelTest() {
                 assertEquals(expectedState, viewModel.stateFlow.value)
             }
     }
+
+    @Test
+    fun `Password minimumLength should be at least as long as the sum of the minimums`() {
+        val password =
+            GeneratorState.MainType.Passcode.PasscodeType.Password(
+                length = 14,
+                minLength = 10,
+                useCapitals = true,
+                capitalsEnabled = false,
+                useLowercase = true,
+                lowercaseEnabled = false,
+                useNumbers = true,
+                numbersEnabled = false,
+                useSpecialChars = true,
+                specialCharsEnabled = false,
+                minNumbers = 9,
+                minNumbersAllowed = 3,
+                minSpecial = 9,
+                minSpecialAllowed = 3,
+                avoidAmbiguousChars = false,
+            )
+        // 9 numbers + 9 special + 1 lowercase + 1 uppercase
+        assertEquals(20, password.computedMinimumLength)
+    }
+
+    @Test
+    fun `Password minimumLength should use minLength if higher than sum of the minimums`() {
+        val password =
+            GeneratorState.MainType.Passcode.PasscodeType.Password(
+                length = 14,
+                minLength = 10,
+                useCapitals = true,
+                capitalsEnabled = false,
+                useLowercase = true,
+                lowercaseEnabled = false,
+                useNumbers = true,
+                numbersEnabled = false,
+                useSpecialChars = true,
+                specialCharsEnabled = false,
+                minNumbers = 1,
+                minNumbersAllowed = 3,
+                minSpecial = 1,
+                minSpecialAllowed = 3,
+                avoidAmbiguousChars = false,
+            )
+        assertEquals(10, password.computedMinimumLength)
+    }
     //region Helper Functions
 
     @Suppress("LongParameterList")