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 94e3d582e..456bbec80 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 @@ -372,7 +372,7 @@ class GeneratorViewModel @Inject constructor( val options = generatorRepository .getPasscodeGenerationOptions() ?: generatePasscodeDefaultOptions() val newOptions = options.copy( - length = password.length, + length = password.minimumLength, allowAmbiguousChar = password.avoidAmbiguousChars, hasNumbers = password.useNumbers, minNumber = password.minNumbers, @@ -477,7 +477,7 @@ class GeneratorViewModel @Inject constructor( val defaultPassphrase = Passphrase() return PasscodeGenerationOptions( - length = defaultPassword.length, + length = defaultPassword.minimumLength, allowAmbiguousChar = defaultPassword.avoidAmbiguousChars, hasNumbers = defaultPassword.useNumbers, minNumber = defaultPassword.minNumbers, @@ -518,7 +518,7 @@ class GeneratorViewModel @Inject constructor( uppercase = password.useCapitals, numbers = password.useNumbers, special = password.useSpecialChars, - length = password.length.toUByte(), + length = password.minimumLength.toUByte(), avoidAmbiguous = password.avoidAmbiguousChars, minLowercase = null, minUppercase = null, @@ -773,6 +773,7 @@ class GeneratorViewModel @Inject constructor( useCapitals = action.useCapitals, ) } + updatePasswordLength() } private fun handleToggleLowercaseLetters( @@ -780,16 +781,22 @@ class GeneratorViewModel @Inject constructor( .ToggleLowercaseLettersChange, ) { updatePasswordType { currentPasswordType -> - currentPasswordType.copy(useLowercase = action.useLowercase) + currentPasswordType.copy( + useLowercase = action.useLowercase, + ) } + updatePasswordLength() } private fun handleToggleNumbers( action: GeneratorAction.MainType.Passcode.PasscodeType.Password.ToggleNumbersChange, ) { updatePasswordType { currentPasswordType -> - currentPasswordType.copy(useNumbers = action.useNumbers) + currentPasswordType.copy( + useNumbers = action.useNumbers, + ) } + updatePasswordLength() } private fun handleToggleSpecialChars( @@ -797,24 +804,33 @@ class GeneratorViewModel @Inject constructor( .ToggleSpecialCharactersChange, ) { updatePasswordType { currentPasswordType -> - currentPasswordType.copy(useSpecialChars = action.useSpecialChars) + currentPasswordType.copy( + useSpecialChars = action.useSpecialChars, + ) } + updatePasswordLength() } private fun handleMinNumbersChange( action: GeneratorAction.MainType.Passcode.PasscodeType.Password.MinNumbersCounterChange, ) { updatePasswordType { currentPasswordType -> - currentPasswordType.copy(minNumbers = action.minNumbers) + currentPasswordType.copy( + minNumbers = action.minNumbers, + ) } + updatePasswordLength() } private fun handleMinSpecialChange( action: GeneratorAction.MainType.Passcode.PasscodeType.Password.MinSpecialCharactersChange, ) { updatePasswordType { currentPasswordType -> - currentPasswordType.copy(minSpecial = action.minSpecial) + currentPasswordType.copy( + minSpecial = action.minSpecial, + ) } + updatePasswordLength() } private fun handleToggleAmbiguousChars( @@ -1385,6 +1401,17 @@ class GeneratorViewModel @Inject constructor( } } + /** + * Updates the length property of the [Password] to reflect the new minimum. + */ + private inline fun updatePasswordLength() { + updatePasswordType { currentPasswordType -> + currentPasswordType.copy( + length = currentPasswordType.minimumLength, + ) + } + } + private inline fun updatePasswordType( crossinline block: (Password) -> Password, ) { @@ -1706,13 +1733,24 @@ data class GeneratorState( * length, character types, and requirements. * * @property length The length of the generated password. + * @property minLength The number available on the low end of the slider. + * @property maxLength The number available on the high end of the slider. * @property useCapitals Whether to include capital letters. + * @property capitalsEnabled Whether capitals are enabled for this password. * @property useLowercase Whether to include lowercase letters. + * @property lowercaseEnabled Whether lowercase letters are enabled. * @property useNumbers Whether to include numbers. + * @property numbersEnabled Whether numbers are enabled for this password. * @property useSpecialChars Whether to include special characters. + * @property specialCharsEnabled Whether special characters are enabled. * @property minNumbers The minimum number of numeric characters. + * @property minNumbersAllowed The minimum number of numbers permitted. + * @property maxNumbersAllowed The maximum number of numbers permitted. * @property minSpecial The minimum number of special characters. + * @property minSpecialAllowed The minimum number of special characters permitted. + * @property maxSpecialAllowed The maximum number of special characters permitted. * @property avoidAmbiguousChars Whether to avoid characters that look similar. + * @property ambiguousCharsEnabled Whether to allow ambiguous characters. * @property isUserInteracting Indicates whether the user is currently interacting * with a control. This flag can be used to prevent unnecessary updates or * processing during continuous interaction. @@ -2508,6 +2546,21 @@ 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 max( + minLowercase + minUppercase + minimumNumbers + minimumSpecial, + length, + ) + } + private fun UsernameGenerationOptions.ForwardedEmailServiceType?.toServiceType( options: UsernameGenerationOptions, ): ForwardedEmailAlias.ServiceType? { 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 23e5e96c4..5e949c064 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 @@ -831,15 +831,22 @@ class GeneratorViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `ToggleCapitalLettersChange should update useCapitals correctly and generate text`() = + fun `ToggleCapitalLettersChange should update useCapital correctly, update length, and generate text`() = runTest { val updatedGeneratedPassword = "updatedPassword" fakeGeneratorRepository.setMockGeneratePasswordResult( GeneratedPasswordResult.Success(updatedGeneratedPassword), ) - val useCapitals = true + // Update the length to something small enough that it updates on capitals toggle. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange( + length = 5, + isUserInteracting = false, + ), + ) + // Set useCapitals to false initially to ensure length change is reflected. viewModel.trySendAction( GeneratorAction .MainType @@ -847,33 +854,87 @@ class GeneratorViewModelTest : BaseViewModelTest() { .PasscodeType .Password .ToggleCapitalLettersChange( - useCapitals = useCapitals, + useCapitals = false, ), ) - val expectedState = defaultPasswordState.copy( + // Verify useCapitals is reflected and initial length is correct. + val expectedState1 = defaultPasswordState.copy( generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - useCapitals = useCapitals, + length = 5, + useCapitals = false, ), ), ) + assertEquals(expectedState1, viewModel.stateFlow.value) - assertEquals(expectedState, viewModel.stateFlow.value) + // Update minNumbers so that length will exceed 5 when useCapitals is enabled. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.MinNumbersCounterChange( + minNumbers = 4, + ), + ) + + // Verify length is still 5, minNumbers is updated, and useCapital is still false. + val expectedState2 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 5, // 0 uppercase + 1 lowercase + 4 numbers + 0 special chars + minNumbers = 4, + useCapitals = false, + useNumbers = true, + ), + ), + ) + assertEquals(expectedState2, viewModel.stateFlow.value) + + // Enable useCapitals. + viewModel.trySendAction( + GeneratorAction + .MainType + .Passcode + .PasscodeType + .Password + .ToggleCapitalLettersChange( + useCapitals = true, + ), + ) + + // Verify this has caused length to increase. + val expectedState3 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars + minNumbers = 4, + useCapitals = true, + ), + ), + ) + assertEquals(expectedState3, viewModel.stateFlow.value) } @Suppress("MaxLineLength") @Test - fun `ToggleLowercaseLettersChange should update useLowercase correctly and generate text`() = + fun `ToggleLowercaseLettersChange should update useLowercase correctly, update length, and generate text`() = runTest { val updatedGeneratedPassword = "updatedPassword" fakeGeneratorRepository.setMockGeneratePasswordResult( GeneratedPasswordResult.Success(updatedGeneratedPassword), ) - val useLowercase = true + // Update the length to something small enough that it updates on lowercase toggle. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange( + length = 5, + isUserInteracting = false, + ), + ) + // Set useLowercase to false initially to ensure length change is reflected. viewModel.trySendAction( GeneratorAction .MainType @@ -881,20 +942,67 @@ class GeneratorViewModelTest : BaseViewModelTest() { .PasscodeType .Password .ToggleLowercaseLettersChange( - useLowercase = useLowercase, + useLowercase = false, ), ) - val expectedState = defaultPasswordState.copy( + // Verify useLowercase is reflected and initial length is correct. + val expectedState1 = defaultPasswordState.copy( generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( - useLowercase = useLowercase, + length = 5, + useLowercase = false, ), ), ) + assertEquals(expectedState1, viewModel.stateFlow.value) - assertEquals(expectedState, viewModel.stateFlow.value) + // Update minNumbers so that length will exceed 5 when useLowercase is enabled. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.MinNumbersCounterChange( + minNumbers = 4, + ), + ) + + // Verify length is still 5, minNumbers is updated, and useLowercase is still false. + val expectedState2 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 5, // 1 uppercase + 0 lowercase + 4 numbers + 0 special chars + minNumbers = 4, + useLowercase = false, + useNumbers = true, + ), + ), + ) + assertEquals(expectedState2, viewModel.stateFlow.value) + + // Enable useLowercase. + viewModel.trySendAction( + GeneratorAction + .MainType + .Passcode + .PasscodeType + .Password + .ToggleLowercaseLettersChange( + useLowercase = true, + ), + ) + + // Verify this has caused length to increase. + val expectedState3 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars + minNumbers = 4, + useLowercase = true, + ), + ), + ) + assertEquals(expectedState3, viewModel.stateFlow.value) } @Test @@ -958,45 +1066,96 @@ class GeneratorViewModelTest : BaseViewModelTest() { assertEquals(expectedState, viewModel.stateFlow.value) } - @Suppress("MaxLineLength") @Test - fun `MinNumbersCounterChange should update minNumbers correctly and generate text`() = + fun `MinNumbersCounterChange should update minNumbers, update length, and generate text`() = runTest { val updatedGeneratedPassword = "updatedPassword" fakeGeneratorRepository.setMockGeneratePasswordResult( GeneratedPasswordResult.Success(updatedGeneratedPassword), ) - val minNumbers = 4 + // Toggle numbers to false initially so we can verify length changes appropriately. + viewModel.trySendAction( + GeneratorAction + .MainType + .Passcode + .PasscodeType + .Password + .ToggleNumbersChange( + useNumbers = false, + ), + ) + // Update the length to something small enough that it's updated on numbers toggle. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange( + length = 5, + isUserInteracting = false, + ), + ) + + val minNumbers = 5 viewModel.trySendAction( GeneratorAction.MainType.Passcode.PasscodeType.Password.MinNumbersCounterChange( minNumbers = minNumbers, ), ) - val expectedState = defaultPasswordState.copy( + val expectedState1 = defaultPasswordState.copy( generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 5, // the current slider value, which the min does not exceed minNumbers = minNumbers, + useNumbers = false, ), ), ) + assertEquals(expectedState1, viewModel.stateFlow.value) - assertEquals(expectedState, viewModel.stateFlow.value) + // Toggle numbers to true so we can verify length changes appropriately. + viewModel.trySendAction( + GeneratorAction + .MainType + .Passcode + .PasscodeType + .Password + .ToggleNumbersChange( + useNumbers = true, + ), + ) + + val expectedState2 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 7, // 1 uppercase + 1 lowercase + 5 numbers + 0 special chars + minNumbers = minNumbers, + useNumbers = true, + ), + ), + ) + assertEquals(expectedState2, viewModel.stateFlow.value) } @Suppress("MaxLineLength") @Test - fun `MinSpecialCharactersChange should update minSpecial correctly and generate text`() = + fun `MinSpecialCharactersChange should update minSpecial, update length, and generate text`() = runTest { val updatedGeneratedPassword = "updatedPassword" fakeGeneratorRepository.setMockGeneratePasswordResult( GeneratedPasswordResult.Success(updatedGeneratedPassword), ) - val minSpecial = 2 + // Update the length to something small enough that it's updated on toggle. + viewModel.trySendAction( + GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange( + length = 5, + isUserInteracting = false, + ), + ) + + val minSpecial = 5 viewModel.trySendAction( GeneratorAction @@ -1009,16 +1168,42 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ) - val expectedState = defaultPasswordState.copy( + // Length should still be 5 because special characters are not enabled. + val expectedState1 = defaultPasswordState.copy( generatedText = updatedGeneratedPassword, selectedType = GeneratorState.MainType.Passcode( GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 5, minSpecial = minSpecial, + useSpecialChars = false, ), ), ) + assertEquals(expectedState1, viewModel.stateFlow.value) - assertEquals(expectedState, viewModel.stateFlow.value) + viewModel.trySendAction( + GeneratorAction + .MainType + .Passcode + .PasscodeType + .Password + .ToggleSpecialCharactersChange( + useSpecialChars = true, + ), + ) + + // Length should update to 7 because special characters are now enabled. + val expectedState2 = defaultPasswordState.copy( + generatedText = updatedGeneratedPassword, + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 8, // 1 uppercase + 1 lowercase + 1 number + 5 special chars + minSpecial = minSpecial, + useSpecialChars = true, + ), + ), + ) + assertEquals(expectedState2, viewModel.stateFlow.value) } @Suppress("MaxLineLength")