Malformed PortRange or ThirdPartyAuthConfig trigger OnPolicyError.

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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "components/policy/core/common/fake_async_policy_loader.h"
 #include "policy/policy_constants.h"
 #include "remoting/host/dns_blackhole_checker.h"
 #include "remoting/host/policy_watcher.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace remoting {
 
+namespace key = ::policy::key;
+
 MATCHER_P(IsPolicies, dict, "") {
   bool equal = arg->Equals(dict);
   if (!equal) {
     std::string actual_value;
     base::JSONWriter::WriteWithOptions(
         arg, base::JSONWriter::OPTIONS_PRETTY_PRINT, &actual_value);
 
     std::string expected_value;
     base::JSONWriter::WriteWithOptions(
         dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, &expected_value);
@@ skipping 19 matching lines @@
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockPolicyCallback);
 };
 
 class PolicyWatcherTest : public testing::Test {
  public:
   PolicyWatcherTest() : message_loop_(base::MessageLoop::TYPE_IO) {}
 
   void SetUp() override {
+    // 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);
+
     message_loop_proxy_ = base::MessageLoopProxy::current();
 
     // Retaining a raw pointer to keep control over policy contents.
     policy_loader_ = new policy::FakeAsyncPolicyLoader(message_loop_proxy_);
     policy_watcher_ =
         PolicyWatcher::CreateFromPolicyLoader(make_scoped_ptr(policy_loader_));
 
-    nat_true_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_false_.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal,
-                          false);
-    nat_one_.SetInteger(policy::key::kRemoteAccessHostFirewallTraversal, 1);
-    domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
-                            std::string());
-    domain_full_.SetString(policy::key::kRemoteAccessHostDomain, kHostDomain);
+    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);
     SetDefaults(nat_true_others_default_);
-    nat_true_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+    nat_true_others_default_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                        true);
     SetDefaults(nat_false_others_default_);
     nat_false_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
+        key::kRemoteAccessHostFirewallTraversal, false);
     SetDefaults(domain_empty_others_default_);
-    domain_empty_others_default_.SetString(policy::key::kRemoteAccessHostDomain,
+    domain_empty_others_default_.SetString(key::kRemoteAccessHostDomain,
                                            std::string());
     SetDefaults(domain_full_others_default_);
-    domain_full_others_default_.SetString(policy::key::kRemoteAccessHostDomain,
+    domain_full_others_default_.SetString(key::kRemoteAccessHostDomain,
                                           kHostDomain);
-    nat_true_domain_empty_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_true_domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
+    nat_true_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                      true);
+    nat_true_domain_empty_.SetString(key::kRemoteAccessHostDomain,
                                      std::string());
-    nat_true_domain_full_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
-    nat_true_domain_full_.SetString(policy::key::kRemoteAccessHostDomain,
-                                    kHostDomain);
-    nat_false_domain_empty_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
-    nat_false_domain_empty_.SetString(policy::key::kRemoteAccessHostDomain,
+    nat_true_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                     true);
+    nat_true_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
+    nat_false_domain_empty_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                       false);
+    nat_false_domain_empty_.SetString(key::kRemoteAccessHostDomain,
                                       std::string());
-    nat_false_domain_full_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
-    nat_false_domain_full_.SetString(policy::key::kRemoteAccessHostDomain,
-                                     kHostDomain);
+    nat_false_domain_full_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                      false);
+    nat_false_domain_full_.SetString(key::kRemoteAccessHostDomain, kHostDomain);
     SetDefaults(nat_true_domain_empty_others_default_);
     nat_true_domain_empty_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+        key::kRemoteAccessHostFirewallTraversal, true);
     nat_true_domain_empty_others_default_.SetString(
-        policy::key::kRemoteAccessHostDomain, std::string());
+        key::kRemoteAccessHostDomain, std::string());
     unknown_policies_.SetString("UnknownPolicyOne", std::string());
     unknown_policies_.SetString("UnknownPolicyTwo", std::string());
     unknown_policies_.SetBoolean("RemoteAccessHostUnknownPolicyThree", true);
 
     const char kOverrideNatTraversalToFalse[] =
         "{ \"RemoteAccessHostFirewallTraversal\": false }";
