Pass signin_scoped_device_id to refresh token request on ChromeOS

Pass signin_scoped_device_id to refresh token request on ChromeOS

During signin, when ChromeOS requests refresh token, get
signin scoped device_id from SigninClient and pass it to GaiaAuthFetcher.

BUG=382968
R=nkostylev@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285422

[pavely, 2014-07-10 22:36:43]

Nikita,

This change is ChromeOS counterpart of
https://codereview.chromium.org/362613002/.
The end goal is to pass some device_id to programmatic_auth and also store that
device_id in sync's data on server so that security center could query device
info on server when showing refresh token entry.

I've enumerated all codepaths how
GaiaAuthFetcher::StartCookieForOAuthTokenExchange
gets called:
http://www.plantuml.com:80/plantuml/png/dPHHQzim4CVVzIcElWp6Ic2FUXW4ipApZPHuV...

This change covers InlineLoginHandlerChromeOS and chromeos::OAuth2LoginManager.
I don't think that refresh tokens requested through
policy::PolicyOAuth2TokenFetcher
need to be annotated since they are essentially one time tokens (not stored on
device).

Let me know if I missed something on ChromeOS side.

Also, could you point me to code that revokes refresh token on ChromeOS when
user
is deleted from login screen?

Thanks, Pavel.

[Nikita (slow), 2014-07-14 09:39:48]

+Pavel for
chrome/browser/ui/webui/chromeos/login/inline_login_handler_chromeos.cc

[dzhioev (left Google), 2014-07-14 13:45:23]

Hi Pavel,
can you please provide a link to design doc?

[Nikita (slow), 2014-07-14 13:52:13]

On 2014/07/10 22:36:43, pavely wrote:
> Nikita,
> 
> This change is ChromeOS counterpart of
> https://codereview.chromium.org/362613002/.
> The end goal is to pass some device_id to programmatic_auth and also store
that
> device_id in sync's data on server so that security center could query device
> info on server when showing refresh token entry.
> 
> I've enumerated all codepaths how
> GaiaAuthFetcher::StartCookieForOAuthTokenExchange
> gets called:
>
http://www.plantuml.com:80/plantuml/png/dPHHQzim4CVVzIcElWp6Ic2FUXW4ipApZPHuV...
> 
> This change covers InlineLoginHandlerChromeOS and
chromeos::OAuth2LoginManager.
> I don't think that refresh tokens requested through
> policy::PolicyOAuth2TokenFetcher
> need to be annotated since they are essentially one time tokens (not stored on
> device).

Sounds good.

> 
> Let me know if I missed something on ChromeOS side.

I don't think that anything else might be broken.

+Xiyuan/Zel since this may affect kiosk apps (not sure).

> Also, could you point me to code that revokes refresh token on ChromeOS when
> user
> is deleted from login screen?

Not really since refresh tokens are insider user cryptohome and when user is
deleted cryptohome is not mounted and we don't have access to those tokens.

[Nikita (slow), 2014-07-14 13:54:23]

On 2014/07/10 22:36:43, pavely wrote:
> Nikita,
> 
> This change is ChromeOS counterpart of
> https://codereview.chromium.org/362613002/.
> The end goal is to pass some device_id to programmatic_auth and also store
that
> device_id in sync's data on server so that security center could query device
> info on server when showing refresh token entry.

What about backwards compatibility for those clients with older Chrome versions?

[xiyuan, 2014-07-14 16:24:05]

Kiosk should be good.

Enterprise kiosk does not have a signed-in profile and exposes its device robot
account via identity api. Consumer kiosk uses inline_login_handler_chromeos.cc
but it's not officially supported and probably does not work at the moment.

[Nikita (slow), 2014-07-14 16:27:26]

On 2014/07/14 16:24:05, xiyuan wrote:
> Enterprise kiosk does not have a signed-in profile and exposes its device
robot
> account via identity api. Consumer kiosk uses inline_login_handler_chromeos.cc
> but it's not officially supported and probably does not work at the moment.

So there're no plans to fix it?

[xiyuan, 2014-07-14 16:41:39]

On 2014/07/14 16:27:26, Nikita Kostylev wrote:
> On 2014/07/14 16:24:05, xiyuan wrote:
> > Enterprise kiosk does not have a signed-in profile and exposes its device
> robot
> > account via identity api. Consumer kiosk uses
inline_login_handler_chromeos.cc
> > but it's not officially supported and probably does not work at the moment.
> 
> So there're no plans to fix it?

