From ba383e4bb1330749c36483b7b5b994a1f5281848 Mon Sep 17 00:00:00 2001 From: Gary Kacmarcik Date: Wed, 20 Dec 2017 23:41:00 +0000 Subject: [PATCH] Update clipboard API calls to use clipboard permissions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds permission checks to make sure that the user has granted appropriate permission for the clipboard API methods. Bug: 677564 Change-Id: I5af221b0e18d45509ff2a8c3856680e598a51dcb Reviewed-on: https://chromium-review.googlesource.com/804973 Reviewed-by: Antoine Labour Reviewed-by: Raymes Khoury Reviewed-by: Philip Jägenstedt Commit-Queue: Gary Kacmarcik Cr-Commit-Position: refs/heads/master@{#525516} --- .../permissions/permission_uma_util.cc | 12 +- chrome/browser/permissions/permission_util.cc | 2 + .../layout_test/layout_test_message_filter.cc | 4 + third_party/WebKit/LayoutTests/NeverFixTests | 5 + .../readtext-denied.https-expected.txt | 12 ++ .../clipboard/readtext-denied.https.html | 34 +++++ .../readtext-granted.https-expected.txt | 12 ++ .../clipboard/readtext-granted.https.html | 34 +++++ .../writetext-denied.https-expected.txt | 12 ++ .../clipboard/writetext-denied.https.html | 34 +++++ .../writetext-granted.https-expected.txt | 12 ++ .../clipboard/writetext-granted.https.html | 34 +++++ .../tests/resources/permissions-helper.js | 4 + .../modules/clipboard/ClipboardPromise.cpp | 139 ++++++++++++++++-- .../modules/clipboard/ClipboardPromise.h | 21 +++ 15 files changed, 356 insertions(+), 15 deletions(-) create mode 100644 third_party/WebKit/LayoutTests/clipboard/readtext-denied.https-expected.txt create mode 100644 third_party/WebKit/LayoutTests/clipboard/readtext-denied.https.html create mode 100644 third_party/WebKit/LayoutTests/clipboard/readtext-granted.https-expected.txt create mode 100644 third_party/WebKit/LayoutTests/clipboard/readtext-granted.https.html create mode 100644 third_party/WebKit/LayoutTests/clipboard/writetext-denied.https-expected.txt create mode 100644 third_party/WebKit/LayoutTests/clipboard/writetext-denied.https.html create mode 100644 third_party/WebKit/LayoutTests/clipboard/writetext-granted.https-expected.txt create mode 100644 third_party/WebKit/LayoutTests/clipboard/writetext-granted.https.html diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc index 9c3832c164a279..7c67292bc7001e 100644 --- a/chrome/browser/permissions/permission_uma_util.cc +++ b/chrome/browser/permissions/permission_uma_util.cc @@ -92,6 +92,8 @@ std::string GetPermissionRequestString(PermissionRequestType type) { return "AudioCapture"; case PermissionRequestType::PERMISSION_MEDIASTREAM_CAMERA: return "VideoCapture"; + case PermissionRequestType::PERMISSION_CLIPBOARD_READ: + return "ClipboardRead"; default: NOTREACHED(); return ""; @@ -500,9 +502,9 @@ void PermissionUmaUtil::RecordPermissionAction( bool secure_origin = content::IsOriginSecure(requesting_origin); switch (permission) { - // Geolocation, MidiSysEx, Push, and Media permissions are disabled on - // insecure origins, so there's no need to record separate metrics for - // secure/insecure. + // Geolocation, MidiSysEx, Push, Media and Clipboard permissions are + // disabled on insecure origins, so there's no need to record separate + // metrics for secure/insecure. case CONTENT_SETTINGS_TYPE_GEOLOCATION: UMA_HISTOGRAM_ENUMERATION("Permissions.Action.Geolocation", action, PermissionAction::NUM); @@ -536,6 +538,10 @@ void PermissionUmaUtil::RecordPermissionAction( "Permissions.Action.SecureOrigin.Flash", "Permissions.Action.InsecureOrigin.Flash", action); break; + case CONTENT_SETTINGS_TYPE_CLIPBOARD_READ: + UMA_HISTOGRAM_ENUMERATION("Permissions.Action.ClipboardRead", action, + PermissionAction::NUM); + break; // The user is not prompted for these permissions, thus there is no // permission action recorded for them. default: diff --git a/chrome/browser/permissions/permission_util.cc b/chrome/browser/permissions/permission_util.cc index 632c9317b10747..6edf0c95f110a0 100644 --- a/chrome/browser/permissions/permission_util.cc +++ b/chrome/browser/permissions/permission_util.cc @@ -46,6 +46,8 @@ std::string PermissionUtil::GetPermissionString( return "AccessibilityEvents"; case CONTENT_SETTINGS_TYPE_CLIPBOARD_READ: return "ClipboardRead"; + case CONTENT_SETTINGS_TYPE_CLIPBOARD_WRITE: + return "ClipboardWrite"; default: break; } diff --git a/content/shell/browser/layout_test/layout_test_message_filter.cc b/content/shell/browser/layout_test/layout_test_message_filter.cc index bd9146e3ce46da..cfd543ab58697a 100644 --- a/content/shell/browser/layout_test/layout_test_message_filter.cc +++ b/content/shell/browser/layout_test/layout_test_message_filter.cc @@ -187,6 +187,10 @@ void LayoutTestMessageFilter::OnSetPermission( type = PermissionType::BACKGROUND_SYNC; } else if (name == "accessibility-events") { type = PermissionType::ACCESSIBILITY_EVENTS; + } else if (name == "clipboard-read") { + type = PermissionType::CLIPBOARD_READ; + } else if (name == "clipboard-write") { + type = PermissionType::CLIPBOARD_WRITE; } else { NOTREACHED(); type = PermissionType::NOTIFICATIONS; diff --git a/third_party/WebKit/LayoutTests/NeverFixTests b/third_party/WebKit/LayoutTests/NeverFixTests index 9863997a810688..c52aae415b5895 100644 --- a/third_party/WebKit/LayoutTests/NeverFixTests +++ b/third_party/WebKit/LayoutTests/NeverFixTests @@ -384,6 +384,11 @@ external/wpt/audio-output/setSinkId-manual.https.html [ WontFix ] external/wpt/battery-status/battery-charging-manual.https.html [ WontFix ] external/wpt/battery-status/battery-plugging-in-manual.https.html [ WontFix ] external/wpt/battery-status/battery-unplugging-manual.https.html [ WontFix ] +external/wpt/clipboard-apis/async-navigator-clipboard-basics.https.html [ WontFix ] +external/wpt/clipboard-apis/async-write-dttext-read-dttext-manual.https.html [ WontFix ] +external/wpt/clipboard-apis/async-write-dttext-read-text-manual.https.html [ WontFix ] +external/wpt/clipboard-apis/async-write-text-read-dttext-manual.https.html [ WontFix ] +external/wpt/clipboard-apis/async-write-text-read-text-manual.https.html [ WontFix ] external/wpt/console/console-count-logging-manual.html [ WontFix ] external/wpt/css/CSS2/linebox/inline-formatting-context-010b.xht [ WontFix ] external/wpt/css/CSS2/normal-flow/inline-block-replaced-height-008.xht [ WontFix ] diff --git a/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https-expected.txt b/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https-expected.txt new file mode 100644 index 00000000000000..d5d1bfb08b2d5e --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https-expected.txt @@ -0,0 +1,12 @@ +Tests navigator.clipboard.readText() permission failure. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS PermissionsHelper.setPermission is defined. +PASS navigator.clipboard is non-null. +PASS clipboard.readText() fail (as expected). +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https.html b/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https.html new file mode 100644 index 00000000000000..c089a3a03e97b2 --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/readtext-denied.https.html @@ -0,0 +1,34 @@ + + + + + + + + + + diff --git a/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https-expected.txt b/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https-expected.txt new file mode 100644 index 00000000000000..7751372d4db72d --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https-expected.txt @@ -0,0 +1,12 @@ +Tests navigator.clipboard.readText() permission success. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS PermissionsHelper.setPermission is defined. +PASS navigator.clipboard is non-null. +PASS clipboard.readText() success (as expected). +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https.html b/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https.html new file mode 100644 index 00000000000000..6f2ada9cd23898 --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/readtext-granted.https.html @@ -0,0 +1,34 @@ + + + + + + + + + + diff --git a/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https-expected.txt b/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https-expected.txt new file mode 100644 index 00000000000000..15877da5e8101d --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https-expected.txt @@ -0,0 +1,12 @@ +Tests navigator.clipboard.writeText() permission failure. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS PermissionsHelper.setPermission is defined. +PASS navigator.clipboard is non-null. +PASS clipboard.writeText() fail (as expected). +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https.html b/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https.html new file mode 100644 index 00000000000000..3abd9b74d75a32 --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/writetext-denied.https.html @@ -0,0 +1,34 @@ + + + + + + + + + + diff --git a/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https-expected.txt b/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https-expected.txt new file mode 100644 index 00000000000000..fbeb5e02e71dfd --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https-expected.txt @@ -0,0 +1,12 @@ +Tests navigator.clipboard.writeText() permission success. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS PermissionsHelper.setPermission is defined. +PASS navigator.clipboard is non-null. +PASS clipboard.writeText() success (as expected). +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https.html b/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https.html new file mode 100644 index 00000000000000..b0716bcb044acb --- /dev/null +++ b/third_party/WebKit/LayoutTests/clipboard/writetext-granted.https.html @@ -0,0 +1,34 @@ + + + + + + + + + + diff --git a/third_party/WebKit/LayoutTests/http/tests/resources/permissions-helper.js b/third_party/WebKit/LayoutTests/http/tests/resources/permissions-helper.js index 286ff1246ffd72..1e18f50e189292 100644 --- a/third_party/WebKit/LayoutTests/http/tests/resources/permissions-helper.js +++ b/third_party/WebKit/LayoutTests/http/tests/resources/permissions-helper.js @@ -27,6 +27,10 @@ var PermissionsHelper = (function() { return {name: "background-sync"}; case "accessibility-events": return {name: "accessibility-events"}; + case "clipboard-read": + return {name: "clipboard-read"}; + case "clipboard-write": + return {name: "clipboard-write"}; default: throw "Invalid permission name provided"; } diff --git a/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.cpp b/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.cpp index 95c24127a8db25..f1ea1d9ff98d84 100644 --- a/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.cpp +++ b/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.cpp @@ -9,14 +9,33 @@ #include "core/clipboard/DataTransfer.h" #include "core/clipboard/DataTransferItem.h" #include "core/clipboard/DataTransferItemList.h" -#include "platform/CrossThreadFunctional.h" +#include "core/dom/ExecutionContext.h" +#include "core/frame/LocalFrame.h" +#include "modules/permissions/PermissionUtils.h" #include "platform/clipboard/ClipboardMimeTypes.h" #include "public/platform/Platform.h" #include "public/platform/TaskType.h" +#include "public/platform/modules/permissions/permission.mojom-blink.h" #include "third_party/WebKit/public/platform/WebClipboard.h" +// And now, a brief note about clipboard permissions. +// +// There are 2 clipboard permissions defined in the spec: +// * clipboard-read +// * clipboard-write +// See https://w3c.github.io/clipboard-apis/#clipboard-permissions +// +// In Chromium we automatically grant clipboard-write access and clipboard-read +// access is gated behind a permission prompt. Both clipboard read and write +// require the tab to be focused (and Chromium must be the foreground app) for +// the operation to be allowed. + namespace blink { +using mojom::blink::PermissionStatus; +using mojom::blink::PermissionService; +using mojom::PageVisibilityState; + ScriptPromise ClipboardPromise::CreateForRead(ScriptState* script_state) { ClipboardPromise* clipboard_promise = new ClipboardPromise(script_state); clipboard_promise->GetTaskRunner()->PostTask( @@ -54,17 +73,84 @@ ScriptPromise ClipboardPromise::CreateForWriteText(ScriptState* script_state, ClipboardPromise::ClipboardPromise(ScriptState* script_state) : ContextLifecycleObserver(blink::ExecutionContext::From(script_state)), + script_state_(script_state), script_promise_resolver_(ScriptPromiseResolver::Create(script_state)), - buffer_(mojom::ClipboardBuffer::kStandard) {} + buffer_(mojom::ClipboardBuffer::kStandard), + write_data_() {} scoped_refptr ClipboardPromise::GetTaskRunner() { // TODO(garykac): Replace MiscPlatformAPI with TaskType specific to clipboard. return GetExecutionContext()->GetTaskRunner(TaskType::kMiscPlatformAPI); } -// TODO(garykac): This currently only handles plain text. -void ClipboardPromise::HandleRead() { +PermissionService* ClipboardPromise::GetPermissionService() { + if (!permission_service_) { + ConnectToPermissionService(ExecutionContext::From(script_state_), + mojo::MakeRequest(&permission_service_)); + } + return permission_service_.get(); +} + +bool ClipboardPromise::IsFocusedDocument(ExecutionContext* context) { + DCHECK(context->IsDocument()); + Document* doc = ToDocumentOrNull(context); + return doc && doc->hasFocus(); +} + +void ClipboardPromise::RequestReadPermission( + PermissionService::RequestPermissionCallback callback) { + DCHECK(script_promise_resolver_); + + ExecutionContext* context = ExecutionContext::From(script_state_); + DCHECK(context->IsSecureContext()); // [SecureContext] in IDL + + // Document must be focused. + if (!IsFocusedDocument(context) || !GetPermissionService()) { + script_promise_resolver_->Reject(); + return; + } + + // Query for permission if necessary. + // See crbug.com/795929 for moving this check into the Browser process. + permission_service_->RequestPermission( + CreateClipboardPermissionDescriptor( + mojom::blink::PermissionName::CLIPBOARD_READ, false), + false, std::move(callback)); +} + +void ClipboardPromise::CheckWritePermission( + PermissionService::HasPermissionCallback callback) { DCHECK(script_promise_resolver_); + + ExecutionContext* context = ExecutionContext::From(script_state_); + DCHECK(context->IsSecureContext()); // [SecureContext] in IDL + + // Document must be focused. + if (!IsFocusedDocument(context) || !GetPermissionService()) { + script_promise_resolver_->Reject(); + return; + } + + // Check current permission (but do not query the user). + // See crbug.com/795929 for moving this check into the Browser process. + permission_service_->HasPermission( + CreateClipboardPermissionDescriptor( + mojom::blink::PermissionName::CLIPBOARD_WRITE, false), + std::move(callback)); +} + +void ClipboardPromise::HandleRead() { + RequestReadPermission(WTF::Bind(&ClipboardPromise::HandleReadWithPermission, + WrapPersistent(this))); +} + +// TODO(garykac): This currently only handles plain text. +void ClipboardPromise::HandleReadWithPermission(PermissionStatus status) { + if (status != PermissionStatus::GRANTED) { + script_promise_resolver_->Reject(); + return; + } + String plain_text = Platform::Current()->Clipboard()->ReadPlainText(buffer_); const DataTransfer::DataTransferType type = @@ -77,32 +163,61 @@ void ClipboardPromise::HandleRead() { } void ClipboardPromise::HandleReadText() { - DCHECK(script_promise_resolver_); + RequestReadPermission(WTF::Bind( + &ClipboardPromise::HandleReadTextWithPermission, WrapPersistent(this))); +} + +void ClipboardPromise::HandleReadTextWithPermission(PermissionStatus status) { + if (status != PermissionStatus::GRANTED) { + script_promise_resolver_->Reject(); + return; + } + String text = Platform::Current()->Clipboard()->ReadPlainText(buffer_); script_promise_resolver_->Resolve(text); } // TODO(garykac): This currently only handles plain text. void ClipboardPromise::HandleWrite(DataTransfer* data) { - DCHECK(script_promise_resolver_); + // Scan DataTransfer and extract data types that we support. size_t num_items = data->items()->length(); for (unsigned long i = 0; i < num_items; i++) { DataTransferItem* item = data->items()->item(i); DataObjectItem* objectItem = item->GetDataObjectItem(); if (objectItem->Kind() == DataObjectItem::kStringKind && objectItem->GetType() == kMimeTypeTextPlain) { - String text = objectItem->GetAsString(); - Platform::Current()->Clipboard()->WritePlainText(text); - script_promise_resolver_->Resolve(); - return; + write_data_ = objectItem->GetAsString(); + break; } } - script_promise_resolver_->Reject(); + CheckWritePermission(WTF::Bind(&ClipboardPromise::HandleWriteWithPermission, + WrapPersistent(this))); +} + +void ClipboardPromise::HandleWriteWithPermission(PermissionStatus status) { + if (status != PermissionStatus::GRANTED) { + script_promise_resolver_->Reject(); + return; + } + + Platform::Current()->Clipboard()->WritePlainText(write_data_); + script_promise_resolver_->Resolve(); } void ClipboardPromise::HandleWriteText(const String& data) { + write_data_ = data; + CheckWritePermission(WTF::Bind( + &ClipboardPromise::HandleWriteTextWithPermission, WrapPersistent(this))); +} + +void ClipboardPromise::HandleWriteTextWithPermission(PermissionStatus status) { + if (status != PermissionStatus::GRANTED) { + script_promise_resolver_->Reject(); + return; + } + DCHECK(script_promise_resolver_); - Platform::Current()->Clipboard()->WritePlainText(data); + Platform::Current()->Clipboard()->WritePlainText(write_data_); script_promise_resolver_->Resolve(); } diff --git a/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.h b/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.h index 8b0c2543931c54..b4aa20e72ee8a9 100644 --- a/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.h +++ b/third_party/WebKit/Source/modules/clipboard/ClipboardPromise.h @@ -8,6 +8,7 @@ #include "bindings/core/v8/ScriptPromise.h" #include "core/CoreExport.h" #include "core/dom/ContextLifecycleObserver.h" +#include "public/platform/modules/permissions/permission.mojom-blink.h" #include "third_party/WebKit/common/clipboard/clipboard.mojom-blink.h" namespace blink { @@ -35,16 +36,36 @@ class ClipboardPromise final ClipboardPromise(ScriptState*); scoped_refptr GetTaskRunner(); + mojom::blink::PermissionService* GetPermissionService(); + + bool IsFocusedDocument(ExecutionContext*); + + void RequestReadPermission( + mojom::blink::PermissionService::RequestPermissionCallback); + void CheckWritePermission( + mojom::blink::PermissionService::HasPermissionCallback); void HandleRead(); + void HandleReadWithPermission(mojom::blink::PermissionStatus); + void HandleReadText(); + void HandleReadTextWithPermission(mojom::blink::PermissionStatus); void HandleWrite(DataTransfer*); + void HandleWriteWithPermission(mojom::blink::PermissionStatus); + void HandleWriteText(const String&); + void HandleWriteTextWithPermission(mojom::blink::PermissionStatus); + + ScriptState* script_state_; Member script_promise_resolver_; + mojom::blink::PermissionServicePtr permission_service_; + mojom::ClipboardBuffer buffer_; + + WebString write_data_; }; } // namespace blink