[PasswordManager] Improve detection of ignorable change password forms.

components/password_manager/core/browser/password_form_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/password_manager/core/browser/password_form_manager.h"
 
 #include <algorithm>
 #include <set>
 
+#include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/validation.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_store.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
 using autofill::FormStructure;
 using autofill::PasswordForm;
 using autofill::PasswordFormMap;
 using base::Time;
 
 // Shorten the name to spare line breaks. The code provides enough context
 // already.
 typedef autofill::SavePasswordProgressLogger Logger;
 
@@ skipping 27 matching lines @@
                             event, SUBMISSION_EVENT_ENUM_COUNT);
 }
 
 PasswordForm CopyAndModifySSLValidity(const PasswordForm& orig,
                                       bool ssl_valid) {
   PasswordForm result(orig);
   result.ssl_valid = ssl_valid;
   return result;
 }
 
+bool IsChangePasswordForm(const PasswordForm& candidate) {
+  return !candidate.new_password_element.empty() &&
+         !candidate.password_element.empty();
+}
+
 }  // namespace
 
 PasswordFormManager::PasswordFormManager(
     PasswordManager* password_manager,
     PasswordManagerClient* client,
     const base::WeakPtr<PasswordManagerDriver>& driver,
     const PasswordForm& observed_form,
     bool ssl_valid)
     : best_matches_deleter_(&best_matches_),
       observed_form_(CopyAndModifySSLValidity(observed_form, ssl_valid)),
