Permit empty allow_credentials for CTAP2 devices.

Otherwise, passing an empty allow_credentials (such as for resident
key devices) returns a NotSupported error. This error is appropriate
for U2F devices but not for CTAP2 support.

Bug: 831712
Change-Id: Ia8126e8cdecc2b9149b72f8e636a08f5d5e71d0b
Reviewed-on: https://chromium-review.googlesource.com/1057960Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558451}

content/browser/webauth/authenticator_impl.cc

@@ -608,18 +608,19 @@ void AuthenticatorImpl::GetAssertion(
       FilterCredentialList(std::move(options->allow_credentials));
 
   // There are two different descriptions of what should happen when
-  // "allowCredentials" is empty.
+  // "allowCredentials" is empty for U2F.
   // a) WebAuthN 6.2.3 step 6[1] implies "NotAllowedError".
   // b) CTAP step 7.2 step 2[2] says the device should error out with
   // "CTAP2_ERR_OPTION_NOT_SUPPORTED". This also resolves to "NotAllowedError".
   // The behavior in both cases is consistent with the current implementation.
-  // TODO(crbug.com/831712): When CTAP2 authenticators are supported, this check
-  // should be enforced by handlers in fido/device on a per-device basis.
+  // When CTAP2 is enabled, however, this check is done by handlers in
+  // fido/device on a per-device basis.
 
   // [1] https://w3c.github.io/webauthn/#authenticatorgetassertion
   // [2]
   // https://fidoalliance.org/specs/fido-v2.0-ps-20170927/fido-client-to-authenticator-protocol-v2.0-ps-20170927.html
-  if (handles.empty()) {
+  if (!base::FeatureList::IsEnabled(features::kWebAuthCtap2) &&
+      handles.empty()) {
     InvokeCallbackAndCleanup(
         std::move(callback),
         webauth::mojom::AuthenticatorStatus::EMPTY_ALLOW_CREDENTIALS, nullptr);

content/browser/webauth/authenticator_impl_unittest.cc

@@ -791,6 +791,42 @@ TEST_F(AuthenticatorImplTest, TestU2fDeviceDoesNotSupportGetAssertion) {
   EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, callback_receiver.status());
 }
 
+TEST_F(AuthenticatorImplTest, Ctap2AcceptsEmptyAllowCredentials) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kWebAuthCtap2);
+
+  SimulateNavigation(GURL(kTestOrigin1));
+  PublicKeyCredentialRequestOptionsPtr options =
+      GetTestPublicKeyCredentialRequestOptions();
+  options->allow_credentials.clear();
+  TestGetAssertionCallback callback_receiver;
+
+  // Set up service_manager::Connector for tests.
+  auto fake_hid_manager = std::make_unique<device::FakeHidManager>();
+  service_manager::mojom::ConnectorRequest request;
+  auto connector = service_manager::Connector::Create(&request);
+
+  // Set up a timer for testing.
+  auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
+      base::Time::Now(), base::TimeTicks::Now());
+  auto timer =
+      std::make_unique<base::OneShotTimer>(task_runner->GetMockTickClock());
+  timer->SetTaskRunner(task_runner);
+  AuthenticatorPtr authenticator =
+      ConnectToAuthenticator(connector.get(), std::move(timer));
+
+  device::test::ScopedVirtualFidoDevice virtual_device;
+  authenticator->GetAssertion(std::move(options), callback_receiver.callback());
+
+  // Trigger timer.
+  base::RunLoop().RunUntilIdle();
+  task_runner->FastForwardBy(base::TimeDelta::FromMinutes(1));
+  callback_receiver.WaitForCallback();
+  // Doesn't error out with EMPTY_ALLOW_CREDENTIALS but continues to a
+  // NOT_ALLOWED_ERROR.
+  EXPECT_EQ(AuthenticatorStatus::NOT_ALLOWED_ERROR, callback_receiver.status());
+}
+
 TEST_F(AuthenticatorImplTest, GetAssertionWithEmptyAllowCredentials) {
   device::test::ScopedVirtualFidoDevice scoped_virtual_device;
   TestServiceManagerContext service_manager_context;