Update authentication code to track multiple Connectors

Bug: 1093321
Change-Id: Id513ffc0b7f2bdae8560817e5a69ecc8f52d26a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2499082Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Commit-Queue: Alice Gong <alicego@google.com>
Cr-Commit-Position: refs/heads/master@{#821897}

chrome/browser/enterprise/connectors/content_analysis_delegate_browsertest.cc

@@ -83,8 +83,10 @@ class FakeBinaryUploadService : public BinaryUploadService {
 
  private:
   void UploadForDeepScanning(std::unique_ptr<Request> request) override {
-    // The first uploaded request is the authentication one.
-    if (++requests_count_ == 1) {
+    ++requests_count_;
+
+    // A request without tags indicates that it's used for authentication
+    if (request->content_analysis_request().tags().empty()) {
       authorization_request_.swap(request);
 
       if (should_automatically_authorize_)
@@ -288,8 +290,9 @@ IN_PROC_BROWSER_TEST_F(ContentAnalysisDelegateBrowserTest, Unauthorized) {
   run_loop.Run();
   EXPECT_TRUE(called);
 
-  // Only 1 request (the authentication one) should have been uploaded.
-  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 1);
+  // 1 request to authenticate for upload,
+  // and 1 request to authenticate for reporting.
+  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 2);
 }
 
 IN_PROC_BROWSER_TEST_F(ContentAnalysisDelegateBrowserTest, Files) {
@@ -370,8 +373,10 @@ IN_PROC_BROWSER_TEST_F(ContentAnalysisDelegateBrowserTest, Files) {
 
   EXPECT_TRUE(called);
 
-  // There should have been 1 request per file and 1 for authentication.
-  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 3);
+  // There should have been 1 request per file (2 files) and 1 for
+  // authentication, and 1 more for final request to validate reporting
+  // authentication with the corresponding request type.
+  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 4);
 }
 
 IN_PROC_BROWSER_TEST_F(ContentAnalysisDelegateBrowserTest, Texts) {
@@ -451,8 +456,10 @@ IN_PROC_BROWSER_TEST_F(ContentAnalysisDelegateBrowserTest, Texts) {
   run_loop.Run();
   EXPECT_TRUE(called);
 
-  // There should have been 1 request for all texts and 1 for authentication.
-  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 2);
+  // There should have been 1 request for all texts,
+  // 1 for authentication of the scanning request,
+  // and 1 for final request to validate reporting authentication.
+  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 3);
 }
 
 class ContentAnalysisDelegatePasswordProtectedFilesBrowserTest
@@ -881,8 +888,11 @@ IN_PROC_BROWSER_TEST_P(ContentAnalysisDelegateDelayDeliveryUntilVerdictTest,
   run_loop.Run();
   EXPECT_TRUE(called);
 
-  // Expect 1 request for authentication and 1 to scan the file in all cases.
-  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 2);
+  // Expect 1 request for initial authentication (unspecified type, to be
+  // removed for crbug.com/1090088, then count should be 2), 1 to scan the file
+  // in all cases, and 1 more for final request to validate reportin
+  // authentication with the corresponding request type.
+  ASSERT_EQ(FakeBinaryUploadServiceStorage()->requests_count(), 3);
 }
 
 INSTANTIATE_TEST_SUITE_P(

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

@@ -876,7 +876,10 @@ void SafeBrowsingPrivateEventRouter::IfAuthorized(
 
   // TODO(crbug/1069049): Use reporting URL.
   if (binary_upload_service_)
-    binary_upload_service_->IsAuthorized(GURL(), std::move(cont));
+    binary_upload_service_->IsAuthorized(
+        GURL(), std::move(cont),
+        enterprise_connectors::AnalysisConnector::
+            ANALYSIS_CONNECTOR_UNSPECIFIED);
 }
 
 void SafeBrowsingPrivateEventRouter::ReportRealtimeEvent(

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.cc

@@ -213,18 +213,21 @@ void BinaryUploadService::MaybeUploadForDeepScanning(
     return;
   }
 
-  if (!can_upload_enterprise_data_.has_value()) {
+  auto connector = request->analysis_connector();
+
+  if (!can_upload_enterprise_data_.contains(request->analysis_connector())) {
     // Get the URL first since |request| is about to move.
     GURL url = request->GetUrlWithParams();
     IsAuthorized(
         std::move(url),
         base::BindOnce(&BinaryUploadService::MaybeUploadForDeepScanningCallback,
-                       weakptr_factory_.GetWeakPtr(), std::move(request)));
+                       weakptr_factory_.GetWeakPtr(), std::move(request)),
+        connector);
     return;
   }
 
   MaybeUploadForDeepScanningCallback(std::move(request),
-                                     can_upload_enterprise_data_.value());
+                                     can_upload_enterprise_data_[connector]);
 }
 
 void BinaryUploadService::MaybeUploadForDeepScanningCallback(
@@ -424,6 +427,7 @@ void BinaryUploadService::FinishRequest(
 
 void BinaryUploadService::FinishRequestCleanup(Request* request,
                                                const std::string& instance_id) {
+  auto connector = request->analysis_connector();
   active_requests_.erase(request);
   active_timers_.erase(request);
   active_uploads_.erase(request);
@@ -441,24 +445,26 @@ void BinaryUploadService::FinishRequestCleanup(Request* request,
     binary_fcm_service_->UnregisterInstanceID(
         instance_id,
         base::BindOnce(&BinaryUploadService::InstanceIDUnregisteredCallback,
-                       weakptr_factory_.GetWeakPtr()));
+                       weakptr_factory_.GetWeakPtr(), connector));
   } else {
     // |binary_fcm_service_| can be null in tests, but
     // InstanceIDUnregisteredCallback should be called anyway so the requests
     // waiting on authentication can complete.
-    InstanceIDUnregisteredCallback(true);
+    InstanceIDUnregisteredCallback(connector, true);
   }
 
   active_tokens_.erase(token_it);
 }
 
-void BinaryUploadService::InstanceIDUnregisteredCallback(bool) {
+void BinaryUploadService::InstanceIDUnregisteredCallback(
+    enterprise_connectors::AnalysisConnector connector,
+    bool) {
   // Calling RunAuthorizationCallbacks after the instance ID of the initial
   // authentication is unregistered avoids registration/unregistration conflicts
   // with normal requests.
   if (!authorization_callbacks_.empty() &&
-      can_upload_enterprise_data_.has_value()) {
-    RunAuthorizationCallbacks();
+      can_upload_enterprise_data_.contains(connector)) {
+    RunAuthorizationCallbacks(connector);
   }
 }
 
@@ -584,6 +590,11 @@ void BinaryUploadService::Request::set_email(const std::string& email) {
   content_analysis_request_.mutable_request_data()->set_email(email);
 }
 
+enterprise_connectors::AnalysisConnector
+BinaryUploadService::Request::analysis_connector() {
+  return content_analysis_request_.analysis_connector();
+}
+
 const std::string& BinaryUploadService::Request::device_token() const {
   return content_analysis_request_.device_token();
 }
@@ -672,8 +683,10 @@ inline void ValidateDataUploadRequest::GetRequestData(DataCallback callback) {
                           BinaryUploadService::Request::Data());
 }
 
