Verify the host-supplied URL matches the domain's allowed URL patterns

Verify the host-supplied URL matches the domain's allowed URL patterns.

1. Modify HostInfo.java to parse the tokenPatternUrl from the host list.
2. Modify ThirdPartyTokenFetcher to check against the domain's allowed URL before directing the user to the login page requested by the host.

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

[rmsousa, 2014-06-18 21:23:57]

lgtm

[Lambros, 2014-06-18 23:29:06]

lgtm with comments (please run findbugs before landing)

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
File remoting/android/java/src/org/chromium/chromoting/HostInfo.java (right):

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/HostInfo.java:19: public final
ArrayList<String> tokenUrlPatterns;
This might trigger a findbugs warning. You might need to somehow make this
immutable.

Perhaps make this private, and provide a public getter method that returns a
copy of this list. Otherwise, you're allowing the caller to modify the internals
of this object, and findbugs won't like that.

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/HostInfo.java:40: //
TODO(kelvinp): remove the line below before shipping.
Please remove these lines before landing.

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
File
remoting/android/java/src/org/chromium/chromoting/ThirdPartyTokenFetcher.java
(right):

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/ThirdPartyTokenFetcher.java:114:
/** Verify the host-supplied URL matches the domain's allowed URL patterns. */
nit: s/Verify/Verifies. Function comments are descriptive, not imperative. They
describe what the function does. They don't tell the function what to do :)

This is from the C++ style guide:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[kelvinp, 2014-06-19 01:03:50]

FYI. New pack uploaded.  I ran findbugs and made sure that there are no new
warnings.

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
File remoting/android/java/src/org/chromium/chromoting/HostInfo.java (right):

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/HostInfo.java:19: public final
ArrayList<String> tokenUrlPatterns;
On 2014/06/18 23:29:06, Lambros wrote:
> This might trigger a findbugs warning. You might need to somehow make this
> immutable.
> 
> Perhaps make this private, and provide a public getter method that returns a
> copy of this list. Otherwise, you're allowing the caller to modify the
internals
> of this object, and findbugs won't like that.

Done.

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/HostInfo.java:40: //
TODO(kelvinp): remove the line below before shipping.
On 2014/06/18 23:29:06, Lambros wrote:
> Please remove these lines before landing.

Done.

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
File
remoting/android/java/src/org/chromium/chromoting/ThirdPartyTokenFetcher.java
(right):

https://codereview.chromium.org/348433002/diff/40001/remoting/android/java/sr...
remoting/android/java/src/org/chromium/chromoting/ThirdPartyTokenFetcher.java:114:
/** Verify the host-supplied URL matches the domain's allowed URL patterns. */
On 2014/06/18 23:29:06, Lambros wrote:
> nit: s/Verify/Verifies. Function comments are descriptive, not imperative.
They
> describe what the function does. They don't tell the function what to do :)
> 
> This is from the C++ style guide:
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

Done.

[kelvinp, 2014-06-19 01:03:59]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2014-06-19 01:06:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kelvinp@chromium.org/348433002/60001

[commit-bot: I haz the power, 2014-06-19 11:58:36]

Change committed as 278337