Implement inline signin with iframe

=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=
This is a dup of https://codereview.chromium.org/130963006/ since I cannot
upload to that issue.  The only change is to address Xiyuan's two comments
in patchset 3 of that CL.
=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=

Inline signin chrome://chrome-signin is currently implemented using webview embedded in webUI, which breaks a couple of features in webUI and has serious accessbility issues. Since webview will be reimplemented based on OOPIF in the near future, and all the issues we have today will no longer apply, thus it is not worth the effort to fix them as they are throw away work. Instead, as suggested by John and prototyped in https://codereview.chromium.org/141363006/, we decide to switch to iframe instead. A few issues worth to mention,

1. The iframe shares the same renderer as the embedder webUI, and thus could be potentially exposed to dangerous webUI privileges. John suggested a fix by assigning a unique storage partition ID to the inline signin page. As a result the inline signin and its embedded web content should never share the same renderer with other webUI pages.

2. webview provides a direct API to inject script and to monitor requests/responses, which is not (directly) available with iframe. The CL works around the issue using content script and background script, quite similar to what CrOS is doing for SAML flow today. Thus it is also the first step towards unifying SAML flows on CrOS and desktop.

3. with webview approach, we used to have a unique temporary partition for each instance of inline signin, in order to make sure multiple instances do not interfere with each other. This is more difficult with the iframe approach, since the partition ID is hardcoded in a quite low layer. In this CL, all inline signin pages share the same persistent partition, which means we have to handle the case when user loads the sign in page with a dirty cookie jar, and thus the newly connected user may not be stored in the primary session. The CL solves the issue by reading 'session_index' from 'google-accounts-signin' header.

BUG=338127

Review URL: https://codereview.chromium.org/134263005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251503 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/browser_resources.grd

@@ -293,7 +293,7 @@
       <include name="IDR_GAIA_AUTH_MANIFEST" file="resources\gaia_auth\manifest.json" type="BINDATA" />
       <include name="IDR_GAIA_AUTH_KEYBOARD_MANIFEST" file="resources\gaia_auth\manifest_keyboard.json" type="BINDATA" />
       <include name="IDR_GAIA_AUTH_SAML_MANIFEST" file="resources\gaia_auth\manifest_saml.json" type="BINDATA" />
-      <include name="IDR_GAIA_AUTH_INLINE_MANIFEST" file="resources\gaia_auth\manifest_inline.json" type="BINDATA" />
+      <include name="IDR_GAIA_AUTH_DESKTOP_MANIFEST" file="resources\gaia_auth\manifest_desktop.json" type="BINDATA" />
       <if expr="pp_ifdef('chromeos')">
         <include name="IDR_BACKLOADER_MANIFEST" file="resources\backloader\manifest.json" type="BINDATA" />
         <include name="IDR_CHOOSE_MOBILE_NETWORK_HTML" file="resources\chromeos\choose_mobile_network.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />

chrome/browser/chrome_content_browser_client.cc

@@ -802,6 +802,10 @@ void ChromeContentBrowserClient::GetStoragePartitionConfigForSite(
       *in_memory = false;
       partition_name->clear();
     }
+  } else if (site.GetOrigin().spec() == kChromeUIChromeSigninURL) {
+    // Chrome signin page has an embedded iframe of extension and web content,
+    // thus it must be isolated from other webUI pages.
+    *partition_domain = chrome::kChromeUIChromeSigninHost;
   }
 
   // Assert that if |can_be_default| is false, the code above must have found a

chrome/browser/extensions/signin/gaia_auth_extension_loader.cc

@@ -68,7 +68,7 @@ void LoadGaiaAuthExtension(Profile* profile) {
     manifest_resource_id = IDR_GAIA_AUTH_SAML_MANIFEST;
   }
 #else
-  int manifest_resource_id = IDR_GAIA_AUTH_INLINE_MANIFEST;
+  int manifest_resource_id = IDR_GAIA_AUTH_DESKTOP_MANIFEST;
 #endif
 
   component_loader->Add(manifest_resource_id,

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

@@ -38,6 +38,7 @@
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/mime_types_handler.h"
 #include "chrome/common/render_messages.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
@@ -612,6 +613,14 @@ void ChromeResourceDispatcherHostDelegate::OnResponseStarted(
     }
   }
 
+  // Ignores x-frame-options for the chrome signin UI.
+  if (request->first_party_for_cookies().GetOrigin().spec() ==
+      chrome::kChromeUIChromeSigninURL) {
+    net::HttpResponseHeaders* response_headers = request->response_headers();
+    if (response_headers->HasHeader("x-frame-options"))
+      response_headers->RemoveHeader("x-frame-options");
+  }
+
   prerender::URLRequestResponseStarted(request);
 }
 

chrome/browser/resources/component_extension_resources.grd

