Update affiliated web credentials when Android credentials are updated.

components/password_manager/core/browser/password_store_unittest.cc

Index: components/password_manager/core/browser/password_store_unittest.cc
diff --git a/components/password_manager/core/browser/password_store_unittest.cc b/components/password_manager/core/browser/password_store_unittest.cc
index f5e2a14eed75bd8d2bc520bbbbf70fc7375dd8c8..b4aac9e57dcae75010900339efb523e5b307f627 100644
--- a/components/password_manager/core/browser/password_store_unittest.cc
+++ b/components/password_manager/core/browser/password_store_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
 #include "components/password_manager/core/browser/affiliated_match_helper.h"
@@ -36,6 +37,18 @@ const char kTestWebRealm1[] = "https://one.example.com/";
 const char kTestWebOrigin1[] = "https://one.example.com/origin";
 const char kTestWebRealm2[] = "https://two.example.com/";
 const char kTestWebOrigin2[] = "https://two.example.com/origin";
+const char kTestWebRealm3[] = "https://three.example.com/";
+const char kTestWebOrigin3[] = "https://three.example.com/origin";
+const char kTestWebRealm4[] = "https://four.example.com/";
+const char kTestWebOrigin4[] = "https://four.example.com/origin";
+const char kTestWebRealm5[] = "https://five.example.com/";
+const char kTestWebOrigin5[] = "https://five.example.com/origin";
+const char kTestPSLMatchingWebRealm[] = "https://psl.example.com/";
+const char kTestPSLMatchingWebOrigin[] = "https://psl.example.com/origin";
+const char kTestUnrelatedWebRealm[] = "https://notexample.com/";
+const char kTestUnrelatedWebOrigin[] = "https:/notexample.com/origin";
+const char kTestInsecureWebRealm[] = "http://one.example.com/";
+const char kTestInsecureWebOrigin[] = "http://one.example.com/origin";
 const char kTestAndroidRealm1[] = "android://hash@com.example.android/";
 const char kTestAndroidRealm2[] = "android://hash@com.example.two.android/";
 const char kTestAndroidRealm3[] = "android://hash@com.example.three.android/";
@@ -53,6 +66,13 @@ class MockPasswordStoreConsumer : public PasswordStoreConsumer {
   }
 };
 
