ChromeOS cryptohome should be able to use gaia id as user identifier.

chrome/browser/chromeos/app_mode/kiosk_app_manager.cc

 // Copyright 2013 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 "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
 
 #include <stddef.h>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 17 matching lines @@
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/extensions/external_loader.h"
 #include "chrome/browser/extensions/external_provider_impl.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chromeos/chromeos_paths.h"
 #include "chromeos/cryptohome/async_method_caller.h"
+#include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/ownership/owner_key_util.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/signin/core/account_id/account_id.h"
+#include "components/user_manager/known_user.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/common/extension_urls.h"
 
 namespace chromeos {
 
 namespace {
 
 // Domain that is used for kiosk-app account IDs.
 const char kKioskAppAccountDomain[] = "kiosk-apps";
 
 // Preference for the dictionary of user ids for which cryptohomes have to be
 // removed upon browser restart.
 const char kKioskUsersToRemove[] = "kiosk-users-to-remove";
 
 std::string GenerateKioskAppAccountId(const std::string& app_id) {
   return app_id + '@' + kKioskAppAccountDomain;
 }
 
-void ScheduleDelayedCryptohomeRemoval(const std::string& user_id,
+void ScheduleDelayedCryptohomeRemoval(const AccountId& account_id,
                                       const std::string& app_id) {
   PrefService* local_state = g_browser_process->local_state();
   DictionaryPrefUpdate dict_update(local_state, kKioskUsersToRemove);
-  dict_update->SetStringWithoutPathExpansion(user_id, app_id);
+  dict_update->SetStringWithoutPathExpansion(account_id.Serialize(), app_id);

[xiyuan, 2016/02/17 23:14:25]

The comment for Serialize() in account_id.h says:

  // These are (for now) unstable and cannot be used to store serialized data to
  // persistent storage. Only in-memory storage is safe.

Should the comments be updated? Or we should not use it as the serialized data
would be persisted here?

[Alexander Alekseev, 2016/02/18 13:45:14]

You're right. We should not use it here. I've replaced it with
cryptohome::Identification.

   local_state->CommitPendingWrite();
 }
 
-void CancelDelayedCryptohomeRemoval(const std::string& user_id) {
+void CancelDelayedCryptohomeRemoval(const AccountId& account_id) {
   PrefService* local_state = g_browser_process->local_state();
   DictionaryPrefUpdate dict_update(local_state, kKioskUsersToRemove);
-  dict_update->RemoveWithoutPathExpansion(user_id, NULL);
+  dict_update->RemoveWithoutPathExpansion(account_id.Serialize(), NULL);
+  const std::string& old_user_id = account_id.GetUserEmail();  // Migrated
+  dict_update->RemoveWithoutPathExpansion(old_user_id, NULL);
   local_state->CommitPendingWrite();
 }
 
-void OnRemoveAppCryptohomeComplete(const std::string& user_id,
+void OnRemoveAppCryptohomeComplete(const AccountId& account_id,
                                    const std::string& app,
                                    const base::Closure& callback,
                                    bool success,
                                    cryptohome::MountError return_code) {
   if (success) {
-    CancelDelayedCryptohomeRemoval(user_id);
+    CancelDelayedCryptohomeRemoval(account_id);
   } else {
-    ScheduleDelayedCryptohomeRemoval(user_id, app);
+    ScheduleDelayedCryptohomeRemoval(account_id, app);
     LOG(ERROR) << "Remove cryptohome for " << app
         << " failed, return code: " << return_code;
   }
   if (!callback.is_null())
     callback.Run();
 }
 
 void PerformDelayedCryptohomeRemovals(bool service_is_available) {
   if (!service_is_available) {
     LOG(ERROR) << "Crypthomed is not available.";
     return;
   }
 
   PrefService* local_state = g_browser_process->local_state();
   const base::DictionaryValue* dict =
       local_state->GetDictionary(kKioskUsersToRemove);
   for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
     std::string user_id = it.key();
+    const AccountId account_id(
+        user_manager::known_user::MayBeDeserializeAccountId(user_id));
     std::string app_id;
     it.value().GetAsString(&app_id);
     VLOG(1) << "Removing obsolete crypthome for " << app_id;
     cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove(
-        user_id,
-        base::Bind(&OnRemoveAppCryptohomeComplete,
-                   user_id,
-                   app_id,
+        cryptohome::Identification(account_id),
+        base::Bind(&OnRemoveAppCryptohomeComplete, account_id, app_id,
                    base::Closure()));
   }
 }
 
 // Check for presence of machine owner public key file.
 void CheckOwnerFilePresence(bool *present) {
   scoped_refptr<ownership::OwnerKeyUtil> util =
       OwnerSettingsServiceChromeOSFactory::GetInstance()->GetOwnerKeyUtil();
   *present = util.get() && util->IsPublicKeyPresent();
 }
@@ skipping 36 matching lines @@
 }
 
 // static
 void KioskAppManager::RemoveObsoleteCryptohomes() {
   chromeos::CryptohomeClient* client =
       chromeos::DBusThreadManager::Get()->GetCryptohomeClient();
   client->WaitForServiceToBeAvailable(
       base::Bind(&PerformDelayedCryptohomeRemovals));
 }
 
-KioskAppManager::App::App(
-    const KioskAppData& data,
-    bool is_extension_pending,
-    bool auto_launched_with_zero_delay)
+KioskAppManager::App::App(const KioskAppData& data,
+                          bool is_extension_pending,
+                          bool auto_launched_with_zero_delay)
     : app_id(data.app_id()),
-      user_id(data.user_id()),
+      account_id(data.account_id()),
       name(data.name()),
       icon(data.icon()),
       is_loading(data.IsLoading() || is_extension_pending),
-      was_auto_launched_with_zero_delay(auto_launched_with_zero_delay) {
-}
+      was_auto_launched_with_zero_delay(auto_launched_with_zero_delay) {}
 
-KioskAppManager::App::App() : is_loading(false),
-                              was_auto_launched_with_zero_delay(false) {}
+KioskAppManager::App::App()
+    : account_id(EmptyAccountId()),
+      is_loading(false),
+      was_auto_launched_with_zero_delay(false) {}
 
 KioskAppManager::App::~App() {}
 
 std::string KioskAppManager::GetAutoLaunchApp() const {
   return auto_launch_app_id_;
 }
 
 void KioskAppManager::SetAutoLaunchApp(const std::string& app_id,
                                        OwnerSettingsServiceChromeOS* service) {
   SetAutoLoginState(AUTOLOGIN_REQUESTED);
@@ skipping 433 matching lines @@
       policy::GetDeviceLocalAccounts(CrosSettings::Get());
   for (std::vector<policy::DeviceLocalAccount>::const_iterator
            it = device_local_accounts.begin();
        it != device_local_accounts.end(); ++it) {
     if (it->type != policy::DeviceLocalAccount::TYPE_KIOSK_APP)
       continue;
 
     if (it->account_id == auto_login_account_id)
       auto_launch_app_id_ = it->kiosk_app_id;
 
+    const AccountId account_id(AccountId::FromUserEmail(it->user_id));

[xiyuan, 2016/02/17 23:14:25]

Note it->user_id could be an arbitrary string set by the admin and not
necessarily a valid email. And there will never be a gaia id for it.

[Alexander Alekseev, 2016/02/18 13:45:14]

It cannot be completely arbitrary, because we use it as cryptohome identifier.
At least session_manager has checks for this string to look like regular email.

As a part of migration to AccountId structure, we are replacing all strings that
are used "as user email" with AccountId::FromUserEmail(). So we have many
account ids with empty GaiaId. And this is why AccountId doesn't check this
string to be a valid e-mail.

The plan is to do further refactoring and introduce different account types (as
we have AccountId::GetAccountType) for regular/guest/kiosk/etc... But for now we
are moving away from std::string as user identifier everywhere.

[xiyuan, 2016/02/18 17:22:33]

I am not objecting to the change. Just want to point out that the AccountId for
kiosk is a bit different.

[Alexander Alekseev, 2016/02/19 00:17:03]

You're right. But it still looks like email. It cannot be canonicalized though.

     std::map<std::string, KioskAppData*>::iterator old_it =
         old_apps.find(it->kiosk_app_id);
     if (old_it != old_apps.end()) {
       apps_.push_back(old_it->second);
       old_apps.erase(old_it);
     } else {
       KioskAppData* new_app = new KioskAppData(
-          this, it->kiosk_app_id, it->user_id, GURL(it->kiosk_app_update_url));
+          this, it->kiosk_app_id, account_id, GURL(it->kiosk_app_update_url));
       apps_.push_back(new_app);  // Takes ownership of |new_app|.
       new_app->Load();
     }
-    CancelDelayedCryptohomeRemoval(it->user_id);
+    CancelDelayedCryptohomeRemoval(account_id);
   }
 
   base::Closure cryptohomes_barrier_closure;
 
   const user_manager::User* active_user =
       user_manager::UserManager::Get()->GetActiveUser();
   if (active_user) {
     const AccountId active_account_id = active_user->GetAccountId();
     for (const auto& it : old_apps) {
-      if (it.second->user_id() == active_account_id.GetUserEmail()) {
+      if (it.second->account_id() == active_account_id) {
         VLOG(1) << "Currently running kiosk app removed from policy, exiting";
         cryptohomes_barrier_closure = BarrierClosure(
             old_apps.size(), base::Bind(&chrome::AttemptUserExit));
         break;
       }
     }
   }
 
   // Clears cache and deletes the remaining old data.
   std::vector<std::string> apps_to_remove;
   for (std::map<std::string, KioskAppData*>::iterator it = old_apps.begin();
        it != old_apps.end(); ++it) {
     it->second->ClearCache();
     cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove(
-        it->second->user_id(),
-        base::Bind(&OnRemoveAppCryptohomeComplete,
-                   it->second->user_id(),
-                   it->first,
-                   cryptohomes_barrier_closure));
+        cryptohome::Identification(it->second->account_id()),
+        base::Bind(&OnRemoveAppCryptohomeComplete, it->second->account_id(),
+                   it->first, cryptohomes_barrier_closure));
     apps_to_remove.push_back(it->second->app_id());
   }
   STLDeleteValues(&old_apps);
   external_cache_->RemoveExtensions(apps_to_remove);
 
   // Request external_cache_ to download new apps and update the existing
   // apps.
   scoped_ptr<base::DictionaryValue> prefs(new base::DictionaryValue);
   for (size_t i = 0; i < apps_.size(); ++i) {
     scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue);
@@ skipping 90 matching lines @@
   *cache_dir = user_data_dir.AppendASCII(kCrxCacheDir);
 }
 
 void KioskAppManager::GetCrxUnpackDir(base::FilePath* unpack_dir) {
   base::FilePath temp_dir;
   base::GetTempDir(&temp_dir);
   *unpack_dir = temp_dir.AppendASCII(kCrxUnpackDir);
 }
 
 }  // namespace chromeos