Allow editing passwords in settings/passwords

chrome/browser/ui/passwords/password_manager_presenter.cc

 // Copyright 2013 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 "chrome/browser/ui/passwords/password_manager_presenter.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/prefs/pref_service.h"
@@ skipping 65 matching lines @@
   last_authentication_time_ = base::TimeTicks();
 
   // Reset the current lists.
   password_list_.clear();
   password_exception_list_.clear();
 
   populater_.Populate();
   exception_populater_.Populate();
 }
 
+// static
+bool PasswordManagerPresenter::CheckOriginValidityForAdding(
+    const GURL& origin) {
+  // Restrict the URL scheme to http and https since a manually-added
+  // PasswordForm entry's |scheme| is assumed to be SCHEME_HTML.
+  return origin.is_valid() && (origin.SchemeIs(url::kHttpScheme) ||
+                               origin.SchemeIs(url::kHttpsScheme));
+}
+
+void PasswordManagerPresenter::AddPassword(
+    const GURL& origin,
+    const base::string16& username_value,
+    const base::string16& password_value) {
+#if !defined(OS_ANDROID)  // This is never called on Android.
+  if (!CheckOriginValidityForAdding(origin) || password_value.empty()) {
+    // Invalid |origin| or empty |password_value| might come from a compromised
+    // renderer, which shouldn't be saved to the password store.

[vabr (Chromium), 2014/08/25 07:35:07]

Is it "might come" or "can only come"? If I understood the code correctly, as
long as the renderer is not compromised, it should not trigger calling this with
an invalid origin or an empty password, no matter what the user types, correct?

grammar nit: "which" in the second sentence seems to refer to the renderer, and
saving a renderer in a password store does not make sense. I suggest not
including the "which"-sentence at all -- it's apparent from the code that such
input is not saved to the store.

[jaekyeom, 2014/08/26 05:34:48]

Done.

+    NOTREACHED();
+    return;
+  }
+  PasswordStore* store = GetPasswordStore();
+  if (!store)
+    return;
+
+  GURL::Replacements replacements;
+  replacements.ClearUsername();
+  replacements.ClearPassword();
+  replacements.ClearQuery();
+  replacements.ClearRef();
+  autofill::PasswordForm form;
+  form.origin = origin.ReplaceComponents(replacements);
+  form.username_value = username_value;
+  form.password_value = password_value;
+  form.signon_realm = origin.GetOrigin().spec();
+  form.ssl_valid = origin.SchemeIsSecure();
+  form.date_created = base::Time::Now();
+
+  store->AddLogin(form);
+#endif
+}
+
+void PasswordManagerPresenter::UpdatePassword(
+    size_t index,
+    const base::string16& password_value) {
+#if !defined(OS_ANDROID)  // This is never called on Android.
+  if (index >= password_list_.size() || password_value.empty()) {
+    // |index| out of bounds might come from a compromised renderer, don't let
+    // it crash the browser. http://crbug.com/362054
+    // Similarly, empty |password_value| also might come from a compromised
+    // renderer, which is forbidden to be saved to the password store.  So use
+    // the same logic.
+    NOTREACHED();
+    return;
+  }
+  PasswordStore* store = GetPasswordStore();
+  if (!store)
+    return;
+  autofill::PasswordForm form(*password_list_[index]);
+  form.password_value = password_value;
+  store->UpdateLogin(form);
+#endif
+}
+
 void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {
   if (index >= password_list_.size()) {
     // |index| out of bounds might come from a compromised renderer, don't let
     // it crash the browser. http://crbug.com/362054
     NOTREACHED();
     return;
   }
   PasswordStore* store = GetPasswordStore();
   if (!store)
     return;
@@ skipping 136 matching lines @@
 }
 
 void PasswordManagerPresenter::PasswordExceptionListPopulater::
     OnGetPasswordStoreResults(
         const std::vector<autofill::PasswordForm*>& results) {
   page_->password_exception_list_.clear();
   page_->password_exception_list_.insert(page_->password_exception_list_.end(),
                                          results.begin(), results.end());
   page_->SetPasswordExceptionList();
 }