Attach token only when enhance protection users are signed in.

It is possible that users are not signed in but enabled enhanced
protection. In this case, the access token is not available. Add a
signin status check in this CL.

Bug: 1041912
Change-Id: I1095a3d26784c6ba4bcf8a768bbed064ae39088e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2138164
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756907}

components/safe_browsing/core/realtime/BUILD.gn

@@ -38,6 +38,7 @@ static_library("policy_engine") {
     "//components/safe_browsing/core:realtimeapi_proto",
     "//components/safe_browsing/core/common",
     "//components/safe_browsing/core/common:safe_browsing_prefs",
+    "//components/signin/public/identity_manager",
     "//components/sync",
     "//components/unified_consent",
     "//components/user_prefs",

components/safe_browsing/core/realtime/policy_engine.cc

@@ -11,6 +11,9 @@
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/safe_browsing/core/common/safebrowsing_constants.h"
 #include "components/safe_browsing/core/features.h"
+#include "components/signin/public/identity_manager/account_info.h"
+#include "components/signin/public/identity_manager/consent_level.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_service_utils.h"
@@ -67,6 +70,15 @@ bool RealTimePolicyEngine::IsUserEpOptedIn(PrefService* pref_service) {
   return IsEnhancedProtectionEnabled(*pref_service);
 }
 
+// static
+bool RealTimePolicyEngine::IsPrimaryAccountSignedIn(
+    signin::IdentityManager* identity_manager) {
+  CoreAccountInfo primary_account_info =
+      identity_manager->GetPrimaryAccountInfo(
+          signin::ConsentLevel::kNotRequired);
+  return !primary_account_info.account_id.empty();
+}
+
 // static
 bool RealTimePolicyEngine::CanPerformFullURLLookup(PrefService* pref_service,
                                                    bool is_off_the_record) {
@@ -83,13 +95,16 @@ bool RealTimePolicyEngine::CanPerformFullURLLookup(PrefService* pref_service,
 bool RealTimePolicyEngine::CanPerformFullURLLookupWithToken(
     PrefService* pref_service,
     bool is_off_the_record,
-    syncer::SyncService* sync_service) {
+    syncer::SyncService* sync_service,
+    signin::IdentityManager* identity_manager) {
   if (!CanPerformFullURLLookup(pref_service, is_off_the_record)) {
     return false;
   }
 
-  if (IsUrlLookupEnabledForEp() && IsUserEpOptedIn(pref_service))
+  if (IsUrlLookupEnabledForEp() && IsUserEpOptedIn(pref_service) &&
+      IsPrimaryAccountSignedIn(identity_manager)) {
     return true;
+  }
 
   if (!base::FeatureList::IsEnabled(kRealTimeUrlLookupEnabledWithToken)) {
     return false;

components/safe_browsing/core/realtime/policy_engine.h

@@ -13,6 +13,10 @@ namespace syncer {
 class SyncService;
 }
 
+namespace signin {
+class IdentityManager;
+}
+
 namespace safe_browsing {
 
 enum class ResourceType;
@@ -51,7 +55,8 @@ class RealTimePolicyEngine {
   static bool CanPerformFullURLLookupWithToken(
       PrefService* pref_service,
       bool is_off_the_record,
-      syncer::SyncService* sync_service);
+      syncer::SyncService* sync_service,
+      signin::IdentityManager* identity_manager);
 
   friend class SafeBrowsingService;
   friend class SafeBrowsingUIHandler;
@@ -70,6 +75,10 @@ class RealTimePolicyEngine {
   // Whether the user has opted-in to Enhanced Protection.
   static bool IsUserEpOptedIn(PrefService* pref_service);
 
+  // Whether the primary account is signed in. Sync is not required.
+  static bool IsPrimaryAccountSignedIn(
+      signin::IdentityManager* identity_manager);
+
   friend class RealTimePolicyEngineTest;
 };  // class RealTimePolicyEngine
 

components/safe_browsing/core/realtime/policy_engine_unittest.cc

@@ -10,6 +10,7 @@
 #include "components/safe_browsing/core/common/safebrowsing_constants.h"
 #include "components/safe_browsing/core/common/test_task_environment.h"
 #include "components/safe_browsing/core/features.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "components/sync/driver/test_sync_service.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/unified_consent/pref_names.h"
@@ -43,10 +44,12 @@ class RealTimePolicyEngineTest : public PlatformTest {
                                                          is_off_the_record);
   }
 
-  bool CanPerformFullURLLookupWithToken(bool is_off_the_record,
-                                        syncer::SyncService* sync_service) {
+  bool CanPerformFullURLLookupWithToken(
+      bool is_off_the_record,
+      syncer::SyncService* sync_service,
+      signin::IdentityManager* identity_manager) {
     return RealTimePolicyEngine::CanPerformFullURLLookupWithToken(
-        &pref_service_, is_off_the_record, sync_service);
+        &pref_service_, is_off_the_record, sync_service, identity_manager);
   }
 
   std::unique_ptr<base::test::TaskEnvironment> task_environment_;
@@ -224,38 +227,58 @@ TEST_F(RealTimePolicyEngineTest,
   pref_service_.SetUserPref(
       unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled,
       std::make_unique<base::Value>(true));
+  std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env =
+      std::make_unique<signin::IdentityTestEnvironment>();
+  signin::IdentityManager* identity_manager =
+      identity_test_env->identity_manager();
   syncer::TestSyncService sync_service;
 
   // Sync is disabled.
   sync_service.SetDisableReasons(
       {syncer::SyncService::DISABLE_REASON_USER_CHOICE});
   sync_service.SetTransportState(syncer::SyncService::TransportState::DISABLED);
-  EXPECT_FALSE(CanPerformFullURLLookupWithToken(/* is_off_the_record */ false,
-                                                &sync_service));
+  EXPECT_FALSE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
+
+  // Sync is enabled.
+  sync_service.SetDisableReasons({});
+  sync_service.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
+  EXPECT_TRUE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
 
   // History sync is disabled.
   sync_service.GetUserSettings()->SetSelectedTypes(
       /* sync_everything */ false, {});
-  EXPECT_FALSE(CanPerformFullURLLookupWithToken(/* is_off_the_record */ false,
-                                                &sync_service));
+  EXPECT_FALSE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
 
   // Custom passphrase is enabled.
   sync_service.GetUserSettings()->SetSelectedTypes(
       false, {syncer::UserSelectableType::kHistory});
   sync_service.SetIsUsingSecondaryPassphrase(true);
-  EXPECT_FALSE(CanPerformFullURLLookupWithToken(/* is_off_the_record */ false,
-                                                &sync_service));
+  EXPECT_FALSE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
 }
 
 TEST_F(RealTimePolicyEngineTest,
        TestCanPerformFullURLLookupWithToken_EnhancedProtection) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(kEnhancedProtection);
+  std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env =
+      std::make_unique<signin::IdentityTestEnvironment>();
+  signin::IdentityManager* identity_manager =
+      identity_test_env->identity_manager();
   syncer::TestSyncService sync_service;
-  // Only enhanced protection is on.
+
+  // Enhanced protection is on but user is not signed in.
   pref_service_.SetBoolean(prefs::kSafeBrowsingEnhanced, true);
-  EXPECT_TRUE(CanPerformFullURLLookupWithToken(/* is_off_the_record */ false,
-                                               &sync_service));
+  EXPECT_FALSE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
+
+  // User is signed in.
+  identity_test_env->MakeUnconsentedPrimaryAccountAvailable("test@example.com");
+  EXPECT_TRUE(CanPerformFullURLLookupWithToken(
+      /* is_off_the_record */ false, &sync_service, identity_manager));
 
   // Sync and history sync is disabled but enhanced protection is enabled.
   sync_service.SetDisableReasons(
@@ -263,8 +286,8 @@ TEST_F(RealTimePolicyEngineTest,
   sync_service.SetTransportState(syncer::SyncService::TransportState::DISABLED);
   sync_service.GetUserSettings()->SetSelectedTypes(
       /* sync_everything */ false, {});
-  EXPECT_TRUE(CanPerformFullURLLookupWithToken(/*is_off_the_record=*/false,
-                                               &sync_service));
+  EXPECT_TRUE(CanPerformFullURLLookupWithToken(
+      /*is_off_the_record=*/false, &sync_service, identity_manager));
 }
 
 TEST_F(RealTimePolicyEngineTest,

components/safe_browsing/core/realtime/url_lookup_service.cc

@@ -407,7 +407,7 @@ bool RealTimeUrlLookupService::CanPerformFullURLLookup() const {
 
 bool RealTimeUrlLookupService::CanPerformFullURLLookupWithToken() const {
   return RealTimePolicyEngine::CanPerformFullURLLookupWithToken(
-      pref_service_, is_off_the_record_, sync_service_);
+      pref_service_, is_off_the_record_, sync_service_, identity_manager_);
 }
 
 bool RealTimeUrlLookupService::IsUserEpOptedIn() const {