Show confirmation dialog for unsecure signin

chrome/browser/ui/webui/signin/inline_login_handler_impl.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/signin/inline_login_handler_impl.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/about_signin_internals_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 #include "chrome/browser/ui/sync/one_click_signin_histogram.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/signin/inline_login_ui.h"
 #include "chrome/common/url_constants.h"
 #include "components/signin/core/browser/about_signin_internals.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/core/browser/signin_oauth_helper.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_ui.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/base/url_util.h"
 
 namespace {
 
 class InlineSigninHelper : public SigninOAuthHelper,
                            public SigninOAuthHelper::Consumer {
  public:
   InlineSigninHelper(
       base::WeakPtr<InlineLoginHandlerImpl> handler,
       net::URLRequestContextGetter* getter,
       Profile* profile,
       const GURL& current_url,
       const std::string& email,
       const std::string& password,
       const std::string& session_index,
       const std::string& signin_scoped_device_id,
-      bool choose_what_to_sync);
+      bool choose_what_to_sync,
+      bool confirm_untrusted_signin);
 
  private:
   // Overriden from SigninOAuthHelper::Consumer.
   virtual void OnSigninOAuthInformationAvailable(
       const std::string& email,
       const std::string& display_email,
       const std::string& refresh_token) OVERRIDE;
   virtual void OnSigninOAuthInformationFailure(
       const GoogleServiceAuthError& error) OVERRIDE;
 
   base::WeakPtr<InlineLoginHandlerImpl> handler_;
   Profile* profile_;
   GURL current_url_;
   std::string email_;
   std::string password_;
   std::string session_index_;
   bool choose_what_to_sync_;
+  bool confirm_untrusted_signin_;
 
   DISALLOW_COPY_AND_ASSIGN(InlineSigninHelper);
 };
 
 InlineSigninHelper::InlineSigninHelper(
     base::WeakPtr<InlineLoginHandlerImpl> handler,
     net::URLRequestContextGetter* getter,
     Profile* profile,
     const GURL& current_url,
     const std::string& email,
     const std::string& password,
     const std::string& session_index,
     const std::string& signin_scoped_device_id,
-    bool choose_what_to_sync)
+    bool choose_what_to_sync,
+    bool confirm_untrusted_signin)
     : SigninOAuthHelper(getter, session_index, signin_scoped_device_id, this),
       handler_(handler),
       profile_(profile),
       current_url_(current_url),
       email_(email),
       password_(password),