-void BinaryUploadService::IsAuthorized(const GURL& url,
-                                       AuthorizationCallback callback) {
+void BinaryUploadService::IsAuthorized(
+    const GURL& url,
+    AuthorizationCallback callback,
+    enterprise_connectors::AnalysisConnector connector) {
   // Start |timer_| on the first call to IsAuthorized. This is necessary in
   // order to invalidate the authorization every 24 hours.
   if (!timer_.IsRunning()) {
@@ -683,14 +696,14 @@ void BinaryUploadService::IsAuthorized(const GURL& url,
                             weakptr_factory_.GetWeakPtr(), url));
   }
 
-  if (!can_upload_enterprise_data_.has_value()) {
+  if (!can_upload_enterprise_data_.contains(connector)) {
     // Send a request to check if the browser can upload data.
     authorization_callbacks_.push_back(std::move(callback));
     if (!pending_validate_data_upload_request_) {
       auto dm_token = policy::GetDMToken(profile_);
       if (!dm_token.is_valid()) {
-        can_upload_enterprise_data_ = false;
-        RunAuthorizationCallbacks();
+        can_upload_enterprise_data_[connector] = false;
+        RunAuthorizationCallbacks(connector);
         return;
       }
 
@@ -698,45 +711,58 @@ void BinaryUploadService::IsAuthorized(const GURL& url,
       auto request = std::make_unique<ValidateDataUploadRequest>(
           base::BindOnce(
               &BinaryUploadService::ValidateDataUploadRequestConnectorCallback,
-              weakptr_factory_.GetWeakPtr()),
+              weakptr_factory_.GetWeakPtr(), connector),
           url);
       request->set_device_token(dm_token.value());
+      request->set_analysis_connector(connector);
       UploadForDeepScanning(std::move(request));
     }
     return;
   }
-  std::move(callback).Run(can_upload_enterprise_data_.value());
+  std::move(callback).Run(can_upload_enterprise_data_[connector]);
 }
 
 void BinaryUploadService::ValidateDataUploadRequestConnectorCallback(
+    enterprise_connectors::AnalysisConnector connector,
     BinaryUploadService::Result result,
     enterprise_connectors::ContentAnalysisResponse response) {
   pending_validate_data_upload_request_ = false;
-  can_upload_enterprise_data_ = result == BinaryUploadService::Result::SUCCESS;
+  can_upload_enterprise_data_[connector] =
+      (result == BinaryUploadService::Result::SUCCESS);
 }
 
 void BinaryUploadService::ValidateDataUploadRequestCallback(
+    enterprise_connectors::AnalysisConnector connector,
     BinaryUploadService::Result result,
     DeepScanningClientResponse response) {
   pending_validate_data_upload_request_ = false;
-  can_upload_enterprise_data_ = result == BinaryUploadService::Result::SUCCESS;
+  can_upload_enterprise_data_[connector] =
+      (result == BinaryUploadService::Result::SUCCESS);
 }
 
-void BinaryUploadService::RunAuthorizationCallbacks() {
-  DCHECK(can_upload_enterprise_data_.has_value());
+void BinaryUploadService::RunAuthorizationCallbacks(
+    enterprise_connectors::AnalysisConnector connector) {
+  DCHECK(can_upload_enterprise_data_.contains(connector));
   for (auto& callback : authorization_callbacks_) {
-    std::move(callback).Run(can_upload_enterprise_data_.value());
+    std::move(callback).Run(can_upload_enterprise_data_[connector]);
   }
   authorization_callbacks_.clear();
 }
 
 void BinaryUploadService::ResetAuthorizationData(const GURL& url) {
-  // Setting |can_upload_enterprise_data_| to base::nullopt will make the next
+  // Clearing |can_upload_enterprise_data_| will make the next
   // call to IsAuthorized send out a request to validate data uploads.
-  can_upload_enterprise_data_ = base::nullopt;
+  can_upload_enterprise_data_.clear();
 
   // Call IsAuthorized  to update |can_upload_enterprise_data_| right away.
-  IsAuthorized(url, base::DoNothing());
+  for (enterprise_connectors::AnalysisConnector connector :
+       {enterprise_connectors::AnalysisConnector::
+            ANALYSIS_CONNECTOR_UNSPECIFIED,
+        enterprise_connectors::AnalysisConnector::FILE_DOWNLOADED,
+        enterprise_connectors::AnalysisConnector::FILE_ATTACHED,
+        enterprise_connectors::AnalysisConnector::BULK_DATA_ENTRY}) {
+    IsAuthorized(url, base::DoNothing(), connector);
+  }
 }
 
 void BinaryUploadService::Shutdown() {
@@ -745,7 +771,14 @@ void BinaryUploadService::Shutdown() {
 }
 
 void BinaryUploadService::SetAuthForTesting(bool authorized) {
-  can_upload_enterprise_data_ = authorized;
+  for (enterprise_connectors::AnalysisConnector connector :
+       {enterprise_connectors::AnalysisConnector::
+            ANALYSIS_CONNECTOR_UNSPECIFIED,
+        enterprise_connectors::AnalysisConnector::FILE_DOWNLOADED,
+        enterprise_connectors::AnalysisConnector::FILE_ATTACHED,
+        enterprise_connectors::AnalysisConnector::BULK_DATA_ENTRY}) {
+    can_upload_enterprise_data_[connector] = authorized;
+  }
 }
 
 // static

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h

@@ -157,6 +157,7 @@ class BinaryUploadService : public KeyedService {
     void clear_dlp_scan_request();
 
     // Methods for accessing the ContentAnalysisRequest.
+    enterprise_connectors::AnalysisConnector analysis_connector();
     const std::string& device_token() const;
     const std::string& request_token() const;
     const std::string& fcm_notification_token() const;
@@ -188,10 +189,13 @@ class BinaryUploadService : public KeyedService {
 
   // Indicates whether the browser is allowed to upload data.
   using AuthorizationCallback = base::OnceCallback<void(bool)>;
-  void IsAuthorized(const GURL& url, AuthorizationCallback callback);
+  void IsAuthorized(const GURL& url,
+                    AuthorizationCallback callback,
+                    enterprise_connectors::AnalysisConnector connector);
 
   // Run every callback in |authorization_callbacks_| and empty it.
-  void RunAuthorizationCallbacks();
+  void RunAuthorizationCallbacks(
+      enterprise_connectors::AnalysisConnector connector);
 
   // Resets |can_upload_data_|. Called every 24 hour by |timer_|.
   void ResetAuthorizationData(const GURL& url);
@@ -243,13 +247,18 @@ class BinaryUploadService : public KeyedService {
 
   // Callback once the response from the backend is received.
   void ValidateDataUploadRequestConnectorCallback(
+      enterprise_connectors::AnalysisConnector connector,
       BinaryUploadService::Result result,
       enterprise_connectors::ContentAnalysisResponse response);
-  void ValidateDataUploadRequestCallback(BinaryUploadService::Result result,
-                                         DeepScanningClientResponse response);
+  void ValidateDataUploadRequestCallback(
+      enterprise_connectors::AnalysisConnector connector,
+      BinaryUploadService::Result result,
+      DeepScanningClientResponse response);
 
   // Callback once a request's instance ID is unregistered.
-  void InstanceIDUnregisteredCallback(bool);
+  void InstanceIDUnregisteredCallback(
+      enterprise_connectors::AnalysisConnector connector,
+      bool);
 
   void RecordRequestMetrics(Request* request, Result result);
   void RecordRequestMetrics(
@@ -294,7 +303,8 @@ class BinaryUploadService : public KeyedService {
   // yet.
   // true means the response indicates data can be uploaded.
   // false means the response indicates data cannot be uploaded.
-  base::Optional<bool> can_upload_enterprise_data_ = base::nullopt;
+  base::flat_map<enterprise_connectors::AnalysisConnector, bool>
+      can_upload_enterprise_data_;
 
   // Callbacks waiting on IsAuthorized request.
   std::list<base::OnceCallback<void(bool)>> authorization_callbacks_;

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service_unittest.cc

@@ -486,7 +486,9 @@ TEST_F(BinaryUploadServiceTest, ReturnsAsynchronouslyWithNoFCM) {
 TEST_F(BinaryUploadServiceTest, IsAuthorizedValidTimer) {
   // The 24 hours timer should be started on the first IsAuthorized call.
   ValidateAuthorizationTimerIdle();
-  service_->IsAuthorized(GURL(), base::DoNothing());
+  service_->IsAuthorized(
+      GURL(), base::DoNothing(),
+      enterprise_connectors::AnalysisConnector::ANALYSIS_CONNECTOR_UNSPECIFIED);
   ValidateAuthorizationTimerStarted();
 }