Detect sync password reuse for incognito profile

SignInManager considers incognito profile as not signed-in. As a result, when
user reuses their sync password in incognito profile, the reuse password type
is mis-identified as OTHER_GAIA_PASSWORD.

This CL makes incognito profile aware of its original profile's sync username
such that sync password reuse detection can work correctly in incognito mode.
This CL also makes it more explicit that Chrome does NOT save any password
hashes if the user is in incognito mode to prevent leaking any browsing
history to original profile.

Change-Id: Ie4398a85e4a13045f418cebd38313bb32c4bb6fd
Bug: 893715
Reviewed-on: https://chromium-review.googlesource.com/c/1273993
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599239}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -121,8 +121,8 @@ const syncer::SyncService* GetSyncService(Profile* profile) {
   return nullptr;
 }
 
-const SigninManagerBase* GetSigninManager(Profile* profile) {
-  return SigninManagerFactory::GetForProfile(profile);
+const SigninManagerBase* GetSigninManagerForOriginalProfile(Profile* profile) {
+  return SigninManagerFactory::GetForProfile(profile->GetOriginalProfile());
 }
 
 #if !defined(OS_ANDROID)
@@ -167,9 +167,10 @@ ChromePasswordManagerClient::ChromePasswordManagerClient(
       password_manager_client_bindings_(web_contents, this),
       password_manager_driver_bindings_(web_contents, this),
       observer_(nullptr),
-      credentials_filter_(this,
-                          base::Bind(&GetSyncService, profile_),
-                          base::Bind(&GetSigninManager, profile_)),
+      credentials_filter_(
+          this,
+          base::BindRepeating(&GetSyncService, profile_),
+          base::BindRepeating(&GetSigninManagerForOriginalProfile, profile_)),
       helper_(this) {
   ContentPasswordManagerDriverFactory::CreateForWebContents(web_contents, this,
                                                             autofill_client);
@@ -186,7 +187,7 @@ ChromePasswordManagerClient::ChromePasswordManagerClient(
       password_manager::prefs::kCredentialsEnableService, GetPrefs());
   static base::NoDestructor<password_manager::StoreMetricsReporter> reporter(
       *saving_and_filling_passwords_enabled_, this, GetSyncService(profile_),
-      GetSigninManager(profile_), GetPrefs());
+      GetSigninManagerForOriginalProfile(profile_), GetPrefs());
   driver_factory_->RequestSendLoggingAvailability();
 }
 

components/password_manager/core/browser/credentials_filter.h

@@ -42,7 +42,9 @@ class CredentialsFilter {
   virtual void ReportFormLoginSuccess(
       const PasswordFormManagerInterface& form_manager) const {}
 
-  // If |username| matches Chrome sync account email.
+  // If |username| matches Chrome sync account email. For incognito profile,
+  // it matches |username| against the sync account email used in its original
+  // profile.
   virtual bool IsSyncAccountEmail(const std::string& username) const = 0;
 
  private:

components/password_manager/core/browser/sync_credentials_filter.cc

@@ -77,7 +77,8 @@ std::vector<std::unique_ptr<PasswordForm>> SyncCredentialsFilter::FilterResults(
 
 bool SyncCredentialsFilter::ShouldSave(
     const autofill::PasswordForm& form) const {
-  return !form.is_gaia_with_skip_save_password_form &&
+  return !client_->IsIncognito() &&
+         !form.is_gaia_with_skip_save_password_form &&
          !sync_util::IsSyncAccountCredential(
              form, sync_service_factory_function_.Run(),
              signin_manager_factory_function_.Run());
@@ -86,7 +87,8 @@ bool SyncCredentialsFilter::ShouldSave(
 bool SyncCredentialsFilter::ShouldSaveGaiaPasswordHash(
     const autofill::PasswordForm& form) const {
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
-  return sync_util::IsGaiaCredentialPage(form.signon_realm);
+  return !client_->IsIncognito() &&
+         sync_util::IsGaiaCredentialPage(form.signon_realm);
 #else
   return false;
 #endif  // SYNC_PASSWORD_REUSE_DETECTION_ENABLED
@@ -94,8 +96,8 @@ bool SyncCredentialsFilter::ShouldSaveGaiaPasswordHash(
 
 bool SyncCredentialsFilter::ShouldSaveEnterprisePasswordHash(
     const autofill::PasswordForm& form) const {
-  return sync_util::ShouldSaveEnterprisePasswordHash(form,
-                                                     *client_->GetPrefs());
+  return !client_->IsIncognito() && sync_util::ShouldSaveEnterprisePasswordHash(
+                                        form, *client_->GetPrefs());
 }
 
 bool SyncCredentialsFilter::IsSyncAccountEmail(

components/password_manager/core/browser/sync_credentials_filter.h

@@ -66,6 +66,8 @@ class SyncCredentialsFilter : public CredentialsFilter {
 
   const SyncServiceFactoryFunction sync_service_factory_function_;
 
+  // For incognito profile, |signin_manager_factory_function_| returns the
+  // sign in manager of its original profile.
   const SigninManagerFactoryFunction signin_manager_factory_function_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncCredentialsFilter);

components/password_manager/core/browser/sync_credentials_filter_unittest.cc

@@ -67,7 +67,8 @@ void DisallowSync(base::test::ScopedFeatureList* feature_list) {
 class FakePasswordManagerClient : public StubPasswordManagerClient {
  public:
   FakePasswordManagerClient()
-      : password_store_(new testing::NiceMock<MockPasswordStore>) {
+      : password_store_(new testing::NiceMock<MockPasswordStore>),
+        is_incognito_(false) {
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
     // Initializes and configures prefs.
     prefs_ = std::make_unique<TestingPrefServiceSimple>();
@@ -99,10 +100,15 @@ class FakePasswordManagerClient : public StubPasswordManagerClient {
   PrefService* GetPrefs() const override { return prefs_.get(); }
 #endif
 
+  bool IsIncognito() const override { return is_incognito_; }
+
+  void SetIsIncognito(bool is_incognito) { is_incognito_ = is_incognito; }
+
  private:
   base::MessageLoop message_loop_;  // For |password_store_|.
   GURL last_committed_entry_url_;
   scoped_refptr<testing::NiceMock<MockPasswordStore>> password_store_;
+  bool is_incognito_;
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
   std::unique_ptr<TestingPrefServiceSimple> prefs_;
 #endif  // SYNC_PASSWORD_REUSE_DETECTION_ENABLED
@@ -450,6 +456,15 @@ TEST_F(CredentialsFilterTest, ShouldSaveGaiaPasswordHash) {
   EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(other_form));
 }
 
+TEST_F(CredentialsFilterTest, ShouldNotSaveGaiaPasswordHashIncognito) {
+  client_.SetIsIncognito(true);
+  PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
+  EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(gaia_form));
+
+  PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
+  EXPECT_FALSE(filter_.ShouldSaveGaiaPasswordHash(other_form));
+}
+
 TEST_F(CredentialsFilterTest, ShouldSaveEnterprisePasswordHash) {
   PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
   EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(gaia_form));
@@ -462,6 +477,19 @@ TEST_F(CredentialsFilterTest, ShouldSaveEnterprisePasswordHash) {
   EXPECT_TRUE(filter_.ShouldSaveEnterprisePasswordHash(enterprise_form));
 }
 
+TEST_F(CredentialsFilterTest, ShouldNotSaveEnterprisePasswordHashIncognito) {
+  client_.SetIsIncognito(true);
+  PasswordForm gaia_form = SimpleGaiaForm("user@gmail.org");
+  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(gaia_form));
+
+  PasswordForm other_form = SimpleNonGaiaForm("user@example.org");
+  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(other_form));
+
+  PasswordForm enterprise_form =
+      SimpleNonGaiaForm("user@enterprise.test", kEnterpriseURL);
+  EXPECT_FALSE(filter_.ShouldSaveEnterprisePasswordHash(enterprise_form));
+}
+
 TEST_F(CredentialsFilterTest, IsSyncAccountEmail) {
   FakeSigninAs("user@gmail.com");
   EXPECT_FALSE(filter_.IsSyncAccountEmail("user"));
@@ -471,6 +499,17 @@ TEST_F(CredentialsFilterTest, IsSyncAccountEmail) {
   EXPECT_TRUE(filter_.IsSyncAccountEmail("us.er@gmail.com"));
   EXPECT_TRUE(filter_.IsSyncAccountEmail("user@googlemail.com"));
 }
+
+TEST_F(CredentialsFilterTest, IsSyncAccountEmailIncognito) {
+  client_.SetIsIncognito(true);
+  FakeSigninAs("user@gmail.com");
+  EXPECT_FALSE(filter_.IsSyncAccountEmail("user"));
+  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@gmail.com"));
+  EXPECT_FALSE(filter_.IsSyncAccountEmail("user2@example.com"));
+  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@gmail.com"));
+  EXPECT_TRUE(filter_.IsSyncAccountEmail("us.er@gmail.com"));
+  EXPECT_TRUE(filter_.IsSyncAccountEmail("user@googlemail.com"));
+}
 #endif  // SYNC_PASSWORD_REUSE_DETECTION_ENABLED
 
 }  // namespace password_manager