Add support for multiple allowed domains

remoting/host/policy_watcher_unittest.cc

 // Copyright 2015 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 "remoting/host/policy_watcher.h"
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 56 matching lines @@
     // We expect no callbacks unless explicitly specified by individual tests.
     EXPECT_CALL(mock_policy_callback_, OnPolicyUpdatePtr(testing::_)).Times(0);
     EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(0);
 
     // Retaining a raw pointer to keep control over policy contents.
     policy_loader_ =
         new policy::FakeAsyncPolicyLoader(base::ThreadTaskRunnerHandle::Get());
     policy_watcher_ = PolicyWatcher::CreateFromPolicyLoaderForTesting(
         base::WrapUnique(policy_loader_));
 
+    base::ListValue host_domain;
+    host_domain.AppendString(kHostDomain);
+    base::ListValue client_domain;
+    client_domain.AppendString(kClientDomain);
+    base::ListValue multiple_host_domains;
+    multiple_host_domains.AppendString("a.com");

[rkjnsn, 2017/04/19 23:13:24]

I could make "a.com", et cetera constants. I didn't here because they're only
used once to initialize members that get used as quasi-constants, anyway, but I
don't really have a preference.

+    multiple_host_domains.AppendString("b.com");
+    multiple_host_domains.AppendString("c.com");
+    base::ListValue multiple_client_domains;
+    multiple_client_domains.AppendString("d.com");
+    multiple_client_domains.AppendString("e.com");
+    multiple_client_domains.AppendString("f.com");
+
     nat_true_.SetBoolean(key::kRemoteAccessHostFirewallTraversal, true);
     nat_false_.SetBoolean(key::kRemoteAccessHostFirewallTraversal, false);
     nat_one_.SetInteger(key::kRemoteAccessHostFirewallTraversal, 1);
     nat_one_domain_full_.SetInteger(key::kRemoteAccessHostFirewallTraversal, 1);
-    nat_one_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
-    domain_empty_.SetString(key::kRemoteAccessHostDomain, std::string());
-    domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    nat_one_domain_full_.Set(key::kRemoteAccessHostDomainList,
+                             host_domain.CreateDeepCopy());
+    domain_empty_.Set(key::kRemoteAccessHostDomainList,
+                      base::MakeUnique<base::ListValue>());
+    domain_full_.Set(key::kRemoteAccessHostDomainList,
+                     host_domain.CreateDeepCopy());
     SetDefaults(nat_true_others_default_);
     nat_true_others_default_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
                                         true);
     SetDefaults(nat_false_others_default_);
     nat_false_others_default_.SetBoolean(
         key::kRemoteAccessHostFirewallTraversal, false);
     SetDefaults(domain_empty_others_default_);
-    domain_empty_others_default_.SetString(key::kRemoteAccessHostDomain,
-                                           std::string());
+    domain_empty_others_default_.Set(key::kRemoteAccessHostDomainList,
+                                     base::MakeUnique<base::ListValue>());
     SetDefaults(domain_full_others_default_);
-    domain_full_others_default_.SetString(key::kRemoteAccessHostDomain,
-                                          kHostDomain);
+    domain_full_others_default_.Set(key::kRemoteAccessHostDomainList,
+                                    host_domain.CreateDeepCopy());
     nat_true_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
                                       true);
-    nat_true_domain_empty_.SetString(key::kRemoteAccessHostDomain,
-                                     std::string());
+    nat_true_domain_empty_.Set(key::kRemoteAccessHostDomainList,
+                               base::MakeUnique<base::ListValue>());
     nat_true_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
                                      true);
-    nat_true_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    nat_true_domain_full_.Set(key::kRemoteAccessHostDomainList,
+                              host_domain.CreateDeepCopy());
     nat_false_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
                                        false);
-    nat_false_domain_empty_.SetString(key::kRemoteAccessHostDomain,
-                                      std::string());
+    nat_false_domain_empty_.Set(key::kRemoteAccessHostDomainList,
+                                base::MakeUnique<base::ListValue>());
     nat_false_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
                                       false);
