cros: Transfer auth cookies for SAML webview sign-in.

cros: Transfer auth cookies for SAML webview sign-in.

- Update ProfileAuthData to transfer cookie from sign-in webview;
- Passing back usingSAML flag for auth code authentication and use
  it to set has_auth_cookie flag when SAML is used;
- Update UserCloudPolicyManagerChromeOS to fetch initial policy from
  correct auth request context;
- Enable SAMLPolicyTest for WebView;

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

[xiyuan, 2015-03-20 20:05:15]

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

[xiyuan, 2015-03-20 20:05:15]

dzhioev@, do you mind helping to review this? Thanks.

[xiyuan, 2015-03-20 20:06:04]

cc bartfab@ fyi

[dzhioev (left Google), 2015-03-23 16:03:10]

Hi, I'll review this CL later today.

[dzhioev_at_google, 2015-03-23 23:01:17]

dzhioev@google.com changed reviewers:
+ dzhioev@google.com

[dzhioev_at_google, 2015-03-23 23:01:17]

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/helper.cc (right):

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/helper.cc:145: return
GetPartition(GetLoginWebContents(), std::string());
I'm adding another <webview> to OOBE page now, used for enterprise enrolment.
Will it brake things? Should I use non-empty partition id for my <webview>?

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
chrome/browser/resources/gaia_auth_host/authenticator.js:256:
history.replaceState({url: url}, '');
Offtop: Do you know what all these history manipulations are needed for?

[xiyuan, 2015-03-23 23:10:48]

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/helper.cc (right):

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/helper.cc:145: return
GetPartition(GetLoginWebContents(), std::string());
On 2015/03/23 23:01:17, dzhioev_at_google wrote:
> I'm adding another <webview> to OOBE page now, used for enterprise enrolment.
> Will it brake things? Should I use non-empty partition id for my <webview>?

I had a long discussion with Ivan in his
https://codereview.chromium.org/1012083002/.

The short version of my take from the discussion is that we should use the
default unnamed shared in-memory partition for all webviews in login screen.
This is because we were using a shared sign-in profile and with proper code
change, we should be able to work with shared storage partition as well.

The shared partition should work because Ivan has code to clear up the partition
now as part of CleanSignInProfile call now. So even if we have cookies after
enrollment, those cookies will be cleared before we bringing up Gaia.

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
chrome/browser/resources/gaia_auth_host/authenticator.js:256:
history.replaceState({url: url}, '');
On 2015/03/23 23:01:17, dzhioev_at_google wrote:
> Offtop: Do you know what all these history manipulations are needed for?

It is for desktop chrome where the sign-in UI could be opened in a tab and the
history needs to work properly.

This is fixed in another CL (https://codereview.chromium.org/1031683002/).

[dzhioev_at_google, 2015-03-23 23:54:23]

Thanks for clarification.
LGTM

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
chrome/browser/resources/gaia_auth_host/authenticator.js:256:
history.replaceState({url: url}, '');
On 2015/03/23 23:10:48, xiyuan wrote:
> On 2015/03/23 23:01:17, dzhioev_at_google wrote:
> > Offtop: Do you know what all these history manipulations are needed for?
> 
> It is for desktop chrome where the sign-in UI could be opened in a tab and the
> history needs to work properly.
> 
> This is fixed in another CL (https://codereview.chromium.org/1031683002/).
> 
I afraid this can break OOBE/Login somehow. Probably this should be implemented
as an another Authenticator's event, with an event listener on desktop page's
side. I'll take care of that.

[dzhioev (left Google), 2015-03-24 13:16:51]

On 2015/03/23 23:54:23, dzhioev_at_google wrote:
> Thanks for clarification.
> LGTM
> 
>
https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
> File chrome/browser/resources/gaia_auth_host/authenticator.js (right):
> 
>
https://codereview.chromium.org/1021383002/diff/1/chrome/browser/resources/ga...
> chrome/browser/resources/gaia_auth_host/authenticator.js:256:
> history.replaceState({url: url}, '');
> On 2015/03/23 23:10:48, xiyuan wrote:
> > On 2015/03/23 23:01:17, dzhioev_at_google wrote:
> > > Offtop: Do you know what all these history manipulations are needed for?
> > 
> > It is for desktop chrome where the sign-in UI could be opened in a tab and
the
> > history needs to work properly.
> > 
> > This is fixed in another CL (https://codereview.chromium.org/1031683002/).
> > 
> I afraid this can break OOBE/Login somehow. Probably this should be
implemented
> as an another Authenticator's event, with an event listener on desktop page's
> side. I'll take care of that.

Wrong account. LGTM

[xiyuan, 2015-03-24 19:20:45]

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

[xiyuan, 2015-03-24 19:20:46]

mnissler@, could you help to review policy/ser_cloud_policy_manager_chromeos*?
The CL changes to use sign-in context from the webview's storage partition when
webview based sign-in is used. Thanks.

[Mattias Nissler (ping if slow), 2015-03-25 08:28:18]

policy bits LGTM

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.cc:596: //  Bootstrap
exp.             F            F
nit: It's totally unclear to me what "Bootstrap exp." means.

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/saml/saml_browsertest.cc (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/saml/saml_browsertest.cc:306: // Otherwise, we
break PRE tests that leave state in it.
Shouldn't we fix these tests instead? Or are you adding them?

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/base_screen_handler.h (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/base_screen_handler.h:180: typename A5>
I think this should be using variadic template arguments. FWIW, base/callback.h
has been ported a while back.

[xiyuan, 2015-03-25 19:26:19]

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.cc:596: //  Bootstrap
exp.             F            F
On 2015/03/25 08:28:18, Mattias Nissler wrote:
> nit: It's totally unclear to me what "Bootstrap exp." means.

Update to "Bootstrap experiment". It is an internal endpoint to experiment
boostrapping. :)

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/saml/saml_browsertest.cc (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/saml/saml_browsertest.cc:306: // Otherwise, we
break PRE tests that leave state in it.
On 2015/03/25 08:28:18, Mattias Nissler wrote:
> Shouldn't we fix these tests instead? Or are you adding them?

The PRE tests are valid. They are added to setup states that could not be easily
mocked in tests. The PRE test here is to add some user and test the user
behavior in the next browser run. So they are valid.

The code here is to set the webview enabled flag in local state. We only need to
set it once and it should be persisted. I am just too lazy to change the logic
to read-modify-write.

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/base_screen_handler.h (right):

https://codereview.chromium.org/1021383002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/base_screen_handler.h:180: typename A5>
On 2015/03/25 08:28:18, Mattias Nissler wrote:
> I think this should be using variadic template arguments. FWIW,
base/callback.h
> has been ported a while back.

Good point. However, it is not trivial to change (espeically dealing with
login::CallbackWrapper to convert args from a ListValue). I would prefer to do
it in a separate CL.

[xiyuan, 2015-03-25 19:27:24]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2015-03-25 19:27:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
dzhioev@chromium.org, dzhioev@google.com, mnissler@chromium.org
Link to the patchset: https://codereview.chromium.org/1021383002/#ps40001
(title: "update comment")

[commit-bot: I haz the power, 2015-03-25 19:28:01]

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

[commit-bot: I haz the power, 2015-03-25 20:01:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-25 20:02:27]

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