state/api: enable authentication

state/api: enable authentication



https://code.launchpad.net/~rogpeppe/juju-core/279-api-enable-auth/+merge/158170

(do not edit description out of merge proposal)

[rog, 2013-04-10 17:20:32]

Please take a look.

[fwereade, 2013-04-10 21:04:29]

LGTM

[dave_cheney.net, 2013-04-11 06:03:01]

LGTM with the general observation that this CL, and (without wanting to pick on
him) Dimiters previous CL both appear to be hard coding what is essentially an
opaque token.

In this CL you call SetPassword("something"), then "something" is then used to
login to that service later on. Would you consider inverting the logic to be
something like

p, err := thing.ResetPassword(), which gives you the new password without
actually having to know it.

https://codereview.appspot.com/8626044/diff/4001/cmd/jujud/bootstrap.go
File cmd/jujud/bootstrap.go (right):

https://codereview.appspot.com/8626044/diff/4001/cmd/jujud/bootstrap.go#newco...
cmd/jujud/bootstrap.go:84: u, err := st.AddUser("admin", "")
Is there a possibility if a step after this fails, the admin user will remain
with a blank password ?

If so, could the initial password be made some random string ?

[rog, 2013-04-11 13:20:17]

On 11 April 2013 07:03, <dave@cheney.net> wrote:

> LGTM with the general observation that this CL, and (without wanting to
> pick on him) Dimiters previous CL both appear to be hard coding what is
> essentially an opaque token.
>
> In this CL you call SetPassword("something"), then "something" is then
> used to login to that service later on. Would you consider inverting the
> logic to be something like
>
> p, err := thing.ResetPassword(), which gives you the new password
> without actually having to know it.
>

As discussed online, the whole point of SetPassword is to set a password
that's been chosen elsewhere (in this particular case, it's the admin
secret)


>
>
> https://codereview.appspot.**com/8626044/diff/4001/cmd/**
>
jujud/bootstrap.go<https://codereview.appspot.com/8626044/diff/4001/cmd/jujud/bootstrap.go>
> File cmd/jujud/bootstrap.go (right):
>
> https://codereview.appspot.**com/8626044/diff/4001/cmd/**
>
jujud/bootstrap.go#newcode84<https://codereview.appspot.com/8626044/diff/4001/cmd/jujud/bootstrap.go#newcode84>
> cmd/jujud/bootstrap.go:84: u, err := st.AddUser("admin", "")
> Is there a possibility if a step after this fails, the admin user will
> remain with a blank password ?
>
> If so, could the initial password be made some random string ?
>

I would prefer to fix this problem at its root - currently we carry on
executing
the cloudinit  script even when the init script has failed.

If we fix that behaviour, then I don't think the above is an issue.

[rog, 2013-04-11 14:09:38]

*** Submitted:

state/api: enable authentication

R=fwereade, dfc
CC=
https://codereview.appspot.com/8626044

[dave_cheney.net, 2013-04-11 23:54:37]

>> If so, could the initial password be made some random string ?
>
>
> I would prefer to fix this problem at its root - currently we carry on
> executing
> the cloudinit  script even when the init script has failed.
>
> If we fix that behaviour, then I don't think the above is an issue.

I think there is another source of error, there are three method calls
which return an error inside that method, the first may succeed,
creating the user Admin, with the password "", but if the second
fails, the account will remain with a password of "".