Test that RelyingParty does not override per-request extension inputs

Yubico · Oct 26, 2021 · c9dda48 · c9dda48
1 parent 8d50db5
commit c9dda48
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 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
@@ -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 (<code>credProps</code>) Extension.
+     *
+     * <p>A <code>true</code> argument enables the extension. A <code>false</code> 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 <a
+     *     href="https://www.w3.org/TR/2021/REC-webauthn-2-20210408/#sctn-authenticator-credential-properties-extension">§10.4.
+     *     Credential Properties Extension (credProps)</a>
+     */
+    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
@@ -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
@@ -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()