fix: hide password for create-service-broker

The `cf create-service-broker` command did not allow the password to be
specified as an environment variable or via a prompt, meaning that users
with certain logging options ended up with a password in the log. This
fix brings the `cf create-service-broker` and `cf update-service-broker`
into line with `cf auth` and `cf login` which allow credentials to be
specified via environment variables and interactive prompts.

command/flag/arguments.go

@@ -324,8 +324,8 @@ type ServiceBroker struct {
 type ServiceBrokerArgs struct {
 	ServiceBroker string `positional-arg-name:"SERVICE_BROKER" required:"true" description:"The service broker name"`
 	Username      string `positional-arg-name:"USERNAME" required:"true" description:"The username"`
-	Password      string `positional-arg-name:"PASSWORD" required:"true" description:"The password"`
-	URL           string `positional-arg-name:"URL" required:"true" description:"The URL of the service broker"`
+	PasswordOrURL string `positional-arg-name:"URL" required:"true" description:"The URL of the service broker"`
+	URL           string `positional-arg-name:"URL" description:"The URL of the service broker"`
 }
 
 type RenameServiceBrokerArgs struct {

command/v7/create_service_broker_command.go

@@ -1,6 +1,9 @@
 package v7
 
 import (
+	"os"
+
+	"code.cloudfoundry.org/cli/command"
 	"code.cloudfoundry.org/cli/command/flag"
 	"code.cloudfoundry.org/cli/resources"
 	"code.cloudfoundry.org/cli/util/configv3"
@@ -9,10 +12,11 @@ import (
 type CreateServiceBrokerCommand struct {
 	BaseCommand
 
-	RequiredArgs    flag.ServiceBrokerArgs `positional-args:"yes"`
+	PositionalArgs  flag.ServiceBrokerArgs `positional-args:"yes"`
 	SpaceScoped     bool                   `long:"space-scoped" description:"Make the broker's service plans only visible within the targeted space"`
-	usage           interface{}            `usage:"CF_NAME create-service-broker SERVICE_BROKER USERNAME PASSWORD URL [--space-scoped]"`
-	relatedCommands interface{}            `related_commands:"enable-service-access, service-brokers, target"`
+	usage           any                    `usage:"CF_NAME create-service-broker SERVICE_BROKER USERNAME PASSWORD URL [--space-scoped]\n   CF_NAME create-service-broker SERVICE_BROKER USERNAME URL [--space-scoped] (omit password to specify interactively or via environment variable)\n\nWARNING:\n   Providing your password as a command line option is highly discouraged\n   Your password may be visible to others and may be recorded in your shell history"`
+	relatedCommands any                    `related_commands:"enable-service-access, service-brokers, target"`
+	envPassword     any                    `environmentName:"CF_BROKER_PASSWORD" environmentDescription:"Password associated with user. Overridden if PASSWORD argument is provided" environmentDefault:"password"`
 }
 
 func (cmd *CreateServiceBrokerCommand) Execute(args []string) error {
@@ -21,6 +25,11 @@ func (cmd *CreateServiceBrokerCommand) Execute(args []string) error {
 		return err
 	}
 
+	brokerName, username, password, url, err := promptUserForBrokerPasswordIfRequired(cmd.PositionalArgs, cmd.UI)
+	if err != nil {
+		return err
+	}
+
 	user, err := cmd.Actor.GetCurrentUser()
 	if err != nil {
 		return err
@@ -31,29 +40,29 @@ func (cmd *CreateServiceBrokerCommand) Execute(args []string) error {
 		space = cmd.Config.TargetedSpace()
 		cmd.UI.DisplayTextWithFlavor(
 			"Creating service broker {{.ServiceBroker}} in org {{.Org}} / space {{.Space}} as {{.Username}}...",
-			map[string]interface{}{
+			map[string]any{
 				"Username":      user.Name,
-				"ServiceBroker": cmd.RequiredArgs.ServiceBroker,
+				"ServiceBroker": brokerName,
 				"Org":           cmd.Config.TargetedOrganizationName(),
 				"Space":         space.Name,
 			},
 		)
 	} else {
 		cmd.UI.DisplayTextWithFlavor(
 			"Creating service broker {{.ServiceBroker}} as {{.Username}}...",
-			map[string]interface{}{
+			map[string]any{
 				"Username":      user.Name,
-				"ServiceBroker": cmd.RequiredArgs.ServiceBroker,
+				"ServiceBroker": brokerName,
 			},
 		)
 	}
 
 	warnings, err := cmd.Actor.CreateServiceBroker(
 		resources.ServiceBroker{
-			Name:      cmd.RequiredArgs.ServiceBroker,
-			Username:  cmd.RequiredArgs.Username,
-			Password:  cmd.RequiredArgs.Password,
-			URL:       cmd.RequiredArgs.URL,
+			Name:      brokerName,
+			Username:  username,
+			Password:  password,
+			URL:       url,
 			SpaceGUID: space.GUID,
 		},
 	)
@@ -65,3 +74,20 @@ func (cmd *CreateServiceBrokerCommand) Execute(args []string) error {
 	cmd.UI.DisplayOK()
 	return nil
 }
+
+func promptUserForBrokerPasswordIfRequired(args flag.ServiceBrokerArgs, ui command.UI) (string, string, string, string, error) {
+	if args.URL != "" {
+		return args.ServiceBroker, args.Username, args.PasswordOrURL, args.URL, nil
+	}
+
+	if password, ok := os.LookupEnv("CF_BROKER_PASSWORD"); ok {
+		return args.ServiceBroker, args.Username, password, args.PasswordOrURL, nil
+	}
+
+	password, err := ui.DisplayPasswordPrompt("Service Broker Password")
+	if err != nil {
+		return "", "", "", "", err
+	}
+
+	return args.ServiceBroker, args.Username, password, args.PasswordOrURL, nil
+}

command/v7/create_service_broker_command_test.go

@@ -2,6 +2,8 @@ package v7_test
 
 import (
 	"errors"
+	"fmt"
+	"os"
 
 	"code.cloudfoundry.org/cli/actor/v7action"
 	"code.cloudfoundry.org/cli/command/commandfakes"
@@ -15,14 +17,22 @@ import (
 )
 
 var _ = Describe("create-service-broker Command", func() {
+	const (
+		binaryName        = "cf-command"
+		user              = "steve"
+		serviceBrokerName = "fake-service-broker-name"
+		username          = "fake-username"
+		password          = "fake-password"
+		url               = "fake-url"
+	)
+
 	var (
 		cmd             *v7.CreateServiceBrokerCommand
 		testUI          *ui.UI
 		fakeConfig      *commandfakes.FakeConfig
 		fakeSharedActor *commandfakes.FakeSharedActor
 		fakeActor       *v7fakes.FakeActor
 		input           *Buffer
-		binaryName      string
 		executeErr      error
 	)
 
@@ -34,7 +44,6 @@ var _ = Describe("create-service-broker Command", func() {
 		fakeActor = new(v7fakes.FakeActor)
 		fakeActor.CreateServiceBrokerReturns(v7action.Warnings{"some default warning"}, nil)
 
-		binaryName = "faceman"
 		fakeConfig.BinaryNameReturns(binaryName)
 
 		cmd = &v7.CreateServiceBrokerCommand{
@@ -45,8 +54,6 @@ var _ = Describe("create-service-broker Command", func() {
 				Actor:       fakeActor,
 			},
 		}
-
-		setPositionalFlags(cmd, "service-broker-name", "username", "password", "https://example.org/super-broker")
 	})
 
 	JustBeforeEach(func() {
@@ -66,6 +73,7 @@ var _ = Describe("create-service-broker Command", func() {
 	When("fetching the current user fails", func() {
 		BeforeEach(func() {
 			fakeActor.GetCurrentUserReturns(configv3.User{}, errors.New("an error occurred"))
+			setPositionalFlags(cmd, serviceBrokerName, username, password, url)
 		})
 
 		It("return an error", func() {
@@ -75,7 +83,8 @@ var _ = Describe("create-service-broker Command", func() {
 
 	When("fetching the current user succeeds", func() {
 		BeforeEach(func() {
-			fakeActor.GetCurrentUserReturns(configv3.User{Name: "steve"}, nil)
+			fakeActor.GetCurrentUserReturns(configv3.User{Name: user}, nil)
+			setPositionalFlags(cmd, serviceBrokerName, username, password, url)
 		})
 
 		It("checks that there is a valid target", func() {
@@ -86,18 +95,18 @@ var _ = Describe("create-service-broker Command", func() {
 		})
 
 		It("displays a message with the username", func() {
-			Expect(testUI.Out).To(Say(`Creating service broker %s as %s\.\.\.`, "service-broker-name", "steve"))
+			Expect(testUI.Out).To(Say(`Creating service broker %s as %s\.\.\.`, serviceBrokerName, user))
 		})
 
 		It("passes the data to the actor layer", func() {
 			Expect(fakeActor.CreateServiceBrokerCallCount()).To(Equal(1))
 
 			model := fakeActor.CreateServiceBrokerArgsForCall(0)
 
-			Expect(model.Name).To(Equal("service-broker-name"))
-			Expect(model.Username).To(Equal("username"))
-			Expect(model.Password).To(Equal("password"))
-			Expect(model.URL).To(Equal("https://example.org/super-broker"))
+			Expect(model.Name).To(Equal(serviceBrokerName))
+			Expect(model.Username).To(Equal(username))
+			Expect(model.Password).To(Equal(password))
+			Expect(model.URL).To(Equal(url))
 			Expect(model.SpaceGUID).To(Equal(""))
 		})
 
@@ -122,13 +131,19 @@ var _ = Describe("create-service-broker Command", func() {
 		})
 
 		When("creating a space scoped broker", func() {
+			const (
+				orgName   = "fake-org-name"
+				spaceName = "fake-space-name"
+				spaceGUID = "fake-space-guid"
+			)
+
 			BeforeEach(func() {
 				cmd.SpaceScoped = true
 				fakeConfig.TargetedSpaceReturns(configv3.Space{
-					Name: "fake-space-name",
-					GUID: "fake-space-guid",
+					Name: spaceName,
+					GUID: spaceGUID,
 				})
-				fakeConfig.TargetedOrganizationNameReturns("fake-org-name")
+				fakeConfig.TargetedOrganizationNameReturns(orgName)
 			})
 
 			It("checks that a space is targeted", func() {
@@ -139,15 +154,75 @@ var _ = Describe("create-service-broker Command", func() {
 			})
 
 			It("displays the space name in the message", func() {
-				Expect(testUI.Out).To(Say(`Creating service broker %s in org %s / space %s as %s\.\.\.`, "service-broker-name", "fake-org-name", "fake-space-name", "steve"))
+				Expect(testUI.Out).To(Say(`Creating service broker %s in org %s / space %s as %s\.\.\.`, serviceBrokerName, orgName, spaceName, user))
 			})
 
 			It("looks up the space guid and passes it to the actor", func() {
 				Expect(fakeActor.CreateServiceBrokerCallCount()).To(Equal(1))
 
 				model := fakeActor.CreateServiceBrokerArgsForCall(0)
-				Expect(model.SpaceGUID).To(Equal("fake-space-guid"))
+				Expect(model.SpaceGUID).To(Equal(spaceGUID))
 			})
 		})
 	})
+
+	When("password is provided as environment variable", func() {
+		const (
+			varName     = "CF_BROKER_PASSWORD"
+			varPassword = "var-password"
+		)
+
+		BeforeEach(func() {
+			setPositionalFlags(cmd, serviceBrokerName, username, url, "")
+			os.Setenv(varName, varPassword)
+		})
+
+		AfterEach(func() {
+			os.Unsetenv(varName)
+		})
+
+		It("passes the data to the actor layer", func() {
+			Expect(fakeActor.CreateServiceBrokerCallCount()).To(Equal(1))
+
+			model := fakeActor.CreateServiceBrokerArgsForCall(0)
+
+			Expect(model.Name).To(Equal(serviceBrokerName))
+			Expect(model.Username).To(Equal(username))
+			Expect(model.Password).To(Equal(varPassword))
+			Expect(model.URL).To(Equal(url))
+			Expect(model.SpaceGUID).To(Equal(""))
+		})
+	})
+
+	When("password is provided via prompt", func() {
+		const promptPassword = "prompt-password"
+
+		BeforeEach(func() {
+			setPositionalFlags(cmd, serviceBrokerName, username, url, "")
+
+			_, err := input.Write([]byte(fmt.Sprintf("%s\n", promptPassword)))
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("prompts the user for credentials", func() {
+			Expect(testUI.Out).To(Say("Service Broker Password: "))
+		})
+
+		It("does not echo the credentials", func() {
+			Expect(testUI.Out).NotTo(Say(promptPassword))
+			Expect(testUI.Err).NotTo(Say(promptPassword))
+		})
+
+		It("passes the data to the actor layer", func() {
+			Expect(fakeActor.CreateServiceBrokerCallCount()).To(Equal(1))
+
+			model := fakeActor.CreateServiceBrokerArgsForCall(0)
+
+			Expect(model.Name).To(Equal(serviceBrokerName))
+			Expect(model.Username).To(Equal(username))
+			Expect(model.Password).To(Equal(promptPassword))
+			Expect(model.URL).To(Equal(url))
+			Expect(model.SpaceGUID).To(Equal(""))
+		})
+	})
 })

command/v7/create_user_provided_service_command.go

@@ -24,7 +24,7 @@ func (cmd CreateUserProvidedServiceCommand) Execute(args []string) error {
 		return err
 	}
 
-	if err := PromptUserForCredentialsIfRequired(&cmd.Credentials, cmd.UI); err != nil {
+	if err := promptUserForCredentialsIfRequired(&cmd.Credentials, cmd.UI); err != nil {
 		return err
 	}
 
@@ -69,7 +69,7 @@ func (cmd CreateUserProvidedServiceCommand) displayMessage() error {
 	return nil
 }
 
-func PromptUserForCredentialsIfRequired(flag *flag.CredentialsOrJSON, ui command.UI) error {
+func promptUserForCredentialsIfRequired(flag *flag.CredentialsOrJSON, ui command.UI) error {
 	if len(flag.UserPromptCredentials) > 0 {
 		flag.Value = make(map[string]interface{})
 

command/v7/update_service_broker_command.go

@@ -8,17 +8,23 @@ import (
 type UpdateServiceBrokerCommand struct {
 	BaseCommand
 
-	RequiredArgs    flag.ServiceBrokerArgs `positional-args:"yes"`
-	usage           interface{}            `usage:"CF_NAME update-service-broker SERVICE_BROKER USERNAME PASSWORD URL"`
-	relatedCommands interface{}            `related_commands:"rename-service-broker, service-brokers"`
+	PositionalArgs  flag.ServiceBrokerArgs `positional-args:"yes"`
+	usage           any                    `usage:"CF_NAME update-service-broker SERVICE_BROKER USERNAME PASSWORD URL\n   CF_NAME update-service-broker SERVICE_BROKER USERNAME URL (omit password to specify interactively or via environment variable)\n\nWARNING:\n   Providing your password as a command line option is highly discouraged\n   Your password may be visible to others and may be recorded in your shell history"`
+	relatedCommands any                    `related_commands:"rename-service-broker, service-brokers"`
+	envPassword     any                    `environmentName:"CF_BROKER_PASSWORD" environmentDescription:"Password associated with user. Overridden if PASSWORD argument is provided" environmentDefault:"password"`
 }
 
 func (cmd UpdateServiceBrokerCommand) Execute(args []string) error {
 	if err := cmd.SharedActor.CheckTarget(false, false); err != nil {
 		return err
 	}
 
-	serviceBroker, warnings, err := cmd.Actor.GetServiceBrokerByName(cmd.RequiredArgs.ServiceBroker)
+	brokerName, username, password, url, err := promptUserForBrokerPasswordIfRequired(cmd.PositionalArgs, cmd.UI)
+	if err != nil {
+		return err
+	}
+
+	serviceBroker, warnings, err := cmd.Actor.GetServiceBrokerByName(brokerName)
 	cmd.UI.DisplayWarnings(warnings)
 	if err != nil {
 		return err
@@ -31,18 +37,18 @@ func (cmd UpdateServiceBrokerCommand) Execute(args []string) error {
 
 	cmd.UI.DisplayTextWithFlavor(
 		"Updating service broker {{.ServiceBroker}} as {{.Username}}...",
-		map[string]interface{}{
+		map[string]any{
 			"Username":      user.Name,
-			"ServiceBroker": cmd.RequiredArgs.ServiceBroker,
+			"ServiceBroker": brokerName,
 		},
 	)
 
 	warnings, err = cmd.Actor.UpdateServiceBroker(
 		serviceBroker.GUID,
 		resources.ServiceBroker{
-			Username: cmd.RequiredArgs.Username,
-			Password: cmd.RequiredArgs.Password,
-			URL:      cmd.RequiredArgs.URL,
+			Username: username,
+			Password: password,
+			URL:      url,
 		},
 	)
 	cmd.UI.DisplayWarnings(warnings)