DLP: Add IsDragDropAllowed

Introduced the following changed to DataTransferPolicyController:
- Renamed IsReadAllowed to IsClipboardReadAllowed to be
used for clipboard copy/paste.
- Added IsDragDropAllowed to be used for drag-n-drop.
- The logic for IsDragDropAllowed will be changed later
upon confirmation from the PM & UX side.

Bug: 1139886
Change-Id: Ie0594e1d68abed98145106c2aa8afe480c9b3b27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595293Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nikita Podguzov <nikitapodguzov@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Commit-Queue: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841156}

chrome/browser/chromeos/policy/dlp/data_transfer_dlp_controller.cc

@@ -30,7 +30,7 @@ void DataTransferDlpController::Init(const DlpRulesManager& dlp_rules_manager) {
     new DataTransferDlpController(dlp_rules_manager);
 }
 
-bool DataTransferDlpController::IsDataReadAllowed(
+bool DataTransferDlpController::IsClipboardReadAllowed(
     const ui::DataTransferEndpoint* const data_src,
     const ui::DataTransferEndpoint* const data_dst) {
   if (!data_src || !data_src->IsUrlType()) {  // Currently we only handle URLs.
@@ -105,6 +105,13 @@ bool DataTransferDlpController::IsDataReadAllowed(
   return level == DlpRulesManager::Level::kAllow;
 }
 
+bool DataTransferDlpController::IsDragDropAllowed(
+    const ui::DataTransferEndpoint* const data_src,
+    const ui::DataTransferEndpoint* const data_dst) {
+  // TODO(crbug.com/1160656): Migrate off using `IsClipboardReadAllowed`.
+  return IsClipboardReadAllowed(data_src, data_dst);
+}
+
 DataTransferDlpController::DataTransferDlpController(
     const DlpRulesManager& dlp_rules_manager)
     : dlp_rules_manager_(dlp_rules_manager) {}

chrome/browser/chromeos/policy/dlp/data_transfer_dlp_controller.h

@@ -33,14 +33,15 @@ class DataTransferDlpController : public ui::DataTransferPolicyController {
   void operator=(const DataTransferDlpController&) = delete;
 
   // ui::DataTransferPolicyController:
-  // nullptr can be passed instead of `data_src` or `data_dst`. If data read is
-  // not allowed, this function will show a notification to the user.
-  bool IsDataReadAllowed(
+  bool IsClipboardReadAllowed(
+      const ui::DataTransferEndpoint* const data_src,
+      const ui::DataTransferEndpoint* const data_dst) override;
+  bool IsDragDropAllowed(
       const ui::DataTransferEndpoint* const data_src,
       const ui::DataTransferEndpoint* const data_dst) override;
 
  protected:
-  DataTransferDlpController(const DlpRulesManager& dlp_rules_manager);
+  explicit DataTransferDlpController(const DlpRulesManager& dlp_rules_manager);
   ~DataTransferDlpController() override;
 
  private:

chrome/browser/chromeos/policy/dlp/data_transfer_dlp_controller_unittest.cc

@@ -52,7 +52,7 @@ class MockDlpRulesManager : public DlpRulesManager {
 
 class MockDlpController : public DataTransferDlpController {
  public:
-  MockDlpController(const DlpRulesManager& dlp_rules_manager)
+  explicit MockDlpController(const DlpRulesManager& dlp_rules_manager)
       : DataTransferDlpController(dlp_rules_manager) {}
 
   MOCK_METHOD2(DoNotifyBlockedPaste,
@@ -81,7 +81,7 @@ class DataTransferDlpControllerTest : public testing::Test {
 };
 
 TEST_F(DataTransferDlpControllerTest, NullSrc) {
-  EXPECT_EQ(true, dlp_controller_.IsDataReadAllowed(nullptr, nullptr));
+  EXPECT_EQ(true, dlp_controller_.IsClipboardReadAllowed(nullptr, nullptr));
 }
 
 TEST_F(DataTransferDlpControllerTest, NullDst) {
@@ -89,7 +89,7 @@ TEST_F(DataTransferDlpControllerTest, NullDst) {
   EXPECT_CALL(rules_manager_, IsRestrictedDestination)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
   EXPECT_CALL(dlp_controller_, DoNotifyBlockedPaste);
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, nullptr));
+  EXPECT_EQ(false, dlp_controller_.IsClipboardReadAllowed(&data_src, nullptr));
 }
 
 TEST_F(DataTransferDlpControllerTest, DefaultDst) {
@@ -98,7 +98,8 @@ TEST_F(DataTransferDlpControllerTest, DefaultDst) {
   EXPECT_CALL(rules_manager_, IsRestrictedDestination)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
   EXPECT_CALL(dlp_controller_, DoNotifyBlockedPaste);
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst_1));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst_1));
   testing::Mock::VerifyAndClearExpectations(&rules_manager_);
   testing::Mock::VerifyAndClearExpectations(&dlp_controller_);
 
@@ -107,13 +108,14 @@ TEST_F(DataTransferDlpControllerTest, DefaultDst) {
                                       /*notify_if_restricted=*/false);
   EXPECT_CALL(rules_manager_, IsRestrictedDestination)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst_2));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst_2));
 }
 
 TEST_F(DataTransferDlpControllerTest, ClipboardHistoryDst) {
   ui::DataTransferEndpoint data_src(url::Origin::Create(GURL(kGoogleUrl)));
   ui::DataTransferEndpoint data_dst(ui::EndpointType::kClipboardHistory);
-  EXPECT_EQ(true, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst));
+  EXPECT_EQ(true, dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst));
 }
 
 TEST_F(DataTransferDlpControllerTest, UrlSrcDst) {
@@ -122,7 +124,8 @@ TEST_F(DataTransferDlpControllerTest, UrlSrcDst) {
   EXPECT_CALL(rules_manager_, IsRestrictedDestination)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
   EXPECT_CALL(dlp_controller_, DoNotifyBlockedPaste);
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst_1));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst_1));
   testing::Mock::VerifyAndClearExpectations(&rules_manager_);
   testing::Mock::VerifyAndClearExpectations(&dlp_controller_);
 
