diff --git a/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/Dialogs.kt b/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/Dialogs.kt index 140fb5ec..79fed079 100644 --- a/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/Dialogs.kt +++ b/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/Dialogs.kt @@ -555,9 +555,9 @@ fun InputPasscodeDialog( .focusRequester(requester), value = passcode, onValueChange = { newPasscode -> - if (newPasscode.length <= 16 && - (newPasscode.toIntOrNull() != null || newPasscode.isEmpty()) - ) { + // Do not parse as Int, passcodes longer than 9 digits + // would overflow and get rejected + if (newPasscode.length <= 16 && newPasscode.all { it.isDigit() }) { setPasscode(newPasscode) } }, @@ -582,7 +582,9 @@ fun InputPasscodeDialog( TextButton( onClick = { - if (passcode.length < 4 || passcode.toIntOrNull() == null) { + // Keep in sync with the input filter above: passcodes may + // be up to 16 digits, which would overflow toIntOrNull() + if (passcode.length < 4 || !passcode.all { it.isDigit() }) { showEmptyError = true } else { onInputPasscode(passcode) diff --git a/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/NCPAppLock.kt b/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/NCPAppLock.kt index 347cf0d6..99b60ef3 100644 --- a/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/NCPAppLock.kt +++ b/app/src/main/java/com/hegocre/nextcloudpasswords/ui/components/NCPAppLock.kt @@ -63,6 +63,7 @@ import com.hegocre.nextcloudpasswords.ui.theme.NextcloudPasswordsTheme import com.hegocre.nextcloudpasswords.utils.AppLockHelper import com.hegocre.nextcloudpasswords.utils.PreferencesManager import com.hegocre.nextcloudpasswords.utils.showBiometricPrompt +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers @@ -87,7 +88,8 @@ fun NCPAppLockWrapper( if (locked) { NextcloudPasswordsAppLock( onCheckPasscode = appLockHelper::checkPasscode, - onCorrectPasscode = appLockHelper::disableLock + onCorrectPasscode = appLockHelper::disableLock, + onGetPasscodeLength = appLockHelper::getPasscodeLength ) } else { if (hasAppLock == false) { @@ -103,15 +105,23 @@ fun NCPAppLockWrapper( @Composable fun NextcloudPasswordsAppLock( onCheckPasscode: (String) -> Deferred, - onCorrectPasscode: () -> Unit + onCorrectPasscode: () -> Unit, + onGetPasscodeLength: () -> Deferred = { CompletableDeferred(null) } ) { val isPreview = LocalInspectionMode.current - val (inputPassword, setInputPassword) = rememberSaveable { + var inputPassword by rememberSaveable { mutableStateOf("") } var isError by remember { mutableStateOf(false) } + var passcodeLength by remember { mutableStateOf(null) } + var passcodeLengthLoaded by remember { mutableStateOf(false) } + LaunchedEffect(key1 = Unit) { + passcodeLength = onGetPasscodeLength().await() + passcodeLengthLoaded = true + } + val context = LocalContext.current val hasBiometricAppLock by if (isPreview) remember { mutableStateOf(true) } else PreferencesManager.getInstance(context).getHasBiometricAppLock().collectAsState( @@ -126,9 +136,18 @@ fun NextcloudPasswordsAppLock( val biometricPromptTitle = stringResource(R.string.biometric_prompt_title) val biometricPromptDescription = stringResource(R.string.biometric_prompt_description) - LaunchedEffect(key1 = inputPassword) { + LaunchedEffect(inputPassword, passcodeLengthLoaded) { if (onCheckPasscode(inputPassword).await()) { onCorrectPasscode() + } else if (passcodeLengthLoaded && + AppLockHelper.shouldRejectPasscodeAttempt(inputPassword, passcodeLength) + ) { + // Complete but incorrect passcode, or the stored passcode is not + // readable: give feedback instead of accepting digits endlessly. + // As with any submit-less PIN pad, clearing on the last digit + // reveals the passcode length, which the indicator dots already show. + isError = true + inputPassword = "" } } @@ -158,17 +177,17 @@ fun NextcloudPasswordsAppLock( .onKeyEvent { keyEvent -> if (keyEvent.type == KeyEventType.KeyDown) { when (keyEvent.key) { - Key.Zero, Key.NumPad0 -> setInputPassword(inputPassword + "0") - Key.One, Key.NumPad1 -> setInputPassword(inputPassword + "1") - Key.Two, Key.NumPad2 -> setInputPassword(inputPassword + "2") - Key.Three, Key.NumPad3 -> setInputPassword(inputPassword + "3") - Key.Four, Key.NumPad4 -> setInputPassword(inputPassword + "4") - Key.Five, Key.NumPad5 -> setInputPassword(inputPassword + "5") - Key.Six, Key.NumPad6 -> setInputPassword(inputPassword + "6") - Key.Seven, Key.NumPad7 -> setInputPassword(inputPassword + "7") - Key.Eight, Key.NumPad8 -> setInputPassword(inputPassword + "8") - Key.Nine, Key.NumPad9 -> setInputPassword(inputPassword + "9") - Key.Backspace -> setInputPassword(inputPassword.dropLast(1)) + Key.Zero, Key.NumPad0 -> inputPassword += "0" + Key.One, Key.NumPad1 -> inputPassword += "1" + Key.Two, Key.NumPad2 -> inputPassword += "2" + Key.Three, Key.NumPad3 -> inputPassword += "3" + Key.Four, Key.NumPad4 -> inputPassword += "4" + Key.Five, Key.NumPad5 -> inputPassword += "5" + Key.Six, Key.NumPad6 -> inputPassword += "6" + Key.Seven, Key.NumPad7 -> inputPassword += "7" + Key.Eight, Key.NumPad8 -> inputPassword += "8" + Key.Nine, Key.NumPad9 -> inputPassword += "9" + Key.Backspace -> inputPassword = inputPassword.dropLast(1) } } return@onKeyEvent true @@ -201,7 +220,7 @@ fun NextcloudPasswordsAppLock( KeyPad( inputPassword = inputPassword, - setInputPassword = { setInputPassword(inputPassword + it) }, + setInputPassword = { inputPassword += it }, showBiometricIndicator = hasBiometricAppLock && canAuthenticateBiometric, onBiometricClick = { showBiometricPrompt( @@ -212,8 +231,8 @@ fun NextcloudPasswordsAppLock( ) }, showBackspaceIndicator = inputPassword.isNotBlank(), - onBackspaceClick = { setInputPassword(inputPassword.dropLast(1)) }, - onBackspaceLongClick = { setInputPassword("") } + onBackspaceClick = { inputPassword = inputPassword.dropLast(1) }, + onBackspaceLongClick = { inputPassword = "" } ) } } else { @@ -234,7 +253,7 @@ fun NextcloudPasswordsAppLock( ) { KeyPad( inputPassword = inputPassword, - setInputPassword = { setInputPassword(inputPassword + it) }, + setInputPassword = { inputPassword += it }, showBiometricIndicator = hasBiometricAppLock && canAuthenticateBiometric, onBiometricClick = { showBiometricPrompt( @@ -245,8 +264,8 @@ fun NextcloudPasswordsAppLock( ) }, showBackspaceIndicator = inputPassword.isNotBlank(), - onBackspaceClick = { setInputPassword(inputPassword.dropLast(1)) }, - onBackspaceLongClick = { setInputPassword("") } + onBackspaceClick = { inputPassword = inputPassword.dropLast(1) }, + onBackspaceLongClick = { inputPassword = "" } ) } } diff --git a/app/src/main/java/com/hegocre/nextcloudpasswords/utils/AppLockHelper.kt b/app/src/main/java/com/hegocre/nextcloudpasswords/utils/AppLockHelper.kt index 72899ddd..669cdbe2 100644 --- a/app/src/main/java/com/hegocre/nextcloudpasswords/utils/AppLockHelper.kt +++ b/app/src/main/java/com/hegocre/nextcloudpasswords/utils/AppLockHelper.kt @@ -19,11 +19,20 @@ class AppLockHelper private constructor(context: Context) { fun checkPasscode(passcode: String): Deferred { return CoroutineScope(Dispatchers.Default).async { - val correctPasscode = preferencesManager.getAppLockPasscode() ?: "0000" + // If the stored passcode cannot be read, never accept any input instead + // of silently falling back to a default passcode + val correctPasscode = preferencesManager.getAppLockPasscode() + ?: return@async false passcode == correctPasscode } } + fun getPasscodeLength(): Deferred { + return CoroutineScope(Dispatchers.Default).async { + preferencesManager.getAppLockPasscode()?.length + } + } + fun disableLock() { CoroutineScope(Dispatchers.Default).launch { _isLocked.emit(false) @@ -49,5 +58,20 @@ class AppLockHelper private constructor(context: Context) { return tempInstance } } + + /** + * Decides whether an incorrect passcode [input] should be rejected with + * visible feedback (and the input cleared). Because the passcode dialog + * has no submit button, an attempt is only considered complete once it + * reaches the length of the stored passcode. + * + * @param correctPasscodeLength length of the stored passcode, or `null` + * when it cannot be read. In the latter case any non-empty [input] is + * rejected, as the passcode can never be verified. + */ + fun shouldRejectPasscodeAttempt(input: String, correctPasscodeLength: Int?): Boolean { + if (input.isEmpty()) return false + return correctPasscodeLength == null || input.length >= correctPasscodeLength + } } } \ No newline at end of file diff --git a/app/src/test/java/com/hegocre/nextcloudpasswords/AppLockHelperTest.kt b/app/src/test/java/com/hegocre/nextcloudpasswords/AppLockHelperTest.kt new file mode 100644 index 00000000..11e08825 --- /dev/null +++ b/app/src/test/java/com/hegocre/nextcloudpasswords/AppLockHelperTest.kt @@ -0,0 +1,50 @@ +package com.hegocre.nextcloudpasswords + +import com.hegocre.nextcloudpasswords.utils.AppLockHelper +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for the passcode attempt evaluation used by the app lock screen. + */ +class AppLockHelperTest { + @Test + fun emptyInputIsNeverRejected() { + assertFalse(AppLockHelper.shouldRejectPasscodeAttempt("", 4)) + assertFalse(AppLockHelper.shouldRejectPasscodeAttempt("", null)) + } + + @Test + fun incompleteInputIsNotRejected() { + assertFalse(AppLockHelper.shouldRejectPasscodeAttempt("12", 4)) + assertFalse(AppLockHelper.shouldRejectPasscodeAttempt("123", 4)) + } + + @Test + fun fullLengthIncorrectInputIsRejected() { + assertTrue(AppLockHelper.shouldRejectPasscodeAttempt("9999", 4)) + } + + @Test + fun overLengthInputIsRejected() { + // Guards against physical keyboard input exceeding the passcode length + assertTrue(AppLockHelper.shouldRejectPasscodeAttempt("99999", 4)) + } + + @Test + fun unreadablePasscodeRejectsAnyNonEmptyInput() { + // When the stored passcode cannot be read, the code can never be + // verified, so no input should be silently accepted + assertTrue(AppLockHelper.shouldRejectPasscodeAttempt("1", null)) + assertTrue(AppLockHelper.shouldRejectPasscodeAttempt("123456", null)) + } + + @Test + fun longPasscodeIsSupported() { + // Passcodes longer than 9 digits must still be handled (they used to + // overflow when parsed as an Int) + assertFalse(AppLockHelper.shouldRejectPasscodeAttempt("1234567890", 12)) + assertTrue(AppLockHelper.shouldRejectPasscodeAttempt("123456789012", 12)) + } +}