Do not involve PasswordManagerDriver in filling HTTP-auth forms; also check realm

chrome/browser/ui/login/login_prompt.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 "chrome/browser/ui/login/login_prompt.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/prerender/prerender_contents.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/login/login_interstitial_delegate.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
-#include "components/password_manager/content/browser/content_password_manager_driver.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/url_formatter/elide_url.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 48 matching lines @@
     signon_realm = url.GetOrigin().spec();
     // This ends with a "/".
   }
   signon_realm.append(auth_info.realm);
   return signon_realm;
 }
 
 // ----------------------------------------------------------------------------
 // LoginHandler
 
+LoginHandler::LoginModelData::LoginModelData(
+    password_manager::LoginModel* login_model,
+    const autofill::PasswordForm& observed_form)
+    : model(login_model), form(observed_form) {
+  DCHECK(model);
+}
+
 LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info,
                            net::URLRequest* request)
     : handled_auth_(false),
       auth_info_(auth_info),
       request_(request),
       http_network_session_(
           request_->context()->http_transaction_factory()->GetSession()),
       password_manager_(NULL),
       login_model_(NULL) {
   // This constructor is called on the I/O thread, so we cannot load the nib
-  // here. BuildViewForPasswordManager() will be invoked on the UI thread
-  // later, so wait with loading the nib until then.
+  // here. BuildView() will be invoked on the UI thread later, so wait with
+  // loading the nib until then.
   DCHECK(request_) << "LoginHandler constructed with NULL request";
   DCHECK(auth_info_.get()) << "LoginHandler constructed with NULL auth info";
 
   AddRef();  // matched by LoginHandler::ReleaseSoon().
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&LoginHandler::AddObservers, this));
 
   if (!ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderFrame(
@@ skipping 23 matching lines @@
 }
 
 WebContents* LoginHandler::GetWebContentsForLogin() const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
       render_process_host_id_, render_frame_id_);
   return WebContents::FromRenderFrameHost(rfh);
 }
 
-password_manager::ContentPasswordManagerDriver*
-LoginHandler::GetPasswordManagerDriverForLogin() {
-  content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
-      render_process_host_id_, render_frame_id_);
-  return password_manager::ContentPasswordManagerDriver::GetForRenderFrameHost(
-      rfh);
+password_manager::PasswordManager* LoginHandler::GetPasswordManagerForLogin() {
+  ChromePasswordManagerClient* client =
+      ChromePasswordManagerClient::FromWebContents(GetWebContentsForLogin());
+  return client ? client->GetPasswordManager() : nullptr;
 }
 
 void LoginHandler::SetAuth(const base::string16& username,
                            const base::string16& password) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   scoped_ptr<password_manager::BrowserSavePasswordProgressLogger> logger;
   if (password_manager_ && password_manager_->client()->IsLoggingActive()) {
     logger.reset(new password_manager::BrowserSavePasswordProgressLogger(
         password_manager_->client()));
@@ skipping 103 matching lines @@
 }
 
 // Returns whether authentication had been handled (SetAuth or CancelAuth).
 bool LoginHandler::WasAuthHandled() const {
   base::AutoLock lock(handled_auth_lock_);
   bool was_handled = handled_auth_;
   return was_handled;
 }
 
 LoginHandler::~LoginHandler() {
-  SetModel(NULL);
+  ResetModel();
 }
 
