Refactor DeviceOperation logic

As DeviceOperation is templated with request and request, we can subsume
all logic in CtapRegisterOperation and same corresponding logic for
CtapGetAssertion to DeviceOperation. Remove CtapRegisterOperation and
add logic for dispatching request/ receiving response in DeviceOperation
interface.

Bug: 798573
Change-Id: I38a36ce53149a473736c0c1dbc0a4c7b65ea130b
Reviewed-on: https://chromium-review.googlesource.com/1034387
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565742}

device/fido/BUILD.gn

@@ -25,14 +25,13 @@ component("fido") {
     "authenticator_selection_criteria.h",
     "authenticator_supported_options.cc",
     "authenticator_supported_options.h",
+    "ctap2_device_operation.h",
     "ctap_empty_authenticator_request.cc",
     "ctap_empty_authenticator_request.h",
     "ctap_get_assertion_request.cc",
     "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",

device/fido/ctap2_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_CTAP2_DEVICE_OPERATION_H_
+#define DEVICE_FIDO_CTAP2_DEVICE_OPERATION_H_
+
+#include <stdint.h>
+
+#include <utility>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/containers/span.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "device/fido/device_operation.h"
+#include "device/fido/device_response_converter.h"
+#include "device/fido/fido_constants.h"
+#include "device/fido/fido_device.h"
+
+namespace device {
+
+// Represents per device logic for CTAP2 authenticators. Ctap2DeviceOperation
+// is owned by FidoTask, and thus |request| outlives Ctap2DeviceOperation.
+template <class Request, class Response>
+class Ctap2DeviceOperation : public DeviceOperation<Request, Response> {
+ public:
+  using DeviceResponseCallback =
+      base::OnceCallback<void(CtapDeviceResponseCode,
+                              base::Optional<Response>)>;
+  using DeviceResponseParser =
+      base::OnceCallback<base::Optional<Response>(base::span<const uint8_t>)>;
+
+  Ctap2DeviceOperation(FidoDevice* device,
+                       const Request& request,
+                       DeviceResponseCallback callback,
+                       DeviceResponseParser device_response_parser)
+      : DeviceOperation<Request, Response>(device,
+                                           request,
+                                           std::move(callback)),
+        device_response_parser_(std::move(device_response_parser)),
+        weak_factory_(this) {}
+
+  ~Ctap2DeviceOperation() override = default;
+
+  void Start() override {
+    this->device()->DeviceTransact(
+        this->request().EncodeAsCBOR(),
+        base::BindOnce(&Ctap2DeviceOperation::OnResponseReceived,
+                       weak_factory_.GetWeakPtr()));
+  }
+
+  void OnResponseReceived(
+      base::Optional<std::vector<uint8_t>> device_response) {
+    if (!device_response) {
+      std::move(this->callback())
+          .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
+      return;
+    }
+
+    const auto response_code = GetResponseCode(*device_response);
+    std::move(this->callback())
+        .Run(response_code, std::move(device_response_parser_)
+                                .Run(std::move(*device_response)));
+  }
+
+ private:
+  DeviceResponseParser device_response_parser_;
+  base::WeakPtrFactory<Ctap2DeviceOperation> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(Ctap2DeviceOperation);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_FIDO_CTAP2_DEVICE_OPERATION_H_

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

@@ -10,12 +10,9 @@
 #include <utility>
 #include <vector>
 
+#include "base/callback.h"
 #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"
 
@@ -27,14 +24,19 @@ class DeviceOperation {
   using DeviceResponseCallback =
       base::OnceCallback<void(CtapDeviceResponseCode,
                               base::Optional<Response>)>;
+  // Represents a per device logic that is owned by FidoTask. Thus,
+  // DeviceOperation does not outlive |request|.
+  DeviceOperation(FidoDevice* device,
+                  const Request& request,
+                  DeviceResponseCallback callback)
+      : device_(device), request_(request), callback_(std::move(callback)) {}
 
-  DeviceOperation(FidoDevice* device, DeviceResponseCallback callback)
-      : device_(device), callback_(std::move(callback)) {}
   virtual ~DeviceOperation() = default;
 
   virtual void Start() = 0;
 
  protected:
+  // TODO(hongjunchoi): Refactor so that |command| is never base::nullopt.
   void DispatchDeviceRequest(base::Optional<std::vector<uint8_t>> command,
                              FidoDevice::DeviceCallback callback) {
     if (!command) {
@@ -45,7 +47,13 @@ class DeviceOperation {
     device_->DeviceTransact(std::move(*command), std::move(callback));
   }
 
+  const Request& request() const { return request_; }
+  FidoDevice* device() const { return device_; }
+  DeviceResponseCallback callback() { return std::move(callback_); }
+
+ private:
   FidoDevice* const device_ = nullptr;
+  const Request& request_;
   DeviceResponseCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceOperation);

device/fido/get_assertion_task.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "device/fido/authenticator_get_assertion_response.h"
+#include "device/fido/ctap2_device_operation.h"
 #include "device/fido/ctap_empty_authenticator_request.h"
 #include "device/fido/device_response_converter.h"
 
@@ -52,10 +53,14 @@ void GetAssertionTask::GetAssertion() {
     return;
   }
 
-  device()->DeviceTransact(
-      request_.EncodeAsCBOR(),
-      base::BindOnce(&GetAssertionTask::OnCtapGetAssertionResponseReceived,
-                     weak_factory_.GetWeakPtr()));
+  sign_operation_ =
+      std::make_unique<Ctap2DeviceOperation<CtapGetAssertionRequest,
+                                            AuthenticatorGetAssertionResponse>>(
+          device(), request_,
+          base::BindOnce(&GetAssertionTask::OnCtapGetAssertionResponseReceived,
+                         weak_factory_.GetWeakPtr()),
+          base::BindOnce(&ReadCTAPGetAssertionResponse));
+  sign_operation_->Start();
 }
 
 void GetAssertionTask::U2fSign() {
@@ -107,29 +112,22 @@ bool GetAssertionTask::CheckRequirementsOnReturnedCredentialId(
 }
 
 void GetAssertionTask::OnCtapGetAssertionResponseReceived(
-    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);
+    CtapDeviceResponseCode response_code,
+    base::Optional<AuthenticatorGetAssertionResponse> device_response) {
   if (response_code != CtapDeviceResponseCode::kSuccess) {
     std::move(callback_).Run(response_code, base::nullopt);
     return;
   }
 
-  auto parsed_response = ReadCTAPGetAssertionResponse(*device_response);
-  if (!parsed_response || !parsed_response->CheckRpIdHash(request_.rp_id()) ||
-      !CheckRequirementsOnReturnedCredentialId(*parsed_response) ||
-      !CheckRequirementsOnReturnedUserEntities(*parsed_response)) {
+  if (!device_response || !device_response->CheckRpIdHash(request_.rp_id()) ||
+      !CheckRequirementsOnReturnedCredentialId(*device_response) ||
+      !CheckRequirementsOnReturnedUserEntities(*device_response)) {
     std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
                              base::nullopt);
     return;
   }
 
-  std::move(callback_).Run(response_code, std::move(parsed_response));
+  std::move(callback_).Run(response_code, std::move(device_response));
 }
 
 bool GetAssertionTask::CheckUserVerificationCompatible() {

device/fido/get_assertion_task.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <vector>
 
 #include "base/callback.h"
@@ -15,6 +16,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "device/fido/ctap_get_assertion_request.h"
+#include "device/fido/device_operation.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_task.h"
 
@@ -29,6 +31,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionTask : public FidoTask {
   using GetAssertionTaskCallback = base::OnceCallback<void(
       CtapDeviceResponseCode,
       base::Optional<AuthenticatorGetAssertionResponse>)>;
+  using SignOperation = DeviceOperation<CtapGetAssertionRequest,
+                                        AuthenticatorGetAssertionResponse>;
 
   GetAssertionTask(FidoDevice* device,
                    CtapGetAssertionRequest request,
@@ -75,9 +79,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) GetAssertionTask : public FidoTask {
   bool CheckUserVerificationCompatible();
 
   void OnCtapGetAssertionResponseReceived(
-      base::Optional<std::vector<uint8_t>> device_response);
+      CtapDeviceResponseCode response_code,
+      base::Optional<AuthenticatorGetAssertionResponse> device_response);
 
   CtapGetAssertionRequest request_;
+  std::unique_ptr<SignOperation> sign_operation_;
   GetAssertionTaskCallback callback_;
   base::WeakPtrFactory<GetAssertionTask> weak_factory_;
 

device/fido/make_credential_task.cc

@@ -8,8 +8,8 @@
 
 #include "base/bind.h"
 #include "device/base/features.h"
+#include "device/fido/ctap2_device_operation.h"
 #include "device/fido/ctap_empty_authenticator_request.h"
-#include "device/fido/ctap_register_operation.h"
 #include "device/fido/device_response_converter.h"
 #include "device/fido/u2f_command_constructor.h"
 #include "device/fido/u2f_register_operation.h"
@@ -48,10 +48,12 @@ void MakeCredentialTask::MakeCredential() {
     return;
   }
 
-  register_operation_ = std::make_unique<CtapRegisterOperation>(
-      device(), &request_parameter_,
+  register_operation_ = std::make_unique<Ctap2DeviceOperation<
+      CtapMakeCredentialRequest, AuthenticatorMakeCredentialResponse>>(
+      device(), request_parameter_,
       base::BindOnce(&MakeCredentialTask::OnCtapMakeCredentialResponseReceived,
-                     weak_factory_.GetWeakPtr()));
+                     weak_factory_.GetWeakPtr()),
+      base::BindOnce(&ReadCTAPMakeCredentialResponse));
   register_operation_->Start();
 }
 
@@ -66,10 +68,9 @@ void MakeCredentialTask::U2fRegister() {
   }
 
   register_operation_ = std::make_unique<U2fRegisterOperation>(
-      device(),
+      device(), request_parameter_,
       base::BindOnce(&MakeCredentialTask::OnCtapMakeCredentialResponseReceived,
-                     weak_factory_.GetWeakPtr()),
-      request_parameter_);
+                     weak_factory_.GetWeakPtr()));
   register_operation_->Start();
 }
 

device/fido/make_credential_task.h

@@ -31,9 +31,9 @@ class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialTask : public FidoTask {
   using MakeCredentialTaskCallback = base::OnceCallback<void(
       CtapDeviceResponseCode,
       base::Optional<AuthenticatorMakeCredentialResponse>)>;
-  using RegisterOperationPtr =
-      std::unique_ptr<DeviceOperation<CtapMakeCredentialRequest,
-                                      AuthenticatorMakeCredentialResponse>>;
+  using RegisterOperation =
+      DeviceOperation<CtapMakeCredentialRequest,
+                      AuthenticatorMakeCredentialResponse>;
 
   MakeCredentialTask(FidoDevice* device,
                      CtapMakeCredentialRequest request_parameter,
@@ -59,7 +59,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) MakeCredentialTask : public FidoTask {
 
   CtapMakeCredentialRequest request_parameter_;
   AuthenticatorSelectionCriteria authenticator_selection_criteria_;
-  RegisterOperationPtr register_operation_;
+  std::unique_ptr<RegisterOperation> register_operation_;
   MakeCredentialTaskCallback callback_;
   base::WeakPtrFactory<MakeCredentialTask> weak_factory_;
 

device/fido/u2f_register_operation.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "components/apdu/apdu_response.h"
 #include "device/fido/authenticator_make_credential_response.h"
@@ -21,24 +22,23 @@ namespace device {
 
 U2fRegisterOperation::U2fRegisterOperation(
     FidoDevice* device,
-    DeviceResponseCallback callback,
-    const CtapMakeCredentialRequest& request)
-    : DeviceOperation(device, std::move(callback)),
-      request_(request),
+    const CtapMakeCredentialRequest& request,
+    DeviceResponseCallback callback)
+    : DeviceOperation(device, request, std::move(callback)),
       weak_factory_(this) {}
 
 U2fRegisterOperation::~U2fRegisterOperation() = default;
 
 void U2fRegisterOperation::Start() {
-  DCHECK(IsConvertibleToU2fRegisterCommand(request_));
+  DCHECK(IsConvertibleToU2fRegisterCommand(request()));
 
-  const auto& exclude_list = request_.exclude_list();
+  const auto& exclude_list = request().exclude_list();
   if (!exclude_list || exclude_list->empty()) {
     TryRegistration(false /* is_duplicate_registration */);
   } else {
-    auto it = request_.exclude_list()->cbegin();
+    auto it = request().exclude_list()->cbegin();
     DispatchDeviceRequest(
-        ConvertToU2fCheckOnlySignCommand(request_, *it),
+        ConvertToU2fCheckOnlySignCommand(request(), *it),
         base::BindOnce(&U2fRegisterOperation::OnCheckForExcludedKeyHandle,
                        weak_factory_.GetWeakPtr(), it));
   }
@@ -47,7 +47,7 @@ void U2fRegisterOperation::Start() {
 void U2fRegisterOperation::TryRegistration(bool is_duplicate_registration) {
   auto command = is_duplicate_registration
                      ? ConstructBogusU2fRegistrationCommand()
-                     : ConvertToU2fRegisterCommand(request_);
+                     : ConvertToU2fRegisterCommand(request());
 
   DispatchDeviceRequest(
       std::move(command),
@@ -68,17 +68,18 @@ void U2fRegisterOperation::OnRegisterResponseReceived(
   switch (return_code) {
     case apdu::ApduResponse::Status::SW_NO_ERROR: {
       if (is_duplicate_registration) {
-        std::move(callback_).Run(
-            CtapDeviceResponseCode::kCtap2ErrCredentialExcluded, base::nullopt);
+        std::move(callback())
+            .Run(CtapDeviceResponseCode::kCtap2ErrCredentialExcluded,
+                 base::nullopt);
         break;
       }
 
       auto response =
           AuthenticatorMakeCredentialResponse::CreateFromU2fRegisterResponse(
-              fido_parsing_utils::CreateSHA256Hash(request_.rp().rp_id()),
+              fido_parsing_utils::CreateSHA256Hash(request().rp().rp_id()),
               apdu_response->data());
-      std::move(callback_).Run(CtapDeviceResponseCode::kSuccess,
-                               std::move(response));
+      std::move(callback())
+          .Run(CtapDeviceResponseCode::kSuccess, std::move(response));
       break;
     }
     case apdu::ApduResponse::Status::SW_CONDITIONS_NOT_SATISFIED:
@@ -92,8 +93,8 @@ void U2fRegisterOperation::OnRegisterResponseReceived(
       break;
     default:
       // An error has occurred, quit trying this device.
-      std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
-                               base::nullopt);
+      std::move(callback())
+          .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
       break;
   }
 }
@@ -123,16 +124,16 @@ void U2fRegisterOperation::OnCheckForExcludedKeyHandle(
     case apdu::ApduResponse::Status::SW_WRONG_DATA:
       // Continue to iterate through the provided key handles in the exclude
       // list and check for already registered keys.
-      if (++it != request_.exclude_list()->cend()) {
+      if (++it != request().exclude_list()->cend()) {
         DispatchDeviceRequest(
-            ConvertToU2fCheckOnlySignCommand(request_, *it),
+            ConvertToU2fCheckOnlySignCommand(request(), *it),
             base::BindOnce(&U2fRegisterOperation::OnCheckForExcludedKeyHandle,
                            weak_factory_.GetWeakPtr(), it));
       } else {
         // Reached the end of exclude list with no duplicate credential.
         // Proceed with registration.
         DispatchDeviceRequest(
-            ConvertToU2fRegisterCommand(request_),
+            ConvertToU2fRegisterCommand(request()),
             base::BindOnce(&U2fRegisterOperation::OnRegisterResponseReceived,
                            weak_factory_.GetWeakPtr(),
                            false /* is_duplicate_registration */));
@@ -140,8 +141,8 @@ void U2fRegisterOperation::OnCheckForExcludedKeyHandle(
       break;
     default:
       // Some sort of failure occurred. Silently drop device request.
-      std::move(callback_).Run(CtapDeviceResponseCode::kCtap2ErrOther,
-                               base::nullopt);
+      std::move(callback())
+          .Run(CtapDeviceResponseCode::kCtap2ErrOther, base::nullopt);
       break;
   }
 }

device/fido/u2f_register_operation.h

@@ -20,6 +20,7 @@ namespace device {
 class FidoDevice;
 class CtapMakeCredentialRequest;
 class AuthenticatorMakeCredentialResponse;
+class PublicKeyCredentialDescriptor;
 
 // Represents per device registration logic for U2F tokens. Handles regular U2F
 // registration as well as the logic of iterating key handles in the exclude
@@ -31,8 +32,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fRegisterOperation
                              AuthenticatorMakeCredentialResponse> {
  public:
   U2fRegisterOperation(FidoDevice* device,
-                       DeviceResponseCallback callback,
-                       const CtapMakeCredentialRequest& request);
+                       const CtapMakeCredentialRequest& request,
+                       DeviceResponseCallback callback);
   ~U2fRegisterOperation() override;
 
   // DeviceOperation:
@@ -50,8 +51,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fRegisterOperation
       ExcludeListIterator it,
       base::Optional<std::vector<uint8_t>> device_response);
 
-  const CtapMakeCredentialRequest& request_;
-
   base::WeakPtrFactory<U2fRegisterOperation> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(U2fRegisterOperation);

device/fido/u2f_register_operation_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/test/scoped_task_environment.h"
 #include "device/fido/authenticator_make_credential_response.h"
+#include "device/fido/ctap_make_credential_request.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_parsing_utils.h"
 #include "device/fido/fido_test_data.h"
@@ -76,8 +77,8 @@ TEST_F(U2fRegisterOperationTest, TestRegisterSuccess) {
       test_data::kApduEncodedNoErrorRegisterResponse);
 
   auto u2f_register = std::make_unique<U2fRegisterOperation>(
-      device.get(), register_callback_receiver().callback(),
-      std::move(request));
+      device.get(), std::move(request),
+      register_callback_receiver().callback());
   u2f_register->Start();
   register_callback_receiver().WaitForCallback();
 
@@ -93,8 +94,8 @@ TEST_F(U2fRegisterOperationTest, TestRegisterSuccessWithFake) {
 
   auto device = std::make_unique<VirtualU2fDevice>();
   auto u2f_register = std::make_unique<U2fRegisterOperation>(
-      device.get(), register_callback_receiver().callback(),
-      std::move(request));
+      device.get(), std::move(request),
+      register_callback_receiver().callback());
   u2f_register->Start();
   register_callback_receiver().WaitForCallback();
 
@@ -123,8 +124,8 @@ TEST_F(U2fRegisterOperationTest, TestDelayedSuccess) {
       test_data::kApduEncodedNoErrorRegisterResponse);
 
   auto u2f_register = std::make_unique<U2fRegisterOperation>(
-      device.get(), register_callback_receiver().callback(),
-      std::move(request));
+      device.get(), std::move(request),
+      register_callback_receiver().callback());
   u2f_register->Start();
   register_callback_receiver().WaitForCallback();
 
@@ -166,8 +167,8 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithExclusionList) {
       test_data::kApduEncodedNoErrorRegisterResponse);
 
   auto u2f_register = std::make_unique<U2fRegisterOperation>(
-      device.get(), register_callback_receiver().callback(),
-      std::move(request));
+      device.get(), std::move(request),
+      register_callback_receiver().callback());
   u2f_register->Start();
   register_callback_receiver().WaitForCallback();
 
@@ -218,8 +219,8 @@ TEST_F(U2fRegisterOperationTest, TestRegistrationWithDuplicateHandle) {
       test_data::kApduEncodedNoErrorRegisterResponse);
 
   auto u2f_register = std::make_unique<U2fRegisterOperation>(
-      device.get(), register_callback_receiver().callback(),
-      std::move(request));
+      device.get(), std::move(request),
+      register_callback_receiver().callback());
   u2f_register->Start();
   register_callback_receiver().WaitForCallback();
 
@@ -250,7 +251,7 @@ TEST_F(U2fRegisterOperationTest, TestIndividualAttestation) {
         test_data::kApduEncodedNoErrorRegisterResponse);
 
     auto u2f_register = std::make_unique<U2fRegisterOperation>(
-        device.get(), cb.callback(), std::move(request));
+        device.get(), std::move(request), cb.callback());
     u2f_register->Start();
     cb.WaitForCallback();