@@ -131,7 +134,8 @@ TEST_F(DataTransferDlpControllerTest, UrlSrcDst) {
                                       /*notify_if_restricted=*/false);
   EXPECT_CALL(rules_manager_, IsRestrictedDestination)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst_2));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst_2));
 }
 
 TEST_F(DataTransferDlpControllerTest, ArcDst) {
@@ -140,7 +144,8 @@ TEST_F(DataTransferDlpControllerTest, ArcDst) {
   EXPECT_CALL(rules_manager_, IsRestrictedComponent)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
   EXPECT_CALL(dlp_controller_, DoNotifyBlockedPaste);
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst));
 }
 
 TEST_F(DataTransferDlpControllerTest, CrostiniDst) {
@@ -149,7 +154,8 @@ TEST_F(DataTransferDlpControllerTest, CrostiniDst) {
   EXPECT_CALL(rules_manager_, IsRestrictedComponent)
       .WillOnce(testing::Return(DlpRulesManager::Level::kBlock));
   EXPECT_CALL(dlp_controller_, DoNotifyBlockedPaste);
-  EXPECT_EQ(false, dlp_controller_.IsDataReadAllowed(&data_src, &data_dst));
+  EXPECT_EQ(false,
+            dlp_controller_.IsClipboardReadAllowed(&data_src, &data_dst));
 }
 
 }  // namespace policy

chrome/browser/ui/ash/clipboard_history_browsertest.cc

@@ -789,7 +789,7 @@ class FakeDataTransferPolicyController
   ~FakeDataTransferPolicyController() override = default;
 
   // ui::DataTransferPolicyController:
-  bool IsDataReadAllowed(
+  bool IsClipboardReadAllowed(
       const ui::DataTransferEndpoint* const data_src,
       const ui::DataTransferEndpoint* const data_dst) override {
     // The multipaste menu should have access to any clipboard data.
@@ -802,6 +802,12 @@ class FakeDataTransferPolicyController
            (*data_src->origin() == allowed_origin_);
   }
 
+  bool IsDragDropAllowed(
+      const ui::DataTransferEndpoint* const data_src,
+      const ui::DataTransferEndpoint* const data_dst) override {
+    return false;
+  }
+
  private:
   const url::Origin allowed_origin_;
 };

ui/base/clipboard/clipboard_non_backed.cc