-    nat_true_and_overridden_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, true);
+    nat_true_and_overridden_.SetBoolean(key::kRemoteAccessHostFirewallTraversal,
+                                        true);
     nat_true_and_overridden_.SetString(
-        policy::key::kRemoteAccessHostDebugOverridePolicies,
+        key::kRemoteAccessHostDebugOverridePolicies,
         kOverrideNatTraversalToFalse);
-    pairing_true_.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing,
-                             true);
-    pairing_false_.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing,
-                              false);
-    gnubby_auth_true_.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth,
-                                 true);
-    gnubby_auth_false_.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth,
-                                  false);
-    relay_true_.SetBoolean(policy::key::kRemoteAccessHostAllowRelayedConnection,
-                           true);
-    relay_false_.SetBoolean(
-        policy::key::kRemoteAccessHostAllowRelayedConnection, false);
-    port_range_full_.SetString(policy::key::kRemoteAccessHostUdpPortRange,
-                               kPortRange);
-    port_range_empty_.SetString(policy::key::kRemoteAccessHostUdpPortRange,
+    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());
-    curtain_true_.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain,
-                             true);
-    curtain_false_.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain,
-                              false);
-    username_true_.SetBoolean(policy::key::kRemoteAccessHostMatchUsername,
-                              true);
-    username_false_.SetBoolean(policy::key::kRemoteAccessHostMatchUsername,
-                               false);
-    talk_gadget_blah_.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
-                                "blah");
-    token_url_https_.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
-                               "https://example.com");
-    token_validation_url_https_.SetString(
-        policy::key::kRemoteAccessHostTalkGadgetPrefix, "https://example.com");
-    token_certificate_blah_.SetString(
-        policy::key::kRemoteAccessHostTokenValidationCertificateIssuer, "blah");
+    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);
+
+    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, "");
 
 #if !defined(NDEBUG)
     SetDefaults(nat_false_overridden_others_default_);
     nat_false_overridden_others_default_.SetBoolean(
-        policy::key::kRemoteAccessHostFirewallTraversal, false);
+        key::kRemoteAccessHostFirewallTraversal, false);
     nat_false_overridden_others_default_.SetString(
-        policy::key::kRemoteAccessHostDebugOverridePolicies,
+        key::kRemoteAccessHostDebugOverridePolicies,
         kOverrideNatTraversalToFalse);
 #endif
   }
 
   void TearDown() override {
     policy_watcher_.reset();
     policy_loader_ = nullptr;
     base::RunLoop().RunUntilIdle();
   }
 
@@ skipping 41 matching lines @@
   // |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_;
   scoped_ptr<PolicyWatcher> policy_watcher_;
 
   base::DictionaryValue empty_;
   base::DictionaryValue nat_true_;
   base::DictionaryValue nat_false_;
   base::DictionaryValue nat_one_;
+  base::DictionaryValue nat_one_domain_full_;
   base::DictionaryValue domain_empty_;
   base::DictionaryValue domain_full_;
   base::DictionaryValue nat_true_others_default_;
   base::DictionaryValue nat_false_others_default_;
   base::DictionaryValue domain_empty_others_default_;
   base::DictionaryValue domain_full_others_default_;
   base::DictionaryValue nat_true_domain_empty_;
   base::DictionaryValue nat_true_domain_full_;
   base::DictionaryValue nat_false_domain_empty_;
   base::DictionaryValue nat_false_domain_full_;
   base::DictionaryValue nat_true_domain_empty_others_default_;
   base::DictionaryValue unknown_policies_;
   base::DictionaryValue nat_true_and_overridden_;
   base::DictionaryValue nat_false_overridden_others_default_;
   base::DictionaryValue pairing_true_;
   base::DictionaryValue pairing_false_;
   base::DictionaryValue gnubby_auth_true_;
   base::DictionaryValue gnubby_auth_false_;
   base::DictionaryValue relay_true_;
   base::DictionaryValue relay_false_;
   base::DictionaryValue port_range_full_;
   base::DictionaryValue port_range_empty_;
+  base::DictionaryValue port_range_malformed_;
+  base::DictionaryValue port_range_malformed_domain_full_;
   base::DictionaryValue curtain_true_;
   base::DictionaryValue curtain_false_;
   base::DictionaryValue username_true_;
   base::DictionaryValue username_false_;
   base::DictionaryValue talk_gadget_blah_;
-  base::DictionaryValue token_url_https_;
-  base::DictionaryValue token_validation_url_https_;
-  base::DictionaryValue token_certificate_blah_;
+  base::DictionaryValue third_party_auth_full_;
+  base::DictionaryValue third_party_auth_partial_;
+  base::DictionaryValue third_party_auth_cert_empty_;
 
  private:
   void SetDefaults(base::DictionaryValue& dict) {
-    dict.SetBoolean(policy::key::kRemoteAccessHostFirewallTraversal, true);
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowRelayedConnection, true);
-    dict.SetString(policy::key::kRemoteAccessHostUdpPortRange, "");
-    dict.SetString(policy::key::kRemoteAccessHostDomain, std::string());
-    dict.SetBoolean(policy::key::kRemoteAccessHostMatchUsername, false);
-    dict.SetString(policy::key::kRemoteAccessHostTalkGadgetPrefix,
+    dict.SetBoolean(key::kRemoteAccessHostFirewallTraversal, true);
+    dict.SetBoolean(key::kRemoteAccessHostAllowRelayedConnection, true);
+    dict.SetString(key::kRemoteAccessHostUdpPortRange, "");
+    dict.SetString(key::kRemoteAccessHostDomain, std::string());
+    dict.SetBoolean(key::kRemoteAccessHostMatchUsername, false);
+    dict.SetString(key::kRemoteAccessHostTalkGadgetPrefix,
                    kDefaultHostTalkGadgetPrefix);
-    dict.SetBoolean(policy::key::kRemoteAccessHostRequireCurtain, false);
-    dict.SetString(policy::key::kRemoteAccessHostTokenUrl, std::string());
-    dict.SetString(policy::key::kRemoteAccessHostTokenValidationUrl,
-                   std::string());
-    dict.SetString(
-        policy::key::kRemoteAccessHostTokenValidationCertificateIssuer,
-        std::string());
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowClientPairing, true);
-    dict.SetBoolean(policy::key::kRemoteAccessHostAllowGnubbyAuth, true);
+    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);
 #if !defined(NDEBUG)
-    dict.SetString(policy::key::kRemoteAccessHostDebugOverridePolicies, "");
+    dict.SetString(key::kRemoteAccessHostDebugOverridePolicies, "");
 #endif
 
     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::kPortRange = "12400-12409";
@@ skipping 22 matching lines @@
   StartWatching();
 }
 
 TEST_F(PolicyWatcherTest, NatWrongType) {
   EXPECT_CALL(mock_policy_callback_, OnPolicyError());
 
   SetPolicies(nat_one_);
   StartWatching();
 }
 
+// This test verifies that a mistyped policy value is still detected
+// even though it doesn't change during the second SetPolicies call.
+TEST_F(PolicyWatcherTest, NatWrongTypeThenIrrelevantChange) {
+  EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(2);
+
+  SetPolicies(nat_one_);
+  StartWatching();
+  SetPolicies(nat_one_domain_full_);
+}
+
+// This test verifies that a malformed policy value is still detected
+// even though it doesn't change during the second SetPolicies call.
+TEST_F(PolicyWatcherTest, PortRangeMalformedThenIrrelevantChange) {
+  EXPECT_CALL(mock_policy_callback_, OnPolicyError()).Times(2);
+
+  SetPolicies(port_range_malformed_);
+  StartWatching();
+  SetPolicies(port_range_malformed_domain_full_);
+}
+
 TEST_F(PolicyWatcherTest, DomainEmpty) {
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&domain_empty_others_default_)));
 
   SetPolicies(domain_empty_);
   StartWatching();
 }
 
 TEST_F(PolicyWatcherTest, DomainFull) {
   EXPECT_CALL(mock_policy_callback_,
@@ skipping 194 matching lines @@
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&talk_gadget_blah_)));
 
   SetPolicies(empty_);
   StartWatching();
   SetPolicies(talk_gadget_blah_);
 }
 
