From a8fd2db3b5a50744646c5601bff2da25555fde61 Mon Sep 17 00:00:00 2001 From: Qinmei Du Date: Fri, 2 Dec 2022 01:22:41 +0000 Subject: [PATCH] Fix some bugs for the create flow 1.When create for the first time and credential type is not passkey(before even when the createOption size is only 1, we still wrongly ask users to select a createOption) 2.When createOption is only one, the activeProvider should not just be the first one but should be the one has this only createOption 3.Only when requestInfo is firstUse and type is passkey, we show passkey intro page Test: deployed locally Bug: 253157231 Change-Id: Ie6ff0582bdf9224a9f714602a356040e16342ee0 --- .../CredentialManagerRepo.kt | 52 ++++------------ .../credentialmanager/DataConverter.kt | 59 ++++++++++++++++++- .../createflow/CreateCredentialViewModel.kt | 35 ++++++----- .../createflow/CreateModel.kt | 1 + 4 files changed, 89 insertions(+), 58 deletions(-) diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt index 1db1d1c1e375c..2780c3cdca797 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt @@ -40,12 +40,9 @@ import android.os.Binder import android.os.Bundle import android.os.ResultReceiver import android.service.credentials.CredentialProviderService -import com.android.credentialmanager.createflow.ActiveEntry import com.android.credentialmanager.createflow.CreateCredentialUiState -import com.android.credentialmanager.createflow.CreateScreenState import com.android.credentialmanager.createflow.EnabledProviderInfo import com.android.credentialmanager.createflow.RemoteInfo -import com.android.credentialmanager.createflow.RequestDisplayInfo import com.android.credentialmanager.getflow.GetCredentialUiState import com.android.credentialmanager.getflow.GetScreenState import com.android.credentialmanager.jetpack.developer.CreatePasswordRequest.Companion.toBundle @@ -146,20 +143,30 @@ class CredentialManagerRepo( providerDisabledList, context) var defaultProvider: EnabledProviderInfo? = null var remoteEntry: RemoteInfo? = null + var createOptionSize = 0 + var lastSeenProviderWithNonEmptyCreateOptions: EnabledProviderInfo? = null providerEnabledList.forEach{providerInfo -> providerInfo.createOptions = providerInfo.createOptions.sortedWith(compareBy { it.lastUsedTimeMillis }).reversed() if (providerInfo.isDefault) {defaultProvider = providerInfo} if (providerInfo.remoteEntry != null) { remoteEntry = providerInfo.remoteEntry!! } + if (providerInfo.createOptions.isNotEmpty()) { + createOptionSize += providerInfo.createOptions.size + lastSeenProviderWithNonEmptyCreateOptions = providerInfo + } } return CreateCredentialUiState( enabledProviders = providerEnabledList, disabledProviders = providerDisabledList, - toCreateScreenState(requestDisplayInfo, defaultProvider, remoteEntry), + CreateFlowUtils.toCreateScreenState( + createOptionSize, false, + requestDisplayInfo, defaultProvider, remoteEntry), requestDisplayInfo, false, - toActiveEntry(defaultProvider, remoteEntry), + CreateFlowUtils.toActiveEntry( + /*defaultProvider=*/defaultProvider, createOptionSize, + lastSeenProviderWithNonEmptyCreateOptions, remoteEntry), ) } @@ -194,6 +201,7 @@ class CredentialManagerRepo( .setRemoteEntry( newRemoteEntry("key2", "subkey-1") ) + .setIsDefaultProvider(true) .build(), CreateCredentialProviderData .Builder("com.dashlane") @@ -517,38 +525,4 @@ class CredentialManagerRepo( "tribank.us" ) } - - private fun toCreateScreenState( - requestDisplayInfo: RequestDisplayInfo, - defaultProvider: EnabledProviderInfo?, - remoteEntry: RemoteInfo?, - ): CreateScreenState { - return if ( - defaultProvider != null && defaultProvider.createOptions.isEmpty() && remoteEntry != null - ){ - CreateScreenState.EXTERNAL_ONLY_SELECTION - } else if (defaultProvider == null || defaultProvider.createOptions.isEmpty()) { - if (requestDisplayInfo.type == TYPE_PUBLIC_KEY_CREDENTIAL) { - CreateScreenState.PASSKEY_INTRO - } else { - CreateScreenState.PROVIDER_SELECTION - } - } else { - CreateScreenState.CREATION_OPTION_SELECTION - } - } - - private fun toActiveEntry( - defaultProvider: EnabledProviderInfo?, - remoteEntry: RemoteInfo?, - ): ActiveEntry? { - return if ( - defaultProvider != null && defaultProvider.createOptions.isNotEmpty() - ) { - ActiveEntry(defaultProvider, defaultProvider.createOptions.first()) - } else if ( - defaultProvider != null && defaultProvider.createOptions.isEmpty() && remoteEntry != null) { - ActiveEntry(defaultProvider, remoteEntry) - } else null - } } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt index 357c55dc27701..0d7e81984eadb 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt @@ -28,6 +28,9 @@ import android.graphics.drawable.Drawable import com.android.credentialmanager.createflow.CreateOptionInfo import com.android.credentialmanager.createflow.RemoteInfo import com.android.credentialmanager.createflow.RequestDisplayInfo +import com.android.credentialmanager.createflow.EnabledProviderInfo +import com.android.credentialmanager.createflow.CreateScreenState +import com.android.credentialmanager.createflow.ActiveEntry import com.android.credentialmanager.getflow.ActionEntryInfo import com.android.credentialmanager.getflow.AuthenticationEntryInfo import com.android.credentialmanager.getflow.CredentialEntryInfo @@ -36,6 +39,7 @@ import com.android.credentialmanager.getflow.RemoteEntryInfo import com.android.credentialmanager.jetpack.developer.CreateCredentialRequest import com.android.credentialmanager.jetpack.developer.CreatePasswordRequest import com.android.credentialmanager.jetpack.developer.CreatePublicKeyCredentialRequest +import com.android.credentialmanager.jetpack.developer.PublicKeyCredential.Companion.TYPE_PUBLIC_KEY_CREDENTIAL import com.android.credentialmanager.jetpack.provider.ActionUi import com.android.credentialmanager.jetpack.provider.CredentialEntryUi import com.android.credentialmanager.jetpack.provider.SaveEntryUi @@ -243,7 +247,8 @@ class CreateFlowUtils { createCredentialRequestJetpack.password, createCredentialRequestJetpack.type, requestInfo.appPackageName, - context.getDrawable(R.drawable.ic_password)!! + context.getDrawable(R.drawable.ic_password)!!, + requestInfo.isFirstUsage ) } is CreatePublicKeyCredentialRequest -> { @@ -261,7 +266,8 @@ class CreateFlowUtils { displayName, createCredentialRequestJetpack.type, requestInfo.appPackageName, - context.getDrawable(R.drawable.ic_passkey)!!) + context.getDrawable(R.drawable.ic_passkey)!!, + requestInfo.isFirstUsage) } // TODO: correctly parsing for other sign-ins else -> { @@ -270,11 +276,58 @@ class CreateFlowUtils { "Elisa Beckett", "other-sign-ins", requestInfo.appPackageName, - context.getDrawable(R.drawable.ic_other_sign_in)!!) + context.getDrawable(R.drawable.ic_other_sign_in)!!, + requestInfo.isFirstUsage) } } } + fun toCreateScreenState( + createOptionSize: Int, + isOnPasskeyIntroStateAlready: Boolean, + requestDisplayInfo: RequestDisplayInfo, + defaultProvider: EnabledProviderInfo?, + remoteEntry: RemoteInfo?, + ): CreateScreenState { + return if (requestDisplayInfo.isFirstUsage && requestDisplayInfo + .type == TYPE_PUBLIC_KEY_CREDENTIAL && !isOnPasskeyIntroStateAlready) { + CreateScreenState.PASSKEY_INTRO + } else if ( + (defaultProvider == null || defaultProvider.createOptions.isEmpty() + ) && createOptionSize > 1) { + CreateScreenState.PROVIDER_SELECTION + } else if ( + ((defaultProvider == null || defaultProvider.createOptions.isEmpty() + ) && createOptionSize == 1) || ( + defaultProvider != null && defaultProvider.createOptions.isNotEmpty())) { + CreateScreenState.CREATION_OPTION_SELECTION + } else if (createOptionSize == 0 && remoteEntry != null) { + CreateScreenState.EXTERNAL_ONLY_SELECTION + } else { + // TODO: properly handle error and gracefully finish itself + throw java.lang.IllegalStateException("Empty provider list.") + } + } + + fun toActiveEntry( + defaultProvider: EnabledProviderInfo?, + createOptionSize: Int, + lastSeenProviderWithNonEmptyCreateOptions: EnabledProviderInfo?, + remoteEntry: RemoteInfo?, + ): ActiveEntry? { + return if ( + defaultProvider != null && defaultProvider.createOptions.isEmpty() && remoteEntry != null) { + ActiveEntry(defaultProvider, remoteEntry) + } else if ( + defaultProvider != null && defaultProvider.createOptions.isNotEmpty() + ) { + ActiveEntry(defaultProvider, defaultProvider.createOptions.first()) + } else if (createOptionSize == 1) { + ActiveEntry(lastSeenProviderWithNonEmptyCreateOptions!!, + lastSeenProviderWithNonEmptyCreateOptions.createOptions.first()) + } else null + } + private fun toCreationOptionInfoList( providerId: String, creationEntries: List, diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt index f81f08babdac0..bdc71cf8434c3 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt @@ -27,6 +27,7 @@ import androidx.compose.runtime.setValue import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel +import com.android.credentialmanager.CreateFlowUtils import com.android.credentialmanager.CredentialManagerRepo import com.android.credentialmanager.common.DialogResult import com.android.credentialmanager.common.ProviderActivityResult @@ -60,24 +61,26 @@ class CreateCredentialViewModel( fun onConfirmIntro() { var createOptionSize = 0 + var lastSeenProviderWithNonEmptyCreateOptions: EnabledProviderInfo? = null + var remoteEntry: RemoteInfo? = null uiState.enabledProviders.forEach { - enabledProvider -> createOptionSize += enabledProvider.createOptions.size} - uiState = if (createOptionSize > 1) { - uiState.copy( - currentScreenState = CreateScreenState.PROVIDER_SELECTION, - showActiveEntryOnly = true - ) - } else if (createOptionSize == 1){ - uiState.copy( - currentScreenState = CreateScreenState.CREATION_OPTION_SELECTION, - showActiveEntryOnly = false, - activeEntry = ActiveEntry(uiState.enabledProviders.first(), - uiState.enabledProviders.first().createOptions.first() - ) - ) - } else { - throw java.lang.IllegalStateException("Empty provider list.") + enabledProvider -> + if (enabledProvider.createOptions.isNotEmpty()) { + createOptionSize += enabledProvider.createOptions.size + lastSeenProviderWithNonEmptyCreateOptions = enabledProvider + } + if (enabledProvider.remoteEntry != null) { + remoteEntry = enabledProvider.remoteEntry!! + } } + uiState = uiState.copy( + currentScreenState = CreateFlowUtils.toCreateScreenState( + createOptionSize, true, + uiState.requestDisplayInfo, null, remoteEntry), + showActiveEntryOnly = createOptionSize > 1, + activeEntry = CreateFlowUtils.toActiveEntry( + null, createOptionSize, lastSeenProviderWithNonEmptyCreateOptions, remoteEntry), + ) } fun getProviderInfoByName(providerName: String): EnabledProviderInfo { diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt index 9ac524a4e6d98..21abe08c04f27 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt @@ -77,6 +77,7 @@ data class RequestDisplayInfo( val type: String, val appDomainName: String, val typeIcon: Drawable, + val isFirstUsage: Boolean, ) /**