+class MockPasswordStoreObserver
+    : public password_manager::PasswordStore::Observer {
+ public:
+  MOCK_METHOD1(OnLoginsChanged,
+               void(const password_manager::PasswordStoreChangeList& changes));
+};
+
 class MockAffiliatedMatchHelper : public AffiliatedMatchHelper {
  public:
   MockAffiliatedMatchHelper()
@@ -70,9 +90,21 @@ class MockAffiliatedMatchHelper : public AffiliatedMatchHelper {
         .WillOnce(testing::Return(results_to_return));
   }
 
+  // Expects GetAffiliatedWebRealms() to be called with the
+  // |expected_android_form|, and will cause the result callback supplied to
+  // GetAffiliatedWebRealms() to be invoked with |results_to_return|.
+  void ExpectCallToGetAffiliatedWebRealms(
+      const autofill::PasswordForm& expected_android_form,
+      const std::vector<std::string>& results_to_return) {
+    EXPECT_CALL(*this, OnGetAffiliatedWebRealmsCalled(expected_android_form))
+        .WillOnce(testing::Return(results_to_return));
+  }
+
  private:
   MOCK_METHOD1(OnGetAffiliatedAndroidRealmsCalled,
                std::vector<std::string>(const PasswordForm&));
+  MOCK_METHOD1(OnGetAffiliatedWebRealmsCalled,
+               std::vector<std::string>(const PasswordForm&));
 
   void GetAffiliatedAndroidRealms(
       const autofill::PasswordForm& observed_form,
@@ -82,6 +114,14 @@ class MockAffiliatedMatchHelper : public AffiliatedMatchHelper {
     result_callback.Run(affiliated_android_realms);
   }
 
+  void GetAffiliatedWebRealms(
+      const autofill::PasswordForm& android_form,
+      const AffiliatedRealmsCallback& result_callback) override {
+    std::vector<std::string> affiliated_web_realms =
+        OnGetAffiliatedWebRealmsCalled(android_form);
+    result_callback.Run(affiliated_web_realms);
+  }
+
   DISALLOW_COPY_AND_ASSIGN(MockAffiliatedMatchHelper);
 };
 
@@ -273,8 +313,8 @@ TEST_F(PasswordStoreTest, GetLoginsWithoutAffiliations) {
        L"", true, true, 1},
       // Credential that is a PSL match of the observed form.
       {PasswordForm::SCHEME_HTML,
-       kTestWebRealm2,
-       kTestWebOrigin2,
+       kTestPSLMatchingWebRealm,
+       kTestPSLMatchingWebOrigin,
        "", L"", L"",  L"",
        L"username_value_2",
        L"", true, true, 1},
@@ -349,8 +389,8 @@ TEST_F(PasswordStoreTest, GetLoginsWithAffiliations) {
        L"", true, true, 1},
       // Credential that is a PSL match of the observed form.
       {PasswordForm::SCHEME_HTML,
-       kTestWebRealm2,
-       kTestWebOrigin2,
+       kTestPSLMatchingWebRealm,
+       kTestPSLMatchingWebOrigin,
        "", L"", L"",  L"",
        L"username_value_2",
        L"", true, true, 1},
@@ -435,4 +475,208 @@ TEST_F(PasswordStoreTest, GetLoginsWithAffiliations) {
   base::MessageLoop::current()->RunUntilIdle();
 }
 
+// This test must use passwords, which are not stored on Mac, therefore the test
+// is disabled on Mac. This should not be a huge issue as functionality in the
+// platform-independent base class is tested. See also the file-level comment.
+#if defined(OS_MACOSX)
+#define MAYBE_UpdatePasswordsStoredForAffiliatedWebsites \
+  DISABLED_UpdatePasswordsStoredForAffiliatedWebsites
+#else
+#define MAYBE_UpdatePasswordsStoredForAffiliatedWebsites \
+  UpdatePasswordsStoredForAffiliatedWebsites
+#endif
+
+// When the password stored for an Android application is updated, credentials
+// with the same username stored for affiliated web sites should also be updated
+// automatically.
+TEST_F(PasswordStoreTest, MAYBE_UpdatePasswordsStoredForAffiliatedWebsites) {
+  const wchar_t kTestUsername[] = L"username_value_1";
+  const wchar_t kTestOtherUsername[] = L"username_value_2";
+  const wchar_t kTestOldPassword[] = L"old_password_value";
+  const wchar_t kTestNewPassword[] = L"new_password_value";
+  const wchar_t kTestOtherPassword[] = L"other_password_value";
+
+  /* clang-format off */
+  static const PasswordFormData kTestCredentials[] = {
+      // The credential for the Android application that will be updated
+      // explicitly so it can be tested if the changes are correctly propagated
+      // to affiliated Web credentials.
+      {PasswordForm::SCHEME_HTML,
+       kTestAndroidRealm1,
+       "", "", L"", L"", L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 2},
+
+      // --- Positive samples --- Credentials that the password update should be
+      // automatically propagated to.
+
+      // Credential for an affiliated web site with the same username.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm1,
+       kTestWebOrigin1,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for another affiliated web site with the same username.
+      // Although the password is different than the current/old password for
+      // the Android application, it should be updated regardless.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm2,
+       kTestWebOrigin2,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOtherPassword, true, true, 1},
+
+      // --- Negative samples --- Credentials that the password update should
+      // not be propagated to.
+
+      // Credential for another affiliated web site, but one that already has
+      // the new password.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm3,
+       kTestWebOrigin3,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestNewPassword, true, true, 1},
+      // Credential for another affiliated web site, but one that was saved
+      // under insecure conditions.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm4,
+       kTestWebOrigin4,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOldPassword, true, false, 1},
+      // Credential for the HTTP version of an affiliated web site.
+      {PasswordForm::SCHEME_HTML,
+       kTestInsecureWebRealm,
+       kTestInsecureWebOrigin,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOldPassword, true, false, 1},
+      // Credential for an affiliated web site, but with a different username.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm1,
+       kTestWebOrigin1,
+       "", L"", L"",  L"",
+       kTestOtherUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for a web site that is a PSL match to a web sites affiliated
+      // with the Android application.
+      {PasswordForm::SCHEME_HTML,
+       kTestPSLMatchingWebRealm,
+       kTestPSLMatchingWebOrigin,
+       "poisoned", L"poisoned", L"",  L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for an unrelated web site.
+      {PasswordForm::SCHEME_HTML,
+       kTestUnrelatedWebRealm,
+       kTestUnrelatedWebOrigin,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for an affiliated Android application.
+      {PasswordForm::SCHEME_HTML,
+       kTestAndroidRealm2,
+       "", "", L"", L"", L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for an unrelated Android application.
+      {PasswordForm::SCHEME_HTML,
+       kTestUnrelatedAndroidRealm,
+       "", "", L"", L"", L"",
+       kTestUsername,
+       kTestOldPassword, true, true, 1},
+      // Credential for an affiliated web site with the same username, but one
+      // that was updated at the same time via Sync as the Android credential.
+      {PasswordForm::SCHEME_HTML,
+       kTestWebRealm5,
+       kTestWebOrigin5,
+       "", L"", L"",  L"",
+       kTestUsername,
+       kTestOtherPassword, true, true, 2}};
+  /* clang-format on */
+
+  // The number of positive samples in |kTestCredentials|.
+  const size_t kExpectedNumberOfUpdatedCredentials = 2u;
+
+  const bool kTestRemoveAndAddLogin[] = {false, true};
+  for (bool test_remove_and_add_login : kTestRemoveAndAddLogin) {

[vasilii, 2015/04/20 13:04:08]

Why not making this a test parameter?

[engedy, 2015/04/20 13:50:43]

The parameter only makes sense for this test, not the entire suite of tests. As
far as I understand, I would need to define a separate test fixture for this
then. I would rather not do that.

+    SCOPED_TRACE(test_remove_and_add_login);
+
+    scoped_refptr<PasswordStoreDefault> store(new PasswordStoreDefault(
+        base::MessageLoopProxy::current(), base::MessageLoopProxy::current(),
+        make_scoped_ptr(new LoginDatabase(test_login_db_file_path()))));
+    store->Init(syncer::SyncableService::StartSyncFlare());
+    store->RemoveLoginsCreatedBetween(base::Time(), base::Time::Max());
+
+    // Set up the initial test data set.
+    ScopedVector<PasswordForm> all_credentials;
+    for (size_t i = 0; i < arraysize(kTestCredentials); ++i) {
+      all_credentials.push_back(
+          CreatePasswordFormFromDataForTesting(kTestCredentials[i]));
+      all_credentials.back()->date_synced =
+          all_credentials.back()->date_created;
+      store->AddLogin(*all_credentials.back());
+      base::MessageLoop::current()->RunUntilIdle();
+    }
+
+    // The helper must be injected after the initial test data is set up,
+    // otherwise it will already start propagating updates as new Android
+    // credentials are added.
+    MockAffiliatedMatchHelper* mock_helper = new MockAffiliatedMatchHelper;
+    store->SetAffiliatedMatchHelper(make_scoped_ptr(mock_helper));
+
+    // Calculate how the correctly updated test data set should look like.
+    ScopedVector<PasswordForm> expected_credentials_after_update;
+    for (size_t i = 0; i < all_credentials.size(); ++i) {
+      expected_credentials_after_update.push_back(
+          new autofill::PasswordForm(*all_credentials[i]));
+      if (i < 1 + kExpectedNumberOfUpdatedCredentials) {
+        expected_credentials_after_update.back()->password_value =
+            base::WideToUTF16(kTestNewPassword);
+      }
+    }
+
+    std::vector<std::string> affiliated_web_realms;
+    affiliated_web_realms.push_back(kTestWebRealm1);
+    affiliated_web_realms.push_back(kTestWebRealm2);
+    affiliated_web_realms.push_back(kTestWebRealm3);
+    affiliated_web_realms.push_back(kTestWebRealm4);
+    affiliated_web_realms.push_back(kTestWebRealm5);
+    mock_helper->ExpectCallToGetAffiliatedWebRealms(
+        *expected_credentials_after_update[0], affiliated_web_realms);
+
+    // Explicitly update the Android credential, wait until things calm down,
+    // then query all passwords and expect that:
+    //   1.) The positive samples in |kTestCredentials| have the new password,
+    //       but the negative samples do not.
+    //   2.) Change notifications are sent about the updates. Note that as the
+    //       test interacts with the (Update|Add)LoginSync methods, only the
+    //       derived changes should trigger notifications, the first one would
+    //       normally be trigger by Sync.
+    MockPasswordStoreObserver mock_observer;
+    store->AddObserver(&mock_observer);
+    EXPECT_CALL(mock_observer, OnLoginsChanged(testing::SizeIs(1u)))
+        .Times(kExpectedNumberOfUpdatedCredentials);
+    if (test_remove_and_add_login) {
+      store->RemoveLoginSync(*all_credentials[0]);
+      store->AddLoginSync(*expected_credentials_after_update[0]);
+    } else {
+      store->UpdateLoginSync(*expected_credentials_after_update[0]);
+    }
+    base::MessageLoop::current()->RunUntilIdle();
+    store->RemoveObserver(&mock_observer);
+
+    MockPasswordStoreConsumer mock_consumer;
+    EXPECT_CALL(
+        mock_consumer,
+        OnGetPasswordStoreResultsConstRef(UnorderedPasswordFormElementsAre(
+            expected_credentials_after_update.get())));
+    store->GetAutofillableLogins(&mock_consumer);
+    store->Shutdown();
+    base::MessageLoop::current()->RunUntilIdle();
+  }
+}
+
 }  // namespace password_manager