Update reporting of kKeySensitiveDataEvent to include the new kKeyEventResult.

Bug: 1088131
Change-Id: Ib27159284cdacbd83ff5e240905fd0a94d7a9705
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296825Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788352}

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.cc

@@ -403,7 +403,8 @@ void SafeBrowsingPrivateEventRouter::OnAnalysisConnectorResult(
     const std::string& trigger,
     safe_browsing::DeepScanAccessPoint /* access_point */,
     const safe_browsing::ContentAnalysisScanResult& result,
-    const int64_t content_size) {
+    const int64_t content_size,
+    safe_browsing::EventResult event_result) {
   if (!IsRealtimeReportingEnabled())
     return;
 
@@ -415,7 +416,7 @@ void SafeBrowsingPrivateEventRouter::OnAnalysisConnectorResult(
         content_size);
   } else if (result.tag == "dlp") {
     OnSensitiveDataEvent(url, file_name, download_digest_sha256, mime_type,
-                         trigger, result, content_size);
+                         trigger, result, content_size, event_result);
   }
 }
 
@@ -466,7 +467,8 @@ void SafeBrowsingPrivateEventRouter::OnSensitiveDataEvent(
     const std::string& mime_type,
     const std::string& trigger,
     const safe_browsing::ContentAnalysisScanResult& result,
-    const int64_t content_size) {
+    const int64_t content_size,
+    safe_browsing::EventResult event_result) {
   if (!IsRealtimeReportingEnabled())
     return;
 
@@ -477,7 +479,8 @@ void SafeBrowsingPrivateEventRouter::OnSensitiveDataEvent(
              const std::string& url, const std::string& file_name,
              const std::string& download_digest_sha256,
              const std::string& profile_user_name, const std::string& mime_type,
-             const std::string& trigger, const int64_t content_size) {
+             const std::string& trigger, const int64_t content_size,
+             safe_browsing::EventResult event_result) {
             // Create a real-time event dictionary from the arguments and
             // report it.
             base::Value event(base::Value::Type::DICTIONARY);
@@ -492,14 +495,17 @@ void SafeBrowsingPrivateEventRouter::OnSensitiveDataEvent(
             if (content_size >= 0)
               event.SetIntKey(kKeyContentSize, content_size);
             event.SetStringKey(kKeyTrigger, trigger);
-            event.SetBoolKey(kKeyClickedThrough, false);
+            event.SetStringKey(
+                kKeyEventResult,
+                safe_browsing::EventResultToString(event_result));
 
             AddAnalysisConnectorVerdictToEvent(result, &event);
 
             return event;
           },
           result, url.spec(), file_name, download_digest_sha256,
-          GetProfileUserName(), mime_type, trigger, content_size));
+          GetProfileUserName(), mime_type, trigger, content_size,
+          event_result));
 }
 
 void SafeBrowsingPrivateEventRouter::OnAnalysisConnectorWarningBypassed(
@@ -540,7 +546,9 @@ void SafeBrowsingPrivateEventRouter::OnAnalysisConnectorWarningBypassed(
             if (content_size >= 0)
               event.SetIntKey(kKeyContentSize, content_size);
             event.SetStringKey(kKeyTrigger, trigger);
-            event.SetBoolKey(kKeyClickedThrough, true);
+            event.SetStringKey(kKeyEventResult,
+                               safe_browsing::EventResultToString(
+                                   safe_browsing::EventResult::BYPASSED));
 
             AddAnalysisConnectorVerdictToEvent(result, &event);
 

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h

@@ -136,7 +136,8 @@ class SafeBrowsingPrivateEventRouter
       const std::string& trigger,
       safe_browsing::DeepScanAccessPoint access_point,
       const safe_browsing::ContentAnalysisScanResult& result,
-      const int64_t content_size);
+      const int64_t content_size,
+      safe_browsing::EventResult event_result);
 
   // Notifies listeners that an analysis connector violation was bypassed.
   void OnAnalysisConnectorWarningBypassed(
@@ -275,7 +276,8 @@ class SafeBrowsingPrivateEventRouter
       const std::string& mime_type,
       const std::string& trigger,
       const safe_browsing::ContentAnalysisScanResult& result,
-      const int64_t content_size);
+      const int64_t content_size,
+      safe_browsing::EventResult event_result);
 
   content::BrowserContext* context_;
   signin::IdentityManager* identity_manager_ = nullptr;

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router_unittest.cc

@@ -190,7 +190,7 @@ class SafeBrowsingPrivateEventRouterTest : public testing::Test {
             "sha256_of_bypass_exe", "BYPASSED_WARNING", "exe", 890);
   }
 
