Skip to content

Commit c81c9a8

Browse files
committed
Fix NoSuchElementException when username and user handle are both absent
1 parent a30239d commit c81c9a8

File tree

3 files changed

+31
-11
lines changed

3 files changed

+31
-11
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ Fixes:
66
`"authenticatorAttachment"` attribute was present.
77
* Bumped Jackson dependency to version [2.13.2.1,3) in response to
88
CVE-2020-36518
9+
* Fixed bug in `RelyingParty.finishAssertion` that would throw a nondescript
10+
`NoSuchElementException` if username and user handle are both absent, instead
11+
of an `IllegalArgumentException` with a better error message.
912

1013

1114
== Version 1.12.2 ==

webauthn-server-core/src/main/java/com/yubico/webauthn/FinishAssertionSteps.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,19 @@ class Step0 implements Step<Step1> {
124124
.getUserHandle()
125125
.map(Optional::of)
126126
.orElseGet(
127-
() -> credentialRepository.getUserHandleForUsername(request.getUsername().get()));
127+
() ->
128+
request.getUsername().flatMap(credentialRepository::getUserHandleForUsername));
128129

129130
private final Optional<String> username =
130131
request
131132
.getUsername()
132133
.map(Optional::of)
133134
.orElseGet(
134135
() ->
135-
credentialRepository.getUsernameForUserHandle(
136-
response.getResponse().getUserHandle().get()));
136+
response
137+
.getResponse()
138+
.getUserHandle()
139+
.flatMap(credentialRepository::getUsernameForUserHandle));
137140

138141
@Override
139142
public Step1 nextStep() {
@@ -147,12 +150,12 @@ public void validate() {
147150
"At least one of username and user handle must be given; none was.");
148151
assure(
149152
userHandle.isPresent(),
150-
"No user found for username: %s, userHandle: %s",
153+
"User handle not found for username: %s",
151154
request.getUsername(),
152155
response.getResponse().getUserHandle());
153156
assure(
154157
username.isPresent(),
155-
"No user found for username: %s, userHandle: %s",
158+
"Username not found for userHandle: %s",
156159
request.getUsername(),
157160
response.getResponse().getUserHandle());
158161
}

webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyAssertionSpec.scala

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class RelyingPartyAssertionSpec
179179
Defaults.requestedExtensions,
180180
rpId: RelyingPartyIdentity = Defaults.rpId,
181181
signature: ByteArray = Defaults.signature,
182-
userHandleForResponse: ByteArray = Defaults.userHandle,
182+
userHandleForResponse: Option[ByteArray] = Some(Defaults.userHandle),
183183
userHandleForUser: ByteArray = Defaults.userHandle,
184184
usernameForRequest: Option[String] = Some(Defaults.username),
185185
usernameForUser: String = Defaults.username,
@@ -220,7 +220,7 @@ class RelyingPartyAssertionSpec
220220
if (clientDataJsonBytes == null) null else clientDataJsonBytes
221221
)
222222
.signature(if (signature == null) null else signature)
223-
.userHandle(userHandleForResponse)
223+
.userHandle(userHandleForResponse.asJava)
224224
.build()
225225
)
226226
.clientExtensionResults(clientExtensionResults)
@@ -523,7 +523,7 @@ class RelyingPartyAssertionSpec
523523
credentialRepository = credentialRepository,
524524
usernameForRequest = Some(owner.username),
525525
userHandleForUser = owner.userHandle,
526-
userHandleForResponse = nonOwner.userHandle,
526+
userHandleForResponse = Some(nonOwner.userHandle),
527527
)
528528
val step: FinishAssertionSteps#Step2 = steps.begin.next.next
529529

@@ -537,7 +537,7 @@ class RelyingPartyAssertionSpec
537537
credentialRepository = credentialRepository,
538538
usernameForRequest = Some(owner.username),
539539
userHandleForUser = owner.userHandle,
540-
userHandleForResponse = owner.userHandle,
540+
userHandleForResponse = Some(owner.userHandle),
541541
)
542542
val step: FinishAssertionSteps#Step2 = steps.begin.next.next
543543

@@ -554,7 +554,7 @@ class RelyingPartyAssertionSpec
554554
credentialRepository = credentialRepository,
555555
usernameForRequest = None,
556556
userHandleForUser = owner.userHandle,
557-
userHandleForResponse = nonOwner.userHandle,
557+
userHandleForResponse = Some(nonOwner.userHandle),
558558
)
559559
val step: FinishAssertionSteps#Step2 = steps.begin.next.next
560560

@@ -563,12 +563,26 @@ class RelyingPartyAssertionSpec
563563
step.tryNext shouldBe a[Failure[_]]
564564
}
565565

566+
it("Fails if neither username nor user handle is given.") {
567+
val steps = finishAssertion(
568+
credentialRepository = credentialRepository,
569+
usernameForRequest = None,
570+
userHandleForUser = owner.userHandle,
571+
userHandleForResponse = None,
572+
)
573+
val step: FinishAssertionSteps#Step0 = steps.begin
574+
575+
step.validations shouldBe a[Failure[_]]
576+
step.validations.failed.get shouldBe an[IllegalArgumentException]
577+
step.tryNext shouldBe a[Failure[_]]
578+
}
579+
566580
it("Succeeds if credential ID is owned by the given user handle.") {
567581
val steps = finishAssertion(
568582
credentialRepository = credentialRepository,
569583
usernameForRequest = None,
570584
userHandleForUser = owner.userHandle,
571-
userHandleForResponse = owner.userHandle,
585+
userHandleForResponse = Some(owner.userHandle),
572586
)
573587
val step: FinishAssertionSteps#Step2 = steps.begin.next.next
574588

0 commit comments

Comments
 (0)