Refactor BiometricAuthenticator and add proper support for retries (#1627)
This commit is contained in:
parent
8b5be3f785
commit
4c9413709d
6 changed files with 58 additions and 16 deletions
|
@ -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
|
- .gpg-id file generated by APS did not work with pass CLI
|
||||||
- All but the latest launcher shortcut would have an empty icon
|
- 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
|
- 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
|
### Changed
|
||||||
|
|
||||||
|
|
|
@ -14,6 +14,7 @@ import dev.msfjarvis.aps.ui.crypto.BasePgpActivity
|
||||||
import dev.msfjarvis.aps.ui.crypto.DecryptActivity
|
import dev.msfjarvis.aps.ui.crypto.DecryptActivity
|
||||||
import dev.msfjarvis.aps.ui.passwords.PasswordStore
|
import dev.msfjarvis.aps.ui.passwords.PasswordStore
|
||||||
import dev.msfjarvis.aps.util.auth.BiometricAuthenticator
|
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.extensions.sharedPrefs
|
||||||
import dev.msfjarvis.aps.util.settings.PreferenceKeys
|
import dev.msfjarvis.aps.util.settings.PreferenceKeys
|
||||||
|
|
||||||
|
@ -23,18 +24,19 @@ class LaunchActivity : AppCompatActivity() {
|
||||||
super.onCreate(savedInstanceState)
|
super.onCreate(savedInstanceState)
|
||||||
val prefs = sharedPrefs
|
val prefs = sharedPrefs
|
||||||
if (prefs.getBoolean(PreferenceKeys.BIOMETRIC_AUTH, false)) {
|
if (prefs.getBoolean(PreferenceKeys.BIOMETRIC_AUTH, false)) {
|
||||||
BiometricAuthenticator.authenticate(this) {
|
BiometricAuthenticator.authenticate(this) { result ->
|
||||||
when (it) {
|
when (result) {
|
||||||
is BiometricAuthenticator.Result.Success -> {
|
is Result.Success -> {
|
||||||
startTargetActivity(false)
|
startTargetActivity(false)
|
||||||
}
|
}
|
||||||
is BiometricAuthenticator.Result.HardwareUnavailableOrDisabled -> {
|
is Result.HardwareUnavailableOrDisabled -> {
|
||||||
prefs.edit { remove(PreferenceKeys.BIOMETRIC_AUTH) }
|
prefs.edit { remove(PreferenceKeys.BIOMETRIC_AUTH) }
|
||||||
startTargetActivity(false)
|
startTargetActivity(false)
|
||||||
}
|
}
|
||||||
is BiometricAuthenticator.Result.Failure, BiometricAuthenticator.Result.Cancelled -> {
|
is Result.Failure, Result.Cancelled -> {
|
||||||
finish()
|
finish()
|
||||||
}
|
}
|
||||||
|
is Result.Retry -> {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
|
|
@ -17,6 +17,7 @@ import de.Maxr1998.modernpreferences.helpers.singleChoice
|
||||||
import de.Maxr1998.modernpreferences.preferences.choice.SelectionItem
|
import de.Maxr1998.modernpreferences.preferences.choice.SelectionItem
|
||||||
import dev.msfjarvis.aps.R
|
import dev.msfjarvis.aps.R
|
||||||
import dev.msfjarvis.aps.util.auth.BiometricAuthenticator
|
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.extensions.sharedPrefs
|
||||||
import dev.msfjarvis.aps.util.settings.PreferenceKeys
|
import dev.msfjarvis.aps.util.settings.PreferenceKeys
|
||||||
|
|
||||||
|
@ -73,11 +74,12 @@ class GeneralSettings(private val activity: FragmentActivity) : SettingsProvider
|
||||||
activity.sharedPrefs.edit {
|
activity.sharedPrefs.edit {
|
||||||
BiometricAuthenticator.authenticate(activity) { result ->
|
BiometricAuthenticator.authenticate(activity) { result ->
|
||||||
when (result) {
|
when (result) {
|
||||||
is BiometricAuthenticator.Result.Success -> {
|
is Result.Success -> {
|
||||||
// Apply the changes
|
// Apply the changes
|
||||||
putBoolean(PreferenceKeys.BIOMETRIC_AUTH, checked)
|
putBoolean(PreferenceKeys.BIOMETRIC_AUTH, checked)
|
||||||
enabled = true
|
enabled = true
|
||||||
}
|
}
|
||||||
|
is Result.Retry -> {}
|
||||||
else -> {
|
else -> {
|
||||||
// If any error occurs, revert back to the previous
|
// If any error occurs, revert back to the previous
|
||||||
// state. This
|
// state. This
|
||||||
|
|
|
@ -22,6 +22,7 @@ import dev.msfjarvis.aps.R
|
||||||
import dev.msfjarvis.aps.databinding.ActivitySshKeygenBinding
|
import dev.msfjarvis.aps.databinding.ActivitySshKeygenBinding
|
||||||
import dev.msfjarvis.aps.injection.prefs.GitPreferences
|
import dev.msfjarvis.aps.injection.prefs.GitPreferences
|
||||||
import dev.msfjarvis.aps.util.auth.BiometricAuthenticator
|
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.keyguardManager
|
||||||
import dev.msfjarvis.aps.util.extensions.viewBinding
|
import dev.msfjarvis.aps.util.extensions.viewBinding
|
||||||
import dev.msfjarvis.aps.util.git.sshj.SshKey
|
import dev.msfjarvis.aps.util.git.sshj.SshKey
|
||||||
|
@ -121,17 +122,17 @@ class SshKeyGenActivity : AppCompatActivity() {
|
||||||
if (requireAuthentication) {
|
if (requireAuthentication) {
|
||||||
val result =
|
val result =
|
||||||
withContext(Dispatchers.Main) {
|
withContext(Dispatchers.Main) {
|
||||||
suspendCoroutine<BiometricAuthenticator.Result> { cont ->
|
suspendCoroutine<Result> { cont ->
|
||||||
BiometricAuthenticator.authenticate(
|
BiometricAuthenticator.authenticate(
|
||||||
this@SshKeyGenActivity,
|
this@SshKeyGenActivity,
|
||||||
R.string.biometric_prompt_title_ssh_keygen
|
R.string.biometric_prompt_title_ssh_keygen
|
||||||
) {
|
) { result ->
|
||||||
// Do not cancel on failed attempts as these are handled by the authenticator UI.
|
// 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))
|
throw UserNotAuthenticatedException(getString(R.string.biometric_auth_generic_failure))
|
||||||
}
|
}
|
||||||
keyGenType.generateKey(requireAuthentication)
|
keyGenType.generateKey(requireAuthentication)
|
||||||
|
|
|
@ -21,10 +21,28 @@ object BiometricAuthenticator {
|
||||||
private const val validAuthenticators =
|
private const val validAuthenticators =
|
||||||
Authenticators.DEVICE_CREDENTIAL or Authenticators.BIOMETRIC_WEAK
|
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 {
|
sealed class Result {
|
||||||
|
|
||||||
|
/** Biometric authentication was a success. */
|
||||||
data class Success(val cryptoObject: BiometricPrompt.CryptoObject?) : Result()
|
data class Success(val cryptoObject: BiometricPrompt.CryptoObject?) : Result()
|
||||||
|
|
||||||
|
/** Biometric authentication has irreversibly failed. */
|
||||||
data class Failure(val code: Int?, val message: CharSequence) : Result()
|
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()
|
object HardwareUnavailableOrDisabled : Result()
|
||||||
|
|
||||||
|
/** The prompt was dismissed. */
|
||||||
object Cancelled : Result()
|
object Cancelled : Result()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -56,18 +74,35 @@ object BiometricAuthenticator {
|
||||||
BiometricPrompt.ERROR_NO_DEVICE_CREDENTIAL -> {
|
BiometricPrompt.ERROR_NO_DEVICE_CREDENTIAL -> {
|
||||||
Result.HardwareUnavailableOrDisabled
|
Result.HardwareUnavailableOrDisabled
|
||||||
}
|
}
|
||||||
else ->
|
BiometricPrompt.ERROR_LOCKOUT,
|
||||||
|
BiometricPrompt.ERROR_LOCKOUT_PERMANENT,
|
||||||
|
BiometricPrompt.ERROR_NO_SPACE,
|
||||||
|
BiometricPrompt.ERROR_TIMEOUT,
|
||||||
|
BiometricPrompt.ERROR_VENDOR -> {
|
||||||
Result.Failure(
|
Result.Failure(
|
||||||
errorCode,
|
errorCode,
|
||||||
activity.getString(R.string.biometric_auth_error_reason, errString)
|
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.
|
||||||
|
else -> {
|
||||||
|
Result.Failure(
|
||||||
|
errorCode,
|
||||||
|
activity.getString(R.string.biometric_auth_error_reason, errString)
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onAuthenticationFailed() {
|
override fun onAuthenticationFailed() {
|
||||||
super.onAuthenticationFailed()
|
super.onAuthenticationFailed()
|
||||||
callback(Result.Failure(null, activity.getString(R.string.biometric_auth_error)))
|
callback(Result.Retry)
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
|
override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
|
||||||
|
|
|
@ -22,6 +22,7 @@ import dev.msfjarvis.aps.data.repo.PasswordRepository
|
||||||
import dev.msfjarvis.aps.ui.sshkeygen.SshKeyGenActivity
|
import dev.msfjarvis.aps.ui.sshkeygen.SshKeyGenActivity
|
||||||
import dev.msfjarvis.aps.ui.sshkeygen.SshKeyImportActivity
|
import dev.msfjarvis.aps.ui.sshkeygen.SshKeyImportActivity
|
||||||
import dev.msfjarvis.aps.util.auth.BiometricAuthenticator
|
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.GitCommandExecutor
|
||||||
import dev.msfjarvis.aps.util.git.sshj.ContinuationContainerActivity
|
import dev.msfjarvis.aps.util.git.sshj.ContinuationContainerActivity
|
||||||
import dev.msfjarvis.aps.util.git.sshj.SshAuthMethod
|
import dev.msfjarvis.aps.util.git.sshj.SshAuthMethod
|
||||||
|
@ -172,17 +173,17 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
|
||||||
BiometricAuthenticator.authenticate(
|
BiometricAuthenticator.authenticate(
|
||||||
callingActivity,
|
callingActivity,
|
||||||
R.string.biometric_prompt_title_ssh_auth
|
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) {
|
when (result) {
|
||||||
is BiometricAuthenticator.Result.Success -> {
|
is Success -> {
|
||||||
registerAuthProviders(SshAuthMethod.SshKey(authActivity))
|
registerAuthProviders(SshAuthMethod.SshKey(authActivity))
|
||||||
}
|
}
|
||||||
is BiometricAuthenticator.Result.Cancelled -> {
|
is Cancelled -> {
|
||||||
return Err(SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER))
|
return Err(SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER))
|
||||||
}
|
}
|
||||||
is BiometricAuthenticator.Result.Failure -> {
|
is Failure -> {
|
||||||
throw IllegalStateException("Biometric authentication failures should be ignored")
|
throw IllegalStateException("Biometric authentication failures should be ignored")
|
||||||
}
|
}
|
||||||
else -> {
|
else -> {
|
||||||
|
|
Loading…
Reference in a new issue