[DurableStorage] Don't grant durable if origin cannot write cookies.

chrome/browser/storage/durable_storage_permission_context.cc

 // Copyright 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.
 
 #include "chrome/browser/storage/durable_storage_permission_context.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/browser/website_settings_registry.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/common/origin_util.h"
 #include "url/gurl.h"
 
 using bookmarks::BookmarkModel;
 
 DurableStoragePermissionContext::DurableStoragePermissionContext(
     Profile* profile)
     : PermissionContextBase(profile,
                             content::PermissionType::DURABLE_STORAGE,
                             CONTENT_SETTINGS_TYPE_DURABLE_STORAGE) {}
 
 void DurableStoragePermissionContext::DecidePermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

[michaeln, 2016/10/05 00:22:40]

A comment about how this is only expected to be called when the permission has
not yet be granted might be good. Otherwise it looks possible for a top level
frame and a nested cross origin iframe to get different answers for the same
origin and the same time, which wouldn't make sense.

Better yet, DCHECKs would speak even louder than a comment.

DCHECK_NEQ(ALLOWED, GetPermissionStatus(requesting_origin, embedding_origin));
DCHECK_NEQ(BLOCKED, GetPermissionStatus(requesting_origin, embedding_origin));

[dmurph, 2016/10/05 03:22:36]

Done.

 
-  // TODO(dgrogan): Remove bookmarks check in favor of site engagement. In the
-  // meantime maybe grant permission to A2HS origins as well.
+  // Don't grant durable if we're off the record.
+  if (profile()->IsOffTheRecord()) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);
+    return;
+  }
+
+  // Durable is only allowed to be granted on top-level origins.
+  if (requesting_origin != embedding_origin) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);
+    return;
+  }
+
+  // Don't grant durable if we can't write cookies.
+  scoped_refptr<content_settings::CookieSettings> cookie_settings =
+      CookieSettingsFactory::GetForProfile(profile());
+  if (!cookie_settings->IsSettingCookieAllowed(requesting_origin,
+                                               requesting_origin)) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_DEFAULT);

[michaeln, 2016/10/05 00:22:40]

Yup, DEFAULT seems right here, as a sticky BLOCK setting would otherwise prevent
future cookie setting changes from allowing durability.

[dmurph, 2016/10/05 03:22:36]

Acknowledged.

+    return;
+  }
+
+  // TODO(dmurph): Remove bookmarks check in favor of important sites.
   BookmarkModel* model =
       BookmarkModelFactory::GetForBrowserContextIfExists(profile());
   if (model) {
     std::vector<bookmarks::BookmarkModel::URLAndTitle> bookmarks;
     model->GetBookmarks(&bookmarks);
     if (IsOriginBookmarked(bookmarks, requesting_origin)) {
       NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
                           true /* persist */, CONTENT_SETTING_ALLOW);

[michaeln, 2016/10/05 00:22:40]

Q: Not about this CL specifically, but does unbookmarking revoke the permission?
Looks like ALLOW is persisted which should prevent future DecidePermission()
invocations?

Ultimately, the system will treat it as durable until this content setting value
is modified.

bool CookieSettings::IsStorageDurable(const GURL& origin) const {
  // TODO(dgrogan): Don't use host_content_settings_map_ directly.
  // https://crbug.com/539538
  ContentSetting setting = host_content_settings_map_->GetContentSetting(
      origin /*primary*/, origin /*secondary*/,
      CONTENT_SETTINGS_TYPE_DURABLE_STORAGE,
      std::string() /*resource_identifier*/);
  return setting == CONTENT_SETTING_ALLOW;
}

[dmurph, 2016/10/05 03:22:36]

Nope, we currently give it forever, until the user clears browsing data. We'll
have to solve that in a future patch. I put that bug on my shortlist.

       return;
     }
   }
 
   NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
                       false /* persist */, CONTENT_SETTING_DEFAULT);
 }
 
 void DurableStoragePermissionContext::UpdateContentSetting(
     const GURL& requesting_origin,
@@ skipping 18 matching lines @@
     const std::vector<bookmarks::BookmarkModel::URLAndTitle>& bookmarks,
     const GURL& origin) {
   BookmarkModel::URLAndTitle looking_for;
   looking_for.url = origin;
   return std::binary_search(bookmarks.begin(), bookmarks.end(), looking_for,
                             [](const BookmarkModel::URLAndTitle& a,
                                const BookmarkModel::URLAndTitle& b) {
                               return a.url.GetOrigin() < b.url.GetOrigin();
                             });
 }