cros: Port SAML support to webview sign-in.

cros: Port SAML support to webview sign-in.

- Extends Channel to PostMessageChannel to switch saml_injected.js to
  use postMessage instead of extension messaging API when used with the
  new Authenticator;
- Port Saml logic in main.js and background.js into saml_handler.js to
  be used with the new Authenticator;
- Hook up confirm/verify password flow etc with new Authenticator;

BUG=461505
Committed: https://crrev.com/f9ddee6a010e752f933ff352ec29d13c5a459f52
Cr-Commit-Position: refs/heads/master@{#321424}

[xiyuan, 2015-03-12 23:45:05]

xiyuan@chromium.org changed reviewers:
+ bartfab@chromium.org, dpolukhin@chromium.org

[xiyuan, 2015-03-12 23:45:05]

Initial pass with manually tested.

I'll check out how to leverage SamlTest.* to cover webview flow as well.

BTW, what about the enrollment? Do we need to change that to webview as well?

[bartfab (slow), 2015-03-13 09:09:48]

On 2015/03/12 23:45:05, xiyuan wrote:
> Initial pass with manually tested.
> 
> I'll check out how to leverage SamlTest.* to cover webview flow as well.
> 
> BTW, what about the enrollment? Do we need to change that to webview as well?

Great to see this. Did you check whether the credentials passing API works as
well by any chance?

[Dmitry Polukhin, 2015-03-13 11:56:20]

LGTM but I'm not familiar with SAML so payed I mostly reviewed change in login
code.

Webview support for enterprise enrollment should fix dzhioev@ next week.

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:479: // TODO(rsorokin):
Investigate whether this breaks SAML.
nit, it seems this todo can be removed now.

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:61: const char
kGaiaSandboxUrlSwitch[] = "gaia-sandbox-url";
We agreed to use accounts.sandbox.google.com temporary to see changes sooner. On
Chrome OS there is no easy way to change flags on login screen. I would keep it
as is until SAML support is ready for dogfood. Alternative solution could be
command hotkey working in canary and dev to change default server.

[xiyuan, 2015-03-13 16:06:43]

On 2015/03/13 09:09:48, bartfab wrote:
> On 2015/03/12 23:45:05, xiyuan wrote:
> > Initial pass with manually tested.
> > 
> > I'll check out how to leverage SamlTest.* to cover webview flow as well.
> > 
> > BTW, what about the enrollment? Do we need to change that to webview as
well?
> 
> Great to see this. Did you check whether the credentials passing API works as
> well by any chance?

I have tried crosdev4.biz accounts and they work fine. The API is called and
credentials are passed as expected.

[dzhioev (left Google), 2015-03-17 16:44:56]

Hello Xiyuan,

> BTW, what about the enrollment? Do we need to change that to webview as well?
Yes, we are changing enterprise enrollment too. I am working on it .
See http://crbug.com/464105

About the CL:

I don't like the way how we deliver injected code to a webview, mainly
because unrelated files gaia_screen_handler.cc and screen_gaia_signin.js
are involved into the process. We are going to replace iframe with webview on
the enrollment screen. If we leave things as is, we will need to write
duplicating code to enable SAML on enrollment screen. I think we need to
isolate injected code loading logic inside SamlHandler.

My proposal:
1. Host JS with injected code file somewhere (for example
http://oobe/webview_saml_injected.js ).
2. Add a boolean parameter for Authenticator (for example 'injectSAML').
3. Pass the parameter's value to SamlHandler when we create an instance in
Authenticator.
4. If 'injecteSAML' is true, SamlHandler loads '/webview_saml_injected.js' using
XMLHttpRequest and then
   injects content into the webview.

Another option is to simply save 'webview_saml_injected.js' content as a static
constant 
string inside SamlHandler.

What do you think?

[dzhioev (left Google), 2015-03-17 16:45:27]

I meant "chrome://oobe" of course.

[Dmitry Polukhin, 2015-03-17 17:51:43]

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:61: const char
kGaiaSandboxUrlSwitch[] = "gaia-sandbox-url";
On 2015/03/13 11:56:20, Dmitry Polukhin wrote:
> We agreed to use http://accounts.sandbox.google.com temporary to see changes
sooner. On
> Chrome OS there is no easy way to change flags on login screen. I would keep
it
> as is until SAML support is ready for dogfood. Alternative solution could be
> command hotkey working in canary and dev to change default server.

As discussed it is not a problem. We will switch to prod.

[xiyuan, 2015-03-17 18:14:34]

On 2015/03/17 16:44:56, dzhioev wrote:
> Hello Xiyuan,
> 
> > BTW, what about the enrollment? Do we need to change that to webview as
well?
> Yes, we are changing enterprise enrollment too. I am working on it .
> See http://crbug.com/464105
> 
> About the CL:
> 
> I don't like the way how we deliver injected code to a webview, mainly
> because unrelated files gaia_screen_handler.cc and screen_gaia_signin.js
> are involved into the process. We are going to replace iframe with webview on
> the enrollment screen. If we leave things as is, we will need to write
> duplicating code to enable SAML on enrollment screen. I think we need to
> isolate injected code loading logic inside SamlHandler.
> 
> My proposal:
> 1. Host JS with injected code file somewhere (for example
> http://oobe/webview_saml_injected.js ).
> 2. Add a boolean parameter for Authenticator (for example 'injectSAML').
> 3. Pass the parameter's value to SamlHandler when we create an instance in
> Authenticator.
> 4. If 'injecteSAML' is true, SamlHandler loads '/webview_saml_injected.js'
using
> XMLHttpRequest and then
>    injects content into the webview.
> 
> Another option is to simply save 'webview_saml_injected.js' content as a
static
> constant 
> string inside SamlHandler.
> 
> What do you think?

he major difficulty with the proposal is the injection timing. We need to make
sure injected script runs before the guest page script. Otherwise, SAML API
might break. The current executeScript is already fragile because "run_at:
document_start" is not really supported with executeScript. There are chances
(almost 100% in tests) that guest page calls API initialize before injected
APIFowarder runs. I am currently fighting the situation in
SamlTest.CredentialPassingAPI.

Using an XHR would make the situation worse because it is an extra step and we
need more time to have things ready. We definitely need to cache
webview_saml_injected.js. But even then, we might not have it ready when we need
to inject it, especially in test where we visit SAML page fast. Passing it via
loadTimeData is better in this regard.

Currently the |injectedJS| is a singleton instance. We only need to set it once
per webui. I could move the loadTimeData related code into OobeUI and make
SamlHandler reference it directly. That should make enrollment screen code use
Authenticator directly without knowning all the details. How does that sound?

[dzhioev_at_google, 2015-03-17 21:24:45]

On 2015/03/17 18:14:34, xiyuan wrote:
> On 2015/03/17 16:44:56, dzhioev wrote:
> > Hello Xiyuan,
> > 
> > > BTW, what about the enrollment? Do we need to change that to webview as
> well?
> > Yes, we are changing enterprise enrollment too. I am working on it .
> > See http://crbug.com/464105
> > 
> > About the CL:
> > 
> > I don't like the way how we deliver injected code to a webview, mainly
> > because unrelated files gaia_screen_handler.cc and screen_gaia_signin.js
> > are involved into the process. We are going to replace iframe with webview
on
> > the enrollment screen. If we leave things as is, we will need to write
> > duplicating code to enable SAML on enrollment screen. I think we need to
> > isolate injected code loading logic inside SamlHandler.
> > 
> > My proposal:
> > 1. Host JS with injected code file somewhere (for example
> > http://oobe/webview_saml_injected.js ).
> > 2. Add a boolean parameter for Authenticator (for example 'injectSAML').
> > 3. Pass the parameter's value to SamlHandler when we create an instance in
> > Authenticator.
> > 4. If 'injecteSAML' is true, SamlHandler loads '/webview_saml_injected.js'
> using
> > XMLHttpRequest and then
> >    injects content into the webview.
> > 
> > Another option is to simply save 'webview_saml_injected.js' content as a
> static
> > constant 
> > string inside SamlHandler.
> > 
> > What do you think?
> 
> he major difficulty with the proposal is the injection timing. We need to make
> sure injected script runs before the guest page script. Otherwise, SAML API
> might break. The current executeScript is already fragile because "run_at:
> document_start" is not really supported with executeScript. There are chances
> (almost 100% in tests) that guest page calls API initialize before injected
> APIFowarder runs. I am currently fighting the situation in
> SamlTest.CredentialPassingAPI.
> 
> Using an XHR would make the situation worse because it is an extra step and we
> need more time to have things ready. We definitely need to cache
> webview_saml_injected.js. But even then, we might not have it ready when we
need
> to inject it, especially in test where we visit SAML page fast. Passing it via
> loadTimeData is better in this regard.
> 
> Currently the |injectedJS| is a singleton instance. We only need to set it
once
> per webui. I could move the loadTimeData related code into OobeUI and make
> SamlHandler reference it directly. That should make enrollment screen code use
> Authenticator directly without knowning all the details. How does that sound?

I have a better idea. With a help of new "raw string" feature [*] and GRIT we
can
define |injectedJS| in compile time. Something like:

var injectedJS = `
<include src="webview_saml_injected.js">
`;

[*]
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

[xiyuan, 2015-03-17 21:38:45]

On 2015/03/17 21:24:45, dzhioev_at_google wrote:
> On 2015/03/17 18:14:34, xiyuan wrote:
> > On 2015/03/17 16:44:56, dzhioev wrote:
> > > Hello Xiyuan,
> > > 
> > > > BTW, what about the enrollment? Do we need to change that to webview as
> > well?
> > > Yes, we are changing enterprise enrollment too. I am working on it .
> > > See http://crbug.com/464105
> > > 
> > > About the CL:
> > > 
> > > I don't like the way how we deliver injected code to a webview, mainly
> > > because unrelated files gaia_screen_handler.cc and screen_gaia_signin.js
> > > are involved into the process. We are going to replace iframe with webview
> on
> > > the enrollment screen. If we leave things as is, we will need to write
> > > duplicating code to enable SAML on enrollment screen. I think we need to
> > > isolate injected code loading logic inside SamlHandler.
> > > 
> > > My proposal:
> > > 1. Host JS with injected code file somewhere (for example
> > > http://oobe/webview_saml_injected.js ).
> > > 2. Add a boolean parameter for Authenticator (for example 'injectSAML').
> > > 3. Pass the parameter's value to SamlHandler when we create an instance in
> > > Authenticator.
> > > 4. If 'injecteSAML' is true, SamlHandler loads '/webview_saml_injected.js'
> > using
> > > XMLHttpRequest and then
> > >    injects content into the webview.
> > > 
> > > Another option is to simply save 'webview_saml_injected.js' content as a
> > static
> > > constant 
> > > string inside SamlHandler.
> > > 
> > > What do you think?
> > 
> > he major difficulty with the proposal is the injection timing. We need to
make
> > sure injected script runs before the guest page script. Otherwise, SAML API
> > might break. The current executeScript is already fragile because "run_at:
> > document_start" is not really supported with executeScript. There are
chances
> > (almost 100% in tests) that guest page calls API initialize before injected
> > APIFowarder runs. I am currently fighting the situation in
> > SamlTest.CredentialPassingAPI.
> > 
> > Using an XHR would make the situation worse because it is an extra step and
we
> > need more time to have things ready. We definitely need to cache
> > webview_saml_injected.js. But even then, we might not have it ready when we
> need
> > to inject it, especially in test where we visit SAML page fast. Passing it
via
> > loadTimeData is better in this regard.
> > 
> > Currently the |injectedJS| is a singleton instance. We only need to set it
> once
> > per webui. I could move the loadTimeData related code into OobeUI and make
> > SamlHandler reference it directly. That should make enrollment screen code
use
> > Authenticator directly without knowning all the details. How does that
sound?
> 
> I have a better idea. With a help of new "raw string" feature [*] and GRIT we
> can
> define |injectedJS| in compile time. Something like:
> 
> var injectedJS = `
> <include src="webview_saml_injected.js">
> `;
> 
> [*]
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

Awesome. Works like a charm. Uploaded PS4 with the trick.

Will rebase and request another round of review.

[xiyuan, 2015-03-17 22:00:15]

Enabled most of SamlTest.* to cover webview based SAML.

I had to disable CredentialPassingAPI for webview because webview.executeScript
does not have full control of injection timing and I could not get it to work
reliably. The test fails about 20% time in my limited testing. This probably had
to wait for webview.addContentScript
(https://codereview.chromium.org/959413003/). :(

PTAL. Thanks.

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1004753004/diff/20001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:479: // TODO(rsorokin):
Investigate whether this breaks SAML.
On 2015/03/13 11:56:19, Dmitry Polukhin wrote:
> nit, it seems this todo can be removed now.

Done.

[xiyuan, 2015-03-18 22:52:16]

xiyuan@chromium.org changed reviewers:
+ rogerta@chromium.org

[xiyuan, 2015-03-18 22:52:17]

+rogerta for fake_gaia.h/cc

[Roger Tawa OOO till Jul 10th, 2015-03-19 20:29:40]

lgtm

[xiyuan, 2015-03-19 20:31:07]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2015-03-19 20:31:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpolukhin@chromium.org
Link to the patchset: https://codereview.chromium.org/1004753004/#ps100001
(title: "handle loadabort to fix test")

[commit-bot: I haz the power, 2015-03-19 20:31:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1004753004/100001

[commit-bot: I haz the power, 2015-03-19 20:36:11]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-19 20:37:06]

Patchset 6 (id:??) landed as
https://crrev.com/f9ddee6a010e752f933ff352ec29d13c5a459f52
Cr-Commit-Position: refs/heads/master@{#321424}