-  void TriggerOnSensitiveDataEvent() {
+  void TriggerOnSensitiveDataEvent(safe_browsing::EventResult event_result) {
     safe_browsing::ContentAnalysisScanResult result;
     result.tag = "dlp";
     result.status = 1;
@@ -205,7 +205,8 @@ class SafeBrowsingPrivateEventRouterTest : public testing::Test {
             GURL("https://evil.com/sensitive_data.txt"), "sensitive_data.txt",
             "sha256_of_data", "text/plain",
             SafeBrowsingPrivateEventRouter::kTriggerFileUpload,
-            safe_browsing::DeepScanAccessPoint::UPLOAD, result, 12345);
+            safe_browsing::DeepScanAccessPoint::UPLOAD, result, 12345,
+            event_result);
   }
 
   void TriggerOnUnscannedFileEvent(safe_browsing::EventResult result) {
@@ -690,14 +691,14 @@ TEST_F(SafeBrowsingPrivateEventRouterTest,
   EXPECT_EQ(base::Value::Type::NONE, report.type());
 }
 
-TEST_F(SafeBrowsingPrivateEventRouterTest, TestOnSensitiveDataEvent) {
+TEST_F(SafeBrowsingPrivateEventRouterTest, TestOnSensitiveDataEvent_Allowed) {
   SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/true);
 
   base::Value report;
   EXPECT_CALL(*client_, UploadRealtimeReport_(_, _))
       .WillOnce(CaptureArg(&report));
 
-  TriggerOnSensitiveDataEvent();
+  TriggerOnSensitiveDataEvent(safe_browsing::EventResult::ALLOWED);
   base::RunLoop().RunUntilIdle();
 
   Mock::VerifyAndClearExpectations(client_.get());
@@ -736,6 +737,63 @@ TEST_F(SafeBrowsingPrivateEventRouterTest, TestOnSensitiveDataEvent) {
                        SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleId));
   EXPECT_EQ(3, triggered_rule.FindIntKey(
                    SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleAction));
+  EXPECT_EQ(
+      safe_browsing::EventResultToString(safe_browsing::EventResult::ALLOWED),
+      *event->FindStringKey(SafeBrowsingPrivateEventRouter::kKeyEventResult));
+  EXPECT_EQ("fake rule",
+            *triggered_rule.FindStringKey(
+                SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleName));
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest, TestOnSensitiveDataEvent_Blocked) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/true);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport_(_, _))
+      .WillOnce(CaptureArg(&report));
+
+  TriggerOnSensitiveDataEvent(safe_browsing::EventResult::BLOCKED);
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_.get());
+  EXPECT_EQ(base::Value::Type::DICTIONARY, report.type());
+  base::Value* event_list =
+      report.FindKey(policy::RealtimeReportingJobConfiguration::kEventListKey);
+  ASSERT_NE(nullptr, event_list);
+  EXPECT_EQ(base::Value::Type::LIST, event_list->type());
+  base::Value::ListView mutable_list = event_list->GetList();
+  ASSERT_EQ(1, (int)mutable_list.size());
+  base::Value wrapper = std::move(mutable_list[0]);
+  EXPECT_EQ(base::Value::Type::DICTIONARY, wrapper.type());
+  base::Value* event =
+      wrapper.FindKey(SafeBrowsingPrivateEventRouter::kKeySensitiveDataEvent);
+  ASSERT_NE(nullptr, event);
+
+  EXPECT_EQ(12345, *event->FindIntKey(
+                       SafeBrowsingPrivateEventRouter::kKeyContentSize));
+  EXPECT_EQ("text/plain", *event->FindStringKey(
+                              SafeBrowsingPrivateEventRouter::kKeyContentType));
+  EXPECT_EQ("sha256_of_data",
+            *event->FindStringKey(
+                SafeBrowsingPrivateEventRouter::kKeyDownloadDigestSha256));
+  EXPECT_EQ(
+      "sensitive_data.txt",
+      *event->FindStringKey(SafeBrowsingPrivateEventRouter::kKeyFileName));
+  EXPECT_EQ(SafeBrowsingPrivateEventRouter::kTriggerFileUpload,
+            *event->FindStringKey(SafeBrowsingPrivateEventRouter::kKeyTrigger));
+
+  base::Value* triggered_rule_info =
+      event->FindKey(SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleInfo);
+  ASSERT_NE(nullptr, triggered_rule_info);
+  ASSERT_EQ(1u, triggered_rule_info->GetList().size());
+  base::Value triggered_rule = std::move(triggered_rule_info->GetList()[0]);
+  EXPECT_EQ(12345, triggered_rule.FindIntKey(
+                       SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleId));
+  EXPECT_EQ(3, triggered_rule.FindIntKey(
+                   SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleAction));
+  EXPECT_EQ(
+      safe_browsing::EventResultToString(safe_browsing::EventResult::BLOCKED),
+      *event->FindStringKey(SafeBrowsingPrivateEventRouter::kKeyEventResult));
   EXPECT_EQ("fake rule",
             *triggered_rule.FindStringKey(
                 SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleName));

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate_browsertest.cc

