Support unknown transports

NEWS

@@ -4,6 +4,10 @@ Changes:
 
 * `RelyingParty` now makes an immutable copy of the `origins` argument, instead
   of storing a reference to a possibly mutable value.
+* The enum `AuthenticatorTransport` has been replaced by a value class
+  containing methods and value constants equivalent to the previous enum.
+* The return type of `PublicKeyCredentialDescriptor.getTransports()` is now a
+  `SortedSet` instead of `Set`. The builder still accepts a plain `Set`.
 
 New features:
 
@@ -12,6 +16,9 @@ New features:
   * `allowOriginSubdomain` (default `false`): Allow any subdomain of any origin
     listed in `RelyingParty.origins`
   * See JavaDoc for details and examples.
+* The new `AuthenticatorTransport` can now contain any string value as the
+  transport identifier, as required in the editor's draft of the L2 spec. See:
+  https://github.com/w3c/webauthn/pull/1275
 
 
 == Version 1.4.1 ==

webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorTransport.java

@@ -25,14 +25,14 @@
 package com.yubico.webauthn.data;
 
 
-import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.yubico.internal.util.json.JsonStringSerializable;
 import com.yubico.internal.util.json.JsonStringSerializer;
-import java.util.Optional;
 import java.util.stream.Stream;
+import lombok.AccessLevel;
 import lombok.AllArgsConstructor;
 import lombok.NonNull;
+import lombok.Value;
 
 /**
  * Authenticators may communicate with Clients using a variety of transports. This enumeration defines a hint as to how
@@ -52,48 +52,80 @@
  * Transport Enumeration (enum AuthenticatorTransport)</a>
  */
 @JsonSerialize(using = JsonStringSerializer.class)
