Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hide password for create-service-broker (v8) #2410

Merged
merged 1 commit into from
Apr 24, 2023
Merged

fix: hide password for create-service-broker (v8) #2410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions command/flag/arguments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
48 changes: 37 additions & 11 deletions command/v7/create_service_broker_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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,
},
)
Expand All @@ -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
}
105 changes: 90 additions & 15 deletions command/v7/create_service_broker_command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package v7_test

import (
"errors"
"fmt"
"os"

"code.cloudfoundry.org/cli/actor/v7action"
"code.cloudfoundry.org/cli/command/commandfakes"
Expand All @@ -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
)

Expand All @@ -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{
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -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(""))
})

Expand All @@ -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() {
Expand All @@ -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(""))
})
})
})
4 changes: 2 additions & 2 deletions command/v7/create_user_provided_service_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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{})

Expand Down
24 changes: 15 additions & 9 deletions command/v7/update_service_broker_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Loading