fix(09): WR-13 read admin passwords from a prompt or stdin and deprecate the --password flag

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 <code>`, `--superuser` | Creates an activated backend administrator. |
-| `admin:reset-password` | `<identifier>` (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 <code>`, `--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` | `<identifier>` (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 '<secret>' --superuser
-./bin/acme admin:reset-password admin@example.com --password '<secret>'
+./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).

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

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())
+	}
+}