Skip to content

Commit

Permalink
[PM-13070] Add userId to Fido2 GetCredentials and CredentialAssertion…
Browse files Browse the repository at this point in the history
… requests (#4003)
  • Loading branch information
SaintPatrck authored Oct 3, 2024
1 parent 569ffc3 commit e6eb626
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import kotlinx.parcelize.Parcelize
*/
@Parcelize
data class Fido2CredentialAssertionRequest(
val userId: String,
val cipherId: String?,
val credentialId: String?,
val requestJson: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import kotlinx.parcelize.Parcelize
data class Fido2GetCredentialsRequest(
val candidateQueryData: Bundle,
val id: String,
val userId: String,
val requestJson: String,
val clientDataHash: ByteArray? = null,
val packageName: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ sealed class Fido2GetCredentialsResult {
/**
* Indicates credentials were successfully queried.
*
* @param userId ID of the user whose credentials were queried.
* @param options Original request options provided by the relying party.
* @param credentials Collection of [Fido2CredentialAutofillView]s matching the original request
* parameters. This may be an empty list if no matching values were found.
*/
data class Success(
val userId: String,
val options: BeginGetPublicKeyCredentialOption,
val credentials: List<Fido2CredentialAutofillView>,
) : Fido2GetCredentialsResult()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class Fido2ProviderProcessorImpl(
title = context.getString(R.string.unlock),
pendingIntent = intentManager.createFido2UnlockPendingIntent(
action = UNLOCK_ACCOUNT_INTENT,
userId = userState.activeUserId,
requestCode = requestCode.getAndIncrement(),
),
)
Expand Down Expand Up @@ -209,13 +210,14 @@ class Fido2ProviderProcessorImpl(
.getPasskeyAssertionOptionsOrNull(requestJson = option.requestJson)
?.relyingPartyId
?: throw GetCredentialUnknownException("Invalid data.")
buildCredentialEntries(relyingPartyId, option)
buildCredentialEntries(userId, relyingPartyId, option)
} else {
throw GetCredentialUnsupportedException("Unsupported option.")
}
}

private suspend fun buildCredentialEntries(
userId: String,
relyingPartyId: String,
option: BeginGetPublicKeyCredentialOption,
): List<CredentialEntry> {
Expand All @@ -236,12 +238,16 @@ class Fido2ProviderProcessorImpl(
result
.fido2CredentialAutofillViews
.filter { it.rpId == relyingPartyId }
.toCredentialEntries(option)
.toCredentialEntries(
userId = userId,
option = option,
)
}
}
}

private fun List<Fido2CredentialAutofillView>.toCredentialEntries(
userId: String,
option: BeginGetPublicKeyCredentialOption,
): List<CredentialEntry> =
this
Expand All @@ -253,6 +259,7 @@ class Fido2ProviderProcessorImpl(
pendingIntent = intentManager
.createFido2GetCredentialPendingIntent(
action = GET_PASSKEY_INTENT,
userId = userId,
credentialId = it.credentialId.toString(),
cipherId = it.cipherId,
requestCode = requestCode.getAndIncrement(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ fun Intent.getFido2AssertionRequestOrNull(): Fido2CredentialAssertionRequest? {
val cipherId = getStringExtra(EXTRA_KEY_CIPHER_ID)
?: return null

val userId: String = getStringExtra(EXTRA_KEY_USER_ID)
?: return null

return Fido2CredentialAssertionRequest(
userId = userId,
cipherId = cipherId,
credentialId = credentialId,
requestJson = option.requestJson,
Expand Down Expand Up @@ -95,9 +99,13 @@ fun Intent.getFido2GetCredentialsRequestOrNull(): Fido2GetCredentialsRequest? {
.callingAppInfo
?: return null

val userId: String = getStringExtra(EXTRA_KEY_USER_ID)
?: return null

return Fido2GetCredentialsRequest(
candidateQueryData = option.candidateQueryData,
id = option.id,
userId = userId,
requestJson = option.requestJson,
clientDataHash = option.clientDataHash,
packageName = callingAppInfo.packageName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class Fido2CompletionManagerImpl(
val pendingIntent = intentManager
.createFido2GetCredentialPendingIntent(
action = GET_PASSKEY_INTENT,
userId = result.userId,
credentialId = it.credentialId.toString(),
cipherId = it.cipherId,
requestCode = Random.nextInt(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ interface IntentManager {
*/
fun createFido2GetCredentialPendingIntent(
action: String,
userId: String,
credentialId: String,
cipherId: String,
requestCode: Int,
Expand All @@ -130,6 +131,7 @@ interface IntentManager {
*/
fun createFido2UnlockPendingIntent(
action: String,
userId: String,
requestCode: Int,
): PendingIntent

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,14 @@ class IntentManagerImpl(

override fun createFido2GetCredentialPendingIntent(
action: String,
userId: String,
credentialId: String,
cipherId: String,
requestCode: Int,
): PendingIntent {
val intent = Intent(action)
.setPackage(context.packageName)
.putExtra(EXTRA_KEY_USER_ID, userId)
.putExtra(EXTRA_KEY_CREDENTIAL_ID, credentialId)
.putExtra(EXTRA_KEY_CIPHER_ID, cipherId)

Expand All @@ -277,9 +279,12 @@ class IntentManagerImpl(

override fun createFido2UnlockPendingIntent(
action: String,
userId: String,
requestCode: Int,
): PendingIntent {
val intent = Intent(action).setPackage(context.packageName)
val intent = Intent(action)
.setPackage(context.packageName)
.putExtra(EXTRA_KEY_USER_ID, userId)

return PendingIntent.getActivity(
/* context = */ context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,7 @@ class VaultItemListingViewModel @Inject constructor(
sendEvent(
VaultItemListingEvent.CompleteFido2GetCredentialsRequest(
Fido2GetCredentialsResult.Success(
userId = fido2GetCredentialsRequest.userId,
options = fido2GetCredentialsRequest.option,
credentials = vaultData
.data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.pm.SigningInfo

fun createMockFido2CredentialAssertionRequest(number: Int = 1): Fido2CredentialAssertionRequest =
Fido2CredentialAssertionRequest(
userId = "mockUserId-$number",
cipherId = "mockCipherId-$number",
credentialId = "mockCredentialId-$number",
requestJson = "mockRequestJson-$number",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ fun createMockFido2GetCredentialsRequest(
): Fido2GetCredentialsRequest = Fido2GetCredentialsRequest(
candidateQueryData = Bundle(),
id = "mockId-$number",
userId = "mockUserId-$number",
requestJson = "requestJson-$number",
clientDataHash = null,
packageName = "mockPackageName-$number",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ class Fido2ProviderProcessorTest {
every {
intentManager.createFido2UnlockPendingIntent(
action = "com.x8bit.bitwarden.fido2.ACTION_UNLOCK_ACCOUNT",
userId = "mockUserId-1",
requestCode = any(),
)
} returns mockIntent
Expand All @@ -317,6 +318,7 @@ class Fido2ProviderProcessorTest {
callback.onResult(any())
intentManager.createFido2UnlockPendingIntent(
action = "com.x8bit.bitwarden.fido2.ACTION_UNLOCK_ACCOUNT",
userId = "mockUserId-1",
requestCode = any(),
)
}
Expand Down Expand Up @@ -463,6 +465,7 @@ class Fido2ProviderProcessorTest {
every {
intentManager.createFido2GetCredentialPendingIntent(
action = "com.x8bit.bitwarden.fido2.ACTION_GET_PASSKEY",
userId = DEFAULT_USER_STATE.activeUserId,
credentialId = mockFido2CredentialAutofillViews.first().credentialId.toString(),
cipherId = mockFido2CredentialAutofillViews.first().cipherId,
requestCode = any(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ class Fido2IntentUtilsTest {
@Test
fun `getFido2AssertionRequestOrNull should return Fido2AssertionRequest when present`() {
val intent = mockk<Intent> {
every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId"
every { getStringExtra(EXTRA_KEY_CIPHER_ID) } returns "mockCipherId"
every { getStringExtra(EXTRA_KEY_CREDENTIAL_ID) } returns "mockCredentialId"
}
Expand Down Expand Up @@ -190,6 +191,7 @@ class Fido2IntentUtilsTest {
assertNotNull(assertionRequest)
assertEquals(
Fido2CredentialAssertionRequest(
userId = "mockUserId",
cipherId = "mockCipherId",
credentialId = "mockCredentialId",
requestJson = mockOption.requestJson,
Expand Down Expand Up @@ -287,14 +289,38 @@ class Fido2IntentUtilsTest {
} returns mockProviderGetCredentialRequest

val assertionRequest = intent.getFido2AssertionRequestOrNull()
assertNull(assertionRequest)
}

@Test
fun `getFido2AssertionRequestOrNull should return null when user id is not in extras`() {
val intent = mockk<Intent> {
every { getStringExtra(EXTRA_KEY_CREDENTIAL_ID) } returns "mockCredentialId"
every { getStringExtra(EXTRA_KEY_CIPHER_ID) } returns "mockCipherId"
every { getStringExtra(EXTRA_KEY_USER_ID) } returns null
}
val mockOption = GetPublicKeyCredentialOption(
requestJson = "requestJson",
clientDataHash = byteArrayOf(0),
allowedProviders = emptySet(),
)
val mockProviderGetCredentialRequest = ProviderGetCredentialRequest(
credentialOptions = listOf(mockOption),
callingAppInfo = mockk(),
)
every {
PendingIntentHandler.retrieveProviderGetCredentialRequest(intent)
} returns mockProviderGetCredentialRequest
val assertionRequest = intent.getFido2AssertionRequestOrNull()
assertNull(assertionRequest)
}

@Suppress("MaxLineLength")
@Test
fun `getFido2GetCredentialsRequestOrNull should return Fido2GetCredentialRequest when present`() {
val intent = mockk<Intent>()
val intent = mockk<Intent> {
every { getStringExtra("user_id") } returns "mockUserId"
}
val mockOption = BeginGetPublicKeyCredentialOption(
candidateQueryData = bundleOf(),
id = "mockId",
Expand All @@ -320,6 +346,7 @@ class Fido2IntentUtilsTest {
Fido2GetCredentialsRequest(
candidateQueryData = mockOption.candidateQueryData,
id = mockOption.id,
userId = "mockUserId",
requestJson = mockOption.requestJson,
clientDataHash = mockOption.clientDataHash,
packageName = mockCallingAppInfo.packageName,
Expand Down Expand Up @@ -378,6 +405,20 @@ class Fido2IntentUtilsTest {
val result = intent.getFido2GetCredentialsRequestOrNull()
assertNull(result)
}

@Test
fun `getFido2GetCredentialRequestOrNull should return null when user id is not in extras`() {
val intent = mockk<Intent> {
every { getStringExtra(EXTRA_KEY_USER_ID) } returns null
}
val mockOption = createMockBeginGetPublicKeyCredentialOption(number = 1)
every { PendingIntentHandler.retrieveBeginGetCredentialRequest(intent) } returns mockk {
every { beginGetCredentialOptions } returns listOf(mockOption)
every { callingAppInfo } returns mockk()
}
val result = intent.getFido2GetCredentialsRequestOrNull()
assertNull(result)
}
}

private fun createMockBeginGetPublicKeyCredentialOption(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,12 @@ import android.content.Intent
import androidx.credentials.provider.BeginGetCredentialResponse
import androidx.credentials.provider.PendingIntentHandler
import androidx.credentials.provider.PublicKeyCredentialEntry
import com.bitwarden.fido.Fido2CredentialAutofillView
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsResult
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult
import com.x8bit.bitwarden.data.autofill.fido2.processor.GET_PASSKEY_INTENT
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockFido2CredentialAutofillView
import com.x8bit.bitwarden.ui.platform.manager.intent.EXTRA_KEY_CIPHER_ID
import com.x8bit.bitwarden.ui.platform.manager.intent.EXTRA_KEY_CREDENTIAL_ID
import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
import io.mockk.Called
import io.mockk.MockKVerificationScope
Expand Down Expand Up @@ -175,6 +172,7 @@ class Fido2CompletionManagerTest {
fido2CompletionManager
.completeFido2GetCredentialRequest(
Fido2GetCredentialsResult.Success(
userId = "mockUserId",
options = mockk(),
credentials = emptyList(),
),
Expand All @@ -201,6 +199,7 @@ class Fido2CompletionManagerTest {
every {
mockIntentManager.createFido2GetCredentialPendingIntent(
action = GET_PASSKEY_INTENT,
userId = "mockUserId",
credentialId = mockFido2AutofillView.credentialId.toString(),
cipherId = mockFido2AutofillView.cipherId,
requestCode = any(),
Expand All @@ -211,6 +210,7 @@ class Fido2CompletionManagerTest {
fido2CompletionManager
.completeFido2GetCredentialRequest(
Fido2GetCredentialsResult.Success(
userId = "mockUserId",
options = mockk(),
credentials = mockFido2AutofillViewList,
),
Expand Down Expand Up @@ -249,6 +249,7 @@ class Fido2CompletionManagerTest {
every {
mockIntentManager.createFido2GetCredentialPendingIntent(
action = GET_PASSKEY_INTENT,
userId = "mockUserId",
credentialId = mockFido2AutofillView.credentialId.toString(),
cipherId = mockFido2AutofillView.cipherId,
requestCode = any(),
Expand All @@ -259,6 +260,7 @@ class Fido2CompletionManagerTest {
fido2CompletionManager
.completeFido2GetCredentialRequest(
Fido2GetCredentialsResult.Success(
userId = "mockUserId",
options = mockk(),
credentials = mockFido2AutofillViewList,
),
Expand Down Expand Up @@ -304,35 +306,5 @@ class Fido2CompletionManagerTest {
mockActivity.finish()
}
}

private fun setupMockCompletionIntent(
mockFido2AutofillView1: Fido2CredentialAutofillView,
mockCredentialEntry1: PublicKeyCredentialEntry,
): Intent {
val mockIntent1 = mockk<Intent> {
every {
putExtra(
EXTRA_KEY_CIPHER_ID,
mockFido2AutofillView1.cipherId,
)
} returns this
every {
putExtra(
EXTRA_KEY_CREDENTIAL_ID,
mockFido2AutofillView1.credentialId.toString(),
)
} returns this
}

every {
anyConstructed<PublicKeyCredentialEntry.Builder>()
.build()
} returns mockCredentialEntry1
every { anyConstructed<Intent>().setPackage(any()) } returns mockIntent1
every {
PendingIntent.getActivity(mockActivity, any(), mockIntent1, any())
} returns mockk()
return mockIntent1
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1794,7 +1794,11 @@ class VaultItemListingScreenTest : BaseComposeTest() {

@Test
fun `CompleteFido2GetCredentials event should call Fido2CompletionManager with result`() {
val result = Fido2GetCredentialsResult.Success(mockk(), mockk())
val result = Fido2GetCredentialsResult.Success(
userId = "mockUserId",
options = mockk(),
credentials = mockk(),
)
mutableEventFlow.tryEmit(VaultItemListingEvent.CompleteFido2GetCredentialsRequest(result))
verify {
fido2CompletionManager.completeFido2GetCredentialRequest(result)
Expand Down
Loading

0 comments on commit e6eb626

Please sign in to comment.