From d4df20cb73167936c4f2748797bbdc91db2fc520 Mon Sep 17 00:00:00 2001 From: MiMoHo <37556964+MiMoHo@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:53:16 +0200 Subject: [PATCH] Fix PIN unlock unusable after cancelling biometric prompt (#151) The app lock passcode screen has no submit button and verifies the code on every keystroke, but a wrong code was never rejected. After cancelling the biometric prompt and falling back to the PIN keypad, the user could type digits endlessly with no feedback and no way to submit, so a correct PIN was effectively unreachable. - NCPAppLock: once the entered code reaches the stored passcode length and is still wrong, show the existing "Incorrect code" error and clear the input. - AppLockHelper: checkPasscode no longer accepts the hardcoded "0000" fallback when the stored passcode is unreadable; added a pure, unit-tested helper shouldRejectPasscodeAttempt. - Dialogs: InputPasscodeDialog now validates with all { isDigit() } instead of toIntOrNull(), which silently rejected passcodes longer than ~9 digits even though the field allows up to 16. Adds unit tests for the passcode attempt evaluation. Co-Authored-By: Claude Fable 5 --- .../ui/components/Dialogs.kt | 10 +-- .../ui/components/NCPAppLock.kt | 61 ++++++++++++------- .../nextcloudpasswords/utils/AppLockHelper.kt | 26 +++++++- .../nextcloudpasswords/AppLockHelperTest.kt | 50 +++++++++++++++ 4 files changed, 121 insertions(+), 26 deletions(-) create mode 100644 app/src/test/java/com/hegocre/nextcloudpasswords/AppLockHelperTest.kt 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)) + } +}