Fix PIN unlock unusable after cancelling biometric prompt (#151)

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)

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<Boolean>,
-    onCorrectPasscode: () -> Unit
+    onCorrectPasscode: () -> Unit,
+    onGetPasscodeLength: () -> Deferred<Int?> = { 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<Int?>(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 = "" }
                             )
                         }
                     }

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<Boolean> {
         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<Int?> {
+        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
+        }
     }
 }

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))
+    }
+}