From 15e9108ccd9e04c7ad729254b7b52bb170696d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Gon=C3=A7alves?= Date: Fri, 16 Dec 2022 08:12:24 +0000 Subject: [PATCH 1/4] Support authenticatorAttachment in PublicKeyCredential --- .../com/yubico/webauthn/AssertionResult.java | 14 +++++++++ .../yubico/webauthn/RegistrationResult.java | 14 +++++++++ .../webauthn/data/PublicKeyCredential.java | 22 +++++++++++-- .../webauthn/RelyingPartyAssertionSpec.scala | 31 +++++++++++++++++++ .../RelyingPartyRegistrationSpec.scala | 28 +++++++++++++++++ 5 files changed, 106 insertions(+), 3 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java index b42ff37f9..f12297400 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.yubico.webauthn.data.AuthenticatorAssertionExtensionOutputs; import com.yubico.webauthn.data.AuthenticatorAssertionResponse; +import com.yubico.webauthn.data.AuthenticatorAttachment; import com.yubico.webauthn.data.AuthenticatorData; import com.yubico.webauthn.data.ByteArray; import com.yubico.webauthn.data.ClientAssertionExtensionOutputs; @@ -195,6 +196,19 @@ public boolean isBackedUp() { return credentialResponse.getResponse().getParsedAuthenticatorData().getFlags().BS; } + /** + * The authenticator + * attachment modality in effect at the time the asserted credential was used. + * + * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as + * the standard matures. + */ + @Deprecated + @JsonIgnore + public Optional getAuthenticatorAttachment() { + return credentialResponse.getAuthenticatorAttachment(); + } + /** * The new signature * count of the credential used for the assertion. diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java index 88bbcfdbb..0e6d24c56 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java @@ -31,6 +31,7 @@ import com.yubico.webauthn.RelyingParty.RelyingPartyBuilder; import com.yubico.webauthn.attestation.AttestationTrustSource; import com.yubico.webauthn.data.AttestationType; +import com.yubico.webauthn.data.AuthenticatorAttachment; import com.yubico.webauthn.data.AuthenticatorAttestationResponse; import com.yubico.webauthn.data.AuthenticatorRegistrationExtensionOutputs; import com.yubico.webauthn.data.ByteArray; @@ -175,6 +176,19 @@ public boolean isBackedUp() { return credential.getResponse().getParsedAuthenticatorData().getFlags().BS; } + /** + * The authenticator + * attachment modality in effect at the time the credential was created. + * + * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as + * the standard matures. + */ + @Deprecated + @JsonIgnore + public Optional getAuthenticatorAttachment() { + return credential.getAuthenticatorAttachment(); + } + /** * The signature count returned with the created credential. * diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java index 3141912ca..fa34e10f6 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java @@ -25,11 +25,11 @@ package com.yubico.webauthn.data; import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.type.TypeReference; import com.yubico.internal.util.JacksonCodecs; import java.io.IOException; +import java.util.Optional; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.NonNull; @@ -46,7 +46,6 @@ */ @Value @Builder(toBuilder = true) -@JsonIgnoreProperties({"authenticatorAttachment"}) public class PublicKeyCredential< A extends AuthenticatorResponse, B extends ClientExtensionOutputs> { @@ -68,6 +67,8 @@ public class PublicKeyCredential< */ @NonNull private final A response; + private final AuthenticatorAttachment authenticatorAttachment; + /** * A map containing extension identifier → client extension output entries produced by the * extension’s client extension processing. @@ -83,6 +84,7 @@ private PublicKeyCredential( @JsonProperty("id") ByteArray id, @JsonProperty("rawId") ByteArray rawId, @NonNull @JsonProperty("response") A response, + @JsonProperty("authenticatorAttachment") AuthenticatorAttachment authenticatorAttachment, @NonNull @JsonProperty("clientExtensionResults") B clientExtensionResults, @NonNull @JsonProperty("type") PublicKeyCredentialType type) { if (id == null && rawId == null) { @@ -95,6 +97,7 @@ private PublicKeyCredential( this.id = id == null ? rawId : id; this.response = response; + this.authenticatorAttachment = authenticatorAttachment; this.clientExtensionResults = clientExtensionResults; this.type = type; } @@ -102,9 +105,22 @@ private PublicKeyCredential( private PublicKeyCredential( ByteArray id, @NonNull A response, + AuthenticatorAttachment authenticatorAttachment, @NonNull B clientExtensionResults, @NonNull PublicKeyCredentialType type) { - this(id, null, response, clientExtensionResults, type); + this(id, null, response, authenticatorAttachment, clientExtensionResults, type); + } + + /** + * The authenticator + * attachment modality in effect at the time the credential was used. + * + * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as + * the standard matures. + */ + @Deprecated + public Optional getAuthenticatorAttachment() { + return Optional.ofNullable(authenticatorAttachment); } public static 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 8f9e309c7..f817ad746 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 @@ -31,6 +31,7 @@ import com.upokecenter.cbor.CBORObject import com.yubico.internal.util.JacksonCodecs import com.yubico.webauthn.data.AssertionExtensionInputs import com.yubico.webauthn.data.AuthenticatorAssertionResponse +import com.yubico.webauthn.data.AuthenticatorAttachment import com.yubico.webauthn.data.AuthenticatorDataFlags import com.yubico.webauthn.data.AuthenticatorTransport import com.yubico.webauthn.data.ByteArray @@ -2592,6 +2593,36 @@ class RelyingPartyAssertionSpec resultWithBeOnly.isBackedUp should be(false) resultWithBackup.isBackedUp should be(true) } + + it( + "exposes getAuthenticatorAttachment() with the authenticatorAttachment value from the PublicKeyCredential." + ) { + val pkcTemplate = + TestAuthenticator.createAssertion( + challenge = + request.getPublicKeyCredentialRequestOptions.getChallenge, + credentialKey = credentialKeypair, + credentialId = credential.getId, + ) + + forAll { authenticatorAttachment: Option[AuthenticatorAttachment] => + val pkc = pkcTemplate.toBuilder + .authenticatorAttachment(authenticatorAttachment.orNull) + .build() + + val result = rp.finishAssertion( + FinishAssertionOptions + .builder() + .request(request) + .response(pkc) + .build() + ) + + result.getAuthenticatorAttachment should equal( + pkc.getAuthenticatorAttachment + ) + } + } } } } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala index 983cd281d..a737f23b4 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala @@ -42,6 +42,7 @@ import com.yubico.webauthn.attestation.AttestationTrustSource import com.yubico.webauthn.attestation.AttestationTrustSource.TrustRootsResult import com.yubico.webauthn.data.AttestationObject import com.yubico.webauthn.data.AttestationType +import com.yubico.webauthn.data.AuthenticatorAttachment import com.yubico.webauthn.data.AuthenticatorAttestationResponse import com.yubico.webauthn.data.AuthenticatorData import com.yubico.webauthn.data.AuthenticatorDataFlags @@ -4619,6 +4620,33 @@ class RelyingPartyRegistrationSpec resultWithBeOnly.isBackedUp should be(false) resultWithBackup.isBackedUp should be(true) } + + it( + "exposes getAuthenticatorAttachment() with the authenticatorAttachment value from the PublicKeyCredential." + ) { + val (pkcTemplate, _, _) = + TestAuthenticator.createUnattestedCredential(challenge = + request.getChallenge + ) + + forAll { authenticatorAttachment: Option[AuthenticatorAttachment] => + val pkc = pkcTemplate.toBuilder + .authenticatorAttachment(authenticatorAttachment.orNull) + .build() + + val result = rp.finishRegistration( + FinishRegistrationOptions + .builder() + .request(request) + .response(pkc) + .build() + ) + + result.getAuthenticatorAttachment should equal( + pkc.getAuthenticatorAttachment + ) + } + } } } From c41bcdbd33d602dcf08ff9ff7c60658043abf8d2 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Fri, 16 Dec 2022 16:00:49 +0100 Subject: [PATCH 2/4] Add JavaDoc cross-references between getAuthenticatorAttachment() methods --- .../com/yubico/webauthn/AssertionResult.java | 1 + .../yubico/webauthn/RegistrationResult.java | 1 + .../webauthn/data/PublicKeyCredential.java | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java index f12297400..f426943f1 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/AssertionResult.java @@ -200,6 +200,7 @@ public boolean isBackedUp() { * The authenticator * attachment modality in effect at the time the asserted credential was used. * + * @see PublicKeyCredential#getAuthenticatorAttachment() * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as * the standard matures. */ diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java index 0e6d24c56..73c8d6e14 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/RegistrationResult.java @@ -180,6 +180,7 @@ public boolean isBackedUp() { * The authenticator * attachment modality in effect at the time the credential was created. * + * @see PublicKeyCredential#getAuthenticatorAttachment() * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as * the standard matures. */ diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java index fa34e10f6..ee0b5a38e 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredential.java @@ -28,6 +28,11 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.type.TypeReference; import com.yubico.internal.util.JacksonCodecs; +import com.yubico.webauthn.AssertionResult; +import com.yubico.webauthn.FinishAssertionOptions; +import com.yubico.webauthn.FinishRegistrationOptions; +import com.yubico.webauthn.RegistrationResult; +import com.yubico.webauthn.RelyingParty; import java.io.IOException; import java.util.Optional; import lombok.AllArgsConstructor; @@ -113,8 +118,19 @@ private PublicKeyCredential( /** * The authenticator - * attachment modality in effect at the time the credential was used. + * attachment modality in effect at the time the credential was created or used. * + *

If parsed from JSON, this will be present if and only if the input was a valid value of + * {@link AuthenticatorAttachment}. + * + *

The same value will also be available via {@link + * RegistrationResult#getAuthenticatorAttachment()} or {@link + * AssertionResult#getAuthenticatorAttachment()} on the result from {@link + * RelyingParty#finishRegistration(FinishRegistrationOptions)} or {@link + * RelyingParty#finishAssertion(FinishAssertionOptions)}. + * + * @see RegistrationResult#getAuthenticatorAttachment() + * @see AssertionResult#getAuthenticatorAttachment() * @deprecated EXPERIMENTAL: This feature is from a not yet mature standard; it could change as * the standard matures. */ From 8c165d89e90312af86c805c8c311f8a9ce8b4aed Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Fri, 16 Dec 2022 16:09:37 +0100 Subject: [PATCH 3/4] Update AuthenticatorAttachment tests --- .../data/AuthenticatorAttachment.java | 13 +----- .../com/yubico/webauthn/data/EnumsSpec.scala | 7 ++- .../com/yubico/webauthn/data/JsonIoSpec.scala | 43 ++++++++++++++++--- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorAttachment.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorAttachment.java index 96c666659..d5d338b42 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorAttachment.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorAttachment.java @@ -26,7 +26,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; -import java.util.Optional; import java.util.stream.Stream; import lombok.AllArgsConstructor; import lombok.Getter; @@ -73,18 +72,8 @@ public enum AuthenticatorAttachment { @JsonValue @Getter @NonNull private final String value; - private static Optional fromString(@NonNull String value) { - return Stream.of(values()).filter(v -> v.value.equals(value)).findAny(); - } - @JsonCreator private static AuthenticatorAttachment fromJsonString(@NonNull String value) { - return fromString(value) - .orElseThrow( - () -> - new IllegalArgumentException( - String.format( - "Unknown %s value: %s", - AuthenticatorAttachment.class.getSimpleName(), value))); + return Stream.of(values()).filter(v -> v.value.equals(value)).findAny().orElse(null); } } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/EnumsSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/EnumsSpec.scala index 8e35e8558..dbe5ca609 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/EnumsSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/EnumsSpec.scala @@ -61,11 +61,10 @@ class EnumsSpec describe("AuthenticatorAttachment") { describe("can be parsed from JSON") { - it("but throws IllegalArgumentException for unknown values.") { - val result = Try( + it("and ignores for unknown values.") { + val result = json.readValue("\"foo\"", classOf[AuthenticatorAttachment]) - ) - result.failed.get.getCause shouldBe an[IllegalArgumentException] + result should be(null) } } } diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/JsonIoSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/JsonIoSpec.scala index 29cfa491f..0fd026f9b 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/JsonIoSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/data/JsonIoSpec.scala @@ -41,12 +41,13 @@ import com.yubico.webauthn.extension.appid.Generators._ import org.junit.runner.RunWith import org.scalacheck.Arbitrary import org.scalacheck.Arbitrary.arbitrary -import org.scalacheck.Gen import org.scalatest.funspec.AnyFunSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.junit.JUnitRunner import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks +import scala.jdk.OptionConverters.RichOptional + @RunWith(classOf[JUnitRunner]) class JsonIoSpec extends AnyFunSpec @@ -351,15 +352,16 @@ class JsonIoSpec ) } - it("allows and ignores an authenticatorAttachment attribute.") { + it( + "allows an authenticatorAttachment attribute, but ignores unknown values." + ) { def test[P <: PublicKeyCredential[_, _]](tpe: TypeReference[P])(implicit a: Arbitrary[P] ): Unit = { forAll( a.arbitrary, - Gen.oneOf( - arbitrary[AuthenticatorAttachment].map(_.getValue), - arbitrary[String], + arbitrary[String].suchThat(s => + !AuthenticatorAttachment.values.map(_.getValue).contains(s) ), ) { (value: P, authenticatorAttachment: String) => val tree: ObjectNode = json.valueToTree(value) @@ -370,8 +372,37 @@ class JsonIoSpec val encoded = json.writeValueAsString(tree) println(authenticatorAttachment) val decoded = json.readValue(encoded, tpe) + decoded.getAuthenticatorAttachment.asScala should be(None) + } + + forAll( + a.arbitrary, + arbitrary[AuthenticatorAttachment], + ) { (value: P, authenticatorAttachment: AuthenticatorAttachment) => + val tree: ObjectNode = json.valueToTree(value) + tree.set( + "authenticatorAttachment", + new TextNode(authenticatorAttachment.getValue), + ) + val encoded = json.writeValueAsString(tree) + println(authenticatorAttachment) + val decoded = json.readValue(encoded, tpe) + + decoded.getAuthenticatorAttachment.asScala should equal( + Some(authenticatorAttachment) + ) + } + + forAll( + a.arbitrary + ) { (value: P) => + val tree: ObjectNode = json.valueToTree( + value.toBuilder.authenticatorAttachment(null).build() + ) + val encoded = json.writeValueAsString(tree) + val decoded = json.readValue(encoded, tpe) - decoded should equal(value) + decoded.getAuthenticatorAttachment.asScala should be(None) } } From 0c817c2675838a076b99bd36bb17891bb99d3eea Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Fri, 16 Dec 2022 16:44:20 +0100 Subject: [PATCH 4/4] Add authenticatorAttachment to NEWS --- NEWS | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/NEWS b/NEWS index 3cc5ee846..57d91e86f 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,20 @@ +== Version 2.3.0 (unreleased) == + +New features: + +* (Experimental) Added `authenticatorAttachment` property to response objects: + ** NOTE: Experimental features may receive breaking changes without a major + version increase. + ** Added method `getAuthenticatorAttachment()` to `PublicKeyCredential` and + corresponding builder method + `authenticatorAttachment(AuthenticatorAttachment)`. + ** Added method `getAuthenticatorAttachment()` to `RegistrationResult` and + `AssertionResult`, which echo `getAuthenticatorAttachment()` from the + corresponding `PublicKeyCredential`. + ** Thanks to GitHub user luisgoncalves for the contribution, see + https://github.com/Yubico/java-webauthn-server/pull/250 + + == Version 2.2.0 == `webauthn-server-core`: