webauthn: add ScopedVirtualU2fDevice.

This change adds a ScopedVirtualU2fDevice to override normal discovery
in tests and always “discover” a VirtualU2fDevice. Using this, the
remaining appid tests are enabled.

Change-Id: I2836e1b9834b662032415fd0f45001cd5bcfae1d
Reviewed-on: https://chromium-review.googlesource.com/963097
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544162}

content/browser/webauth/authenticator_impl.cc

@@ -132,7 +132,7 @@ bool IsAppIdAllowedForOrigin(const GURL& appid, const url::Origin& origin) {
   const GURL kGstatic1 =
       GURL("https://www.gstatic.com/securitykey/origins.json");
   const GURL kGstatic2 =
-      GURL("https://www.gstatic.com/a/google.com/securitykey/origins.json");
+      GURL("https://www.gstatic.com/securitykey/a/google.com/origins.json");
   DCHECK(kGstatic1.is_valid() && kGstatic2.is_valid());
 
   if (origin.DomainIs("google.com") && !appid.has_ref() &&

content/browser/webauth/authenticator_impl_unittest.cc

@@ -17,8 +17,10 @@
 #include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/test/test_service_manager_context.h"
 #include "content/test/test_render_frame_host.h"
 #include "device/fido/fake_hid_impl_for_testing.h"
+#include "device/fido/scoped_virtual_u2f_device.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "services/device/public/mojom/constants.mojom.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -577,7 +579,7 @@ typedef struct {
   bool should_succeed;
 } OriginAppIdPair;
 
-constexpr OriginAppIdPair kInvalidAppIdCases[] = {
+constexpr OriginAppIdPair kAppIdCases[] = {
     {"https://example.com", "https://com/foo", false},
     {"https://example.com", "https://foo.com/", false},
     {"https://example.com", "http://example.com", false},
@@ -586,7 +588,6 @@ constexpr OriginAppIdPair kInvalidAppIdCases[] = {
     {"https://www.notgoogle.com",
      "https://www.gstatic.com/securitykey/origins.json", false},
 
-    /* Cannot be tested yet:
     {"https://example.com", "https://example.com", true},
     {"https://www.example.com", "https://example.com", true},
     {"https://example.com", "https://www.example.com", true},
@@ -596,17 +597,19 @@ constexpr OriginAppIdPair kInvalidAppIdCases[] = {
      "https://www.gstatic.com/securitykey/origins.json", true},
     {"https://www.google.com",
      "https://www.gstatic.com/securitykey/a/google.com/origins.json", true},
-     */
+    {"https://accounts.google.com",
+     "https://www.gstatic.com/securitykey/origins.json", true},
 };
 
 // Verify behavior for various combinations of origins and RP IDs.
 TEST_F(AuthenticatorImplTest, AppIdExtension) {
-  for (const auto& test_case : kInvalidAppIdCases) {
+  TestServiceManagerContext smc;
+  device::test::ScopedVirtualU2fDevice virtual_device;
+
+  for (const auto& test_case : kAppIdCases) {
     SCOPED_TRACE(std::string(test_case.origin) + " " +
                  std::string(test_case.appid));
 
-    CHECK(!test_case.should_succeed) << "can't test this yet";
-
     const GURL origin_url(test_case.origin);
     NavigateAndCommit(origin_url);
     AuthenticatorPtr authenticator = ConnectToAuthenticator();
@@ -618,7 +621,12 @@ TEST_F(AuthenticatorImplTest, AppIdExtension) {
     TestGetAssertionCallback cb;
     authenticator->GetAssertion(std::move(options), cb.callback());
     cb.WaitForCallback();
-    EXPECT_EQ(AuthenticatorStatus::INVALID_DOMAIN, cb.GetResponseStatus());
+
+    if (test_case.should_succeed) {
+      EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, cb.GetResponseStatus());
+    } else {
+      EXPECT_EQ(AuthenticatorStatus::INVALID_DOMAIN, cb.GetResponseStatus());
+    }
   }
 
   // TODO(agl): test positive cases once a mock U2F device exists.

device/fido/BUILD.gn

@@ -206,6 +206,8 @@ source_set("test_support") {
   sources = [
     "fake_fido_discovery.cc",
     "fake_fido_discovery.h",
+    "scoped_virtual_u2f_device.cc",
+    "scoped_virtual_u2f_device.h",
     "test_callback_receiver.h",
   ]
   deps = [

device/fido/fake_fido_discovery.h

@@ -123,7 +123,7 @@ class ScopedFakeFidoDiscoveryFactory
   using StartStopMode = FakeFidoDiscovery::StartStopMode;
 
   ScopedFakeFidoDiscoveryFactory();
-  ~ScopedFakeFidoDiscoveryFactory();
+  ~ScopedFakeFidoDiscoveryFactory() override;
 
   // Constructs a fake BLE/HID discovery to be returned from the next call to
   // FidoDiscovery::Create. Returns a raw pointer to the fake so that tests can

device/fido/fido_discovery.h

@@ -101,7 +101,7 @@ namespace internal {
 class COMPONENT_EXPORT(DEVICE_FIDO) ScopedFidoDiscoveryFactory {
  public:
   ScopedFidoDiscoveryFactory();
-  ~ScopedFidoDiscoveryFactory();
+  virtual ~ScopedFidoDiscoveryFactory();
 
  protected:
   virtual std::unique_ptr<FidoDiscovery> CreateFidoDiscovery(

device/fido/scoped_virtual_u2f_device.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/fido/scoped_virtual_u2f_device.h"
+
+#include "base/bind.h"
+
+namespace device {
+namespace test {
+
+// A FidoDiscovery that always vends a single |VirtualU2fDevice|.
+class VirtualU2fDeviceDiscovery : public FidoDiscovery {
+ public:
+  ~VirtualU2fDeviceDiscovery() override = default;
+
+  U2fTransportProtocol GetTransportProtocol() const override {
+    return U2fTransportProtocol::kUsbHumanInterfaceDevice;
+  }
+
+  void Start() override {
+    auto device = std::make_unique<VirtualU2fDevice>();
+    AddDevice(std::move(device));
+    NotifyDiscoveryStarted(true);
+  }
+
+  void Stop() override { NotifyDiscoveryStopped(true); }
+};
+
+ScopedVirtualU2fDevice::ScopedVirtualU2fDevice() = default;
+ScopedVirtualU2fDevice::~ScopedVirtualU2fDevice() = default;
+
+std::unique_ptr<FidoDiscovery> ScopedVirtualU2fDevice::CreateFidoDiscovery(
+    U2fTransportProtocol transport,
+    ::service_manager::Connector* connector) {
+  if (transport != U2fTransportProtocol::kUsbHumanInterfaceDevice) {
+    return nullptr;
+  }
+  return std::make_unique<VirtualU2fDeviceDiscovery>();
+}
+
+}  // namespace test
+}  // namespace device

device/fido/scoped_virtual_u2f_device.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_FIDO_SCOPED_VIRTUAL_U2F_DEVICE_H_
+#define DEVICE_FIDO_SCOPED_VIRTUAL_U2F_DEVICE_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "device/fido/fido_discovery.h"
+#include "device/fido/virtual_u2f_device.h"
+
+namespace device {
+namespace test {
+
+// Creating a |ScopedVirtualU2fDevice| causes normal device discovery to be
+// hijacked while the object is in scope. Instead a |VirtualU2fDevice| will
+// always be discovered. This object pretends to be a HID device.
+class ScopedVirtualU2fDevice
+    : public ::device::internal::ScopedFidoDiscoveryFactory {
+ public:
+  ScopedVirtualU2fDevice();
+  ~ScopedVirtualU2fDevice() override;
+
+ protected:
+  std::unique_ptr<FidoDiscovery> CreateFidoDiscovery(
+      U2fTransportProtocol transport,
+      ::service_manager::Connector* connector) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScopedVirtualU2fDevice);
+};
+
+}  // namespace test
+}  // namespace device
+
+#endif  // DEVICE_FIDO_SCOPED_VIRTUAL_U2F_DEVICE_H_

device/fido/u2f_request.cc

@@ -30,6 +30,11 @@ U2fRequest::U2fRequest(service_manager::Connector* connector,
       weak_factory_(this) {
   for (const auto transport : transports) {
     auto discovery = FidoDiscovery::Create(transport, connector);
+    if (discovery == nullptr) {
+      // This can occur in tests when a ScopedVirtualU2fDevice is in effect and
+      // non-HID transports are configured.
+      continue;
+    }
     discovery->AddObserver(this);
     discoveries_.push_back(std::move(discovery));
   }

device/fido/virtual_u2f_device.h

@@ -70,6 +70,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) VirtualU2fDevice : public FidoDevice {
                                               uint8_t p2,
                                               base::span<const uint8_t> data);
 
+  // TODO(agl): this state is in the wrong place: U2fDevice objects are
+  // ephemeral so to maintain state across requests this will have to be kept
+  // elsewhere.
   // Keyed on appId/rpId hash (aka "applicationParam")
   std::map<std::vector<uint8_t>, RegistrationData> registrations_;
   base::WeakPtrFactory<FidoDevice> weak_factory_;