[autotest] More ownership/device policy tests

[autotest] More ownership/device policy tests

Improve ownership test infrastructure, make a base class for ownership tests
that actually exercise the API, and add a new test that validates the owner
key setting/rotating logic that will be (mostly) used by enterprise managed
device support.

BUG=13746
TEST=login_Ownership*, login_RemoteOwnership

Change-Id: I6a3a405744ca585bb93bb7b0a47f4536b0832744

R=sosa@chromium.org

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=eac4f4f

[sosa, 2011-04-06 01:25:00]

Nice cleanup and refactoring!

Sorry rietveld is hating at the moment but my only real comments were
if you could add docstrings to your super test class so it's easier
for others to use.  Specifically, can you add a sentence string for
connect_to_session_manager, generate_policy, and push_policy?

Also, I'm not sure this affects your tests or not, but your
cros_owner_test doesn't actually call it's parent's (test.Test)
parent's class versions of initialize and cleanup.  You may want to
add this.

Finally, you don't use ownership.pairgen_as_data anywhere.  Did you
mean to include this now?

-Sosa

On Tue, Apr 5, 2011 at 4:41 PM,  <cmasone@chromium.org> wrote:
> Reviewers: sosa,
>
> Description:
> [autotest] More ownership/device policy tests
>
> Improve ownership test infrastructure, make a base class for ownership tests
> that actually exercise the API, and add a new test that validates the owner
> key setting/rotating logic that will be (mostly) used by enterprise managed
> device support.
>
> BUG=13746
> TEST=login_Ownership*, login_RemoteOwnership
>
> Change-Id: I6a3a405744ca585bb93bb7b0a47f4536b0832744
>
> R=sosa@chromium.org
>
>
> Please review this at http://codereview.chromium.org/6677180/
>
> SVN Base: http://git.chromium.org/git/autotest.git@master
>
> Affected files:
>  A client/cros/cros_ownership_test.py
>  M client/cros/cros_ui_test.py
>  M client/cros/ownership.py
>  M client/site_tests/login_OwnershipApi/control
>  M client/site_tests/login_OwnershipApi/login_OwnershipApi.py
>
>
>

[Chris Masone, 2011-04-06 01:27:57]

heh...I forgot to add a bunch of files to the CL!  Lemme add those tonight
and then I"ll ping back for moar review

On Tue, Apr 5, 2011 at 6:00 PM, Chris Sosa <sosa@chromium.org> wrote:

> Nice cleanup and refactoring!
>
> Sorry rietveld is hating at the moment but my only real comments were
> if you could add docstrings to your super test class so it's easier
> for others to use.  Specifically, can you add a sentence string for
> connect_to_session_manager, generate_policy, and push_policy?
>
> Also, I'm not sure this affects your tests or not, but your
> cros_owner_test doesn't actually call it's parent's (test.Test)
> parent's class versions of initialize and cleanup.  You may want to
> add this.
>
> Finally, you don't use ownership.pairgen_as_data anywhere.  Did you
> mean to include this now?
>
> -Sosa
>
> On Tue, Apr 5, 2011 at 4:41 PM,  <cmasone@chromium.org> wrote:
> > Reviewers: sosa,
> >
> > Description:
> > [autotest] More ownership/device policy tests
> >
> > Improve ownership test infrastructure, make a base class for ownership
> tests
> > that actually exercise the API, and add a new test that validates the
> owner
> > key setting/rotating logic that will be (mostly) used by enterprise
> managed
> > device support.
> >
> > BUG=13746
> > TEST=login_Ownership*, login_RemoteOwnership
> >
> > Change-Id: I6a3a405744ca585bb93bb7b0a47f4536b0832744
> >
> > R=sosa@chromium.org
> >
> >
> > Please review this at http://codereview.chromium.org/6677180/
> >
> > SVN Base: http://git.chromium.org/git/autotest.git@master
> >
> > Affected files:
> >  A client/cros/cros_ownership_test.py
> >  M client/cros/cros_ui_test.py
> >  M client/cros/ownership.py
> >  M client/site_tests/login_OwnershipApi/control
> >  M client/site_tests/login_OwnershipApi/login_OwnershipApi.py
> >
> >
> >
>

[Chris Masone, 2011-04-06 04:43:42]

Ok, now all the right code is here, and comments are added.

[sosa, 2011-04-06 21:19:52]

If I understand the second test correctly ... we:

Set up a key to emulate we have some device signed with initial policy set up. 
However, we then emulate someone re-setting up a policy (but before ownership is
taken) and then verify that we can set up a policy with the private key of what
we re-signed the device with.

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
File client/site_tests/login_RemoteOwnership/control (right):

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
client/site_tests/login_RemoteOwnership/control:12: PURPOSE = "Validate the the
Ownership DBus API works for device policies"
s/first "the"/that

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
File client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py (right):

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py:57: raise
error.TestFail('Could not start session for random user')
s/start/stop?

[Chris Masone, 2011-04-06 21:28:19]

login_RemoteOwnership isn't testing local-device-owner cases.

The first thing it does is take ownership, as an enterprise would, by pushing a
policy with a key in it before anyone has logged in.

The second thing is force a new key to be honored, which we have decided to do
by just blindly accepting any key that is pushed down in a policy while no one
is logged in.

The last thing is the graceful key-rotation case, where we only honor new keys
that are signed with the currently-registered owner key

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
File client/site_tests/login_RemoteOwnership/control (right):

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
client/site_tests/login_RemoteOwnership/control:12: PURPOSE = "Validate the the
Ownership DBus API works for device policies"
On 2011/04/06 21:19:52, sosa wrote:
> s/first "the"/that

Done.

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
File client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py (right):

http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py:57: raise
error.TestFail('Could not start session for random user')
On 2011/04/06 21:19:52, sosa wrote:
> s/start/stop?

Done.

[sosa, 2011-04-06 21:30:35]

Ok that's also consistent with what I saw.

LGTM

On Wed, Apr 6, 2011 at 2:28 PM,  <cmasone@chromium.org> wrote:
> login_RemoteOwnership isn't testing local-device-owner cases.
>
> The first thing it does is take ownership, as an enterprise would, by
> pushing a
> policy with a key in it before anyone has logged in.
>
> The second thing is force a new key to be honored, which we have decided to
> do
> by just blindly accepting any key that is pushed down in a policy while no
> one
> is logged in.
>
> The last thing is the graceful key-rotation case, where we only honor new
> keys
> that are signed with the currently-registered owner key
>
>
>
http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
> File client/site_tests/login_RemoteOwnership/control (right):
>
>
http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
> client/site_tests/login_RemoteOwnership/control:12: PURPOSE = "Validate
> the the Ownership DBus API works for device policies"
> On 2011/04/06 21:19:52, sosa wrote:
>>
>> s/first "the"/that
>
> Done.
>
>
http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
> File client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py
> (right):
>
>
http://codereview.chromium.org/6677180/diff/5005/client/site_tests/login_Remo...
> client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py:57:
> raise error.TestFail('Could not start session for random user')
> On 2011/04/06 21:19:52, sosa wrote:
>>
>> s/start/stop?
>
> Done.
>
> http://codereview.chromium.org/6677180/
>