Enable import/export of passwords into/from Password Manager

chrome/browser/ui/webui/options/password_manager_handler.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/webui/options/password_manager_handler.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/feature_list.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
+#if defined(OS_WIN) && defined(USE_ASH)
+#include "chrome/browser/ui/ash/ash_util.h"

[vabr (Chromium), 2016/03/10 10:53:31]

nit: Please move this #ifdefed #include into the section below line 50.

[xunlu, 2016/03/16 07:23:58]

Done.

+#endif
+#include "chrome/browser/ui/chrome_select_file_policy.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/browser_sync/browser/profile_sync_service.h"
+#include "components/password_manager/core/browser/export/password_exporter.h"
 #include "components/password_manager/core/browser/password_bubble_experiment.h"
 #include "components/password_manager/core/browser/password_manager_constants.h"
+#include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/password_ui_utils.h"
 #include "components/password_manager/core/common/experiments.h"
 #include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/url_formatter.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/origin_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_WIN) && defined(USE_ASH)
@@ skipping 23 matching lines @@
       url_formatter::FormatUrl(
           form.origin, languages, url_formatter::kFormatUrlOmitNothing,
           net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
   bool is_android_uri = false;
   entry->SetString(kShownUrlField, password_manager::GetShownOrigin(
                                        form, languages, &is_android_uri));
   entry->SetBoolean(kIsAndroidUriField, is_android_uri);
   entry->SetBoolean(kIsSecureField, content::IsOriginSecure(form.origin));
 }
 
+// Enumeration of different callers of SelectFile. Starting count at 1 so
+// accidental call of SelectFile with params=NULL will error out.
+enum FileSelectorCaller {
+  IMPORT_FILE_SELECTED = 1,
+  EXPORT_FILE_SELECTED,
+};
+
 }  // namespace
 
-PasswordManagerHandler::PasswordManagerHandler()
-    : password_manager_presenter_(this) {}
+PasswordManagerHandler::PasswordManagerHandler() {
+  password_manager_presenter_.reset(new PasswordManagerPresenter(this));
+}
+
+PasswordManagerHandler::PasswordManagerHandler(
+    PasswordManagerPresenter* presenter) {
+  password_manager_presenter_.reset(presenter);
+}
 
 PasswordManagerHandler::~PasswordManagerHandler() {}
 
 Profile* PasswordManagerHandler::GetProfile() {
   return Profile::FromWebUI(web_ui());
 }
 
 #if !defined(OS_ANDROID)
 gfx::NativeWindow PasswordManagerHandler::GetNativeWindow() const {
   return web_ui()->GetWebContents()->GetTopLevelNativeWindow();
 }
 #endif
 
 void PasswordManagerHandler::GetLocalizedValues(
     base::DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
   static const OptionsStringResource resources[] = {
       {"autoSigninTitle", IDS_PASSWORDS_AUTO_SIGNIN_TITLE},
       {"autoSigninDescription", IDS_PASSWORDS_AUTO_SIGNIN_DESCRIPTION},
       {"savedPasswordsTitle", IDS_PASSWORD_MANAGER_SHOW_PASSWORDS_TAB_TITLE},
       {"passwordExceptionsTitle", IDS_PASSWORD_MANAGER_EXCEPTIONS_TAB_TITLE},
       {"passwordSearchPlaceholder", IDS_PASSWORDS_PAGE_SEARCH_PASSWORDS},
       {"passwordShowButton", IDS_PASSWORDS_PAGE_VIEW_SHOW_BUTTON},
       {"passwordHideButton", IDS_PASSWORDS_PAGE_VIEW_HIDE_BUTTON},
       {"passwordsNoPasswordsDescription",
        IDS_PASSWORDS_PAGE_VIEW_NO_PASSWORDS_DESCRIPTION},
       {"passwordsNoExceptionsDescription",
        IDS_PASSWORDS_PAGE_VIEW_NO_EXCEPTIONS_DESCRIPTION},
+      {"passwordManagerImportPasswordButtonText",
+       IDS_PASSWORD_MANAGER_IMPORT_BUTTON},
+      {"passwordManagerExportPasswordButtonText",
+       IDS_PASSWORD_MANAGER_EXPORT_BUTTON},
   };
 
   RegisterStrings(localized_strings, resources, arraysize(resources));
 
   const ProfileSyncService* sync_service =
       ProfileSyncServiceFactory::GetForProfile(GetProfile());
   int title_id =
       password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service)
           ? IDS_PASSWORD_MANAGER_SMART_LOCK_FOR_PASSWORDS
           : IDS_PASSWORDS_EXCEPTIONS_WINDOW_TITLE;
