Skip to content

Commit

Permalink
Merge pull request #15475 from hmlanigan/merge-to-3.0
Browse files Browse the repository at this point in the history
#15475

- #15473 from jack-w-shaw/JUJU-3486_improve_logging
- #15472 from hpidcock/no-prompt-password
- #15470 from jack-w-shaw/JUJU-3486_fix_verbosity
- #15465 from barrettj12/bundle-arm


Conflicts:
* cmd/juju/user/change_password_test.go
  • Loading branch information
jujubot committed Apr 13, 2023
2 parents 9bf0cc3 + 523ac90 commit 37e2743
Show file tree
Hide file tree
Showing 12 changed files with 193 additions and 61 deletions.
23 changes: 20 additions & 3 deletions cmd/juju/user/change_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ type changePasswordCommand struct {
User string
Reset bool

noPrompt bool

// Internally initialised and used during run
controllerName string
userTag names.UserTag
Expand All @@ -79,6 +81,7 @@ type changePasswordCommand struct {

func (c *changePasswordCommand) SetFlags(f *gnuflag.FlagSet) {
f.BoolVar(&c.Reset, "reset", false, "Reset user password")
f.BoolVar(&c.noPrompt, "no-prompt", false, "don't prompt for password and just read a line from stdin")
}

// Info implements Command.Info.
Expand Down Expand Up @@ -199,9 +202,23 @@ func (c *changePasswordCommand) resetUserPassword(ctx *cmd.Context) error {
}

func (c *changePasswordCommand) updateUserPassword(ctx *cmd.Context) error {
newPassword, err := readAndConfirmPassword(ctx)
if err != nil {
return errors.Trace(err)
var err error
var newPassword string
if c.noPrompt {
fmt.Fprintln(ctx.Stderr, "reading password from stdin...")
newPassword, err = readLine(ctx.Stdin)
if err != nil {
return errors.Trace(err)
}
} else {
newPassword, err = readAndConfirmPassword(ctx)
if err != nil {
return errors.Trace(err)
}
}

if newPassword == "" {
return errors.Errorf("password cannot be empty")
}

if err := c.api.SetPassword(c.userTag.Id(), newPassword); err != nil {
Expand Down
33 changes: 24 additions & 9 deletions cmd/juju/user/change_password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (s *ChangePasswordCommandSuite) SetUpTest(c *gc.C) {
s.store = s.BaseSuite.store
}

func (s *ChangePasswordCommandSuite) run(c *gc.C, args ...string) (*cmd.Context, *juju.NewAPIConnectionParams, error) {
func (s *ChangePasswordCommandSuite) run(c *gc.C, stdin string, args ...string) (*cmd.Context, *juju.NewAPIConnectionParams, error) {
var argsOut juju.NewAPIConnectionParams
newAPIConnection := func(args juju.NewAPIConnectionParams) (api.Connection, error) {
argsOut = args
Expand All @@ -43,7 +43,7 @@ func (s *ChangePasswordCommandSuite) run(c *gc.C, args ...string) (*cmd.Context,
newAPIConnection, s.mockAPI, s.store,
)
ctx := cmdtesting.Context(c)
ctx.Stdin = strings.NewReader("sekrit\nsekrit\n")
ctx.Stdin = strings.NewReader(stdin)
err := cmdtesting.InitCommand(changePasswordCommand, args)
if err != nil {
return ctx, nil, err
Expand Down Expand Up @@ -91,7 +91,7 @@ func (s *ChangePasswordCommandSuite) assertAPICalls(c *gc.C, user, pass string)
}

func (s *ChangePasswordCommandSuite) TestChangePassword(c *gc.C) {
context, args, err := s.run(c)
context, args, err := s.run(c, "sekrit\nsekrit\n")
c.Assert(err, jc.ErrorIsNil)
s.assertAPICalls(c, "current-user", "sekrit")
c.Assert(cmdtesting.Stdout(context), gc.Equals, "")
Expand All @@ -106,34 +106,49 @@ Your password has been changed.
})
}

func (s *ChangePasswordCommandSuite) TestChangePasswordNoPrompt(c *gc.C) {
context, args, err := s.run(c, "sneaky-password\n", "--no-prompt")
c.Assert(err, jc.ErrorIsNil)
s.assertAPICalls(c, "current-user", "sneaky-password")
c.Assert(cmdtesting.Stdout(context), gc.Equals, "")
c.Assert(cmdtesting.Stderr(context), gc.Equals, `
reading password from stdin...
Your password has been changed.
`[1:])
// The command should have logged in without a password to get a macaroon.
c.Assert(args.AccountDetails, jc.DeepEquals, &jujuclient.AccountDetails{
User: "current-user",
})
}

func (s *ChangePasswordCommandSuite) TestChangePasswordFail(c *gc.C) {
s.mockAPI.SetErrors(errors.New("failed to do something"))
_, _, err := s.run(c)
_, _, err := s.run(c, "sekrit\nsekrit\n")
c.Assert(err, gc.ErrorMatches, "failed to do something")
s.assertAPICalls(c, "current-user", "sekrit")
}

func (s *ChangePasswordCommandSuite) TestChangeOthersPassword(c *gc.C) {
// The checks for user existence and admin rights are tested
// at the apiserver level.
_, _, err := s.run(c, "other")
_, _, err := s.run(c, "sekrit\nsekrit\n", "other")
c.Assert(err, jc.ErrorIsNil)
s.assertAPICalls(c, "other", "sekrit")
}

func (s *ChangePasswordCommandSuite) TestResetSelfPasswordFail(c *gc.C) {
context, _, err := s.run(c, "--reset")
context, _, err := s.run(c, "", "--reset")
s.assertResetSelfPasswordFail(c, context, err)
}

func (s *ChangePasswordCommandSuite) TestResetSelfPasswordSpecifyYourselfFail(c *gc.C) {
context, _, err := s.run(c, "--reset", "current-user")
context, _, err := s.run(c, "", "--reset", "current-user")
s.assertResetSelfPasswordFail(c, context, err)
}

func (s *ChangePasswordCommandSuite) TestResetPasswordFail(c *gc.C) {
s.mockAPI.SetErrors(errors.New("failed to do something"))
context, _, err := s.run(c, "--reset", "other")
context, _, err := s.run(c, "", "--reset", "other")
c.Assert(err, gc.ErrorMatches, "failed to do something")
s.mockAPI.CheckCalls(c, []testing.StubCall{
{"ResetPassword", []interface{}{"other"}},
Expand All @@ -148,7 +163,7 @@ func (s *ChangePasswordCommandSuite) TestResetOthersPassword(c *gc.C) {
// The checks for user existence and admin rights are tested
// at the apiserver level.
s.mockAPI.key = []byte("no cats or dragons")
context, _, err := s.run(c, "other", "--reset")
context, _, err := s.run(c, "", "other", "--reset")
c.Assert(err, jc.ErrorIsNil)
s.mockAPI.CheckCalls(c, []testing.StubCall{
{"ResetPassword", []interface{}{"other"}},
Expand Down
49 changes: 40 additions & 9 deletions cmd/juju/user/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,12 @@ func NewLoginCommand() cmd.Command {
// loginCommand changes the password for a user.
type loginCommand struct {
modelcmd.ControllerCommandBase
domain string
username string
pollster *interact.Pollster
domain string
username string
noPrompt bool
noPromptPassword string
trust bool
pollster *interact.Pollster

// controllerName holds the name of the current controller.
// We define this and the --controller flag here because
Expand Down Expand Up @@ -133,6 +136,8 @@ func (c *loginCommand) SetFlags(fset *gnuflag.FlagSet) {
fset.StringVar(&c.controllerName, "controller", "", "")
fset.StringVar(&c.username, "u", "", "log in as this local user")
fset.StringVar(&c.username, "user", "", "")
fset.BoolVar(&c.noPrompt, "no-prompt", false, "don't prompt for password just read a line from stdin")
fset.BoolVar(&c.trust, "trust", false, "automatically trust controller CA certificate")
}

// Init implements Command.Init.
Expand All @@ -147,8 +152,10 @@ func (c *loginCommand) Init(args []string) error {

// Run implements Command.Run.
func (c *loginCommand) Run(ctx *cmd.Context) error {
errout := interact.NewErrWriter(ctx.Stdout)
c.pollster = interact.New(ctx.Stdin, ctx.Stderr, errout)
if !c.noPrompt {
errout := interact.NewErrWriter(ctx.Stdout)
c.pollster = interact.New(ctx.Stdin, ctx.Stderr, errout)
}

err := c.run(ctx)
if err != nil && c.onRunError != nil {
Expand Down Expand Up @@ -351,6 +358,19 @@ func (c *loginCommand) publicControllerLogin(
}
dialOpts.BakeryClient.InteractionMethods = []httpbakery.Interactor{
authentication.NewInteractor(d.User, func(string) (string, error) {
if c.noPrompt {
if c.noPromptPassword != "" {
return c.noPromptPassword, nil
}
fmt.Fprintln(ctx.Stderr, "reading password from stdin...")
password, err := readLine(ctx.Stdin)
if err != nil {
return "", err
}
c.noPromptPassword = password
return password, nil
}

// The visitor from the authentication package
// passes the username to the password getter
// func. As other password getters may rely on
Expand Down Expand Up @@ -442,6 +462,10 @@ Run "juju logout" first before attempting to log in as a different user.`,
return nil, nil, errors.Trace(err)
}

if c.noPrompt {
return nil, nil, errors.Errorf("cannot deduce user, please pass --user with --no-prompt")
}

// CodeNoCreds was returned, which means that external users
// are not supported. Ask the user to type in their username
// and try a macaroon-based authentication; if that also fails
Expand Down Expand Up @@ -593,10 +617,17 @@ func (c *loginCommand) promptUserToTrustCA(ctx *cmd.Context, ctrlDetails *jujucl
prettyName = fmt.Sprintf("%q (%s)", host, endpoint)
}
fmt.Fprintf(ctx.Stderr, "Controller %s presented a CA cert that could not be verified.\nCA fingerprint: [%s]\n", prettyName, fingerprint)
// If the user does not type Y, pollster returns false (and an error
// which doesn't really matter) causing the if block below to abort
// the connection attempt with an error.
trust, _ := c.pollster.YN("Trust remote controller", false)

trust := c.trust
if !c.noPrompt {
if trust {
fmt.Fprintln(ctx.Stderr, "Ignoring --trust without --no-prompt")
}
// If the user does not type Y, pollster returns false (and an error
// which doesn't really matter) causing the if block below to abort
// the connection attempt with an error.
trust, _ = c.pollster.YN("Trust remote controller", false)
}
if !trust {
return errors.New("controller CA not trusted")
}
Expand Down
60 changes: 53 additions & 7 deletions cmd/juju/user/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,13 @@ func (s *LoginCommandSuite) TestLoginWithCAVerification(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)

specs := []struct {
descr string
input string
host string
endpoint string
expRegex string
expCode int
descr string
input string
host string
endpoint string
expRegex string
expCode int
extraArgs []string
}{
{
descr: "user trusts CA cert",
Expand Down Expand Up @@ -314,6 +315,51 @@ Controller "127.0.0.1:443" presented a CA cert that could not be verified.
CA fingerprint: [` + fingerprint + `]
Trust remote controller? (y/N):
ERROR cannot log into "127.0.0.1:443": controller CA not trusted
`,
expCode: 1,
},
{
descr: "user does not trust CA cert when logging to a controller IP without prompt",
input: "",
extraArgs: []string{"--no-prompt"},
host: "127.0.0.1:443",
endpoint: "127.0.0.1:443",
expRegex: `
Controller "127.0.0.1:443" presented a CA cert that could not be verified.
CA fingerprint: [` + fingerprint + `]
ERROR cannot log into "127.0.0.1:443": controller CA not trusted
`,
expCode: 1,
},
{
descr: "user trusts blindly with --trust",
input: "",
extraArgs: []string{"--no-prompt", "--trust"},
host: "127.0.0.1:443",
endpoint: "127.0.0.1:443",
expRegex: `
Controller "127.0.0.1:443" presented a CA cert that could not be verified.
CA fingerprint: [` + fingerprint + `]
Welcome, new-user. You are now logged into "foo".
There are no models available. You can add models with
"juju add-model", or you can ask an administrator or owner
of a model to grant access to that model with "juju grant".
`,
expCode: 0,
},
{
descr: "user does not trust CA cert when logging to a controller IP ignoring --trust",
input: "n\n",
extraArgs: []string{"--trust"},
host: "127.0.0.1:443",
endpoint: "127.0.0.1:443",
expRegex: `
Controller "127.0.0.1:443" presented a CA cert that could not be verified.
CA fingerprint: [` + fingerprint + `]
Ignoring --trust without --no-prompt
Trust remote controller? (y/N):
ERROR cannot log into "127.0.0.1:443": controller CA not trusted
`,
expCode: 1,
},
Expand All @@ -331,7 +377,7 @@ ERROR cannot log into "127.0.0.1:443": controller CA not trusted
return s.apiConnection, nil
}

stdout, stderr, code := runLogin(c, spec.input, spec.host, "-c", "foo", "-u", "new-user")
stdout, stderr, code := runLogin(c, spec.input, append([]string{spec.host, "-c", "foo", "-u", "new-user"}, spec.extraArgs...)...)
c.Check(stdout, gc.Equals, ``)
c.Check(stderr, gc.Equals, spec.expRegex[1:])
c.Assert(code, gc.Equals, spec.expCode)
Expand Down
6 changes: 3 additions & 3 deletions tests/suites/deploy/bundles/overlay_bundle.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
applications:
tiny-bash:
charm: tiny-bash
juju-qa-test:
charm: juju-qa-test
scale: 1

---

applications:
tiny-bash:
juju-qa-test:
scale: 2
10 changes: 6 additions & 4 deletions tests/suites/deploy/bundles/telegraf_bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ applications:
charm: telegraf
channel: stable
revision: 53
tinybash:
charm: tiny-bash
juju-qa-test:
charm: juju-qa-test
channel: stable
revision: 2
revision: 4
resources:
foo-file: 2
num_units: 1
to:
- "1"
Expand All @@ -25,6 +27,6 @@ machines:
"1": {}
relations:
- - telegraf:juju-info
- tinybash:juju-info
- juju-qa-test:juju-info
- - telegraf:influxdb-api
- influxdb:query
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ applications:
charm: telegraf
channel: stable
revision: -1
tinybash:
charm: tiny-bash
channel: stable
juju-qa-test:
charm: juju-qa-test
channel: candidate
revision: -1
resources:
foo-file: 2
num_units: 1
to:
- "1"
Expand All @@ -25,6 +27,6 @@ machines:
"1": {}
relations:
- - telegraf:juju-info
- tinybash:juju-info
- juju-qa-test:juju-info
- - telegraf:influxdb-api
- influxdb:query
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ applications:
telegraf:
charm: telegraf
channel: stable
tinybash:
charm: tiny-bash
channel: stable
juju-qa-test:
charm: juju-qa-test
channel: candidate
num_units: 1
resources:
foo-file: 2
to:
- "1"
constraints: arch=amd64
Expand All @@ -22,6 +24,6 @@ machines:
"1": {}
relations:
- - telegraf:juju-info
- tinybash:juju-info
- juju-qa-test:juju-info
- - telegraf:influxdb-api
- influxdb:query
Loading

0 comments on commit 37e2743

Please sign in to comment.