Verify browser can upload data.

Added IsAuthorized to BinaryUploadService. Makes UploadForDeepScanning private and instead exposes MaybeUploadForDeepScanning, which calls IsAuthorized before UploadForDeepScanning.

Bug: 1014926, 1014927
Change-Id: I2ae991569a171eeeef447ff8a47f41f9ed99460f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865412
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709083}

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

@@ -8,8 +8,10 @@
 #include <memory>
 #include <string>
 
+#include "base/callback_forward.h"
 #include "base/feature_list.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -30,9 +32,11 @@ class GURL;
 
 namespace policy {
 class CloudPolicyClient;
+class DeviceManagementService;
 }
 
 namespace safe_browsing {
+class BinaryUploadService;
 class DlpDeepScanningVerdict;
 }
 
@@ -160,12 +164,28 @@ class SafeBrowsingPrivateEventRouter : public KeyedService {
   void SetCloudPolicyClientForTesting(
       std::unique_ptr<policy::CloudPolicyClient> client);
 
+ protected:
+  // Callback to report safe browsing event through real-time reporting channel,
+  // if the browser is authorized to do so. Declared as protected to be called
+  // directly by tests. Events are created lazily to avoid doing useless work if
+  // they are discarded.
+  using EventBuilder = base::OnceCallback<base::Value()>;
+  void ReportRealtimeEventCallback(const std::string& name,
+                                   EventBuilder event_builder,
+                                   bool authorized);
+
  private:
   // Initialize the real-time report client if needed.  This client is used only
   // if real-time reporting is enabled, the machine is properly reigistered
   // with CBCM and the appropriate policies are enabled.
   void InitRealtimeReportingClient();
 
+  // Initialize DeviceManagementService and |client_| after validating the
+  // browser can upload data.
+  void InitRealtimeReportingClientCallback(
+      policy::DeviceManagementService* device_management_service,
+      bool authorized);
+
   // Determines if the real-time reporting feature is enabled.
   bool IsRealtimeReportingEnabled();
 
@@ -173,7 +193,9 @@ class SafeBrowsingPrivateEventRouter : public KeyedService {
   void RealtimeReportingPrefChanged(const std::string& pref);
 
   // Report safe browsing event through real-time reporting channel, if enabled.
-  void ReportRealtimeEvent(const char* name, base::Value event);
+  // Declared as virtual for tests.
+  virtual void ReportRealtimeEvent(const std::string&,
+                                   EventBuilder event_builder);
 
   // Returns the Gaia email address of the account signed in to the profile or
   // an empty string if the profile is not signed in.
@@ -182,9 +204,11 @@ class SafeBrowsingPrivateEventRouter : public KeyedService {
   content::BrowserContext* context_;
   signin::IdentityManager* identity_manager_ = nullptr;
   EventRouter* event_router_ = nullptr;
+  safe_browsing::BinaryUploadService* binary_upload_service_;
   std::unique_ptr<policy::CloudPolicyClient> client_;
   PrefChangeRegistrar registrar_;
 
+  base::WeakPtrFactory<SafeBrowsingPrivateEventRouter> weakptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingPrivateEventRouter);
 };
 

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

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 #include "chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
@@ -35,6 +37,34 @@ ACTION_P(CaptureArg, wrapper) {
 
 }  // namespace
 
+class FakeAuthorizedSafeBrowsingPrivateEventRouter
+    : public SafeBrowsingPrivateEventRouter {
+ public:
+  explicit FakeAuthorizedSafeBrowsingPrivateEventRouter(
+      content::BrowserContext* context)
+      : SafeBrowsingPrivateEventRouter(context) {}
+
+ private:
+  void ReportRealtimeEvent(const std::string& name,
+                           EventBuilder event_builder) override {
+    ReportRealtimeEventCallback(name, std::move(event_builder), true);
+  }
+};
+
+class FakeUnauthorizedSafeBrowsingPrivateEventRouter
+    : public SafeBrowsingPrivateEventRouter {
+ public:
+  explicit FakeUnauthorizedSafeBrowsingPrivateEventRouter(
+      content::BrowserContext* context)
+      : SafeBrowsingPrivateEventRouter(context) {}
+
+ private:
+  void ReportRealtimeEvent(const std::string& name,
+                           EventBuilder event_builder) override {
+    ReportRealtimeEventCallback(name, std::move(event_builder), false);
+  }
+};
+
 class SafeBrowsingEventObserver : public TestEventRouter::EventObserver {
  public:
   // The observer will only listen to events with the |event_name|.
@@ -64,9 +94,14 @@ class SafeBrowsingEventObserver : public TestEventRouter::EventObserver {
 };
 
 std::unique_ptr<KeyedService> BuildSafeBrowsingPrivateEventRouter(
+    bool authorized,
     content::BrowserContext* context) {
-  return std::unique_ptr<KeyedService>(
-      new SafeBrowsingPrivateEventRouter(context));
+  if (authorized)
+    return std::unique_ptr<KeyedService>(
+        new FakeAuthorizedSafeBrowsingPrivateEventRouter(context));
+  else
+    return std::unique_ptr<KeyedService>(
+        new FakeUnauthorizedSafeBrowsingPrivateEventRouter(context));
 }
 
 class SafeBrowsingPrivateEventRouterTest : public testing::Test {
@@ -142,10 +177,12 @@ class SafeBrowsingPrivateEventRouterTest : public testing::Test {
         ->SetCloudPolicyClientForTesting(std::move(client));
   }
 
-  void SetUpRouters(bool realtime_reporting_enable = true) {
+  void SetUpRouters(bool realtime_reporting_enable = true,
+                    bool authorized = true) {
     event_router_ = extensions::CreateAndUseTestEventRouter(profile_);
     SafeBrowsingPrivateEventRouterFactory::GetInstance()->SetTestingFactory(
-        profile_, base::BindRepeating(&BuildSafeBrowsingPrivateEventRouter));
+        profile_,
+        base::BindRepeating(&BuildSafeBrowsingPrivateEventRouter, authorized));
 
     SetReportingPolicy(realtime_reporting_enable);
   }
@@ -475,4 +512,122 @@ TEST_F(SafeBrowsingPrivateEventRouterTest, PolicyControlOffToOnIsDynamic) {
   Mock::VerifyAndClearExpectations(client_);
 }
 
+TEST_F(SafeBrowsingPrivateEventRouterTest, TestUnauthorizedOnReuseDetected) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnPolicySpecifiedPasswordReuseDetected::
+          kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnPolicySpecifiedPasswordReuseDetectedEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest, TestUnauthorizedOnPasswordChanged) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnPolicySpecifiedPasswordChanged::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnPolicySpecifiedPasswordChangedEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest,
+       TestUnauthorizedOnDangerousDownloadOpened) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnDangerousDownloadOpened::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnDangerousDownloadOpenedEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest,
+       TestUnauthorizedOnSecurityInterstitialProceeded) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnSecurityInterstitialProceeded::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnSecurityInterstitialProceededEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest,
+       TestUnauthorizedOnSecurityInterstitialShown) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnSecurityInterstitialShown::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnSecurityInterstitialShownEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest,
+       TestUnauthorizedOnDangerousDownloadWarning) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnDangerousDownloadOpened::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnDangerousDownloadWarningEvent();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
+TEST_F(SafeBrowsingPrivateEventRouterTest,
+       TestUnauthorizedOnDangerousDownloadWarningBypass) {
+  SetUpRouters(/*realtime_reporting_enable=*/true, /*authorized=*/false);
+  SafeBrowsingEventObserver event_observer(
+      api::safe_browsing_private::OnDangerousDownloadOpened::kEventName);
+  event_router_->AddEventObserver(&event_observer);
+
+  base::Value report;
+  EXPECT_CALL(*client_, UploadRealtimeReport(_, _)).Times(0);
+
+  TriggerOnDangerousDownloadWarningEventBypass();
+  base::RunLoop().RunUntilIdle();
+
+  Mock::VerifyAndClearExpectations(client_);
+  EXPECT_EQ(base::Value::Type::NONE, report.type());
+}
+
 }  // namespace extensions

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -5,9 +5,9 @@
 #include "chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.h"
 
 #include <algorithm>
