BIT-2136: Update minimum length handling in Generator (#1238)

This commit is contained in:
Caleb Derosier 2024-04-08 14:16:28 -06:00 committed by Álison Fernandes
parent 72d3758a12
commit ee12bd9da5
4 changed files with 43 additions and 32 deletions

View file

@ -28,9 +28,9 @@ import androidx.compose.material3.TopAppBarScrollBehavior
import androidx.compose.material3.rememberTopAppBarState import androidx.compose.material3.rememberTopAppBarState
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
@ -580,7 +580,7 @@ private fun PasswordLengthSliderItem(
minValue: Int, minValue: Int,
maxValue: Int, maxValue: Int,
) { ) {
var sliderValue by remember { mutableIntStateOf(length.coerceIn(minValue, maxValue)) } val sliderValue by rememberUpdatedState(newValue = length.coerceIn(minValue, maxValue))
var labelTextWidth by remember { mutableStateOf(Dp.Unspecified) } var labelTextWidth by remember { mutableStateOf(Dp.Unspecified) }
val density = LocalDensity.current val density = LocalDensity.current
@ -617,8 +617,7 @@ private fun PasswordLengthSliderItem(
Slider( Slider(
value = sliderValue.toFloat(), value = sliderValue.toFloat(),
onValueChange = { newValue -> onValueChange = { newValue ->
sliderValue = newValue.toInt().coerceIn(minValue, maxValue) onPasswordSliderLengthChange(newValue.toInt(), true)
onPasswordSliderLengthChange(sliderValue, true)
}, },
onValueChangeFinished = { onValueChangeFinished = {
onPasswordSliderLengthChange(sliderValue, false) onPasswordSliderLengthChange(sliderValue, false)

View file

@ -300,7 +300,7 @@ class GeneratorViewModel @Inject constructor(
is Password -> { is Password -> {
val minLength = policy.minLength ?: Password.PASSWORD_LENGTH_SLIDER_MIN val minLength = policy.minLength ?: Password.PASSWORD_LENGTH_SLIDER_MIN
val password = Password( val password = Password(
length = max(options.length, minLength), rawLength = max(options.length, minLength),
minLength = minLength, minLength = minLength,
useCapitals = options.hasUppercase || policy.useUpper == true, useCapitals = options.hasUppercase || policy.useUpper == true,
capitalsEnabled = policy.useUpper != true, capitalsEnabled = policy.useUpper != true,
@ -372,7 +372,7 @@ class GeneratorViewModel @Inject constructor(
val options = generatorRepository val options = generatorRepository
.getPasscodeGenerationOptions() ?: generatePasscodeDefaultOptions() .getPasscodeGenerationOptions() ?: generatePasscodeDefaultOptions()
val newOptions = options.copy( val newOptions = options.copy(
length = password.minimumLength, length = password.length,
allowAmbiguousChar = password.avoidAmbiguousChars, allowAmbiguousChar = password.avoidAmbiguousChars,
hasNumbers = password.useNumbers, hasNumbers = password.useNumbers,
minNumber = password.minNumbers, minNumber = password.minNumbers,
@ -477,7 +477,7 @@ class GeneratorViewModel @Inject constructor(
val defaultPassphrase = Passphrase() val defaultPassphrase = Passphrase()
return PasscodeGenerationOptions( return PasscodeGenerationOptions(
length = defaultPassword.minimumLength, length = defaultPassword.length,
allowAmbiguousChar = defaultPassword.avoidAmbiguousChars, allowAmbiguousChar = defaultPassword.avoidAmbiguousChars,
hasNumbers = defaultPassword.useNumbers, hasNumbers = defaultPassword.useNumbers,
minNumber = defaultPassword.minNumbers, minNumber = defaultPassword.minNumbers,
@ -518,7 +518,7 @@ class GeneratorViewModel @Inject constructor(
uppercase = password.useCapitals, uppercase = password.useCapitals,
numbers = password.useNumbers, numbers = password.useNumbers,
special = password.useSpecialChars, special = password.useSpecialChars,
length = password.minimumLength.toUByte(), length = password.length.toUByte(),
avoidAmbiguous = password.avoidAmbiguousChars, avoidAmbiguous = password.avoidAmbiguousChars,
minLowercase = null, minLowercase = null,
minUppercase = null, minUppercase = null,
@ -764,10 +764,14 @@ class GeneratorViewModel @Inject constructor(
updatePasswordType { currentPasswordType -> updatePasswordType { currentPasswordType ->
currentPasswordType.copy( currentPasswordType.copy(
length = adjustedLength, rawLength = adjustedLength.coerceIn(
minimumValue = currentPasswordType.minLength,
maximumValue = currentPasswordType.maxLength,
),
isUserInteracting = action.isUserInteracting, isUserInteracting = action.isUserInteracting,
) )
} }
updatePasswordLength()
} }
private fun handleToggleCapitalLetters( private fun handleToggleCapitalLetters(
@ -1412,7 +1416,7 @@ class GeneratorViewModel @Inject constructor(
private fun updatePasswordLength() { private fun updatePasswordLength() {
updatePasswordType { currentPasswordType -> updatePasswordType { currentPasswordType ->
currentPasswordType.copy( currentPasswordType.copy(
length = currentPasswordType.minimumLength, rawLength = currentPasswordType.length,
) )
} }
} }
@ -1737,7 +1741,7 @@ data class GeneratorState(
* Represents a standard PASSWORD type, with configurable options for * Represents a standard PASSWORD type, with configurable options for
* length, character types, and requirements. * length, character types, and requirements.
* *
* @property length The length of the generated password. * @property rawLength The length of the generated password.
* @property minLength The number available on the low end of the slider. * @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 maxLength The number available on the high end of the slider.
* @property useCapitals Whether to include capital letters. * @property useCapitals Whether to include capital letters.
@ -1762,7 +1766,7 @@ data class GeneratorState(
*/ */
@Parcelize @Parcelize
data class Password( data class Password(
val length: Int = DEFAULT_PASSWORD_LENGTH, private val rawLength: Int = DEFAULT_PASSWORD_LENGTH,
val minLength: Int = PASSWORD_LENGTH_SLIDER_MIN, val minLength: Int = PASSWORD_LENGTH_SLIDER_MIN,
val maxLength: Int = PASSWORD_LENGTH_SLIDER_MAX, val maxLength: Int = PASSWORD_LENGTH_SLIDER_MAX,
val useCapitals: Boolean = true, val useCapitals: Boolean = true,
@ -1786,6 +1790,13 @@ data class GeneratorState(
override val displayStringResId: Int override val displayStringResId: Int
get() = PasscodeTypeOption.PASSWORD.labelRes get() = PasscodeTypeOption.PASSWORD.labelRes
/**
* The larger of the user-set length and the minimum length this password can be
* (as determined by which characters are enabled, and how many).
*/
val length: Int
get() = max(rawLength, minimumLength)
companion object { companion object {
private const val DEFAULT_PASSWORD_LENGTH: Int = 14 private const val DEFAULT_PASSWORD_LENGTH: Int = 14
private const val MIN_NUMBERS: Int = 1 private const val MIN_NUMBERS: Int = 1
@ -2560,10 +2571,7 @@ private val Password.minimumLength: Int
val minUppercase = if (useCapitals) 1 else 0 val minUppercase = if (useCapitals) 1 else 0
val minimumNumbers = if (useNumbers) max(1, minNumbers) else 0 val minimumNumbers = if (useNumbers) max(1, minNumbers) else 0
val minimumSpecial = if (useSpecialChars) max(1, minSpecial) else 0 val minimumSpecial = if (useSpecialChars) max(1, minSpecial) else 0
return max( return minLowercase + minUppercase + minimumNumbers + minimumSpecial
minLowercase + minUppercase + minimumNumbers + minimumSpecial,
length,
)
} }
private fun UsernameGenerationOptions.ForwardedEmailServiceType?.toServiceType( private fun UsernameGenerationOptions.ForwardedEmailServiceType?.toServiceType(

View file

@ -374,10 +374,14 @@ class GeneratorScreenTest : BaseComposeTest() {
) )
} }
// This value would be 128 in a real scenario, because length passed here depends on the
// `sliderValue` which is indirectly updated via the call verified above. However, because
// the view model is a mock, the `length` value that `sliderValue` depends on will not
// actually get updated from its original value, and thus will be its original value of 14.
verify { verify {
viewModel.trySendAction( viewModel.trySendAction(
GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange( GeneratorAction.MainType.Passcode.PasscodeType.Password.SliderLengthChange(
length = 128, length = 14,
isUserInteracting = false, isUserInteracting = false,
), ),
) )

View file

@ -148,7 +148,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
initialPasscodeState.copy( initialPasscodeState.copy(
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password( selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 14, rawLength = 14,
minLength = 10, minLength = 10,
maxLength = 128, maxLength = 128,
useCapitals = true, useCapitals = true,
@ -182,7 +182,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
initialPasscodeState.copy( initialPasscodeState.copy(
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password( selectedType = GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 14, rawLength = 14,
minLength = 5, minLength = 5,
maxLength = 128, maxLength = 128,
useCapitals = true, useCapitals = true,
@ -499,7 +499,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
initialPasscodeState.copy( initialPasscodeState.copy(
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 14, rawLength = 14,
minLength = 10, minLength = 10,
useCapitals = true, useCapitals = true,
capitalsEnabled = false, capitalsEnabled = false,
@ -821,7 +821,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = newLength, rawLength = newLength,
), ),
), ),
) )
@ -863,7 +863,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, rawLength = 5,
useCapitals = false, useCapitals = false,
), ),
), ),
@ -882,7 +882,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, // 0 uppercase + 1 lowercase + 4 numbers + 0 special chars rawLength = 5, // 0 uppercase + 1 lowercase + 4 numbers + 0 special chars
minNumbers = 4, minNumbers = 4,
useCapitals = false, useCapitals = false,
useNumbers = true, useNumbers = true,
@ -908,7 +908,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars rawLength = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars
minNumbers = 4, minNumbers = 4,
useCapitals = true, useCapitals = true,
), ),
@ -951,7 +951,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, rawLength = 5,
useLowercase = false, useLowercase = false,
), ),
), ),
@ -970,7 +970,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, // 1 uppercase + 0 lowercase + 4 numbers + 0 special chars rawLength = 5, // 1 uppercase + 0 lowercase + 4 numbers + 0 special chars
minNumbers = 4, minNumbers = 4,
useLowercase = false, useLowercase = false,
useNumbers = true, useNumbers = true,
@ -996,7 +996,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars rawLength = 6, // 1 uppercase + 1 lowercase + 4 numbers + 0 special chars
minNumbers = 4, minNumbers = 4,
useLowercase = true, useLowercase = true,
), ),
@ -1105,7 +1105,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, // the current slider value, which the min does not exceed rawLength = 5, // the current slider value, which the min doesn't exceed
minNumbers = minNumbers, minNumbers = minNumbers,
useNumbers = false, useNumbers = false,
), ),
@ -1129,7 +1129,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 7, // 1 uppercase + 1 lowercase + 5 numbers + 0 special chars rawLength = 7, // 1 uppercase + 1 lowercase + 5 numbers + 0 special
minNumbers = minNumbers, minNumbers = minNumbers,
useNumbers = true, useNumbers = true,
), ),
@ -1173,7 +1173,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 5, rawLength = 5,
minSpecial = minSpecial, minSpecial = minSpecial,
useSpecialChars = false, useSpecialChars = false,
), ),
@ -1197,7 +1197,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = updatedGeneratedPassword, generatedText = updatedGeneratedPassword,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = 8, // 1 uppercase + 1 lowercase + 1 number + 5 special chars rawLength = 8, // 1 uppercase + 1 lowercase + 1 number + 5 special chars
minSpecial = minSpecial, minSpecial = minSpecial,
useSpecialChars = true, useSpecialChars = true,
), ),
@ -2098,7 +2098,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
generatedText = generatedText, generatedText = generatedText,
selectedType = GeneratorState.MainType.Passcode( selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password( GeneratorState.MainType.Passcode.PasscodeType.Password(
length = length, rawLength = length,
useCapitals = useCapitals, useCapitals = useCapitals,
useLowercase = useLowercase, useLowercase = useLowercase,
useNumbers = useNumbers, useNumbers = useNumbers,