@@ -248,7 +248,7 @@ class ClipboardInternal {
     auto* data = GetData();
     if (!policy_controller || !data)
       return true;
-    return policy_controller->IsDataReadAllowed(data->source(), data_dst);
+    return policy_controller->IsClipboardReadAllowed(data->source(), data_dst);
   }
 
  private:

ui/base/clipboard/clipboard_test_template.h

@@ -109,7 +109,10 @@ class MockPolicyController : public DataTransferPolicyController {
   MockPolicyController();
   ~MockPolicyController() override;
 
-  MOCK_METHOD2(IsDataReadAllowed,
+  MOCK_METHOD2(IsClipboardReadAllowed,
+               bool(const DataTransferEndpoint* const data_src,
+                    const DataTransferEndpoint* const data_dst));
+  MOCK_METHOD2(IsDragDropAllowed,
                bool(const DataTransferEndpoint* const data_src,
                     const DataTransferEndpoint* const data_dst));
 };
@@ -1038,7 +1041,7 @@ TYPED_TEST(ClipboardTest, PolicyAllowDataRead) {
         std::make_unique<DataTransferEndpoint>(url::Origin()));
     writer.WriteText(kTestText);
   }
-  EXPECT_CALL(*policy_controller, IsDataReadAllowed)
+  EXPECT_CALL(*policy_controller, IsClipboardReadAllowed)
       .WillRepeatedly(testing::Return(true));
   base::string16 read_result;
   this->clipboard().ReadText(ClipboardBuffer::kCopyPaste,
@@ -1058,7 +1061,7 @@ TYPED_TEST(ClipboardTest, PolicyDisallow_ReadText) {
         std::make_unique<DataTransferEndpoint>(url::Origin()));
     writer.WriteText(kTestText);
   }
