[shared storage] implement permissions policy

Add the "shared-storage" permissions policy that disallows all Shared Storage methods. https://github.com/WICG/shared-storage/blob/main/README.md#permissions-policy Due to this change, Shared Storage won't be allowed in Fenced Frames as Fenced Frames disallow all permissions policies. This decision may change in the future: WICG/fenced-frame#44 Bug: 1337454 Change-Id: I856d31933032355409585bc376f2b6826f667270 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3710841 Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Commit-Queue: Yao Xiao <[email protected]> Reviewed-by: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1023892}
chromium · Jul 13, 2022 · 5187275 · 5187275
1 parent 8130feb
commit 5187275
Show file tree

Hide file tree

Showing 20 changed files with 328 additions and 43 deletions.
diff --git a/content/browser/shared_storage/shared_storage_browsertest.cc b/content/browser/shared_storage/shared_storage_browsertest.cc
@@ -1746,8 +1746,11 @@ IN_PROC_BROWSER_TEST_P(SharedStorageFencedFrameInteractionBrowserTest,
       fenced_frame_root_node->current_frame_host()->GetLastCommittedURL());
 }
 
+// Currently, Shared Storage is not allowed in Fenced Frames as Fenced Frames
+// disallow all permissions policies. This may change in the future.
+// https://github.com/WICG/fenced-frame/issues/44
 IN_PROC_BROWSER_TEST_P(SharedStorageFencedFrameInteractionBrowserTest,
-                       SelectURLNotAllowedInFencedFrame) {
+                       SharedStorageNotAllowedInFencedFrame) {
   GURL main_frame_url = https_server()->GetURL("a.test", kSimplePagePath);
 
   EXPECT_TRUE(NavigateToURL(shell(), main_frame_url));
@@ -1757,21 +1760,17 @@ IN_PROC_BROWSER_TEST_P(SharedStorageFencedFrameInteractionBrowserTest,
 
   FrameTreeNode* fenced_frame_node = CreateFencedFrame(fenced_frame_url);
 
-  EXPECT_TRUE(ExecJs(fenced_frame_node, R"(
-      sharedStorage.worklet.addModule('/shared_storage/simple_module.js');
-    )"));
-
-  EXPECT_EQ(1u, test_worklet_host_manager().GetAttachedWorkletHostsCount());
-  EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());
-
   EvalJsResult result = EvalJs(fenced_frame_node, R"(
-      sharedStorage.selectURL(
-          'test-url-selection-operation',
-          [{url: "title0.html"}], {data: {'mockResult': 0}});
+      sharedStorage.worklet.addModule('/shared_storage/simple_module.js');
     )");
 
-  EXPECT_TRUE(result.error.find("sharedStorage.selectURL() is not allowed in "
-                                "fenced frame") != std::string::npos);
+  EXPECT_THAT(
+      result.error,
+      testing::HasSubstr("The \"shared-storage\" Permissions Policy denied the "
+                         "method on window.sharedStorage."));
+
+  EXPECT_EQ(0u, test_worklet_host_manager().GetAttachedWorkletHostsCount());
+  EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());
 }
 
 IN_PROC_BROWSER_TEST_P(SharedStorageFencedFrameInteractionBrowserTest,

diff --git a/third_party/blink/public/devtools_protocol/browser_protocol.pdl b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
@@ -7109,6 +7109,7 @@ domain Page
       screen-wake-lock
       serial
       shared-autofill
+      shared-storage
       storage-access-api
       sync-xhr
       trust-token-redemption

diff --git a/third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom b/third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom
@@ -214,6 +214,10 @@ enum PermissionsPolicyFeature {
   // Controls use of FedCM API.
   kFederatedCredentials = 102,
 
+  // "shared-storage" permissions policy that controls the use of Shared Storage
+  // API.
+  kSharedStorage = 103,
+
   // Don't change assigned numbers of any item, and don't reuse removed slots.
   // Add new features at the end of the enum.
   // Also, run update_permissions_policy_enum.py in

diff --git a/third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5 b/third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
@@ -341,6 +341,12 @@
       permissions_policy_name: "shared-autofill",
       depends_on: ["SharedAutofill"],
     },
+    {
+      name: "SharedStorage",
+      permissions_policy_name: "shared-storage",
+      feature_default: "EnableForAll",
+      depends_on: ["SharedStorageAPI"],
+    },
     {
       name: "StorageAccessAPI",
       feature_default: "EnableForAll",

diff --git a/third_party/blink/renderer/modules/shared_storage/BUILD.gn b/third_party/blink/renderer/modules/shared_storage/BUILD.gn
@@ -10,6 +10,8 @@ blink_modules_sources("shared_storage") {
     "shared_storage.h",
     "shared_storage_worklet.cc",
     "shared_storage_worklet.h",
+    "util.cc",
+    "util.h",
     "window_shared_storage.cc",
     "window_shared_storage.h",
   ]

diff --git a/third_party/blink/renderer/modules/shared_storage/shared_storage.cc b/third_party/blink/renderer/modules/shared_storage/shared_storage.cc
@@ -30,6 +30,7 @@
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/modules/shared_storage/shared_storage_worklet.h"
+#include "third_party/blink/renderer/modules/shared_storage/util.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/heap/persistent.h"
@@ -150,16 +151,18 @@ ScriptPromise SharedStorage::set(ScriptState* script_state,
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   if (!IsValidSharedStorageKeyStringLength(key.length())) {
     resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
         script_state->GetIsolate(), DOMExceptionCode::kDataError,
@@ -192,16 +195,18 @@ ScriptPromise SharedStorage::append(ScriptState* script_state,
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   if (!IsValidSharedStorageKeyStringLength(key.length())) {
     resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
         script_state->GetIsolate(), DOMExceptionCode::kDataError,
@@ -231,16 +236,18 @@ ScriptPromise SharedStorage::Delete(ScriptState* script_state,
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   if (!IsValidSharedStorageKeyStringLength(key.length())) {
     resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
         script_state->GetIsolate(), DOMExceptionCode::kDataError,
@@ -261,16 +268,18 @@ ScriptPromise SharedStorage::clear(ScriptState* script_state,
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   GetSharedStorageDocumentService(execution_context)
       ->SharedStorageClear(WTF::Bind(&OnVoidOperationFinished,
                                      WrapPersistent(resolver),
@@ -298,11 +307,8 @@ ScriptPromise SharedStorage::selectURL(
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   LocalFrame* frame = To<LocalDOMWindow>(execution_context)->GetFrame();
   DCHECK(frame);
@@ -319,6 +325,16 @@ ScriptPromise SharedStorage::selectURL(
     return promise;
   }
 
+  // For `selectURL()` to succeed, it is currently enforced in the browser side
+  // that `addModule()` must be called beforehand that passed the early
+  // permission checks. Thus the permissions-policy check here isn't strictly
+  // needed. But here we still check the permissions-policy for consistency and
+  // consider this a higher priority error.
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   if (!IsValidSharedStorageURLsArrayLength(urls.size())) {
     resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
         script_state->GetIsolate(), DOMExceptionCode::kDataError,
@@ -460,11 +476,8 @@ ScriptPromise SharedStorage::run(
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   Vector<uint8_t> serialized_data;
   if (!Serialize(script_state, options, exception_state, serialized_data))
@@ -474,6 +487,11 @@ ScriptPromise SharedStorage::run(
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   GetSharedStorageDocumentService(execution_context)
       ->RunOperationOnWorklet(
           name, std::move(serialized_data),

diff --git a/third_party/blink/renderer/modules/shared_storage/shared_storage_worklet.cc b/third_party/blink/renderer/modules/shared_storage/shared_storage_worklet.cc
@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/modules/shared_storage/shared_storage.h"
+#include "third_party/blink/renderer/modules/shared_storage/util.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 
@@ -29,17 +30,19 @@ ScriptPromise SharedStorageWorklet::addModule(ScriptState* script_state,
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   CHECK(execution_context->IsWindow());
 
-  if (!script_state->ContextIsValid()) {
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
-                                      "A browsing context is required.");
+  if (!CheckBrowsingContextIsValid(*script_state, exception_state))
     return ScriptPromise();
-  }
 
   KURL script_source_url = execution_context->CompleteURL(module_url);
 
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
+  if (!CheckSharedStoragePermissionsPolicy(*script_state, *execution_context,
+                                           *resolver)) {
+    return promise;
+  }
+
   if (!script_source_url.IsValid()) {
     resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
         script_state->GetIsolate(), DOMExceptionCode::kDataError,

diff --git a/third_party/blink/renderer/modules/shared_storage/util.cc b/third_party/blink/renderer/modules/shared_storage/util.cc
@@ -0,0 +1,43 @@
+// Copyright 2022 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 "third_party/blink/renderer/modules/shared_storage/util.h"
+
+#include "third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom-blink.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
+#include "third_party/blink/renderer/platform/bindings/script_state.h"
+
+namespace blink {
+
+bool CheckBrowsingContextIsValid(ScriptState& script_state,
+                                 ExceptionState& exception_state) {
+  if (!script_state.ContextIsValid()) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidAccessError,
+                                      "A browsing context is required.");
+    return false;
+  }
+
+  return true;
+}
+
+bool CheckSharedStoragePermissionsPolicy(ScriptState& script_state,
+                                         ExecutionContext& execution_context,
+                                         ScriptPromiseResolver& resolver) {
+  if (!execution_context.IsFeatureEnabled(
+          mojom::blink::PermissionsPolicyFeature::kSharedStorage)) {
+    resolver.Reject(V8ThrowDOMException::CreateOrEmpty(
+        script_state.GetIsolate(), DOMExceptionCode::kInvalidAccessError,
+        "The \"shared-storage\" Permissions Policy denied the method on "
+        "window.sharedStorage."));
+
+    return false;
+  }
+
+  return true;
+}
+
+}  // namespace blink
diff --git a/third_party/blink/renderer/modules/shared_storage/util.h b/third_party/blink/renderer/modules/shared_storage/util.h
@@ -0,0 +1,28 @@
+// Copyright 2022 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 THIRD_PARTY_BLINK_RENDERER_MODULES_SHARED_STORAGE_UTIL_H_
+#define THIRD_PARTY_BLINK_RENDERER_MODULES_SHARED_STORAGE_UTIL_H_
+
+namespace blink {
+
+class ExecutionContext;
+class ExceptionState;
+class ScriptState;
+class ScriptPromiseResolver;
+
+// Return if there is a valid browsing context associated with `script_state`.
+// Throw an error via `exception_state` if invalid.
+bool CheckBrowsingContextIsValid(ScriptState& script_state,
+                                 ExceptionState& exception_state);
+
+// Return if the shared-storage permissions policy is allowed in
+// `execution_context`. Reject the `resolver` with an error if disallowed.
+bool CheckSharedStoragePermissionsPolicy(ScriptState& script_state,
+                                         ExecutionContext& execution_context,
+                                         ScriptPromiseResolver& resolver);
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_SHARED_STORAGE_UTIL_H_
diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -261,6 +261,7 @@ crbug.com/1322089 virtual/gpu/fast/canvas/OffscreenCanvas-MessageChannel-transfe
 # These tests require shared-storage and fenced-frames
 # Keep this in sync with VirtualTestSuites.
 crbug.com/1218540 wpt_internal/shared_storage/* [ Skip ]
+crbug.com/1218540 external/wpt/shared-storage/* [ Skip ]
 crbug.com/1218540 virtual/shared-storage-fenced-frame-mparch/* [ Pass ]
 crbug.com/1218540 virtual/shared-storage-fenced-frame-shadow-dom/* [ Pass ]
 

diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites
@@ -1123,15 +1123,17 @@
     "prefix": "shared-storage-fenced-frame-mparch",
     "platforms": ["Linux", "Mac", "Win"],
     "bases": [
-      "wpt_internal/shared_storage"
+      "wpt_internal/shared_storage",
+      "external/wpt/shared-storage"
     ],
     "args": ["--enable-features=SharedStorageAPI,FencedFrames:implementation_type/mparch,PrivacySandboxAdsAPIsOverride"]
   },
   {
     "prefix": "shared-storage-fenced-frame-shadow-dom",
     "platforms": ["Linux", "Mac", "Win"],
     "bases": [
-      "wpt_internal/shared_storage"
+      "wpt_internal/shared_storage",
+      "external/wpt/shared-storage"
     ],
     "args": ["--enable-features=SharedStorageAPI,FencedFrames:implementation_type/shadow_dom,PrivacySandboxAdsAPIsOverride"]
   },