-@AllArgsConstructor
-public enum AuthenticatorTransport implements JsonStringSerializable {
+@Value
+@AllArgsConstructor(access = AccessLevel.PRIVATE)
+public class AuthenticatorTransport implements Comparable<AuthenticatorTransport>, JsonStringSerializable {
+
+    @NonNull
+    private final String id;
 
     /**
      * Indicates the respective authenticator can be contacted over removable USB.
      */
-    USB("usb"),
+    public static final AuthenticatorTransport USB = new AuthenticatorTransport("usb");
 
     /**
      * Indicates the respective authenticator can be contacted over Near Field Communication (NFC).
      */
-    NFC("nfc"),
+    public static final AuthenticatorTransport NFC = new AuthenticatorTransport("nfc");
 
     /**
      * Indicates the respective authenticator can be contacted over Bluetooth Smart (Bluetooth Low Energy / BLE).
      */
-    BLE("ble"),
+    public static final AuthenticatorTransport BLE = new AuthenticatorTransport("ble");
 
     /**
      * Indicates the respective authenticator is contacted using a client device-specific transport. These
      * authenticators are not removable from the client device.
      */
-    INTERNAL("internal")
-    ;
+    public static final AuthenticatorTransport INTERNAL = new AuthenticatorTransport("internal");
 
-    @NonNull
-    private final String id;
+    /**
+     * @return An array containing all predefined values of {@link AuthenticatorTransport} known by this implementation.
+     */
+    public static AuthenticatorTransport[] values() {
+        return new AuthenticatorTransport[]{ USB, NFC, BLE, INTERNAL };
+    }
 
-    private static Optional<AuthenticatorTransport> fromString(@NonNull String id) {
-        return Stream.of(values()).filter(v -> v.id.equals(id)).findAny();
+    /**
+     * @return If <code>id</code> is the same as that of any of {@link #USB}, {@link #NFC}, {@link #BLE} or {@link
+     * #INTERNAL}, returns that constant instance. Otherwise returns a new instance containing <code>id</code>.
+     * @see #valueOf(String)
+     */
+    public static AuthenticatorTransport of(@NonNull String id) {
+        return Stream.of(values())
+            .filter(v -> v.getId().equals(id))
+            .findAny()
+            .orElseGet(() -> new AuthenticatorTransport(id));
     }
 
-    @JsonCreator
-    private static AuthenticatorTransport fromJsonString(@NonNull String id) {
-        return fromString(id).orElseThrow(() -> new IllegalArgumentException(String.format(
-            "Unknown %s value: %s", AuthenticatorTransport.class.getSimpleName(), id
-        )));
+    /**
+     * @return If <code>name</code> equals <code>"USB"</code>, <code>"NFC"</code>, <code>"BLE"</code> or
+     * <code>"INTERNAL"</code>, returns the constant by that name.
+     * @throws IllegalArgumentException
+     *     if <code>name</code> is anything else.
+     *
+     * @see #of(String)
+     */
+    public static AuthenticatorTransport valueOf(String name) {
+        switch (name) {
+            case "USB": return USB;
+            case "NFC": return NFC;
+            case "BLE": return BLE;
+            case "INTERNAL": return INTERNAL;
+        default:
+            throw new IllegalArgumentException("No enum constant com.yubico.webauthn.data.AuthenticatorTransport." + name);
+        }
     }
 
     @Override
     public String toJsonString() {
         return id;
     }
 
+    @Override
+    public int compareTo(AuthenticatorTransport other) {
+        return id.compareTo(other.id);
+    }
+
 }

webauthn-server-core/src/main/java/com/yubico/webauthn/data/AuthenticatorTransportTest.java

@@ -0,0 +1,4 @@
+package com.yubico.webauthn.data;
+
+public class AuthenticatorTransportTest {
+}

webauthn-server-core/src/main/java/com/yubico/webauthn/data/PublicKeyCredentialDescriptor.java

@@ -27,9 +27,10 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.yubico.internal.util.CollectionUtil;
-import com.yubico.internal.util.EnumUtil;
+import com.yubico.internal.util.ComparableUtil;
 import java.util.Optional;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 import lombok.Builder;
 import lombok.NonNull;
@@ -65,7 +66,7 @@ public class PublicKeyCredentialDescriptor implements Comparable<PublicKeyCreden
      * An OPTIONAL hint as to how the client might communicate with the managing authenticator of the public key
      * credential the caller is referring to.
      */
-    private final Set<AuthenticatorTransport> transports;
+    private final SortedSet<AuthenticatorTransport> transports;
 
     @JsonCreator
     private PublicKeyCredentialDescriptor(
@@ -94,7 +95,7 @@ public int compareTo(PublicKeyCredentialDescriptor other) {
         } else if (getTransports().isPresent() && !other.getTransports().isPresent()) {
             return 1;
         } else if (getTransports().isPresent() && other.getTransports().isPresent()) {
-            int transportsComparison = EnumUtil.compareSets(getTransports().get(), other.getTransports().get(), AuthenticatorTransport.class);
+            int transportsComparison = ComparableUtil.compareComparableSets(getTransports().get(), other.getTransports().get());
             if (transportsComparison != 0) {
                 return transportsComparison;
             }
@@ -136,7 +137,7 @@ public PublicKeyCredentialDescriptorBuilder transports(Set<AuthenticatorTranspor
         }
     }
 
-    public Optional<Set<AuthenticatorTransport>> getTransports() {
+    public Optional<SortedSet<AuthenticatorTransport>> getTransports() {
         return Optional.ofNullable(transports);
     }
 

webauthn-server-core/src/test/scala/com/yubico/webauthn/data/AuthenticatorTransportSpec.scala

@@ -0,0 +1,82 @@
+// Copyright (c) 2019, Yubico AB
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+//    list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+//    this list of conditions and the following disclaimer in the documentation
+//    and/or other materials provided with the distribution.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package com.yubico.webauthn.data
+
+import org.junit.runner.RunWith
+import org.scalatest.FunSpec
+import org.scalatest.Matchers
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.prop.GeneratorDrivenPropertyChecks
+
+@RunWith(classOf[JUnitRunner])
+class AuthenticatorTransportSpec extends FunSpec with Matchers with GeneratorDrivenPropertyChecks {
+
+  describe("The AuthenticatorTransport type") {
+
+    describe("has the constant") {
+      it("USB.") {
+        AuthenticatorTransport.USB.getId should equal ("usb")
+      }
+      it("NFC.") {
+        AuthenticatorTransport.NFC.getId should equal ("nfc")
+      }
+      it("BLE.") {
+        AuthenticatorTransport.BLE.getId should equal ("ble")
+      }
+      it("INTERNAL.") {
+        AuthenticatorTransport.INTERNAL.getId should equal ("internal")
+      }
+    }
+
+    it("has a values() function.") {
+      AuthenticatorTransport.values().length should equal (4)
+      AuthenticatorTransport.values() should not be theSameInstanceAs (AuthenticatorTransport.values())
+    }
+
+    it("has a valueOf(name) function mimicking that of an enum type.") {
+      AuthenticatorTransport.valueOf("USB") should be theSameInstanceAs AuthenticatorTransport.USB
+      AuthenticatorTransport.valueOf("NFC") should be theSameInstanceAs AuthenticatorTransport.NFC
+      AuthenticatorTransport.valueOf("BLE") should be theSameInstanceAs AuthenticatorTransport.BLE
+      AuthenticatorTransport.valueOf("INTERNAL") should be theSameInstanceAs AuthenticatorTransport.INTERNAL
+      an[IllegalArgumentException] should be thrownBy {
+        AuthenticatorTransport.valueOf("foo")
+      }
+    }
+
+    it("can contain any value.") {
+      forAll { transport: String =>
+        AuthenticatorTransport.of(transport).getId should equal (transport)
+      }
+    }
+
+    it("has an of(id) function which returns the corresponding constant instance if applicable, and a new instance otherwise.") {
+      for { constant <- AuthenticatorTransport.values() } {
+        AuthenticatorTransport.of(constant.getId) should equal (constant)
+        AuthenticatorTransport.of(constant.getId) should be theSameInstanceAs constant
+        AuthenticatorTransport.of(constant.getId.toUpperCase) should not equal constant
+      }
+    }
+  }
+}

webauthn-server-core/src/test/scala/com/yubico/webauthn/data/Generators.scala

@@ -183,6 +183,12 @@ object Generators {
     .userVerification(userVerification)
     .build())
 
+  implicit val arbitraryAuthenticatorTransport: Arbitrary[AuthenticatorTransport] = Arbitrary(
+    Gen.oneOf(
+      Gen.oneOf(AuthenticatorTransport.values()),
+      arbitrary[String] map AuthenticatorTransport.of
+    ))
+
   implicit val arbitraryByteArray: Arbitrary[ByteArray] = Arbitrary(arbitrary[Array[Byte]].map(new ByteArray(_)))
   def byteArray(size: Int): Gen[ByteArray] = Gen.listOfN(size, arbitrary[Byte]).map(ba => new ByteArray(ba.toArray))
 

yubico-util/src/main/java/com/yubico/internal/util/CollectionUtil.java

@@ -44,7 +44,7 @@ public static <T> Set<T> immutableSet(Set<T> s) {
      *
      * @return A shallow copy of <code>s</code> which cannot be modified
      */
-    public static <T> Set<T> immutableSortedSet(SortedSet<T> s) {
+    public static <T> SortedSet<T> immutableSortedSet(SortedSet<T> s) {
         return Collections.unmodifiableSortedSet(new TreeSet<>(s));
     }
 

yubico-util/src/main/java/com/yubico/internal/util/EnumUtil.java → yubico-util/src/main/java/com/yubico/internal/util/ComparableUtil.java

@@ -24,19 +24,33 @@
 
 package com.yubico.internal.util;
 
-import java.util.Set;
+import java.util.Iterator;
+import java.util.SortedSet;
 
-public class EnumUtil {
+public class ComparableUtil {
 
-    public static <T extends Enum<?>> int compareSets(Set<T> a, Set<T> b, Class<T> clazz) {
-        for (T value : clazz.getEnumConstants()) {
-            if (a.contains(value) && !b.contains(value)) {
+    public static <T extends Comparable<T>> int compareComparableSets(SortedSet<T> a, SortedSet<T> b) {
+        if (a.size() == b.size()) {
+            final Iterator<T> as = a.iterator();
+            final Iterator<T> bs = b.iterator();
+
+            while (as.hasNext() && bs.hasNext()) {
+                final int comp = as.next().compareTo(bs.next());
+                if (comp != 0) {
+                    return comp;
+                }
+            }
+
+            if (as.hasNext()) {
                 return 1;
-            } else if (!a.contains(value) && b.contains(value)) {
+            } else if (bs.hasNext()) {
                 return -1;
+            } else {
+                return 0;
             }
+        } else {
+            return a.size() - b.size();
         }
-        return 0;
     }
 
 }

yubico-util/src/test/scala/com/yubico/internal/util/ComparableUtilSpec.scala

@@ -0,0 +1,57 @@
+package com.yubico.internal.util
+
+import org.junit.runner.RunWith
+import org.scalacheck.Gen
+import org.scalacheck.Arbitrary.arbitrary
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.prop.GeneratorDrivenPropertyChecks
+import org.scalatest.FunSpec
+import org.scalatest.Matchers
+
+import _root_.scala.collection.JavaConverters._
+
+
+@RunWith(classOf[JUnitRunner])
+class ComparableUtilSpec extends FunSpec with Matchers with GeneratorDrivenPropertyChecks {
+
+  def sameSizeSets[T](implicit gent: Gen[T]): Gen[(Set[T], Set[T])] = for {
+    n: Int <- Gen.chooseNum(0, 100)
+    a: Set[T] <- Gen.containerOfN[Set, T](n, gent)
+    b: Set[T] <- Gen.containerOfN[Set, T](n, gent)
+  } yield (a, b)
+
+  def toJava(s: Set[Int]): java.util.SortedSet[Integer] =
+    new java.util.TreeSet[Integer](s.map(new Integer(_)).asJava)
+
+  describe("compareComparableSets") {
+    it("sorts differently-sized sets in order of cardinality.") {
+      forAll { (a: Set[Int], b: Set[Int]) =>
+        whenever(a.size != b.size) {
+          val comp = ComparableUtil.compareComparableSets(toJava(a), toJava(b))
+          if (a.size < b.size) {
+            comp should be < 0
+          } else {
+            comp should be > 0
+          }
+        }
+      }
+    }
+
+    it("sorts same-sized sets like sorted lists.") {
+      forAll(sameSizeSets(arbitrary[Int])) { case (a, b) =>
+        whenever(a.size == b.size) {
+          val comp = ComparableUtil.compareComparableSets(toJava(a), toJava(b))
+
+          val aList = a.toList.sorted
+          val bList = b.toList.sorted
+          val firstDiff = aList.zip(bList).find({ case (a, b) => a != b })
+          firstDiff match {
+            case Some((a, b)) => comp should equal (a.compareTo(b))
+            case None => comp should equal (0)
+          }
+        }
+      }
+    }
+  }
+
+}