Display schema validation errors

This CL makes sure all JSON device policies use
DecodeJsonStringAndDropUnknownBySchema to decode the proto and adhere
to the associated schema. Unknown properties are dropped. If the json
string can't be decoded or does not comply with the associated schema
in policy_templates.json, the policy value will be set to the original
json string. This way, the faulty value can still be seen in
chrome://policy along with any errors/warnings.

Bug: 855054
Change-Id: I73a2bc7cc88a293b06339482437f88ac48932b83
Reviewed-on: https://chromium-review.googlesource.com/1163507Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585451}

chrome/browser/chromeos/BUILD.gn

@@ -2203,6 +2203,7 @@ source_set("unit_tests") {
     "policy/device_cloud_policy_manager_chromeos_unittest.cc",
     "policy/device_cloud_policy_store_chromeos_unittest.cc",
     "policy/device_local_account_policy_service_unittest.cc",
+    "policy/device_policy_decoder_chromeos_unittest.cc",
     "policy/dm_token_storage_unittest.cc",
     "policy/extension_cache_unittest.cc",
     "policy/fake_affiliated_invalidation_service_provider.cc",

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc

@@ -15,7 +15,6 @@
 #include "base/strings/stringprintf.h"
 #include "base/syslog_logging.h"
 #include "base/values.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h"
 #include "chrome/browser/chromeos/tpm_firmware_update.h"
@@ -23,6 +22,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/update_engine_client.h"
 #include "chromeos/settings/cros_settings_names.h"
+#include "components/policy/core/common/chrome_schema.h"
 #include "components/policy/core/common/external_data_fetcher.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
@@ -40,6 +40,26 @@ namespace policy {
 
 namespace {
 
+// If the |json_string| can be decoded and validated against the schema
+// identified by |policy_name| in policy_templates.json, the policy
+// |policy_name| in |policies| will be set to the decoded base::Value.
+// Otherwise, the policy will be set to a base::Value of the original
+// |json_string|. This way, the faulty value can still be seen in
+// chrome://policy along with any errors/warnings.
+void SetJsonDevicePolicy(const std::string& policy_name,
+                         const std::string& json_string,
+                         PolicyMap* policies) {
+  std::string error;
+  std::unique_ptr<base::Value> decoded_json =
+      DecodeJsonStringAndNormalize(json_string, policy_name, &error);
+  auto value_to_set = decoded_json ? std::move(decoded_json)
+                                   : std::make_unique<base::Value>(json_string);
+  policies->Set(policy_name, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
+                POLICY_SOURCE_CLOUD, std::move(value_to_set), nullptr);
+  if (!error.empty())
+    policies->SetError(policy_name, error);
+}
+
 // Decodes a protobuf integer to an IntegerValue. Returns NULL in case the input
 // value is out of bounds.
 std::unique_ptr<base::Value> DecodeIntegerValue(google::protobuf::int64 value) {
@@ -53,50 +73,6 @@ std::unique_ptr<base::Value> DecodeIntegerValue(google::protobuf::int64 value) {
   return std::unique_ptr<base::Value>(new base::Value(static_cast<int>(value)));
 }
 
-// Decodes a JSON string to a base::Value, and drops unknown properties
-// according to a policy schema. |policy_name| is the name of a policy schema
-// defined in policy_templates.json. Returns NULL in case the input is not a
-// valid JSON string.
-std::unique_ptr<base::Value> DecodeJsonStringAndDropUnknownBySchema(
-    const std::string& json_string,
-    const std::string& policy_name) {
-  std::string error;
-  std::unique_ptr<base::Value> root = base::JSONReader::ReadAndReturnError(
-      json_string, base::JSON_ALLOW_TRAILING_COMMAS, NULL, &error);
-
-  if (!root) {
-    LOG(WARNING) << "Invalid JSON string: " << error << ", ignoring.";
-    return std::unique_ptr<base::Value>();
-  }
-
-  const Schema& schema = g_browser_process->browser_policy_connector()
-                             ->GetChromeSchema()
-                             .GetKnownProperty(policy_name);
-
-  if (schema.valid()) {
-    std::string error_path;
-    bool changed = false;
-
-    if (!schema.Normalize(root.get(), SCHEMA_ALLOW_UNKNOWN, &error_path, &error,
-                          &changed)) {
-      LOG(WARNING) << "Invalid policy value for " << policy_name << ": "
-                   << error << " at " << error_path << ".";
-      return std::unique_ptr<base::Value>();
-    }
-
-    if (changed) {
-      LOG(WARNING) << "Some properties in " << policy_name
-                   << " were dropped: " << error << " at " << error_path << ".";
-    }
-  } else {
-    LOG(WARNING) << "Unknown or invalid policy schema for " << policy_name
-                 << ".";
-    return std::unique_ptr<base::Value>();
-  }
-
-  return root;
-}
-
 std::unique_ptr<base::Value> DecodeConnectionType(int value) {
   static const char* const kConnectionTypes[] = {
       shill::kTypeEthernet,  shill::kTypeWifi,     shill::kTypeWimax,
@@ -335,15 +311,8 @@ void DecodeLoginPolicies(const em::ChromeDeviceSettingsProto& policy,
     const em::LoginScreenPowerManagementProto& container(
         policy.login_screen_power_management());
     if (container.has_login_screen_power_management()) {
-      std::unique_ptr<base::Value> decoded_json;
-      decoded_json = DecodeJsonStringAndDropUnknownBySchema(
-          container.login_screen_power_management(),
-          key::kDeviceLoginScreenPowerManagement);
-      if (decoded_json) {
-        policies->Set(key::kDeviceLoginScreenPowerManagement,
-                      POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
-                      POLICY_SOURCE_CLOUD, std::move(decoded_json), nullptr);
-      }
+      SetJsonDevicePolicy(key::kDeviceLoginScreenPowerManagement,
+                          container.login_screen_power_management(), policies);
     }
   }
 
@@ -674,28 +643,13 @@ void DecodeAutoUpdatePolicies(const em::ChromeDeviceSettingsProto& policy,
     }
 
     if (container.has_disallowed_time_intervals()) {
-      std::unique_ptr<base::Value> decoded_json =
-          DecodeJsonStringAndDropUnknownBySchema(
-              container.disallowed_time_intervals(),
-              key::kDeviceAutoUpdateTimeRestrictions);
-      if (decoded_json && !decoded_json->is_none()) {
-        policies->Set(key::kDeviceAutoUpdateTimeRestrictions,
-                      POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
-                      POLICY_SOURCE_CLOUD, std::move(decoded_json), nullptr);
-      }
+      SetJsonDevicePolicy(key::kDeviceAutoUpdateTimeRestrictions,
+                          container.disallowed_time_intervals(), policies);
     }
 
     if (container.has_staging_schedule()) {
-      std::unique_ptr<base::Value> staging_percent_of_fleet_per_week_policy =
-          DecodeJsonStringAndDropUnknownBySchema(
-              container.staging_schedule(), key::kDeviceUpdateStagingSchedule);
-
-      if (staging_percent_of_fleet_per_week_policy) {
-        policies->Set(key::kDeviceUpdateStagingSchedule, POLICY_LEVEL_MANDATORY,
-                      POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD,
-                      std::move(staging_percent_of_fleet_per_week_policy),
-                      nullptr);
-      }
+      SetJsonDevicePolicy(key::kDeviceUpdateStagingSchedule,
+                          container.staging_schedule(), policies);
     }
   }
 
@@ -933,17 +887,8 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
     const em::DeviceWallpaperImageProto& container(
         policy.device_wallpaper_image());
     if (container.has_device_wallpaper_image()) {
-      std::unique_ptr<base::DictionaryValue> dict_val =
-          base::DictionaryValue::From(
-              base::JSONReader::Read(container.device_wallpaper_image()));
-      if (dict_val) {
-        policies->Set(key::kDeviceWallpaperImage, POLICY_LEVEL_MANDATORY,
-                      POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD,
-                      std::move(dict_val), nullptr);
-      } else {
-        SYSLOG(ERROR) << "Value of wallpaper policy has invalid format: "
-                      << container.device_wallpaper_image();
-      }
+      SetJsonDevicePolicy(key::kDeviceWallpaperImage,
+                          container.device_wallpaper_image(), policies);
     }
   }
 
@@ -977,12 +922,8 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
     const em::DeviceNativePrintersProto& container(
         policy.native_device_printers());
     if (container.has_external_policy()) {
-      std::unique_ptr<base::DictionaryValue> dict_val =
-          base::DictionaryValue::From(
-              base::JSONReader::Read(container.external_policy()));
-      policies->Set(key::kDeviceNativePrinters, POLICY_LEVEL_MANDATORY,
-                    POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD,
-                    std::move(dict_val), nullptr);
+      SetJsonDevicePolicy(key::kDeviceNativePrinters,
+                          container.external_policy(), policies);
     }
   }
 
@@ -1112,6 +1053,43 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
 
 }  // namespace
 
+std::unique_ptr<base::Value> DecodeJsonStringAndNormalize(
+    const std::string& json_string,
+    const std::string& policy_name,
+    std::string* error) {
+  std::string json_error;
+  std::unique_ptr<base::Value> root = base::JSONReader::ReadAndReturnError(
+      json_string, base::JSON_ALLOW_TRAILING_COMMAS, NULL, &json_error);
+  if (!root) {
+    *error = "Invalid JSON string: " + json_error;
+    return nullptr;
+  }
+
+  const Schema& schema =
+      policy::GetChromeSchema().GetKnownProperty(policy_name);
+  CHECK(schema.valid());
+
+  std::string schema_error;
+  std::string error_path;
+  bool changed = false;
+  if (!schema.Normalize(root.get(), SCHEMA_ALLOW_UNKNOWN, &error_path,
+                        &schema_error, &changed)) {
+    std::ostringstream msg;
+    msg << "Invalid policy value: " << schema_error << " (at "
+        << (error_path.empty() ? "toplevel" : error_path) << ")";
+    *error = msg.str();
+    return nullptr;
+  }
+  if (changed) {
+    std::ostringstream msg;
+    msg << "Dropped unknown properties: " << schema_error << " (at "
+        << (error_path.empty() ? "toplevel" : error_path) << ")";
+    *error = msg.str();
+  }
+
+  return root;
+}
+
 void DecodeDevicePolicy(const em::ChromeDeviceSettingsProto& policy,
                         PolicyMap* policies) {
   // Decode the various groups of policies.

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.h

@@ -5,14 +5,32 @@
 #ifndef CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_POLICY_DECODER_CHROMEOS_H_
 #define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_POLICY_DECODER_CHROMEOS_H_
 
+#include <memory>
+#include <string>
+
 namespace enterprise_management {
 class ChromeDeviceSettingsProto;
 }
 
+namespace base {
+class Value;
+}
+
 namespace policy {
 
 class PolicyMap;
 
+// Decodes a JSON string to a base::Value and validates it against the schema
+// defined in policy_templates.json for the policy named |policy_name|. Unknown
+// properties are dropped. Returns nullptr if the input cannot be parsed as
+// valid JSON string or doesn't comply with the declared schema (e.g. mismatched
+// type, missing required field, etc.). Any warning or error messages from the
+// decoding and schema validation process are stored in |error|.
+std::unique_ptr<base::Value> DecodeJsonStringAndNormalize(
+    const std::string& json_string,
+    const std::string& policy_name,
+    std::string* error);
+
 // Decodes device policy in ChromeDeviceSettingsProto representation into the a
 // PolicyMap.
 void DecodeDevicePolicy(

chrome/browser/chromeos/policy/device_policy_decoder_chromeos_unittest.cc

+// Copyright 2018 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/policy/device_policy_decoder_chromeos.h"
+#include "components/policy/policy_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace policy {
+
+namespace {
+
+constexpr char kInvalidJson[] = R"({"foo": "bar")";
+
+constexpr char kInvalidPolicyName[] = "invalid-policy-name";
+
+constexpr char kWallpaperJson[] = R"({
+      "url": "https://example.com/device_wallpaper.jpg",
+      "hash": "examplewallpaperhash"
+    })";
+
+constexpr char kWallpaperJsonInvalidValue[] = R"({
+      "url": 123,
+      "hash": "examplewallpaperhash"
+    })";
+
+constexpr char kWallpaperJsonUnknownProperty[] = R"({
+    "url": "https://example.com/device_wallpaper.jpg",
+    "hash": "examplewallpaperhash",
+    "unknown-field": "random-value"
+  })";
+
+constexpr char kWallpaperUrlPropertyName[] = "url";
+constexpr char kWallpaperUrlPropertyValue[] =
+    "https://example.com/device_wallpaper.jpg";
+constexpr char kWallpaperHashPropertyName[] = "hash";
+constexpr char kWallpaperHashPropertyValue[] = "examplewallpaperhash";
+
+}  // namespace
+
+class DevicePolicyDecoderChromeOSTest : public testing::Test {
+ public:
+  DevicePolicyDecoderChromeOSTest() = default;
+  ~DevicePolicyDecoderChromeOSTest() override = default;
+
+ protected:
+  std::unique_ptr<base::Value> GetWallpaperDict() const;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DevicePolicyDecoderChromeOSTest);
+};
+
+std::unique_ptr<base::Value> DevicePolicyDecoderChromeOSTest::GetWallpaperDict()
+    const {
+  auto dict = std::make_unique<base::DictionaryValue>();
+  dict->SetKey(kWallpaperUrlPropertyName,
+               base::Value(kWallpaperUrlPropertyValue));
+  dict->SetKey(kWallpaperHashPropertyName,
+               base::Value(kWallpaperHashPropertyValue));
+  return dict;
+}
+
+TEST_F(DevicePolicyDecoderChromeOSTest,
+       DecodeJsonStringAndNormalizeJSONParseError) {
+  std::string error;
+  std::unique_ptr<base::Value> decoded_json = DecodeJsonStringAndNormalize(
+      kInvalidJson, key::kDeviceWallpaperImage, &error);
+  EXPECT_FALSE(decoded_json);
+  EXPECT_EQ("Invalid JSON string: Line: 1, column: 13, Syntax error.", error);
+}
+
+#if GTEST_HAS_DEATH_TEST
+TEST_F(DevicePolicyDecoderChromeOSTest,
+       DecodeJsonStringAndNormalizeInvalidSchema) {
+  std::string error;
+  EXPECT_DEATH(
+      DecodeJsonStringAndNormalize(kWallpaperJson, kInvalidPolicyName, &error),
+      "");
+}
+#endif
+
+TEST_F(DevicePolicyDecoderChromeOSTest,
+       DecodeJsonStringAndNormalizeInvalidValue) {
+  std::string error;
+  std::unique_ptr<base::Value> decoded_json = DecodeJsonStringAndNormalize(
+      kWallpaperJsonInvalidValue, key::kDeviceWallpaperImage, &error);
+  EXPECT_FALSE(decoded_json);
+  EXPECT_EQ(
+      "Invalid policy value: The value type doesn't match the schema type. (at "
+      "url)",
+      error);
+}
+
+TEST_F(DevicePolicyDecoderChromeOSTest,
+       DecodeJsonStringAndNormalizeUnknownProperty) {
+  std::string error;
+  std::unique_ptr<base::Value> decoded_json = DecodeJsonStringAndNormalize(
+      kWallpaperJsonUnknownProperty, key::kDeviceWallpaperImage, &error);
+  EXPECT_EQ(*GetWallpaperDict(), *decoded_json);
+  EXPECT_EQ(
+      "Dropped unknown properties: Unknown property: unknown-field (at "
+      "toplevel)",
+      error);
+}
+
+TEST_F(DevicePolicyDecoderChromeOSTest, DecodeJsonStringAndNormalizeSuccess) {
+  std::string error;
+  std::unique_ptr<base::Value> decoded_json = DecodeJsonStringAndNormalize(
+      kWallpaperJson, key::kDeviceWallpaperImage, &error);
+  EXPECT_EQ(*GetWallpaperDict(), *decoded_json);
+  EXPECT_TRUE(error.empty());
+}
+
+}  // namespace policy

