Fixed misuse of GetProfileByUser in MultiProfileUserController.

chrome/browser/chromeos/login/users/multi_profile_user_controller.cc

Index: chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
diff --git a/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc b/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
index 1d315d305560b3508157528e5820ba61058b4950..eff3ffbd847f6d1374f783f40ff6e689095e5559 100644
--- a/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
+++ b/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
@@ -94,13 +94,10 @@ bool MultiProfileUserController::IsUserAllowedInSession(
   CHECK(user_manager);
 
   const user_manager::User* primary_user = user_manager->GetPrimaryUser();
-  std::string primary_user_email;
-  if (primary_user)
-    primary_user_email = primary_user->email();
 
   // Always allow if there is no primary user or user being checked is the
   // primary user.
-  if (primary_user_email.empty() || primary_user_email == user_email)
+  if (!primary_user || primary_user->email() == user_email)
     return SetUserAllowedReason(reason, ALLOWED);
 
   // Owner is not allowed to be secondary user.
@@ -114,7 +111,7 @@ bool MultiProfileUserController::IsUserAllowedInSession(
 
   // Don't allow any secondary profiles if the primary profile is tainted.
   if (policy::PolicyCertServiceFactory::UsedPolicyCertificates(
-          primary_user_email)) {
+          primary_user->email())) {
     // Check directly in local_state before checking if the primary user has
     // a PolicyCertService. His profile may have been tainted previously though
     // he didn't get a PolicyCertService created for this session.
@@ -122,27 +119,29 @@ bool MultiProfileUserController::IsUserAllowedInSession(
                                 NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED);
   }
 
-  // If the primary profile already has policy certificates installed but hasn't
-  // used them yet then it can become tainted at any time during this session;
-  // disable secondary profiles in this case too.
-  Profile* primary_user_profile =
-      primary_user ? ProfileHelper::Get()->GetProfileByUserUnsafe(primary_user)
-                   : NULL;
-  policy::PolicyCertService* service =
-      primary_user_profile ? policy::PolicyCertServiceFactory::GetForProfile(
-                                 primary_user_profile)
-                           : NULL;
-  if (service && service->has_policy_certificates())
-    return SetUserAllowedReason(reason,
-                                NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED);
+  if (Profile* primary_user_profile =

[Nikita (slow), 2014/08/13 09:16:46]

I worry that if someone cals IsUserAllowedInSession() early when profile is not
available yet he will get cached value i.e. ignoring for instance
NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED /
NOT_ALLOWED_PRIMARY_USER_POLICY_FORBIDS.

Can you make sure that this value is not cached by caller and once profile is
available it will still be checked?

[dzhioev (left Google), 2014/08/13 16:41:13]

Checked, nobody caches this value. 

+          ProfileHelper::Get()->GetProfileByUser(primary_user)) {
+    // If the primary profile already has policy certificates installed but
+    // hasn't  used them yet then it can become tainted at any time during this
+    // session disable secondary profiles in this case too.
+    if (policy::PolicyCertService* service =
+            policy::PolicyCertServiceFactory::GetForProfile(
+                 primary_user_profile)) {
+      if (service->has_policy_certificates()) {
+        return SetUserAllowedReason(reason,
+                                    NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED);
+      }
+    }
 
-  // No user is allowed if the primary user policy forbids it.
-  const std::string primary_user_behavior =
-      primary_user_profile->GetPrefs()->GetString(
-          prefs::kMultiProfileUserBehavior);
-  if (primary_user_behavior == kBehaviorNotAllowed)
-    return SetUserAllowedReason(reason,
-                                NOT_ALLOWED_PRIMARY_USER_POLICY_FORBIDS);
+    // No user is allowed if the primary user policy forbids it.
+    const std::string primary_user_behavior =
+        primary_user_profile->GetPrefs()->GetString(
+            prefs::kMultiProfileUserBehavior);
+    if (primary_user_behavior == kBehaviorNotAllowed) {
+      return SetUserAllowedReason(reason,
+                                  NOT_ALLOWED_PRIMARY_USER_POLICY_FORBIDS);
+    }
+  }
 
   // The user must have 'unrestricted' policy to be a secondary user.
   const std::string behavior = GetCachedValue(user_email);