Display confirmation dialog for untrusted signins

chrome/browser/ui/sync/one_click_signin_helper.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/sync/one_click_signin_helper.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 #include <vector>
@@ skipping 211 matching lines @@
 }
 
 // Start syncing with the given user information.
 void StartSync(const StartSyncArgs& args,
                OneClickSigninSyncStarter::StartSyncMode start_mode) {
   if (start_mode == OneClickSigninSyncStarter::UNDO_SYNC) {
     LogOneClickHistogramValue(one_click_signin::HISTOGRAM_UNDO);
     return;
   }
 
-  // If we are giving the user the option to configure sync, then that will
-  // suffice as a confirmation.
   OneClickSigninSyncStarter::ConfirmationRequired confirmation =
       args.confirmation_required;

[Roger Tawa OOO till Jul 10th, 2013/06/20 07:51:29]

Can probably get rid of this local var too.  Its now only used once at line 239.

[noms (inactive), 2013/06/21 16:59:24]

Done.

-  if (start_mode == OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST &&
-      confirmation == OneClickSigninSyncStarter::CONFIRM_UNTRUSTED_SIGNIN) {
-    confirmation = OneClickSigninSyncStarter::CONFIRM_AFTER_SIGNIN;
-  }
 
   // The starter deletes itself once its done.
   new OneClickSigninSyncStarter(args.profile, args.browser, args.session_index,
                                 args.email, args.password, start_mode,
                                 args.force_same_tab_navigation,
                                 confirmation);
 
   int action = one_click_signin::HISTOGRAM_MAX;
   switch (args.auto_accept) {
     case OneClickSigninHelper::AUTO_ACCEPT_EXPLICIT:
@@ skipping 737 matching lines @@
       SyncPromoUI::GetNextPageURLForSyncPromoURL(
           SyncPromoUI::GetSyncPromoURL(SyncPromoUI::SOURCE_START_PAGE,
                                        false));
   GURL::Replacements replacements;
   replacements.ClearQuery();
 
   if (!IsValidGaiaSigninRedirectOrResponseURL(url) &&
       continue_url_.is_valid() &&
       url.ReplaceComponents(replacements) !=
           continue_url_.ReplaceComponents(replacements)) {
+    untrusted_confirmation_required_ = true;

[Roger Tawa OOO till Jul 10th, 2013/06/20 07:51:29]

I don't think this change and the one at line 1063 below are strictly required,
since the untrusted check is performed at the end of GAIA sign in process when
we get the G-A-S header.  However, these changes are useful in case the process
swap logic fails for some reason.

[noms (inactive), 2013/06/21 16:59:24]

As discussed, removed this change.
On 2013/06/20 07:51:29, Roger Tawa wrote:

     if (++untrusted_navigations_since_signin_visit_ > kMaxNavigationsSince)
       CleanTransientState();
   }
 }
 
 void OneClickSigninHelper::DidStopLoading(
     content::RenderViewHost* render_view_host) {
   // If the user left the sign in process, clear all members.
   // TODO(rogerta): might need to allow some youtube URLs.
   content::WebContents* contents = web_contents();
@@ skipping 50 matching lines @@
   if (email_.empty()) {
     VLOG(1) << "OneClickSigninHelper::DidStopLoading: nothing to do";
     if (continue_url_match && auto_accept_ == AUTO_ACCEPT_EXPLICIT)
       RedirectToSignin();
     std::string unused_value;
     if (net::GetValueForKeyInQuery(url, "ntp", &unused_value)) {
       SyncPromoUI::SetUserSkippedSyncPromo(profile);
       RedirectToNtpOrAppsPage(web_contents(), source_);
     }
 
-    if (!continue_url_match && !IsValidGaiaSigninRedirectOrResponseURL(url) &&
-        ++untrusted_navigations_since_signin_visit_ > kMaxNavigationsSince) {
-      CleanTransientState();
+    if (!continue_url_match && !IsValidGaiaSigninRedirectOrResponseURL(url)) {
+      untrusted_confirmation_required_ = true;
+      if (++untrusted_navigations_since_signin_visit_ > kMaxNavigationsSince)
+        CleanTransientState();
     }
 
     return;
   }
 
   // When the user uses the first-run, ntp, or hotdog menu to sign in, then have
   // the option of checking the the box "Let me choose what to sync".  When the
   // sign in process started, the source parameter in the continue URL may have
   // indicated one of the three options above.  However, once this box is
   // checked, the source parameter will indicate settings.  This will only be
@@ skipping 68 matching lines @@
       break;
     case AUTO_ACCEPT_CONFIGURE:
       LogOneClickHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
       LogOneClickHistogramValue(one_click_signin::HISTOGRAM_WITH_ADVANCED);
       SigninManager::DisableOneClickSignIn(profile);
       // Don't bother displaying an extra confirmation (even in the SAML case)
       // since the user will get prompted to setup sync anyway.
       StartSync(
           StartSyncArgs(profile, browser, auto_accept_, session_index_, email_,
                         password_, false /* force_same_tab_navigation */,
                         false /* confirmation_required */, source_),

[Roger Tawa OOO till Jul 10th, 2013/06/20 07:51:29]

I think we should change this false to a true as well.  Its not explicitly
mentioned in the bug, but we are disabling confirmation here because we assume
the advanced sync config dialog will handle aborting the sign in, but it no
longer does.

           OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
       break;
     case AUTO_ACCEPT_EXPLICIT: {
       if (switched_to_advanced_) {
         LogHistogramValue(original_source_,
                           one_click_signin::HISTOGRAM_WITH_ADVANCED);
         LogHistogramValue(original_source_,
                           one_click_signin::HISTOGRAM_ACCEPTED);
       } else {
         LogHistogramValue(source_, one_click_signin::HISTOGRAM_ACCEPTED);
@@ skipping 82 matching lines @@
     contents->GetController().LoadURL(redirect_url_,
                                       content::Referrer(),
                                       content::PAGE_TRANSITION_AUTO_TOPLEVEL,
                                       std::string());
   }
 
   // Clear the redirect URL.
   redirect_url_ = GURL();
   sync_service->RemoveObserver(this);
 }