From a15d5dbf9e24cc27c9b626db4bd74b5f0d7c0c86 Mon Sep 17 00:00:00 2001 From: Pavel Solovev Date: Sun, 6 Sep 2026 01:22:10 +0300 Subject: [PATCH] Match the WebAuthn spec when parsing passkey creation options PasskeyAttestationOptions differs from PublicKeyCredentialCreationOptionsJSON in four places. One breaks registration, the other three lose data quietly. authenticatorSelection is optional in the spec but was declared non-nullable with no default, so a request without it failed to deserialize with MissingFieldException. Registration then ended in MissingHostUrl for an unprivileged caller and InternalError for a privileged one. The remaining three are name mismatches, and they matter beyond parsing: registerFido2CredentialInternal re-serializes this model and sends the result to the SDK, so anything the model cannot name never reaches the authenticator. excludedCredentials -> excludeCredentials, so the relying party's exclusion list is no longer replaced by an empty one and duplicate passkeys can be prevented again cross_platform -> cross-platform, so the attachment survives instead of being coerced to null ResidentKeyRequirement gains discouraged, which the spec defines and which was being coerced to null the same way The fixtures in this repo already used the spec spelling excludeCredentials with empty arrays, which is why none of this showed up in the tests. --- .../model/PasskeyAttestationOptions.kt | 16 +- .../model/PasskeyAttestationOptionsTest.kt | 197 ++++++++++++++++++ 2 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 app/src/test/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptionsTest.kt diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptions.kt index 62a364410f7..93cac923d4a 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptions.kt @@ -4,15 +4,17 @@ import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable /** - * Models FIDO 2 credential creation request options received from a Relying Party (RP). + * Models FIDO 2 credential creation request options received from a Relying Party (RP), + * based off the spec found at: + * https://www.w3.org/TR/webauthn-2/#dictionary-makecredentialoptions */ @Serializable data class PasskeyAttestationOptions( @SerialName("authenticatorSelection") - val authenticatorSelection: AuthenticatorSelectionCriteria, + val authenticatorSelection: AuthenticatorSelectionCriteria? = null, @SerialName("challenge") val challenge: String, - @SerialName("excludedCredentials") + @SerialName("excludeCredentials") val excludeCredentials: List = emptyList(), @SerialName("pubKeyCredParams") val pubKeyCredParams: List, @@ -42,7 +44,7 @@ data class PasskeyAttestationOptions( @SerialName("platform") PLATFORM, - @SerialName("cross_platform") + @SerialName("cross-platform") CROSS_PLATFORM, } @@ -51,6 +53,12 @@ data class PasskeyAttestationOptions( */ @Serializable enum class ResidentKeyRequirement { + /** + * Resident keys are not preferred during selection. + */ + @SerialName("discouraged") + DISCOURAGED, + /** * Resident keys are preferred during selection, if supported. */ diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptionsTest.kt new file mode 100644 index 00000000000..1438c51661e --- /dev/null +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/credentials/model/PasskeyAttestationOptionsTest.kt @@ -0,0 +1,197 @@ +package com.x8bit.bitwarden.data.credentials.model + +import com.bitwarden.core.data.util.decodeFromStringOrNull +import com.bitwarden.core.di.CoreModule +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertNotNull + +class PasskeyAttestationOptionsTest { + + private val json = CoreModule.providesJson(buildInfoManager = mockk(relaxed = true)) + + @Test + fun `options without authenticatorSelection should be deserialized`() { + val result = json.decodeFromStringOrNull( + OPTIONS_WITHOUT_AUTHENTICATOR_SELECTION_JSON, + ) + + assertNotNull(result) + assertEquals(RELYING_PARTY_ID, result.relyingParty.id) + } + + @Test + fun `excludeCredentials should be deserialized from the spec member name`() { + val result = json.decodeFromStringOrNull(OPTIONS_JSON) + + assertNotNull(result) + assertEquals( + listOf(EXCLUDED_CREDENTIAL_ID), + result.excludeCredentials.map { it.id }, + ) + } + + @Test + fun `excludeCredentials should be serialized with the spec member name`() { + val options = createOptions( + excludeCredentials = listOf( + PublicKeyCredentialDescriptor( + type = "public-key", + id = EXCLUDED_CREDENTIAL_ID, + transports = null, + ), + ), + ) + + val result = json.encodeToString(options) + + assertTrue(result.contains("\"excludeCredentials\"")) + assertFalse(result.contains("\"excludedCredentials\"")) + } + + @Test + fun `cross-platform attachment should be deserialized from the spec value`() { + val result = json.decodeFromStringOrNull(OPTIONS_JSON) + + assertNotNull(result) + assertEquals( + PasskeyAttestationOptions + .AuthenticatorSelectionCriteria + .AuthenticatorAttachment + .CROSS_PLATFORM, + result.authenticatorSelection?.authenticatorAttachment, + ) + } + + @Test + fun `cross-platform attachment should be serialized with the spec value`() { + val options = createOptions( + authenticatorSelection = PasskeyAttestationOptions.AuthenticatorSelectionCriteria( + authenticatorAttachment = PasskeyAttestationOptions + .AuthenticatorSelectionCriteria + .AuthenticatorAttachment + .CROSS_PLATFORM, + ), + ) + + val result = json.encodeToString(options) + + assertTrue(result.contains("\"cross-platform\"")) + assertFalse(result.contains("\"cross_platform\"")) + } + + @Test + fun `discouraged residentKey should be deserialized from the spec value`() { + val result = json.decodeFromStringOrNull(OPTIONS_JSON) + + assertNotNull(result) + assertEquals( + PasskeyAttestationOptions + .AuthenticatorSelectionCriteria + .ResidentKeyRequirement + .DISCOURAGED, + result.authenticatorSelection?.residentKeyRequirement, + ) + } + + @Test + fun `discouraged residentKey should be serialized with the spec value`() { + val options = createOptions( + authenticatorSelection = PasskeyAttestationOptions.AuthenticatorSelectionCriteria( + residentKeyRequirement = PasskeyAttestationOptions + .AuthenticatorSelectionCriteria + .ResidentKeyRequirement + .DISCOURAGED, + ), + ) + + val result = json.encodeToString(options) + + assertTrue(result.contains("\"discouraged\"")) + } +} + +private const val RELYING_PARTY_ID = "www.bitwarden.com" +private const val EXCLUDED_CREDENTIAL_ID = "mockCredentialId" + +private fun createOptions( + authenticatorSelection: PasskeyAttestationOptions.AuthenticatorSelectionCriteria? = null, + excludeCredentials: List = emptyList(), +): PasskeyAttestationOptions = PasskeyAttestationOptions( + authenticatorSelection = authenticatorSelection, + challenge = "tZ1rLJ_paLC8IMmg", + excludeCredentials = excludeCredentials, + pubKeyCredParams = listOf( + PasskeyAttestationOptions.PublicKeyCredentialParameters( + type = "public-key", + alg = -7.0, + ), + ), + relyingParty = PasskeyAttestationOptions.PublicKeyCredentialRpEntity( + id = RELYING_PARTY_ID, + name = "mockRpName", + ), + user = PasskeyAttestationOptions.PublicKeyCredentialUserEntity( + id = "UmhpTE9NOUY", + name = "mockUserName", + displayName = "mockDisplayName", + ), +) + +private val OPTIONS_JSON = """ +{ + "authenticatorSelection": { + "authenticatorAttachment": "cross-platform", + "residentKey": "discouraged", + "userVerification": "preferred" + }, + "challenge": "tZ1rLJ_paLC8IMmg", + "excludeCredentials": [ + { + "type": "public-key", + "id": "$EXCLUDED_CREDENTIAL_ID" + } + ], + "pubKeyCredParams": [ + { + "alg": -7, + "type": "public-key" + } + ], + "rp": { + "id": "$RELYING_PARTY_ID", + "name": "mockRpName" + }, + "user": { + "displayName": "mockDisplayName", + "id": "UmhpTE9NOUY", + "name": "mockUserName" + } +} +""" + .trimIndent() + +private val OPTIONS_WITHOUT_AUTHENTICATOR_SELECTION_JSON = """ +{ + "challenge": "tZ1rLJ_paLC8IMmg", + "pubKeyCredParams": [ + { + "alg": -7, + "type": "public-key" + } + ], + "rp": { + "id": "$RELYING_PARTY_ID", + "name": "mockRpName" + }, + "user": { + "displayName": "mockDisplayName", + "id": "UmhpTE9NOUY", + "name": "mockUserName" + } +} +""" + .trimIndent()