@@ skipping 46 matching lines @@
       base::Bind(&PasswordManagerHandler::HandleRemoveSavedPassword,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "removePasswordException",
       base::Bind(&PasswordManagerHandler::HandleRemovePasswordException,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "requestShowPassword",
       base::Bind(&PasswordManagerHandler::HandleRequestShowPassword,
                  base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "importPassword",
+      base::Bind(&PasswordManagerHandler::HandlePasswordImport,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "exportPassword",
+      base::Bind(&PasswordManagerHandler::HandlePasswordExport,
+                 base::Unretained(this)));
 }
 
 void PasswordManagerHandler::InitializeHandler() {
-  password_manager_presenter_.Initialize();
+  password_manager_presenter_->Initialize();
+}
+
+void PasswordManagerHandler::InitializePage() {
+  if (password_manager::ImportExportExperimentEnabled())
+    web_ui()->CallJavascriptFunction("PasswordManager.showImportExportButton");
 }
 
 void PasswordManagerHandler::HandleRemoveSavedPassword(
     const base::ListValue* args) {
   std::string string_value = base::UTF16ToUTF8(ExtractStringValue(args));
   int index;
   if (base::StringToInt(string_value, &index) && index >= 0) {
-    password_manager_presenter_.RemoveSavedPassword(static_cast<size_t>(index));
+    password_manager_presenter_->RemoveSavedPassword(
+        static_cast<size_t>(index));
   }
 }
 
 void PasswordManagerHandler::HandleRemovePasswordException(
     const base::ListValue* args) {
   std::string string_value = base::UTF16ToUTF8(ExtractStringValue(args));
   int index;
   if (base::StringToInt(string_value, &index) && index >= 0) {
-    password_manager_presenter_.RemovePasswordException(
+    password_manager_presenter_->RemovePasswordException(
         static_cast<size_t>(index));
   }
 }
 
 void PasswordManagerHandler::HandleRequestShowPassword(
     const base::ListValue* args) {
   int index;
   if (!ExtractIntegerValue(args, &index))
     NOTREACHED();
 
-  password_manager_presenter_.RequestShowPassword(static_cast<size_t>(index));
+  password_manager_presenter_->RequestShowPassword(static_cast<size_t>(index));
 }
 
 void PasswordManagerHandler::ShowPassword(
     size_t index,
     const std::string& origin_url,
     const std::string& username,
     const base::string16& password_value) {
   // Call back the front end to reveal the password.
   web_ui()->CallJavascriptFunction(
       "PasswordManager.showPassword",
       base::FundamentalValue(static_cast<int>(index)),
       base::StringValue(password_value));
 }
 
 void PasswordManagerHandler::HandleUpdatePasswordLists(
     const base::ListValue* args) {
-  password_manager_presenter_.UpdatePasswordLists();
+  password_manager_presenter_->UpdatePasswordLists();
 }
 
 void PasswordManagerHandler::SetPasswordList(
     const std::vector<scoped_ptr<autofill::PasswordForm>>& password_list,
     bool show_passwords) {
   base::ListValue entries;
   languages_ = GetProfile()->GetPrefs()->GetString(prefs::kAcceptLanguages);
   base::string16 placeholder(base::ASCIIToUTF16("        "));
   for (const auto& saved_password : password_list) {
     scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue);
@@ skipping 30 matching lines @@
   for (const auto& exception : password_exception_list) {
     scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue);
     CopyOriginInfoOfPasswordForm(*exception, languages_, entry.get());
     entries.Append(entry.release());
   }
 
   web_ui()->CallJavascriptFunction("PasswordManager.setPasswordExceptionsList",
                                    entries);
 }
 
+void PasswordManagerHandler::FileSelected(const base::FilePath& path,
+                                          int index,
+                                          void* params) {
+  switch (static_cast<FileSelectorCaller>(reinterpret_cast<intptr_t>(params))) {
+    case IMPORT_FILE_SELECTED:
+      ImportPasswordFileSelected(path);
+      break;
+    case EXPORT_FILE_SELECTED:
+      ExportPasswordFileSelected(path);
+      break;
+  }
+}
+
+void PasswordManagerHandler::HandlePasswordImport(const base::ListValue* args) {
+#if !defined(OS_ANDROID)  // This is never called on Android.
+  ui::SelectFileDialog::FileTypeInfo file_type_info;
+  file_type_info.extensions =
+      password_manager::PasswordImporter::GetSupportedFileExtensions();
+  DCHECK(!file_type_info.extensions.empty() &&
+         !file_type_info.extensions[0].empty());
+  file_type_info.include_all_files = true;
+  selected_file_dialog_ = ui::SelectFileDialog::Create(
+      this, new ChromeSelectFilePolicy(web_ui()->GetWebContents()));
+  selected_file_dialog_->SelectFile(
+      ui::SelectFileDialog::SELECT_OPEN_FILE,
+      l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_IMPORT_DIALOG_TITLE),
+      base::FilePath(), &file_type_info, 1, file_type_info.extensions[0][0],
+      web_ui()->GetWebContents()->GetTopLevelNativeWindow(),
+      reinterpret_cast<void*>(IMPORT_FILE_SELECTED));
+#endif
+}
+
+void PasswordManagerHandler::ImportPasswordFileSelected(
+    const base::FilePath& path) {
+  password_manager::PasswordImporter::Import(
+      path, content::BrowserThread::GetMessageLoopProxyForThread(
+                content::BrowserThread::FILE)
+                .get(),
+      base::Bind(&PasswordManagerHandler::ImportPasswordFileRead,

[vabr (Chromium), 2016/03/10 10:53:31]

What happens when the user closes the settings page while the import is in
progress? Is it really safe to have Unretained here?

[xunlu, 2016/03/16 07:23:58]

Maybe not, but the other options
here(https://www.chromium.org/developers/coding-style/important-abstractions-and-data-structures#TOC-base::Callback-and-base::Bind-)
don't seem to apply well either. Any suggestion?

[vabr (Chromium), 2016/03/16 17:48:28]

My suggestion is to split the functionality: Instead of PasswordManagerHandler
taking care of executing ImportPasswordFileRead directly, this functionality
should be transferred to a separate object (called, e.g.,
ImportPasswordResultConsumer, or some similar name). This object should be
refcounted, created by PasswordManagerHandler in a local scoped_refptr within
ImportPasswordFileSelected, and then just passed into the Bind call. The bind
call will increase the refcount, making sure that the object survives the
destruction of PasswordManagerHandler, and finishes the import.

[xunlu, 2016/03/18 21:15:30]

There is a problem:
ImportPasswordFileRead needs to use PasswordStore. And getting the PasswordStore
requires the Profile(e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...).
So unless the Profile object can outlive the handler object, splitting the
functionality is not going to help. I'm not sure about the life cycle of
Profile, can you confirm? 

[vabr (Chromium), 2016/03/21 14:01:30]

Yes, Profile definitely outlives the handler object. It represents the whole
Chrome profile, and outlives all Chrome windows for that profile.

The worst which can happen is that the helper object will ask for the password
store and will get a null pointer, because the user just closed the very last
tab, and Chrome is shutting down its services (including PasswordStore). In that
case the import will fail, but the code should not crash.

[xunlu, 2016/03/22 07:17:15]

Done.

+                 base::Unretained(this)));
+}
+
+void PasswordManagerHandler::ImportPasswordFileRead(
+    password_manager::PasswordImporter::Result result,
+    const std::vector<autofill::PasswordForm>& forms) {
+  UMA_HISTOGRAM_ENUMERATION("PasswordManager.ImportPasswordFromCSVResult",
+                            result,
+                            password_manager::PasswordImporter::SEMANTIC_ERROR);

[vabr (Chromium), 2016/03/10 10:53:31]

Please do not hard-code the fact that SEMANTIC_ERROR is last here. Also, the
boundary value should be strictly greater than the highest possible value.

To fix both, please define an additional enum value (NUM_IMPORT_RESULTS), make
it the last enum value of Result and use NUM_IMPORT_RESULTS as the boundary
value here. See
components/password_manager/core/browser/password_manager_metrics_util.h for
similar cases.

[xunlu, 2016/03/16 07:23:58]

Done.

+  if (result != password_manager::PasswordImporter::SUCCESS)
+    return;
+
+  UMA_HISTOGRAM_COUNTS("PasswordManager.ImportedPasswordsPerUserInCSV",
+                       forms.size());
+
+  password_manager::PasswordStore* store =
+      password_manager_presenter_->GetPasswordStore();

[vabr (Chromium), 2016/03/10 10:53:31]

Could you use PasswordStoreFactory::GetForProfile directly? That seems to make
more sense that using the presenter for it (the presenter should take care of
presenting the settings page, not providing the Chrome services). If you
PasswordStoreFactory::GetForProfile, please also restore the presenter's
GetPasswordStore as private again.

[xunlu, 2016/03/16 07:23:58]

Done.

+  if (store) {
+    for (const autofill::PasswordForm& form : forms) {
+      store->AddLogin(form);
+    }
+  } else {
+    UMA_HISTOGRAM_COUNTS("PasswordManager.FailToStorePasswordImportedFromCSV",
+                         1);
+  }
+}
+
+void PasswordManagerHandler::HandlePasswordExport(const base::ListValue* args) {
+#if !defined(OS_ANDROID)  // This is never called on Android.
+  if (!password_manager_presenter_->IsUserAuthenticated()) {
+    return;
+  }
+  ui::SelectFileDialog::FileTypeInfo file_type_info;
+  file_type_info.extensions =
+      password_manager::PasswordExporter::GetSupportedFileExtensions();
+  DCHECK(!file_type_info.extensions.empty() &&
+         !file_type_info.extensions[0].empty());
+  file_type_info.include_all_files = true;
+  selected_file_dialog_ = ui::SelectFileDialog::Create(
+      this, new ChromeSelectFilePolicy(web_ui()->GetWebContents()));
+  selected_file_dialog_->SelectFile(
+      ui::SelectFileDialog::SELECT_SAVEAS_FILE,
+      l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EXPORT_DIALOG_TITLE),
+      base::FilePath(), &file_type_info, 1, file_type_info.extensions[0][0],
+      GetNativeWindow(), reinterpret_cast<void*>(EXPORT_FILE_SELECTED));
+#endif
+}
+
+void PasswordManagerHandler::ExportPasswordFileSelected(
+    const base::FilePath& path) {
+  std::vector<scoped_ptr<autofill::PasswordForm>> password_list =
+      password_manager_presenter_->GetAllPasswords();
+  UMA_HISTOGRAM_COUNTS("PasswordManager.ExportedPasswordsPerUserInCSV",
+                       password_list.size());
+  password_manager::PasswordExporter::Export(
+      path, std::move(password_list),
+      content::BrowserThread::GetMessageLoopProxyForThread(
+          content::BrowserThread::FILE)
+          .get(),
+      base::Closure());
+}
+
 }  // namespace options