Canonicalize gmail prefix when saving password hash

Google login page allows users to input their Gmail prefix or input the
full gmail email address for sign-in. For example, user's gmail is
user@gmail.com, then on https://accounts.google.com..., typing "user"
or "user@gmail.com" as username have the same effect. However,
current HashPasswordManager treats them as two separate accounts when
saving password hashes.

This CL improves the canonicalization of username by appending "@gmail.com"
to PasswordHashData.username if it is a Gmail email prefix.

Note that Dasher account always require full email as username.

Bug: 858865
Change-Id: I9ddf23f54ee67bd73bd754953a3b80758e062ef3
Reviewed-on: https://chromium-review.googlesource.com/1135906Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574953}

components/password_manager/core/browser/hash_password_manager.cc

@@ -86,6 +86,10 @@ std::string GetAndDecryptField(const base::Value& dict,
              : std::string();
 }
 
+bool IsGaiaPassword(const base::Value& dict) {
+  return GetAndDecryptField(dict, kIsGaiaFieldKey) == "true";
+}
+
 // Packs |salt| and |password_length| to a string.
 std::string LengthAndSaltToString(const std::string& salt,
                                   size_t password_length) {
@@ -168,7 +172,7 @@ bool HashPasswordManager::SavePasswordHash(const std::string username,
   for (base::Value& password_hash_data : update.Get()->GetList()) {
     if (AreUsernamesSame(
             GetAndDecryptField(password_hash_data, kUsernameFieldKey),
-            username)) {
+            IsGaiaPassword(password_hash_data), username, is_gaia_password)) {
       base::Optional<PasswordHashData> existing_password_hash =
           ConvertToPasswordHashData(password_hash_data);
       if (existing_password_hash && existing_password_hash->MatchesPassword(
@@ -217,9 +221,7 @@ void HashPasswordManager::ClearSavedPasswordHash(const std::string& username,
     ListPrefUpdate update(prefs_, prefs::kPasswordHashDataList);
     for (auto it = update->GetList().begin(); it != update->GetList().end();) {
       if (AreUsernamesSame(GetAndDecryptField(*it, kUsernameFieldKey),
-                           username) &&
-          GetAndDecryptField(*it, kIsGaiaFieldKey) ==
-              BooleanToString(is_gaia_password)) {
+                           IsGaiaPassword(*it), username, is_gaia_password)) {
         it = update->GetList().erase(it);
       } else {
         it++;
@@ -287,9 +289,7 @@ base::Optional<PasswordHashData> HashPasswordManager::RetrievePasswordHash(
   for (const base::Value& entry :
        prefs_->GetList(prefs::kPasswordHashDataList)->GetList()) {
     if (AreUsernamesSame(GetAndDecryptField(entry, kUsernameFieldKey),
-                         username) &&
-        GetAndDecryptField(entry, kIsGaiaFieldKey) ==
-            BooleanToString(is_gaia_password)) {
+                         IsGaiaPassword(entry), username, is_gaia_password)) {
       return ConvertToPasswordHashData(entry);
     }
   }
@@ -311,9 +311,7 @@ bool HashPasswordManager::HasPasswordHash(const std::string& username,
   for (const base::Value& entry :
        prefs_->GetList(prefs::kPasswordHashDataList)->GetList()) {
     if (AreUsernamesSame(GetAndDecryptField(entry, kUsernameFieldKey),
-                         username) &&
-        BooleanToString(is_gaia_password) ==
-            GetAndDecryptField(entry, kIsGaiaFieldKey)) {
+                         IsGaiaPassword(entry), username, is_gaia_password)) {
       return true;
     }
   }
@@ -397,8 +395,8 @@ bool HashPasswordManager::EncryptAndSave(
     return false;
   }
 
-  std::string encrypted_username =
-      EncryptString(CanonicalizeUsername(password_hash_data.username));
+  std::string encrypted_username = EncryptString(CanonicalizeUsername(
+      password_hash_data.username, password_hash_data.is_gaia_password));
   if (encrypted_username.empty())
     return false;
 
@@ -432,9 +430,8 @@ bool HashPasswordManager::EncryptAndSave(
   bool replace_old_entry = false;
   for (auto it = update->GetList().begin(); it != update->GetList().end();) {
     if (AreUsernamesSame(GetAndDecryptField(*it, kUsernameFieldKey),
-                         password_hash_data.username) &&
-        GetAndDecryptField(*it, kIsGaiaFieldKey) ==
-            BooleanToString(password_hash_data.is_gaia_password)) {
+                         IsGaiaPassword(*it), password_hash_data.username,
+                         password_hash_data.is_gaia_password)) {
       it = update->GetList().erase(it);
       replace_old_entry = true;
     } else {

components/password_manager/core/browser/hash_password_manager_unittest.cc

@@ -110,6 +110,7 @@ TEST_F(HashPasswordManagerTest, SavingPasswordHashDataNotCanonicalized) {
   base::string16 password(base::UTF8ToUTF16("password"));
   std::string canonical_username("user@gmail.com");
   std::string username("US.ER@gmail.com");
+  std::string gmail_prefix("user");
 
   // Verify |SavePasswordHash(const std::string,const base::string16&)|
   // behavior.
@@ -129,7 +130,7 @@ TEST_F(HashPasswordManagerTest, SavingPasswordHashDataNotCanonicalized) {
       hash_password_manager.RetrievePasswordHash(username,
                                                  /*is_gaia_password=*/true);
   hash_password_manager.SavePasswordHash(username, password,
-                                         /*force_update=*/true);
+                                         /*is_gaia_password=*/true);
   base::Optional<PasswordHashData> existing_password_data =
       hash_password_manager.RetrievePasswordHash(username,
                                                  /*is_gaia_password=*/true);
@@ -139,6 +140,28 @@ TEST_F(HashPasswordManagerTest, SavingPasswordHashDataNotCanonicalized) {
             hash_password_manager
                 .RetrievePasswordHash(username, /*is_gaia_password=*/true)
                 ->username);
+  hash_password_manager.SavePasswordHash(gmail_prefix, password,
+                                         /*is_gaia_password=*/true);
+  EXPECT_EQ(current_password_hash_data->hash,
+            hash_password_manager
+                .RetrievePasswordHash(gmail_prefix,
+                                      /*is_gaia_password=*/true)
+                ->hash);
+  EXPECT_EQ(1u, prefs_.GetList(prefs::kPasswordHashDataList)->GetList().size());
+  EXPECT_EQ(canonical_username,
+            hash_password_manager
+                .RetrievePasswordHash(gmail_prefix, /*is_gaia_password=*/true)
+                ->username);
+
+  // Saves the password with gmail prefix only should be canonicalized into
+  // full gmail user name.
+  hash_password_manager.SavePasswordHash("user.name", password,
+                                         /*is_gaia_password=*/true);
+  EXPECT_EQ(2u, prefs_.GetList(prefs::kPasswordHashDataList)->GetList().size());
+  EXPECT_EQ("username@gmail.com",
+            hash_password_manager
+                .RetrievePasswordHash("user.name", /*is_gaia_password=*/true)
+                ->username);
 }
 
 TEST_F(HashPasswordManagerTest, SavingGaiaPasswordAndNonGaiaPassword) {

components/password_manager/core/browser/password_hash_data.cc

@@ -48,8 +48,8 @@ bool PasswordHashData::MatchesPassword(const std::string& username,
                                        const base::string16& password,
                                        bool is_gaia_password) const {
   if (password.size() != this->length ||
-      !AreUsernamesSame(username, this->username) ||
-      is_gaia_password != this->is_gaia_password) {
+      !AreUsernamesSame(username, is_gaia_password, this->username,
+                        this->is_gaia_password)) {
     return false;
   }
 
@@ -103,15 +103,26 @@ uint64_t CalculatePasswordHash(const base::StringPiece16& text,
   return hash37;
 }
 
-std::string CanonicalizeUsername(const std::string& username) {
+std::string CanonicalizeUsername(const std::string& username,
+                                 bool is_gaia_account) {
   std::vector<std::string> parts = base::SplitString(
       username, "@", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  return parts.size() != 2U ? username : gaia::CanonicalizeEmail(username);
+  if (parts.size() != 2U) {
+    if (is_gaia_account && parts.size() == 1U)
+      return gaia::CanonicalizeEmail(username + "@gmail.com");
+    return username;
+  }
+  return gaia::CanonicalizeEmail(username);
 }
 
 bool AreUsernamesSame(const std::string& username1,
-                      const std::string& username2) {
-  return CanonicalizeUsername(username1) == CanonicalizeUsername(username2);
+                      bool is_username1_gaia_account,
+                      const std::string& username2,
+                      bool is_username2_gaia_account) {
+  if (is_username1_gaia_account != is_username2_gaia_account)
+    return false;
+  return CanonicalizeUsername(username1, is_username1_gaia_account) ==
+         CanonicalizeUsername(username2, is_username2_gaia_account);
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/password_hash_data.h

@@ -56,13 +56,16 @@ struct SyncPasswordData {
 uint64_t CalculatePasswordHash(const base::StringPiece16& text,
                                const std::string& salt);
 
-// If username is an email address, canonicalizes this email. Otherwise, returns
-// |username|.
-std::string CanonicalizeUsername(const std::string& username);
+// If username is an email address, canonicalizes this email. Otherwise,
+// append "@gmail.com" if it is gaia or returns |username| for non-Gaia account.
+std::string CanonicalizeUsername(const std::string& username,
+                                 bool is_gaia_account);
 
 // Returns true if the two usernames the same after canonicalization.
 bool AreUsernamesSame(const std::string& username1,
-                      const std::string& username2);
+                      bool is_username1_gaia_account,
+                      const std::string& username2,
+                      bool is_username2_gaia_account);
 
 }  // namespace password_manager