-      choose_what_to_sync_(choose_what_to_sync) {
+      session_index_(session_index),
+      choose_what_to_sync_(choose_what_to_sync),
+      confirm_untrusted_signin_(confirm_untrusted_signin) {
   DCHECK(profile_);
   DCHECK(!email_.empty());
 }
 
 void InlineSigninHelper::OnSigninOAuthInformationAvailable(
     const std::string& email,
     const std::string& display_email,
     const std::string& refresh_token) {
   content::WebContents* contents = NULL;
   Browser* browser = NULL;
@@ skipping 27 matching lines @@
     SigninErrorController* error_controller =
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->
             signin_error_controller();
     OneClickSigninSyncStarter::StartSyncMode start_mode =
         source == signin::SOURCE_SETTINGS || choose_what_to_sync_ ?
             (error_controller->HasError() &&
               sync_service && sync_service->HasSyncSetupCompleted()) ?
                 OneClickSigninSyncStarter::SHOW_SETTINGS_WITHOUT_CONFIGURE :
                 OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST :
                 OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS;
-    OneClickSigninSyncStarter::ConfirmationRequired confirmation_required =
-        source == signin::SOURCE_SETTINGS ||
-        source == signin::SOURCE_WEBSTORE_INSTALL ||
-        choose_what_to_sync_ ?
-            OneClickSigninSyncStarter::NO_CONFIRMATION :
-            OneClickSigninSyncStarter::CONFIRM_AFTER_SIGNIN;
+
+    OneClickSigninSyncStarter::ConfirmationRequired confirmation_required;
+    if (confirm_untrusted_signin_) {
+      confirmation_required =
+          OneClickSigninSyncStarter::CONFIRM_UNTRUSTED_SIGNIN;
+    } else {
+      confirmation_required =
+          source == signin::SOURCE_SETTINGS ||
+          source == signin::SOURCE_WEBSTORE_INSTALL ||
+          choose_what_to_sync_ ?
+              OneClickSigninSyncStarter::NO_CONFIRMATION :
+              OneClickSigninSyncStarter::CONFIRM_AFTER_SIGNIN;
+    }
 
     bool start_signin =
         !OneClickSigninHelper::HandleCrossAccountError(
             contents, "",
             email, password_, refresh_token,
             OneClickSigninHelper::AUTO_ACCEPT_EXPLICIT,
             source, start_mode,
             base::Bind(&InlineLoginHandlerImpl::SyncStarterCallback,
                        handler_));
     if (start_signin) {
@@ skipping 22 matching lines @@
     AboutSigninInternalsFactory::GetForProfile(profile_);
   about_signin_internals->OnRefreshTokenReceived("Failure");
 
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 }  // namespace
 
 InlineLoginHandlerImpl::InlineLoginHandlerImpl()
       : weak_factory_(this),
-        choose_what_to_sync_(false) {
+        confirm_untrusted_signin_(false) {
 }
 
 InlineLoginHandlerImpl::~InlineLoginHandlerImpl() {}
 
 bool InlineLoginHandlerImpl::HandleContextMenu(
     const content::ContextMenuParams& params) {
 #ifndef NDEBUG
   return false;
 #else
   return true;
 #endif
 }
 
+void InlineLoginHandlerImpl::DidCommitProvisionalLoadForFrame(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& url,
+    content::PageTransition transition_type) {
+  if (!web_contents())
+    return;
+
+  // Return early if this is not a gaia iframe navigation.
+  const GURL kGaiaExtOrigin(
+      "chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/");
+  content::RenderFrameHost* gaia_iframe = InlineLoginUI::GetAuthIframe(
+      web_contents(), kGaiaExtOrigin, "signin-frame");
+  if (render_frame_host != gaia_iframe)
+    return;
+
+  if (url.spec() != url::kAboutBlankURL &&

[Charlie Reis, 2014/07/23 21:42:00]

Let's put a comment here saying that loading any untrusted (e.g., HTTP) URLs in
the privileged sign-in process should require confirmation before the sign in
takes effect.

[guohui, 2014/07/23 21:57:51]

Done.

[guohui, 2014/07/23 21:57:51]

Done.

+      !gaia::IsGaiaSignonRealm(url.GetOrigin()) &&
+      !signin::IsContinueUrlForWebBasedSigninFlow(url)) {

[Charlie Reis, 2014/07/23 21:42:00]

Does the continue URL get loaded in the sign-in process?  Can it be HTTP?  That
could be a loophole.

[guohui, 2014/07/23 21:57:51]

The continue URL does get loaded in the signin process at the end of signin
flow. It is a fixed url as defined at,
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/sig...

Basically it is a blank page created solely for the chrome signin process that
is used to signal the completion of signin flow.

And in my test cl, i actually change the continue url to a local page in the
gaia auth extension. Either way this should not expose any security risk.

[Charlie Reis, 2014/07/23 22:18:06]

But it can be specified manually as a URL parameter, right?  That means we don't
have control over it, so I'm not sure why it's considered trusted.  Perhaps I'm
misunderstanding, but if it gets loaded in the process then it seems like it
poses a risk.

[guohui, 2014/07/23 23:19:46]

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/sig...

IsContinueUrlForWebBasedSigninFlow checks for a hardcoded URL thus we do have
control over it.

In some cases (webstore signin) we allow a customized continue URL, but the
customized continue URL param does not override the hardcoded url in the method
IsContinueUrlForWebBasedSigninFlow, thus the check should be good in all cases.

[Charlie Reis, 2014/07/23 23:36:12]

Acknowledged.

+    confirm_untrusted_signin_ = true;
+  }
+}
+
 void InlineLoginHandlerImpl::SetExtraInitParams(base::DictionaryValue& params) {
   params.SetString("service", "chromiumsync");
 
   content::WebContents* contents = web_ui()->GetWebContents();
   const GURL& current_url = contents->GetURL();
   std::string is_constrained;
   net::GetValueForKeyInQuery(current_url, "constrained", &is_constrained);
   if (is_constrained == "1")
     contents->SetDelegate(this);
 
+  content::WebContentsObserver::Observe(contents);

[Charlie Reis, 2014/07/23 21:42:00]

I think this means we only observe when the WebContents is used for sign-in? 
That's good.

Is there a point at which we can stop observing (e.g., Observe(NULL))?

[guohui, 2014/07/23 21:57:51]

We don't explicitly stop observing, so the observation will be stopped upon
destruction, which happens when the signin tab is closed.

[Charlie Reis, 2014/07/23 22:18:06]

Is it necessary to keep it around that long?  It seems like unnecessary overhead
once the sign in flow is done.

[guohui, 2014/07/23 23:19:46]

sorry i made a mistake in my earlier comment, this class is destroyed when the
inline signin is closed or when the tab navigates to some other URL. And once
signin completes, chrome will immediately navigate to either ntp or settings
page or close the tab, thus there should be not much overhead.

[Charlie Reis, 2014/07/23 23:36:12]

Ah, good.  That sounds good to me.

+
   signin::Source source = signin::GetSourceForPromoURL(current_url);
   OneClickSigninHelper::LogHistogramValue(
       source, one_click_signin::HISTOGRAM_SHOWN);
 }
 
 void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
   content::WebContents* contents = web_ui()->GetWebContents();
   const GURL& current_url = contents->GetURL();
 
   const base::DictionaryValue* dict = NULL;
   args->GetDictionary(0, &dict);
 
   bool skip_for_now = false;
   dict->GetBoolean("skipForNow", &skip_for_now);
   if (skip_for_now) {
     signin::SetUserSkippedPromo(Profile::FromWebUI(web_ui()));
     SyncStarterCallback(OneClickSigninSyncStarter::SYNC_SETUP_FAILURE);
     return;
   }
 
-  base::string16 email;
-  dict->GetString("email", &email);
-  DCHECK(!email.empty());
-  email_ = base::UTF16ToASCII(email);
-  base::string16 password;
-  dict->GetString("password", &password);
-  password_ = base::UTF16ToASCII(password);
+  base::string16 email_string16;
+  dict->GetString("email", &email_string16);
+  DCHECK(!email_string16.empty());
+  std::string email(base::UTF16ToASCII(email_string16));
+
+  base::string16 password_string16;
+  dict->GetString("password", &password_string16);
+  std::string password(base::UTF16ToASCII(password_string16));
 
   // When doing a SAML sign in, this email check may result in a false
   // positive.  This happens when the user types one email address in the
   // gaia sign in page, but signs in to a different account in the SAML sign in
   // page.
   std::string default_email;
   std::string validate_email;
   if (net::GetValueForKeyInQuery(current_url, "email", &default_email) &&
       net::GetValueForKeyInQuery(current_url, "validateEmail",
                                  &validate_email) &&
       validate_email == "1") {
-    if (!gaia::AreEmailsSame(email_, default_email)) {
+    if (!gaia::AreEmailsSame(email, default_email)) {
       SyncStarterCallback(OneClickSigninSyncStarter::SYNC_SETUP_FAILURE);
       return;
     }
   }
 
-  base::string16 session_index;
-  dict->GetString("sessionIndex", &session_index);
-  session_index_ = base::UTF16ToASCII(session_index);
-  DCHECK(!session_index_.empty());
-  dict->GetBoolean("chooseWhatToSync", &choose_what_to_sync_);
+  base::string16 session_index_string16;
+  dict->GetString("sessionIndex", &session_index_string16);
+  std::string session_index = base::UTF16ToASCII(session_index_string16);
+  DCHECK(!session_index.empty());
+
+  bool choose_what_to_sync = false;
+  dict->GetBoolean("chooseWhatToSync", &choose_what_to_sync);
 
   signin::Source source = signin::GetSourceForPromoURL(current_url);
   OneClickSigninHelper::LogHistogramValue(
       source, one_click_signin::HISTOGRAM_ACCEPTED);
   bool switch_to_advanced =
-      choose_what_to_sync_ && (source != signin::SOURCE_SETTINGS);
+      choose_what_to_sync && (source != signin::SOURCE_SETTINGS);
   OneClickSigninHelper::LogHistogramValue(
       source,
       switch_to_advanced ? one_click_signin::HISTOGRAM_WITH_ADVANCED :
                            one_click_signin::HISTOGRAM_WITH_DEFAULTS);
 
   OneClickSigninHelper::CanOfferFor can_offer_for =
       OneClickSigninHelper::CAN_OFFER_FOR_ALL;
   switch (source) {
     case signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT:
       can_offer_for = OneClickSigninHelper::CAN_OFFER_FOR_SECONDARY_ACCOUNT;
       break;
     case signin::SOURCE_REAUTH: {
       std::string primary_username =
           SigninManagerFactory::GetForProfile(
               Profile::FromWebUI(web_ui()))->GetAuthenticatedUsername();
       if (!gaia::AreEmailsSame(default_email, primary_username))
         can_offer_for = OneClickSigninHelper::CAN_OFFER_FOR_SECONDARY_ACCOUNT;
       break;
     }
     default:
       // No need to change |can_offer_for|.
       break;
   }
 
   std::string error_msg;
   bool can_offer = OneClickSigninHelper::CanOffer(
-      contents, can_offer_for, email_, &error_msg);
+      contents, can_offer_for, email, &error_msg);
   if (!can_offer) {
     HandleLoginError(error_msg);
     return;
   }
 
   AboutSigninInternals* about_signin_internals =
       AboutSigninInternalsFactory::GetForProfile(Profile::FromWebUI(web_ui()));
   about_signin_internals->OnAuthenticationResultReceived(
       "GAIA Auth Successful");
 
   content::StoragePartition* partition =
       content::BrowserContext::GetStoragePartitionForSite(
           contents->GetBrowserContext(),
           GURL(chrome::kChromeUIChromeSigninURL));
 
   SigninClient* signin_client =
       ChromeSigninClientFactory::GetForProfile(Profile::FromWebUI(web_ui()));
   std::string signin_scoped_device_id =
       signin_client->GetSigninScopedDeviceId();
   // InlineSigninHelper will delete itself.
   new InlineSigninHelper(GetWeakPtr(), partition->GetURLRequestContext(),
                          Profile::FromWebUI(web_ui()), current_url,
-                         email_, password_, session_index_,
-                         signin_scoped_device_id, choose_what_to_sync_);
+                         email, password, session_index,
+                         signin_scoped_device_id, choose_what_to_sync,
+                         confirm_untrusted_signin_);
 
-  email_.clear();
-  password_.clear();
-  session_index_.clear();
   web_ui()->CallJavascriptFunction("inline.login.closeDialog");
 }
 
 void InlineLoginHandlerImpl::HandleLoginError(const std::string& error_msg) {
   SyncStarterCallback(OneClickSigninSyncStarter::SYNC_SETUP_FAILURE);
 
   Browser* browser = GetDesktopBrowser();
   if (browser && !error_msg.empty()) {
     VLOG(1) << "InlineLoginHandlerImpl::HandleLoginError shows error message: "
             << error_msg;
     OneClickSigninHelper::ShowSigninErrorBubble(browser, error_msg);
   }
-
-  email_.clear();
-  password_.clear();
-  session_index_.clear();
 }
 
 Browser* InlineLoginHandlerImpl::GetDesktopBrowser() {
   Browser* browser = chrome::FindBrowserWithWebContents(
       web_ui()->GetWebContents());
   if (!browser) {
     browser = chrome::FindLastActiveWithProfile(
         Profile::FromWebUI(web_ui()), chrome::GetActiveDesktop());
   }
   return browser;
@@ skipping 40 matching lines @@
       }
     }
 
     if (show_account_management) {
       browser->window()->ShowAvatarBubbleFromAvatarButton(
             BrowserWindow::AVATAR_BUBBLE_MODE_ACCOUNT_MANAGEMENT,
             signin::ManageAccountsParams());
     }
   }
 }