Unify AuthorizationRequest creation methods.

This change should make it easier to make HomeDirMethods completely
pass-through, and eventually the method should live with CryptohomeClient.

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

chromeos/cryptohome/homedir_methods.cc

@@ -25,16 +25,6 @@ namespace {
 
 HomedirMethods* g_homedir_methods = NULL;
 
-// Fill authorization protobuffer.
-void FillAuthorizationProtobuf(const Authorization& auth,
-                               cryptohome::AuthorizationRequest* auth_proto) {
-  Key* key = auth_proto->mutable_key();
-  if (!auth.label.empty()) {
-    key->mutable_data()->set_label(auth.label);
-  }
-  key->set_secret(auth.key);
-}
-
 void ParseAuthorizationDataProtobuf(
     const KeyAuthorizationData& authorization_data_proto,
     KeyDefinition::AuthorizationData* authorization_data) {
@@ -143,15 +133,12 @@ class HomedirMethodsImpl : public HomedirMethods {
                 const KeyDefinition& new_key,
                 bool clobber_if_exists,
                 const Callback& callback) override {
-    cryptohome::AuthorizationRequest auth_proto;
     cryptohome::AddKeyRequest request;
-
-    FillAuthorizationProtobuf(auth, &auth_proto);
     KeyDefinitionToKey(new_key, request.mutable_key());
     request.set_clobber_if_exists(clobber_if_exists);
 
     DBusThreadManager::Get()->GetCryptohomeClient()->AddKeyEx(
-        id, auth_proto, request,
+        id, CreateAuthorizationRequest(auth.label, auth.key), request,
         base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback,
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
@@ -160,14 +147,12 @@ class HomedirMethodsImpl : public HomedirMethods {
                    const Authorization& auth,
                    const std::string& label,
                    const Callback& callback) override {
-    cryptohome::AuthorizationRequest auth_proto;
     cryptohome::RemoveKeyRequest request;
 
-    FillAuthorizationProtobuf(auth, &auth_proto);
     request.mutable_key()->mutable_data()->set_label(label);
 
     DBusThreadManager::Get()->GetCryptohomeClient()->RemoveKeyEx(
-        id, auth_proto, request,
+        id, CreateAuthorizationRequest(auth.label, auth.key), request,
         base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback,
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
@@ -177,15 +162,13 @@ class HomedirMethodsImpl : public HomedirMethods {
                    const KeyDefinition& new_key,
                    const std::string& signature,
                    const Callback& callback) override {
-    cryptohome::AuthorizationRequest auth_proto;
     cryptohome::UpdateKeyRequest pb_update_key;
 
-    FillAuthorizationProtobuf(auth, &auth_proto);
     KeyDefinitionToKey(new_key, pb_update_key.mutable_changes());
     pb_update_key.set_authorization_signature(signature);
 
     DBusThreadManager::Get()->GetCryptohomeClient()->UpdateKeyEx(
-        id, auth_proto, pb_update_key,
+        id, CreateAuthorizationRequest(auth.label, auth.key), pb_update_key,
         base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback,
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
@@ -437,6 +420,17 @@ void KeyDefinitionToKey(const KeyDefinition& key_def, Key* key) {
   }
 }
 
+cryptohome::AuthorizationRequest CreateAuthorizationRequest(
+    const std::string& label,
+    const std::string& secret) {
+  cryptohome::AuthorizationRequest auth_request;
+  Key* key = auth_request.mutable_key();
+  if (!label.empty())
+    key->mutable_data()->set_label(label);
+  key->set_secret(secret);
+  return auth_request;
+}
+
 // static
 void HomedirMethods::Initialize() {
   if (g_homedir_methods) {

chromeos/cryptohome/homedir_methods.h

@@ -23,6 +23,10 @@ namespace cryptohome {
 // Converts the given KeyDefinition to a Key.
 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);
+
 // This class manages calls to Cryptohome service's home directory methods:
 // Mount, CheckKey, Add/UpdateKey.
 class CHROMEOS_EXPORT HomedirMethods {

chromeos/login/auth/extended_authenticator_impl.cc

@@ -191,42 +191,24 @@ void ExtendedAuthenticatorImpl::DoAuthenticateToMount(
     const ResultCallback& success_callback,
     const UserContext& user_context) {
   RecordStartMarker("MountEx");
-
-  cryptohome::Identification id(user_context.GetAccountId());
   const Key* const key = user_context.GetKey();
-  cryptohome::MountRequest mount;
-  cryptohome::AuthorizationRequest auth;
-  cryptohome::Key* auth_key = auth.mutable_key();
-  if (!key->GetLabel().empty()) {
-    auth_key->mutable_data()->set_label(key->GetLabel());
-  }
-  auth_key->set_secret(key->GetSecret());
   cryptohome::HomedirMethods::GetInstance()->MountEx(
-      id,
-      auth,
-      mount,
-      base::Bind(&ExtendedAuthenticatorImpl::OnMountComplete,
-                 this,
-                 "MountEx",
-                 user_context,
-                 success_callback));
+      cryptohome::Identification(user_context.GetAccountId()),
+      cryptohome::CreateAuthorizationRequest(key->GetLabel(), key->GetSecret()),
+      cryptohome::MountRequest(),
+      base::Bind(&ExtendedAuthenticatorImpl::OnMountComplete, this, "MountEx",
+                 user_context, success_callback));
 }
 
 void ExtendedAuthenticatorImpl::DoAuthenticateToCheck(
     const base::Closure& success_callback,
     const UserContext& user_context) {
   RecordStartMarker("CheckKeyEx");
-
-  cryptohome::Identification id(user_context.GetAccountId());
   const Key* const key = user_context.GetKey();
-  cryptohome::AuthorizationRequest auth;
-  cryptohome::Key* auth_key = auth.mutable_key();
-  if (!key->GetLabel().empty()) {
-    auth_key->mutable_data()->set_label(key->GetLabel());
-  }
-  auth_key->set_secret(key->GetSecret());
   cryptohome::HomedirMethods::GetInstance()->CheckKeyEx(
-      id, auth, cryptohome::CheckKeyRequest(),
+      cryptohome::Identification(user_context.GetAccountId()),
+      cryptohome::CreateAuthorizationRequest(key->GetLabel(), key->GetSecret()),
+      cryptohome::CheckKeyRequest(),
       base::Bind(&ExtendedAuthenticatorImpl::OnOperationComplete, this,
                  "CheckKeyEx", user_context, success_callback));
 }