-  EXPECT_CALL(*policy_controller, IsDataReadAllowed)
+  EXPECT_CALL(*policy_controller, IsClipboardReadAllowed)
       .WillRepeatedly(testing::Return(false));
   base::string16 read_result;
   this->clipboard().ReadText(ClipboardBuffer::kCopyPaste,
@@ -1069,7 +1072,7 @@ TYPED_TEST(ClipboardTest, PolicyDisallow_ReadText) {
 
 TYPED_TEST(ClipboardTest, PolicyDisallow_ReadImage) {
   auto policy_controller = std::make_unique<MockPolicyController>();
-  EXPECT_CALL(*policy_controller, IsDataReadAllowed)
+  EXPECT_CALL(*policy_controller, IsClipboardReadAllowed)
       .WillRepeatedly(testing::Return(false));
   const SkBitmap& image = clipboard_test_util::ReadImage(&this->clipboard());
   ::testing::Mock::VerifyAndClearExpectations(policy_controller.get());

ui/base/clipboard/test/test_clipboard.cc

@@ -20,12 +20,12 @@
 namespace ui {
 
 namespace {
-bool IsDataReadAllowed(const DataTransferEndpoint* src,
-                       const DataTransferEndpoint* dst) {
+bool IsReadAllowed(const DataTransferEndpoint* src,
+                   const DataTransferEndpoint* dst) {
   auto* policy_controller = DataTransferPolicyController::Get();
   if (!policy_controller)
     return true;
-  return policy_controller->IsDataReadAllowed(src, dst);
+  return policy_controller->IsClipboardReadAllowed(src, dst);
 }
 }  // namespace
 
@@ -56,7 +56,7 @@ bool TestClipboard::IsFormatAvailable(
     const ClipboardFormatType& format,
     ClipboardBuffer buffer,
     const ui::DataTransferEndpoint* data_dst) const {
-  if (!IsDataReadAllowed(GetStore(buffer).data_src.get(), data_dst))
+  if (!IsReadAllowed(GetStore(buffer).data_src.get(), data_dst))
     return false;
 #if defined(OS_LINUX) || defined(OS_CHROMEOS)
   // The linux clipboard treats the presence of text on the clipboard
@@ -79,7 +79,7 @@ void TestClipboard::ReadAvailableTypes(
     std::vector<base::string16>* types) const {
   DCHECK(types);
   types->clear();
-  if (!IsDataReadAllowed(GetStore(buffer).data_src.get(), data_dst))
+  if (!IsReadAllowed(GetStore(buffer).data_src.get(), data_dst))
     return;
 
   if (IsFormatAvailable(ClipboardFormatType::GetPlainTextType(), buffer,
@@ -99,7 +99,7 @@ TestClipboard::ReadAvailablePlatformSpecificFormatNames(
     ClipboardBuffer buffer,
     const ui::DataTransferEndpoint* data_dst) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return {};
 
   const auto& data = store.data;
@@ -129,7 +129,7 @@ TestClipboard::ReadAvailablePlatformSpecificFormatNames(
 void TestClipboard::ReadText(ClipboardBuffer buffer,
                              const DataTransferEndpoint* data_dst,
                              base::string16* result) const {
-  if (!IsDataReadAllowed(GetStore(buffer).data_src.get(), data_dst))
+  if (!IsReadAllowed(GetStore(buffer).data_src.get(), data_dst))
     return;
 
   std::string result8;
@@ -142,7 +142,7 @@ void TestClipboard::ReadAsciiText(ClipboardBuffer buffer,
                                   const DataTransferEndpoint* data_dst,
                                   std::string* result) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   result->clear();
@@ -158,7 +158,7 @@ void TestClipboard::ReadHTML(ClipboardBuffer buffer,
                              uint32_t* fragment_start,
                              uint32_t* fragment_end) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   markup->clear();
@@ -175,7 +175,7 @@ void TestClipboard::ReadSvg(ClipboardBuffer buffer,
                             const DataTransferEndpoint* data_dst,
                             base::string16* result) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   result->clear();
@@ -188,7 +188,7 @@ void TestClipboard::ReadRTF(ClipboardBuffer buffer,
                             const DataTransferEndpoint* data_dst,
                             std::string* result) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   result->clear();
@@ -201,7 +201,7 @@ void TestClipboard::ReadImage(ClipboardBuffer buffer,
                               const DataTransferEndpoint* data_dst,
                               ReadImageCallback callback) const {
   const DataStore& store = GetStore(buffer);
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst)) {
+  if (!IsReadAllowed(store.data_src.get(), data_dst)) {
     std::move(callback).Run(SkBitmap());
     return;
   }
@@ -219,7 +219,7 @@ void TestClipboard::ReadBookmark(const DataTransferEndpoint* data_dst,
                                  base::string16* title,
                                  std::string* url) const {
   const DataStore& store = GetDefaultStore();
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   if (url) {
@@ -235,7 +235,7 @@ void TestClipboard::ReadData(const ClipboardFormatType& format,
                              const DataTransferEndpoint* data_dst,
                              std::string* result) const {
   const DataStore& store = GetDefaultStore();
-  if (!IsDataReadAllowed(store.data_src.get(), data_dst))
+  if (!IsReadAllowed(store.data_src.get(), data_dst))
     return;
 
   result->clear();

ui/base/data_transfer_policy/data_transfer_policy_controller.h

@@ -10,10 +10,10 @@
 
 namespace ui {
 
-// The DataTransfer filter controls transferring data via drag-and-drop and
-// clipboard read operations. It allows/disallows transferring the data given
-// the source of the data and the destination trying to access the data and a
-// set of rules controlling these source and destination.
+// The DataTransfer policy controller controls transferring data via
+// drag-and-drop and clipboard read operations. It allows/disallows transferring
+// the data given the source of the data and the destination trying to access
+// the data and a set of rules controlling these source and destination.
 class COMPONENT_EXPORT(UI_BASE_DATA_TRANSFER_POLICY)
     DataTransferPolicyController {
  public:
@@ -27,7 +27,15 @@ class COMPONENT_EXPORT(UI_BASE_DATA_TRANSFER_POLICY)
   // Indicates that restricting data transfer is no longer required.
   static void DeleteInstance();
 
-  virtual bool IsDataReadAllowed(
+  // nullptr can be passed instead of `data_src` or `data_dst`. If clipboard
+  // read is not allowed, this function will show a notification to the user.
+  virtual bool IsClipboardReadAllowed(
+      const DataTransferEndpoint* const data_src,
+      const DataTransferEndpoint* const data_dst) = 0;
+
+  // nullptr can be passed instead of `data_src` or `data_dst`. If dropping the
+  // data is not allowed, this function will show a notification to the user.
+  virtual bool IsDragDropAllowed(
       const DataTransferEndpoint* const data_src,
       const DataTransferEndpoint* const data_dst) = 0;