Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10)

Side by Side Diff: components/password_manager/core/browser/password_form_manager.cc

Issue 870513002: [PasswordManager] Improve detection of ignorable change password forms. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/password_manager/core/browser/password_form_manager.h" 5 #include "components/password_manager/core/browser/password_form_manager.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <set> 8 #include <set>
9 9
10 #include "base/command_line.h"
10 #include "base/metrics/histogram.h" 11 #include "base/metrics/histogram.h"
11 #include "base/metrics/user_metrics.h" 12 #include "base/metrics/user_metrics.h"
12 #include "base/strings/string_split.h" 13 #include "base/strings/string_split.h"
13 #include "base/strings/string_util.h" 14 #include "base/strings/string_util.h"
14 #include "base/strings/utf_string_conversions.h" 15 #include "base/strings/utf_string_conversions.h"
15 #include "components/autofill/core/browser/autofill_manager.h" 16 #include "components/autofill/core/browser/autofill_manager.h"
16 #include "components/autofill/core/browser/form_structure.h" 17 #include "components/autofill/core/browser/form_structure.h"
17 #include "components/autofill/core/browser/validation.h" 18 #include "components/autofill/core/browser/validation.h"
19 #include "components/autofill/core/common/autofill_switches.h"
18 #include "components/autofill/core/common/password_form.h" 20 #include "components/autofill/core/common/password_form.h"
19 #include "components/password_manager/core/browser/browser_save_password_progres s_logger.h" 21 #include "components/password_manager/core/browser/browser_save_password_progres s_logger.h"
20 #include "components/password_manager/core/browser/password_manager.h" 22 #include "components/password_manager/core/browser/password_manager.h"
21 #include "components/password_manager/core/browser/password_manager_client.h" 23 #include "components/password_manager/core/browser/password_manager_client.h"
22 #include "components/password_manager/core/browser/password_manager_driver.h" 24 #include "components/password_manager/core/browser/password_manager_driver.h"
23 #include "components/password_manager/core/browser/password_store.h" 25 #include "components/password_manager/core/browser/password_store.h"
24 26
25 using autofill::FormStructure; 27 using autofill::FormStructure;
26 using autofill::PasswordForm; 28 using autofill::PasswordForm;
27 using autofill::PasswordFormMap; 29 using autofill::PasswordFormMap;
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
61 event, SUBMISSION_EVENT_ENUM_COUNT); 63 event, SUBMISSION_EVENT_ENUM_COUNT);
62 } 64 }
63 65
64 PasswordForm CopyAndModifySSLValidity(const PasswordForm& orig, 66 PasswordForm CopyAndModifySSLValidity(const PasswordForm& orig,
65 bool ssl_valid) { 67 bool ssl_valid) {
66 PasswordForm result(orig); 68 PasswordForm result(orig);
67 result.ssl_valid = ssl_valid; 69 result.ssl_valid = ssl_valid;
68 return result; 70 return result;
69 } 71 }
70 72
73 bool IsChangePasswordForm(const PasswordForm& candidate) {
74 return !candidate.new_password_element.empty() &&
75 !candidate.password_element.empty();
76 }
77
71 } // namespace 78 } // namespace
72 79
73 PasswordFormManager::PasswordFormManager( 80 PasswordFormManager::PasswordFormManager(
74 PasswordManager* password_manager, 81 PasswordManager* password_manager,
75 PasswordManagerClient* client, 82 PasswordManagerClient* client,
76 const base::WeakPtr<PasswordManagerDriver>& driver, 83 const base::WeakPtr<PasswordManagerDriver>& driver,
77 const PasswordForm& observed_form, 84 const PasswordForm& observed_form,
78 bool ssl_valid) 85 bool ssl_valid)
79 : best_matches_deleter_(&best_matches_), 86 : best_matches_deleter_(&best_matches_),
80 observed_form_(CopyAndModifySSLValidity(observed_form, ssl_valid)), 87 observed_form_(CopyAndModifySSLValidity(observed_form, ssl_valid)),
(...skipping 158 matching lines...) Expand 10 before | Expand all | Expand 10 after
239 DCHECK_NE(RESULT_NO_MATCH, DoesManage(credentials)); 246 DCHECK_NE(RESULT_NO_MATCH, DoesManage(credentials));
240 247
241 // If this was a sign-up or change password form, we want to persist the new 248 // If this was a sign-up or change password form, we want to persist the new
242 // password; if this was a login form, then the current password (which might 249 // password; if this was a login form, then the current password (which might
243 // still be "new" in the sense that we see these credentials for the first 250 // still be "new" in the sense that we see these credentials for the first
244 // time, or that the user manually entered his actual password to overwrite an 251 // time, or that the user manually entered his actual password to overwrite an
245 // obsolete password we had in the store). 252 // obsolete password we had in the store).
246 base::string16 password_to_save(credentials.new_password_element.empty() ? 253 base::string16 password_to_save(credentials.new_password_element.empty() ?
247 credentials.password_value : credentials.new_password_value); 254 credentials.password_value : credentials.new_password_value);
248 255
256 bool is_username_certainly_correct = credentials.username_marked_by_site;
257
249 // Make sure the important fields stay the same as the initially observed or 258 // Make sure the important fields stay the same as the initially observed or
250 // autofilled ones, as they may have changed if the user experienced a login 259 // autofilled ones, as they may have changed if the user experienced a login
251 // failure. 260 // failure.
252 // Look for these credentials in the list containing auto-fill entries. 261 // Look for these credentials in the list containing auto-fill entries.
253 PasswordFormMap::const_iterator it = 262 PasswordFormMap::const_iterator it =
254 best_matches_.find(credentials.username_value); 263 best_matches_.find(credentials.username_value);
255 if (it != best_matches_.end()) { 264 if (!is_username_certainly_correct && it != best_matches_.end()) {
256 // The user signed in with a login we autofilled. 265 // The user signed in with a login we autofilled.
257 pending_credentials_ = *it->second; 266 pending_credentials_ = *it->second;
258 bool password_changed = 267 bool password_changed =
259 pending_credentials_.password_value != password_to_save; 268 pending_credentials_.password_value != password_to_save;
260 if (IsPendingCredentialsPublicSuffixMatch()) { 269 if (IsPendingCredentialsPublicSuffixMatch()) {
261 // If the autofilled credentials were only a PSL match, store a copy with 270 // If the autofilled credentials were only a PSL match, store a copy with
262 // the current origin and signon realm. This ensures that on the next 271 // the current origin and signon realm. This ensures that on the next
263 // visit, a precise match is found. 272 // visit, a precise match is found.
264 is_new_login_ = true; 273 is_new_login_ = true;
265 user_action_ = password_changed ? kUserActionChoosePslMatch 274 user_action_ = password_changed ? kUserActionChoosePslMatch
(...skipping 30 matching lines...) Expand all
296 // infrequently, and the inconvenience put on the user by asking them is 305 // infrequently, and the inconvenience put on the user by asking them is
297 // not significant, so we are fine with asking here again. 306 // not significant, so we are fine with asking here again.
298 if (password_changed) { 307 if (password_changed) {
299 pending_credentials_.original_signon_realm.clear(); 308 pending_credentials_.original_signon_realm.clear();
300 DCHECK(!IsPendingCredentialsPublicSuffixMatch()); 309 DCHECK(!IsPendingCredentialsPublicSuffixMatch());
301 } 310 }
302 } else { // Not a PSL match. 311 } else { // Not a PSL match.
303 is_new_login_ = false; 312 is_new_login_ = false;
304 if (password_changed) 313 if (password_changed)
305 user_action_ = kUserActionOverridePassword; 314 user_action_ = kUserActionOverridePassword;
315
316 // Check whether its a change-password form with matching credentials in
317 // stored form.
318 PasswordForm matching_form;
319 if (IsChangePasswordForm(credentials) &&
320 HasMatchingCredentialsInStore(credentials, &matching_form)) {
321 pending_credentials_ = matching_form;
322 selected_username_ = matching_form.username_value;
323 user_action_ = kUserActionOverridePassword;
324 }
306 } 325 }
307 } else if (action == ALLOW_OTHER_POSSIBLE_USERNAMES && 326 } else if (action == ALLOW_OTHER_POSSIBLE_USERNAMES &&
308 UpdatePendingCredentialsIfOtherPossibleUsername( 327 UpdatePendingCredentialsIfOtherPossibleUsername(
309 credentials.username_value)) { 328 credentials.username_value)) {
310 // |pending_credentials_| is now set. Note we don't update 329 // |pending_credentials_| is now set. Note we don't update
311 // |pending_credentials_.username_value| to |credentials.username_value| 330 // |pending_credentials_.username_value| to |credentials.username_value|
312 // yet because we need to keep the original username to modify the stored 331 // yet because we need to keep the original username to modify the stored
313 // credential. 332 // credential.
314 selected_username_ = credentials.username_value; 333 selected_username_ = credentials.username_value;
315 is_new_login_ = false; 334 is_new_login_ = false;
(...skipping 53 matching lines...) Expand 10 before | Expand all | Expand 10 after
369 PasswordStore::AuthorizationPromptPolicy prompt_policy) { 388 PasswordStore::AuthorizationPromptPolicy prompt_policy) {
370 DCHECK_EQ(state_, PRE_MATCHING_PHASE); 389 DCHECK_EQ(state_, PRE_MATCHING_PHASE);
371 state_ = MATCHING_PHASE; 390 state_ = MATCHING_PHASE;
372 391
373 scoped_ptr<BrowserSavePasswordProgressLogger> logger; 392 scoped_ptr<BrowserSavePasswordProgressLogger> logger;
374 if (client_->IsLoggingActive()) { 393 if (client_->IsLoggingActive()) {
375 logger.reset(new BrowserSavePasswordProgressLogger(client_)); 394 logger.reset(new BrowserSavePasswordProgressLogger(client_));
376 logger->LogMessage(Logger::STRING_FETCH_LOGINS_METHOD); 395 logger->LogMessage(Logger::STRING_FETCH_LOGINS_METHOD);
377 } 396 }
378 397
379 // Do not autofill on sign-up or change password forms (until we have some
380 // working change password functionality).
381 if (!observed_form_.new_password_element.empty()) {
382 if (logger)
383 logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
384 client_->AutofillResultsComputed();
385 // There is no point in looking for the credentials in the store when they
386 // won't be autofilled, so pretend there were none.
387 std::vector<autofill::PasswordForm*> dummy_results;
388 OnGetPasswordStoreResults(dummy_results);
389 return;
390 }
391
392 PasswordStore* password_store = client_->GetPasswordStore(); 398 PasswordStore* password_store = client_->GetPasswordStore();
393 if (!password_store) { 399 if (!password_store) {
394 if (logger) 400 if (logger)
395 logger->LogMessage(Logger::STRING_NO_STORE); 401 logger->LogMessage(Logger::STRING_NO_STORE);
396 NOTREACHED(); 402 NOTREACHED();
397 return; 403 return;
398 } 404 }
399 password_store->GetLogins(observed_form_, prompt_policy, this); 405 password_store->GetLogins(observed_form_, prompt_policy, this);
400 } 406 }
401 407
402 bool PasswordFormManager::HasCompletedMatching() const { 408 bool PasswordFormManager::HasCompletedMatching() const {
403 return state_ == POST_MATCHING_PHASE; 409 return state_ == POST_MATCHING_PHASE;
404 } 410 }
405 411
406 bool PasswordFormManager::IsIgnorableChangePasswordForm() const { 412 bool PasswordFormManager::IsIgnorableChangePasswordForm() const {
407 bool is_change_password_form = !observed_form_.new_password_element.empty() &&
408 !observed_form_.password_element.empty();
409 bool is_username_certainly_correct = observed_form_.username_marked_by_site; 413 bool is_username_certainly_correct = observed_form_.username_marked_by_site;
410 return is_change_password_form && !is_username_certainly_correct; 414 return IsChangePasswordForm(observed_form_) && !is_username_certainly_correct;
411 } 415 }
412 416
413 void PasswordFormManager::OnRequestDone( 417 void PasswordFormManager::OnRequestDone(
414 const std::vector<PasswordForm*>& logins_result) { 418 const std::vector<PasswordForm*>& logins_result) {
415 // Note that the result gets deleted after this call completes, but we own 419 // Note that the result gets deleted after this call completes, but we own
416 // the PasswordForm objects pointed to by the result vector, thus we keep 420 // the PasswordForm objects pointed to by the result vector, thus we keep
417 // copies to a minimum here. 421 // copies to a minimum here.
418 422
419 scoped_ptr<BrowserSavePasswordProgressLogger> logger; 423 scoped_ptr<BrowserSavePasswordProgressLogger> logger;
420 if (client_->IsLoggingActive()) { 424 if (client_->IsLoggingActive()) {
(...skipping 123 matching lines...) Expand 10 before | Expand all | Expand 10 after
544 // (1) we are in Incognito mode, (2) the ACTION paths don't match, 548 // (1) we are in Incognito mode, (2) the ACTION paths don't match,
545 // or (3) if it matched using public suffix domain matching. 549 // or (3) if it matched using public suffix domain matching.
546 bool wait_for_username = client_->IsOffTheRecord() || 550 bool wait_for_username = client_->IsOffTheRecord() ||
547 observed_form_.action.GetWithEmptyPath() != 551 observed_form_.action.GetWithEmptyPath() !=
548 preferred_match_->action.GetWithEmptyPath() || 552 preferred_match_->action.GetWithEmptyPath() ||
549 preferred_match_->IsPublicSuffixMatch(); 553 preferred_match_->IsPublicSuffixMatch();
550 if (wait_for_username) 554 if (wait_for_username)
551 manager_action_ = kManagerActionNone; 555 manager_action_ = kManagerActionNone;
552 else 556 else
553 manager_action_ = kManagerActionAutofilled; 557 manager_action_ = kManagerActionAutofilled;
554 password_manager_->Autofill(driver.get(), observed_form_, best_matches_, 558
555 *preferred_match_, wait_for_username); 559 // Do not autofill on sign-up or change password forms (until we have some
560 // working change password functionality).
561 if (observed_form_.new_password_element.empty()) {
562 password_manager_->Autofill(driver.get(), observed_form_, best_matches_,
563 *preferred_match_, wait_for_username);
564 }
556 } 565 }
557 566
558 void PasswordFormManager::OnGetPasswordStoreResults( 567 void PasswordFormManager::OnGetPasswordStoreResults(
559 const std::vector<autofill::PasswordForm*>& results) { 568 const std::vector<autofill::PasswordForm*>& results) {
560 DCHECK_EQ(state_, MATCHING_PHASE); 569 DCHECK_EQ(state_, MATCHING_PHASE);
561 570
562 scoped_ptr<BrowserSavePasswordProgressLogger> logger; 571 scoped_ptr<BrowserSavePasswordProgressLogger> logger;
563 if (client_->IsLoggingActive()) { 572 if (client_->IsLoggingActive()) {
564 logger.reset(new BrowserSavePasswordProgressLogger(client_)); 573 logger.reset(new BrowserSavePasswordProgressLogger(client_));
565 logger->LogMessage(Logger::STRING_ON_GET_STORE_RESULTS_METHOD); 574 logger->LogMessage(Logger::STRING_ON_GET_STORE_RESULTS_METHOD);
(...skipping 295 matching lines...) Expand 10 before | Expand all | Expand 10 after
861 if (has_generated_password_) 870 if (has_generated_password_)
862 LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMITTED); 871 LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMITTED);
863 } 872 }
864 873
865 void PasswordFormManager::SubmitFailed() { 874 void PasswordFormManager::SubmitFailed() {
866 submit_result_ = kSubmitResultFailed; 875 submit_result_ = kSubmitResultFailed;
867 if (has_generated_password_) 876 if (has_generated_password_)
868 LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED); 877 LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED);
869 } 878 }
870 879
880 bool PasswordFormManager::HasMatchingCredentialsInStore(
881 const autofill::PasswordForm& candidate,
882 autofill::PasswordForm* matching_form) const {
883 bool match_found = false;
884 for (const auto& match : best_matches_) {
885 // Make sure the username, password and origin signon realm matches to
886 // ensure that the candidate form is one available in the store.
887 if (match.second->username_value == candidate.username_value &&
888 match.second->password_value == candidate.password_value &&
889 match.second->original_signon_realm ==
890 candidate.original_signon_realm) {
891 if (matching_form)
892 *matching_form = *(match.second);
893 match_found = true;
894 break;
895 }
896 }
897
898 return match_found;
899 }
900
871 } // namespace password_manager 901 } // namespace password_manager
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698