From c81c9a8841c70854b385f2bf8e4f97bbb0778b09 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Thu, 31 Mar 2022 11:52:50 +0200 Subject: [PATCH 1/2] Fix NoSuchElementException when username and user handle are both absent --- NEWS | 3 +++ .../yubico/webauthn/FinishAssertionSteps.java | 13 ++++++---- .../webauthn/RelyingPartyAssertionSpec.scala | 26 ++++++++++++++----- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index 85e47c2c0..7a8410a81 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,9 @@ Fixes: `"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/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java index 4f5ba22a9..04024cad7 100644 --- 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,7 +124,8 @@ class Step0 implements Step { .getUserHandle() .map(Optional::of) .orElseGet( - () -> credentialRepository.getUserHandleForUsername(request.getUsername().get())); + () -> + request.getUsername().flatMap(credentialRepository::getUserHandleForUsername)); private final Optional username = request @@ -132,8 +133,10 @@ class Step0 implements Step { .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 index b5ed3ff7f..e65eb6ffe 100644 --- 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 From 5b5f65f5ae839128429e09be161a1e3a6b457bda Mon Sep 17 00:00:00 2001 From: Coleb Mujurizi Date: Wed, 6 Apr 2022 18:21:20 +0200 Subject: [PATCH 2/2] tests: remove deprecated scan.yml workflow --- .github/workflows/scan.yml | 37 ------------------------------------- 1 file changed, 37 deletions(-) delete mode 100644 .github/workflows/scan.yml diff --git a/.github/workflows/scan.yml b/.github/workflows/scan.yml deleted file mode 100644 index 286857edd..000000000 --- a/.github/workflows/scan.yml +++ /dev/null @@ -1,37 +0,0 @@ -name: static code analysis -# Documentation: https://github.com/Yubico/yes-static-code-analysis - -on: - push: - schedule: - - cron: '0 0 * * 1' - -env: - SCAN_IMG: - yubico-yes-docker-local.jfrog.io/static-code-analysis/java:v1 - SECRET: ${{ secrets.ARTIFACTORY_READER_TOKEN }} - -jobs: - build: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@master - - - name: Scan and fail on warnings - run: | - if [ "${SECRET}" != "" ]; then - docker login yubico-yes-docker-local.jfrog.io/ \ - -u svc-static-code-analysis-reader -p ${SECRET} - docker pull ${SCAN_IMG} - docker run -v${PWD}:/k \ - -e PROJECT_NAME=${GITHUB_REPOSITORY#Yubico/} -t ${SCAN_IMG} - else - echo "No docker registry credentials, not scanning" - fi - - - uses: actions/upload-artifact@master - if: failure() - with: - name: suppression_files - path: suppression_files