@@ -514,7 +514,8 @@ IN_PROC_BROWSER_TEST_P(DeepScanningDialogDelegateSimpleBrowserTest, Texts) {
       /*trigger*/ SafeBrowsingPrivateEventRouter::kTriggerWebContentUpload,
       /*dlp_verdict*/ dlp_verdict,
       /*mimetype*/ TextMimeTypes(),
-      /*size*/ 400);
+      /*size*/ 400,
+      /*result*/ EventResultToString(EventResult::BLOCKED));
 
   bool called = false;
   base::RunLoop run_loop;
@@ -966,7 +967,8 @@ IN_PROC_BROWSER_TEST_P(DeepScanningDialogDelegateDelayDeliveryUntilVerdictTest,
       extensions::SafeBrowsingPrivateEventRouter::kTriggerFileUpload,
       /*dlp_verdict*/ dlp_verdict,
       /*mimetypes*/ DocMimeTypes(),
-      /*size*/ std::string("foo content").size());
+      /*size*/ std::string("foo content").size(),
+      /*result*/ EventResultToString(EventResult::BLOCKED));
 
   bool called = false;
   base::RunLoop run_loop;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.cc

@@ -191,7 +191,8 @@ void EventReportValidator::ExpectSensitiveDataEvent(
     const std::string& expected_trigger,
     const ContentAnalysisScanResult& expected_dlp_verdict,
     const std::set<std::string>* expected_mimetypes,
-    int expected_content_size) {
+    int expected_content_size,
+    const std::string& expected_result) {
   event_key_ = SafeBrowsingPrivateEventRouter::kKeySensitiveDataEvent;
   url_ = expected_url;
   dlp_verdict_ = expected_dlp_verdict;
@@ -199,8 +200,8 @@ void EventReportValidator::ExpectSensitiveDataEvent(
   sha256_ = expected_sha256;
   mimetypes_ = expected_mimetypes;
   trigger_ = expected_trigger;
-  clicked_through_ = false;
   content_size_ = expected_content_size;
+  result_ = expected_result;
   EXPECT_CALL(*client_, UploadRealtimeReport_(_, _))
       .WillOnce([this](base::Value& report,
                        base::OnceCallback<void(bool)>& callback) {
@@ -219,7 +220,8 @@ void EventReportValidator::
         const std::string& expected_trigger,
         const ContentAnalysisScanResult& expected_dlp_verdict,
         const std::set<std::string>* expected_mimetypes,
-        int expected_content_size) {
+        int expected_content_size,
+        const std::string& expected_result) {
   event_key_ = SafeBrowsingPrivateEventRouter::kKeyDangerousDownloadEvent;
   url_ = expected_url;
   filename_ = expected_filename;
@@ -234,12 +236,12 @@ void EventReportValidator::
         ValidateReport(&report);
       })
       .WillOnce(
-          [this, expected_dlp_verdict](
+          [this, expected_dlp_verdict, expected_result](
               base::Value& report, base::OnceCallback<void(bool)>& callback) {
             event_key_ = SafeBrowsingPrivateEventRouter::kKeySensitiveDataEvent;
             threat_type_ = base::nullopt;
-            clicked_through_ = false;
             dlp_verdict_ = expected_dlp_verdict;
+            result_ = expected_result;
             ValidateReport(&report);
             if (!done_closure_.is_null())
               done_closure_.Run();
@@ -295,8 +297,6 @@ void EventReportValidator::ValidateDlpVerdict(base::Value* value) {
   if (!dlp_verdict_.has_value())
     return;
 
-  ValidateField(value, SafeBrowsingPrivateEventRouter::kKeyClickedThrough,
-                clicked_through_);
   base::Value* triggered_rules =
       value->FindListKey(SafeBrowsingPrivateEventRouter::kKeyTriggeredRuleInfo);
   ASSERT_NE(nullptr, triggered_rules);

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.h

@@ -49,7 +49,8 @@ class EventReportValidator {
       const std::string& expected_trigger,
       const ContentAnalysisScanResult& expected_dlp_verdict,
       const std::set<std::string>* expected_mimetypes,
-      int expected_content_size);
+      int expected_content_size,
+      const std::string& expected_result);
 
   void ExpectDangerousDeepScanningResultAndSensitiveDataEvent(
       const std::string& expected_url,
@@ -59,7 +60,8 @@ class EventReportValidator {
       const std::string& expected_trigger,
       const ContentAnalysisScanResult& expected_dlp_verdict,
       const std::set<std::string>* expected_mimetypes,
-      int expected_content_size);
+      int expected_content_size,
+      const std::string& expected_result);
 
   void ExpectUnscannedFileEvent(const std::string& expected_url,
                                 const std::string& expected_filename,
@@ -99,7 +101,6 @@ class EventReportValidator {
   base::Optional<ContentAnalysisScanResult> dlp_verdict_ = base::nullopt;
   base::Optional<std::string> threat_type_ = base::nullopt;
   base::Optional<std::string> unscanned_reason_ = base::nullopt;
-  base::Optional<bool> clicked_through_ = base::nullopt;
   base::Optional<int> content_size_ = base::nullopt;
   const std::set<std::string>* mimetypes_ = nullptr;
   base::Optional<std::string> result_ = base::nullopt;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_utils.cc

@@ -122,7 +122,7 @@ void MaybeReportDeepScanningVerdict(Profile* profile,
             url, file_name, download_digest_sha256, mime_type, trigger,
             access_point,
             MalwareVerdictToResult(response.malware_scan_verdict()),
-            content_size);
+            content_size, event_result);
   }
 
   if (response.dlp_scan_verdict().status() == DlpDeepScanningVerdict::SUCCESS) {
@@ -132,7 +132,7 @@ void MaybeReportDeepScanningVerdict(Profile* profile,
               url, file_name, download_digest_sha256, mime_type, trigger,
               access_point,
               SensitiveDataVerdictToResult(response.dlp_scan_verdict()),
-              content_size);
+              content_size, event_result);
     }
   }
 }
@@ -179,7 +179,7 @@ void MaybeReportDeepScanningVerdict(
           ->OnAnalysisConnectorResult(url, file_name, download_digest_sha256,
                                       mime_type, trigger, access_point,
                                       ContentAnalysisResultToResult(result),
-                                      content_size);
+                                      content_size, event_result);
     }
   }
 }

