Implement {Device, CtapRegister}Operation interface

DeviceOperation is an interface for Ctap{Register, Sign}Operation
and U2f{Register, Sign}Operation, and will be a branching point for
separation of logic between CTAP and U2F requests. CtapRegisterOperation
is an implementation of DeviceOperation and is owned by
MakeCredentialTask to handle dispatching CTAP MakeCredential request to
an authenticator.

Bug: 798573
Change-Id: I290ab7b3f686d4418d783723a0ab510f27362462
Reviewed-on: https://chromium-review.googlesource.com/1028792
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554565}

device/fido/BUILD.gn

@@ -31,6 +31,9 @@ component("fido") {
     "ctap_get_assertion_request.h",
     "ctap_make_credential_request.cc",
     "ctap_make_credential_request.h",
+    "ctap_register_operation.cc",
+    "ctap_register_operation.h",
+    "device_operation.h",
     "device_response_converter.cc",
     "device_response_converter.h",
     "ec_public_key.cc",

device/fido/ctap_register_operation.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/ctap_register_operation.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "device/fido/authenticator_make_credential_response.h"
+#include "device/fido/ctap_make_credential_request.h"
+#include "device/fido/device_response_converter.h"
+#include "device/fido/fido_device.h"
+
+namespace device {
+
+CtapRegisterOperation::CtapRegisterOperation(
+    FidoDevice* device,
+    const CtapMakeCredentialRequest* request,
+    DeviceResponseCallback callback)
+    : DeviceOperation(device, std::move(callback)),
+      request_(request),
+      weak_factory_(this) {}
+
+CtapRegisterOperation::~CtapRegisterOperation() = default;
+
+void CtapRegisterOperation::Start() {
+  device_->DeviceTransact(
+      request_->EncodeAsCBOR(),
+      base::BindOnce(&CtapRegisterOperation::OnResponseReceived,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void CtapRegisterOperation::OnResponseReceived(
+    base::Optional<std::vector<uint8_t>> device_response) {
+  if (!device_response) {
+    std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
+                             base::nullopt);
+    return;
+  }
+
+  std::move(callback_).Run(GetResponseCode(*device_response),
+                           ReadCTAPMakeCredentialResponse(*device_response));
+}
+
+}  // namespace device

device/fido/ctap_register_operation.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_CTAP_REGISTER_OPERATION_H_
+#define DEVICE_FIDO_CTAP_REGISTER_OPERATION_H_
+
+#include <stdint.h>
+
+#include <memory>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "device/fido/device_operation.h"
+#include "device/fido/fido_constants.h"
+
+namespace device {
+
+class FidoDevice;
+class CtapMakeCredentialRequest;
+class AuthenticatorMakeCredentialResponse;
+
+// Represents per device registration logic for CTAP device.
+// CtapRegisterOperation is owned by MakeCredentialTask, and the lifetime of
+// CtapRegisterOperation does not exceed that of MakeCredentialTask. As so,
+// |request_| member variable is dependency injected from MakeCredentialTask.
+class COMPONENT_EXPORT(DEVICE_FIDO) CtapRegisterOperation
+    : public DeviceOperation<CtapMakeCredentialRequest,
+                             AuthenticatorMakeCredentialResponse> {
+ public:
+  CtapRegisterOperation(FidoDevice* device,
+                        const CtapMakeCredentialRequest* request,
+                        DeviceResponseCallback callback);
+
+  ~CtapRegisterOperation() override;
+
+  // DeviceOperation:
+  void Start() override;
+
+ private:
+  void OnResponseReceived(base::Optional<std::vector<uint8_t>> device_response);
+
+  const CtapMakeCredentialRequest* const request_;
+
+  base::WeakPtrFactory<CtapRegisterOperation> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CtapRegisterOperation);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_FIDO_CTAP_REGISTER_OPERATION_H_

device/fido/device_operation.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_DEVICE_OPERATION_H_
+#define DEVICE_FIDO_DEVICE_OPERATION_H_
+
+#include <stdint.h>
+
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/optional.h"
+#include "device/fido/authenticator_get_assertion_response.h"
+#include "device/fido/authenticator_make_credential_response.h"
+#include "device/fido/ctap_get_assertion_request.h"
+#include "device/fido/ctap_make_credential_request.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/fido_device.h"
+
+namespace device {
+
+template <class Request, class Response>
+class DeviceOperation {
+ public:
+  using DeviceResponseCallback =
+      base::OnceCallback<void(CtapDeviceResponseCode,
+                              base::Optional<Response>)>;
+
+  DeviceOperation(FidoDevice* device, DeviceResponseCallback callback)
+      : device_(device), callback_(std::move(callback)) {}
+  virtual ~DeviceOperation() = default;
+
+  virtual void Start() = 0;
+
+ protected:
+  FidoDevice* const device_ = nullptr;
+  DeviceResponseCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeviceOperation);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_FIDO_DEVICE_OPERATION_H_

device/fido/fido_device.h

@@ -25,7 +25,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDevice {
  public:
   using WinkCallback = base::OnceClosure;
   using DeviceCallback =
-      base::OnceCallback<void(base::Optional<std::vector<uint8_t>> response)>;
+      base::OnceCallback<void(base::Optional<std::vector<uint8_t>>)>;
 
   // Internal state machine states.
   enum class State { kInit, kConnected, kBusy, kReady, kDeviceError };

device/fido/get_assertion_request_handler.h

@@ -26,9 +26,9 @@ class FidoDevice;
 class FidoTask;
 class AuthenticatorGetAssertionResponse;
 
-using SignResponseCallback = base::OnceCallback<void(
-    FidoReturnCode status_code,
-    base::Optional<AuthenticatorGetAssertionResponse> response_data)>;
+using SignResponseCallback =
+    base::OnceCallback<void(FidoReturnCode,
+                            base::Optional<AuthenticatorGetAssertionResponse>)>;
 
 class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionRequestHandler
     : public FidoRequestHandler<AuthenticatorGetAssertionResponse> {

device/fido/get_assertion_task.h

@@ -27,8 +27,8 @@ class AuthenticatorGetAssertionResponse;
 class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionTask : public FidoTask {
  public:
   using GetAssertionTaskCallback = base::OnceCallback<void(
-      CtapDeviceResponseCode return_code,
-      base::Optional<AuthenticatorGetAssertionResponse> response_data)>;
+      CtapDeviceResponseCode,
+      base::Optional<AuthenticatorGetAssertionResponse>)>;
 
   GetAssertionTask(FidoDevice* device,
                    CtapGetAssertionRequest request,

device/fido/make_credential_request_handler.h

@@ -28,9 +28,8 @@ class FidoDevice;
 class FidoTask;
 class AuthenticatorMakeCredentialResponse;
 
-using RegisterResponseCallback = base::OnceCallback<void(
-    FidoReturnCode status_code,
-    base::Optional<AuthenticatorMakeCredentialResponse> response_data)>;
+using RegisterResponseCallback = base::OnceCallback<
+    void(FidoReturnCode, base::Optional<AuthenticatorMakeCredentialResponse>)>;
 
 class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialRequestHandler
     : public FidoRequestHandler<AuthenticatorMakeCredentialResponse> {

device/fido/make_credential_task.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "device/fido/ctap_empty_authenticator_request.h"
+#include "device/fido/ctap_register_operation.h"
 #include "device/fido/device_response_converter.h"
 
 namespace device {
@@ -40,10 +41,11 @@ void MakeCredentialTask::MakeCredential() {
     return;
   }
 
-  device()->DeviceTransact(
-      request_parameter_.EncodeAsCBOR(),
+  register_operation_ = std::make_unique<CtapRegisterOperation>(
+      device(), &request_parameter_,
       base::BindOnce(&MakeCredentialTask::OnCtapMakeCredentialResponseReceived,
                      weak_factory_.GetWeakPtr()));
+  register_operation_->Start();
 }
 
 void MakeCredentialTask::U2fRegister() {
@@ -56,28 +58,21 @@ void MakeCredentialTask::U2fRegister() {
 }
 
 void MakeCredentialTask::OnCtapMakeCredentialResponseReceived(
-    base::Optional<std::vector<uint8_t>> device_response) {
-  if (!device_response) {
-    std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
-                             base::nullopt);
-    return;
-  }
-
-  auto response_code = GetResponseCode(*device_response);
-  if (response_code != CtapDeviceResponseCode::kSuccess) {
-    std::move(callback_).Run(response_code, base::nullopt);
+    CtapDeviceResponseCode return_code,
+    base::Optional<AuthenticatorMakeCredentialResponse> response_data) {
+  if (return_code != CtapDeviceResponseCode::kSuccess) {
+    std::move(callback_).Run(return_code, base::nullopt);
     return;
   }
 
-  auto parsed_response = ReadCTAPMakeCredentialResponse(*device_response);
-  if (!parsed_response ||
-      !parsed_response->CheckRpIdHash(request_parameter_.rp().rp_id())) {
+  if (!response_data ||
+      !response_data->CheckRpIdHash(request_parameter_.rp().rp_id())) {
     std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
                              base::nullopt);
     return;
   }
 
-  std::move(callback_).Run(response_code, std::move(parsed_response));
+  std::move(callback_).Run(return_code, std::move(response_data));
 }
 
 bool MakeCredentialTask::CheckIfAuthenticatorSelectionCriteriaAreSatisfied() {

device/fido/make_credential_task.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <vector>
 
 #include "base/callback.h"
@@ -17,6 +18,7 @@
 #include "device/fido/authenticator_make_credential_response.h"
 #include "device/fido/authenticator_selection_criteria.h"
 #include "device/fido/ctap_make_credential_request.h"
+#include "device/fido/ctap_register_operation.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_task.h"
 
@@ -27,8 +29,8 @@ namespace device {
 class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialTask : public FidoTask {
  public:
   using MakeCredentialTaskCallback = base::OnceCallback<void(
-      CtapDeviceResponseCode return_code,
-      base::Optional<AuthenticatorMakeCredentialResponse> response_data)>;
+      CtapDeviceResponseCode,
+      base::Optional<AuthenticatorMakeCredentialResponse>)>;
 
   MakeCredentialTask(FidoDevice* device,
                      CtapMakeCredentialRequest request_parameter,
@@ -43,7 +45,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialTask : public FidoTask {
   void MakeCredential();
   void U2fRegister();
   void OnCtapMakeCredentialResponseReceived(
-      base::Optional<std::vector<uint8_t>> device_response);
+      CtapDeviceResponseCode return_code,
+      base::Optional<AuthenticatorMakeCredentialResponse> response_data);
 
   // Invoked after retrieving response to AuthenticatorGetInfo request. Filters
   // out authenticators based on |authenticator_selection_criteria_| constraints
@@ -53,8 +56,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialTask : public FidoTask {
 
   CtapMakeCredentialRequest request_parameter_;
   AuthenticatorSelectionCriteria authenticator_selection_criteria_;
+  std::unique_ptr<CtapRegisterOperation> register_operation_;
   MakeCredentialTaskCallback callback_;
-
   base::WeakPtrFactory<MakeCredentialTask> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MakeCredentialTask);