Skip to content

Commit cfaaaed

Browse files
neelanshsahaineelanshsahai
committed
Fix code review changes
Change-Id: I0571c151e62851634783b2aa8943b47fb84eea36
1 parent 42b9454 commit cfaaaed

File tree

9 files changed

+207
-57
lines changed

9 files changed

+207
-57
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import com.example.android.authentication.myvault.data.CredentialsDataSource
2323
import com.example.android.authentication.myvault.data.CredentialsRepository
2424
import com.example.android.authentication.myvault.data.RPIconDataSource
2525
import com.example.android.authentication.myvault.data.room.MyVaultDatabase
26+
import kotlinx.coroutines.CoroutineScope
27+
import kotlinx.coroutines.Dispatchers
28+
import kotlinx.coroutines.SupervisorJob
2629

2730
/**
2831
* This class is an application-level singleton object which is providing dependencies required for the app to function.
@@ -42,6 +45,8 @@ object AppDependencies {
4245

4346
lateinit var rpIconDataSource: RPIconDataSource
4447

48+
lateinit var coroutineScope: CoroutineScope
49+
4550
/**
4651
* Initializes the core components required for the application's data storage and icon handling.
4752
* This includes:
@@ -72,5 +77,7 @@ object AppDependencies {
7277
credentialsDataSource,
7378
context,
7479
)
80+
81+
coroutineScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
7582
}
7683
}

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ import androidx.core.app.NotificationCompat
1313
import androidx.core.app.NotificationManagerCompat
1414
import com.example.android.authentication.myvault.ui.MainActivity
1515

16+
/**
17+
* Creates and registers a notification channel with the system.
18+
*
19+
* This is a utility extension function that creates a Notification Channel with
20+
* [NotificationManager.IMPORTANCE_HIGH] to show pop-up notification on receiving
21+
* signals from the RP apps
22+
*
23+
* @param channelName The user-visible name of the channel.
24+
* This is displayed in the system's notification settings.
25+
* @param channelDescription The user-visible description of the channel.
26+
* This is displayed in the system's notification settings.
27+
*/
1628
fun Context.createNotificationChannel(
1729
channelName: String,
1830
channelDescription: String,
@@ -24,11 +36,17 @@ fun Context.createNotificationChannel(
2436
).apply {
2537
description = channelDescription
2638
}
27-
// Register the channel with the system.
39+
2840
val notificationManager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
2941
notificationManager.createNotificationChannel(channel)
3042
}
3143