The code path is not actively maintained. It was working back in M33/34 time but
I have not checked it after that. The code uses inline_login_handler_chromeos.cc
and works in a way that signs in the user in the app's profile. This is not what
we want and could be a problem when the kiosk device is shared and publicly
accessible. We need to figure out whether there is real use case of this for
consumer mode. Publicly accessible kiosks are usually enterprised managed which
is supported so there might not be real use case for consumer kiosk. If there
were real needs, we need a proper design and decide how it should be
implemented.

[pavely, 2014-07-15 22:06:40]

On 2014/07/14 13:54:23, Nikita Kostylev wrote:
> On 2014/07/10 22:36:43, pavely wrote:
> > Nikita,
> > 
> > This change is ChromeOS counterpart of
> > https://codereview.chromium.org/362613002/.
> > The end goal is to pass some device_id to programmatic_auth and also store
> that
> > device_id in sync's data on server so that security center could query
device
> > info on server when showing refresh token entry.
> 
> What about backwards compatibility for those clients with older Chrome
versions?

If user is on older chrome then refresh token is not going to be annotated with
device_id
and all tokens will be collapsed into one entry on security page.
If user is on new version of chrome but refresh token was issued long ago (this
is true
for most users) then this change is not going to help, refresh tokens will still
be collapsed.
I'm working on solution for this. There is a solution used in Android that
allows to backfill
device_id for existing refresh tokens, I'm evaluating if I can do similar thing
for Chrome.
Let me know in email if you want to learn details.

[pavely, 2014-07-15 22:08:12]

On 2014/07/14 13:45:23, dzhioev wrote:
> Hi Pavel,
> can you please provide a link to design doc?

https://docs.google.com/a/google.com/document/d/1gOCNHPDxnCWejzZMoL2txNQ4XW6i...

[dzhioev (left Google), 2014-07-18 13:31:52]

https://codereview.chromium.org/382983003/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/signin/oauth2_token_fetcher.h (right):

https://codereview.chromium.org/382983003/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/signin/oauth2_token_fetcher.h:43: void
StartExchangeFromAuthCode(const std::string& auth_code);
Don't we need device id for auth code authentication?

[pavely, 2014-07-18 22:30:04]

https://codereview.chromium.org/382983003/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/signin/oauth2_token_fetcher.h (right):

https://codereview.chromium.org/382983003/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/signin/oauth2_token_fetcher.h:43: void
StartExchangeFromAuthCode(const std::string& auth_code);
On 2014/07/18 13:31:52, dzhioev wrote:
> Don't we need device id for auth code authentication?
The way how (I think) exchange with gaia works is:
- Chrome shows user signin form where user enters credentials. This form sends
requests to gaia. Result of this interaction is cookie jar that can be exchanged
for auth_code.
- Cookie jar is exchanged for auth_code (one time short lived grant) by sending
request to programmatic_auth endpoint. This is the place where I can pass device
information (device_id, device_name, etc.)
- auth_code is exchanged for token pair (access_token/refresh_token) using
o/oauth2/token endpoint. I cannot pass device_id there today, api doesn't accept
it. 

StartExchangeFromCookies is 2 step exchange: cookies->auth_code->token pair.
StartEachangeFromAuthCode is shortcut into this flow skipping first exchange,
the one I use to pass device_id. That's the reason I don't pass device_id to
StartExchangeFromAuthCode.

The code that calls StartExchangeFromAuthCode should have obtained this code
from gaia in some way. I guess device_id should be injected into that process
somehow, but I don't know how to do it.

I tested two main scenarios: signing in for the first time and signing in after
token revocation (password change). In both cases code goes through
StartExchangeFromCookies. Do you know how I can trigger code to call
StartExchangeFromAuthCode instead? I'd love to understand it better.

[pavely, 2014-07-23 17:41:15]

Ping.

Pavel, could you review this change?

[dzhioev (left Google), 2014-07-24 12:43:00]

On 2014/07/23 17:41:15, pavely wrote:
> Ping.
> 
> Pavel, could you review this change?

LGTM

[pavely, 2014-07-24 18:01:56]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2014-07-24 18:08:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pavely@chromium.org/382983003/20001

[commit-bot: I haz the power, 2014-07-24 22:13:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-25 00:05:29]

Change committed as 285422