-    nat_false_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    nat_false_domain_full_.Set(key::kRemoteAccessHostDomainList,
+                               host_domain.CreateDeepCopy());
     SetDefaults(nat_true_domain_empty_others_default_);
     nat_true_domain_empty_others_default_.SetBoolean(
         key::kRemoteAccessHostFirewallTraversal, true);
-    nat_true_domain_empty_others_default_.SetString(
-        key::kRemoteAccessHostDomain, std::string());
+    nat_true_domain_empty_others_default_.Set(
+        key::kRemoteAccessHostDomainList, base::MakeUnique<base::ListValue>());
     unknown_policies_.SetString("UnknownPolicyOne", std::string());
     unknown_policies_.SetString("UnknownPolicyTwo", std::string());
     unknown_policies_.SetBoolean("RemoteAccessHostUnknownPolicyThree", true);
 
     pairing_true_.SetBoolean(key::kRemoteAccessHostAllowClientPairing, true);
     pairing_false_.SetBoolean(key::kRemoteAccessHostAllowClientPairing, false);
     gnubby_auth_true_.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, true);
     gnubby_auth_false_.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, false);
     relay_true_.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection, true);
     relay_false_.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection,
                             false);
     port_range_full_.SetString(key::kRemoteAccessHostUdpPortRange, kPortRange);
     port_range_empty_.SetString(key::kRemoteAccessHostUdpPortRange,
                                 std::string());
     port_range_malformed_.SetString(key::kRemoteAccessHostUdpPortRange,
                                     "malformed");
     port_range_malformed_domain_full_.MergeDictionary(&port_range_malformed_);
-    port_range_malformed_domain_full_.SetString(key::kRemoteAccessHostDomain,
-                                                kHostDomain);
+    port_range_malformed_domain_full_.Set(key::kRemoteAccessHostDomainList,
+                                          host_domain.CreateDeepCopy());
 
     curtain_true_.SetBoolean(key::kRemoteAccessHostRequireCurtain, true);
     curtain_false_.SetBoolean(key::kRemoteAccessHostRequireCurtain, false);
     username_true_.SetBoolean(key::kRemoteAccessHostMatchUsername, true);
     username_false_.SetBoolean(key::kRemoteAccessHostMatchUsername, false);
     talk_gadget_blah_.SetString(key::kRemoteAccessHostTalkGadgetPrefix, "blah");
     third_party_auth_partial_.SetString(key::kRemoteAccessHostTokenUrl,
                                         "https://token.com");
     third_party_auth_partial_.SetString(
         key::kRemoteAccessHostTokenValidationUrl, "https://validation.com");
     third_party_auth_full_.MergeDictionary(&third_party_auth_partial_);
     third_party_auth_full_.SetString(
         key::kRemoteAccessHostTokenValidationCertificateIssuer,
         "certificate subject");
     third_party_auth_cert_empty_.MergeDictionary(&third_party_auth_partial_);
     third_party_auth_cert_empty_.SetString(
         key::kRemoteAccessHostTokenValidationCertificateIssuer, "");
     remote_assistance_uiaccess_true_.SetBoolean(
         key::kRemoteAccessHostAllowUiAccessForRemoteAssistance, true);
     remote_assistance_uiaccess_false_.SetBoolean(
         key::kRemoteAccessHostAllowUiAccessForRemoteAssistance, false);
+
+    deprecated_policies_.SetString(key::kRemoteAccessHostDomain, kHostDomain);

[rkjnsn, 2017/04/19 23:13:24]

I mainly just added stuff to the end of the relevant section. I'm not sure if
there's an organization that I'm missing.

