[signin] Add DICe flow for account consistency requests.

components/signin/core/browser/signin_header_helper.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 "components/signin/core/browser/signin_header_helper.h"
 
 #include <stddef.h>
 
+#include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "google_apis/gaia/gaia_auth_util.h"
@@ skipping 108 matching lines @@
                        (google_util::IsGoogleDomainUrl(
                             url, google_util::ALLOW_SUBDOMAIN,
                             google_util::DISALLOW_NON_STANDARD_PORTS) ||
                         google_util::IsYoutubeDomainUrl(
                             url, google_util::ALLOW_SUBDOMAIN,
                             google_util::DISALLOW_NON_STANDARD_PORTS));
   return is_google_url || IsDriveOrigin(origin) ||
          gaia::IsGaiaSignonRealm(origin);
 }
 
+// Checks if the url has the required properties to have a
+// X-Chrome-ID-Consistency-Request header.
+bool IsUrlEligibleForXChromeIDConsistencyRequestHeader(const GURL& url) {

[msarda, 2017/06/07 13:06:30]

IsUrlEligibleForDiceRequestHeadr

[droger, 2017/06/07 13:29:57]

I'm fine doing this but then I think we should rename
IsUrlEligibleForXChromeConnectedHeader() to IsUrlEligibleForMirrorHeader() for
consistency.

[msarda, 2017/06/08 02:53:38]

I think we should name this to IsUrlEligibleForDiceRequestHeader (but keep the
other one as IsUrlEligibleForXChromeConnectedHeader).

+  return gaia::IsGaiaSignonRealm(url.GetOrigin());
+}
+
 // Checks if the url has the required properties to have an account consistency
 // header.
 bool IsUrlEligibleForAccountConsistencyRequestHeader(const GURL& url) {
-  // TODO(droger): Support X-Chrome-ID-Consistency-Request.
-  return IsUrlEligibleForXChromeConnectedHeader(url);
+  switch (switches::GetAccountConsistencyMethod()) {
+    case switches::AccountConsistencyMethod::kDisabled:
+    // Mirror header can be added even if account consistency is disabled.

[msarda, 2017/06/07 13:06:30]

I think this should never be the case - we should never append the mirror header
when account consistency is disabled.

[droger, 2017/06/07 13:29:57]

As discussed offline, this currently happens: see line 134 just above.
If (IsDriveOrigin(origin) || gaia::IsGaiaSignonRealm(origin)), the header is
appended, even if mirror is disabled.

[msarda, 2017/06/08 02:53:38]

Agreed. Thank you for explaining this.

+    // Fall through.
+    case switches::AccountConsistencyMethod::kMirror:
+      return IsUrlEligibleForXChromeConnectedHeader(url);
+    case switches::AccountConsistencyMethod::kDice:
+      return IsUrlEligibleForXChromeIDConsistencyRequestHeader(url);
+  }
+
+  NOTREACHED();
+  return false;
 }
 
+// Builds the value of the header to be included in DICe requests.

[msarda, 2017/06/08 02:53:38]

s/DICe/Dice or DICE. What would you prefer?

+std::string BuildDiceRequestIfPossible(
+    const GURL& url,
+    const content_settings::CookieSettings* cookie_settings) {
+  // If signin cookies are not allowed, don't add the header.
+  if (!SettingsAllowSigninCookies(cookie_settings))
+    return std::string();
+
+  // Check if url is elligible for the header.
+  if (!IsUrlEligibleForXChromeIDConsistencyRequestHeader(url))
+    return std::string();
+
+  return "client_id=" + GaiaUrls::GetInstance()->oauth2_chrome_client_id();
+}
+
+// Builds the value of the header to be included in Mirror requests.
 std::string BuildMirrorRequestIfPossible(
     bool is_header_request,
     const GURL& url,
     const std::string& account_id,
     const content_settings::CookieSettings* cookie_settings,
     int profile_mode_mask) {
   if (account_id.empty())
     return std::string();
 
   // If signin cookies are not allowed, don't add the header.
-  if (!SettingsAllowSigninCookies(cookie_settings)) {
+  if (!SettingsAllowSigninCookies(cookie_settings))
     return std::string();
-  }
 
   // Check if url is elligible for the header.
   if (!IsUrlEligibleForXChromeConnectedHeader(url))
     return std::string();
 
   std::vector<std::string> parts;
   if (IsUrlEligibleToIncludeGaiaId(url, is_header_request)) {
     // Only set the GAIA Id on domains that actually requires it.
     parts.push_back(
         base::StringPrintf("%s=%s", kGaiaIdAttrName, account_id.c_str()));
   }
   parts.push_back(
       base::StringPrintf("%s=%s", kProfileModeAttrName,
                          base::IntToString(profile_mode_mask).c_str()));
   parts.push_back(base::StringPrintf(
       "%s=%s", kEnableAccountConsistencyAttrName,
       switches::IsAccountConsistencyMirrorEnabled() ? "true" : "false"));
 
   return base::JoinString(parts, is_header_request ? "," : ":");
 }
 
 }  // namespace
 
