diff --git a/docs/backend/users-and-permissions.md b/docs/backend/users-and-permissions.md index 357aaca..94da4e0 100644 --- a/docs/backend/users-and-permissions.md +++ b/docs/backend/users-and-permissions.md @@ -73,11 +73,16 @@ Actions registered through `pact.HasAdminActions` may name extra permissions, ch The application binary has two commands for operators: ```sh -./bin/acme admin:create --email admin@example.com --password '' --superuser -./bin/acme admin:reset-password admin@example.com --password '' +./bin/acme admin:create --email admin@example.com --superuser +./bin/acme admin:reset-password admin@example.com ``` -`admin:create` creates an activated administrator; `--login` defaults to the lower-cased email and `--role ` assigns a role. It refuses a login or email that matches another administrator's login or email in either field, because a sign-in identifier that matches two administrators is answered like a wrong password. `admin:reset-password` takes a login or an email, sets the password and revokes every token issued before the reset. Passwords are hashed with bcrypt at `admin.password.bcrypt_cost`, so hashes copied from a WinterCMS database keep working. +Both commands ask for the password at a prompt that does not echo it. In a script, pipe it on stdin so it never appears in the process list or the shell history: -> [!TIP] -> Pass the password through an environment variable or a prompt of your shell rather than typing it on the command line, where it stays in the shell history. +```sh +printf '%s\n' "$ADMIN_PASSWORD" | ./bin/acme admin:create --email admin@example.com --superuser +``` + +`--password` is still accepted but deprecated: the command prints a warning, because the value is visible to other users in the process list and stays in the shell history. + +`admin:create` creates an activated administrator; `--login` defaults to the lower-cased email and `--role ` assigns a role. It refuses a login or email that matches another administrator's login or email in either field, because a sign-in identifier that matches two administrators is answered like a wrong password. `admin:reset-password` takes a login or an email, sets the password and revokes every token issued before the reset. Passwords are hashed with bcrypt at `admin.password.bcrypt_cost`, so hashes copied from a WinterCMS database keep working. diff --git a/docs/console/setup-and-maintenance.md b/docs/console/setup-and-maintenance.md index e157463..fc84260 100644 --- a/docs/console/setup-and-maintenance.md +++ b/docs/console/setup-and-maintenance.md @@ -54,15 +54,15 @@ The default `--addr` listens on every interface. Pass a loopback address during | Command | Arguments and flags | Purpose | |---------|---------------------|---------| -| `admin:create` | `--email`, `--password` (both required), `--login`, `--role `, `--superuser` | Creates an activated backend administrator. `--login` defaults to the lower-cased email. | -| `admin:reset-password` | `` (login or email), `--password` | Sets a new password and revokes every token issued before the reset. | +| `admin:create` | `--email` (required), `--login`, `--role `, `--superuser`, `--password` (deprecated) | Creates an activated backend administrator. `--login` defaults to the lower-cased email. The password is read from a hidden prompt, or from stdin when the input is not a terminal. | +| `admin:reset-password` | `` (login or email), `--password` (deprecated) | Sets a new password, read like the one of `admin:create`, and revokes every token issued before the reset. | ```sh -./bin/acme admin:create --email admin@example.com --password '' --superuser -./bin/acme admin:reset-password admin@example.com --password '' +./bin/acme admin:create --email admin@example.com --superuser +./bin/acme admin:reset-password admin@example.com ``` -Passwords passed as flags end up in your shell history. Prefer reading them from a secrets manager into a variable. The admin is described in [cabana](../../modules/cabana/README.md). +Both commands prompt for the password without echoing it. In a script, pipe it on stdin, for example `printf '%s\n' "$ADMIN_PASSWORD" | ./bin/acme admin:create --email admin@example.com --superuser`. The `--password` flag still works but is deprecated and prints a warning, because the value stays in your shell history and is visible in the process list. The admin is described in [cabana](../../modules/cabana/README.md). ## Queues and the scheduler diff --git a/modules/cabana/README.md b/modules/cabana/README.md index be473f3..62e351e 100644 --- a/modules/cabana/README.md +++ b/modules/cabana/README.md @@ -184,14 +184,16 @@ Both commands are added to every application binary by the generated `main` and | Command | Arguments and flags | Effect | |---------|---------------------|--------| -| `admin:create` | `--email`, `--password` (both required), `--login` (defaults to the lower-cased email), `--role `, `--superuser` | Creates an activated backend administrator. | -| `admin:reset-password` | `` (login or email), `--password` | Sets a new password and revokes every token issued before the reset. | +| `admin:create` | `--email` (required), `--login` (defaults to the lower-cased email), `--role `, `--superuser`, `--password` (deprecated) | Creates an activated backend administrator. The password is read from a hidden prompt, or from stdin when it is not a terminal. | +| `admin:reset-password` | `` (login or email), `--password` (deprecated) | Sets a new password, read like the one of `admin:create`, and revokes every token issued before the reset. | ```sh -./bin/acme admin:create --email admin@example.com --password '' --superuser -./bin/acme admin:reset-password admin@example.com --password '' +./bin/acme admin:create --email admin@example.com --superuser +printf '%s\n' "$ADMIN_PASSWORD" | ./bin/acme admin:reset-password admin@example.com ``` +`--password` still works, but it prints a deprecation warning: a value on the command line is visible in the process list and stays in the shell history. + ## Dependencies - SummerCMS modules: [backpack](../backpack/README.md), [boardwalk](../boardwalk/README.md), [bonfire](../bonfire/README.md), [bouncer](../bouncer/README.md), [lagoon](../lagoon/README.md), [pact](../pact/README.md), [party](../party/README.md), [phrasebook](../phrasebook/README.md), [towel](../towel/README.md). diff --git a/modules/cabana/commands.go b/modules/cabana/commands.go index c99481f..55bef13 100644 --- a/modules/cabana/commands.go +++ b/modules/cabana/commands.go @@ -22,7 +22,7 @@ func RuntimeCommands(app *backpack.App) []bonfire.Command { Description: "Create an activated backend administrator", Flags: []bonfire.Flag{ {Name: "email", Description: "Admin email"}, - {Name: "password", Description: "Admin password"}, + {Name: "password", Description: "Admin password (deprecated: visible in the process list and shell history; omit it to be prompted, or pipe it on stdin)"}, {Name: "login", Description: "Login; defaults to the lower-cased email"}, {Name: "superuser", Description: "Grant superuser", Bare: true}, {Name: "role", Description: "Role code"}, @@ -41,7 +41,7 @@ func RuntimeCommands(app *backpack.App) []bonfire.Command { }}, Flags: []bonfire.Flag{{ Name: "password", - Description: "New password", + Description: "New password (deprecated: visible in the process list and shell history; omit it to be prompted, or pipe it on stdin)", }}, Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { return adminResetPassword(ctx, app, in, out) @@ -52,9 +52,12 @@ func RuntimeCommands(app *backpack.App) []bonfire.Command { func adminCreate(ctx context.Context, app *backpack.App, in bonfire.Input, out bonfire.Output) error { email := strings.ToLower(strings.TrimSpace(flagValue(in, "email"))) - password := flagValue(in, "password") - if email == "" || !strings.Contains(email, "@") || strings.TrimSpace(password) == "" { - return errors.New("cabana: email and password are required") + if email == "" || !strings.Contains(email, "@") { + return errors.New("cabana: a valid email is required") + } + password, err := resolvePassword(in, out) + if err != nil { + return err } login := strings.TrimSpace(flagValue(in, "login")) if login == "" { @@ -109,9 +112,12 @@ func adminResetPassword(ctx context.Context, app *backpack.App, in bonfire.Input if identifier == "" && len(in.Args()) > 0 { identifier = strings.TrimSpace(in.Args()[0]) } - password := flagValue(in, "password") - if identifier == "" || strings.TrimSpace(password) == "" { - return errors.New("cabana: identifier and password are required") + if identifier == "" { + return errors.New("cabana: an identifier is required") + } + password, err := resolvePassword(in, out) + if err != nil { + return err } return withAdminDB(ctx, app, func(gdb *gorm.DB) error { return gdb.WithContext(ctx).Transaction(func(tx *gorm.DB) error { @@ -143,6 +149,28 @@ func adminResetPassword(ctx context.Context, app *backpack.App, in bonfire.Input }) } +// resolvePassword returns the password for admin:create and +// admin:reset-password without putting it on the command line. Without +// --password it is read through out.Secret: hidden on a terminal, one line from +// stdin otherwise, so scripts can pipe it. --password still works, because the +// documented usage and existing scripts pass it, but it is deprecated: the value +// is visible in the process list and the shell history. +func resolvePassword(in bonfire.Input, out bonfire.Output) (string, error) { + password := flagValue(in, "password") + if password != "" { + out.Warning("--password is deprecated: the value is visible in the process list and the shell history. Omit it to be prompted, or pipe the password on stdin.") + } else { + var err error + if password, err = out.Secret("Password"); err != nil { + return "", err + } + } + if strings.TrimSpace(password) == "" { + return "", errors.New("cabana: a password is required (enter it at the prompt or pipe it on stdin)") + } + return password, nil +} + func roleIDByCode(tx *gorm.DB, code string) (*uint, error) { if code == "" { return nil, nil diff --git a/modules/cabana/commands_test.go b/modules/cabana/commands_test.go index ace11a7..9a48f3a 100644 --- a/modules/cabana/commands_test.go +++ b/modules/cabana/commands_test.go @@ -226,3 +226,57 @@ func TestAdminCreateRejectsCrossFieldCollision(t *testing.T) { t.Fatalf("admins after refused creates = %d, %v; want 1", n, err) } } + +// TestAdminPasswordWithoutFlag pins WR-13: the password can come from stdin +// (the non-terminal path of the prompt) instead of the command line, the +// --password flag still works but warns that it is deprecated, and no password +// at all is refused. +func TestAdminPasswordWithoutFlag(t *testing.T) { + gdb := adminGorm(t) + app := commandApp(t, gdb) + create := commandByName(t, cabana.RuntimeCommands(app), "admin:create") + reset := commandByName(t, cabana.RuntimeCommands(app), "admin:reset-password") + const piped = "password-piped-on-stdin" + + var buf bytes.Buffer + out := bonfire.NewOutput(strings.NewReader(piped+"\n"), &buf, &buf) + if err := create.Run(context.Background(), flagInput{flags: map[string]string{"email": "stdin-pw@example.test"}}, out); err != nil { + t.Fatalf("create with the password on stdin: %v", err) + } + var user cabana.BackendUser + if err := gdb.Where("login = ?", "stdin-pw@example.test").First(&user).Error; err != nil { + t.Fatal(err) + } + if !bouncer.CheckPassword(user.Password, piped) { + t.Fatal("password read from stdin was not stored") + } + if strings.Contains(buf.String(), piped) || strings.Contains(buf.String(), "deprecated") { + t.Fatalf("stdin path leaked the password or warned: %s", buf.String()) + } + + const next = "replacement-piped-on-stdin" + buf.Reset() + out = bonfire.NewOutput(strings.NewReader(next+"\n"), &buf, &buf) + if err := reset.Run(context.Background(), flagInput{args: []string{"stdin-pw@example.test"}}, out); err != nil { + t.Fatalf("reset with the password on stdin: %v", err) + } + if err := gdb.Where("login = ?", "stdin-pw@example.test").First(&user).Error; err != nil || !bouncer.CheckPassword(user.Password, next) { + t.Fatalf("reset did not store the stdin password: %v", err) + } + + buf.Reset() + out = bonfire.NewOutput(strings.NewReader(""), &buf, &buf) + err := reset.Run(context.Background(), flagInput{args: []string{"stdin-pw@example.test"}}, out) + if err == nil || !strings.Contains(err.Error(), "password is required") { + t.Fatalf("reset with no password err = %v", err) + } + + buf.Reset() + out = bonfire.NewOutput(strings.NewReader(""), &buf, &buf) + if err := reset.Run(context.Background(), flagInput{args: []string{"stdin-pw@example.test"}, flags: map[string]string{"password": "flag-password-value"}}, out); err != nil { + t.Fatalf("reset with the deprecated flag: %v", err) + } + if !strings.Contains(buf.String(), "deprecated") || strings.Contains(buf.String(), "flag-password-value") { + t.Fatalf("deprecated flag output = %s", buf.String()) + } +}