Merge branch 'master' into release-1.12.4

Yubico · Apr 15, 2022 · 5e25a02 · 5e25a02
2 parents 9ce1422 + fb01b31
commit 5e25a02
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 51 deletions.
diff --git a/.github/workflows/scan.yml b/.github/workflows/scan.yml
diff --git a/NEWS b/NEWS
@@ -12,14 +12,17 @@ Deprecated features:
   have been part of the public API to begin with.
 
 
-== Version 1.12.3 (unreleased) ==
+== Version 1.12.3 ==
 
 Fixes:
 
 * Fixed `PublicKeyCredential` failing to parse from JSON if an
   `"authenticatorAttachment"` attribute was present.
 * Bumped Jackson dependency to version [2.13.2.1,3) in response to
   CVE-2020-36518
+* Fixed bug in `RelyingParty.finishAssertion` that would throw a nondescript
+  `NoSuchElementException` if username and user handle are both absent, instead
+  of an `IllegalArgumentException` with a better error message.
 
 
 == Version 1.12.2 ==

diff --git a/README b/README
@@ -25,15 +25,15 @@ Maven:
 <dependency>
   <groupId>com.yubico</groupId>
   <artifactId>webauthn-server-core</artifactId>
-  <version>1.12.2</version>
+  <version>1.12.3</version>
   <scope>compile</scope>
 </dependency>
 ----------
 
 Gradle:
 
 ----------
-compile 'com.yubico:webauthn-server-core:1.12.2'
+compile 'com.yubico:webauthn-server-core:1.12.3'
 ----------
 
 === Semantic versioning

diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java
@@ -124,16 +124,19 @@ class Step0 implements Step<Step1> {
             .getUserHandle()
             .map(Optional::of)
             .orElseGet(
-                () -> credentialRepository.getUserHandleForUsername(request.getUsername().get()));
+                () ->
+                    request.getUsername().flatMap(credentialRepository::getUserHandleForUsername));
 
     private final Optional<String> username =
         request
             .getUsername()
             .map(Optional::of)
             .orElseGet(
                 () ->
-                    credentialRepository.getUsernameForUserHandle(
-                        response.getResponse().getUserHandle().get()));
+                    response
+                        .getResponse()
+                        .getUserHandle()
+                        .flatMap(credentialRepository::getUsernameForUserHandle));
 
     @Override
     public Step1 nextStep() {
@@ -147,12 +150,12 @@ public void validate() {
           "At least one of username and user handle must be given; none was.");
       assure(
           userHandle.isPresent(),
-          "No user found for username: %s, userHandle: %s",
+          "User handle not found for username: %s",
           request.getUsername(),
           response.getResponse().getUserHandle());
       assure(
           username.isPresent(),
-          "No user found for username: %s, userHandle: %s",
+          "Username not found for userHandle: %s",
           request.getUsername(),
           response.getResponse().getUserHandle());
     }

diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala
@@ -179,7 +179,7 @@ class RelyingPartyAssertionSpec
         Defaults.requestedExtensions,
       rpId: RelyingPartyIdentity = Defaults.rpId,
       signature: ByteArray = Defaults.signature,
-      userHandleForResponse: ByteArray = Defaults.userHandle,
+      userHandleForResponse: Option[ByteArray] = Some(Defaults.userHandle),
       userHandleForUser: ByteArray = Defaults.userHandle,
       usernameForRequest: Option[String] = Some(Defaults.username),
       usernameForUser: String = Defaults.username,
@@ -220,7 +220,7 @@ class RelyingPartyAssertionSpec
             if (clientDataJsonBytes == null) null else clientDataJsonBytes
           )
           .signature(if (signature == null) null else signature)
-          .userHandle(userHandleForResponse)
+          .userHandle(userHandleForResponse.asJava)
           .build()
       )
       .clientExtensionResults(clientExtensionResults)
@@ -523,7 +523,7 @@ class RelyingPartyAssertionSpec
                 credentialRepository = credentialRepository,
                 usernameForRequest = Some(owner.username),
                 userHandleForUser = owner.userHandle,
-                userHandleForResponse = nonOwner.userHandle,
+                userHandleForResponse = Some(nonOwner.userHandle),
               )
               val step: FinishAssertionSteps#Step2 = steps.begin.next.next
 
@@ -537,7 +537,7 @@ class RelyingPartyAssertionSpec
                 credentialRepository = credentialRepository,
                 usernameForRequest = Some(owner.username),
                 userHandleForUser = owner.userHandle,
-                userHandleForResponse = owner.userHandle,
+                userHandleForResponse = Some(owner.userHandle),
               )
               val step: FinishAssertionSteps#Step2 = steps.begin.next.next
 
@@ -554,7 +554,7 @@ class RelyingPartyAssertionSpec
                 credentialRepository = credentialRepository,
                 usernameForRequest = None,
                 userHandleForUser = owner.userHandle,
-                userHandleForResponse = nonOwner.userHandle,
+                userHandleForResponse = Some(nonOwner.userHandle),
               )
               val step: FinishAssertionSteps#Step2 = steps.begin.next.next
 
@@ -563,12 +563,26 @@ class RelyingPartyAssertionSpec
               step.tryNext shouldBe a[Failure[_]]
             }
 
+            it("Fails if neither username nor user handle is given.") {
+              val steps = finishAssertion(
+                credentialRepository = credentialRepository,
+                usernameForRequest = None,
+                userHandleForUser = owner.userHandle,
+                userHandleForResponse = None,
+              )
+              val step: FinishAssertionSteps#Step0 = steps.begin
+
+              step.validations shouldBe a[Failure[_]]
+              step.validations.failed.get shouldBe an[IllegalArgumentException]
+              step.tryNext shouldBe a[Failure[_]]
+            }
+
             it("Succeeds if credential ID is owned by the given user handle.") {
               val steps = finishAssertion(
                 credentialRepository = credentialRepository,
                 usernameForRequest = None,
                 userHandleForUser = owner.userHandle,
-                userHandleForResponse = owner.userHandle,
+                userHandleForResponse = Some(owner.userHandle),
               )
               val step: FinishAssertionSteps#Step2 = steps.begin.next.next