+    deprecated_policies_.SetString(key::kRemoteAccessHostClientDomain,
+                                   kClientDomain);
+    // Deprecated policies should get converted if new ones aren't present.
+    SetDefaults(deprecated_policies_expected_);
+    deprecated_policies_expected_.Set(key::kRemoteAccessHostDomainList,
+                                      host_domain.CreateDeepCopy());
+    deprecated_policies_expected_.Set(key::kRemoteAccessHostClientDomainList,
+                                      client_domain.CreateDeepCopy());
+
+    deprecated_and_new_policies_.SetString(key::kRemoteAccessHostDomain,
+                                           kHostDomain);
+    deprecated_and_new_policies_.SetString(key::kRemoteAccessHostClientDomain,
+                                           kClientDomain);
+    deprecated_and_new_policies_.Set(key::kRemoteAccessHostDomainList,
+                                     multiple_host_domains.CreateDeepCopy());
+    deprecated_and_new_policies_.Set(key::kRemoteAccessHostClientDomainList,
+                                     multiple_client_domains.CreateDeepCopy());
+    // Deprecated policies should just be dropped in new ones are present.
+    SetDefaults(deprecated_and_new_policies_expected_);
+    deprecated_and_new_policies_expected_.Set(
+        key::kRemoteAccessHostDomainList,
+        multiple_host_domains.CreateDeepCopy());
+    deprecated_and_new_policies_expected_.Set(
+        key::kRemoteAccessHostClientDomainList,
+        multiple_client_domains.CreateDeepCopy());
+
+    // Empty strings should be treated as not set.
+    deprecated_empty_strings_.SetString(key::kRemoteAccessHostDomain, "");
+    deprecated_empty_strings_.SetString(key::kRemoteAccessHostClientDomain, "");
   }
 
   void TearDown() override {
     policy_watcher_.reset();
     policy_loader_ = nullptr;
     base::RunLoop().RunUntilIdle();
   }
 
  protected:
   void StartWatching() {
@@ skipping 25 matching lines @@
     return policy_watcher_->GetPolicySchema();
   }
 
   const base::DictionaryValue& GetDefaultValues() {
     return *(policy_watcher_->default_values_);
   }
 
   MOCK_METHOD0(PostPolicyWatcherShutdown, void());
 
   static const char* kHostDomain;
+  static const char* kClientDomain;
   static const char* kPortRange;
   base::MessageLoop message_loop_;
   MockPolicyCallback mock_policy_callback_;
 
   // |policy_loader_| is owned by |policy_watcher_|. PolicyWatcherTest retains
   // a raw pointer to |policy_loader_| in order to control the simulated / faked
   // policy contents.
   policy::FakeAsyncPolicyLoader* policy_loader_;
   std::unique_ptr<PolicyWatcher> policy_watcher_;
 
@@ skipping 27 matching lines @@
   base::DictionaryValue curtain_true_;
   base::DictionaryValue curtain_false_;
   base::DictionaryValue username_true_;
   base::DictionaryValue username_false_;
   base::DictionaryValue talk_gadget_blah_;
   base::DictionaryValue third_party_auth_full_;
   base::DictionaryValue third_party_auth_partial_;
   base::DictionaryValue third_party_auth_cert_empty_;
   base::DictionaryValue remote_assistance_uiaccess_true_;
   base::DictionaryValue remote_assistance_uiaccess_false_;
+  base::DictionaryValue deprecated_policies_;
+  base::DictionaryValue deprecated_policies_expected_;
+  base::DictionaryValue deprecated_and_new_policies_;
+  base::DictionaryValue deprecated_and_new_policies_expected_;
+  base::DictionaryValue deprecated_empty_strings_;
 
  private:
   void SetDefaults(base::DictionaryValue& dict) {
     dict.SetBoolean(key::kRemoteAccessHostFirewallTraversal, true);
     dict.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection, true);
     dict.SetString(key::kRemoteAccessHostUdpPortRange, "");