-TEST_F(PolicyWatcherTest, TokenUrl) {
+TEST_F(PolicyWatcherTest, ThirdPartyAuthFull) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_url_https_)));
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_full_)));
 
   SetPolicies(empty_);
   StartWatching();
-  SetPolicies(token_url_https_);
+  SetPolicies(third_party_auth_full_);
 }
 
-TEST_F(PolicyWatcherTest, TokenValidationUrl) {
+// This test verifies what happens when only 1 out of 3 third-party auth
+// policies changes.  Without the other 2 policy values such policy values
+// combination is invalid (i.e. cannot have TokenUrl without
+// TokenValidationUrl) and can trigger OnPolicyError unless PolicyWatcher
+// implementation is careful around this scenario.
+TEST_F(PolicyWatcherTest, ThirdPartyAuthPartialToFull) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_validation_url_https_)));
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_cert_empty_)));
+  EXPECT_CALL(mock_policy_callback_,
+              OnPolicyUpdatePtr(IsPolicies(&third_party_auth_full_)));
 
   SetPolicies(empty_);
   StartWatching();
-  SetPolicies(token_validation_url_https_);
-}
-
-TEST_F(PolicyWatcherTest, TokenValidationCertificateIssuer) {
-  testing::InSequence sequence;
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
-  EXPECT_CALL(mock_policy_callback_,
-              OnPolicyUpdatePtr(IsPolicies(&token_certificate_blah_)));
-
-  SetPolicies(empty_);
-  StartWatching();
-  SetPolicies(token_certificate_blah_);
+  SetPolicies(third_party_auth_partial_);
+  SetPolicies(third_party_auth_full_);
 }
 
 TEST_F(PolicyWatcherTest, UdpPortRange) {
   testing::InSequence sequence;
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&nat_true_others_default_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&port_range_full_)));
   EXPECT_CALL(mock_policy_callback_,
               OnPolicyUpdatePtr(IsPolicies(&port_range_empty_)));
@@ skipping 12 matching lines @@
   // are kept in-sync.
 
   std::map<std::string, base::Value::Type> expected_schema;
   for (base::DictionaryValue::Iterator i(GetDefaultValues()); !i.IsAtEnd();
        i.Advance()) {
     expected_schema[i.key()] = i.value().GetType();
   }
 #if defined(OS_WIN)
   // RemoteAccessHostMatchUsername is marked in policy_templates.json as not
   // supported on Windows and therefore is (by design) excluded from the schema.
-  expected_schema.erase(policy::key::kRemoteAccessHostMatchUsername);
+  expected_schema.erase(key::kRemoteAccessHostMatchUsername);
 #endif
 #if defined(NDEBUG)
   // Policy schema / policy_templates.json cannot differ between debug and
   // release builds so we compensate below to account for the fact that
   // PolicyWatcher::default_values_ does differ between debug and release.
-  expected_schema[policy::key::kRemoteAccessHostDebugOverridePolicies] =
+  expected_schema[key::kRemoteAccessHostDebugOverridePolicies] =
       base::Value::TYPE_STRING;
 #endif
 
   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
@@ skipping 78 matching lines @@
   }
 
   // Today, the only verification offered by this test is:
   // - Manual verification of policy values dumped by OnPolicyUpdatedDumpPolicy
   // - Automated verification that nothing crashed
 }
 
 #endif
 
 }  // namespace remoting