-#include <memory>
 #include <numeric>
 #include <string>
+#include <utility>
 
 #include "base/files/file_path.h"
 #include "base/memory/weak_ptr.h"
@@ -567,7 +567,7 @@ void DeepScanningDialogDelegate::UploadTextForDeepScanning(
       g_browser_process->safe_browsing_service()->GetBinaryUploadService(
           Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
   if (upload_service)
-    upload_service->UploadForDeepScanning(std::move(request));
+    upload_service->MaybeUploadForDeepScanning(std::move(request));
 }
 
 void DeepScanningDialogDelegate::UploadFileForDeepScanning(
@@ -580,7 +580,7 @@ void DeepScanningDialogDelegate::UploadFileForDeepScanning(
       g_browser_process->safe_browsing_service()->GetBinaryUploadService(
           Profile::FromBrowserContext(web_contents_->GetBrowserContext()));
   if (upload_service)
-    upload_service->UploadForDeepScanning(std::move(request));
+    upload_service->MaybeUploadForDeepScanning(std::move(request));
 }
 
 bool DeepScanningDialogDelegate::CloseTabModalDialog() {

chrome/browser/safe_browsing/download_protection/binary_upload_service.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/safe_browsing/download_protection/binary_upload_service.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/base64.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
@@ -15,6 +17,8 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/policy/browser_dm_token_storage.h"
+#include "chrome/browser/policy/chrome_browser_cloud_management_controller.h"
 #include "chrome/browser/safe_browsing/download_protection/binary_fcm_service.h"
 #include "chrome/browser/safe_browsing/download_protection/multipart_uploader.h"
 #include "components/prefs/pref_service.h"
@@ -31,6 +35,29 @@ const int kScanningTimeoutSeconds = 5 * 60;           // 5 minutes
 const char kSbBinaryUploadUrl[] =
     "https://safebrowsing.google.com/safebrowsing/uploads/webprotect";
 
+std::string* GetDMTokenForTestingStorage() {
+  static std::string dm_token;
+  return &dm_token;
+}
+
+std::string GetDMToken() {
+  std::string dm_token = *GetDMTokenForTestingStorage();
+
+#if !defined(OS_CHROMEOS)
+  // This is not compiled on chromeos because
+  // ChromeBrowserCloudManagementController does not exist.  Also,
+  // policy::BrowserDMTokenStorage::Get()->RetrieveDMToken() does not return a
+  // valid token either.  Once these are fixed the #if !defined can be removed.
+
+  if (dm_token.empty() &&
+      policy::ChromeBrowserCloudManagementController::IsEnabled()) {
+    dm_token = policy::BrowserDMTokenStorage::Get()->RetrieveDMToken();
+  }
+#endif
+
+  return dm_token;
+}
+
 }  // namespace
 
 BinaryUploadService::BinaryUploadService(
@@ -49,6 +76,29 @@ BinaryUploadService::BinaryUploadService(
 
 BinaryUploadService::~BinaryUploadService() {}
 
+void BinaryUploadService::MaybeUploadForDeepScanning(
+    std::unique_ptr<BinaryUploadService::Request> request) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!can_upload_data_.has_value()) {
+    IsAuthorized(
+        base::BindOnce(&BinaryUploadService::MaybeUploadForDeepScanningCallback,
+                       weakptr_factory_.GetWeakPtr(), std::move(request)));
+    return;
+  }
+
+  MaybeUploadForDeepScanningCallback(std::move(request),
+                                     can_upload_data_.value());
+}
+
+void BinaryUploadService::MaybeUploadForDeepScanningCallback(
+    std::unique_ptr<BinaryUploadService::Request> request,
+    bool authorized) {
+  // Ignore the request if the browser cannot upload data.
+  if (!authorized)
+    return;
+  UploadForDeepScanning(std::move(request));
+}
+
 void BinaryUploadService::UploadForDeepScanning(
     std::unique_ptr<BinaryUploadService::Request> request) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -300,6 +350,80 @@ bool BinaryUploadService::IsActive(Request* request) {
   return (active_requests_.find(request) != active_requests_.end());
 }
 
+class ValidateDataUploadRequest : public BinaryUploadService::Request {
+ public:
+  explicit ValidateDataUploadRequest(BinaryUploadService::Callback callback)
+      : BinaryUploadService::Request(std::move(callback)) {}
+  ValidateDataUploadRequest(const ValidateDataUploadRequest&) = delete;
+  ValidateDataUploadRequest& operator=(const ValidateDataUploadRequest&) =
+      delete;
+  ~ValidateDataUploadRequest() override = default;
+
+ private:
+  // BinaryUploadService::Request implementation.
+  void GetRequestData(DataCallback callback) override;
+};
+
+inline void ValidateDataUploadRequest::GetRequestData(DataCallback callback) {
+  std::move(callback).Run(BinaryUploadService::Result::SUCCESS,
+                          BinaryUploadService::Request::Data());
+}
+
+void BinaryUploadService::IsAuthorized(AuthorizationCallback callback) {
+  // Start |timer_| on the first call to IsAuthorized. This is necessary in
+  // order to invalidate the authorization every 24 hours.
+  if (!timer_.IsRunning()) {
+    timer_.Start(FROM_HERE, base::TimeDelta::FromHours(24), this,
+                 &BinaryUploadService::ResetAuthorizationData);
+  }
+
+  if (!can_upload_data_.has_value()) {
+    // Send a request to check if the browser can upload data.
+    if (!pending_validate_data_upload_request_) {
+      std::string dm_token = GetDMToken();
+      if (dm_token.empty()) {
+        std::move(callback).Run(false);
+        return;
+      }
+
+      pending_validate_data_upload_request_ = true;
+      auto request = std::make_unique<ValidateDataUploadRequest>(base::BindOnce(
+          &BinaryUploadService::ValidateDataUploadRequestCallback,
+          weakptr_factory_.GetWeakPtr()));
+      request->set_dm_token(dm_token);
+      UploadForDeepScanning(std::move(request));
+    }
+    authorization_callbacks_.push_back(std::move(callback));
+    return;
+  }
+  std::move(callback).Run(can_upload_data_.value());
+}
+
+void BinaryUploadService::ValidateDataUploadRequestCallback(
+    BinaryUploadService::Result result,
+    DeepScanningClientResponse response) {
+  pending_validate_data_upload_request_ = false;
+  can_upload_data_ = result == BinaryUploadService::Result::SUCCESS;
+  RunAuthorizationCallbacks();
+}
+
+void BinaryUploadService::RunAuthorizationCallbacks() {
+  DCHECK(can_upload_data_.has_value());
+  for (auto& callback : authorization_callbacks_) {
+    std::move(callback).Run(can_upload_data_.value());
+  }
+  authorization_callbacks_.clear();
+}
+
+void BinaryUploadService::ResetAuthorizationData() {
+  // Setting |can_upload_data_| to base::nullopt will make the next call to
+  // IsAuthorized send out a request to validate data uploads.
+  can_upload_data_ = base::nullopt;
+
+  // Call IsAuthorized  to update |can_upload_data_| right away.
+  IsAuthorized(base::DoNothing());
+}
+
 // static
 bool BinaryUploadService::ShouldBlockFileSize(size_t file_size) {
   int block_large_file_transfer = g_browser_process->local_state()->GetInteger(

chrome/browser/safe_browsing/download_protection/binary_upload_service.h

@@ -5,13 +5,18 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_BINARY_UPLOAD_SERVICE_H_
 #define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_BINARY_UPLOAD_SERVICE_H_
 
+#include <list>
 #include <memory>
+#include <string>
 #include <unordered_map>
 
 #include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "base/timer/timer.h"
 #include "chrome/browser/safe_browsing/download_protection/binary_fcm_service.h"
 #include "chrome/browser/safe_browsing/download_protection/multipart_uploader.h"
 #include "components/safe_browsing/proto/webprotect.pb.h"
@@ -114,10 +119,19 @@ class BinaryUploadService {
     Callback callback_;
   };
 
-  // Upload the given file contents for deep scanning. The results will be
-  // returned asynchronously by calling |request|'s |callback|. This must be
-  // called on the UI thread.
-  void UploadForDeepScanning(std::unique_ptr<Request> request);
+  // Upload the given file contents for deep scanning if the browser is
+  // authorized to upload data, otherwise queue the request.
+  void MaybeUploadForDeepScanning(std::unique_ptr<Request> request);
+
+  // Indicates whether the browser is allowed to upload data.
+  using AuthorizationCallback = base::OnceCallback<void(bool)>;
+  void IsAuthorized(AuthorizationCallback callback);
+
+  // Run every callback in |authorization_callbacks_| and empty it.
+  void RunAuthorizationCallbacks();
+
+  // Resets |can_upload_data_|. Called every 24 hour by |timer_|.
+  void ResetAuthorizationData();
 
   // Returns whether a download should be blocked based on file size alone. It
   // checks the enterprise policy BlockLargeFileTransfer to decide this.
@@ -126,6 +140,11 @@ class BinaryUploadService {
  private:
   friend class BinaryUploadServiceTest;
 
+  // Upload the given file contents for deep scanning. The results will be
+  // returned asynchronously by calling |request|'s |callback|. This must be
+  // called on the UI thread.
+  void UploadForDeepScanning(std::unique_ptr<Request> request);
+
   void OnGetInstanceID(Request* request, const std::string& token);
 
   void OnGetRequestData(Request* request,
@@ -148,6 +167,13 @@ class BinaryUploadService {
 
   bool IsActive(Request* request);
 
+  void MaybeUploadForDeepScanningCallback(std::unique_ptr<Request> request,
+                                          bool authorized);
+
+  // Callback once the response from the backend is received.
+  void ValidateDataUploadRequestCallback(BinaryUploadService::Result result,
+                                         DeepScanningClientResponse response);
+
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
   std::unique_ptr<BinaryFCMService> binary_fcm_service_;
 
@@ -162,6 +188,24 @@ class BinaryUploadService {
   base::flat_map<Request*, std::unique_ptr<DlpDeepScanningVerdict>>
       received_dlp_verdicts_;
 
+  // Indicates whether this browser can upload data.
+  // base::nullopt means the response from the backend has not been received
+  // yet.
+  // true means the response indicates data can be uploaded.
+  // false means the response indicates data cannot be uploaded.
+  base::Optional<bool> can_upload_data_ = base::nullopt;
+
+  // Callbacks waiting on IsAuthorized request.
+  std::list<base::OnceCallback<void(bool)>> authorization_callbacks_;
+
+  // Indicates if this service is waiting on the backend to validate event
+  // reporting. Used to avoid spamming the backend.
+  bool pending_validate_data_upload_request_ = false;
+
+  // Ensures we validate the browser is registered with the backend every 24
+  // hours.
+  base::RepeatingTimer timer_;
+
   base::WeakPtrFactory<BinaryUploadService> weakptr_factory_;
 };
 

chrome/browser/safe_browsing/download_protection/binary_upload_service_unittest.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/safe_browsing/download_protection/binary_upload_service.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/callback_forward.h"
+#include "base/time/time.h"
 #include "chrome/browser/safe_browsing/download_protection/binary_fcm_service.h"
 #include "chrome/browser/safe_browsing/download_protection/multipart_uploader.h"
 #include "components/safe_browsing/proto/webprotect.pb.h"
@@ -118,6 +120,13 @@ class BinaryUploadServiceTest : public testing::Test {
             }));
   }
 
+  void UploadForDeepScanning(
+      std::unique_ptr<BinaryUploadService::Request> request,
+      bool authorized = true) {
+    service_->MaybeUploadForDeepScanningCallback(std::move(request),
+                                                 authorized);
+  }
+
   void ReceiveMessageForRequest(BinaryUploadService::Request* request,
                                 const DeepScanningClientResponse& response) {
     service_->OnGetResponse(request, response);
@@ -129,6 +138,11 @@ class BinaryUploadServiceTest : public testing::Test {
     service_->OnUploadComplete(request, success, response);
   }
 
+  void ServiceWithNoFCMConnection() {
+    service_ = std::make_unique<BinaryUploadService>(
+        nullptr, std::unique_ptr<BinaryFCMService>(nullptr));
+  }
+
   std::unique_ptr<MockRequest> MakeRequest(
       BinaryUploadService::Result* scanning_result,
       DeepScanningClientResponse* scanning_response) {
@@ -152,6 +166,18 @@ class BinaryUploadServiceTest : public testing::Test {
     return request;
   }
 
+  void ValidateAuthorizationTimerIdle() {
+    EXPECT_FALSE(service_->timer_.IsRunning());
+    EXPECT_EQ(base::TimeDelta::FromHours(0),
+              service_->timer_.GetCurrentDelay());
+  }
+
+  void ValidateAuthorizationTimerStarted() {
+    EXPECT_TRUE(service_->timer_.IsRunning());
+    EXPECT_EQ(base::TimeDelta::FromHours(24),
+              service_->timer_.GetCurrentDelay());
+  }
+
  protected:
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<BinaryUploadService> service_;
@@ -173,7 +199,7 @@ TEST_F(BinaryUploadServiceTest, FailsForLargeFile) {
             std::move(callback).Run(BinaryUploadService::Result::FILE_TOO_LARGE,
                                     data);
           }));
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
 
   content::RunAllTasksUntilIdle();
 
@@ -189,7 +215,7 @@ TEST_F(BinaryUploadServiceTest, FailsWhenMissingInstanceID) {
 
   ExpectInstanceID(BinaryFCMService::kInvalidId);
 
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
 
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::FAILED_TO_GET_TOKEN);
@@ -204,7 +230,7 @@ TEST_F(BinaryUploadServiceTest, FailsWhenUploadFails) {
   ExpectInstanceID("valid id");
   ExpectNetworkResponse(false, DeepScanningClientResponse());
 
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
 
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::UPLOAD_FAILURE);
@@ -223,7 +249,7 @@ TEST_F(BinaryUploadServiceTest, HoldsScanResponsesUntilAllReady) {
   ExpectNetworkResponse(true, DeepScanningClientResponse());
 
   MockRequest* raw_request = request.get();
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
 
   // Simulate receiving the DLP response
@@ -255,7 +281,7 @@ TEST_F(BinaryUploadServiceTest, TimesOut) {
 
   ExpectInstanceID("valid id");
   ExpectNetworkResponse(true, DeepScanningClientResponse());
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
   task_environment_.FastForwardUntilNoTasksRemain();
 
@@ -280,7 +306,7 @@ TEST_F(BinaryUploadServiceTest, OnInstanceIDAfterTimeout) {
           }));
 
   ExpectNetworkResponse(true, DeepScanningClientResponse());
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
   task_environment_.FastForwardUntilNoTasksRemain();
 
@@ -304,7 +330,7 @@ TEST_F(BinaryUploadServiceTest, OnUploadCompleteAfterTimeout) {
   ExpectNetworkResponse(true, DeepScanningClientResponse());
 
   MockRequest* raw_request = request.get();
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
   task_environment_.FastForwardUntilNoTasksRemain();
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::TIMEOUT);
@@ -327,7 +353,7 @@ TEST_F(BinaryUploadServiceTest, OnGetResponseAfterTimeout) {
   ExpectNetworkResponse(true, DeepScanningClientResponse());
 
   MockRequest* raw_request = request.get();
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
   task_environment_.FastForwardUntilNoTasksRemain();
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::TIMEOUT);
@@ -337,6 +363,31 @@ TEST_F(BinaryUploadServiceTest, OnGetResponseAfterTimeout) {
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::TIMEOUT);
 }
 
+TEST_F(BinaryUploadServiceTest, OnUnauthorized) {
+  BinaryUploadService::Result scanning_result =
+      BinaryUploadService::Result::UNKNOWN;
+  DeepScanningClientResponse scanning_response;
+  std::unique_ptr<MockRequest> request =
+      MakeRequest(&scanning_result, &scanning_response);
+  request->set_request_dlp_scan(DlpDeepScanningClientRequest());
+  request->set_request_malware_scan(MalwareDeepScanningClientRequest());
+
+  ExpectInstanceID("valid id");
+
+  DeepScanningClientResponse simulated_response;
+  simulated_response.mutable_dlp_scan_verdict();
+  simulated_response.mutable_malware_scan_verdict();
+  ExpectNetworkResponse(true, simulated_response);
+
+  UploadForDeepScanning(std::move(request), /*authorized=*/false);
+
+  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
+
+  content::RunAllTasksUntilIdle();
+
+  EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
+}
+
 TEST_F(BinaryUploadServiceTest, OnGetSynchronousResponse) {
   BinaryUploadService::Result scanning_result =
       BinaryUploadService::Result::UNKNOWN;
@@ -353,16 +404,14 @@ TEST_F(BinaryUploadServiceTest, OnGetSynchronousResponse) {
   simulated_response.mutable_malware_scan_verdict();
   ExpectNetworkResponse(true, simulated_response);
 
-  service_->UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
   content::RunAllTasksUntilIdle();
 
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::SUCCESS);
 }
 
 TEST_F(BinaryUploadServiceTest, ReturnsAsynchronouslyWithNoFCM) {
-  // Instantiate a BinaryUploadService with no FCM connection.
-  BinaryUploadService service(nullptr,
-                              std::unique_ptr<BinaryFCMService>(nullptr));
+  ServiceWithNoFCMConnection();
 
   BinaryUploadService::Result scanning_result =
       BinaryUploadService::Result::UNKNOWN;
@@ -372,7 +421,7 @@ TEST_F(BinaryUploadServiceTest, ReturnsAsynchronouslyWithNoFCM) {
   request->set_request_dlp_scan(DlpDeepScanningClientRequest());
   request->set_request_malware_scan(MalwareDeepScanningClientRequest());
 
-  service.UploadForDeepScanning(std::move(request));
+  UploadForDeepScanning(std::move(request));
 
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::UNKNOWN);
 
@@ -381,4 +430,11 @@ TEST_F(BinaryUploadServiceTest, ReturnsAsynchronouslyWithNoFCM) {
   EXPECT_EQ(scanning_result, BinaryUploadService::Result::FAILED_TO_GET_TOKEN);
 }
 
+TEST_F(BinaryUploadServiceTest, IsAuthorizedValidTimer) {
+  // The 24 hours timer should be started on the first IsAuthorized call.
+  ValidateAuthorizationTimerIdle();
+  service_->IsAuthorized(base::DoNothing());
+  ValidateAuthorizationTimerStarted();
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/download_protection/download_protection_service.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/safe_browsing/download_protection/download_protection_service.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
@@ -546,7 +548,7 @@ void DownloadProtectionService::UploadForDeepScanning(
     Profile* profile,
     std::unique_ptr<BinaryUploadService::Request> request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  sb_service_->GetBinaryUploadService(profile)->UploadForDeepScanning(
+  sb_service_->GetBinaryUploadService(profile)->MaybeUploadForDeepScanning(
       std::move(request));
 }