PS - Adjusting webRequest API for use in Public Sessions

extensions/browser/api/web_request/web_request_api.cc

Index: extensions/browser/api/web_request/web_request_api.cc
diff --git a/extensions/browser/api/web_request/web_request_api.cc b/extensions/browser/api/web_request/web_request_api.cc
index 829ebb512eb72d044537eb4afa30a5f800fcf704..172ace20ef103c54432e9197de29cf690213a2b3 100644
--- a/extensions/browser/api/web_request/web_request_api.cc
+++ b/extensions/browser/api/web_request/web_request_api.cc
@@ -22,6 +22,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "chromeos/login/login_state.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/user_metrics.h"
@@ -338,6 +339,16 @@ bool ShouldHideEvent(void* browser_context,
                                              navigation_ui_data));
 }
 
+// Returns true if we're in a Public Session.
+bool IsPublicSession() {
+#if defined(OS_CHROMEOS)
+  if (chromeos::LoginState::IsInitialized()) {
+    return chromeos::LoginState::Get()->IsPublicSessionUser();
+  }
+#endif
+  return false;
+}
+
 }  // namespace
 
 WebRequestAPI::WebRequestAPI(content::BrowserContext* context)
@@ -1440,6 +1451,17 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
               extension_info_map, listener->id.extension_id, url,
               frame_data.tab_id, crosses_incognito,
               WebRequestPermissions::REQUIRE_HOST_PERMISSION);
+
+      // When we are in a Public Session, allow all URL's for webRequests
+      // initiated by a regular extension.
+      if (access != PermissionsData::ACCESS_ALLOWED && extension_info_map) {
+        const extensions::Extension* extension =
+            extension_info_map->extensions().GetByID(listener->id.extension_id);
+        if (IsPublicSession() && extension && extension->is_extension()) {

[Devlin, 2016/11/03 00:33:46]

We want to do this for *all* extensions?  Is it always safe to assume that an
extension in a public session was installed by the admin?

Also, this will allow even restricted urls, like chrome:// urls.  I get the
feeling we don't want to allow that.

[Ivan Šandrk, 2016/11/03 14:45:12]

Yes, for all regular extensions. Yes, users aren't allowed to install
extensions, there is a check in place for this -
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/extensions/devic...
(EXTERNAL_POLICY means that the extension is force installed by policy).

That's a good point about restricted urls. I've got a simple test extension for
testing this stuff out, I couldn't register a chrome:// url. Following the error
message I ended up here
https://cs.chromium.org/chromium/src/extensions/browser/api/web_request/web_r...

Http/https/ftp links are fine, chrome-extension:// links should also be fine
(all extensions are admin installed), and I think file:// links should also be
fine. I don't think people use the browser to browse their local files +
everything is stripped out; for example file:///boot/abi-3.13.0-100-generic is
shown only as file:/// (URLs are stripped down to origin). I could limit the
urls only to http?/ftp, but I think that's just adding unnecessary complexity.
WDYT?

[Devlin, 2016/11/05 06:04:37]

If security/privacy are on board with this, then that's fine.  I was mostly
worried about chrome:// urls.

If all extensions must be installed by policy, can we add a
CHECK(Manifest::IsPolicyLocation(extension->location())); here?  That would make
me feel a bit better.

[Ivan Šandrk, 2016/11/07 10:50:02]

S&P are fine.

Done.

+          access = PermissionsData::ACCESS_ALLOWED;
+        }
+      }
+
       if (access != PermissionsData::ACCESS_ALLOWED) {
         if (access == PermissionsData::ACCESS_WITHHELD &&
             web_request_event_router_delegate_) {
@@ -2127,7 +2149,10 @@ WebRequestInternalAddEventListenerFunction::Run() {
     // http://www.example.com/bar/*.
     // For this reason we do only a coarse check here to warn the extension
     // developer if they do something obviously wrong.
-    if (extension->permissions_data()
+    // When we are in a Public Session, allow all URL's for webRequests
+    // initiated by a regular extension.
+    if (!(IsPublicSession() && extension->is_extension()) &&
+        extension->permissions_data()
             ->GetEffectiveHostPermissions()
             .is_empty() &&
         extension->permissions_data()