-    dict.SetString(key::kRemoteAccessHostClientDomain, std::string());
-    dict.SetString(key::kRemoteAccessHostDomain, std::string());
+    dict.Set(key::kRemoteAccessHostClientDomainList,
+             base::MakeUnique<base::ListValue>());
+    dict.Set(key::kRemoteAccessHostDomainList,
+             base::MakeUnique<base::ListValue>());
     dict.SetBoolean(key::kRemoteAccessHostMatchUsername, false);
     dict.SetString(key::kRemoteAccessHostTalkGadgetPrefix,
                    kDefaultHostTalkGadgetPrefix);
     dict.SetBoolean(key::kRemoteAccessHostRequireCurtain, false);
     dict.SetString(key::kRemoteAccessHostTokenUrl, "");
     dict.SetString(key::kRemoteAccessHostTokenValidationUrl, "");
     dict.SetString(key::kRemoteAccessHostTokenValidationCertificateIssuer, "");
     dict.SetBoolean(key::kRemoteAccessHostAllowClientPairing, true);
     dict.SetBoolean(key::kRemoteAccessHostAllowGnubbyAuth, true);
     dict.SetBoolean(key::kRemoteAccessHostAllowUiAccessForRemoteAssistance,
                     false);
 
     ASSERT_THAT(&dict, IsPolicies(&GetDefaultValues()))
         << "Sanity check that defaults expected by the test code "
         << "match what is stored in PolicyWatcher::default_values_";
   }
 };
 
 const char* PolicyWatcherTest::kHostDomain = "google.com";
+const char* PolicyWatcherTest::kClientDomain = "client.com";
 const char* PolicyWatcherTest::kPortRange = "12400-12409";
 
 TEST_F(PolicyWatcherTest, None) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
 
   SetPolicies(empty_);
   StartWatching();
 }
 
@@ skipping 368 matching lines @@
   std::map<std::string, base::Value::Type> actual_schema;
   const policy::Schema* schema = GetPolicySchema();
   ASSERT_TRUE(schema->valid());
   for (auto it = schema->GetPropertiesIterator(); !it.IsAtEnd(); it.Advance()) {
     std::string key = it.key();
     if (key.find("RemoteAccessHost") == std::string::npos) {
       // For now PolicyWatcher::GetPolicySchema() mixes Chrome and Chromoting
       // policies, so we have to skip them here.
       continue;
     }
+    if (key == policy::key::kRemoteAccessHostDomain ||
+        key == policy::key::kRemoteAccessHostClientDomain) {
+      // These policies are deprecated and get removed during normalization
+      continue;
+    }
     actual_schema[key] = it.schema().type();
   }
 
   EXPECT_THAT(actual_schema, testing::ContainerEq(expected_schema));
 }
 
 TEST_F(PolicyWatcherTest, SchemaTypeCheck) {
   const policy::Schema* schema = GetPolicySchema();
   ASSERT_TRUE(schema->valid());
 
   // Check one, random "string" policy to see if the type propagated correctly
   // from policy_templates.json file.
   const policy::Schema string_schema =
       schema->GetKnownProperty("RemoteAccessHostDomain");
   EXPECT_TRUE(string_schema.valid());
   EXPECT_EQ(string_schema.type(), base::Value::Type::STRING);
 
   // And check one, random "boolean" policy to see if the type propagated
   // correctly from policy_templates.json file.
   const policy::Schema boolean_schema =
       schema->GetKnownProperty("RemoteAccessHostRequireCurtain");
   EXPECT_TRUE(boolean_schema.valid());
   EXPECT_EQ(boolean_schema.type(), base::Value::Type::BOOLEAN);
 }
 
+TEST_F(PolicyWatcherTest, DeprecatedOnly) {
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(IsPolicies(&deprecated_policies_expected_)));
+  SetPolicies(deprecated_policies_);
+  StartWatching();
+}
+
+TEST_F(PolicyWatcherTest, DeprecatedAndNew) {
+  EXPECT_CALL(
+      mock_policy_callback_,
+      OnPolicyUpdatePtr(IsPolicies(&deprecated_and_new_policies_expected_)));
+  SetPolicies(deprecated_and_new_policies_);
+  StartWatching();
+}
+
+TEST_F(PolicyWatcherTest, DeprecatedEmpty) {
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(IsPolicies(&GetDefaultValues())));
+  SetPolicies(deprecated_empty_strings_);
+  StartWatching();
+}
+
 }  // namespace remoting