Restrict notification permission requests to top-level secure contexts

Requesting notification permission could previously happen from any
context, which included iframes and insecure origins. Starting with
Chrome 61 we're restricting this to top-level secure contexts.

Usage will continue to be allowed in iframes once permission has been
granted from a top-level frame. Origins could easily work around such a
restriction by posting a message to their Service Worker, so it doesn't
make sense to impose it.

BUG=695693
TBR=stevenjb (http -> https in site settings unit test)

Change-Id: I33fe4278ed3ecd388872c74e6f299589855ea835
Reviewed-on: https://chromium-review.googlesource.com/558095Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487935}

chrome/browser/budget_service/budget_service_impl.cc

@@ -63,8 +63,8 @@ void BudgetServiceImpl::GetBudget(const url::Origin& origin,
   // something the impact of which has to be reconsidered when the feature is
   // ready to ship for real. See https://crbug.com/710809 for context.
   if (permission_manager->GetPermissionStatus(
-          content::PermissionType::NOTIFICATIONS, origin.GetURL(), GURL()) !=
-      blink::mojom::PermissionStatus::GRANTED) {
+          content::PermissionType::NOTIFICATIONS, origin.GetURL(),
+          origin.GetURL()) != blink::mojom::PermissionStatus::GRANTED) {
     blink::mojom::BudgetStatePtr empty_state(blink::mojom::BudgetState::New());
     empty_state->budget_at = 0;
     empty_state->time =

chrome/browser/notifications/notification_permission_context.cc

@@ -169,20 +169,6 @@ NotificationPermissionContext::NotificationPermissionContext(
 
 NotificationPermissionContext::~NotificationPermissionContext() {}
 
-ContentSetting NotificationPermissionContext::GetPermissionStatusInternal(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& requesting_origin,
-    const GURL& embedding_origin) const {
-  // Push messaging is only allowed to be granted on top-level origins.
-  if (content_settings_type() == CONTENT_SETTINGS_TYPE_PUSH_MESSAGING
-          && requesting_origin != embedding_origin) {
-    return CONTENT_SETTING_BLOCK;
-  }
-
-  return PermissionContextBase::GetPermissionStatusInternal(
-      render_frame_host, requesting_origin, embedding_origin);
-}
-
 void NotificationPermissionContext::ResetPermission(
     const GURL& requesting_origin,
     const GURL& embedder_origin) {
@@ -208,6 +194,15 @@ void NotificationPermissionContext::DecidePermission(
     const BrowserPermissionCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  // Permission requests for either Web Notifications and Push Notifications may
+  // only happen on top-level frames. Usage will continue to be allowed in
+  // iframes: such frames could trivially work around the restriction by posting
+  // a message to their Service Worker, where showing a notification is allowed.
+  if (requesting_origin != embedding_origin) {
+    callback.Run(CONTENT_SETTING_BLOCK);
+    return;
+  }
+
   // Notifications permission is always denied in incognito. To prevent sites
   // from using that to detect whether incognito mode is active, we deny after a
   // random time delay, to simulate a user clicking a bubble/infobar. See also
@@ -257,5 +252,5 @@ void NotificationPermissionContext::UpdateContentSetting(
 }
 
 bool NotificationPermissionContext::IsRestrictedToSecureOrigins() const {
-  return content_settings_type() == CONTENT_SETTINGS_TYPE_PUSH_MESSAGING;
+  return true;
 }

chrome/browser/notifications/notification_permission_context.h

@@ -18,16 +18,14 @@ class NotificationPermissionContext : public PermissionContextBase {
   ~NotificationPermissionContext() override;
 
   // PermissionContextBase implementation.
-  ContentSetting GetPermissionStatusInternal(
-      content::RenderFrameHost* render_frame_host,
-      const GURL& requesting_origin,
-      const GURL& embedding_origin) const override;
   void ResetPermission(const GURL& requesting_origin,
                        const GURL& embedder_origin) override;
   void CancelPermissionRequest(content::WebContents* web_contents,
                                const PermissionRequestID& id) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(NotificationPermissionContextTest,
+                           PushTopLevelOriginOnly);
   friend class NotificationPermissionContextTest;
 
   // PermissionContextBase implementation.

chrome/browser/notifications/notification_permission_context_unittest.cc

@@ -29,6 +29,12 @@ namespace {
 
 void DoNothing(ContentSetting content_setting) {}
 
+void StoreContentSetting(ContentSetting* out_content_setting,
+                         ContentSetting content_setting) {
+  DCHECK(out_content_setting);
+  *out_content_setting = content_setting;
+}
+
 class TestNotificationPermissionContext : public NotificationPermissionContext {
  public:
   explicit TestNotificationPermissionContext(Profile* profile)
@@ -144,99 +150,77 @@ TEST_F(NotificationPermissionContextTest, IgnoresEmbedderOrigin) {
 }
 
 // Push messaging permission requests should only succeed for top level origins
-// (embedding origin == requesting origin).
+// (embedding origin == requesting origin). Retrieving previously granted
+// permissions should continue to be possible regardless of being top level.
 TEST_F(NotificationPermissionContextTest, PushTopLevelOriginOnly) {
   GURL requesting_origin("https://example.com");
   GURL embedding_origin("https://chrome.com");
 
   NotificationPermissionContext context(profile(),
                                         CONTENT_SETTINGS_TYPE_PUSH_MESSAGING);
-  UpdateContentSetting(&context, requesting_origin, embedding_origin,
-                       CONTENT_SETTING_ALLOW);
 
-  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+  EXPECT_EQ(CONTENT_SETTING_ASK,
             context
                 .GetPermissionStatus(nullptr /* render_frame_host */,
                                      requesting_origin, embedding_origin)
                 .content_setting);
 
-  context.ResetPermission(requesting_origin, embedding_origin);
+  // Requesting permission for different origins should fail.
+  PermissionRequestID fake_id(0 /* render_process_id */,
+                              0 /* render_frame_id */, 0 /* request_id */);
 
-  UpdateContentSetting(&context, embedding_origin, embedding_origin,
-                       CONTENT_SETTING_ALLOW);
+  ContentSetting result = CONTENT_SETTING_DEFAULT;
+  context.DecidePermission(web_contents(), fake_id, requesting_origin,
+                           embedding_origin, true /* user_gesture */,
+                           base::Bind(&StoreContentSetting, &result));
 
-  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+  ASSERT_EQ(result, CONTENT_SETTING_BLOCK);
+  EXPECT_EQ(CONTENT_SETTING_ASK,
             context
                 .GetPermissionStatus(nullptr /* render_frame_host */,
-                                     embedding_origin, embedding_origin)
+                                     requesting_origin, embedding_origin)
                 .content_setting);
 
-  context.ResetPermission(embedding_origin, embedding_origin);
+  // Reading previously set permissions should continue to work.
+  UpdateContentSetting(&context, requesting_origin, embedding_origin,
+                       CONTENT_SETTING_ALLOW);
 
-  EXPECT_EQ(CONTENT_SETTING_ASK,
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
             context
                 .GetPermissionStatus(nullptr /* render_frame_host */,
-                                     embedding_origin, embedding_origin)
+                                     requesting_origin, embedding_origin)
                 .content_setting);
-}
 
-// Web Notifications do not require a secure origin when requesting permission.
-// See https://crbug.com/404095.
-TEST_F(NotificationPermissionContextTest, NoSecureOriginRequirement) {
-  GURL origin("http://example.com");
-
-  NotificationPermissionContext context(profile(),
-                                        CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
-  EXPECT_EQ(
-      CONTENT_SETTING_ASK,
-      context
-          .GetPermissionStatus(nullptr /* render_frame_host */, origin, origin)
-          .content_setting);
-
-  UpdateContentSetting(&context, origin, origin, CONTENT_SETTING_ALLOW);
-
-  EXPECT_EQ(
-      CONTENT_SETTING_ALLOW,
-      context
-          .GetPermissionStatus(nullptr /* render_frame_host */, origin, origin)
-          .content_setting);
+  context.ResetPermission(requesting_origin, embedding_origin);
 }
 
-// Push notifications requires a secure origin to acquire permission.
-TEST_F(NotificationPermissionContextTest, PushSecureOriginRequirement) {
-  GURL origin("http://example.com");
-  GURL secure_origin("https://example.com");
-
-  NotificationPermissionContext context(
-      profile(), CONTENT_SETTINGS_TYPE_PUSH_MESSAGING);
-  EXPECT_EQ(
-      CONTENT_SETTING_BLOCK,
-      context
-          .GetPermissionStatus(nullptr /* render_frame_host */, origin, origin)
-          .content_setting);
+// Both Web Notifications and Push Notifications require secure origins.
+TEST_F(NotificationPermissionContextTest, SecureOriginRequirement) {
+  GURL insecure_origin("http://example.com");
 
-  UpdateContentSetting(&context, origin, origin, CONTENT_SETTING_ALLOW);
+  // Web Notifications
+  {
+    NotificationPermissionContext web_notification_context(
+        profile(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
 
-  EXPECT_EQ(
-      CONTENT_SETTING_BLOCK,
-      context
-          .GetPermissionStatus(nullptr /* render_frame_host */, origin, origin)
-          .content_setting);
-
-  EXPECT_EQ(CONTENT_SETTING_ASK,
-            context
-                .GetPermissionStatus(nullptr /* render_frame_host */,
-                                     secure_origin, secure_origin)
-                .content_setting);
+    EXPECT_EQ(CONTENT_SETTING_BLOCK,
+              web_notification_context
+                  .GetPermissionStatus(nullptr /* render_frame_host */,
+                                       insecure_origin, insecure_origin)
+                  .content_setting);
+  }
 
-  UpdateContentSetting(&context, secure_origin, secure_origin,
-                       CONTENT_SETTING_ALLOW);
+  // Push Notifications
+  {
+    NotificationPermissionContext push_notification_context(
+        profile(), CONTENT_SETTINGS_TYPE_PUSH_MESSAGING);
 
-  EXPECT_EQ(CONTENT_SETTING_ALLOW,
-            context
-                .GetPermissionStatus(nullptr /* render_frame_host */,
-                                     secure_origin, secure_origin)
-                .content_setting);
+    EXPECT_EQ(CONTENT_SETTING_BLOCK,
+              push_notification_context
+                  .GetPermissionStatus(nullptr /* render_frame_host */,
+                                       insecure_origin, insecure_origin)
+                  .content_setting);
+  }
 }
 
 // Tests auto-denial after a time delay in incognito.

chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc

@@ -254,7 +254,7 @@ TEST_F(SiteSettingsHandlerTest, GetAndSetDefault) {
 TEST_F(SiteSettingsHandlerTest, Origins) {
   // Test the JS -> C++ -> JS callback path for configuring origins, by setting
   // Google.com to blocked.
-  const std::string google("http://www.google.com");
+  const std::string google("https://www.google.com");
   const std::string kUmaBase("WebsiteSettings.Menu.PermissionChanged");
   {
     base::ListValue set_args;

third_party/WebKit/Source/core/frame/Deprecation.cpp

@@ -396,20 +396,17 @@ String Deprecation::DeprecationMessage(WebFeature feature) {
     case WebFeature::kNotificationAPIInsecureOriginIframe:
     case WebFeature::kNotificationPermissionRequestedInsecureOrigin:
       return String::Format(
-          "Using the Notification API on insecure origins is "
-          "deprecated and will be removed in %s. You should consider "
-          "switching your application to a secure origin, such as HTTPS. See "
-          "https://goo.gl/rStTGz for more details.",
-          milestoneString(M61));
+          "The Notification API may no longer be used from insecure origins. "
+          "You should consider switching your application to a secure origin, "
+          "such as HTTPS. See https://goo.gl/rStTGz for more details.");
 
     case WebFeature::kNotificationPermissionRequestedIframe:
       return String::Format(
-          "Using the Notification API from an iframe is deprecated and will "
-          "be removed in %s. You should consider requesting permission from "
-          "the top-level frame or opening a new window instead. See "
+          "Permission for the Notification API may no longer be requested from "
+          "an iframe. You should consider requesting permission from the "
+          "top-level frame or opening a new window instead. See "
           "https://www.chromestatus.com/feature/6451284559265792 for more "
-          "details.",
-          milestoneString(M61));
+          "details.");
 
     case WebFeature::kElementCreateShadowRootMultiple:
       return "Calling Element.createShadowRoot() for an element which already "

third_party/WebKit/Source/modules/notifications/Notification.cpp

@@ -48,6 +48,7 @@
 #include "modules/notifications/NotificationData.h"
 #include "modules/notifications/NotificationManager.h"
 #include "modules/notifications/NotificationOptions.h"
+#include "modules/notifications/NotificationPermissionCallback.h"
 #include "modules/notifications/NotificationResourcesLoader.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/bindings/ScriptState.h"
@@ -155,6 +156,11 @@ void Notification::SchedulePrepareShow() {
 
 void Notification::PrepareShow() {
   DCHECK_EQ(state_, State::kLoading);
+  if (!GetExecutionContext()->IsSecureContext()) {
+    DispatchErrorEvent();
+    return;
+  }
+
   if (NotificationManager::From(GetExecutionContext())
           ->GetPermissionStatus(GetExecutionContext()) !=
       mojom::blink::PermissionStatus::GRANTED) {
@@ -357,17 +363,33 @@ String Notification::PermissionString(
 
 String Notification::permission(ScriptState* script_state) {
   ExecutionContext* context = ExecutionContext::From(script_state);
-  return PermissionString(
-      NotificationManager::From(context)->GetPermissionStatus(context));
+  mojom::blink::PermissionStatus status =
+      mojom::blink::PermissionStatus::DENIED;
+
+  if (context->IsSecureContext())
+    status = NotificationManager::From(context)->GetPermissionStatus(context);
+
+  return PermissionString(status);
 }
 
 ScriptPromise Notification::requestPermission(
     ScriptState* script_state,
     NotificationPermissionCallback* deprecated_callback) {
   ExecutionContext* context = ExecutionContext::From(script_state);
+
+  // Sites no longer have the ability to request notification permission from
+  // insecure contexts. Inform the developer.
   if (!context->IsSecureContext()) {
     Deprecation::CountDeprecation(
         context, WebFeature::kNotificationPermissionRequestedInsecureOrigin);
+
+    String status_denied =
+        Notification::PermissionString(mojom::blink::PermissionStatus::DENIED);
+
+    if (deprecated_callback)
+      deprecated_callback->handleEvent(status_denied);
+
+    return ScriptPromise::Cast(script_state, ToV8(status_denied, script_state));
   }
 
   if (context->IsDocument()) {