From e2e5042be57423c087166083e16eb89a039df9ab Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:30:54 +0200 Subject: [PATCH] [PM-12739] adjusted generator length to not be lower than minimum length (#4016) --- .../feature/generator/GeneratorScreen.kt | 2 +- .../feature/generator/GeneratorViewModel.kt | 37 +++++++++------ .../generator/GeneratorViewModelTest.kt | 47 +++++++++++++++++++ 3 files changed, 70 insertions(+), 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 index 3a5fdc44b..eee922d3d 100644 --- 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 index 0d5573bbe..39b0769dd 100644 --- 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 index 44376d443..5115dff37 100644 --- 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")