44+
/**
45+
* Utility extension function that displays a system notification with the given title and content.
46+
*
47+
* @param title The title of the notification.
48+
* @param content The main content text of the notification.
49+
*/
3250
fun Context.showNotification(
3351
title: String,
3452
content: String,

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

Lines changed: 165 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,91 +16,210 @@ import com.example.android.authentication.myvault.NAME
1616
import com.example.android.authentication.myvault.R
1717
import com.example.android.authentication.myvault.USER_ID
1818
import com.example.android.authentication.myvault.showNotification
19-
import kotlinx.coroutines.runBlocking
19+
import kotlinx.coroutines.Dispatchers
20+
import kotlinx.coroutines.launch
21+
import kotlinx.coroutines.withContext
2022
import org.json.JSONArray
2123
import org.json.JSONObject
2224

25+
/**
26+
* A service that listens to credential provider events triggered by the relying parties
27+
*
28+
* This service is responsible for handling signals related to credential state changes in the RPs,
29+
* such as when a credential is no longer valid, when a list of accepted credentials is accepted,
30+
* or when current user details change for credentials
31+
*/
2332
class CredentialProviderService: CredentialProviderEventsService() {
2433
private val dataSource = AppDependencies.credentialsDataSource
34+
private val coroutineScope = AppDependencies.coroutineScope
2535

36+
/**
37+
* Called when the system or another credential provider signals a change in credential state.
38+
*
39+
* This method inspects the type of [ProviderSignalCredentialStateRequest] and delegates
40+
* to the appropriate handler function to update the local data store and show a notification.
41+
* After processing the signal, {@link ProviderSignalCredentialStateCallback#onSignalConsumed()}
42+
* is called to acknowledge receipt of the signal.
43+
*
44+
* The {@link SuppressLint("RestrictedApi")} annotation is used because this method
45+
* interacts with APIs from the {@code androidx.credentials} library that might be
46+
* marked as restricted for extension by library developers.
47+
*
48+
* @param request The request containing details about the credential state signal.
49+
* @param callback The callback to be invoked after the signal has been processed.
50+
*/
2651
@SuppressLint("RestrictedApi")
2752
override fun onSignalCredentialStateRequest(
2853
request: ProviderSignalCredentialStateRequest,
2954
callback: ProviderSignalCredentialStateCallback,
3055
) {
3156
when (request.callingRequest) {
3257
is SignalUnknownCredentialRequest -> {
33-
handleUnknownCredentialRequest(request.callingRequest.requestJson)
34-
showNotification(
35-
getString(R.string.credential_deletion),
36-
getString(R.string.unknown_signal_message)
58+
updateDataOnSignalAndShowNotification(
59+
handleRequest = ::handleUnknownCredentialRequest,
60+
requestJson = request.callingRequest.requestJson,
61+
notificationTitle = getString(R.string.credential_deletion),
62+
notificationContent = getString(R.string.unknown_signal_message)
3763
)
3864
}
3965

4066
is SignalAllAcceptedCredentialIdsRequest -> {
41-
handleAcceptedCredentialsRequest(request.callingRequest.requestJson)
42-
showNotification(
43-
getString(R.string.credentials_list_updation),
44-
getString(R.string.all_accepted_signal_message)
67+
updateDataOnSignalAndShowNotification(
68+
handleRequest = ::handleAcceptedCredentialsRequest,
69+
requestJson = request.callingRequest.requestJson,
70+
notificationTitle = getString(R.string.credentials_list_updation),
71+
notificationContent = getString(R.string.all_accepted_signal_message)
4572
)
4673
}
74+
4775
is SignalCurrentUserDetailsRequest -> {
48-
handleCurrentUserDetailRequest(request.callingRequest.requestJson)
49-
showNotification(
50-
getString(R.string.user_details_updation),
51-
getString(R.string.current_user_signal_message)
76+
updateDataOnSignalAndShowNotification(
77+
handleRequest = ::handleCurrentUserDetailRequest,
78+
requestJson = request.callingRequest.requestJson,
79+
notificationTitle = getString(R.string.user_details_updation),
80+
notificationContent = getString(R.string.current_user_signal_message)
5281
)
5382
}
83+
5484
else -> { }
5585
}
5686

5787
callback.onSignalConsumed()
5888
}
5989

60-
private fun handleUnknownCredentialRequest(requestJson: String) = runBlocking {
61-
val credentialId = JSONObject(requestJson).getString(CREDENTIAL_ID)
62-
dataSource.getPasskey(credentialId)?.let {
63-
dataSource.hidePasskey(it)
90+
/**
91+
* A helper function to asynchronously handle a credential state update request,
92+
* update the data source, and then show a system notification on the main thread.
93+
*
94+
* @param handleRequest A suspend function that takes the request JSON string and processes it.
95+
* This function is responsible for interacting with the data source.
96+
* @param requestJson The JSON string payload from the original credential signal request.
97+
* @param notificationTitle The title to be used for the system notification.
98+
* @param notificationContent The content text for the system notification.
99+
*/
100+
private fun updateDataOnSignalAndShowNotification(
101+
handleRequest: suspend (String) -> Boolean,
102+
requestJson: String,
103+
notificationTitle: String,
104+
notificationContent: String,
105+
) {
106+
coroutineScope.launch {
107+
val success = handleRequest(requestJson)
108+
withContext(Dispatchers.Main) {
109+
if (success) {
110+
showNotification(
111+
title = notificationTitle,
112+
content = notificationContent,
113+
)
114+
}
115+
}
64116
}
65117
}
66118

67-
private fun handleAcceptedCredentialsRequest(requestJson: String) = runBlocking {
68-
val request = JSONObject(requestJson)
69-
val userId = request.getString(USER_ID)
70-
val listCurrentPasskeysForUser = dataSource.getPasskeyForUser(userId) ?: emptyList()
71-
val listAllAcceptedCredIds = mutableListOf<String>()
72-
when (val value = request.get(ACCEPTED_CREDENTIAL_IDS)) {
73-
is String -> listAllAcceptedCredIds.add(value)
74-
is JSONArray -> {
75-
for (i in 0 until value.length()) {
76-
val item = value.get(i)
77-
if (item is String) {
78-
listAllAcceptedCredIds.add(item)
119+
/**
120+
* Handles a [SignalUnknownCredentialRequest] by parsing the credential ID
121+
* from the request JSON and attempting to hide the corresponding passkey in the data source.
122+
*
123+
* "Hiding" a passkey typically means marking it as inactive or not to be suggested
124+
* for autofill, often because the system has indicated it's no longer valid
125+
* (e.g., deleted from the authenticator).
126+
*
127+
* @param requestJson The JSON string payload from the [SignalUnknownCredentialRequest].
128+
* Expected to contain a {@code CREDENTIAL_ID}.
129+
*/
130+
private suspend fun handleUnknownCredentialRequest(requestJson: String): Boolean {
131+
try {
132+
val credentialId = JSONObject(requestJson).getString(CREDENTIAL_ID)
133+
dataSource.getPasskey(credentialId)?.let {
134+
// Currently hiding the passkey on UnknownSignal for testing purpose
135+
// If the business logc requires deletion, please add deletion code instead
136+
dataSource.hidePasskey(it)
137+
}
138+
return true
139+
} catch (e: Exception) {
140+
Log.e(getString(R.string.failed_to_handle_unknowncredentialrequest), e.toString())
141+
return false
142+
}
143+
}
144+
145+
/**
146+
* Handles a {@link SignalAllAcceptedCredentialIdsRequest} by synchronizing the visibility
147+
* state of passkeys for a specific user.
148+
*
149+
* It retrieves all current passkeys for the user from the data source. Then, it compares
150+
* this list against the list of accepted credential IDs provided in the signal.
151+
* Passkeys whose IDs are in the accepted list are unhidden (made active).
152+
* Passkeys whose IDs are not in the accepted list are hidden (made inactive).
153+
*
154+
* This is useful for scenarios where the system provides an authoritative list of
155+
* credentials that are currently valid or preferred for a user.
156+
*
157+
* @param requestJson The JSON string payload from the {@link SignalAllAcceptedCredentialIdsRequest}.
158+
* Expected to contain a {@code USER_ID} and {@code ACCEPTED_CREDENTIAL_IDS}
159+
* (which can be a string or a JSON array of strings).
160+
*/
161+
private suspend fun handleAcceptedCredentialsRequest(requestJson: String): Boolean {
162+
try {
163+
val request = JSONObject(requestJson)
164+
val userId = request.getString(USER_ID)
165+
val listCurrentPasskeysForUser = dataSource.getAllPasskeysForUser(userId) ?: emptyList()
166+
val listAllAcceptedCredIds = mutableListOf<String>()
167+
when (val value = request.get(ACCEPTED_CREDENTIAL_IDS)) {
168+
is String -> listAllAcceptedCredIds.add(value)
169+
is JSONArray -> {
170+
for (i in 0 until value.length()) {
171+
val item = value.get(i)
172+
if (item is String) {
173+
listAllAcceptedCredIds.add(item)
174+
}
79175
}
80176
}
177+
178+
else -> { /*do nothing*/ }
81179
}
82-
else -> { /*do nothing*/ }
83-
}
84180

85-
for (key in listCurrentPasskeysForUser) {
86-
if (listAllAcceptedCredIds.contains(key.credId)) {
87-
dataSource.unhidePasskey(key)
88-
} else {
89-
dataSource.hidePasskey(key)
181+
for (key in listCurrentPasskeysForUser) {
182+
if (listAllAcceptedCredIds.contains(key.credId)) {
183+
dataSource.unhidePasskey(key)
184+
} else {
185+
dataSource.hidePasskey(key)
186+
}
90187
}
188+
return true
189+
} catch (e: Exception) {
190+
Log.e(getString(R.string.failed_to_handle_acceptedcredentialsrequest), e.toString())
191+
return false
91192
}
92193
}
93194

94-
private fun handleCurrentUserDetailRequest(requestJson: String) = runBlocking {
95-
val request = JSONObject(requestJson)
96-
val userId = request.getString(USER_ID)
97-
val updatedName = request.getString(NAME)
98-
val updatedDisplayName = request.getString(DISPLAY_NAME)
99-
val listPasskeys = dataSource.getPasskeyForUser(userId) ?: emptyList()
100-
// Update user details for each passkey
101-
for (key in listPasskeys) {
102-
val newPasskeyItem = key.copy(username = updatedName, displayName = updatedDisplayName)
103-
dataSource.updatePasskey(newPasskeyItem)
195+
/**
196+
* Handles a {@link SignalCurrentUserDetailsRequest} by updating the username and display name
197+
* for all passkeys associated with a given user ID.
198+
*
199+
* This is useful when the user's profile information (like name or display name)
200+
* changes elsewhere, and the credential provider needs to reflect these changes
201+
* in its stored passkey data.
202+
*
203+
* @param requestJson The JSON string payload from the {@link SignalCurrentUserDetailsRequest}.
204+
* Expected to contain {@code USER_ID}, {@code NAME}, and {@code DISPLAY_NAME}.
205+
*/
206+
private suspend fun handleCurrentUserDetailRequest(requestJson: String): Boolean {
207+
try {
208+
val request = JSONObject(requestJson)
209+
val userId = request.getString(USER_ID)
210+
val updatedName = request.getString(NAME)
211+
val updatedDisplayName = request.getString(DISPLAY_NAME)
212+
val listPasskeys = dataSource.getAllPasskeysForUser(userId) ?: emptyList()
213+
// Update user details for each passkey
214+
for (key in listPasskeys) {
215+
val newPasskeyItem =
216+
key.copy(username = updatedName, displayName = updatedDisplayName)
217+
dataSource.updatePasskey(newPasskeyItem)
218+
}
219+
return true
220+
} catch (e: Exception) {
221+
Log.e(getString(R.string.failed_to_handle_currentuserdetailrequest), e.toString())
222+
return false
104223
}
105224
}
106225
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ class CredentialsDataSource(
127127
return myVaultDao.getPasskey(credId)
128128
}
129129

130-
fun getPasskeyForUser(userId: String): List<PasskeyItem>? {
131-
return myVaultDao.getPasskeysForUser(userId)
130+
suspend fun getAllPasskeysForUser(userId: String): List<PasskeyItem>? {
131+
return myVaultDao.getAllPasskeysForUser(userId)
132132
}
133133

134134
suspend fun hidePasskey(passkey: PasskeyItem) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,9 @@ class CredentialsRepository(
228228
val credentials = credentialsDataSource.credentialsForSite(request.rpId) ?: return false
229229

230230
val passkeys = credentials.passkeys
231-
for (passkey in passkeys) {
232-
if (!passkey.hidden) {
231+
passkeys
232+
.filter { !it.hidden }
233+
.forEach { passkey ->
233234
val data = Bundle()
234235
data.putString("requestJson", option.requestJson)
235236
data.putString("credId", passkey.credId)
@@ -259,7 +260,6 @@ class CredentialsRepository(
259260
// Add the entry to the response builder.
260261
responseBuilder.addCredentialEntry(entry)
261262
}
262-
}
263263
} catch (e: IOException) {
264264
return false
265265
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,6 @@ interface MyVaultDao {
8989
@Query("SELECT * from passkeys WHERE credId = :credId")
9090
fun getPasskey(credId: String): PasskeyItem?
9191

92-
@Query("SELECT * from passkeys WHERE uid = :userId and hidden = false")
93-
fun getPasskeysForUser(userId: String): List<PasskeyItem>?
92+
@Query("SELECT * from passkeys WHERE uid = :userId")
93+
suspend fun getAllPasskeysForUser(userId: String): List<PasskeyItem>?
9494
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import androidx.activity.ComponentActivity
2020
import androidx.activity.compose.setContent
2121
import androidx.activity.enableEdgeToEdge
2222
import androidx.core.view.WindowCompat
23+
import com.example.android.authentication.myvault.R
2324
import com.example.android.authentication.myvault.createNotificationChannel
2425
import com.example.android.authentication.myvault.ui.theme.MyVaultTheme
2526

@@ -28,8 +29,8 @@ class MainActivity : ComponentActivity() {
2829
enableEdgeToEdge()
2930
super.onCreate(savedInstanceState)
3031
createNotificationChannel(
31-
"Signal API notification channel",
32-
"Notification channel used for testing Signal APIs. Apps pushes a notification if a Signal from RP is received"
32+
getString(R.string.notification_channel_name),
33+
getString(R.string.notification_channel_description),
3334
)
3435

3536
WindowCompat.setDecorFitsSystemWindows(window, false)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,9 @@
7575
<string name="all_accepted_signal_message">The provider received the AcceptedCredentialIds request and thus updated the list of user credentials</string>
7676
<string name="user_details_updation">User details updated successfully</string>
7777
<string name="current_user_signal_message">The provider received the CurrentUserDetails request and thus updated the user details in the credential</string>
78+
<string name="notification_channel_description">Notification channel used for testing Signal APIs. Apps pushes a notification if a Signal from RP is received</string>
79+
<string name="notification_channel_name">Signal API notification channel</string>
80+
<string name="failed_to_handle_unknowncredentialrequest">Failed to handle UnknownCredentialRequest</string>
81+
<string name="failed_to_handle_acceptedcredentialsrequest">Failed to handle AcceptedCredentialsRequest</string>
82+
<string name="failed_to_handle_currentuserdetailrequest">Failed to handle CurrentUserDetailRequest</string>
7883
</resources>

‎CredentialProvider/MyVault/gradle/libs.versions.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ espressoCore = "3.6.1"
99
lifecycleRuntime = "2.8.7"
1010
activityCompose = "1.10.0"
1111
composeBom = "2025.02.00"
12-
credentials = "1.6.0-alpha04"
12+
credentials = "1.6.0-alpha05"
1313
room = "2.7.2"
1414
biometrics = "1.2.0-alpha05"
1515
accompanist = "0.28.0"

0 commit comments

Comments
 (0)