Make AddKeyEx pass-through.

AddKeyEx now takes AuthorizationRequest and AddKeyRequest directly.

Bug: 741274
Change-Id: I200b7bd8255010b51376d15bcff3e190b651fb13
Reviewed-on: https://chromium-review.googlesource.com/776518
Commit-Queue: Maajid <maajid@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517762}

chrome/browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc

@@ -338,17 +338,18 @@ void EasyUnlockCreateKeysOperation::OnGetSystemSalt(
       kEasyUnlockKeyMetaNameSerializedBeaconSeeds,
       device->serialized_beacon_seeds));
 
-  // Add cryptohome key.
-  const cryptohome::Identification id(user_context_.GetAccountId());
-
   std::unique_ptr<Key> auth_key(new Key(*user_context_.GetKey()));
   if (auth_key->GetKeyType() == Key::KEY_TYPE_PASSWORD_PLAIN)
     auth_key->Transform(Key::KEY_TYPE_SALTED_SHA256_TOP_HALF, system_salt);
 
-  cryptohome::Authorization auth(auth_key->GetSecret(), auth_key->GetLabel());
+  cryptohome::AddKeyRequest request;
+  KeyDefinitionToKey(key_def, request.mutable_key());
+  request.set_clobber_if_exists(true);
   cryptohome::HomedirMethods::GetInstance()->AddKeyEx(
-      id, auth, key_def,
-      true,  // clobber
+      cryptohome::Identification(user_context_.GetAccountId()),
+      cryptohome::CreateAuthorizationRequest(auth_key->GetLabel(),
+                                             auth_key->GetSecret()),
+      request,
       base::Bind(&EasyUnlockCreateKeysOperation::OnKeyCreated,
                  weak_ptr_factory_.GetWeakPtr(), index, user_key));
 }

chrome/browser/chromeos/login/supervised/supervised_user_creation_browsertest.cc

@@ -203,7 +203,7 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserTransactionCleanupTest,
   mock_homedir_methods_->set_mount_callback(mount_wait_loop.QuitClosure());
   mock_homedir_methods_->set_add_key_callback(add_key_wait_loop.QuitClosure());
   EXPECT_CALL(*mock_homedir_methods_, MountEx(_, _, _, _)).Times(1);
-  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _, _)).Times(1);
+  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _)).Times(1);
 
   JSEval("$('supervised-user-creation-next-button').click()");
 

chrome/browser/chromeos/login/supervised/supervised_user_password_browsertest.cc

@@ -134,7 +134,7 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserPasswordTest,
 
   SigninAsManager(1);
 
-  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _, _)).Times(1);
+  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _)).Times(1);
 
   std::string sync_id =
       ChromeUserManager::Get()->GetSupervisedUserManager()->GetUserSyncId(
@@ -223,7 +223,7 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserPasswordTest,
 
   SigninAsManager(1);
 
-  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _, _)).Times(1);
+  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _)).Times(1);
 
   std::string sync_id =
       ChromeUserManager::Get()->GetSupervisedUserManager()->GetUserSyncId(

chrome/browser/chromeos/login/supervised/supervised_user_test_base.cc

@@ -389,7 +389,7 @@ void SupervisedUserTestBase::StartUserCreation(
   mock_homedir_methods_->set_mount_callback(mount_wait_loop.QuitClosure());
   mock_homedir_methods_->set_add_key_callback(add_key_wait_loop.QuitClosure());
   EXPECT_CALL(*mock_homedir_methods_, MountEx(_, _, _, _)).Times(1);
-  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _, _)).Times(1);
+  EXPECT_CALL(*mock_homedir_methods_, AddKeyEx(_, _, _, _)).Times(1);
 
   JSEval(std::string("$('").append(button_id).append("').click()"));
 

chromeos/cryptohome/homedir_methods.cc

