[Chromoting] Add new Chromoting enterprise policies

[Chromoting] Add new policies for Chromoting

This adds definitions in Chrome for the new policies required by Chromoting: HostDomain, HostRequireTwoFactor and HostTalkGadgetPrefix. These policies are read and enforced by the Chromoting host code.

BUG=132345
TEST=None

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

[simonmorris, 2012-08-02 23:49:38]

remoting/host LGTM with optional suggestions.

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h
File remoting/host/constants.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h#...
remoting/host/constants.h:12: // This is the default prefix that is pre-pended
to "talkgadget.google.com"
"pre-pended" -> "prepended".
"talkgadget" -> ".talkgadget".

http://codereview.chromium.org/10825149/diff/10001/remoting/host/policy_hack/...
File remoting/host/policy_hack/policy_watcher.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/policy_hack/...
remoting/host/policy_hack/policy_watcher.h:53: static const char
kHostRequireTwoFactorPolicyName[];
This file is in remoting/host, so is it helpful to have "Host" in the variable
name? Maybe it should come out of "kHostDomainPolicyName" as well, for
consistency with "kNatPolicyName"?

[Nirnimesh, 2012-08-02 23:58:37]

http://codereview.chromium.org/10825149/diff/10001/chrome/test/functional/pol...
File chrome/test/functional/policy_test_cases.py (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/test/functional/pol...
chrome/test/functional/policy_test_cases.py:69:
'RemoteAccessHostRequireTwoFactor':
Do these apply to chromeos too?

[garykac, 2012-08-03 01:35:07]

http://codereview.chromium.org/10825149/diff/10001/chrome/test/functional/pol...
File chrome/test/functional/policy_test_cases.py (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/test/functional/pol...
chrome/test/functional/policy_test_cases.py:69:
'RemoteAccessHostRequireTwoFactor':
On 2012/08/02 23:58:38, Nirnimesh wrote:
> Do these apply to chromeos too?

We currently don't support hosts on ChromeOS, but if/when we do the should apply
to ChromeOS as well.

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h
File remoting/host/constants.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h#...
remoting/host/constants.h:12: // This is the default prefix that is pre-pended
to "talkgadget.google.com"
On 2012/08/02 23:49:39, simonmorris wrote:
> "pre-pended" -> "prepended".
> "talkgadget" -> ".talkgadget".

Done.

http://codereview.chromium.org/10825149/diff/10001/remoting/host/policy_hack/...
File remoting/host/policy_hack/policy_watcher.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/policy_hack/...
remoting/host/policy_hack/policy_watcher.h:53: static const char
kHostRequireTwoFactorPolicyName[];
On 2012/08/02 23:49:39, simonmorris wrote:
> This file is in remoting/host, so is it helpful to have "Host" in the variable
> name? Maybe it should come out of "kHostDomainPolicyName" as well, for
> consistency with "kNatPolicyName"?

The "Host" is in the name because I want the policy name to emphasize that it's
only for the Host. Our policy names have followed the convention of:
* Start with "Client" - applies to client only
* Start with "Host" - applies to host only
* Otherwise, applies to both client/host

I think kNatPolicyName is inconsistent with the actual policy name and I'll
probably fix that in a separate cl.

[Mattias Nissler (ping if slow), 2012-08-03 09:05:52]

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:465: 'caption': '''Configure the
required host domain name''',
Can you add "for Chromoting" or something alike so admins get a hint on what
this applies to when they go through the list of policies?

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:468: If this setting is enabled, then
hosts can be shared only using accounts registered on the specified domain name.
Same here. If you don't have the Chromoting context, it's very hard to
understand what "host sharing" actually refers to.

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:480: 'caption': '''Enable two-factor
authentication on host''',
ditto

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:496: 'desc': '''Configures the host
TalkGadget prefix and prevents users from changing it.
ditto

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h
File remoting/host/constants.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h#...
remoting/host/constants.h:15: #define DEFAULT_TALKGADGET_PREFIX
"chromoting-host"
other code usually prefers const char[] over #defines.

[garykac, 2012-08-03 17:28:24]

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:465: 'caption': '''Configure the
required host domain name''',
On 2012/08/03 09:05:52, Mattias Nissler wrote:
> Can you add "for Chromoting" or something alike so admins get a hint on what
> this applies to when they go through the list of policies?

I don't think this is necessary because all of these policies are contained
within the RemoteAccess group (which starts on line 415).

I haven't used the admin UI to set policy, but I believe that admins won't see
these options unless they open up the RemoteAccess group, which should provide
the required context.

Does that seem sufficient to you?

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:468: If this setting is enabled, then
hosts can be shared only using accounts registered on the specified domain name.
On 2012/08/03 09:05:52, Mattias Nissler wrote:
> Same here. If you don't have the Chromoting context, it's very hard to
> understand what "host sharing" actually refers to.

same

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:480: 'caption': '''Enable two-factor
authentication on host''',
On 2012/08/03 09:05:52, Mattias Nissler wrote:
> ditto

same

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:496: 'desc': '''Configures the host
TalkGadget prefix and prevents users from changing it.
On 2012/08/03 09:05:52, Mattias Nissler wrote:
> ditto

same

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h
File remoting/host/constants.h (right):

http://codereview.chromium.org/10825149/diff/10001/remoting/host/constants.h#...
remoting/host/constants.h:15: #define DEFAULT_TALKGADGET_PREFIX
"chromoting-host"
On 2012/08/03 09:05:52, Mattias Nissler wrote:
> other code usually prefers const char[] over #defines.

Done.

[Nirnimesh, 2012-08-03 21:49:51]

http://codereview.chromium.org/10825149/diff/5002/chrome/test/functional/poli...
File chrome/test/functional/policy_test_cases.py (right):

http://codereview.chromium.org/10825149/diff/5002/chrome/test/functional/poli...
chrome/test/functional/policy_test_cases.py:70:
('kRemoteAccessHostRequireTwoFactor', False, [], OS_ALL),
Since it's not meant for chromeos, shouldn't you have ['win', 'mac', 'linux']
instead of OS_ALL?

[garykac, 2012-08-03 21:59:49]

http://codereview.chromium.org/10825149/diff/5002/chrome/test/functional/poli...
File chrome/test/functional/policy_test_cases.py (right):

http://codereview.chromium.org/10825149/diff/5002/chrome/test/functional/poli...
chrome/test/functional/policy_test_cases.py:70:
('kRemoteAccessHostRequireTwoFactor', False, [], OS_ALL),
On 2012/08/03 21:49:52, Nirnimesh wrote:
> Since it's not meant for chromeos, shouldn't you have ['win', 'mac', 'linux']
> instead of OS_ALL?

Done.

[Mattias Nissler (ping if slow), 2012-08-06 08:49:45]

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:465: 'caption': '''Configure the
required host domain name''',
On 2012/08/03 17:28:24, garykac wrote:
> On 2012/08/03 09:05:52, Mattias Nissler wrote:
> > Can you add "for Chromoting" or something alike so admins get a hint on what
> > this applies to when they go through the list of policies?
> 
> I don't think this is necessary because all of these policies are contained
> within the RemoteAccess group (which starts on line 415).
> 
> I haven't used the admin UI to set policy, but I believe that admins won't see
> these options unless they open up the RemoteAccess group, which should provide
> the required context.
> 
> Does that seem sufficient to you?

No. The grouping is nice, but unfortunately only shows up in documentation and
group policy templates. For example, application manifest on the Mac doesn't
have grouping. Thus, the policy descriptions need to make sense even with out
the group description context.

[garykac, 2012-08-06 14:36:32]

ptal

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10825149/diff/10001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:465: 'caption': '''Configure the
required host domain name''',
On 2012/08/06 08:49:45, Mattias Nissler wrote:
> No. The grouping is nice, but unfortunately only shows up in documentation and
> group policy templates. For example, application manifest on the Mac doesn't
> have grouping. Thus, the policy descriptions need to make sense even with out
> the group description context.

Ah. That's unfortunate.

I've updated the captions and descriptions to include "remote access".

[Mattias Nissler (ping if slow), 2012-08-06 15:04:19]

LGTM for chrome/browser/policy

[garykac, 2012-08-06 16:18:52]

On 2012/08/06 15:04:19, Mattias Nissler wrote:
> LGTM for chrome/browser/policy

for chrome/app/policy as well?

jam: can you check chrome/common?
nirnimesh: anything else with policy_test_cases?

[Mattias Nissler (ping if slow), 2012-08-06 16:43:04]

On 2012/08/06 16:18:52, garykac wrote:
> On 2012/08/06 15:04:19, Mattias Nissler wrote:
> > LGTM for chrome/browser/policy
> 
> for chrome/app/policy as well?

LGTM for that as well.

> 
> jam: can you check chrome/common?
> nirnimesh: anything else with policy_test_cases?

[Nirnimesh, 2012-08-06 17:47:36]

chrome/test/functional LGTM

[garykac, 2012-08-06 17:50:28]

+thakis
-jam (since he's currently slammed with content reviews)

[Nico, 2012-08-06 20:01:21]

chrome/common lgtm