-void LoginHandler::SetModel(password_manager::LoginModel* model) {
+void LoginHandler::SetModel(LoginModelData model_data) {
   if (login_model_)
     login_model_->RemoveObserver(this);

[Peter Kasting, 2015/10/07 22:27:07]

Nit: I wonder if this should be a call to ResetModel() instead?

It doesn't matter much for this implementation; the question would be, let's say
someday we want resetting the model to include some more complicated logic or
series of actions.  In that world, would we need to add that code to both these
functions?  If yes, then this should probably just call that.

[vabr (Chromium), 2015/10/08 10:24:12]

I was considering this, and did not find compelling reasons to do so. But you
found one, and I agree, so will switch to ResetModel here.
Done.

-  login_model_ = model;
+  login_model_ = model_data.model;
+  login_model_->AddObserverAndDeliverCredentials(this, model_data.form);
+}
+
+void LoginHandler::ResetModel() {
   if (login_model_)
-    login_model_->AddObserver(this);
+    login_model_->RemoveObserver(this);
+  login_model_ = nullptr;
 }
 
 void LoginHandler::NotifyAuthNeeded() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (WasAuthHandled())
     return;
 
   content::NotificationService* service =
       content::NotificationService::current();
   NavigationController* controller = NULL;
@@ skipping 126 matching lines @@
   if (!requesting_contents)
     return;
   // If a (blank) login interstitial was displayed, proceed so that the
   // navigation is committed.
   content::InterstitialPage* interstitial_page =
       requesting_contents->GetInterstitialPage();
   if (interstitial_page)
     interstitial_page->Proceed();
 }
 
