Implement ephemeral users

chrome/browser/chromeos/login/user_manager.cc

Index: chrome/browser/chromeos/login/user_manager.cc
diff --git a/chrome/browser/chromeos/login/user_manager.cc b/chrome/browser/chromeos/login/user_manager.cc
index 3b56995820865b357fa56d6669ff451387731f35..744d8d06c284cd853ed9df382a45f50430cbab39 100644
--- a/chrome/browser/chromeos/login/user_manager.cc
+++ b/chrome/browser/chromeos/login/user_manager.cc
@@ -36,6 +36,7 @@
 #include "chrome/browser/chromeos/login/ownership_service.h"
 #include "chrome/browser/chromeos/login/remove_user_delegate.h"
 #include "chrome/browser/chromeos/system/runtime_environment.h"
+#include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile_downloader.h"
@@ -322,15 +323,24 @@ void UserManager::UserLoggedIn(const std::string& email) {
   user_is_logged_in_ = true;
 
   if (email == kGuestUser) {
+    current_user_is_ephemeral_ = true;
     GuestUserLoggedIn();
     return;
   }
 
   if (email == kDemoUser) {
+    current_user_is_ephemeral_ = true;
     DemoUserLoggedIn();
     return;
   }
 
+  if (IsEphemeralUser(email)) {
+    current_user_is_ephemeral_ = true;
+    logged_in_user_ = CreateUser(email);
+    NotifyOnLogin();
+    return;
+  }
+
   EnsureUsersLoaded();
 
   // Clear the prefs view of the users.
@@ -480,15 +490,9 @@ bool UserManager::IsKnownUser(const std::string& email) const {
 }
 
 const User* UserManager::FindUser(const std::string& email) const {
-  // Speed up search by checking the logged-in user first.
   if (logged_in_user_ && logged_in_user_->email() == email)
     return logged_in_user_;
-  const UserList& users = GetUsers();
-  for (UserList::const_iterator it = users.begin(); it != users.end(); ++it) {
-    if ((*it)->email() == email)
-      return *it;
-  }
-  return NULL;
+  return FindUserInList(email);
 }
 
 bool UserManager::IsDisplayNameUnique(const std::string& display_name) const {
@@ -499,14 +503,21 @@ void UserManager::SaveUserOAuthStatus(
     const std::string& username,
     User::OAuthTokenStatus oauth_token_status) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  PrefService* local_state = g_browser_process->local_state();
-  DictionaryPrefUpdate oauth_status_update(local_state, kUserOAuthTokenStatus);
-  oauth_status_update->SetWithoutPathExpansion(username,
-      new base::FundamentalValue(static_cast<int>(oauth_token_status)));
+
   DVLOG(1) << "Saving user OAuth token status in Local State";
   User* user = const_cast<User*>(FindUser(username));
   if (user)
     user->set_oauth_token_status(oauth_token_status);
+
+  // Do not update local store if the user is ephemeral.
+  if (IsEphemeralUser(username))
+    return;
+
+  PrefService* local_state = g_browser_process->local_state();
+
+  DictionaryPrefUpdate oauth_status_update(local_state, kUserOAuthTokenStatus);
+  oauth_status_update->SetWithoutPathExpansion(username,
+      new base::FundamentalValue(static_cast<int>(oauth_token_status)));
 }
 
 User::OAuthTokenStatus UserManager::LoadUserOAuthStatus(
@@ -543,6 +554,10 @@ void UserManager::SaveUserDisplayEmail(const std::string& username,
 
   user->set_display_email(display_email);
 
+  // Do not update local store if the user is ephemeral.

[Ivan Korotkov, 2012/03/01 13:37:50]

You don't need most of these checks, only those where persistent storage is
modified.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   PrefService* local_state = g_browser_process->local_state();
 
   DictionaryPrefUpdate display_email_update(local_state, kUserDisplayEmail);
@@ -560,17 +575,30 @@ std::string UserManager::GetUserDisplayEmail(
 void UserManager::SaveUserDefaultImageIndex(const std::string& username,
                                             int image_index) {
   DCHECK(image_index >= 0 && image_index < kDefaultImagesCount);
+
+  // Ignore for ephemeral users.

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   SetUserImage(username, image_index, GetDefaultImage(image_index));
   SaveImageToLocalState(username, "", image_index, false);
 }
 
 void UserManager::SaveUserImage(const std::string& username,
                                 const SkBitmap& image) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   SaveUserImageInternal(username, User::kExternalImageIndex, image);
 }
 
 void UserManager::SaveUserImageFromFile(const std::string& username,
                                         const FilePath& path) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   image_loader_->Start(
       path.value(), login::kUserImageSize,
       base::Bind(&UserManager::SaveUserImage,
@@ -578,6 +606,10 @@ void UserManager::SaveUserImageFromFile(const std::string& username,
 }
 
 void UserManager::SaveUserImageFromProfileImage(const std::string& username) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   if (!downloaded_profile_image_.empty()) {
     // Profile image has already been downloaded, so save it to file right now.
     SaveUserImageInternal(username, User::kProfileImageIndex,
@@ -600,6 +632,11 @@ void UserManager::DownloadProfileImage(const std::string& reason) {
     return;
   }
 
+  if (current_user_is_ephemeral_) {

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    // Ignore for ephemeral users.
+    return;
+  }
+
   profile_image_download_reason_ = reason;
   profile_image_load_start_time_ = base::Time::Now();
   profile_image_downloader_.reset(new ProfileDownloader(this));
@@ -614,6 +651,9 @@ void UserManager::Observe(int type,
       BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                               base::Bind(&UserManager::CheckOwnership,
                                          base::Unretained(this)));
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+          base::Bind(&UserManager::RetrieveTrustedDevicePolicies,
+          base::Unretained(this)));
       break;
     case chrome::NOTIFICATION_PROFILE_ADDED:
       if (user_is_logged_in() && !IsLoggedInAsGuest()) {
@@ -679,6 +719,34 @@ void UserManager::NotifyLocalStateChanged() {
     LocalStateChanged(this));
 }
 
+void UserManager::RetrieveTrustedDevicePolicies() {
+  ephemeral_users_enabled_.reset();
+  owner_.reset();
+
+  CrosSettings* cros_settings = CrosSettings::Get();
+  // Schedule a callback if device policy has not yet been verified.
+  if (!cros_settings->GetTrusted(
+      kAccountsPrefEphemeralUsersEnabled,
+      base::Bind(&UserManager::RetrieveTrustedDevicePolicies,
+                 base::Unretained(this))))
+    return;

[Ivan Korotkov, 2012/03/01 13:37:50]

Multiline if conditions require braces, too.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  bool* ephemeral_users_enabled = new bool;
+  cros_settings->GetBoolean(kAccountsPrefEphemeralUsersEnabled,
+                            ephemeral_users_enabled);
+  ephemeral_users_enabled_.reset(ephemeral_users_enabled);
+
+  if (g_browser_process->browser_policy_connector()->IsEnterpriseManaged()) {
+    owner_.reset(new std::string);

[Ivan Korotkov, 2012/03/01 13:37:50]

There are alredy pieces of code that fetch owner (see RemoveUserInternal). I
suggest to add method GetOwner() that will call GetTrusted/GetString from all
places.

[use bartfab instead, 2012/03/01 18:33:43]

The problem is that GetTrusted() must be provided with a callback to retry
whatever action was desired if trusted information is not available yet. If
GetTrusted() was called from within a new GetOwner(), this method would need to
take a callback parameter to tell it how to repeat the entire operation if
trusted information is not yet available. This would seem more complicated to me
but if you wish, I can change the code.

[Ivan Korotkov, 2012/03/01 20:16:30]

Right, I'm sorry for the confusion. In that case, please make RemoveUserInternal
a member function so that it makes use of owner_.

[use bartfab instead, 2012/03/05 18:07:32]

This will not work I fear. What if the owner_ has not been successfully fetched
yet? RemoveUserInternal needs to find out somehow and schedule a callback to
itself in this case. Even if RemoveUserInternal is made a member function, it
will still need to make its own call to GetTrusted() so that it can schedule its
own callback.

+  } else {
+    std::string* owner = new std::string;
+    cros_settings->GetString(kDeviceOwner, owner);
+    owner_.reset(owner);
+  }
+
+  if (*ephemeral_users_enabled_)
+    RemoveAllExceptOwnerFromList();
+}
+
 // Protected constructor and destructor.
 UserManager::UserManager()
     : ALLOW_THIS_IN_INITIALIZER_LIST(image_loader_(new UserImageLoader)),
@@ -688,6 +756,7 @@ UserManager::UserManager()
       logged_in_user_(NULL),
       current_user_is_owner_(false),
       current_user_is_new_(false),
+      current_user_is_ephemeral_(false),
       user_is_logged_in_(false),
       observed_sync_service_(NULL),
       last_image_set_async_(false),
@@ -699,6 +768,7 @@ UserManager::UserManager()
       content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
       content::NotificationService::AllSources());
+  RetrieveTrustedDevicePolicies();
 }
 
 UserManager::~UserManager() {
@@ -794,6 +864,37 @@ void UserManager::EnsureUsersLoaded() {
   }
 }
 
+bool UserManager::AreEphemeralUsersEnabled() const {
+  return ephemeral_users_enabled_.get() &&
+      *ephemeral_users_enabled_ &&
+      owner_.get();
+}
+
+bool UserManager::IsEphemeralUser(const std::string& email) const {
+  // The guest user always is ephemeral.
+  if (email == guest_user_.email())
+    return true;
+
+  // The currently logged-in user is ephemeral iff logged in as ephemeral.
+  if (logged_in_user_ && (email == logged_in_user_->email()))
+    return current_user_is_ephemeral_;
+
+  // Any other user is ephemeral iff ephemeral users are enabled, the user is
+  // not the owner and is not in the persistent list.
+  return AreEphemeralUsersEnabled() &&
+      (email != *owner_) &&
+      !FindUserInList(email);
+}
+
+const User* UserManager::FindUserInList(const std::string& email) const {
+  const UserList& users = GetUsers();
+  for (UserList::const_iterator it = users.begin(); it != users.end(); ++it) {
+    if ((*it)->email() == email)
+      return *it;
+  }
+  return NULL;
+}
+
 void UserManager::StubUserLoggedIn() {
   logged_in_user_ = &stub_user_;
   stub_user_.SetImage(GetDefaultImage(kStubDefaultImageIndex),
@@ -834,6 +935,10 @@ void UserManager::NotifyOnLogin() {
 }
 
 void UserManager::SetInitialUserImage(const std::string& username) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   // Choose a random default image.
   int image_id = base::RandInt(0, kDefaultImagesCount - 1);
   SaveUserDefaultImageIndex(username, image_id);
@@ -842,6 +947,10 @@ void UserManager::SetInitialUserImage(const std::string& username) {
 void UserManager::SetUserImage(const std::string& username,
                                int image_index,
                                const SkBitmap& image) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   User* user = const_cast<User*>(FindUser(username));
   // User may have been removed by now.
   if (user) {
@@ -874,6 +983,10 @@ void UserManager::SaveUserImageInternal(const std::string& username,
                                         const SkBitmap& image) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

This should be after SetUserImage.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

[Ivan Korotkov, 2012/03/01 20:16:30]

Hey, it's gone now. I meant that it should be after the SetUserImage call
(before the SaveImageToFile task is posted).

[use bartfab instead, 2012/03/05 18:07:32]

Done. I do not understand how this makes sense though. SaveUserImageInternal()
now sets the image index to either kExternalImageIndex or kProfileImageIndex but
then returns before creating an image file. So you have an index implying an
external image which does not actually exist. Would it not be better to move the
IsEphemeralUser() check to the top of the function so that the index is not set
in the first place?

[Ivan Korotkov, 2012/03/06 20:37:17]

The point is that the picture should be still set in memory (so that it will be
displayed on the lock screen/prefs page).

+    return;
+
   SetUserImage(username, image_index, image);
 
   FilePath image_path = GetImagePathForUser(username);
@@ -895,6 +1008,10 @@ void UserManager::SaveImageToFile(const std::string& username,
                                   int image_index) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed (never reached).

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   std::vector<unsigned char> encoded_image;
   if (!gfx::PNGCodec::EncodeBGRASkBitmap(image, false, &encoded_image)) {
     LOG(ERROR) << "Failed to PNG encode the image.";
@@ -922,6 +1039,10 @@ void UserManager::SaveImageToLocalState(const std::string& username,
                                         bool is_async) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Ignore for ephemeral users.

[Ivan Korotkov, 2012/03/01 13:37:50]

Here it's ok.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   // TODO(ivankr): use unique filenames for user images each time
   // a new image is set so that only the last image update is saved
   // to Local State and notified.
@@ -1073,4 +1194,72 @@ User* UserManager::CreateUser(const std::string& email) const {
   return user;
 }
 
+void UserManager::RemoveAllExceptOwnerFromList() {
+  if (!AreEphemeralUsersEnabled())

[Ivan Korotkov, 2012/03/01 13:37:50]

No point in this check.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
+  const std::string owner_email = *owner_;

[Ivan Korotkov, 2012/03/01 13:37:50]

This looks overly complicated. Since this should be doing any real job only
once, when ephemeral users are enabled, I'd just call RemoveUserFromList for
every user other than owner and logged-in user.

[use bartfab instead, 2012/03/01 18:33:43]

I considered this but it will not work:

RemoveUserFromList() calls EnsureUsersLoaded(). If any user has a custom profile
image, this will kick off a download of that image in the background. If the
user is deleted a moment later, the download will continue and will leave a
stale image file behind. This is why I rolled my own removal that does not
download anything.

[Ivan Korotkov, 2012/03/01 20:16:30]

Ah, I see. Add a comment about that, please.

[rkc, 2012/03/01 22:17:37]

Another option could be to refactor RemoveUserFromList to first call
EnsureUsersLoaded() then call a RemoveUserFromListWithoutLoadedCheck(); your
function could call the latter method directly.

It would avoid all this repeated code and keep the remove user logic in a
central method.

[use bartfab instead, 2012/03/05 18:07:32]

Good idea. I implemented it like this using a new RemoveUserFromListInternal()
function for the shared code.

+  // If a user is currently logged in and the login occurred before ephemeral
+  // users were enabled, this user should not be deleted yet either.
+  std::string logged_in_user_email;
+  if (user_is_logged_in_ && !current_user_is_ephemeral_)
+    logged_in_user_email = logged_in_user_->email();
+
+  PrefService* prefs = g_browser_process->local_state();
+  ListPrefUpdate prefs_users_update(prefs, kLoggedInUsers);
+  DictionaryPrefUpdate prefs_images_update(prefs, kUserImages);
+  DictionaryPrefUpdate prefs_oauth_update(prefs, kUserOAuthTokenStatus);
+  DictionaryPrefUpdate prefs_display_email_update(prefs, kUserDisplayEmail);
+
+  base::ListValue* users = prefs_users_update->DeepCopy();
+  prefs_users_update->Clear();
+  for (base::ListValue::const_iterator user = users->begin();
+      user != users->end(); ++user) {
+    std::string user_email;
+    (*user)->GetAsString(&user_email);
+    if (user_email == owner_email || user_email == logged_in_user_email) {
+      prefs_users_update->Append(Value::CreateStringValue(user_email));
+      continue;
+    }
+    std::string image_path_string;
+    prefs_images_update->GetStringWithoutPathExpansion(user_email,
+                                                       &image_path_string);
+    prefs_images_update->RemoveWithoutPathExpansion(user_email, NULL);
+    prefs_oauth_update->RemoveWithoutPathExpansion(user_email, NULL);
+    prefs_display_email_update->RemoveWithoutPathExpansion(user_email, NULL);
+
+    int default_image_id = User::kInvalidImageIndex;
+    if (!image_path_string.empty() &&
+        !IsDefaultImagePath(image_path_string, &default_image_id)) {
+      FilePath image_path(image_path_string);
+      BrowserThread::PostTask(
+          BrowserThread::FILE,
+          FROM_HERE,
+          base::Bind(&UserManager::DeleteUserImage,
+                     base::Unretained(this),
+                     image_path));
+    }
+  }
+  delete users;

[Nikita (slow), 2012/03/01 10:01:58]

scoped_ptr<>?

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+
+  if (!users_.empty()) {
+    UserList users_update;
+    for (UserList::iterator user = users_.begin(); user != users_.end();
+        ++user) {
+      if ((*user)->email() ==  owner_email || *user == logged_in_user_) {
+        users_update.push_back(*user);
+      } else {
+        --display_name_count_[(*user)->GetDisplayName()];
+        delete *user;
+      }
+    }
+    users_ = users_update;
+    // Trigger a redraw of the login window.
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED,
+        content::Source<UserManager>(this),
+        content::NotificationService::NoDetails());
+  }
+}
+
 }  // namespace chromeos