Add policy controlled permission block list for extensions

chrome/browser/extensions/extension_management_internal.cc

 // Copyright 2014 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/extensions/extension_management_internal.h"
 
 #include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_management_constants.h"
 #include "extensions/common/url_pattern_set.h"
@@ skipping 49 matching lines @@
           GURL(update_url_str).is_valid()) {
         update_url = update_url_str;
       } else {
         // No valid update URL for extension.
         LOG(WARNING) << kMalformedPreferenceWarning;
         return false;
       }
     }
   }
 
+  // Parses the blocked permission settings.
+  const base::ListValue* list_value;

[Joao da Silva, 2014/10/15 14:39:25]

= NULL

[binjin, 2014/10/16 18:13:58]

Done.

+  base::string16 error;
+
+  // If applicable, inherit from global block list and remove all explicitly
+  // allowed permissions.
+  if (scope != SCOPE_DEFAULT &&
+      dict->GetListWithoutPathExpansion(schema_constants::kAllowedPermissions,
+                                        &list_value)) {
+    APIPermissionSet globally_blocked_permissions = blocked_permissions;

[Joao da Silva, 2014/10/15 14:39:25]

This is extremely subtle.

If I understood it correctly: ExtensionManagement::Refresh() parses the
default_settings_ first, and then individual settings start as a copy of the
default, right? So we if make that copy and then parse the settings for an
extension, the current |blocked_permissions| is the global configuration.

It would be much better if this was made very clear in the code. As it is,
please leave a comment here explaining why |blocked_permissions| is expected to
have the global settings at this step.

[binjin, 2014/10/16 18:13:58]

Done.

+    APIPermissionSet explicitly_allowed_permissions;
+    // Reuses code for parsing API permissions from manifest. But note that we
+    // only support list of strings type.
+    if (!APIPermissionSet::ParseFromJSON(
+            list_value,
+            APIPermissionSet::kDisallowInternalPermissions,
+            &explicitly_allowed_permissions,
+            &error,
+            NULL)) {
+      // There might be unknown permissions, warn and just ignore them;
+      LOG(WARNING) << error;
+    }
+    APIPermissionSet::Difference(globally_blocked_permissions,
+                                 explicitly_allowed_permissions,
+                                 &blocked_permissions);
+  }
+
+  // Then add all newly blocked permissions to the list.
+  if (dict->GetListWithoutPathExpansion(schema_constants::kBlockedPermissions,
+                                        &list_value)) {
+    APIPermissionSet permissions_to_merge_from = blocked_permissions;

[Joao da Silva, 2014/10/15 14:39:25]

Same here.

[binjin, 2014/10/16 18:13:58]

Done. But note that it's slightly different: |block_permissions| here might be
the result of the routines above.

+    APIPermissionSet permissions_parsed;
+    if (!APIPermissionSet::ParseFromJSON(
+            list_value,
+            APIPermissionSet::kDisallowInternalPermissions,
+            &permissions_parsed,
+            &error,
+            NULL)) {
+      LOG(WARNING) << error;
+    }
+    APIPermissionSet::Union(
+        permissions_to_merge_from, permissions_parsed, &blocked_permissions);
+  }
+
   return true;
 }
 
 void IndividualSettings::Reset() {
   installation_mode = ExtensionManagement::INSTALLATION_ALLOWED;
   update_url.clear();
+  blocked_permissions.clear();
 }
 
 GlobalSettings::GlobalSettings() {
   Reset();
 }
 
 GlobalSettings::~GlobalSettings() {
 }
 
 void GlobalSettings::Reset() {
   has_restricted_install_sources = false;
   install_sources.ClearPatterns();
   has_restricted_allowed_types = false;
   allowed_types.clear();
 }
 
 }  // namespace internal
 
 }  // namespace extensions