From 0c0d9621cb15dbe2b4725bf7797f0514abfe2713 Mon Sep 17 00:00:00 2001 From: Helen Qin Date: Wed, 8 Feb 2023 01:34:09 +0000 Subject: [PATCH] Allow UI update upon auth/action entry changes. Also tested that multiple auth entries work. Will submit a follow up for updating the titles. Bug: 267669563 Test: manual (see demo in comment) Change-Id: I1cabc35f8e94d81b6ac0d04320dd8ce378f42ece --- .../CredentialSelectorActivity.kt | 26 +- .../CredentialSelectorViewModel.kt | 4 +- .../credentialmanager/common/BaseEntry.kt | 1 + .../createflow/CreateModel.kt | 18 +- .../credentialmanager/getflow/GetModel.kt | 333 ++++++++++-------- 5 files changed, 216 insertions(+), 166 deletions(-) diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt index 0802afe9a2d28..5136f04c3e120 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorActivity.kt @@ -27,6 +27,7 @@ import androidx.activity.ComponentActivity import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.compose.setContent import androidx.activity.result.contract.ActivityResultContracts +import androidx.activity.viewModels import androidx.compose.material.ExperimentalMaterialApi import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -43,17 +44,6 @@ class CredentialSelectorActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) Log.d(Constants.LOG_TAG, "Creating new CredentialSelectorActivity") - init(intent) - } - - override fun onNewIntent(intent: Intent) { - super.onNewIntent(intent) - setIntent(intent) - Log.d(Constants.LOG_TAG, "Existing activity received new intent") - init(intent) - } - - fun init(intent: Intent) { try { val userConfigRepo = UserConfigRepo(this) val credManRepo = CredentialManagerRepo(this, intent, userConfigRepo) @@ -70,6 +60,20 @@ class CredentialSelectorActivity : ComponentActivity() { } } + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + setIntent(intent) + Log.d(Constants.LOG_TAG, "Existing activity received new intent") + try { + val userConfigRepo = UserConfigRepo(this) + val credManRepo = CredentialManagerRepo(this, intent, userConfigRepo) + val viewModel: CredentialSelectorViewModel by viewModels() + viewModel.onNewCredentialManagerRepo(credManRepo) + } catch (e: Exception) { + onInitializationError(e, intent) + } + } + @ExperimentalMaterialApi @Composable fun CredentialManagerBottomSheet( diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorViewModel.kt index 47ea53b00a39c..6bf1513631cec 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialSelectorViewModel.kt @@ -104,7 +104,9 @@ class CredentialSelectorViewModel( entry.providerId, entry.entryKey, entry.entrySubkey, resultCode, resultData, ) - uiState = uiState.copy(dialogState = DialogState.COMPLETE) + if (entry.shouldTerminateUiUponSuccessfulProviderResult) { + uiState = uiState.copy(dialogState = DialogState.COMPLETE) + } } else { Log.w(Constants.LOG_TAG, "Illegal state: received a provider result but found no matching entry.") diff --git a/packages/CredentialManager/src/com/android/credentialmanager/common/BaseEntry.kt b/packages/CredentialManager/src/com/android/credentialmanager/common/BaseEntry.kt index 4b8bc97d96595..ee36989b3da0f 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/common/BaseEntry.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/common/BaseEntry.kt @@ -25,4 +25,5 @@ open class BaseEntry ( val entrySubkey: String, val pendingIntent: PendingIntent?, val fillInIntent: Intent?, + val shouldTerminateUiUponSuccessfulProviderResult: Boolean, ) \ No newline at end of file diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt index 919411ef10720..8d20564fe2a80 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt @@ -69,7 +69,14 @@ class CreateOptionInfo( val totalCredentialCount: Int?, val lastUsedTime: Instant?, val footerDescription: String?, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) +) : BaseEntry( + providerId, + entryKey, + entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = true, +) class RemoteInfo( providerId: String, @@ -77,7 +84,14 @@ class RemoteInfo( entrySubkey: String, pendingIntent: PendingIntent?, fillInIntent: Intent?, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) +) : BaseEntry( + providerId, + entryKey, + entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = true, +) data class RequestDisplayInfo( val title: String, diff --git a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetModel.kt index 5ab933a0f6515..bca06c729073a 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetModel.kt @@ -35,78 +35,105 @@ data class GetCredentialUiState( ) data class ProviderInfo( - /** - * Unique id (component name) of this provider. - * Not for display purpose - [displayName] should be used for ui rendering. - */ - val id: String, - val icon: Drawable, - val displayName: String, - val credentialEntryList: List, - val authenticationEntryList: List, - val remoteEntry: RemoteEntryInfo?, - val actionEntryList: List, + /** + * Unique id (component name) of this provider. + * Not for display purpose - [displayName] should be used for ui rendering. + */ + val id: String, + val icon: Drawable, + val displayName: String, + val credentialEntryList: List, + val authenticationEntryList: List, + val remoteEntry: RemoteEntryInfo?, + val actionEntryList: List, ) /** Display-centric data structure derived from the [ProviderInfo]. This abstraction is not grouping * by the provider id but instead focuses on structures convenient for display purposes. */ data class ProviderDisplayInfo( - /** - * The credential entries grouped by userName, derived from all entries of the [providerInfoList]. - * Note that the list order matters to the display order. - */ - val sortedUserNameToCredentialEntryList: List, - val authenticationEntryList: List, - val remoteEntry: RemoteEntryInfo? + /** + * The credential entries grouped by userName, derived from all entries of the [providerInfoList]. + * Note that the list order matters to the display order. + */ + val sortedUserNameToCredentialEntryList: List, + val authenticationEntryList: List, + val remoteEntry: RemoteEntryInfo? ) class CredentialEntryInfo( - providerId: String, - entryKey: String, - entrySubkey: String, - pendingIntent: PendingIntent?, - fillInIntent: Intent?, - /** Type of this credential used for sorting. Not localized so must not be directly displayed. */ - val credentialType: CredentialType, - /** Localized type value of this credential used for display purpose. */ - val credentialTypeDisplayName: String, - val userName: String, - val displayName: String?, - val icon: Drawable?, - val lastUsedTimeMillis: Instant?, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) + providerId: String, + entryKey: String, + entrySubkey: String, + pendingIntent: PendingIntent?, + fillInIntent: Intent?, + /** Type of this credential used for sorting. Not localized so must not be directly displayed. */ + val credentialType: CredentialType, + /** Localized type value of this credential used for display purpose. */ + val credentialTypeDisplayName: String, + val userName: String, + val displayName: String?, + val icon: Drawable?, + val lastUsedTimeMillis: Instant?, +) : BaseEntry( + providerId, + entryKey, + entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = true, +) class AuthenticationEntryInfo( - providerId: String, - entryKey: String, - entrySubkey: String, - pendingIntent: PendingIntent?, - fillInIntent: Intent?, - val title: String, - val icon: Drawable, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) + providerId: String, + entryKey: String, + entrySubkey: String, + pendingIntent: PendingIntent?, + fillInIntent: Intent?, + val title: String, + val icon: Drawable, +) : BaseEntry( + providerId, + entryKey, entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = false, +) class RemoteEntryInfo( - providerId: String, - entryKey: String, - entrySubkey: String, - pendingIntent: PendingIntent?, - fillInIntent: Intent?, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) + providerId: String, + entryKey: String, + entrySubkey: String, + pendingIntent: PendingIntent?, + fillInIntent: Intent?, +) : BaseEntry( + providerId, + entryKey, + entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = true, +) class ActionEntryInfo( - providerId: String, - entryKey: String, - entrySubkey: String, - pendingIntent: PendingIntent?, - fillInIntent: Intent?, - val title: String, - val icon: Drawable, - val subTitle: String?, -) : BaseEntry(providerId, entryKey, entrySubkey, pendingIntent, fillInIntent) + providerId: String, + entryKey: String, + entrySubkey: String, + pendingIntent: PendingIntent?, + fillInIntent: Intent?, + val title: String, + val icon: Drawable, + val subTitle: String?, +) : BaseEntry( + providerId, + entryKey, + entrySubkey, + pendingIntent, + fillInIntent, + shouldTerminateUiUponSuccessfulProviderResult = false, +) data class RequestDisplayInfo( - val appName: String, + val appName: String, ) /** @@ -115,18 +142,20 @@ data class RequestDisplayInfo( * by last used timestamps and then by credential types */ data class PerUserNameCredentialEntryList( - val userName: String, - val sortedCredentialEntryList: List, + val userName: String, + val sortedCredentialEntryList: List, ) /** The name of the current screen. */ enum class GetScreenState { - /** The primary credential selection page. */ - PRIMARY_SELECTION, - /** The secondary credential selection page, where all sign-in options are listed. */ - ALL_SIGN_IN_OPTIONS, - /** The snackbar only page when there's no account but only a remoteEntry. */ - REMOTE_ONLY, + /** The primary credential selection page. */ + PRIMARY_SELECTION, + + /** The secondary credential selection page, where all sign-in options are listed. */ + ALL_SIGN_IN_OPTIONS, + + /** The snackbar only page when there's no account but only a remoteEntry. */ + REMOTE_ONLY, } // IMPORTANT: new invocation should be mindful that this method will throw if more than 1 remote @@ -135,113 +164,113 @@ private fun toProviderDisplayInfo( providerInfoList: List ): ProviderDisplayInfo { - val userNameToCredentialEntryMap = mutableMapOf>() - val authenticationEntryList = mutableListOf() - val remoteEntryList = mutableListOf() - providerInfoList.forEach { providerInfo -> - authenticationEntryList.addAll(providerInfo.authenticationEntryList) - if (providerInfo.remoteEntry != null) { - remoteEntryList.add(providerInfo.remoteEntry) - } - // There can only be at most one remote entry - Preconditions.checkState(remoteEntryList.size <= 1) - - providerInfo.credentialEntryList.forEach { - userNameToCredentialEntryMap.compute( - it.userName - ) { _, v -> - if (v == null) { - mutableListOf(it) - } else { - v.add(it) - v + val userNameToCredentialEntryMap = mutableMapOf>() + val authenticationEntryList = mutableListOf() + val remoteEntryList = mutableListOf() + providerInfoList.forEach { providerInfo -> + authenticationEntryList.addAll(providerInfo.authenticationEntryList) + if (providerInfo.remoteEntry != null) { + remoteEntryList.add(providerInfo.remoteEntry) + } + // There can only be at most one remote entry + Preconditions.checkState(remoteEntryList.size <= 1) + + providerInfo.credentialEntryList.forEach { + userNameToCredentialEntryMap.compute( + it.userName + ) { _, v -> + if (v == null) { + mutableListOf(it) + } else { + v.add(it) + v + } + } } - } } - } - // Compose sortedUserNameToCredentialEntryList - val comparator = CredentialEntryInfoComparatorByTypeThenTimestamp() - // Sort per username - userNameToCredentialEntryMap.values.forEach { - it.sortWith(comparator) - } - // Transform to list of PerUserNameCredentialEntryLists and then sort across usernames - val sortedUserNameToCredentialEntryList = userNameToCredentialEntryMap.map { - PerUserNameCredentialEntryList(it.key, it.value) - }.sortedWith( - compareByDescending { it.sortedCredentialEntryList.first().lastUsedTimeMillis } - ) + // Compose sortedUserNameToCredentialEntryList + val comparator = CredentialEntryInfoComparatorByTypeThenTimestamp() + // Sort per username + userNameToCredentialEntryMap.values.forEach { + it.sortWith(comparator) + } + // Transform to list of PerUserNameCredentialEntryLists and then sort across usernames + val sortedUserNameToCredentialEntryList = userNameToCredentialEntryMap.map { + PerUserNameCredentialEntryList(it.key, it.value) + }.sortedWith( + compareByDescending { it.sortedCredentialEntryList.first().lastUsedTimeMillis } + ) - return ProviderDisplayInfo( - sortedUserNameToCredentialEntryList = sortedUserNameToCredentialEntryList, - authenticationEntryList = authenticationEntryList, - remoteEntry = remoteEntryList.getOrNull(0), - ) + return ProviderDisplayInfo( + sortedUserNameToCredentialEntryList = sortedUserNameToCredentialEntryList, + authenticationEntryList = authenticationEntryList, + remoteEntry = remoteEntryList.getOrNull(0), + ) } private fun toActiveEntry( providerDisplayInfo: ProviderDisplayInfo, ): BaseEntry? { - val sortedUserNameToCredentialEntryList = - providerDisplayInfo.sortedUserNameToCredentialEntryList - val authenticationEntryList = providerDisplayInfo.authenticationEntryList - var activeEntry: BaseEntry? = null - if (sortedUserNameToCredentialEntryList - .size == 1 && authenticationEntryList.isEmpty() - ) { - activeEntry = sortedUserNameToCredentialEntryList.first().sortedCredentialEntryList.first() - } else if ( - sortedUserNameToCredentialEntryList - .isEmpty() && authenticationEntryList.size == 1 - ) { - activeEntry = authenticationEntryList.first() - } - return activeEntry + val sortedUserNameToCredentialEntryList = + providerDisplayInfo.sortedUserNameToCredentialEntryList + val authenticationEntryList = providerDisplayInfo.authenticationEntryList + var activeEntry: BaseEntry? = null + if (sortedUserNameToCredentialEntryList + .size == 1 && authenticationEntryList.isEmpty() + ) { + activeEntry = sortedUserNameToCredentialEntryList.first().sortedCredentialEntryList.first() + } else if ( + sortedUserNameToCredentialEntryList + .isEmpty() && authenticationEntryList.size == 1 + ) { + activeEntry = authenticationEntryList.first() + } + return activeEntry } private fun toGetScreenState( providerInfoList: List ): GetScreenState { - var noLocalAccount = true - var remoteInfo: RemoteEntryInfo? = null - providerInfoList.forEach { providerInfo -> - if (providerInfo.credentialEntryList.isNotEmpty() || - providerInfo.authenticationEntryList.isNotEmpty()) { - noLocalAccount = false + var noLocalAccount = true + var remoteInfo: RemoteEntryInfo? = null + providerInfoList.forEach { providerInfo -> + if (providerInfo.credentialEntryList.isNotEmpty() || + providerInfo.authenticationEntryList.isNotEmpty()) { + noLocalAccount = false + } + if (providerInfo.remoteEntry != null) { + remoteInfo = providerInfo.remoteEntry + } } - if (providerInfo.remoteEntry != null) { - remoteInfo = providerInfo.remoteEntry - } - } - return if (noLocalAccount && remoteInfo != null) - GetScreenState.REMOTE_ONLY else GetScreenState.PRIMARY_SELECTION + return if (noLocalAccount && remoteInfo != null) + GetScreenState.REMOTE_ONLY else GetScreenState.PRIMARY_SELECTION } internal class CredentialEntryInfoComparatorByTypeThenTimestamp : Comparator { - override fun compare(p0: CredentialEntryInfo, p1: CredentialEntryInfo): Int { - // First prefer passkey type for its security benefits - if (p0.credentialType != p1.credentialType) { - if (CredentialType.PASSKEY == p0.credentialType) { - return -1 - } else if (CredentialType.PASSKEY == p1.credentialType) { - return 1 - } - } + override fun compare(p0: CredentialEntryInfo, p1: CredentialEntryInfo): Int { + // First prefer passkey type for its security benefits + if (p0.credentialType != p1.credentialType) { + if (CredentialType.PASSKEY == p0.credentialType) { + return -1 + } else if (CredentialType.PASSKEY == p1.credentialType) { + return 1 + } + } - // Then order by last used timestamp - if (p0.lastUsedTimeMillis != null && p1.lastUsedTimeMillis != null) { - if (p0.lastUsedTimeMillis < p1.lastUsedTimeMillis) { - return 1 - } else if (p0.lastUsedTimeMillis > p1.lastUsedTimeMillis) { - return -1 - } - } else if (p0.lastUsedTimeMillis != null) { - return -1 - } else if (p1.lastUsedTimeMillis != null) { - return 1 + // Then order by last used timestamp + if (p0.lastUsedTimeMillis != null && p1.lastUsedTimeMillis != null) { + if (p0.lastUsedTimeMillis < p1.lastUsedTimeMillis) { + return 1 + } else if (p0.lastUsedTimeMillis > p1.lastUsedTimeMillis) { + return -1 + } + } else if (p0.lastUsedTimeMillis != null) { + return -1 + } else if (p1.lastUsedTimeMillis != null) { + return 1 + } + return 0 } - return 0 - } } \ No newline at end of file