chrome/browser/chromeos/settings/device_settings_provider.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
+#include "chrome/browser/chromeos/policy/device_policy_decoder_chromeos.h"
 #include "chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_cache.h"
@@ -35,6 +36,7 @@
 #include "components/policy/core/common/chrome_schema.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/core/common/schema.h"
+#include "components/policy/policy_constants.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/prefs/pref_service.h"
 
@@ -121,49 +123,23 @@ const char* const kKnownSettings[] = {
     kDeviceAutoUpdateTimeRestrictions,
 };
 
-// Decodes a JSON string to a base::Value, and drops unknown properties
-// according to a policy schema. |policy_name| is the name of a policy schema
-// defined in policy_templates.json. Returns null in case the input is not a
-// valid JSON string.
-std::unique_ptr<base::Value> DecodeJsonStringAndDropUnknownBySchema(
-    const std::string& json_string,
-    const std::string& policy_name) {
+// Re-use the DecodeJsonStringAndNormalize from device_policy_decoder_chromeos.h
+// here to decode the json string and validate it against |policy_name|'s
+// schema. If the json string is valid, the decoded base::Value will be stored
+// as |setting_name| in |pref_value_map|. The error can be ignored here since it
+// is already reported during decoding in device_policy_decoder_chromeos.cc.
+void SetJsonDeviceSetting(const std::string& setting_name,
+                          const std::string& policy_name,
+                          const std::string& json_string,
+                          PrefValueMap* pref_value_map) {
   std::string error;
-  std::unique_ptr<base::Value> root = base::JSONReader::ReadAndReturnError(
-      json_string, base::JSON_ALLOW_TRAILING_COMMAS, nullptr, &error);
-
-  if (!root) {
-    LOG(WARNING) << "Invalid JSON string: " << error << ", ignoring.";
-    return nullptr;
-  }
-
-  const policy::Schema& schema =
-      policy::GetChromeSchema().GetKnownProperty(policy_name);
-
-  if (!schema.valid()) {
-    LOG(WARNING) << "Unknown or invalid policy schema for " << policy_name
-                 << ".";
-    return nullptr;
-  }
-
-  std::string error_path;
-  bool changed = false;
-  if (!schema.Normalize(root.get(), policy::SCHEMA_ALLOW_UNKNOWN, &error_path,
-                        &error, &changed)) {
-    LOG(WARNING) << "Invalid policy value for " << policy_name << ": " << error
-                 << " at " << error_path << ".";
-    return nullptr;
-  }
-  if (changed) {
-    LOG(WARNING) << "Some properties in " << policy_name
-                 << " were dropped: " << error << " at " << error_path << ".";
-  }
-
-  return root;
+  std::unique_ptr<base::Value> decoded_json =
+      policy::DecodeJsonStringAndNormalize(json_string, policy_name, &error);
+  if (decoded_json)
+    pref_value_map->SetValue(setting_name, std::move(decoded_json));
 }
 
 void DecodeLoginPolicies(const em::ChromeDeviceSettingsProto& policy,
-                         bool is_enterprise_managed,
                          PrefValueMap* new_values_cache) {
   // For all our boolean settings the following is applicable:
   // true is default permissive value and false is safe prohibitive value.
@@ -202,7 +178,7 @@ void DecodeLoginPolicies(const em::ChromeDeviceSettingsProto& policy,
       policy.guest_mode_enabled().guest_mode_enabled());
 
   bool supervised_users_enabled = false;
-  if (is_enterprise_managed) {
+  if (InstallAttributes::Get()->IsEnterpriseManaged()) {
     supervised_users_enabled =
         policy.has_supervised_users_settings() &&
         policy.supervised_users_settings().has_supervised_users_enabled() &&
@@ -444,14 +420,10 @@ void DecodeAutoUpdatePolicies(
     }
 
     if (au_settings_proto.has_disallowed_time_intervals()) {
-      std::unique_ptr<base::Value> decoded_intervals =
-          DecodeJsonStringAndDropUnknownBySchema(
-              au_settings_proto.disallowed_time_intervals(),
-              "DeviceAutoUpdateTimeRestrictions");
-      if (decoded_intervals) {
-        new_values_cache->SetValue(kDeviceAutoUpdateTimeRestrictions,
-                                   std::move(decoded_intervals));
-      }
+      SetJsonDeviceSetting(kDeviceAutoUpdateTimeRestrictions,
+                           policy::key::kDeviceAutoUpdateTimeRestrictions,
+                           au_settings_proto.disallowed_time_intervals(),
+                           new_values_cache);
     }
   }
 }
@@ -534,7 +506,6 @@ void DecodeHeartbeatPolicies(
 }
 
 void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
-                           bool is_enterprise_managed,
                            PrefValueMap* new_values_cache) {
   if (policy.has_metrics_enabled() &&
       policy.metrics_enabled().has_metrics_enabled()) {
@@ -543,7 +514,8 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
   } else {
     // If the policy is missing, default to reporting enabled on enterprise-
     // enrolled devices, c.f. crbug/456186.
-    new_values_cache->SetBoolean(kStatsReportingPref, is_enterprise_managed);
+    new_values_cache->SetBoolean(
+        kStatsReportingPref, InstallAttributes::Get()->IsEnterpriseManaged());
   }
 
   if (!policy.has_release_channel() ||
@@ -637,16 +609,10 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
 
   if (policy.has_device_wallpaper_image() &&
       policy.device_wallpaper_image().has_device_wallpaper_image()) {
-    const std::string& wallpaper_policy(
-        policy.device_wallpaper_image().device_wallpaper_image());
-    std::unique_ptr<base::DictionaryValue> dict_val =
-        base::DictionaryValue::From(base::JSONReader::Read(wallpaper_policy));
-    if (dict_val) {
-      new_values_cache->SetValue(kDeviceWallpaperImage, std::move(dict_val));
-    } else {
-      SYSLOG(ERROR) << "Value of wallpaper policy has invalid format: "
-                    << wallpaper_policy;
-    }
+    SetJsonDeviceSetting(
+        kDeviceWallpaperImage, policy::key::kDeviceWallpaperImage,
+        policy.device_wallpaper_image().device_wallpaper_image(),
+        new_values_cache);
   }
 
   if (policy.has_device_off_hours()) {
@@ -704,7 +670,7 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
   } else {
     // If the policy is missing, default to false on enterprise-enrolled
     // devices.
-    if (is_enterprise_managed) {
+    if (InstallAttributes::Get()->IsEnterpriseManaged()) {
       new_values_cache->SetBoolean(kVirtualMachinesAllowed, false);
     }
   }
@@ -772,14 +738,12 @@ bool DeviceSettingsProvider::IsDeviceSetting(const std::string& name) {
 void DeviceSettingsProvider::DecodePolicies(
     const em::ChromeDeviceSettingsProto& policy,
     PrefValueMap* new_values_cache) {
-  bool is_enterprise_managed = InstallAttributes::Get()->IsEnterpriseManaged();
-
-  DecodeLoginPolicies(policy, is_enterprise_managed, new_values_cache);
+  DecodeLoginPolicies(policy, new_values_cache);
   DecodeNetworkPolicies(policy, new_values_cache);
   DecodeAutoUpdatePolicies(policy, new_values_cache);
   DecodeReportingPolicies(policy, new_values_cache);
   DecodeHeartbeatPolicies(policy, new_values_cache);
-  DecodeGenericPolicies(policy, is_enterprise_managed, new_values_cache);
+  DecodeGenericPolicies(policy, new_values_cache);
   DecodeLogUploadPolicies(policy, new_values_cache);
 }
 

chrome/browser/chromeos/settings/device_settings_provider_unittest.cc

@@ -604,7 +604,7 @@ TEST_F(DeviceSettingsProviderTest, SetWallpaperSettings) {
   EXPECT_EQ(nullptr, provider_->Get(kDeviceWallpaperImage));
 
   // Set with valid json format.
-  const std::string valid_format("{\"type\":\"object\"}");
+  const std::string valid_format(R"({"url":"foo", "hash": "bar"})");
   SetWallpaperSettings(valid_format);
   std::unique_ptr<base::DictionaryValue> expected_value =
       base::DictionaryValue::From(base::JSONReader::Read(valid_format));