@@ skipping 226 matching lines @@
   } else if (action == ALLOW_OTHER_POSSIBLE_USERNAMES &&
              UpdatePendingCredentialsIfOtherPossibleUsername(
                  credentials.username_value)) {
     // |pending_credentials_| is now set. Note we don't update
     // |pending_credentials_.username_value| to |credentials.username_value|
     // yet because we need to keep the original username to modify the stored
     // credential.
     selected_username_ = credentials.username_value;
     is_new_login_ = false;
   } else {
-    // User typed in a new, unknown username.
-    user_action_ = kUserActionOverrideUsernameAndPassword;
-    pending_credentials_ = observed_form_;
-    pending_credentials_.username_value = credentials.username_value;
-    pending_credentials_.other_possible_usernames =
-        credentials.other_possible_usernames;
+    // Check whether its a change-password form with missing username or its
+    // obscured. E.g. numeric indicator of the password strength please refer
+    // http://crbug.com/448351.
+    PasswordForm matching_form;
+    if (IsChangePasswordForm(credentials) &&
+        HasMatchingCredentialsInStore(credentials, &matching_form)) {
+      is_new_login_ = false;
+      pending_credentials_ = matching_form;
+      selected_username_ = matching_form.username_value;
+      user_action_ = kUserActionOverridePassword;
+    } else {
+      // User typed in a new, unknown username.
+      user_action_ = kUserActionOverrideUsernameAndPassword;
+      pending_credentials_ = observed_form_;
+      pending_credentials_.username_value = credentials.username_value;
+      pending_credentials_.other_possible_usernames =
+          credentials.other_possible_usernames;
 
-    // The password value will be filled in later, remove any garbage for now.
-    pending_credentials_.password_value.clear();
-    pending_credentials_.new_password_value.clear();
+      // The password value will be filled in later, remove any garbage for now.
+      pending_credentials_.password_value.clear();
+      pending_credentials_.new_password_value.clear();
 
-    // If this was a sign-up or change password form, the names of the elements
-    // are likely different than those on a login form, so do not bother saving
-    // them. We will fill them with meaningful values in UpdateLogin() when the
-    // user goes onto a real login form for the first time.
-    if (!credentials.new_password_element.empty()) {
-      pending_credentials_.password_element.clear();
-      pending_credentials_.new_password_element.clear();
+      // If this was a sign-up or change password form, the names of the
+      // elements are likely different than those on a login form, so do not
+      // bother saving them. We will fill them with meaningful values in
+      // UpdateLogin() when the user goes onto a real login form for the first
+      // time.
+      if (!credentials.new_password_element.empty()) {
+        pending_credentials_.password_element.clear();
+        pending_credentials_.new_password_element.clear();
+      }
     }
   }
 
   pending_credentials_.action = credentials.action;
   // If the user selected credentials we autofilled from a PasswordForm
   // that contained no action URL (IE6/7 imported passwords, for example),
   // bless it with the action URL from the observed form. See bug 1107719.
   if (pending_credentials_.action.is_empty())
     pending_credentials_.action = observed_form_.action;
 
@@ skipping 26 matching lines @@
   state_ = MATCHING_PHASE;
 
   scoped_ptr<BrowserSavePasswordProgressLogger> logger;
   if (client_->IsLoggingActive()) {
     logger.reset(new BrowserSavePasswordProgressLogger(client_));
     logger->LogMessage(Logger::STRING_FETCH_LOGINS_METHOD);
   }
 
   // Do not autofill on sign-up or change password forms (until we have some
   // working change password functionality).
-  if (!observed_form_.new_password_element.empty()) {
+  if (!observed_form_.new_password_element.empty() &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          autofill::switches::kEnablePasswordSaveOnInPageNavigation)) {
     if (logger)
       logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
     client_->AutofillResultsComputed();
     // There is no point in looking for the credentials in the store when they
     // won't be autofilled, so pretend there were none.
     std::vector<autofill::PasswordForm*> dummy_results;
     OnGetPasswordStoreResults(dummy_results);
     return;
   }
 
   PasswordStore* password_store = client_->GetPasswordStore();
   if (!password_store) {
     if (logger)
       logger->LogMessage(Logger::STRING_NO_STORE);
     NOTREACHED();
     return;
   }
   password_store->GetLogins(observed_form_, prompt_policy, this);
 }
 
 bool PasswordFormManager::HasCompletedMatching() const {
   return state_ == POST_MATCHING_PHASE;
 }
 
 bool PasswordFormManager::IsIgnorableChangePasswordForm() const {
-  bool is_change_password_form = !observed_form_.new_password_element.empty() &&
-                                 !observed_form_.password_element.empty();
-  bool is_username_certainly_correct = observed_form_.username_marked_by_site;
-  return is_change_password_form && !is_username_certainly_correct;
+  bool is_username_certainly_correct =
+      observed_form_.username_marked_by_site ||
+      HasMatchingCredentialsInStore(observed_form_, nullptr);
+  return IsChangePasswordForm(observed_form_) && !is_username_certainly_correct;
 }
 
 void PasswordFormManager::OnRequestDone(
     const std::vector<PasswordForm*>& logins_result) {
   // Note that the result gets deleted after this call completes, but we own
   // the PasswordForm objects pointed to by the result vector, thus we keep
   // copies to a minimum here.
 
   scoped_ptr<BrowserSavePasswordProgressLogger> logger;
   if (client_->IsLoggingActive()) {
@@ skipping 440 matching lines @@
   if (has_generated_password_)
     LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMITTED);
 }
 
 void PasswordFormManager::SubmitFailed() {
   submit_result_ = kSubmitResultFailed;
   if (has_generated_password_)
     LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED);
 }
 
+bool PasswordFormManager::HasMatchingCredentialsInStore(
+    const autofill::PasswordForm& candidate,
+    autofill::PasswordForm* matching_form) const {
+  bool match_found = false;
+  for (PasswordFormMap::const_iterator iter = best_matches_.begin();

[vabr (Chromium), 2015/01/28 13:25:32]

nit: You can use
for (const auto& match : best_matches_) {...}

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+       iter != best_matches_.end(); ++iter) {
+    if (net::registry_controlled_domains::SameDomainOrHost(

[vabr (Chromium), 2015/01/28 13:25:32]

This is too permissive, we cannot just update passwords across domains. We do
not do that for PSL matched permissions either.

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+            iter->second->origin, candidate.origin,
+            net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES) &&
+        (iter->second->password_value == candidate.password_value)) {
+      if (matching_form != nullptr)

[vabr (Chromium), 2015/01/28 13:25:32]

nit: Just write:
if (matching_form)

[Pritam Nikam, 2015/01/29 14:14:35]

Done.

+        *matching_form = *(iter->second);
+      match_found = true;
+      break;
+    }
+  }
+
+  return match_found;
+}
+
 }  // namespace password_manager