From c9dda48e26cca8634a9b4b742837d9231625db90 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Mon, 25 Oct 2021 18:29:18 +0200 Subject: [PATCH] Test that RelyingParty does not override per-request extension inputs --- .../data/RegistrationExtensionInputs.java | 18 ++- .../RelyingPartyStartOperationSpec.scala | 126 ++++++++++++++++-- .../com/yubico/webauthn/data/Generators.scala | 4 +- 3 files changed, 136 insertions(+), 12 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/RegistrationExtensionInputs.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/RegistrationExtensionInputs.java index ec7db8c40..0bcb3bd5a 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/RegistrationExtensionInputs.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/RegistrationExtensionInputs.java @@ -200,8 +200,22 @@ public RegistrationExtensionInputsBuilder credProps() { this.credProps = true; return this; } - /** For compatibility with {@link Builder}(toBuilder = true) */ - private RegistrationExtensionInputsBuilder credProps(Boolean credProps) { + + /** + * Enable or disable the Credential Properties (credProps) Extension. + * + *

A true argument enables the extension. A false argument disables + * the extension, and will not be overwritten by {@link + * RelyingParty#startRegistration(StartRegistrationOptions)}. A null argument disables the + * extension, and will be overwritten by {@link + * RelyingParty#startRegistration(StartRegistrationOptions)}. + * + * @see RelyingParty#startRegistration(StartRegistrationOptions) + * @see ยง10.4. + * Credential Properties Extension (credProps) + */ + public RegistrationExtensionInputsBuilder credProps(Boolean credProps) { this.credProps = credProps; return this; } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyStartOperationSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyStartOperationSpec.scala index c46c263ec..10dd8f359 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyStartOperationSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyStartOperationSpec.scala @@ -32,6 +32,7 @@ import com.yubico.webauthn.data.AuthenticatorAttachment import com.yubico.webauthn.data.AuthenticatorSelectionCriteria import com.yubico.webauthn.data.AuthenticatorTransport import com.yubico.webauthn.data.ByteArray +import com.yubico.webauthn.data.Generators.Extensions.registrationExtensionInputs import com.yubico.webauthn.data.Generators._ import com.yubico.webauthn.data.PublicKeyCredentialDescriptor import com.yubico.webauthn.data.PublicKeyCredentialParameters @@ -296,21 +297,84 @@ class RelyingPartyStartOperationSpec result.getExtensions.getAppidExclude.asScala should equal(None) } - it("by default always sets the credProps extension.") { - forAll { extensions: RegistrationExtensionInputs => - println(extensions.getExtensionIds) - println(extensions) - - val rp = relyingParty(userId = userId) + it("does not override the appidExclude extension with an empty value if already non-null in StartRegistrationOptions.") { + forAll { requestAppId: AppId => + val rp = relyingParty(appId = None, userId = userId) val result = rp.startRegistration( StartRegistrationOptions .builder() .user(userId) - .extensions(extensions) + .extensions( + RegistrationExtensionInputs + .builder() + .appidExclude(requestAppId) + .build() + ) .build() ) - result.getExtensions.getCredProps should be(true) + result.getExtensions.getAppidExclude.asScala should equal( + Some(requestAppId) + ) + } + } + + it("does not override the appidExclude extension if already non-null in StartRegistrationOptions.") { + forAll { (requestAppId: AppId, rpAppId: AppId) => + whenever(requestAppId != rpAppId) { + val rp = relyingParty(appId = Some(rpAppId), userId = userId) + val result = rp.startRegistration( + StartRegistrationOptions + .builder() + .user(userId) + .extensions( + RegistrationExtensionInputs + .builder() + .appidExclude(requestAppId) + .build() + ) + .build() + ) + + result.getExtensions.getAppidExclude.asScala should equal( + Some(requestAppId) + ) + } + } + } + + it("by default sets the credProps extension.") { + forAll(registrationExtensionInputs(credPropsGen = None)) { + extensions: RegistrationExtensionInputs => + println(extensions.getExtensionIds) + println(extensions) + + val rp = relyingParty(userId = userId) + val result = rp.startRegistration( + StartRegistrationOptions + .builder() + .user(userId) + .extensions(extensions) + .build() + ) + + result.getExtensions.getCredProps should be(true) + } + } + + it("does not override the credProps extension if explicitly set to false in StartRegistrationOptions.") { + forAll(registrationExtensionInputs(credPropsGen = Some(false))) { + extensions: RegistrationExtensionInputs => + val rp = relyingParty(userId = userId) + val result = rp.startRegistration( + StartRegistrationOptions + .builder() + .user(userId) + .extensions(extensions) + .build() + ) + + result.getExtensions.getCredProps should be(false) } } @@ -629,6 +693,52 @@ class RelyingPartyStartOperationSpec ) } + it("does not override the appid extension with an empty value if already non-null in StartAssertionOptions.") { + forAll { requestAppId: AppId => + val rp = relyingParty(appId = None, userId = userId) + val result = rp.startAssertion( + StartAssertionOptions + .builder() + .username(userId.getName) + .extensions( + AssertionExtensionInputs + .builder() + .appid(requestAppId) + .build() + ) + .build() + ) + + result.getPublicKeyCredentialRequestOptions.getExtensions.getAppid.asScala should equal( + Some(requestAppId) + ) + } + } + + it("does not override the appid extension if already non-null in StartAssertionOptions.") { + forAll { (requestAppId: AppId, rpAppId: AppId) => + whenever(requestAppId != rpAppId) { + val rp = relyingParty(appId = Some(rpAppId), userId = userId) + val result = rp.startAssertion( + StartAssertionOptions + .builder() + .username(userId.getName) + .extensions( + AssertionExtensionInputs + .builder() + .appid(requestAppId) + .build() + ) + .build() + ) + + result.getPublicKeyCredentialRequestOptions.getExtensions.getAppid.asScala should equal( + Some(requestAppId) + ) + } + } + } + it("allows setting the timeout to empty.") { val req = relyingParty(userId = userId).startAssertion( StartAssertionOptions diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/Generators.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/Generators.scala index 24e389811..3d7a1238c 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/Generators.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/Generators.scala @@ -346,7 +346,7 @@ object Generators { def registrationExtensionInputs( appidExcludeGen: Gen[Option[AppId]] = Gen.option(arbitrary[AppId]), - credPropsGen: Gen[Option[Boolean]] = Gen.option(true), + credPropsGen: Gen[Option[Boolean]] = Gen.option(arbitrary[Boolean]), largeBlobGen: Gen[ Option[com.yubico.webauthn.data.Extensions.LargeBlob.LargeBlobRegistrationInput] ] = Gen.option(LargeBlob.largeBlobRegistrationInput), @@ -360,7 +360,7 @@ object Generators { } yield { val b = RegistrationExtensionInputs.builder() appidExclude.foreach({ i => b.appidExclude(i) }) - if (credProps.contains(true)) { b.credProps() } + credProps.foreach({ i => b.credProps(i) }) largeBlob.foreach({ i => b.largeBlob(i) }) if (uvm.contains(true)) { b.uvm() } val result = b.build()