@@ -61,8 +61,7 @@
       <include name="IDR_GAIA_AUTH_BACKGROUND_JS" file="gaia_auth/background.js" type="BINDATA" />
       <include name="IDR_GAIA_AUTH_SAML_INJECTED_JS" file="gaia_auth/saml_injected.js" type="BINDATA" />
       <include name="IDR_GAIA_AUTH_CHANNEL_JS" file="gaia_auth/channel.js" type="BINDATA" />
-      <include name="IDR_GAIA_AUTH_INLINE_INJECTED_JS" file="gaia_auth/inline_injected.js" type="BINDATA" />
-      <include name="IDR_GAIA_AUTH_INLINE_MAIN" file="gaia_auth/inline_main.html" allowexternalscript="true" type="BINDATA" />
+      <include name="IDR_GAIA_AUTH_DESKTOP_INJECTED_JS" file="gaia_auth/desktop_injected.js" type="BINDATA" />
       <!-- Hangout Services extension, included in Google Chrome builds only. -->
       <if expr="pp_ifdef('_google_chrome') or pp_ifdef('enable_hangout_services_extension')">
         <include name="IDR_HANGOUT_SERVICES_BACKGROUND_HTML" file="hangout_services/background.html" type="BINDATA" />

chrome/browser/resources/gaia_auth/background.js

