From 5b4dbbf989ce6e2549d49704be6ebcf641dec2ae Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Dec 2021 13:40:52 +0530 Subject: [PATCH 1/5] Document `BiometricAuthenticator.Result` and add a `Retry` type --- .../aps/util/auth/BiometricAuthenticator.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt index a582337b71..ca53704810 100644 --- a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt +++ b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt @@ -21,10 +21,28 @@ object BiometricAuthenticator { private const val validAuthenticators = Authenticators.DEVICE_CREDENTIAL or Authenticators.BIOMETRIC_WEAK + /** + * Sealed class to wrap [BiometricPrompt]'s [Int]-based return codes into more easily-interpreted + * types. + */ sealed class Result { + + /** Biometric authentication was a success. */ data class Success(val cryptoObject: BiometricPrompt.CryptoObject?) : Result() + + /** Biometric authentication has irreversibly failed. */ data class Failure(val code: Int?, val message: CharSequence) : Result() + + /** + * An incorrect biometric was entered, but the prompt UI is offering the option to retry the + * operation. + */ + object Retry : Result() + + /** The biometric hardware is unavailable or disabled on a software or hardware level. */ object HardwareUnavailableOrDisabled : Result() + + /** The prompt was dismissed. */ object Cancelled : Result() } From 3eb5987f5df50a664f91ebb100fa8d668fba14bf Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Dec 2021 13:41:54 +0530 Subject: [PATCH 2/5] Add all BiometricPrompt error codes and use `Result.Retry` for authentication failure --- .../aps/util/auth/BiometricAuthenticator.kt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt index ca53704810..3446a0cb27 100644 --- a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt +++ b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt @@ -63,6 +63,7 @@ object BiometricAuthenticator { logcat(TAG) { "BiometricAuthentication error: errorCode=$errorCode, msg=$errString" } callback( when (errorCode) { + BiometricPrompt.ERROR_UNABLE_TO_PROCESS, BiometricPrompt.ERROR_CANCELED, BiometricPrompt.ERROR_USER_CANCELED, BiometricPrompt.ERROR_NEGATIVE_BUTTON -> { @@ -74,18 +75,32 @@ object BiometricAuthenticator { BiometricPrompt.ERROR_NO_DEVICE_CREDENTIAL -> { Result.HardwareUnavailableOrDisabled } - else -> + BiometricPrompt.ERROR_LOCKOUT, + BiometricPrompt.ERROR_LOCKOUT_PERMANENT, + BiometricPrompt.ERROR_NO_SPACE, + BiometricPrompt.ERROR_TIMEOUT, + BiometricPrompt.ERROR_VENDOR -> { Result.Failure( errorCode, activity.getString(R.string.biometric_auth_error_reason, errString) ) + } + // We cover all guaranteed values above, but [errorCode] is still an Int at the end of + // the day so a + // catch-all else will always be required. + else -> { + Result.Failure( + errorCode, + activity.getString(R.string.biometric_auth_error_reason, errString) + ) + } } ) } override fun onAuthenticationFailed() { super.onAuthenticationFailed() - callback(Result.Failure(null, activity.getString(R.string.biometric_auth_error))) + callback(Result.Retry) } override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) { From e7f12436780797bc0d8560b241fd73fb067e5f79 Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Dec 2021 13:42:32 +0530 Subject: [PATCH 3/5] Integrate new `Result.Retry` type and add imports for `Result` --- .../java/dev/msfjarvis/aps/ui/main/LaunchActivity.kt | 12 +++++++----- .../dev/msfjarvis/aps/ui/settings/GeneralSettings.kt | 4 +++- .../msfjarvis/aps/ui/sshkeygen/SshKeyGenActivity.kt | 9 +++++---- .../msfjarvis/aps/util/git/operation/GitOperation.kt | 9 +++++---- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/dev/msfjarvis/aps/ui/main/LaunchActivity.kt b/app/src/main/java/dev/msfjarvis/aps/ui/main/LaunchActivity.kt index 629ddb9c8e..b5f7a593a7 100644 --- a/app/src/main/java/dev/msfjarvis/aps/ui/main/LaunchActivity.kt +++ b/app/src/main/java/dev/msfjarvis/aps/ui/main/LaunchActivity.kt @@ -14,6 +14,7 @@ import dev.msfjarvis.aps.ui.crypto.BasePgpActivity import dev.msfjarvis.aps.ui.crypto.DecryptActivity import dev.msfjarvis.aps.ui.passwords.PasswordStore import dev.msfjarvis.aps.util.auth.BiometricAuthenticator +import dev.msfjarvis.aps.util.auth.BiometricAuthenticator.Result import dev.msfjarvis.aps.util.extensions.sharedPrefs import dev.msfjarvis.aps.util.settings.PreferenceKeys @@ -23,18 +24,19 @@ class LaunchActivity : AppCompatActivity() { super.onCreate(savedInstanceState) val prefs = sharedPrefs if (prefs.getBoolean(PreferenceKeys.BIOMETRIC_AUTH, false)) { - BiometricAuthenticator.authenticate(this) { - when (it) { - is BiometricAuthenticator.Result.Success -> { + BiometricAuthenticator.authenticate(this) { result -> + when (result) { + is Result.Success -> { startTargetActivity(false) } - is BiometricAuthenticator.Result.HardwareUnavailableOrDisabled -> { + is Result.HardwareUnavailableOrDisabled -> { prefs.edit { remove(PreferenceKeys.BIOMETRIC_AUTH) } startTargetActivity(false) } - is BiometricAuthenticator.Result.Failure, BiometricAuthenticator.Result.Cancelled -> { + is Result.Failure, Result.Cancelled -> { finish() } + is Result.Retry -> {} } } } else { diff --git a/app/src/main/java/dev/msfjarvis/aps/ui/settings/GeneralSettings.kt b/app/src/main/java/dev/msfjarvis/aps/ui/settings/GeneralSettings.kt index 1e67a2b5b5..f78f2cb28a 100644 --- a/app/src/main/java/dev/msfjarvis/aps/ui/settings/GeneralSettings.kt +++ b/app/src/main/java/dev/msfjarvis/aps/ui/settings/GeneralSettings.kt @@ -17,6 +17,7 @@ import de.Maxr1998.modernpreferences.helpers.singleChoice import de.Maxr1998.modernpreferences.preferences.choice.SelectionItem import dev.msfjarvis.aps.R import dev.msfjarvis.aps.util.auth.BiometricAuthenticator +import dev.msfjarvis.aps.util.auth.BiometricAuthenticator.Result import dev.msfjarvis.aps.util.extensions.sharedPrefs import dev.msfjarvis.aps.util.settings.PreferenceKeys @@ -73,11 +74,12 @@ class GeneralSettings(private val activity: FragmentActivity) : SettingsProvider activity.sharedPrefs.edit { BiometricAuthenticator.authenticate(activity) { result -> when (result) { - is BiometricAuthenticator.Result.Success -> { + is Result.Success -> { // Apply the changes putBoolean(PreferenceKeys.BIOMETRIC_AUTH, checked) enabled = true } + is Result.Retry -> {} else -> { // If any error occurs, revert back to the previous // state. This diff --git a/app/src/main/java/dev/msfjarvis/aps/ui/sshkeygen/SshKeyGenActivity.kt b/app/src/main/java/dev/msfjarvis/aps/ui/sshkeygen/SshKeyGenActivity.kt index 0d8ffcbccc..63abdf7c37 100644 --- a/app/src/main/java/dev/msfjarvis/aps/ui/sshkeygen/SshKeyGenActivity.kt +++ b/app/src/main/java/dev/msfjarvis/aps/ui/sshkeygen/SshKeyGenActivity.kt @@ -22,6 +22,7 @@ import dev.msfjarvis.aps.R import dev.msfjarvis.aps.databinding.ActivitySshKeygenBinding import dev.msfjarvis.aps.injection.prefs.GitPreferences import dev.msfjarvis.aps.util.auth.BiometricAuthenticator +import dev.msfjarvis.aps.util.auth.BiometricAuthenticator.Result import dev.msfjarvis.aps.util.extensions.keyguardManager import dev.msfjarvis.aps.util.extensions.viewBinding import dev.msfjarvis.aps.util.git.sshj.SshKey @@ -121,17 +122,17 @@ class SshKeyGenActivity : AppCompatActivity() { if (requireAuthentication) { val result = withContext(Dispatchers.Main) { - suspendCoroutine { cont -> + suspendCoroutine { cont -> BiometricAuthenticator.authenticate( this@SshKeyGenActivity, R.string.biometric_prompt_title_ssh_keygen - ) { + ) { result -> // Do not cancel on failed attempts as these are handled by the authenticator UI. - if (it !is BiometricAuthenticator.Result.Failure) cont.resume(it) + if (result !is Result.Retry) cont.resume(result) } } } - if (result !is BiometricAuthenticator.Result.Success) + if (result !is Result.Success) throw UserNotAuthenticatedException(getString(R.string.biometric_auth_generic_failure)) } keyGenType.generateKey(requireAuthentication) diff --git a/app/src/main/java/dev/msfjarvis/aps/util/git/operation/GitOperation.kt b/app/src/main/java/dev/msfjarvis/aps/util/git/operation/GitOperation.kt index 736b69c2cd..141de348fe 100644 --- a/app/src/main/java/dev/msfjarvis/aps/util/git/operation/GitOperation.kt +++ b/app/src/main/java/dev/msfjarvis/aps/util/git/operation/GitOperation.kt @@ -22,6 +22,7 @@ import dev.msfjarvis.aps.data.repo.PasswordRepository import dev.msfjarvis.aps.ui.sshkeygen.SshKeyGenActivity import dev.msfjarvis.aps.ui.sshkeygen.SshKeyImportActivity import dev.msfjarvis.aps.util.auth.BiometricAuthenticator +import dev.msfjarvis.aps.util.auth.BiometricAuthenticator.Result.* import dev.msfjarvis.aps.util.git.GitCommandExecutor import dev.msfjarvis.aps.util.git.sshj.ContinuationContainerActivity import dev.msfjarvis.aps.util.git.sshj.SshAuthMethod @@ -172,17 +173,17 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) { BiometricAuthenticator.authenticate( callingActivity, R.string.biometric_prompt_title_ssh_auth - ) { if (it !is BiometricAuthenticator.Result.Failure) cont.resume(it) } + ) { result -> if (result !is Failure) cont.resume(result) } } } when (result) { - is BiometricAuthenticator.Result.Success -> { + is Success -> { registerAuthProviders(SshAuthMethod.SshKey(authActivity)) } - is BiometricAuthenticator.Result.Cancelled -> { + is Cancelled -> { return Err(SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER)) } - is BiometricAuthenticator.Result.Failure -> { + is Failure -> { throw IllegalStateException("Biometric authentication failures should be ignored") } else -> { From 9216a0752485fd18c2482add11cffd23dc64aa0e Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Dec 2021 14:08:01 +0530 Subject: [PATCH 4/5] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 307f7a8244..2c7d201819 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to this project will be documented in this file. - .gpg-id file generated by APS did not work with pass CLI - All but the latest launcher shortcut would have an empty icon - When prompted to select a GPG key during onboarding, the app would crash if the user did not make a selection in OpenKeychain +- Biometric authentication prompts no longer inexplicably dismiss when an incorrect biometric is entered ### Changed From a398e69a184f021a4f4d3890901a859ada74a32c Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Dec 2021 15:58:33 +0530 Subject: [PATCH 5/5] Make ERROR_UNABLE_TO_PROCESS retry --- .../dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt index 3446a0cb27..50f11b5b6c 100644 --- a/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt +++ b/app/src/main/java/dev/msfjarvis/aps/util/auth/BiometricAuthenticator.kt @@ -63,7 +63,6 @@ object BiometricAuthenticator { logcat(TAG) { "BiometricAuthentication error: errorCode=$errorCode, msg=$errString" } callback( when (errorCode) { - BiometricPrompt.ERROR_UNABLE_TO_PROCESS, BiometricPrompt.ERROR_CANCELED, BiometricPrompt.ERROR_USER_CANCELED, BiometricPrompt.ERROR_NEGATIVE_BUTTON -> { @@ -85,6 +84,9 @@ object BiometricAuthenticator { activity.getString(R.string.biometric_auth_error_reason, errString) ) } + BiometricPrompt.ERROR_UNABLE_TO_PROCESS -> { + Result.Retry + } // We cover all guaranteed values above, but [errorCode] is still an Int at the end of // the day so a // catch-all else will always be required.