[Chromoting] Let the Linux start-host app take a redirect-URL parameter.

[Chromoting] Let the Linux start-host app take a redirect-URL parameter.

This lets us change the flow that produces an OAuth authorization code.

BUG=155431
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162752

[simonmorris, 2012-10-17 20:17:37]

PTAL

I'll need to rebase this CL against http://codereview.chromium.org/11090063/,
when it lands, because this CL changes the HostStarter constructor.

[Jamie, 2012-10-17 20:52:07]

Is the intention here that users would need to copy two separate things from the
auth web-site? Could we streamline that so that both things are part of a single
string?

Also, what is the redirect URL used for? Is it contacted with the auth code at
any point? If so, then we should take care that we're not leaking the code to an
untrusted site in case of copy/paste errors.

[simonmorris, 2012-10-17 21:01:19]

On 2012/10/17 20:52:07, Jamie wrote:
> Is the intention here that users would need to copy two separate things from
the
> auth web-site? Could we streamline that so that both things are part of a
single
> string?
> 

The auth AppsScript shows the whole command-line, up to the point where the user
should append the name of the host.

> Also, what is the redirect URL used for? Is it contacted with the auth code at
> any point? If so, then we should take care that we're not leaking the code to
an
> untrusted site in case of copy/paste errors.

The redirect URL is used only as a string/token - it's not used as the target of
an http request.

To map an authorization code to a refresh token, we need to specify the redirect
URL that received the authorization code.

[Sergey Ulanov, 2012-10-17 23:41:36]

lgtm.

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/host_start...
File remoting/host/setup/host_starter.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/host_start...
remoting/host/setup/host_starter.cc:107:
google_apis::GetOAuth2ClientID(google_apis::CLIENT_REMOTING);
nit: 4-space indent here and below.

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
File remoting/host/setup/oauth_helper.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:35:
"https://chromoting-oauth.talkgadget.google.com/talkgadget/oauth/"
nit: indent 4 spaces.

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:59: ParseStandardURL(redirect_url.c_str(),
redirect_url.length(),
nit: indentation.

[simonmorris, 2012-10-18 00:10:08]

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/host_start...
File remoting/host/setup/host_starter.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/host_start...
remoting/host/setup/host_starter.cc:107:
google_apis::GetOAuth2ClientID(google_apis::CLIENT_REMOTING);
On 2012/10/17 23:41:36, sergeyu wrote:
> nit: 4-space indent here and below.

Done.

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
File remoting/host/setup/oauth_helper.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:35:
"https://chromoting-oauth.talkgadget.google.com/talkgadget/oauth/"
On 2012/10/17 23:41:36, sergeyu wrote:
> nit: indent 4 spaces.

Done.

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:59: ParseStandardURL(redirect_url.c_str(),
redirect_url.length(),
On 2012/10/17 23:41:36, sergeyu wrote:
> nit: indentation.

I don't see what's wrong here.

[commit-bot: I haz the power, 2012-10-18 16:09:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonmorris@chromium.org/11185037/4002

[commit-bot: I haz the power, 2012-10-18 17:05:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonmorris@chromium.org/11185037/10010

[Sergey Ulanov, 2012-10-18 18:46:51]

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
File remoting/host/setup/oauth_helper.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:59: ParseStandardURL(redirect_url.c_str(),
redirect_url.length(),
On 2012/10/18 00:10:09, simonmorris wrote:
> On 2012/10/17 23:41:36, sergeyu wrote:
> > nit: indentation.
> 
> I don't see what's wrong here.

This line is indented 2 spaces relative to the previous line, but it shouldn't
be.

[simonmorris, 2012-10-18 18:50:42]

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
File remoting/host/setup/oauth_helper.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:59: ParseStandardURL(redirect_url.c_str(),
redirect_url.length(),
On 2012/10/18 18:46:51, sergeyu wrote:
> On 2012/10/18 00:10:09, simonmorris wrote:
> > On 2012/10/17 23:41:36, sergeyu wrote:
> > > nit: indentation.
> > 
> > I don't see what's wrong here.
> 
> This line is indented 2 spaces relative to the previous line, but it shouldn't
> be.

Thanks - I didn't see that when I was editing the file. I'll check my editor,
and fix it in a follow-up CL.

[commit-bot: I haz the power, 2012-10-18 19:00:08]

Change committed as 162752

[simonmorris, 2012-10-18 20:18:15]

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
File remoting/host/setup/oauth_helper.cc (right):

http://codereview.chromium.org/11185037/diff/1/remoting/host/setup/oauth_help...
remoting/host/setup/oauth_helper.cc:59: ParseStandardURL(redirect_url.c_str(),
redirect_url.length(),
On 2012/10/18 18:50:42, simonmorris wrote:
> On 2012/10/18 18:46:51, sergeyu wrote:
> > On 2012/10/18 00:10:09, simonmorris wrote:
> > > On 2012/10/17 23:41:36, sergeyu wrote:
> > > > nit: indentation.
> > > 
> > > I don't see what's wrong here.
> > 
> > This line is indented 2 spaces relative to the previous line, but it
shouldn't
> > be.
> 
> Thanks - I didn't see that when I was editing the file. I'll check my editor,
> and fix it in a follow-up CL.

This looks like a glitch in the side-by-side diffs view. The other views
correctly show the code without the extra indentation.