Skip to content

Commit c672bff

Browse files
SaintPatrckSaintPatrck
authored
[PM-17694] Only update FIDO2 user verification status during single-tap sign-in (#4680)
1 parent 7ab5972 commit c672bff

File tree

4 files changed

+123
-15
lines changed

4 files changed

+123
-15
lines changed

‎app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,8 @@ class MainViewModel @Inject constructor(
339339
// Set the user's verification status when a new FIDO 2 request is received to force
340340
// explicit verification if the user's vault is unlocked when the request is
341341
// received.
342-
fido2CredentialManager.isUserVerified =
343-
fido2CreateCredentialRequestData.isUserVerified
344-
?: fido2CredentialManager.isUserVerified
342+
fido2CreateCredentialRequestData.isUserVerified
343+
?.let { isVerified -> fido2CredentialManager.isUserVerified = isVerified }
345344
specialCircumstanceManager.specialCircumstance =
346345
SpecialCircumstance.Fido2Save(
347346
fido2CreateCredentialRequest = fido2CreateCredentialRequestData,
@@ -356,9 +355,11 @@ class MainViewModel @Inject constructor(
356355
}
357356

358357
fido2CredentialAssertionRequest != null -> {
359-
fido2CredentialManager.isUserVerified =
360-
fido2CredentialAssertionRequest.isUserVerified
361-
?: false
358+
// If device biometric verification was performed as part of single-tap
359+
// authentication, set the user's verification state to the device result.
360+
// Otherwise, retain the verification state as-is.
361+
fido2CredentialAssertionRequest.isUserVerified
362+
?.let { isVerified -> fido2CredentialManager.isUserVerified = isVerified }
362363
specialCircumstanceManager.specialCircumstance =
363364
SpecialCircumstance.Fido2Assertion(
364365
fido2AssertionRequest = fido2CredentialAssertionRequest,

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequest.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ import kotlinx.parcelize.Parcelize
77

88
/**
99
* Models a FIDO 2 credential authentication request parsed from the launching intent.
10+
*
11+
* @param userId The ID of the Bitwarden user to authenticate.
12+
* @param cipherId The ID of the cipher that contains the passkey to authenticate.
13+
* @param credentialId The ID of the credential to authenticate.
14+
* @param requestJson The JSON representation of the FIDO 2 request.
15+
* @param clientDataHash The hash of the client data.
16+
* @param packageName The package name of the calling app.
17+
* @param signingInfo The signing info of the calling app.
18+
* @param origin The origin of the calling app. Only populated if the calling application is a
19+
* privileged application. I.e., a web browser.
20+
* @param isUserVerified Whether the user has been verified prior to receiving this request. Only
21+
* populated if device biometric verification was performed. If null, the application is responsible
22+
* for prompting user verification when it is deemed necessary.
1023
*/
1124
@Parcelize
1225
data class Fido2CredentialAssertionRequest(

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtils.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fun Intent.getFido2CreateCredentialRequestOrNull(): Fido2CreateCredentialRequest
3939
packageName = systemRequest.callingAppInfo.packageName,
4040
signingInfo = systemRequest.callingAppInfo.signingInfo,
4141
origin = systemRequest.callingAppInfo.origin,
42-
isUserVerified = systemRequest.biometricPromptResult?.isSuccessful ?: false,
42+
isUserVerified = systemRequest.biometricPromptResult?.isSuccessful,
4343
)
4444
}
4545

@@ -69,7 +69,6 @@ fun Intent.getFido2AssertionRequestOrNull(): Fido2CredentialAssertionRequest? {
6969
?: return null
7070

7171
val isUserVerified = systemRequest.biometricPromptResult?.isSuccessful
72-
?: false
7372

7473
return Fido2CredentialAssertionRequest(
7574
userId = userId,

‎app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtilsTest.kt

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ class Fido2IntentUtilsTest {
4949
unmockkObject(PendingIntentHandler.Companion)
5050
}
5151

52+
@Suppress("MaxLineLength")
5253
@Test
53-
fun `getFido2CredentialRequestOrNull should return Fido2CredentialRequest when present`() {
54+
fun `getFido2CreateCredentialRequestOrNull should return Fido2CreateCredentialRequest when present`() {
5455
val intent = mockk<Intent> {
5556
every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId"
5657
}
@@ -83,14 +84,59 @@ class Fido2IntentUtilsTest {
8384
packageName = mockCallingAppInfo.packageName,
8485
signingInfo = mockCallingAppInfo.signingInfo,
8586
origin = mockCallingAppInfo.origin,
86-
isUserVerified = false,
87+
isUserVerified = null,
8788
),
8889
createRequest,
8990
)
9091
}
9192

93+
@Suppress("MaxLineLength")
9294
@Test
93-
fun `getFido2CredentialRequestOrNull should return null when build version is below 34`() {
95+
fun `getFido2CreateCredentialRequestOrNull should set isUserVerified when biometric prompt result is present`() {
96+
val intent = mockk<Intent> {
97+
every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId"
98+
}
99+
val mockCallingRequest = mockk<CreatePublicKeyCredentialRequest> {
100+
every { requestJson } returns "requestJson"
101+
every { clientDataHash } returns byteArrayOf(0)
102+
every { preferImmediatelyAvailableCredentials } returns false
103+
every { origin } returns "mockOrigin"
104+
every { isAutoSelectAllowed } returns true
105+
}
106+
val mockCallingAppInfo = CallingAppInfo(
107+
packageName = "mockPackageName",
108+
signingInfo = SigningInfo(),
109+
origin = "mockOrigin",
110+
)
111+
val mockProviderRequest = ProviderCreateCredentialRequest(
112+
callingRequest = mockCallingRequest,
113+
callingAppInfo = mockCallingAppInfo,
114+
biometricPromptResult = mockk {
115+
every { isSuccessful } returns true
116+
},
117+
)
118+
119+
every {
120+
PendingIntentHandler.retrieveProviderCreateCredentialRequest(intent)
121+
} returns mockProviderRequest
122+
123+
val createRequest = intent.getFido2CreateCredentialRequestOrNull()
124+
assertEquals(
125+
Fido2CreateCredentialRequest(
126+
userId = "mockUserId",
127+
requestJson = mockCallingRequest.requestJson,
128+
packageName = mockCallingAppInfo.packageName,
129+
signingInfo = mockCallingAppInfo.signingInfo,
130+
origin = mockCallingAppInfo.origin,
131+
isUserVerified = true,
132+
),
133+
createRequest,
134+
)
135+
}
136+
137+
@Suppress("MaxLineLength")
138+
@Test
139+
fun `getFido2CreateCredentialRequestOrNull should return null when build version is below 34`() {
94140
val intent = mockk<Intent>()
95141

96142
every { isBuildVersionBelow(34) } returns true
@@ -100,7 +146,7 @@ class Fido2IntentUtilsTest {
100146

101147
@Suppress("MaxLineLength")
102148
@Test
103-
fun `getFido2CredentialRequestOrNull should return null when intent is not a provider create credential request`() {
149+
fun `getFido2CreateCredentialRequestOrNull should return null when intent is not a provider create credential request`() {
104150
val intent = mockk<Intent>()
105151

106152
every {
@@ -112,7 +158,7 @@ class Fido2IntentUtilsTest {
112158

113159
@Suppress("MaxLineLength")
114160
@Test
115-
fun `getFido2CredentialRequestOrNull should return null when calling request is not a public key credential create request`() {
161+
fun `getFido2CreateCredentialRequestOrNull should return null when calling request is not a public key credential create request`() {
116162
val intent = mockk<Intent>()
117163
val mockCallingRequest = mockk<CreatePasswordRequest>()
118164
val mockCallingAppInfo = CallingAppInfo(
@@ -133,7 +179,7 @@ class Fido2IntentUtilsTest {
133179

134180
@Suppress("MaxLineLength")
135181
@Test
136-
fun `getFido2CredentialRequestOrNull should return null when user id is not present in extras`() {
182+
fun `getFido2CreateCredentialRequestOrNull should return null when user id is not present in extras`() {
137183
val intent = mockk<Intent> {
138184
every { getStringExtra(EXTRA_KEY_USER_ID) } returns null
139185
}
@@ -200,7 +246,56 @@ class Fido2IntentUtilsTest {
200246
packageName = mockCallingAppInfo.packageName,
201247
signingInfo = mockCallingAppInfo.signingInfo,
202248
origin = mockCallingAppInfo.origin,
203-
isUserVerified = false,
249+
isUserVerified = null,
250+
),
251+
assertionRequest,
252+
)
253+
}
254+
255+
@Suppress("MaxLineLength")
256+
@Test
257+
fun `getFido2AssertionRequestOrNull should set isUserVerified when biometric prompt result is present`() {
258+
val intent = mockk<Intent> {
259+
every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId"
260+
every { getStringExtra(EXTRA_KEY_CIPHER_ID) } returns "mockCipherId"
261+
every { getStringExtra(EXTRA_KEY_CREDENTIAL_ID) } returns "mockCredentialId"
262+
}
263+
val mockOption = GetPublicKeyCredentialOption(
264+
requestJson = "requestJson",
265+
clientDataHash = byteArrayOf(0),
266+
allowedProviders = emptySet(),
267+
)
268+
val mockCallingAppInfo = CallingAppInfo(
269+
packageName = "mockPackageName",
270+
signingInfo = SigningInfo(),
271+
origin = "mockOrigin",
272+
)
273+
val mockProviderGetCredentialRequest = ProviderGetCredentialRequest(
274+
credentialOptions = listOf(mockOption),
275+
callingAppInfo = mockCallingAppInfo,
276+
biometricPromptResult = mockk {
277+
every { isSuccessful } returns true
278+
},
279+
)
280+
281+
every {
282+
PendingIntentHandler.retrieveProviderGetCredentialRequest(intent)
283+
} returns mockProviderGetCredentialRequest
284+
285+
val assertionRequest = intent.getFido2AssertionRequestOrNull()
286+
287+
assertNotNull(assertionRequest)
288+
assertEquals(
289+
Fido2CredentialAssertionRequest(
290+
userId = "mockUserId",
291+
cipherId = "mockCipherId",
292+
credentialId = "mockCredentialId",
293+
requestJson = mockOption.requestJson,
294+
clientDataHash = mockOption.clientDataHash,
295+
packageName = mockCallingAppInfo.packageName,
296+
signingInfo = mockCallingAppInfo.signingInfo,
297+
origin = mockCallingAppInfo.origin,
298+
isUserVerified = true,
204299
),
205300
assertionRequest,
206301
)

0 commit comments

Comments
 (0)