Fix code review changes

Change-Id: I0571c151e62851634783b2aa8943b47fb84eea36

Author: neelanshsahai

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/AppDependencies.kt

@@ -23,6 +23,9 @@ import com.example.android.authentication.myvault.data.CredentialsDataSource
 import com.example.android.authentication.myvault.data.CredentialsRepository
 import com.example.android.authentication.myvault.data.RPIconDataSource
 import com.example.android.authentication.myvault.data.room.MyVaultDatabase
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
 
 /**
  * This class is an application-level singleton object which is providing dependencies required for the app to function.
@@ -42,6 +45,8 @@ object AppDependencies {
 
     lateinit var rpIconDataSource: RPIconDataSource
 
+    lateinit var coroutineScope: CoroutineScope
+
     /**
      * Initializes the core components required for the application's data storage and icon handling.
      * This includes:
@@ -72,5 +77,7 @@ object AppDependencies {
                 credentialsDataSource,
                 context,
             )
+
+        coroutineScope = CoroutineScope(Dispatchers.Default + SupervisorJob())
     }
 }

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/NotificationUtils.kt

@@ -13,6 +13,24 @@ import androidx.core.app.NotificationCompat
 import androidx.core.app.NotificationManagerCompat
 import com.example.android.authentication.myvault.ui.MainActivity
 
+/**
+ * Creates and registers a notification channel with the system.
+ *
+ * This method is intended to be called when your application starts up or before
+ * you attempt to post any notifications. On Android 8.0 (API level 26) and higher,
+ * all notifications must be assigned to a channel. If you don't specify a channel,
+ * your notifications won't appear.
+ *
+ * The created channel will have {@link NotificationManager#IMPORTANCE_HIGH}.
+ *
+ * @param channelName The user-visible name of the channel.
+ *                    This is displayed in the system's notification settings.
+ * @param channelDescription The user-visible description of the channel.
+ *                           This is displayed in the system's notification settings.
+ * @see NotificationChannel
+ * @see NotificationManager#createNotificationChannel(NotificationChannel)
+ * @see Context#NOTIFICATION_SERVICE
+ */
 fun Context.createNotificationChannel(
     channelName: String,
     channelDescription: String,
@@ -29,6 +47,33 @@ fun Context.createNotificationChannel(
     notificationManager.createNotificationChannel(channel)
 }
 
+/**
+ * Displays a system notification with the given title and content.
+ *
+ * This notification will navigate to {@link MainActivity} when tapped.
+ * It uses the channel ID defined by {@code NOTIFICATION_CHANNEL_ID} and will have
+ * {@link NotificationCompat#PRIORITY_MAX}. The notification is automatically canceled
+ * when the user taps it.
+ *
+ * Before posting the notification, this method checks for the
+ * {@link android.Manifest.permission#POST_NOTIFICATIONS} permission. If the permission
+ * is not granted, the notification will not be shown. This check is primarily for
+ * Android 13 (API level 33) and higher, where this permission is required.
+ *
+ * **Note:** It's assumed that {@code NOTIFICATION_CHANNEL_ID} is a predefined constant
+ * representing the ID of a channel that has already been created (e.g., using
+ * {@code createNotificationChannel}). Also, {@code NOTIFICATION_ID} should be a unique
+ * integer for this notification, allowing you to update or cancel it later.
+ * The small icon {@code R.drawable.android_secure} should exist in your project's drawables.
+ *
+ * @param title The title of the notification.
+ * @param content The main content text of the notification.
+ *
+ * @see NotificationCompat.Builder
+ * @see NotificationManagerCompat
+ * @see PendingIntent
+ * @see android.Manifest.permission#POST_NOTIFICATIONS
+ */
 fun Context.showNotification(
     title: String,
     content: String,

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/data/CredentialProviderService.kt

@@ -1,7 +1,6 @@
 package com.example.android.authentication.myvault.data
 
 import android.annotation.SuppressLint
-import android.util.Log
 import androidx.credentials.SignalAllAcceptedCredentialIdsRequest
 import androidx.credentials.SignalCurrentUserDetailsRequest
 import androidx.credentials.SignalUnknownCredentialRequest
@@ -16,58 +15,152 @@ import com.example.android.authentication.myvault.NAME
 import com.example.android.authentication.myvault.R
 import com.example.android.authentication.myvault.USER_ID
 import com.example.android.authentication.myvault.showNotification
-import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import org.json.JSONArray
 import org.json.JSONObject
 
+/**
+ * A service that listens to credential provider events triggered by the Android system
+ * or other credential providers.
+ *
+ * This service is responsible for handling signals related to credential state changes,
+ * such as when a credential is no longer valid, when a list of accepted credentials
+ * is updated, or when current user details change. It updates the local data source
+ * accordingly and notifies the user about these changes via system notifications.
+ *
+ * This service directly interacts with {@link AppDependencies#credentialsDataSource}
+ * to persist changes and uses {@link AppDependencies#coroutineScope} for performing
+ * background operations.
+ *
+ * @see CredentialProviderEventsService
+ * @see ProviderSignalCredentialStateRequest
+ * @see AppDependencies
+ */
 class CredentialProviderService: CredentialProviderEventsService() {
     private val dataSource = AppDependencies.credentialsDataSource
+    private val coroutineScope = AppDependencies.coroutineScope
 
+    /**
+     * Called when the system or another credential provider signals a change in credential state.
+     *
+     * This method inspects the type of [ProviderSignalCredentialStateRequest] and delegates
+     * to the appropriate handler function to update the local data store and show a notification.
+     * After processing the signal, {@link ProviderSignalCredentialStateCallback#onSignalConsumed()}
+     * is called to acknowledge receipt of the signal.
+     *
+     * The {@link SuppressLint("RestrictedApi")} annotation is used because this method
+     * interacts with APIs from the {@code androidx.credentials} library that might be
+     * marked as restricted for extension by library developers.
+     *
+     * @param request The request containing details about the credential state signal.
+     * @param callback The callback to be invoked after the signal has been processed.
+     */
     @SuppressLint("RestrictedApi")
     override fun onSignalCredentialStateRequest(
         request: ProviderSignalCredentialStateRequest,
         callback: ProviderSignalCredentialStateCallback,
     ) {
         when (request.callingRequest) {
             is SignalUnknownCredentialRequest -> {
-                handleUnknownCredentialRequest(request.callingRequest.requestJson)
-                showNotification(
-                    getString(R.string.credential_deletion),
-                    getString(R.string.unknown_signal_message)
+                updateDataOnSignalAndShowNotification(
+                    handleRequest = ::handleUnknownCredentialRequest,
+                    requestJson = request.callingRequest.requestJson,
+                    notificationTitle = getString(R.string.credential_deletion),
+                    notificationContent = getString(R.string.unknown_signal_message)
                 )
             }
 
             is SignalAllAcceptedCredentialIdsRequest -> {
-                handleAcceptedCredentialsRequest(request.callingRequest.requestJson)
-                showNotification(
-                    getString(R.string.credentials_list_updation),
-                    getString(R.string.all_accepted_signal_message)
+                updateDataOnSignalAndShowNotification(
+                    handleRequest = ::handleAcceptedCredentialsRequest,
+                    requestJson = request.callingRequest.requestJson,
+                    notificationTitle = getString(R.string.credentials_list_updation),
+                    notificationContent = getString(R.string.all_accepted_signal_message)
                 )
             }
+
             is SignalCurrentUserDetailsRequest -> {
-                handleCurrentUserDetailRequest(request.callingRequest.requestJson)
-                showNotification(
-                    getString(R.string.user_details_updation),
-                    getString(R.string.current_user_signal_message)
+                updateDataOnSignalAndShowNotification(
+                    handleRequest = ::handleCurrentUserDetailRequest,
+                    requestJson = request.callingRequest.requestJson,
+                    notificationTitle = getString(R.string.user_details_updation),
+                    notificationContent = getString(R.string.current_user_signal_message)
                 )
             }
+
             else -> { }
         }
 
         callback.onSignalConsumed()
     }
 
-    private fun handleUnknownCredentialRequest(requestJson: String) = runBlocking {
+    /**
+     * A helper function to asynchronously handle a credential state update request,
+     * update the data source, and then show a system notification on the main thread.
+     *
+     * @param handleRequest A suspend function that takes the request JSON string and processes it.
+     *                      This function is responsible for interacting with the data source.
+     * @param requestJson The JSON string payload from the original credential signal request.
+     * @param notificationTitle The title to be used for the system notification.
+     * @param notificationContent The content text for the system notification.
+     */
+    private fun updateDataOnSignalAndShowNotification(
+        handleRequest: suspend (String) -> Unit,
+        requestJson: String,
+        notificationTitle: String,
+        notificationContent: String,
+    ) {
+        coroutineScope.launch {
+            handleRequest(requestJson)
+            withContext(Dispatchers.Main) {
+                showNotification(
+                    title = notificationTitle,
+                    content = notificationContent,
+                )
+            }
+        }
+    }
+
+    /**
+     * Handles a [SignalUnknownCredentialRequest] by parsing the credential ID
+     * from the request JSON and attempting to hide the corresponding passkey in the data source.
+     *
+     * "Hiding" a passkey typically means marking it as inactive or not to be suggested
+     * for autofill, often because the system has indicated it's no longer valid
+     * (e.g., deleted from the authenticator).
+     *
+     * @param requestJson The JSON string payload from the [SignalUnknownCredentialRequest].
+     *                   Expected to contain a {@code CREDENTIAL_ID}.
+     */
+    private suspend fun handleUnknownCredentialRequest(requestJson: String) {
         val credentialId = JSONObject(requestJson).getString(CREDENTIAL_ID)
         dataSource.getPasskey(credentialId)?.let {
             dataSource.hidePasskey(it)
         }
     }
 
-    private fun handleAcceptedCredentialsRequest(requestJson: String) = runBlocking {
+    /**
+     * Handles a {@link SignalAllAcceptedCredentialIdsRequest} by synchronizing the visibility
+     * state of passkeys for a specific user.
+     *
+     * It retrieves all current passkeys for the user from the data source. Then, it compares
+     * this list against the list of accepted credential IDs provided in the signal.
+     * Passkeys whose IDs are in the accepted list are unhidden (made active).
+     * Passkeys whose IDs are not in the accepted list are hidden (made inactive).
+     *
+     * This is useful for scenarios where the system provides an authoritative list of
+     * credentials that are currently valid or preferred for a user.
+     *
+     * @param requestJson The JSON string payload from the {@link SignalAllAcceptedCredentialIdsRequest}.
+     *                   Expected to contain a {@code USER_ID} and {@code ACCEPTED_CREDENTIAL_IDS}
+     *                   (which can be a string or a JSON array of strings).
+     */
+    private suspend fun handleAcceptedCredentialsRequest(requestJson: String) {
         val request = JSONObject(requestJson)
         val userId = request.getString(USER_ID)
-        val listCurrentPasskeysForUser = dataSource.getPasskeyForUser(userId) ?: emptyList()
+        val listCurrentPasskeysForUser = dataSource.getAllPasskeysForUser(userId) ?: emptyList()
         val listAllAcceptedCredIds = mutableListOf<String>()
         when (val value = request.get(ACCEPTED_CREDENTIAL_IDS)) {
             is String -> listAllAcceptedCredIds.add(value)
@@ -91,12 +184,23 @@ class CredentialProviderService: CredentialProviderEventsService() {
         }
     }
 
-    private fun handleCurrentUserDetailRequest(requestJson: String) = runBlocking {
+    /**
+     * Handles a {@link SignalCurrentUserDetailsRequest} by updating the username and display name
+     * for all passkeys associated with a given user ID.
+     *
+     * This is useful when the user's profile information (like name or display name)
+     * changes elsewhere, and the credential provider needs to reflect these changes
+     * in its stored passkey data.
+     *
+     * @param requestJson The JSON string payload from the {@link SignalCurrentUserDetailsRequest}.
+     *                   Expected to contain {@code USER_ID}, {@code NAME}, and {@code DISPLAY_NAME}.
+     */
+    private suspend fun handleCurrentUserDetailRequest(requestJson: String) {
         val request = JSONObject(requestJson)
         val userId = request.getString(USER_ID)
         val updatedName = request.getString(NAME)
         val updatedDisplayName = request.getString(DISPLAY_NAME)
-        val listPasskeys = dataSource.getPasskeyForUser(userId) ?: emptyList()
+        val listPasskeys = dataSource.getAllPasskeysForUser(userId) ?: emptyList()
         // Update user details for each passkey
         for (key in listPasskeys) {
             val newPasskeyItem = key.copy(username = updatedName, displayName = updatedDisplayName)

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/data/CredentialsDataSource.kt

@@ -127,8 +127,8 @@ class CredentialsDataSource(
         return myVaultDao.getPasskey(credId)
     }
 
-    fun getPasskeyForUser(userId: String): List<PasskeyItem>? {
-        return myVaultDao.getPasskeysForUser(userId)
+    fun getAllPasskeysForUser(userId: String): List<PasskeyItem>? {
+        return myVaultDao.getAllPasskeysForUser(userId)
     }
 
     suspend fun hidePasskey(passkey: PasskeyItem) {

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/data/CredentialsRepository.kt

@@ -228,8 +228,9 @@ class CredentialsRepository(
             val credentials = credentialsDataSource.credentialsForSite(request.rpId) ?: return false
 
             val passkeys = credentials.passkeys
-            for (passkey in passkeys) {
-                if (!passkey.hidden) {
+            passkeys
+                .filter { !it.hidden }
+                .forEach { passkey ->
                     val data = Bundle()
                     data.putString("requestJson", option.requestJson)
                     data.putString("credId", passkey.credId)
@@ -259,7 +260,6 @@ class CredentialsRepository(
                     // Add the entry to the response builder.
                     responseBuilder.addCredentialEntry(entry)
                 }
-            }
         } catch (e: IOException) {
             return false
         }

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/data/room/MyVaultDatabase.kt

@@ -89,6 +89,6 @@ interface MyVaultDao {
     @Query("SELECT * from passkeys WHERE credId = :credId")
     fun getPasskey(credId: String): PasskeyItem?
 
-    @Query("SELECT * from passkeys WHERE uid = :userId and hidden = false")
-    fun getPasskeysForUser(userId: String): List<PasskeyItem>?
+    @Query("SELECT * from passkeys WHERE uid = :userId")
+    fun getAllPasskeysForUser(userId: String): List<PasskeyItem>?
 }

CredentialProvider/MyVault/app/src/main/java/com/example/android/authentication/myvault/ui/MainActivity.kt

@@ -20,6 +20,7 @@ import androidx.activity.ComponentActivity
 import androidx.activity.compose.setContent
 import androidx.activity.enableEdgeToEdge
 import androidx.core.view.WindowCompat
+import com.example.android.authentication.myvault.R
 import com.example.android.authentication.myvault.createNotificationChannel
 import com.example.android.authentication.myvault.ui.theme.MyVaultTheme
 
@@ -28,8 +29,8 @@ class MainActivity : ComponentActivity() {
         enableEdgeToEdge()
         super.onCreate(savedInstanceState)
         createNotificationChannel(
-            "Signal API notification channel",
-            "Notification channel used for testing Signal APIs. Apps pushes a notification if a Signal from RP is received"
+            getString(R.string.notification_channel_name),
+            getString(R.string.notification_channel_description),
         )
 
         WindowCompat.setDecorFitsSystemWindows(window, false)

CredentialProvider/MyVault/app/src/main/res/values/strings.xml

@@ -75,4 +75,6 @@
     <string name="all_accepted_signal_message">The provider received the AcceptedCredentialIds request and thus updated the list of user credentials</string>
     <string name="user_details_updation">User details updated successfully</string>
     <string name="current_user_signal_message">The provider received the CurrentUserDetails request and thus updated the user details in the credential</string>
+    <string name="notification_channel_description">Notification channel used for testing Signal APIs. Apps pushes a notification if a Signal from RP is received</string>
+    <string name="notification_channel_name">Signal API notification channel</string>
 </resources>

CredentialProvider/MyVault/gradle/libs.versions.toml

@@ -9,7 +9,7 @@ espressoCore = "3.6.1"
 lifecycleRuntime = "2.8.7"
 activityCompose = "1.10.0"
 composeBom = "2025.02.00"
-credentials = "1.6.0-alpha04"
+credentials = "1.6.0-alpha05"
 room = "2.7.2"
 biometrics = "1.2.0-alpha05"
 accompanist = "0.28.0"