@@ -129,16 +129,11 @@ class HomedirMethodsImpl : public HomedirMethods {
   }
 
   void AddKeyEx(const Identification& id,
-                const Authorization& auth,
-                const KeyDefinition& new_key,
-                bool clobber_if_exists,
+                const AuthorizationRequest& auth,
+                const AddKeyRequest& request,
                 const Callback& callback) override {
-    cryptohome::AddKeyRequest request;
-    KeyDefinitionToKey(new_key, request.mutable_key());
-    request.set_clobber_if_exists(clobber_if_exists);
-
     DBusThreadManager::Get()->GetCryptohomeClient()->AddKeyEx(
-        id, CreateAuthorizationRequest(auth.label, auth.key), request,
+        id, auth, request,
         base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback,
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
@@ -427,6 +422,7 @@ cryptohome::AuthorizationRequest CreateAuthorizationRequest(
   Key* key = auth_request.mutable_key();
   if (!label.empty())
     key->mutable_data()->set_label(label);
+
   key->set_secret(secret);
   return auth_request;
 }

chromeos/cryptohome/homedir_methods.h

@@ -25,7 +25,7 @@ void CHROMEOS_EXPORT KeyDefinitionToKey(const KeyDefinition& key_def, Key* key);
 
 // Creates an AuthorizationRequest from the given secret and label.
 AuthorizationRequest CHROMEOS_EXPORT
-CreateAuthorizationRequest(const std::string& secret, const std::string& label);
+CreateAuthorizationRequest(const std::string& label, const std::string& secret);
 
 // This class manages calls to Cryptohome service's home directory methods:
 // Mount, CheckKey, Add/UpdateKey.
@@ -74,17 +74,13 @@ class CHROMEOS_EXPORT HomedirMethods {
                        const MountRequest& request,
                        const MountCallback& callback) = 0;
 
-  // Asks cryptohomed to try to add another |key| for user identified by |id|
+  // Asks cryptohomed to try to add another key for the user identified by |id|
   // using |auth| to unlock the key.
-  // |clobber_if_exist| governs action if key with same label already exists for
-  // this user. if |true| old key will be replaced, if  |false| old key will be
-  // preserved.
-  // Key used in |auth| should have PRIV_ADD privilege.
+  // Key used in |auth| should have the PRIV_ADD privilege.
   // |callback| will be called with status info on completion.
   virtual void AddKeyEx(const Identification& id,
-                        const Authorization& auth,
-                        const KeyDefinition& key,
-                        bool clobber_if_exist,
+                        const AuthorizationRequest& auth,
+                        const AddKeyRequest& request,
                         const Callback& callback) = 0;
 
   // Asks cryptohomed to update |key| for user identified by |id| using |auth|

chromeos/cryptohome/mock_homedir_methods.cc

@@ -29,9 +29,9 @@ void MockHomedirMethods::SetUp(bool success, MountError return_code) {
           WithArgs<3>(Invoke(this, &MockHomedirMethods::DoCallback)));
   ON_CALL(*this, MountEx(_, _, _, _)).WillByDefault(
       WithArgs<3>(Invoke(this, &MockHomedirMethods::DoMountCallback)));
-  ON_CALL(*this, AddKeyEx(_, _, _, _, _))
+  ON_CALL(*this, AddKeyEx(_, _, _, _))
       .WillByDefault(
-          WithArgs<4>(Invoke(this, &MockHomedirMethods::DoAddKeyCallback)));
+          WithArgs<3>(Invoke(this, &MockHomedirMethods::DoAddKeyCallback)));
   ON_CALL(*this, UpdateKeyEx(_, _, _, _, _)).WillByDefault(
       WithArgs<4>(Invoke(this, &MockHomedirMethods::DoCallback)));
   ON_CALL(*this, RemoveKeyEx(_, _, _, _)).WillByDefault(

chromeos/cryptohome/mock_homedir_methods.h

@@ -37,11 +37,10 @@ class CHROMEOS_EXPORT MockHomedirMethods : public HomedirMethods {
                     const AuthorizationRequest& auth,
                     const MountRequest& request,
                     const MountCallback& callback));
-  MOCK_METHOD5(AddKeyEx,
+  MOCK_METHOD4(AddKeyEx,
                void(const Identification& id,
-                    const Authorization& auth,
-                    const KeyDefinition& key,
-                    bool clobber_if_exist,
+                    const AuthorizationRequest& auth,
+                    const AddKeyRequest& request,
                     const Callback& callback));
   MOCK_METHOD4(RemoveKeyEx,
                void(const Identification& id,

chromeos/login/auth/extended_authenticator_impl.cc

@@ -114,15 +114,12 @@ void ExtendedAuthenticatorImpl::CreateMount(
 }
 
 void ExtendedAuthenticatorImpl::AddKey(const UserContext& context,
-                                   const cryptohome::KeyDefinition& key,
-                                   bool replace_existing,
-                                   const base::Closure& success_callback) {
-  TransformKeyIfNeeded(context,
-                       base::Bind(&ExtendedAuthenticatorImpl::DoAddKey,
-                                  this,
-                                  key,
-                                  replace_existing,
-                                  success_callback));
+                                       const cryptohome::KeyDefinition& key,
+                                       bool clobber_if_exists,
+                                       const base::Closure& success_callback) {
+  TransformKeyIfNeeded(
+      context, base::Bind(&ExtendedAuthenticatorImpl::DoAddKey, this, key,
+                          clobber_if_exists, success_callback));
 }
 
 void ExtendedAuthenticatorImpl::UpdateKeyAuthorized(
@@ -214,25 +211,22 @@ void ExtendedAuthenticatorImpl::DoAuthenticateToCheck(
 }
 
 void ExtendedAuthenticatorImpl::DoAddKey(const cryptohome::KeyDefinition& key,
-                                     bool replace_existing,
-                                     const base::Closure& success_callback,
-                                     const UserContext& user_context) {
+                                         bool clobber_if_exists,
+                                         const base::Closure& success_callback,
+                                         const UserContext& user_context) {
   RecordStartMarker("AddKeyEx");
 
-  cryptohome::Identification id(user_context.GetAccountId());
+  cryptohome::AddKeyRequest request;
+  KeyDefinitionToKey(key, request.mutable_key());
+  request.set_clobber_if_exists(clobber_if_exists);
   const Key* const auth_key = user_context.GetKey();
-  cryptohome::Authorization auth(auth_key->GetSecret(), auth_key->GetLabel());
-
   cryptohome::HomedirMethods::GetInstance()->AddKeyEx(
-      id,
-      auth,
-      key,
-      replace_existing,
-      base::Bind(&ExtendedAuthenticatorImpl::OnOperationComplete,
-                 this,
-                 "AddKeyEx",
-                 user_context,
-                 success_callback));
+      cryptohome::Identification(user_context.GetAccountId()),
+      cryptohome::CreateAuthorizationRequest(auth_key->GetLabel(),
+                                             auth_key->GetSecret()),
+      request,
+      base::Bind(&ExtendedAuthenticatorImpl::OnOperationComplete, this,
+                 "AddKeyEx", user_context, success_callback));
 }
 
 void ExtendedAuthenticatorImpl::DoUpdateKeyAuthorized(

chromeos/login/auth/extended_authenticator_impl.h

@@ -39,7 +39,7 @@ class CHROMEOS_EXPORT ExtendedAuthenticatorImpl : public ExtendedAuthenticator {
                    const ResultCallback& success_callback) override;
   void AddKey(const UserContext& context,
               const cryptohome::KeyDefinition& key,
-              bool replace_existing,
+              bool clobber_if_exists,
               const base::Closure& success_callback) override;
   void UpdateKeyAuthorized(const UserContext& context,
                            const cryptohome::KeyDefinition& key,
@@ -63,7 +63,7 @@ class CHROMEOS_EXPORT ExtendedAuthenticatorImpl : public ExtendedAuthenticator {
   void DoAuthenticateToCheck(const base::Closure& success_callback,
                              const UserContext& context);
   void DoAddKey(const cryptohome::KeyDefinition& key,
-                bool replace_existing,
+                bool clobber_if_exists,
                 const base::Closure& success_callback,
                 const UserContext& context);
   void DoUpdateKeyAuthorized(const cryptohome::KeyDefinition& key,