mirror of
https://github.com/bitwarden/android.git
synced 2024-10-31 07:05:35 +03:00
BIT-2136: Fix generator error when length can't accommodate settings (#1181)
This commit is contained in:
Caleb Derosier
Álison Fernandes
parent
de1c76d772
commit
c8525989e0
2 changed files with 267 additions and 29 deletions
|
|
@ -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? {
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Reference in a new issue