mirror of
https://github.com/bitwarden/android.git
synced 2025-02-23 00:59:16 +03:00
[PM-10678] (WIP) Refactor passkey part 2 - Improve passkey autofill for multiple accounts
This commit refactors the passkey autofill process to support multiple accounts and improve the overall user experience. The key changes include: - Allowing users to unlock and use passkeys stored in other accounts. - Implementing a more comprehensive approach to display options for authentication and unlocking accounts during passkey selection. - Handling cancellation scenarios more robustly to prevent errors. - Fixing an issue that may lead to failures in certain cases. - Updating error handling to provide clearer messages to users.
This commit is contained in:
parent
217efae259
commit
8fbb4b7af4
7 changed files with 107 additions and 64 deletions
app/src
main
java/com/x8bit/bitwarden
data/autofill/fido2
ui
autofill/fido2/manager
vault/feature/itemlisting
res/values
test/java/com/x8bit/bitwarden/data/autofill/fido2/processor
|
@ -49,8 +49,6 @@ object Fido2ProviderModule {
|
|||
Fido2ProviderProcessorImpl(
|
||||
context,
|
||||
authRepository,
|
||||
vaultRepository,
|
||||
fido2CredentialStore,
|
||||
fido2CredentialManager,
|
||||
intentManager,
|
||||
clock,
|
||||
|
|
|
@ -27,4 +27,9 @@ sealed class Fido2GetCredentialsResult {
|
|||
* Indicates an error was encountered when querying for matching credentials.
|
||||
*/
|
||||
data object Error : Fido2GetCredentialsResult()
|
||||
|
||||
/**
|
||||
* Indicates the user has cancelled credential discovery.
|
||||
*/
|
||||
data object Cancelled : Fido2GetCredentialsResult()
|
||||
}
|
||||
|
|
|
@ -24,7 +24,6 @@ import androidx.credentials.provider.BeginGetPublicKeyCredentialOption
|
|||
import androidx.credentials.provider.CreateEntry
|
||||
import androidx.credentials.provider.CredentialEntry
|
||||
import androidx.credentials.provider.ProviderClearCredentialStateRequest
|
||||
import com.bitwarden.sdk.Fido2CredentialStore
|
||||
import com.x8bit.bitwarden.R
|
||||
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserState
|
||||
|
@ -32,7 +31,6 @@ import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
|
|||
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsRequest
|
||||
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsResult
|
||||
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
|
||||
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
|
||||
import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.launch
|
||||
|
@ -47,13 +45,11 @@ const val UNLOCK_ACCOUNT_INTENT = "com.x8bit.bitwarden.fido2.ACTION_UNLOCK_ACCOU
|
|||
* The default implementation of [Fido2ProviderProcessor]. Its purpose is to handle FIDO2 related
|
||||
* processing.
|
||||
*/
|
||||
@Suppress("LongParameterList")
|
||||
@Suppress("LongParameterList", "TooManyFunctions")
|
||||
@RequiresApi(Build.VERSION_CODES.S)
|
||||
class Fido2ProviderProcessorImpl(
|
||||
private val context: Context,
|
||||
private val authRepository: AuthRepository,
|
||||
private val vaultRepository: VaultRepository,
|
||||
private val fido2CredentialStore: Fido2CredentialStore,
|
||||
private val fido2CredentialManager: Fido2CredentialManager,
|
||||
private val intentManager: IntentManager,
|
||||
private val clock: Clock,
|
||||
|
@ -153,35 +149,27 @@ class Fido2ProviderProcessorImpl(
|
|||
return
|
||||
}
|
||||
|
||||
// Return an unlock action if the current account is locked.
|
||||
if (!userState.activeAccount.isVaultUnlocked) {
|
||||
val authenticationAction = AuthenticationAction(
|
||||
title = context.getString(R.string.unlock),
|
||||
pendingIntent = intentManager.createFido2UnlockPendingIntent(
|
||||
action = UNLOCK_ACCOUNT_INTENT,
|
||||
userId = userState.activeUserId,
|
||||
requestCode = requestCode.getAndIncrement(),
|
||||
),
|
||||
)
|
||||
|
||||
callback.onResult(
|
||||
BeginGetCredentialResponse(
|
||||
authenticationActions = listOf(authenticationAction),
|
||||
),
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
// Otherwise, find all matching credentials from the current vault.
|
||||
val getCredentialJob = scope.launch {
|
||||
try {
|
||||
val credentialEntries = getMatchingFido2CredentialEntries(
|
||||
userId = userState.activeUserId,
|
||||
request = request,
|
||||
)
|
||||
val authenticationActions = userState.accounts
|
||||
.toAuthenticationActions()
|
||||
|
||||
val switchAccountActions = userState.accounts
|
||||
.toSwitchAccountActions(userState.activeUserId)
|
||||
|
||||
val credentialEntries =
|
||||
if (userState.activeAccount.isVaultUnlocked) {
|
||||
getMatchingFido2CredentialEntries(
|
||||
userId = userState.activeUserId,
|
||||
request = request,
|
||||
)
|
||||
} else {
|
||||
emptyList()
|
||||
}
|
||||
|
||||
callback.onResult(
|
||||
BeginGetCredentialResponse(
|
||||
authenticationActions = authenticationActions + switchAccountActions,
|
||||
credentialEntries = credentialEntries,
|
||||
),
|
||||
)
|
||||
|
@ -190,45 +178,86 @@ class Fido2ProviderProcessorImpl(
|
|||
}
|
||||
}
|
||||
cancellationSignal.setOnCancelListener {
|
||||
if (getCredentialJob.isActive) {
|
||||
getCredentialJob.cancel()
|
||||
}
|
||||
callback.onError(GetCredentialCancellationException())
|
||||
getCredentialJob.cancel()
|
||||
}
|
||||
}
|
||||
|
||||
private fun List<UserState.Account>.toAuthenticationActions(): List<AuthenticationAction> =
|
||||
this.filterNot { it.isVaultUnlocked }
|
||||
.map { it.toAuthenticationAction() }
|
||||
|
||||
private fun UserState.Account.toAuthenticationAction(): AuthenticationAction =
|
||||
AuthenticationAction(
|
||||
title = context.getString(R.string.unlock_vault_for_x, name ?: email),
|
||||
pendingIntent = intentManager.createFido2UnlockPendingIntent(
|
||||
action = UNLOCK_ACCOUNT_INTENT,
|
||||
userId = userId,
|
||||
requestCode = requestCode.getAndIncrement(),
|
||||
),
|
||||
)
|
||||
|
||||
private fun List<UserState.Account>.toSwitchAccountActions(
|
||||
activeUserId: String,
|
||||
): List<AuthenticationAction> = this
|
||||
.filter { it.isVaultUnlocked && it.userId != activeUserId }
|
||||
.map { it.toSwitchAccountAction() }
|
||||
|
||||
private fun UserState.Account.toSwitchAccountAction(): AuthenticationAction =
|
||||
AuthenticationAction
|
||||
.Builder(
|
||||
title = context.getString(R.string.check_x_for_passkeys, name ?: email),
|
||||
pendingIntent = intentManager.createFido2UnlockPendingIntent(
|
||||
action = UNLOCK_ACCOUNT_INTENT,
|
||||
userId = userId,
|
||||
requestCode = requestCode.getAndIncrement(),
|
||||
),
|
||||
)
|
||||
.build()
|
||||
|
||||
@Suppress("ThrowsCount")
|
||||
@Throws(GetCredentialUnsupportedException::class)
|
||||
private suspend fun getMatchingFido2CredentialEntries(
|
||||
userId: String,
|
||||
request: BeginGetCredentialRequest,
|
||||
): List<CredentialEntry> {
|
||||
val callingAppInfo = request.callingAppInfo
|
||||
?: throw GetCredentialUnknownException()
|
||||
val option = request.beginGetCredentialOptions
|
||||
.firstNotNullOfOrNull { it as? BeginGetPublicKeyCredentialOption }
|
||||
?: throw GetCredentialUnknownException()
|
||||
val callingAppInfo = requireNotNull(request.callingAppInfo)
|
||||
return request
|
||||
.beginGetCredentialOptions
|
||||
.flatMap { option ->
|
||||
if (option is BeginGetPublicKeyCredentialOption) {
|
||||
val result = fido2CredentialManager
|
||||
.getFido2CredentialsForRelyingParty(
|
||||
Fido2GetCredentialsRequest(
|
||||
candidateQueryData = option.candidateQueryData,
|
||||
id = option.id,
|
||||
userId = userId,
|
||||
requestJson = option.requestJson,
|
||||
clientDataHash = option.clientDataHash,
|
||||
packageName = callingAppInfo.packageName,
|
||||
signingInfo = callingAppInfo.signingInfo,
|
||||
origin = request.callingAppInfo?.origin,
|
||||
),
|
||||
)
|
||||
when (result) {
|
||||
Fido2GetCredentialsResult.Error -> {
|
||||
throw GetCredentialUnknownException("Error retrieving credentials.")
|
||||
}
|
||||
|
||||
val getCredentialsResult = fido2CredentialManager
|
||||
.getFido2CredentialsForRelyingParty(
|
||||
fido2GetCredentialsRequest = Fido2GetCredentialsRequest(
|
||||
candidateQueryData = option.candidateQueryData,
|
||||
id = option.id,
|
||||
userId = userId,
|
||||
requestJson = option.requestJson,
|
||||
clientDataHash = option.clientDataHash,
|
||||
packageName = callingAppInfo.packageName,
|
||||
signingInfo = callingAppInfo.signingInfo,
|
||||
origin = callingAppInfo.origin,
|
||||
),
|
||||
)
|
||||
return when (getCredentialsResult) {
|
||||
is Fido2GetCredentialsResult.Error -> {
|
||||
throw GetCredentialUnknownException()
|
||||
}
|
||||
is Fido2GetCredentialsResult.Success -> {
|
||||
result.credentialEntries
|
||||
}
|
||||
|
||||
is Fido2GetCredentialsResult.Success -> {
|
||||
getCredentialsResult.credentialEntries
|
||||
Fido2GetCredentialsResult.Cancelled -> {
|
||||
throw GetCredentialCancellationException()
|
||||
}
|
||||
}
|
||||
} else {
|
||||
throw GetCredentialUnsupportedException("Unsupported option.")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override fun processClearCredentialStateRequest(
|
||||
|
|
|
@ -9,6 +9,7 @@ import androidx.credentials.GetCredentialResponse
|
|||
import androidx.credentials.PublicKeyCredential
|
||||
import androidx.credentials.exceptions.CreateCredentialCancellationException
|
||||
import androidx.credentials.exceptions.CreateCredentialUnknownException
|
||||
import androidx.credentials.exceptions.GetCredentialCancellationException
|
||||
import androidx.credentials.exceptions.GetCredentialUnknownException
|
||||
import androidx.credentials.provider.BeginGetCredentialResponse
|
||||
import androidx.credentials.provider.PendingIntentHandler
|
||||
|
@ -101,13 +102,19 @@ class Fido2CompletionManagerImpl(
|
|||
)
|
||||
}
|
||||
|
||||
Fido2GetCredentialsResult.Error,
|
||||
-> {
|
||||
Fido2GetCredentialsResult.Error -> {
|
||||
PendingIntentHandler.setGetCredentialException(
|
||||
resultIntent,
|
||||
GetCredentialUnknownException(),
|
||||
)
|
||||
}
|
||||
|
||||
Fido2GetCredentialsResult.Cancelled -> {
|
||||
PendingIntentHandler.setGetCredentialException(
|
||||
resultIntent,
|
||||
GetCredentialCancellationException(),
|
||||
)
|
||||
}
|
||||
}
|
||||
activity.setResult(Activity.RESULT_OK, resultIntent)
|
||||
activity.finish()
|
||||
|
|
|
@ -1238,7 +1238,9 @@ class VaultItemListingViewModel @Inject constructor(
|
|||
|
||||
private fun vaultLoadedReceive(vaultData: DataState.Loaded<VaultData>) {
|
||||
updateStateWithVaultData(vaultData = vaultData.data, clearDialogState = true)
|
||||
state.fido2GetCredentialsRequest
|
||||
state
|
||||
// TODO: Move FIDO 2 get credentials to vault unlock screen
|
||||
.fido2GetCredentialsRequest
|
||||
?.let { fido2GetCredentialsRequest ->
|
||||
viewModelScope.launch {
|
||||
sendEvent(
|
||||
|
@ -1250,8 +1252,9 @@ class VaultItemListingViewModel @Inject constructor(
|
|||
)
|
||||
}
|
||||
}
|
||||
// TODO: Move FIDO 2 authentication to vault unlock screen
|
||||
?: state.fido2CredentialAssertionRequest
|
||||
?: state
|
||||
// TODO: Move FIDO 2 authentication to vault unlock screen
|
||||
.fido2CredentialAssertionRequest
|
||||
?.let { fido2AssertionRequest ->
|
||||
trySendAction(
|
||||
VaultItemListingsAction.Internal.Fido2AssertionDataReceive(
|
||||
|
|
|
@ -1087,4 +1087,7 @@ Do you want to switch to this account?</string>
|
|||
<string name="bitwarden_can_notify_you_each_time_you_receive_a_new_login_request_from_another_device">Bitwarden can notify you each time you receive a new login request from another device.</string>
|
||||
<string name="skip_for_now">Skip for now</string>
|
||||
<string name="done_text">Done</string>
|
||||
<string name="passkey_operation_failed_because_passkey_does_not_exist">Passkey operation failed because passkey does not exist.</string>
|
||||
<string name="unlock_vault_for_x">Unlock vault for %s</string>
|
||||
<string name="check_x_for_passkeys">Check %s for passkeys</string>
|
||||
</resources>
|
||||
|
|
|
@ -90,8 +90,6 @@ class Fido2ProviderProcessorTest {
|
|||
fido2Processor = Fido2ProviderProcessorImpl(
|
||||
context,
|
||||
authRepository,
|
||||
vaultRepository,
|
||||
fido2CredentialStore,
|
||||
fido2CredentialManager,
|
||||
intentManager,
|
||||
clock,
|
||||
|
|
Loading…
Add table
Reference in a new issue