+extern const char kChromeIDConsistencyRequestHeader[] =

[msarda, 2017/06/08 02:53:38]

nit: move this below kChromeConnectedHeader to keep the same header as in the
header file.

+    "X-Chrome-ID-Consistency-Request";
 extern const char kChromeConnectedHeader[] = "X-Chrome-Connected";
 
 ManageAccountsParams::ManageAccountsParams()
     : service_type(GAIA_SERVICE_TYPE_NONE),
       email(""),
       is_saml(false),
       continue_url(""),
       is_same_tab(false) {
 #if !defined(OS_IOS)
   child_id = 0;
@@ skipping 24 matching lines @@
 }
 
 bool AppendOrRemoveAccountConsistentyRequestHeader(
     net::URLRequest* request,
     const GURL& redirect_url,
     const std::string& account_id,
     const content_settings::CookieSettings* cookie_settings,
     int profile_mode_mask) {
   const GURL& url = redirect_url.is_empty() ? request->url() : redirect_url;
 
-  // TODO(droger): Support X-Chrome-ID-Consistency-Request.
-  std::string header_name = kChromeConnectedHeader;
-  std::string header_value = BuildMirrorRequestIfPossible(
-      true /* is_header_request */, url, account_id, cookie_settings,
-      profile_mode_mask);
+  std::string header_name;
+  std::string header_value;
+  switch (switches::GetAccountConsistencyMethod()) {

[msarda, 2017/06/08 02:53:37]

I think after our discussion today, we need to do the following:
* we continue to send the X-Chrome-Connected header in the same cases as we do
today (independently if Dice is enabled or not)
* we send the new Dice header only when Dice is enabled.

WDYT?

+    case switches::AccountConsistencyMethod::kDice:
+      header_name = kChromeIDConsistencyRequestHeader;
+      header_value = BuildDiceRequestIfPossible(url, cookie_settings);
+      break;
+    case switches::AccountConsistencyMethod::kDisabled:

[msarda, 2017/06/07 13:06:30]

No, we must not append the mirror header if account consistency is disabled (we
may remove it in that case, but that would be strange). There is a possibility
that this code is never called when account consistency is enabled.

I think this should maybe be NOTREACHED.

May I ask you to test as follows:
* run chrome without mirror enabled
* sign in to chrome
* go to google.com
* check if we pass the header on this request (we *must* not pass that header in
this case)

[droger, 2017/06/07 13:29:57]

Same as above, this currently happens (I did not try though, I'm saying that
only from looking at the code).

+    // The mirror header is added even if account consistency is disabled.
+    // Fall through.
+    case switches::AccountConsistencyMethod::kMirror:
+      header_name = kChromeConnectedHeader;
+      header_value = BuildMirrorRequestIfPossible(
+          true /* is_header_request */, url, account_id, cookie_settings,
+          profile_mode_mask);
+      break;
+  }
 
   if (!header_name.empty() && header_value.empty()) {
     // If the request is being redirected, and it has the account consistency
     // header, and current url is a Google URL, and the redirected one is not,
     // remove the header.
     if (!redirect_url.is_empty() &&
         request->extra_request_headers().HasHeader(header_name) &&
         IsUrlEligibleForAccountConsistencyRequestHeader(request->url()) &&
         !IsUrlEligibleForAccountConsistencyRequestHeader(redirect_url)) {
       request->RemoveRequestHeaderByName(header_name);
@@ skipping 43 matching lines @@
       !response_headers->GetNormalizedHeader(
           kChromeManageAccountsHeader, &header_value)) {
     return empty_params;
   }
 
   DCHECK(switches::IsAccountConsistencyMirrorEnabled() && !is_off_the_record);
   return BuildManageAccountsParams(header_value);
 }
 
 }  // namespace signin