Allow editing passwords in settings/passwords

chrome/browser/ui/passwords/password_manager_presenter.cc

Index: chrome/browser/ui/passwords/password_manager_presenter.cc
diff --git a/chrome/browser/ui/passwords/password_manager_presenter.cc b/chrome/browser/ui/passwords/password_manager_presenter.cc
index 52d2eb80ef518a453732bff1da87088fde0da24f..e75fb970ef10a368d095a26680e28046eb9f3a64 100644
--- a/chrome/browser/ui/passwords/password_manager_presenter.cc
+++ b/chrome/browser/ui/passwords/password_manager_presenter.cc
@@ -83,6 +83,69 @@ void PasswordManagerPresenter::UpdatePasswordLists() {
   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