Add a 'default' field to policy_templates.json so policies can indicate

their default value in a machine readable way.

Add presubmit check to ensure new policies that should have defaults
specify their defaults.

Added the new default field to one of each of the types that should
have defaults to ensure the validation code works.

Default were added to:
 MaxConnectionsPerProxy
 AbusiveExperienceInterventionEnforce
 AssistantOnboardingMode
 CACertificateManagementAllowed

Bug: 1139046
Change-Id: I456d45bb98e6ffeb5447d03117f429df8936f8a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476585Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Chris Sharp <csharp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821757}

components/policy/resources/policy_templates.json

@@ -123,10 +123,29 @@
 #   Note that 'chrome_frame' has been deprecated.
 #
 # Annotations:
-#   Additional information is specified under keys 'features' and
-#   'example_value'. These are used in the generated documentation and example
-#   policy configuration files. Examples must cover the entire schema, i.e. use
-#   every defined property at least once.
+#   'example_value' is used in the generated documentation and example policy
+#   configuration files. Examples must cover the entire schema, i.e. use every
+#   defined property at least once.
+#
+#   'default' is used to provide a machine readable way of indicating the
+#   behavior of the controlled setting/feature when the policy is not set. For
+#   example, if 'default' is set to True, it documents that when the policy is
+#   not set, the feature controlled by the policy will behave in the same way
+#   it would behave if the policy was set to True.
+#   This field is only used for policies of type 'int', 'main', 'string-enum',
+#   or 'int-enum'. Other policy types are assumed to have a default equal to
+#   an empty string or empty list. If the default value can't be expressed with
+#   the given type and is equivalent to unset, it should be represent with None.
+#   For example, this would be used for a boolean policy where unset means the
+#   user can turn the feature on and off, while a set policy will force the
+#   feature to always be on or off.
+#   Note that this is different from 'default_for_enterprise_users', as that
+#   flag sets a policy value for managed users if the policy is not set by some
+#   other policy source (such as cloud policy). It is also different from
+#   'default_for_managed_devices_doc_only', which documents an explicitly
+#   implemented default policy value for managed devices.
+#
+#   Additional information is specified under the 'features' key:
 #   'dynamic_refresh' controls if the generated documentation should state that
 #   the policy supports dynamic refresh or not. Supporting dynamic refresh means
 #   that Chrome respects the changes to the policy immediately, without the need
@@ -7823,6 +7842,7 @@
         'per_profile': False,
       },
       'example_value': 32,
+      'default': 32,
       'id': 92,
       'caption': '''Maximal number of concurrent connections to the proxy server''',
       'tags': [],
@@ -17095,6 +17115,7 @@
         'per_profile': True,
       },
       'example_value': True,
+      'default': True,
       'id': 404,
       'caption': '''Abusive Experience Intervention Enforce''',
       'tags': [],
@@ -19816,6 +19837,7 @@
         'per_profile': True,
       },
       'example_value': 'Default',
+      'default': 'Default',
       'id': 729,
       'caption': '''Mode of the Assistant onboarding experience''',
       'desc':  '''This policy controls the mode of the Assistant onboarding experience.
@@ -20882,6 +20904,7 @@
         'per_profile': True
       },
       'example_value': 1,
+      'default': 0,
       'id': 579,
       'caption': '''Allow users to manage installed CA certificates.''',
       'desc': '''Setting the policy to All (0) or leaving it unset lets users edit trust settings for all CA certificates, remove user-imported certificates, and import certificates using Certificate Manager. Setting the policy to UserOnly (1) lets users manage only user-imported certificates, but not change trust settings of built-in certificates. Setting it to None (2) lets users view (not manage) CA certificates.''',

components/policy/tools/syntax_check_policy_template_json.py

@@ -477,6 +477,48 @@ class PolicyTemplateChecker(object):
                       "Field '%s' not present in device policy proto." %
                       (policy, field))
 
+  def _NeedsDefault(self, policy):
+    return policy.get('type') in ('int', 'main', 'string-enum', 'int-enum')
+
+  def _CheckDefault(self, policy):
+    if not self._NeedsDefault(policy):
+      return
+
+    # Only validate the default when present.
+    # TODO(crbug.com/1139046): Always validate the default for types that
+    # should have it.
+    if 'default' not in policy:
+      return
+
+    policy_type = policy.get('type')
+    default = policy.get('default')
+    if policy_type == 'int':
+      # A default value of None is acceptable when the default case is
+      # equivalent to the policy being unset and there is no numeric equivalent.
+      if default is None:
+        return
+
+      default = self._CheckContains(policy, 'default', int)
+      if default is None or default < 0:
+        self._Error(
+            ('Default for policy %s of type int should be an int >= 0 or None, '
+             'got %s') % (policy.get('name'), default))
+      return
+
+    if policy_type == 'main':
+      # TODO(crbug.com/1139306): Query the acceptable values from items
+      # once that is used for policy type main.
+      acceptable_values = (True, False, None)
+    elif policy_type in ('string-enum', 'int-enum'):
+      acceptable_values = [None] + [x['value'] for x in policy['items']]
+    else:
+      raise NotImplementedError('Unimplemented policy type: %s' % policy_type)
+
+    if default not in acceptable_values:
+      self._Error(
+          ('Default for policy %s of type %s should be one of %s, got %s') %
+          (policy.get('name'), policy_type, acceptable_values, default))
+
   def _CheckPolicy(self, policy, is_in_group, policy_ids, deleted_policy_ids,
                    current_version):
     if not isinstance(policy, dict):
@@ -508,6 +550,7 @@ class PolicyTemplateChecker(object):
           'url_schema',
           'max_size',
           'tags',
+          'default',
           'default_for_enterprise_users',
           'default_for_managed_devices_doc_only',
           'arc_support',
@@ -837,6 +880,8 @@ class PolicyTemplateChecker(object):
             self._Error(('Example for policy %s does not comply to the ' +
                          'policy\'s validation_schema') % policy.get('name'))
 
+      self._CheckDefault(policy)
+
       # Statistics.
       self.num_policies += 1
       if is_in_group:
@@ -1428,6 +1473,12 @@ class PolicyTemplateChecker(object):
                                         current_version, new_policy_name)
       self._CheckDeprecatedFutureField(None, new_policy, new_policy_name)
 
+      # TODO(crbug.com/1139046): This default check should apply to all
+      # policies instead of just new ones.
+      if self._NeedsDefault(new_policy) and new_policy.get('default',
+                                                           None) == None:
+        self._Error(('Missing default field for policy %s') % (new_policy_name))
+
   def _LeadingWhitespace(self, line):
     match = LEADING_WHITESPACE.match(line)
     if match: