Wire the device requisition parameter for enterprise enrollment.

Wire the device requisition parameter for enterprise enrollment.

Add a new parameter for device management registration requests that
indicates to the server an intended purpose / mode for the device. Wire
the parameter for device-enrollment to pass the requisition parameter
from the Kiosk manifest.

BUG=chromium:238655
TEST=Unit tests, enroll a device with kiosk manifest present and observer requisition parameter being passed to the server.

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

[Mattias Nissler (ping if slow), 2013-05-10 14:20:20]

Please review.

[bartfab (slow), 2013-05-10 15:25:50]

lgtm

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc:53: //
Fetches a machine statics from StatisticsProvider, returns an empty string on
Nit: s/statistics/statistic/

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc:60:
LOG(WARNING) << "Failed to get machine statistic " << key;
Nit: #include "base/logging.h"

[Mattias Nissler (ping if slow), 2013-05-13 10:52:35]

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
File chrome/browser/policy/proto/cloud/device_management_backend.proto (right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
string requisition = 6;
Bin, Raymond: I'd like to get your thoughts on this (or barring any thoughts, a
thumbs up).

[Bin, 2013-05-14 06:36:40]

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
File chrome/browser/policy/proto/cloud/device_management_backend.proto (right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
string requisition = 6;
On 2013/05/13 10:52:35, Mattias Nissler wrote:
> Bin, Raymond: I'd like to get your thoughts on this (or barring any thoughts,
a
> thumbs up).

Should we define some enum or we have to agree on some string values?

[Mattias Nissler (ping if slow), 2013-05-14 06:49:51]

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
File chrome/browser/policy/proto/cloud/device_management_backend.proto (right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
string requisition = 6;
On 2013/05/14 06:36:41, Bin wrote:
> On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > Bin, Raymond: I'd like to get your thoughts on this (or barring any
thoughts,
> a
> > thumbs up).
> 
> Should we define some enum or we have to agree on some string values?

I thought about this as well, but then decided to go with the string for
flexibility: If we have some new device type at some point, we can just let the
user enter a different requisition string and/or put new requisitions in the OEM
partition manifest.

[Bin, 2013-05-14 06:56:55]

On 2013/05/14 06:49:51, Mattias Nissler wrote:
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> File chrome/browser/policy/proto/cloud/device_management_backend.proto
(right):
> 
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
> string requisition = 6;
> On 2013/05/14 06:36:41, Bin wrote:
> > On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > > Bin, Raymond: I'd like to get your thoughts on this (or barring any
> thoughts,
> > a
> > > thumbs up).
> > 
> > Should we define some enum or we have to agree on some string values?
> 
> I thought about this as well, but then decided to go with the string for
> flexibility: If we have some new device type at some point, we can just let
the
> user enter a different requisition string and/or put new requisitions in the
OEM
> partition manifest.

But if server does not understand these new strings, how do they work? Server
most likely will send back a 400 as it does not understand the new values. Or
you mean this mainly to avoid a new Chrome release, but we still need to wait
for server change to make new value useful?

[Mattias Nissler (ping if slow), 2013-05-14 07:37:43]

On 2013/05/14 06:56:55, Bin wrote:
> On 2013/05/14 06:49:51, Mattias Nissler wrote:
> >
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> > File chrome/browser/policy/proto/cloud/device_management_backend.proto
> (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> > chrome/browser/policy/proto/cloud/device_management_backend.proto:51:
optional
> > string requisition = 6;
> > On 2013/05/14 06:36:41, Bin wrote:
> > > On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > > > Bin, Raymond: I'd like to get your thoughts on this (or barring any
> > thoughts,
> > > a
> > > > thumbs up).
> > > 
> > > Should we define some enum or we have to agree on some string values?
> > 
> > I thought about this as well, but then decided to go with the string for
> > flexibility: If we have some new device type at some point, we can just let
> the
> > user enter a different requisition string and/or put new requisitions in the
> OEM
> > partition manifest.
> 
> But if server does not understand these new strings, how do they work? Server
> most likely will send back a 400 as it does not understand the new values. Or
> you mean this mainly to avoid a new Chrome release, but we still need to wait
> for server change to make new value useful?

I'm also thinking about small-scale experiments, using this for assigning
devices that enrolled after the fact to categories etc. For example, you could
just expose this as a field in the device record.

Also, I don't think hardcoding all the potential device requisitions in the
management protocol does make sense - this shouldn't be a concern of the
protocol, but a concern of the organization using the protocol.

[Bin, 2013-05-15 06:13:27]

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
File chrome/browser/policy/proto/cloud/device_management_backend.proto (right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
string requisition = 6;
My understanding (and also the comments indicates) is that this field should be
something the server understands and acts on. For example, we use this field to
indicate it is a vc device so the server can act differently.

You said the server can treat unknown values are an opaque string and just
record it down for the purpose of categorizing the devices. This is inconsistent
with the comment - "server can act upon".

My concern is that if we make it generic, people don't know what to put into
this field and if it affects server behavior. For some requisition values, the
server will act on them. For some requisition values, the server simply just
treat as opaque strings.

If we really want to keep this way, then please document the values which cause
server to act upon. Those values affect server-side behavior and should be
documented as part of protocol.


On 2013/05/14 06:49:51, Mattias Nissler wrote:
> On 2013/05/14 06:36:41, Bin wrote:
> > On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > > Bin, Raymond: I'd like to get your thoughts on this (or barring any
> thoughts,
> > a
> > > thumbs up).
> > 
> > Should we define some enum or we have to agree on some string values?
> 
> I thought about this as well, but then decided to go with the string for
> flexibility: If we have some new device type at some point, we can just let
the
> user enter a different requisition string and/or put new requisitions in the
OEM
> partition manifest.

[Mattias Nissler (ping if slow), 2013-05-15 12:02:29]

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
File chrome/browser/policy/proto/cloud/device_management_backend.proto (right):

https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
string requisition = 6;
On 2013/05/15 06:13:27, Bin wrote:
> My understanding (and also the comments indicates) is that this field should
be
> something the server understands and acts on. For example, we use this field
to
> indicate it is a vc device so the server can act differently.

No, it's just potentially so. We're essentially passing payload for a
higher-level protocol here: The client is telling the server what type of device
it is. The protocol itself doesn't need to (and IMHO shouldn't) define what the
available device types are.

> 
> You said the server can treat unknown values are an opaque string and just
> record it down for the purpose of categorizing the devices. This is
inconsistent
> with the comment - "server can act upon".

It's not - I say the server _can_ act, so it doesn't have to. In fact, you can
just decide to return a 400 if the requisition the client is sending isn't
understood by the server.

> 
> My concern is that if we make it generic, people don't know what to put into
> this field and if it affects server behavior. For some requisition values, the
> server will act on them. For some requisition values, the server simply just
> treat as opaque strings.

Well, you could document somewhere that Google's DMServer does configure clients
as Kiosk device if you set a specific requisition. Or you could allow the admin
to define new requisitions in CPanel that assign the device to specific OUs at
enrollment time. The possibilities are endless, but all that is on an entirely
different layer than the guts of the client-server handshake at device
registration time.

> 
> If we really want to keep this way, then please document the values which
cause
> server to act upon. Those values affect server-side behavior and should be
> documented as part of protocol.

I disagree. To make this clearer, let my make a comparison with TCP: There's a
port field in the message header. It allows us to route connections to the
appropriate endpoint. The TCP protocol spec doesn't say anything about what
services are to be expected on different ports and intentionally leaves the port
assignment problem to the user of the protocol. All this makes TCP flexible, not
fragile (which seems to be your main point of concern with the string field?).

> 
> 
> On 2013/05/14 06:49:51, Mattias Nissler wrote:
> > On 2013/05/14 06:36:41, Bin wrote:
> > > On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > > > Bin, Raymond: I'd like to get your thoughts on this (or barring any
> > thoughts,
> > > a
> > > > thumbs up).
> > > 
> > > Should we define some enum or we have to agree on some string values?
> > 
> > I thought about this as well, but then decided to go with the string for
> > flexibility: If we have some new device type at some point, we can just let
> the
> > user enter a different requisition string and/or put new requisitions in the
> OEM
> > partition manifest.
>

[Bin, 2013-05-15 23:43:23]

On 2013/05/15 12:02:29, Mattias Nissler wrote:
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> File chrome/browser/policy/proto/cloud/device_management_backend.proto
(right):
> 
>
https://chromiumcodereview.appspot.com/14843009/diff/1/chrome/browser/policy/...
> chrome/browser/policy/proto/cloud/device_management_backend.proto:51: optional
> string requisition = 6;
> On 2013/05/15 06:13:27, Bin wrote:
> > My understanding (and also the comments indicates) is that this field should
> be
> > something the server understands and acts on. For example, we use this field
> to
> > indicate it is a vc device so the server can act differently.
> 
> No, it's just potentially so. We're essentially passing payload for a
> higher-level protocol here: The client is telling the server what type of
device
> it is. The protocol itself doesn't need to (and IMHO shouldn't) define what
the
> available device types are.
> 
> > 
> > You said the server can treat unknown values are an opaque string and just
> > record it down for the purpose of categorizing the devices. This is
> inconsistent
> > with the comment - "server can act upon".
> 
> It's not - I say the server _can_ act, so it doesn't have to. In fact, you can
> just decide to return a 400 if the requisition the client is sending isn't
> understood by the server.
> 
> > 
> > My concern is that if we make it generic, people don't know what to put into
> > this field and if it affects server behavior. For some requisition values,
the
> > server will act on them. For some requisition values, the server simply just
> > treat as opaque strings.
> 
> Well, you could document somewhere that Google's DMServer does configure
clients
> as Kiosk device if you set a specific requisition. Or you could allow the
admin
> to define new requisitions in CPanel that assign the device to specific OUs at
> enrollment time. The possibilities are endless, but all that is on an entirely
> different layer than the guts of the client-server handshake at device
> registration time.
> 
> > 
> > If we really want to keep this way, then please document the values which
> cause
> > server to act upon. Those values affect server-side behavior and should be
> > documented as part of protocol.
> 
> I disagree. To make this clearer, let my make a comparison with TCP: There's a
> port field in the message header. It allows us to route connections to the
> appropriate endpoint. The TCP protocol spec doesn't say anything about what
> services are to be expected on different ports and intentionally leaves the
port
> assignment problem to the user of the protocol. All this makes TCP flexible,
not
> fragile (which seems to be your main point of concern with the string field?).
> 
> > 
> > 
> > On 2013/05/14 06:49:51, Mattias Nissler wrote:
> > > On 2013/05/14 06:36:41, Bin wrote:
> > > > On 2013/05/13 10:52:35, Mattias Nissler wrote:
> > > > > Bin, Raymond: I'd like to get your thoughts on this (or barring any
> > > thoughts,
> > > > a
> > > > > thumbs up).
> > > > 
> > > > Should we define some enum or we have to agree on some string values?
> > > 
> > > I thought about this as well, but then decided to go with the string for
> > > flexibility: If we have some new device type at some point, we can just
let
> > the
> > > user enter a different requisition string and/or put new requisitions in
the
> > OEM
> > > partition manifest.
> >

As discussed offline, we can go with the string and on the server-side, we will
document which type we accept and the client-side can document it when the time
is appropriate.

[Bin, 2013-05-15 23:43:34]

lgtm

[commit-bot: I haz the power, 2013-05-16 12:28:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/14843009/17001

[commit-bot: I haz the power, 2013-05-16 12:50:21]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-16 14:21:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/14843009/29001

[commit-bot: I haz the power, 2013-05-16 14:45:05]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-17 11:33:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/14843009/29001

[commit-bot: I haz the power, 2013-05-17 14:30:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/14843009/48001

[commit-bot: I haz the power, 2013-05-17 16:32:53]

Change committed as 200822