Explicitly wait for user policy before completing login.

chrome/browser/chromeos/login/login_utils.cc

 // Copyright (c) 2012 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/login/login_utils.h"
 
 #include <vector>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
@@ skipping 516 matching lines @@
                        public GaiaOAuthConsumer,
                        public OAuthLoginVerifier::Delegate,
                        public net::NetworkChangeNotifier::OnlineStateObserver,
                        public base::SupportsWeakPtr<LoginUtilsImpl> {
  public:
   LoginUtilsImpl()
       : pending_requests_(false),
         using_oauth_(false),
         has_cookies_(false),
         delegate_(NULL),
-        job_restart_request_(NULL) {
+        job_restart_request_(NULL),
+        user_policy_ready_(false),
+        profile_pending_creation_(NULL) {
     net::NetworkChangeNotifier::AddOnlineStateObserver(this);
   }
 
   virtual ~LoginUtilsImpl() {
     net::NetworkChangeNotifier::RemoveOnlineStateObserver(this);
   }
 
   // LoginUtils implementation:
   virtual void PrepareProfile(
       const std::string& username,
@@ skipping 85 matching lines @@
                         const std::string& token,
                         const std::string& secret);
 
   // Check user's profile for kApplicationLocale setting.
   void RespectLocalePreference(Profile* pref);
 
   // Callback for asynchronous profile creation.
   void OnProfileCreated(Profile* profile,
                         Profile::CreateStatus status);
 
+  // Callback for asynchronous notification that user policy is ready.
+  void OnUserPolicyReady();
+
+  // Invoked to resume profile creation after the profile is created and user
+  // policy has been loaded.
+  void ResumeProfileCreation(Profile* user_profile);
+
   std::string password_;
   GaiaAuthConsumer::ClientLoginResult credentials_;
   bool pending_requests_;
   bool using_oauth_;
   bool has_cookies_;
   // Has to be scoped_refptr, see comment for CreateAuthenticator(...).
   scoped_refptr<Authenticator> authenticator_;
   scoped_ptr<GaiaOAuthFetcher> oauth_fetcher_;
   scoped_ptr<PolicyOAuthFetcher> policy_oauth_fetcher_;
   scoped_ptr<OAuthLoginVerifier> oauth_login_verifier_;
 
   // Delegate to be fired when the profile will be prepared.
   LoginUtils::Delegate* delegate_;
 
   // Used to restart Chrome to switch to the guest mode.
   JobRestartRequest* job_restart_request_;
 
+  // Profile creation should only resume once user policy is ready. Since both
+  // profile creation and user policy readiness notifications come

[Mattias Nissler (ping if slow), 2012/02/16 10:24:11]

s/come/are reported/

+  // asynchronously, these fields are used to track whether both are done.
+  bool user_policy_ready_;
+  Profile* profile_pending_creation_;
+
   DISALLOW_COPY_AND_ASSIGN(LoginUtilsImpl);
 };
 
 class LoginUtilsWrapper {
  public:
   static LoginUtilsWrapper* GetInstance() {
     return Singleton<LoginUtilsWrapper>::get();
   }
 
   LoginUtils* get() {
@@ skipping 66 matching lines @@
   // PrefService has the right values.
   // Profile creation is also resumed if the fetch attempt fails.
   bool wait_for_policy_fetch =
       using_oauth_ &&
       authenticator_.get() &&
       (connector->GetUserAffiliation(username) ==
            policy::USER_AFFILIATION_MANAGED);
 
   // Initialize user policy before the profile is created so the profile
   // initialization code sees the cached policy settings.
-  connector->InitializeUserPolicy(username, wait_for_policy_fetch);
+  user_policy_ready_ = false;
+  profile_pending_creation_ = NULL;
+  connector->InitializeUserPolicy(

[Nikita (slow), 2012/02/20 16:10:24]

What's the cost (and what happens behind the scenes) on any subsequent login
(i.e. when policy has been already fetched)?

Does this also involve policy update as well (it shouldn't) or policy update is
postponed?

[Joao da Silva, 2012/02/20 16:23:15]

The cost is always the same. A couple of objects are created and plugged into
places, a URL fetch is started asynchronously, and an asynchronous load of
cached policy is started too. This call won't do any IO itself, and returns
quickly.

|wait_for_policy_fetch| is required precisely because this call can return while
the cache is still being loaded.
An update is triggered by this call but is delayed by 5 seconds, unless
|wait_for_policy_fetch| is true (in that case the fetch starts immediately).

+      username,
+      wait_for_policy_fetch,
+      base::Bind(&LoginUtilsImpl::OnUserPolicyReady, AsWeakPtr()));
 
   if (wait_for_policy_fetch) {
     // Profile creation will block until user policy is fetched, which
     // requires the DeviceManagement token. Try to fetch it now.
     VLOG(1) << "Profile creation requires policy token, fetching now";
     policy_oauth_fetcher_.reset(
         new PolicyOAuthFetcher(authenticator_->authentication_profile()));
     policy_oauth_fetcher_->Start();
   }
 
   // The default profile will have been changed because the ProfileManager
   // will process the notification that the UserManager sends out.
   ProfileManager::CreateDefaultProfileAsync(
       base::Bind(&LoginUtilsImpl::OnProfileCreated, AsWeakPtr()));

[Mattias Nissler (ping if slow), 2012/02/16 10:24:11]

Since user policy is not initialized at this point, I'm worrying that running
ProfileImpl::OnPrefsLoaded will actually start up lots of pref-consuming code
without policy in place. I think we need to split profile initialization to not
automatically call ProfileImpl::OnPrefsLoaded, s.t. we can create the profile
and its PrefService and wait for user policy. Once all that has finished we
should kick off what's now happening in OnPrefsLoaded.

[Nikita (slow), 2012/02/20 16:10:24]

I think this part could be done in a separate CL prior to that one?

[Joao da Silva, 2012/02/20 16:23:15]

Yes. This turned out to be a larger issue and we've been debating how to design
a nicer fix that plugs more clearly into the Profile initialization code. This
CL is now likely to be closed; I'll chime you in again if any changes are
required in LoginUtils.

 }
 
 void LoginUtilsImpl::DelegateDeleted(LoginUtils::Delegate* delegate) {
   if (delegate_ == delegate)
     delegate_ = NULL;
 }
 
+void LoginUtilsImpl::OnUserPolicyReady() {
+  user_policy_ready_ = true;
+  if (profile_pending_creation_) {
+    ResumeProfileCreation(profile_pending_creation_);
+    profile_pending_creation_ = NULL;
+  }
+}
+
 void LoginUtilsImpl::OnProfileCreated(
     Profile* user_profile,
     Profile::CreateStatus status) {
   CHECK(user_profile);
   switch (status) {
     case Profile::CREATE_STATUS_INITIALIZED:
+      if (user_policy_ready_)
+        ResumeProfileCreation(user_profile);
+      else
+        profile_pending_creation_ = user_profile;
       break;
     case Profile::CREATE_STATUS_CREATED: {
       if (UserManager::Get()->current_user_is_new())
         SetFirstLoginPrefs(user_profile->GetPrefs());
       // Make sure that the google service username is properly set (we do this
       // on every sign in, not just the first login, to deal with existing
       // profiles that might not have it set yet).
       StringPrefMember google_services_username;
       google_services_username.Init(prefs::kGoogleServicesUsername,
                                     user_profile->GetPrefs(), NULL);
       google_services_username.SetValue(
           UserManager::Get()->logged_in_user().display_email());
       // Make sure we flip every profile to not share proxies if the user hasn't
       // specified so explicitly.
       const PrefService::Preference* use_shared_proxies_pref =
           user_profile->GetPrefs()->FindPreference(prefs::kUseSharedProxies);
       if (use_shared_proxies_pref->IsDefaultValue())
         user_profile->GetPrefs()->SetBoolean(prefs::kUseSharedProxies, false);
       RespectLocalePreference(user_profile);
-      return;
+      break;
     }
     case Profile::CREATE_STATUS_FAIL:
     default:
       NOTREACHED();
-      return;
+      break;
   }
+}
 
+void LoginUtilsImpl::ResumeProfileCreation(Profile* user_profile) {
   BootTimesLoader* btl = BootTimesLoader::Get();
   btl->AddLoginTimeMarker("UserProfileGotten", false);
 
   if (using_oauth_) {
     // Reuse the access token fetched by the PolicyOAuthFetcher, if it was
     // used to fetch policies before Profile creation.
     if (policy_oauth_fetcher_.get() &&
         !policy_oauth_fetcher_->oauth1_token().empty()) {
       VLOG(1) << "Resuming profile creation after fetching policy token";
       StoreOAuth1AccessToken(user_profile,
@@ skipping 587 matching lines @@
 bool LoginUtils::IsWhitelisted(const std::string& username) {
   CrosSettings* cros_settings = CrosSettings::Get();
   bool allow_new_user = false;
   cros_settings->GetBoolean(kAccountsPrefAllowNewUser, &allow_new_user);
   if (allow_new_user)
     return true;
   return cros_settings->FindEmailInList(kAccountsPrefUsers, username);
 }
 
 }  // namespace chromeos