Create withheld permissions

extensions/common/permissions/permissions_data.h

 // Copyright (c) 2013 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.
 
 #ifndef EXTENSIONS_COMMON_PERMISSIONS_PERMISSIONS_DATA_H_
 #define EXTENSIONS_COMMON_PERMISSIONS_PERMISSIONS_DATA_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 17 matching lines @@
 class URLPatternSet;
 class UserScript;
 
 // A container for the active permissions of an extension.
 // TODO(rdevlin.cronin): For the love of everything good, rename this class to
 // ActivePermissions. We do *not* need PermissionsParser, PermissionSet,
 // PermissionInfo, and PermissionsData. No one will be able to keep them
 // straight.
 class PermissionsData {
  public:
+  // The possible types of access for a given frame.
+  enum AccessType {
+    DENY_ACCESS,    // The extension is not allowed to access the given page.
+    ALLOW_ACCESS,   // The extension is allowed to access the given page.
+    REQUEST_ACCESS  // The browser must determine if the extension can access

[not at google - send to devlin, 2014/06/27 23:24:34]

the concept of "request access" isn't really the concern of permissions data
(and this goes for the methods like CanRunCOntentScriptWIthUserConsent). it
should just deal with granted vs withheld permissions. like the enum should be
"yes", "no", or "no but it's been withheld". or maybe more mathematically,
perhaps:

enum AccessType {
  kAccessAllowed,
  kAccessDenied,
  kAccessWithheld
};

and then let callers interpret that.

[Devlin, 2014/06/30 17:06:10]

Done (though sadly we use SCREAMING_STYLE instead of kConstantStyle for enums
:()

+                    // the given page.
+  };
+
   // Delegate class to allow different contexts (e.g. browser vs renderer) to
   // have control over policy decisions.
   class PolicyDelegate {
    public:
     virtual ~PolicyDelegate() {}
 
     // Returns false if script access should be blocked on this page.
     // Otherwise, default policy should decide.
     virtual bool CanExecuteScriptOnPage(const Extension* extension,
                                         const GURL& document_url,
@@ skipping 23 matching lines @@
   // Returns true if the given |url| is restricted for the given |extension|,
   // as is commonly the case for chrome:// urls.
   // NOTE: You probably want to use CanAccessPage().
   static bool IsRestrictedUrl(const GURL& document_url,
                               const GURL& top_frame_url,
                               const Extension* extension,
                               std::string* error);
 
   // Sets the runtime permissions of the given |extension| to |permissions|.
   void SetActivePermissions(const PermissionSet* active) const;
+  // Initializes the withheld/active permissions from |permissions|.

[not at google - send to devlin, 2014/06/27 23:24:34]

blank line above here

[Devlin, 2014/06/30 17:06:10]

Done.

+  void InitializePermissions(const PermissionSet* permissions,

[not at google - send to devlin, 2014/06/27 23:24:34]

it's odd to call this InitializePermissions when it's not actually being used on
initialization (PermissionsUpdater uses it). ideally you'd just not expose
anything like this and have the |permissions_data| of Extension replaced by a
new permission set. in lieu of that... call this something else.
ImportFromExtension maybe. Reset. along those lines.

[Devlin, 2014/06/30 17:06:10]

I don't feel really strongly about the name on this... but it's called from
PermissionsUpdater::InitializeActivePermissions(), which is called when an
extension's permissions are initialized... so isn't the name fairly accurate? 
It's used for initialization, and should be used for initialization _only_
(since it does all the withholding stuff).

[Devlin, 2014/06/30 20:28:58]

Per offline chat, this has basically been moved to PermissionsUpdater.

+                             const Extension* extension) const;
+
+  // Sets the runtime permissions of the given |extension| to |active| and
+  // |withheld|.
+  // This should only be used as a means of "copying" permissions data, e.g.
+  // to duplicate the information in the renderer.

[not at google - send to devlin, 2014/06/27 23:24:34]

I don't see this method being used that way, like it's used in
PermissionsUpdater::SetActivePermissions.

I find the name a bit confusing compared to InitializePermissions.

could both of these be replaced by a set_permissions_data method on Extension?

[Devlin, 2014/06/30 17:06:10]

You're right - I didn't update the comment after the changes to
PermissionsUpdater.  Done.
And we couldn't really use a set_permissions_data()... see below on immutable
permissions datas.

+  void SetPermissions(const PermissionSet* active,
+                      const PermissionSet* withheld) const;
 
   // Updates the tab-specific permissions of |tab_id| to include those from
   // |permissions|.
   void UpdateTabSpecificPermissions(
       int tab_id,
       scoped_refptr<const PermissionSet> permissions) const;
 
   // Clears the tab-specific permissions of |tab_id|.
   void ClearTabSpecificPermissions(int tab_id) const;
 
@@ skipping 34 matching lines @@
   PermissionMessages GetPermissionMessages() const;
 
   // Returns the full list of permission messages that should display at install
   // time as strings.
   std::vector<base::string16> GetPermissionMessageStrings() const;
 
   // Returns the full list of permission details for messages that should
   // display at install time as strings.
   std::vector<base::string16> GetPermissionMessageDetailsStrings() const;
 
+  // The following three functions pertain to (possibly withheld) all-hosts
+  // permissions. Since all-hosts can take many different forms, we have
+  // these functions for them. Any additional withheld permissions should
+  // simply be checked, granted, and withheld via a generic
+  // HasWithheldPermission(), GrantWithheldPermission(), and
+  // WithholdPermission().
+
+  // Returns true if the extension has requested all-hosts permissions (or
+  // something close to it), but has had it withheld.
+  bool HasWithheldAllHosts() const;

[not at google - send to devlin, 2014/06/30 14:37:03]

Why specifically all hosts? The way I was imagining this was more abstract.

- See comment in ActiveTabPermissionGranter for how that could be implemented
without using this.

- The logic that deals with actually withholding permissions (initialization,
preferences) can just rebuild the permissions data each time based on the state
of the world. That is, rather than the permissions logic being

    if (allowed)
      extension->permissions_data()->GrantWithheldAllHosts();
    else
      extension->permissions_data()->WithholdAllHosts();

it could be

    // Putting this on Extension seems like the easiest way to share state,
    // particularly since this will need to work on the renderer eventually,
    // not to mention on startup.
    extension->SetWithholdingPermissions(allowed);
    // and I mentioned elsewhere that I'd rather PermissionsData be
    // immutable and Extension just rebuild it itself, if possible.
    extension->permissions_data()->Rebuild();

[Devlin, 2014/06/30 17:06:10]

See comment in ActiveTabPermissionGranter for why we need this. ;)

I think there are a few things wrong with this.
- We've deliberately tried to _remove_ permissions logic from the Extension
class.
- Simply saying "set withholding permissions" isn't scalable.  We don't want to
have to choose between "withhold none" and "withhold all withholdable".
- PermissionsData is by nature not immutable.  It'd be weird enough having to
rebuild it each time we grant an optional permission, but having to rebuild it
for every active tab?

[not at google - send to devlin, 2014/07/01 00:28:35]

good points. moving this to PermissionsUpdater has made this more or less moot,
right?

[Devlin, 2014/07/01 16:27:05]

Yep.

+
+  // Grants any withheld all-hosts (or all-hosts-like) permissions.
+  void GrantWithheldAllHosts() const;
+
+  // Revokes any requests all-hosts (or all-hosts-like) permissions.
+  void WithholdAllHosts() const;
+
   // Returns true if the |extension| has permission to access and interact with
   // the specified page, in order to do things like inject scripts or modify
   // the content.
   // If this returns false and |error| is non-NULL, |error| will be popualted
   // with the reason the extension cannot access the page.
   bool CanAccessPage(const Extension* extension,
                      const GURL& document_url,
                      const GURL& top_document_url,
                      int tab_id,
                      int process_id,
                      std::string* error) const;
+  // Like CanAccessPage, but also takes withheld permissions into account.
+  // TODO(rdevlin.cronin) We shouldn't have two functions, but not all callers
+  // know how to wait for permission.
+  AccessType CanAccessPageWithUserConsent(const Extension* extension,

[not at google - send to devlin, 2014/06/27 23:24:34]

in the spirit of the comment at the top of the enum, this name shouldn't imply a
boolean since it's no longer a boolean. instead GetPageAccess.

[Devlin, 2014/06/30 17:06:10]

Shorter to write, yay.

+                                          const GURL& document_url,
+                                          const GURL& top_document_url,
+                                          int tab_id,
+                                          int process_id,
+                                          std::string* error) const;
 
   // Returns true if the |extension| has permission to inject a content script
   // on the page.
   // If this returns false and |error| is non-NULL, |error| will be popualted
   // with the reason the extension cannot script the page.
   // NOTE: You almost certainly want to use CanAccessPage() instead of this
   // method.
   bool CanRunContentScriptOnPage(const Extension* extension,
                                  const GURL& document_url,
                                  const GURL& top_document_url,
                                  int tab_id,
                                  int process_id,
                                  std::string* error) const;
+  // Like CanRunContentScriptOnPage, but also takes withheld permissions into
+  // account.
+  // TODO(rdevlin.cronin) We shouldn't have two functions, but not all callers
+  // know how to wait for permission.
+  AccessType CanRunContentScriptOnPageWithUserConsent(

[not at google - send to devlin, 2014/06/27 23:24:34]

likewise, GetContentScriptAccess

[Devlin, 2014/06/30 17:06:10]

Done.

+      const Extension* extension,
+      const GURL& document_url,
+      const GURL& top_document_url,
+      int tab_id,
+      int process_id,
+      std::string* error) const;
 
   // Returns true if extension is allowed to obtain the contents of a page as
   // an image. Since a page may contain sensitive information, this is
   // restricted to the extension's host permissions as well as the extension
   // page itself.
   bool CanCaptureVisiblePage(int tab_id, std::string* error) const;
 
-  // Returns true if the user should be alerted that the |extension| is running
-  // a script. If |tab_id| and |url| are included, this also considers tab-
-  // specific permissions.
-  bool RequiresActionForScriptExecution(const Extension* extension) const;
-  bool RequiresActionForScriptExecution(const Extension* extension,
-                                        int tab_id,
-                                        const GURL& url) const;
-
   scoped_refptr<const PermissionSet> active_permissions() const {
     base::AutoLock auto_lock(runtime_lock_);
     return active_permissions_unsafe_;
   }
 
+  scoped_refptr<const PermissionSet> withheld_permissions() const {
+    base::AutoLock auto_lock(runtime_lock_);
+    return withheld_permissions_unsafe_;
+  }
+
 #if defined(UNIT_TEST)
   scoped_refptr<const PermissionSet> GetTabSpecificPermissionsForTesting(
       int tab_id) const {
     return GetTabSpecificPermissions(tab_id);
   }
 #endif
 
  private:
   typedef std::map<int, scoped_refptr<const PermissionSet> > TabPermissionsMap;
 
   // Gets the tab-specific host permissions of |tab_id|, or NULL if there
   // aren't any.
   scoped_refptr<const PermissionSet> GetTabSpecificPermissions(
       int tab_id) const;
 
   // Returns true if the |extension| has tab-specific permission to operate on
   // the tab specified by |tab_id| with the given |url|.
   // Note that if this returns false, it doesn't mean the extension can't run on
   // the given tab, only that it does not have tab-specific permission to do so.
   bool HasTabSpecificPermissionToExecuteScript(int tab_id,
                                                const GURL& url) const;
 
-  // Returns true if the extension is permitted to run on the given page,
+  // Returns whether or not the extension is permitted to run on the given page,
   // checking against |permitted_url_patterns| in addition to blocking special
   // sites (like the webstore or chrome:// urls).
-  bool CanRunOnPage(const Extension* extension,
-                    const GURL& document_url,
-                    const GURL& top_document_url,
-                    int tab_id,
-                    int process_id,
-                    const URLPatternSet& permitted_url_patterns,
-                    std::string* error) const;
+  AccessType CanRunOnPage(const Extension* extension,
+                          const GURL& document_url,
+                          const GURL& top_document_url,
+                          int tab_id,
+                          int process_id,
+                          const URLPatternSet& permitted_url_patterns,
+                          const URLPatternSet& withheld_url_patterns,
+                          std::string* error) const;
 
   // The associated extension's id.
   std::string extension_id_;
 
   // The associated extension's manifest type.
   Manifest::Type manifest_type_;
 
   mutable base::Lock runtime_lock_;
 
   // The permission's which are currently active on the extension during
   // runtime.
   // Unsafe indicates that we must lock anytime this is directly accessed.
   // Unless you need to change |active_permissions_unsafe_|, use the (safe)
   // active_permissions() accessor.
   mutable scoped_refptr<const PermissionSet> active_permissions_unsafe_;
 
+  // The permissions the extension requested, but was not granted due because
+  // they are too powerful. This includes things like all_hosts.
+  // Unsafe indicates that we must lock anytime this is directly accessed.
+  // Unless you need to change |withheld_permissions_unsafe_|, use the (safe)
+  // withheld_permissions() accessor.
+  mutable scoped_refptr<const PermissionSet> withheld_permissions_unsafe_;
+
   mutable TabPermissionsMap tab_specific_permissions_;
 
   DISALLOW_COPY_AND_ASSIGN(PermissionsData);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_COMMON_PERMISSIONS_PERMISSIONS_DATA_H_