@@ -32,6 +32,21 @@ function BackgroundBridge() {
 }
 
 BackgroundBridge.prototype = {
+  // Continue URL that is set from main auth script.
+  continueUrl_: null,
+
+  // Whether the extension is loaded in a constrained window.
+  // Set from main auth script.
+  isConstrainedWindow_: null,
+
+  // Email of the newly authenticated user based on the gaia response header
+  // 'google-accounts-signin'.
+  email_: null,
+
+  // Session index of the newly authenticated user based on the gaia response
+  // header 'google-accounts-signin'.
+  sessionIndex_: null,
+
   // Gaia URL base that is set from main auth script.
   gaiaUrl_: null,
 
@@ -41,8 +56,8 @@ BackgroundBridge.prototype = {
 
   passwordStore_: {},
 
-  channelMain_: null,
-  channelInjected_: null,
+  channelMain_: {},
+  channelInjected_: {},
 
   run: function() {
     chrome.runtime.onConnect.addListener(this.onConnect_.bind(this));
@@ -79,31 +94,129 @@ BackgroundBridge.prototype = {
    * Sets up the communication channel with the main script.
    */
   setupForAuthMain_: function(port) {
-    this.channelMain_ = new Channel();
-    this.channelMain_.init(port);
-    this.channelMain_.registerMessage(
+    var currentChannel = new Channel();
+    currentChannel.init(port);
+
+    // Registers for desktop related messages.
+    currentChannel.registerMessage(
+        'initDesktopFlow', this.onInitDesktopFlow_.bind(this));
+
+    // Registers for SAML related messages.
+    currentChannel.registerMessage(
         'setGaiaUrl', this.onSetGaiaUrl_.bind(this));
-    this.channelMain_.registerMessage(
+    currentChannel.registerMessage(
         'resetAuth', this.onResetAuth_.bind(this));
-    this.channelMain_.registerMessage(
+    currentChannel.registerMessage(
         'startAuth', this.onAuthStarted_.bind(this));
-    this.channelMain_.registerMessage(
+    currentChannel.registerMessage(
         'getScrapedPasswords',
         this.onGetScrapedPasswords_.bind(this));
+
+    this.channelMain_[this.getTabIdFromPort_(port)] = currentChannel;
+
   },
 
   /**
    * Sets up the communication channel with the injected script.
    */
   setupForInjected_: function(port) {
-    this.channelInjected_ = new Channel();
-    this.channelInjected_.init(port);
-    this.channelInjected_.registerMessage(
-        'apiCall', this.onAPICall_.bind(this));
-    this.channelInjected_.registerMessage(
+    var currentChannel = new Channel();
+    currentChannel.init(port);
+
+    var tabId = this.getTabIdFromPort_(port);
+    currentChannel.registerMessage(
+        'apiCall', this.onAPICall_.bind(this, tabId));
+    currentChannel.registerMessage(
         'updatePassword', this.onUpdatePassword_.bind(this));
-    this.channelInjected_.registerMessage(
-        'pageLoaded', this.onPageLoaded_.bind(this));
+    currentChannel.registerMessage(
+        'pageLoaded', this.onPageLoaded_.bind(this, tabId));
+
+    this.channelInjected_[this.getTabIdFromPort_(port)] = currentChannel;
+  },
+
+  getTabIdFromPort_: function(port) {
+    return port.sender.tab ? port.sender.tab.id : -1;
+  },
+
+  /**
+   * Handler for 'initDesktopFlow' signal sent from the main script.
+   * Only called in desktop mode.
+   */
+  onInitDesktopFlow_: function(msg) {
+    this.gaiaUrl_ = msg.gaiaUrl;
+    this.continueUrl_ = msg.continueUrl;
+    this.isConstrainedWindow_ = msg.isConstrainedWindow;
+
+    var urls = [];
+    var filter = {urls: urls, types: ['sub_frame']};
+    var optExtraInfoSpec = [];
+    if (msg.isConstrainedWindow) {
+      urls.push('<all_urls>');
+      optExtraInfoSpec.push('responseHeaders');
+    } else {
+      urls.push(this.continueUrl_ + '*');
+    }
+
+    chrome.webRequest.onCompleted.addListener(
+        this.onRequestCompletedInDesktopMode_.bind(this),
+        filter, optExtraInfoSpec);
+    chrome.webRequest.onHeadersReceived.addListener(
+        this.onHeadersReceivedInDesktopMode_.bind(this),
+        {urls: [this.gaiaUrl_ + '*'], types: ['sub_frame']},
+        ['responseHeaders']);
+  },
+
+  /**
+   * Event listener for webRequest.onCompleted in desktop mode.
+   */
+  onRequestCompletedInDesktopMode_: function(details) {
+    var msg = null;
+    if (details.url.lastIndexOf(this.continueUrl_, 0) == 0) {
+      var skipForNow = false;
+      if (details.url.indexOf('ntp=1') >= 0) {
+        skipForNow = true;
+      }
+      msg = {
+        'name': 'completeLogin',
+        'email': this.email_,
+        'sessionIndex': this.sessionIndex_,
+        'skipForNow': skipForNow
+      };
+    } else if (this.isConstrainedWindow_) {
+      var headers = details.responseHeaders;
+      for (var i = 0; headers && i < headers.length; ++i) {
+        if (headers[i].name.toLowerCase() == 'google-accounts-embedded') {
+          return;
+        }
+      }
+      msg = {
+        'name': 'switchToFullTab',
+        'url': details.url
+      };
+    }
+
+    if (msg != null)
+      this.channelMain_[details.tabId].send(msg);
+  },
+
+  /**
+   * Event listener for webRequest.onHeadersReceived in desktop mode.
+   */
+  onHeadersReceivedInDesktopMode_: function(details) {
+    var headers = details.responseHeaders;
+    for (var i = 0; headers && i < headers.length; ++i) {
+      if (headers[i].name.toLowerCase() == 'google-accounts-signin') {
+        var headerValues = headers[i].value.toLowerCase().split(',');
+        var signinDetails = {};
+        headerValues.forEach(function(e) {
+          var pair = e.split('=');
+          signinDetails[pair[0].trim()] = pair[1].trim();
+        });
+        this.email_ = signinDetails['email'].slice(1, -1); // Remove "" around.
+        this.sessionIndex_ = signinDetails['sessionindex'];
+        return;
+      }
+    }
   },
 
   /**
@@ -153,8 +266,10 @@ BackgroundBridge.prototype = {
     return Object.keys(passwords);
   },
 
-  onAPICall_: function(msg) {
-    this.channelMain_.send(msg);
+  onAPICall_: function(tabId, msg) {
+    if (tabId in this.channelMain_) {
+      this.channelMain_[tabId].send(msg);
+    }
   },
 
   onUpdatePassword_: function(msg) {
@@ -164,8 +279,10 @@ BackgroundBridge.prototype = {
     this.passwordStore_[msg.id] = msg.password;
   },
 
-  onPageLoaded_: function(msg) {
-    this.channelMain_.send({name: 'onAuthPageLoaded', url: msg.url});
+  onPageLoaded_: function(tabId, msg) {
+    if (tabId in this.channelMain_) {
+      this.channelMain_[tabId].send({name: 'onAuthPageLoaded', url: msg.url});
+    }
   }
 };
 

chrome/browser/resources/gaia_auth/inline_injected.js → chrome/browser/resources/gaia_auth/desktop_injected.js

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2014 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.
 
@@ -11,23 +11,14 @@
  */
 
 (function() {
-  var extWindow;
-
   var $ = function(id) { return document.getElementById(id); };
-  var gaiaLoginForm = $('gaia_loginform');
 
-  var onMessage = function(e) {
-    extWindow = e.source;
-  };
-  window.addEventListener('message', onMessage);
+  var gaiaLoginForm = $('gaia_loginform');
+  if (!gaiaLoginForm) {
+    return;
+  }
 
   var onLoginSubmit = function(e) {
-    if (!extWindow) {
-      console.log('ERROR: no initial message received from the gaia ext');
-      e.preventDefault();
-      return;
-    }
-
     var checkboxElement = $('advanced-box');
     var chooseWhatToSync = checkboxElement && checkboxElement.checked;
     var msg = {method: 'attemptLogin',
@@ -36,7 +27,8 @@
                attemptToken: new Date().getTime(),
                chooseWhatToSync: chooseWhatToSync};
 
-    extWindow.postMessage(msg, 'chrome://chrome-signin');
+    window.parent.postMessage(
+        msg, 'chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik');
     console.log('Credentials sent');
 
     return;

chrome/browser/resources/gaia_auth/inline_main.html

-<!DOCTYPE html>
-<html>
-<head>
-  <link rel="stylesheet" href="main.css">
-  <meta charset="utf-8">
-  <script src="channel.js"></script>
-  <script src="util.js"></script>
-  <script src="main.js"></script>
-</head>
-<body>
-  <webview id="gaia-frame"></webview>
-</body>
-</html>

chrome/browser/resources/gaia_auth/manifest_inline.json → chrome/browser/resources/gaia_auth/manifest_desktop.json

@@ -4,29 +4,29 @@
   "name": "GaiaAuthExtension",
   "version": "0.0.1",
   "manifest_version": 2,
+  "background" : {
+    "scripts": ["background.js", "channel.js"]
+  },
+  "content_scripts": [
+    {
+      "matches": [
+        "<all_urls>"
+      ],
+      "js": ["channel.js", "desktop_injected.js"],
+      "all_frames": true
+    }
+  ],
   "content_security_policy": "default-src 'self'; script-src 'self'; frame-src *; style-src 'self' 'unsafe-inline'",
   "description": "GAIA Component Extension",
   "web_accessible_resources": [
     "main.css",
-    "inline_main.html",
+    "main.html",
     "main.js",
-    "offline.css",
-    "offline.html",
-    "offline.js",
-    "success.html",
-    "success.js",
     "util.js"
   ],
-  // <all_urls> for intercepting all URL requests in the main frame, and
-  //     switching to a full tab if needed.
-  // cookies for getting hash passed back from GAIA on login success.
-  // tabs for calling current webui's login. This might not be needed once
-  //     we have extension API.
-  // webview for interacting with the GAIA sign in page loaded in a webview.
   "permissions": [
-      "<all_urls>",
-      "cookies",
-      "tabs",
-      "webview"
+    "<all_urls>",
+    "tabs",
+    "webRequest"
   ]
 }

chrome/browser/resources/gaia_auth_host/gaia_auth_host.js

@@ -34,12 +34,6 @@ cr.define('cr.login', function() {
    */
   var OFFLINE_AUTH_URL = AUTH_URL_BASE + '/offline.html';
 
-  /**
-   * Auth URL to use for inline flow.
-   * @const
-   */
-  var INLINE_AUTH_URL = AUTH_URL_BASE + '/inline_main.html';
-
   /**
    * Origin of the gaia sign in page.
    * @const
@@ -59,7 +53,6 @@ cr.define('cr.login', function() {
     'email',         // Pre-fill the email field in Gaia UI;
     'service',       // Name of Gaia service;
     'continueUrl',   // Continue url to use;
-    'partitionId',   // Partition ID for the embedded Gaia webview;
     'frameUrl',      // Initial frame URL to use. If empty defaults to gaiaUrl.
     'constrained'    // Whether the extension is loaded in a constrained window;
   ];
@@ -87,7 +80,7 @@ cr.define('cr.login', function() {
   var AuthMode = {
     DEFAULT: 0,
     OFFLINE: 1,
-    INLINE: 2
+    DESKTOP: 2
   };
 
   /**
@@ -111,8 +104,6 @@ cr.define('cr.login', function() {
     assert(this.frame_);
     window.addEventListener('message',
                             this.onMessage_.bind(this), false);
-    window.addEventListener('popstate',
-                            this.onPopState_.bind(this), false);
   }
 
   GaiaAuthHost.prototype = {
@@ -140,7 +131,7 @@ cr.define('cr.login', function() {
      *   email: 'xx@gmail.com',
      *   password: 'xxxx',  // May not present
      *   authCode: 'x/xx',  // May not present
-     *   authMode: 'x',     // Authorization mode, default/inline/offline.
+     *   authMode: 'x',     // Authorization mode, default/offline/desktop.
      * }
      * }
      * </pre>
@@ -245,9 +236,9 @@ cr.define('cr.login', function() {
         case AuthMode.OFFLINE:
           url = OFFLINE_AUTH_URL;
           break;
-        case AuthMode.INLINE:
-          url = INLINE_AUTH_URL;
-          params.push('inlineMode=1');
+        case AuthMode.DESKTOP:
+          url = AUTH_URL;
+          params.push('desktopMode=1');
           break;
         default:
           url = AUTH_URL;
@@ -326,17 +317,6 @@ cr.define('cr.login', function() {
     onMessage_: function(e) {
       var msg = e.data;
 
-      // In the inline sign in flow, the embedded gaia webview posts credential
-      // directly to the inline sign in page, because its parent JavaScript
-      // reference points to the top frame of the embedder instead of the sub
-      // frame of the gaia auth extension.
-      if (e.origin == GAIA_ORIGIN && msg.method == 'attemptLogin') {
-        this.email_ = msg.email;
-        this.password_ = msg.password;
-        this.chooseWhatToSync_ = msg.chooseWhatToSync;
-        return;
-      }
-
       if (!this.isAuthExtMessage_(e))
         return;
 
@@ -351,13 +331,13 @@ cr.define('cr.login', function() {
           this.frame_.contentWindow.postMessage(msg, AUTH_URL_BASE);
           return;
         }
-        this.onAuthSuccess_({email: msg.email || this.email_,
-                             password: msg.password || this.password_,
-                             authCode: msg.authCode,
+        this.onAuthSuccess_({email: msg.email,
+                             password: msg.password,
                              useOffline: msg.method == 'offlineLogin',
                              usingSAML: msg.usingSAML || false,
-                             chooseWhatToSync: this.chooseWhatToSync_,
-                             skipForNow: msg.skipForNow || false });
+                             chooseWhatToSync: msg.chooseWhatToSync,
+                             skipForNow: msg.skipForNow || false,
+                             sessionIndex: msg.sessionIndex || ''});
         return;
       }
 
@@ -394,39 +374,12 @@ cr.define('cr.login', function() {
         return;
       }
 
-      if (msg.method == 'reportState') {
-        var newUrl = setQueryParam(location, 'frameUrl', msg.src);
-        if (history.state) {
-          if (history.state.src != msg.src) {
-            history.pushState({src: msg.src}, '', newUrl);
-          }
-        } else {
-          history.replaceState({src: msg.src});
-        }
-        return;
-      }
-
       if (msg.method == 'switchToFullTab') {
         chrome.send('switchToFullTab', [msg.url]);
         return;
       }
 
       console.error('Unknown message method=' + msg.method);
-    },
-
-    /**
-     * Event handler that is invoked when the history state is changed.
-     * @param {object} e The popstate event being triggered.
-     */
-    onPopState_: function(e) {
-      var state = e.state;
-      if (state) {
-        var msg = {
-          method: 'navigate',
-          src: state.src
-        };
-        this.frame_.contentWindow.postMessage(msg, AUTH_URL_BASE);
-      }
     }
   };
 

chrome/browser/resources/inline_login/inline_login.js

@@ -49,10 +49,7 @@ cr.define('inline.login', function() {
    */
   function loadAuthExtension(data) {
     authExtHost.load(data.authMode, data, onAuthCompleted);
-    // Do not show loading spinner to give user a faster response
-    // with inline flows.
-    $('contents').classList.toggle('loading',
-        data.authMode != cr.login.GaiaAuthHost.AuthMode.INLINE);
+    $('contents').classList.toggle('loading', true);
   }
 
   /**

chrome/browser/ui/webui/signin/inline_login_handler.h

@@ -22,7 +22,7 @@ class InlineLoginHandler : public content::WebUIMessageHandler {
   enum AuthMode {
     kDefaultAuthMode = 0,
     kOfflineAuthMode = 1,
-    kInlineAuthMode = 2
+    kDesktopAuthMode = 2
   };
 
  private:

chrome/browser/ui/webui/signin/inline_login_handler_impl.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/webui/signin/inline_login_handler_impl.h"
 
-#include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
@@ -20,6 +19,7 @@
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
@@ -30,13 +30,10 @@
 
 namespace {
 
-// Global SequenceNumber used for generating unique webview partition IDs.
-base::StaticAtomicSequenceNumber next_partition_id;
-
 } // empty namespace
 
 InlineLoginHandlerImpl::InlineLoginHandlerImpl()
-      : weak_factory_(this), choose_what_to_sync_(false), partition_id_("") {
+      : weak_factory_(this), choose_what_to_sync_(false) {
 }
 
 InlineLoginHandlerImpl::~InlineLoginHandlerImpl() {}
@@ -50,7 +47,7 @@ void InlineLoginHandlerImpl::RegisterMessages() {
 }
 
 void InlineLoginHandlerImpl::SetExtraInitParams(base::DictionaryValue& params) {
-  params.SetInteger("authMode", InlineLoginHandler::kInlineAuthMode);
+  params.SetInteger("authMode", InlineLoginHandler::kDesktopAuthMode);
 
   const GURL& current_url = web_ui()->GetWebContents()->GetURL();
   signin::Source source = signin::GetSourceForPromoURL(current_url);
@@ -92,13 +89,6 @@ void InlineLoginHandlerImpl::SetExtraInitParams(base::DictionaryValue& params) {
   net::GetValueForKeyInQuery(current_url, "readOnlyEmail", &read_only_email);
   if (!read_only_email.empty())
     params.SetString("readOnlyEmail", read_only_email);
-
-  net::GetValueForKeyInQuery(current_url, "partitionId", &partition_id_);
-  if (partition_id_.empty()) {
-    partition_id_ =
-        "gaia-webview-" + base::IntToString(next_partition_id.GetNext());
-  }
-  params.SetString("partitionId", partition_id_);
 }
 
 
@@ -111,8 +101,6 @@ void InlineLoginHandlerImpl::HandleSwitchToFullTabMessage(
   GURL main_frame_url(web_contents->GetURL());
   main_frame_url = net::AppendOrReplaceQueryParameter(
       main_frame_url, "frameUrl", UTF16ToASCII(url_str));
-  main_frame_url = net::AppendOrReplaceQueryParameter(
-      main_frame_url, "partitionId", partition_id_);
   chrome::NavigateParams params(
       Profile::FromWebUI(web_ui()),
       net::AppendOrReplaceQueryParameter(main_frame_url, "constrained", "0"),
@@ -123,7 +111,7 @@ void InlineLoginHandlerImpl::HandleSwitchToFullTabMessage(
 }
 
 void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
-  DCHECK(email_.empty() && password_.empty());
+  DCHECK(email_.empty() && password_.empty() && session_index_.empty());
 
   content::WebContents* contents = web_ui()->GetWebContents();
   const GURL& current_url = contents->GetURL();
@@ -162,6 +150,10 @@ void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
     }
   }
 
+  base::string16 session_index;
+  dict->GetString("sessionIndex", &session_index);
+  session_index_ = UTF16ToASCII(session_index);
+  DCHECK(!session_index_.empty());
   dict->GetBoolean("chooseWhatToSync", &choose_what_to_sync_);
 
   signin::Source source = signin::GetSourceForPromoURL(current_url);
@@ -180,13 +172,12 @@ void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
   content::StoragePartition* partition =
       content::BrowserContext::GetStoragePartitionForSite(
           contents->GetBrowserContext(),
-          GURL("chrome-guest://mfffpogegjflfpflabcdkioaeobkgjik/?" +
-                partition_id_));
+          GURL(chrome::kChromeUIChromeSigninURL));
 
   auth_fetcher_.reset(new GaiaAuthFetcher(this,
                                           GaiaConstants::kChromeSource,
                                           partition->GetURLRequestContext()));
-  auth_fetcher_->StartCookieForOAuthCodeExchange("0");
+  auth_fetcher_->StartCookieForOAuthCodeExchange(session_index_);
 }
 
 void InlineLoginHandlerImpl::OnClientOAuthCodeSuccess(
@@ -253,6 +244,7 @@ void InlineLoginHandlerImpl::OnClientOAuthCodeSuccess(
 
   email_.clear();
   password_.clear();
+  session_index_.clear();
   web_ui()->CallJavascriptFunction("inline.login.closeDialog");
 }
 
@@ -276,6 +268,7 @@ void InlineLoginHandlerImpl::HandleLoginError(const std::string& error_msg) {
 
   email_.clear();
   password_.clear();
+  session_index_.clear();
 }
 
 void InlineLoginHandlerImpl::SyncStarterCallback(

chrome/browser/ui/webui/signin/inline_login_handler_impl.h

@@ -43,9 +43,8 @@ class InlineLoginHandlerImpl : public GaiaAuthConsumer,
   scoped_ptr<GaiaAuthFetcher> auth_fetcher_;
   std::string email_;
   std::string password_;
+  std::string session_index_;
   bool choose_what_to_sync_;
-  // Partition id for the gaia webview;
-  std::string partition_id_;
 
   DISALLOW_COPY_AND_ASSIGN(InlineLoginHandlerImpl);
 };

chrome/browser/ui/webui/signin/inline_login_ui.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/webui/signin/inline_login_ui.h"
 
+#include "chrome/browser/extensions/extension_web_contents_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_ui.h"
@@ -22,6 +23,7 @@ namespace {
 content::WebUIDataSource* CreateWebUIDataSource() {
   content::WebUIDataSource* source =
         content::WebUIDataSource::Create(chrome::kChromeUIChromeSigninHost);
+  source->OverrideContentSecurityPolicyFrameSrc("frame-src chrome-extension:;");
   source->SetUseJsonJSFormatV2();
   source->SetJsonPath("strings.js");
 
@@ -48,7 +50,8 @@ InlineLoginUI::InlineLoginUI(content::WebUI* web_ui)
   // Required for intercepting extension function calls when the page is loaded
   // in a bubble (not a full tab, thus tab helpers are not registered
   // automatically).
-  extensions::TabHelper::CreateForWebContents(web_ui->GetWebContents());
+  extensions::ExtensionWebContentsObserver::CreateForWebContents(
+      web_ui->GetWebContents());
 #endif
 }
 

chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc

+// Copyright 2014 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/signin/signin_promo.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/test_chrome_web_ui_controller_factory.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/session_storage_namespace.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui_controller.h"
+#include "content/public/common/url_constants.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+
+namespace {
+
+struct ContentInfo {
+  ContentInfo(int pid, content::StoragePartition* storage_partition) {
+    this->pid = pid;
+    this->storage_partition = storage_partition;
+  }
+
+  int pid;
+  content::StoragePartition* storage_partition;
+};
+
+ContentInfo NavigateAndGetInfo(
+    Browser* browser,
+    const GURL& url,
+    WindowOpenDisposition disposition) {
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser, url, disposition,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  content::WebContents* contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  content::RenderProcessHost* process = contents->GetRenderProcessHost();
+  return ContentInfo(process->GetID(), process->GetStoragePartition());
+}
+
+// Returns a new WebUI object for the WebContents from |arg0|.
+ACTION(ReturnNewWebUI) {
+  return new content::WebUIController(arg0);
+}
+
+// Mock the TestChromeWebUIControllerFactory::WebUIProvider to prove that we are
+// not called as expected.
+class FooWebUIProvider
+    : public TestChromeWebUIControllerFactory::WebUIProvider {
+ public:
+  MOCK_METHOD2(NewWebUI, content::WebUIController*(content::WebUI* web_ui,
+                                                   const GURL& url));
+};
+
+const char kFooWebUIURL[] = "chrome://foo/";
+
+}  // namespace
+
+class InlineLoginUIBrowserTest : public InProcessBrowserTest {
+ public:
+  InlineLoginUIBrowserTest() {}
+};
+
+IN_PROC_BROWSER_TEST_F(InlineLoginUIBrowserTest, DifferentStorageId) {
+  GURL test_url = ui_test_utils::GetTestUrl(
+      base::FilePath(base::FilePath::kCurrentDirectory),
+      base::FilePath(FILE_PATH_LITERAL("title1.html")));
+
+  ContentInfo info1 =
+      NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
+  ContentInfo info2 =
+      NavigateAndGetInfo(browser(),
+                         signin::GetPromoURL(signin::SOURCE_START_PAGE, false),
+                         CURRENT_TAB);
+  NavigateAndGetInfo(browser(), test_url, CURRENT_TAB);
+  ContentInfo info3 =
+      NavigateAndGetInfo(browser(),
+                         signin::GetPromoURL( signin::SOURCE_START_PAGE, false),
+                         NEW_FOREGROUND_TAB);
+
+  // The info for signin should be the same.
+  ASSERT_EQ(info2.storage_partition, info3.storage_partition);
+  // The info for test_url and signin should be different.
+  ASSERT_NE(info1.storage_partition, info2.storage_partition);
+}
+
+IN_PROC_BROWSER_TEST_F(InlineLoginUIBrowserTest, OneProcessLimit) {
+  GURL test_url_1 = ui_test_utils::GetTestUrl(
+      base::FilePath(base::FilePath::kCurrentDirectory),
+      base::FilePath(FILE_PATH_LITERAL("title1.html")));
+  GURL test_url_2 = ui_test_utils::GetTestUrl(
+      base::FilePath(base::FilePath::kCurrentDirectory),
+      base::FilePath(FILE_PATH_LITERAL("data:text/html,Hello world!")));
+
+  // Even when the process limit is set to one, the signin process should
+  // still be given its own process and storage partition.
+  content::RenderProcessHost::SetMaxRendererProcessCount(1);
+
+  ContentInfo info1 =
+      NavigateAndGetInfo(browser(), test_url_1, CURRENT_TAB);
+  ContentInfo info2 =
+      NavigateAndGetInfo(browser(), test_url_2, CURRENT_TAB);
+  ContentInfo info3 =
+      NavigateAndGetInfo(browser(),
+                         signin::GetPromoURL( signin::SOURCE_START_PAGE, false),
+                         CURRENT_TAB);
+
+  ASSERT_EQ(info1.pid, info2.pid);
+  ASSERT_NE(info1.pid, info3.pid);
+}
+
+class InlineLoginUISafeIframeBrowserTest : public InProcessBrowserTest {
+ public:
+  FooWebUIProvider& foo_provider() { return foo_provider_; }
+
+ private:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    content::WebUIControllerFactory::UnregisterFactoryForTesting(
+        ChromeWebUIControllerFactory::GetInstance());
+    test_factory_.reset(new TestChromeWebUIControllerFactory);
+    content::WebUIControllerFactory::RegisterFactory(test_factory_.get());
+    test_factory_->AddFactoryOverride(
+        GURL(kFooWebUIURL).host(), &foo_provider_);
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    test_factory_->RemoveFactoryOverride(GURL(kFooWebUIURL).host());
+    content::WebUIControllerFactory::UnregisterFactoryForTesting(
+        test_factory_.get());
+    test_factory_.reset();
+  }
+
+  FooWebUIProvider foo_provider_;
+  scoped_ptr<TestChromeWebUIControllerFactory> test_factory_;
+};
+
+// Make sure that the foo webui handler is working properly and that it gets
+// created when navigated to normally.
+IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest, Basic) {
+  const GURL kUrl(kFooWebUIURL);
+  EXPECT_CALL(foo_provider(), NewWebUI(_, ::testing::Eq(kUrl)))
+      .WillOnce(ReturnNewWebUI());
+  ui_test_utils::NavigateToURL(browser(), GURL(kFooWebUIURL));
+}
+
+// Make sure that the foo webui handler does not get created when we try to
+// load it inside the iframe of the login ui.
+IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest, NoWebUIInIframe) {
+  GURL url = signin::GetPromoURL(signin::SOURCE_START_PAGE, false).
+      Resolve("?source=0&frameUrl=chrome://foo");
+  EXPECT_CALL(foo_provider(), NewWebUI(_, _)).Times(0);
+  ui_test_utils::NavigateToURL(browser(), url);
+}

chrome/chrome_tests.gypi

@@ -1574,6 +1574,7 @@
         'browser/ui/webui/policy_ui_browsertest.cc',
         'browser/ui/webui/print_preview/print_preview_ui_browsertest.cc',
         'browser/ui/webui/signin/user_manager_ui_browsertest.cc',
+        'browser/ui/webui/signin/inline_login_ui_browsertest.cc',
         'browser/ui/webui/sync_internals_browsertest.js',
         'browser/ui/webui/sync_setup_browsertest.js',
         'browser/ui/webui/web_ui_test_handler.cc',

content/browser/webui/url_data_manager_backend.cc

@@ -26,6 +26,7 @@
 #include "content/browser/tcmalloc_internals_request_job.h"
 #include "content/browser/webui/shared_resources_data_source.h"
 #include "content/browser/webui/url_data_source_impl.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host.h"
@@ -161,7 +162,13 @@ class URLRequestChromeJob : public net::URLRequestJob,
 
   // Helper for Start(), to let us start asynchronously.
   // (This pattern is shared by most net::URLRequestJob implementations.)
-  void StartAsync();
+  void StartAsync(bool allowed);
+
+  // Called on the UI thread to check if this request is allowed.
+  static void CheckStoragePartitionMatches(
+      int render_process_id,
+      const GURL& url,
+      const base::WeakPtr<URLRequestChromeJob>& job);
 
   // Do the actual copy from data_ (the data we're serving) into |buf|.
   // Separate from ReadRawData so we can handle async I/O.
@@ -230,12 +237,14 @@ URLRequestChromeJob::~URLRequestChromeJob() {
 }
 
 void URLRequestChromeJob::Start() {
-  // Start reading asynchronously so that all error reporting and data
-  // callbacks happen as they would for network requests.
-  base::MessageLoop::current()->PostTask(
+  int render_process_id, unused;
+  ResourceRequestInfo::GetRenderFrameForRequest(
+      request_, &render_process_id, &unused);
+  BrowserThread::PostTask(
+      BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&URLRequestChromeJob::StartAsync, weak_factory_.GetWeakPtr()));
-
+      base::Bind(&URLRequestChromeJob::CheckStoragePartitionMatches,
+                 render_process_id, request_->url(), AsWeakPtr()));
   TRACE_EVENT_ASYNC_BEGIN1("browser", "DataManager:Request", this, "URL",
       request_->url().possibly_invalid_spec());
 }
@@ -339,11 +348,38 @@ void URLRequestChromeJob::CompleteRead(net::IOBuffer* buf, int buf_size,
   *bytes_read = buf_size;
 }
 
-void URLRequestChromeJob::StartAsync() {
+void URLRequestChromeJob::CheckStoragePartitionMatches(
+    int render_process_id,
+    const GURL& url,
+    const base::WeakPtr<URLRequestChromeJob>& job) {
+  // The embedder could put some webui pages in separate storage partition.
+  // RenderProcessHostImpl::IsSuitableHost would guard against top level pages
+  // being in the same process. We do an extra check to guard against an
+  // exploited renderer pretending to add them as a subframe. We skip this check
+  // for resources.
+  bool allowed = false;
+  if (url.SchemeIs(kChromeUIScheme) && url.host() == kChromeUIResourcesHost) {
+    allowed = true;
+  } else {
+    RenderProcessHost* process = RenderProcessHost::FromID(render_process_id);
+    if (process) {
+      StoragePartition* partition = BrowserContext::GetStoragePartitionForSite(
+          process->GetBrowserContext(), url);
+      allowed = partition == process->GetStoragePartition();
+    }
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&URLRequestChromeJob::StartAsync, job, allowed));
+}
+
+void URLRequestChromeJob::StartAsync(bool allowed) {
   if (!request_)
     return;
 
-  if (!backend_->StartRequest(request_, this)) {
+  if (!allowed || !backend_->StartRequest(request_, this)) {
     NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
                                            net::ERR_INVALID_URL));
   }