Making user network settings persistent over logout/login.

This fix ensures that the user network policy isn't applied before the user's
profile is created.

BUG=161283


Review URL: https://chromiumcodereview.appspot.com/11348070

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@169043 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 59 insertions, 18 deletions

diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index 7d73b73..9cd0b86 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -52,6 +52,7 @@
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/cloud_policy_client.h"
 #include "chrome/browser/policy/cloud_policy_service.h"
+#include "chrome/browser/policy/network_configuration_updater.h"
 #include "chrome/browser/policy/user_cloud_policy_manager.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -557,6 +558,11 @@ void LoginUtilsImpl::OnProfileCreated(
           user_profile->GetPrefs()->FindPreference(prefs::kUseSharedProxies);
       if (use_shared_proxies_pref->IsDefaultValue())
         user_profile->GetPrefs()->SetBoolean(prefs::kUseSharedProxies, false);
+      policy::NetworkConfigurationUpdater* network_configuration_updater =
+          g_browser_process->browser_policy_connector()->
+          GetNetworkConfigurationUpdater();
+      if (network_configuration_updater)
+        network_configuration_updater->OnUserPolicyInitialized();
       RespectLocalePreference(user_profile);
       return;
     }
diff --git a/chrome/browser/policy/network_configuration_updater.cc b/chrome/browser/policy/network_configuration_updater.cc
index 2642e3e..8842cf5 100644
--- a/chrome/browser/policy/network_configuration_updater.cc
+++ b/chrome/browser/policy/network_configuration_updater.cc
@@ -23,6 +23,7 @@ NetworkConfigurationUpdater::NetworkConfigurationUpdater(
     : policy_change_registrar_(
           policy_service, POLICY_DOMAIN_CHROME, std::string()),
       network_library_(network_library),
+      user_policy_initialized_(false),
       allow_web_trust_(false),
       policy_service_(policy_service) {
   DCHECK(network_library_);
@@ -52,6 +53,12 @@ void NetworkConfigurationUpdater::OnProfileListChanged() {
   ApplyNetworkConfigurations();
 }
 
+void NetworkConfigurationUpdater::OnUserPolicyInitialized() {
+  VLOG(1) << "User policy initialized, applying policies. Ignoring.";
+  user_policy_initialized_ = true;
+  ApplyNetworkConfigurations();
+}
+
 void NetworkConfigurationUpdater::OnPolicyChanged(
     chromeos::NetworkUIData::ONCSource onc_source,
     const base::Value* previous,
@@ -63,8 +70,10 @@ void NetworkConfigurationUpdater::OnPolicyChanged(
 void NetworkConfigurationUpdater::ApplyNetworkConfigurations() {
   ApplyNetworkConfiguration(key::kDeviceOpenNetworkConfiguration,
                             chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY);
-  ApplyNetworkConfiguration(key::kOpenNetworkConfiguration,
-                            chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY);
+  if (user_policy_initialized_) {
+    ApplyNetworkConfiguration(key::kOpenNetworkConfiguration,
+                              chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY);
+  }
 }
 
 void NetworkConfigurationUpdater::ApplyNetworkConfiguration(
diff --git a/chrome/browser/policy/network_configuration_updater.h b/chrome/browser/policy/network_configuration_updater.h
index 9b4ed5b..1fad772d 100644
--- a/chrome/browser/policy/network_configuration_updater.h
+++ b/chrome/browser/policy/network_configuration_updater.h
@@ -22,7 +22,10 @@ class PolicyMap;
 
 // Keeps track of the network configuration policy settings and Shill's
 // profiles. Requests the NetworkLibrary to apply the ONC of the network
-// policies when necessary.
+// policies every time one of the relevant policies or Shill's profiles changes
+// or OnUserPolicyInitialized() is called. If the user policy is available,
+// always both the device and the user policy are applied. Otherwise only the
+// device policy is applied.
 class NetworkConfigurationUpdater
     : public chromeos::NetworkLibrary::NetworkProfileObserver {
  public:
@@ -33,9 +36,16 @@ class NetworkConfigurationUpdater
   // NetworkProfileObserver overrides.
   virtual void OnProfileListChanged() OVERRIDE;
 
-  // Web trust isn't given to certificates imported from ONC by default.
-  // Setting |allow_web_trust| to true allows giving Web trust to the
-  // certificates that request it.
+  // Notifies this updater that the user policy is initialized. Before this
+  // function is called, the user policy is not applied. Afterwards, always both
+  // device and user policy are applied as described in the class comment. This
+  // function also triggers an immediate policy application of both device and
+  // user policy.
+  void OnUserPolicyInitialized();
+
+  // Web trust isn't given to certificates imported from ONC by default. Setting
+  // |allow_web_trust| to true allows giving Web trust to the certificates that
+  // request it.
   void set_allow_web_trust(bool allow) { allow_web_trust_ = allow; }
 
   // Empty network configuration blob.
@@ -64,6 +74,9 @@ class NetworkConfigurationUpdater
   // Network library to write network configuration to.
   chromeos::NetworkLibrary* network_library_;
 
+  // Whether the user policy is already available.
+  bool user_policy_initialized_;
+
   // Whether Web trust is allowed or not.
   bool allow_web_trust_;
 
diff --git a/chrome/browser/policy/network_configuration_updater_unittest.cc b/chrome/browser/policy/network_configuration_updater_unittest.cc
index 454d0eb..64c47cd 100644
--- a/chrome/browser/policy/network_configuration_updater_unittest.cc
+++ b/chrome/browser/policy/network_configuration_updater_unittest.cc
@@ -22,6 +22,8 @@ using testing::_;
 namespace policy {
 
 static const char kFakeONC[] = "{ \"GUID\": \"1234\" }";
+static const char* kEmptyConfiguration =
+    NetworkConfigurationUpdater::kEmptyConfiguration;
 
 class NetworkConfigurationUpdaterTest
     : public testing::TestWithParam<const char*>{
@@ -54,7 +56,7 @@ class NetworkConfigurationUpdaterTest
   scoped_ptr<PolicyServiceImpl> policy_service_;
 };
 
-TEST_P(NetworkConfigurationUpdaterTest, InitialUpdate) {
+TEST_P(NetworkConfigurationUpdaterTest, InitialUpdates) {
   PolicyMap policy;
   policy.Set(GetParam(), POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
              Value::CreateStringValue(kFakeONC));
@@ -62,18 +64,28 @@ TEST_P(NetworkConfigurationUpdaterTest, InitialUpdate) {
 
   EXPECT_CALL(network_library_, AddNetworkProfileObserver(_));
 
-  // Initially, both policies are applied.
+  // Initially, only the device policy is applied. The user policy is only
+  // applied after the user profile was initialized.
+  const char* device_onc = GetParam() == key::kDeviceOpenNetworkConfiguration ?
+      kFakeONC : kEmptyConfiguration;
   EXPECT_CALL(network_library_, LoadOncNetworks(
-      kFakeONC, "", NameToONCSource(GetParam()), _, _));
-  EXPECT_CALL(network_library_, LoadOncNetworks(
-      NetworkConfigurationUpdater::kEmptyConfiguration, "",
-      Ne(NameToONCSource(GetParam())), _, _));
-
-  EXPECT_CALL(network_library_, RemoveNetworkProfileObserver(_));
+      device_onc, "", chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY, _, _));
 
   {
     NetworkConfigurationUpdater updater(policy_service_.get(),
                                         &network_library_);
+    Mock::VerifyAndClearExpectations(&network_library_);
+
+    // After the user policy is initialized, we always push both policies to the
+    // NetworkLibrary.
+    EXPECT_CALL(network_library_, LoadOncNetworks(
+        kFakeONC, "", NameToONCSource(GetParam()), _, _));
+    EXPECT_CALL(network_library_, LoadOncNetworks(
+        kEmptyConfiguration, "", Ne(NameToONCSource(GetParam())), _, _));
+
+    EXPECT_CALL(network_library_, RemoveNetworkProfileObserver(_));
+
+    updater.OnUserPolicyInitialized();
   }
   Mock::VerifyAndClearExpectations(&network_library_);
 }
@@ -87,6 +99,7 @@ TEST_P(NetworkConfigurationUpdaterTest, AllowWebTrust) {
         .Times(AtLeast(0));
     NetworkConfigurationUpdater updater(policy_service_.get(),
                                         &network_library_);
+    updater.OnUserPolicyInitialized();
     Mock::VerifyAndClearExpectations(&network_library_);
 
     // Web trust should be forwarded to LoadOncNetworks.
@@ -115,6 +128,7 @@ TEST_P(NetworkConfigurationUpdaterTest, PolicyChange) {
         .Times(AtLeast(0));
     NetworkConfigurationUpdater updater(policy_service_.get(),
                                         &network_library_);
+    updater.OnUserPolicyInitialized();
     Mock::VerifyAndClearExpectations(&network_library_);
 
     // We should update if policy changes.
@@ -123,8 +137,7 @@ TEST_P(NetworkConfigurationUpdaterTest, PolicyChange) {
 
     // In the current implementation, we always apply both policies.
     EXPECT_CALL(network_library_, LoadOncNetworks(
-        NetworkConfigurationUpdater::kEmptyConfiguration, "",
-        Ne(NameToONCSource(GetParam())), _, _));
+        kEmptyConfiguration, "", Ne(NameToONCSource(GetParam())), _, _));
 
     PolicyMap policy;
     policy.Set(GetParam(), POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
@@ -135,11 +148,11 @@ TEST_P(NetworkConfigurationUpdaterTest, PolicyChange) {
     // Another update is expected if the policy goes away. In the current
     // implementation, we always apply both policies.
     EXPECT_CALL(network_library_, LoadOncNetworks(
-        NetworkConfigurationUpdater::kEmptyConfiguration, "",
+        kEmptyConfiguration, "",
         chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY, _, _));
 
     EXPECT_CALL(network_library_, LoadOncNetworks(
-        NetworkConfigurationUpdater::kEmptyConfiguration, "",
+        kEmptyConfiguration, "",
         chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY, _, _));
 
     EXPECT_CALL(network_library_, RemoveNetworkProfileObserver(_));