chrome/browser/safe_browsing/download_protection/deep_scanning_request_unittest.cc

@@ -673,7 +673,8 @@ TEST_P(DeepScanningReportingTest, ProcessesResponseCorrectly) {
         extensions::SafeBrowsingPrivateEventRouter::kTriggerFileDownload,
         /*dlp_verdict*/ dlp_verdict,
         /*mimetypes*/ ExeMimeTypes(),
-        /*size*/ std::string("download contents").size());
+        /*size*/ std::string("download contents").size(),
+        EventResultToString(EventResult::WARNED));
 
     request.Start();
 
@@ -737,7 +738,8 @@ TEST_P(DeepScanningReportingTest, ProcessesResponseCorrectly) {
         extensions::SafeBrowsingPrivateEventRouter::kTriggerFileDownload,
         /*dlp_verdict*/ dlp_verdict,
         /*mimetypes*/ ExeMimeTypes(),
-        /*size*/ std::string("download contents").size());
+        /*size*/ std::string("download contents").size(),
+        EventResultToString(EventResult::WARNED));
 
     request.Start();
 
@@ -789,7 +791,8 @@ TEST_P(DeepScanningReportingTest, ProcessesResponseCorrectly) {
         extensions::SafeBrowsingPrivateEventRouter::kTriggerFileDownload,
         /*dlp_verdict*/ dlp_verdict,
         /*mimetypes*/ ExeMimeTypes(),
-        /*size*/ std::string("download contents").size());
+        /*size*/ std::string("download contents").size(),
+        EventResultToString(EventResult::BLOCKED));
 
     request.Start();
 
@@ -841,7 +844,8 @@ TEST_P(DeepScanningReportingTest, ProcessesResponseCorrectly) {
         extensions::SafeBrowsingPrivateEventRouter::kTriggerFileDownload,
         /*dlp_verdict*/ dlp_verdict,
         /*mimetypes*/ ExeMimeTypes(),
-        /*size*/ std::string("download contents").size());
+        /*size*/ std::string("download contents").size(),
+        EventResultToString(EventResult::WARNED));
 
     request.Start();
 
@@ -899,7 +903,8 @@ TEST_P(DeepScanningReportingTest, ProcessesResponseCorrectly) {
         extensions::SafeBrowsingPrivateEventRouter::kTriggerFileDownload,
         /*dlp_verdict*/ dlp_verdict,
         /*mimetypes*/ ExeMimeTypes(),
-        /*size*/ std::string("download contents").size());
+        /*size*/ std::string("download contents").size(),
+        EventResultToString(EventResult::BLOCKED));
 
     request.Start();