-// Helper to create a PasswordForm and stuff it into a vector as input
-// for PasswordManager::PasswordFormsParsed, the hook into PasswordManager.
-void MakeInputForPasswordManager(
-    const GURL& request_url,
-    net::AuthChallengeInfo* auth_info,
-    LoginHandler* handler,
-    std::vector<PasswordForm>* password_manager_input) {
+// Helper to create a PasswordForm for PasswordManager to start looking for
+// saved credentials.
+PasswordForm MakeInputForPasswordManager(const GURL& request_url,
+                                         net::AuthChallengeInfo* auth_info,
+                                         LoginHandler* handler) {

[davidben, 2015/10/07 23:13:54]

(Existing unrelated issue, but this and the two functions below probably want to
be in some anonymous namespace since they're not exported outside this header.)

[vabr (Chromium), 2015/10/08 10:24:12]

Done.

   PasswordForm dialog_form;
   if (base::LowerCaseEqualsASCII(auth_info->scheme, "basic")) {
     dialog_form.scheme = PasswordForm::SCHEME_BASIC;
   } else if (base::LowerCaseEqualsASCII(auth_info->scheme, "digest")) {
     dialog_form.scheme = PasswordForm::SCHEME_DIGEST;
   } else {
     dialog_form.scheme = PasswordForm::SCHEME_OTHER;
   }
   std::string host_and_port(auth_info->challenger.ToString());
   if (auth_info->is_proxy) {
     std::string origin = host_and_port;
     // We don't expect this to already start with http:// or https://.
     DCHECK(origin.find("http://") != 0 && origin.find("https://") != 0);
     origin = std::string("http://") + origin;
     dialog_form.origin = GURL(origin);
   } else if (!auth_info->challenger.Equals(
       net::HostPortPair::FromURL(request_url))) {
     dialog_form.origin = GURL();
     NOTREACHED();  // crbug.com/32718
   } else {
     dialog_form.origin = GURL(request_url.scheme() + "://" + host_and_port);
   }
   dialog_form.signon_realm = GetSignonRealm(dialog_form.origin, *auth_info);
-  password_manager_input->push_back(dialog_form);
   // Set the password form for the handler (by copy).
   handler->SetPasswordForm(dialog_form);

[davidben, 2015/10/07 23:13:54]

(Existing unrelated issue, but it's pretty weird to have this function bother
return a PasswordForm and also stash it somewhere. Could remove the LoginHandler
parameter so this purely creates a PasswordForm and then move the
SetPasswordForm call to the caller.)

[vabr (Chromium), 2015/10/08 10:24:12]

Done.

+  return dialog_form;
 }
 
 void ShowLoginPrompt(const GURL& request_url,
                      net::AuthChallengeInfo* auth_info,
                      LoginHandler* handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   WebContents* parent_contents = handler->GetWebContentsForLogin();
   if (!parent_contents)
     return;
   prerender::PrerenderContents* prerender_contents =
       prerender::PrerenderContents::FromWebContents(parent_contents);
   if (prerender_contents) {
     prerender_contents->Destroy(prerender::FINAL_STATUS_AUTH_NEEDED);
     return;
   }
 
-  password_manager::ContentPasswordManagerDriver* driver =
-      handler->GetPasswordManagerDriverForLogin();
-
   // The realm is controlled by the remote server, so there is no reason
   // to believe it is of a reasonable length.
   base::string16 elided_realm;
   gfx::ElideString(base::UTF8ToUTF16(auth_info->realm), 120, &elided_realm);
 
   std::string languages;
   content::WebContents* web_contents = handler->GetWebContentsForLogin();
   if (web_contents) {
     Profile* profile =
         Profile::FromBrowserContext(web_contents->GetBrowserContext());
     if (profile)
       languages = profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
   }
 
   base::string16 authority =
       url_formatter::FormatUrlForSecurityDisplay(request_url, languages);
   base::string16 explanation =
       elided_realm.empty()
           ? l10n_util::GetStringFUTF16(IDS_LOGIN_DIALOG_DESCRIPTION_NO_REALM,
                                        authority)
           : l10n_util::GetStringFUTF16(IDS_LOGIN_DIALOG_DESCRIPTION, authority,
                                        elided_realm);
 
-  if (!driver) {
+  password_manager::PasswordManager* password_manager =
+      handler->GetPasswordManagerForLogin();
+
+  if (!password_manager) {
 #if defined(ENABLE_EXTENSIONS)
     // A WebContents in a <webview> (a GuestView type) does not have a password
     // manager, but still needs to be able to show login prompts.
     if (guest_view::GuestViewBase::FromWebContents(parent_contents)) {
-      handler->BuildViewForPasswordManager(nullptr, explanation);
+      handler->BuildView(explanation, nullptr);
       return;
     }
 #endif
     handler->CancelAuth();
     return;
   }
 
-  password_manager::PasswordManager* password_manager =
-      driver->GetPasswordManager();
   if (password_manager && password_manager->client()->IsLoggingActive()) {
     password_manager::BrowserSavePasswordProgressLogger logger(
         password_manager->client());
     logger.LogMessage(
         autofill::SavePasswordProgressLogger::STRING_SHOW_LOGIN_PROMPT_METHOD);
   }
 
-  // Tell the password manager to look for saved passwords.
-  std::vector<PasswordForm> v;
-  MakeInputForPasswordManager(request_url, auth_info, handler, &v);
-  driver->OnPasswordFormsParsedNoRenderCheck(v);
-  handler->SetPasswordManager(driver->GetPasswordManager());
+  handler->SetPasswordManager(password_manager);
 
-  handler->BuildViewForPasswordManager(driver->GetPasswordManager(),
-                                       explanation);
+  PasswordForm observed_form(
+      MakeInputForPasswordManager(request_url, auth_info, handler));
+  LoginHandler::LoginModelData model_data(password_manager, observed_form);
+  handler->BuildView(explanation, &model_data);

[davidben, 2015/10/07 23:13:54]

(Existing tangentially-related issue, but it's pretty weird to have LoginHandler
receive PasswordManager and PasswordForm in two ways, both in
SetPassword{Form,Manager} and as a parameter to BuildView.)

[vabr (Chromium), 2015/10/08 10:24:12]

Done.

 }
 
 // This callback is run on the UI thread and creates a constrained window with
 // a LoginView to prompt the user. If the prompt is triggered because of
 // a cross origin navigation in the main frame, a blank interstitial is first
 // created which in turn creates the LoginView. Otherwise, a LoginView is
 // directly in this callback. In both cases, the response will be sent to
 // LoginHandler, which then routes it to the net::URLRequest on the I/O thread.
 void LoginDialogCallback(const GURL& request_url,
                          net::AuthChallengeInfo* auth_info,
@@ skipping 53 matching lines @@
                                 net::URLRequest* request) {
   bool is_main_frame = (request->load_flags() & net::LOAD_MAIN_FRAME) != 0;
   LoginHandler* handler = LoginHandler::Create(auth_info, request);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&LoginDialogCallback, request->url(),
                  make_scoped_refptr(auth_info), make_scoped_refptr(handler),
                  is_main_frame));
   return handler;
 }