From 779fe37a1c9f1f5ce0bc9d574021e1b292f4b0a8 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 24 Jun 2020 14:47:59 -0400 Subject: [PATCH] command/login: Require "yes" to confirm This is for consistency with other commands which use prompts, all of which require "yes" rather than "y" to confirm. We also migrate the login command to use UIInput, which now supports securely asking for passwords or secrets via the speakeasy library. --- command/login.go | 34 +++++++++++++------ command/login_test.go | 77 ++++++++++++++++++++++++++++++++----------- command/ui_input.go | 12 +++++-- go.mod | 2 ++ go.sum | 10 ------ terraform/ui_input.go | 4 +++ vendor/modules.txt | 2 ++ 7 files changed, 99 insertions(+), 42 deletions(-) diff --git a/command/login.go b/command/login.go index 586e9414c..3a60c6d93 100644 --- a/command/login.go +++ b/command/login.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/command/cliconfig" "github.com/hashicorp/terraform/httpclient" + "github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/tfdiags" uuid "github.com/hashicorp/go-uuid" @@ -453,12 +454,19 @@ func (c *LoginCommand) interactiveGetTokenByPassword(hostname svchost.Hostname, c.Ui.Output("\n---------------------------------------------------------------------------------\n") c.Ui.Output("Terraform must temporarily use your password to request an API token.\nThis password will NOT be saved locally.\n") - username, err := c.Ui.Ask(fmt.Sprintf("Username for %s:", hostname.ForDisplay())) + username, err := c.UIInput().Input(context.Background(), &terraform.InputOpts{ + Id: "username", + Query: fmt.Sprintf("Username for %s:", hostname.ForDisplay()), + }) if err != nil { diags = diags.Append(fmt.Errorf("Failed to request username: %s", err)) return nil, diags } - password, err := c.Ui.AskSecret(fmt.Sprintf("Password for %s:", username)) + password, err := c.UIInput().Input(context.Background(), &terraform.InputOpts{ + Id: "password", + Query: fmt.Sprintf("Password for %s:", hostname.ForDisplay()), + Secret: true, + }) if err != nil { diags = diags.Append(fmt.Errorf("Failed to request password: %s", err)) return nil, diags @@ -494,6 +502,8 @@ func (c *LoginCommand) interactiveGetTokenByUI(hostname svchost.Hostname, credsC return "", diags } + c.Ui.Output("\n---------------------------------------------------------------------------------\n") + tokensURL := url.URL{ Scheme: "https", Host: service.Hostname(), @@ -508,6 +518,7 @@ func (c *LoginCommand) interactiveGetTokenByUI(hostname svchost.Hostname, credsC c.Ui.Output(fmt.Sprintf("Terraform must now open a web browser to the tokens page for %s.\n", hostname.ForDisplay())) c.Ui.Output(fmt.Sprintf("If a browser does not open this automatically, open the following URL to proceed:\n %s\n", tokensURL.String())) } else { + log.Printf("[DEBUG] error opening web browser: %s", err) // Assume we're on a platform where opening a browser isn't possible. launchBrowserManually = true } @@ -533,7 +544,11 @@ func (c *LoginCommand) interactiveGetTokenByUI(hostname svchost.Hostname, credsC } } - token, err := c.Ui.AskSecret(fmt.Sprintf(c.Colorize().Color("Token for [bold]%s[reset]:"), hostname.ForDisplay())) + token, err := c.UIInput().Input(context.Background(), &terraform.InputOpts{ + Id: "token", + Query: fmt.Sprintf("Token for %s:", hostname.ForDisplay()), + Secret: true, + }) if err != nil { diags := diags.Append(fmt.Errorf("Failed to retrieve token: %s", err)) return "", diags @@ -590,7 +605,11 @@ func (c *LoginCommand) interactiveContextConsent(hostname svchost.Hostname, gran } } - v, err := c.Ui.Ask("Do you want to proceed? (y/n)") + v, err := c.UIInput().Input(context.Background(), &terraform.InputOpts{ + Id: "approve", + Query: "Do you want to proceed?", + Description: `Only 'yes' will be accepted to confirm.`, + }) if err != nil { // Should not happen because this command checks that input is enabled // before we get to this point. @@ -598,12 +617,7 @@ func (c *LoginCommand) interactiveContextConsent(hostname svchost.Hostname, gran return false, diags } - switch strings.ToLower(v) { - case "y", "yes": - return true, diags - default: - return false, diags - } + return strings.ToLower(v) == "yes", diags } func (c *LoginCommand) listenerForCallback(minPort, maxPort uint16) (net.Listener, string, error) { diff --git a/command/login_test.go b/command/login_test.go index b03e49c82..9b2a8a546 100644 --- a/command/login_test.go +++ b/command/login_test.go @@ -1,7 +1,6 @@ package command import ( - "bytes" "context" "io/ioutil" "net/http/httptest" @@ -34,7 +33,7 @@ func TestLogin(t *testing.T) { ts := httptest.NewServer(tfeserver.Handler) defer ts.Close() - loginTestCase := func(test func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string))) func(t *testing.T) { + loginTestCase := func(test func(t *testing.T, c *LoginCommand, ui *cli.MockUi)) func(t *testing.T) { return func(t *testing.T) { t.Helper() workDir, err := ioutil.TempDir("", "terraform-test-command-login") @@ -48,15 +47,15 @@ func TestLogin(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - ui := cli.NewMockUi() + // Do not use the NewMockUi initializer here, as we want to delay + // the call to init until after setting up the input mocks + ui := new(cli.MockUi) + browserLauncher := webbrowser.NewMockLauncher(ctx) creds := cliconfig.EmptyCredentialsSourceForTests(filepath.Join(workDir, "credentials.tfrc.json")) svcs := disco.NewWithCredentialsSource(creds) svcs.SetUserAgent(httpclient.TerraformUserAgent(version.String())) - inputBuf := &bytes.Buffer{} - ui.InputReader = inputBuf - svcs.ForceHostServices(svchost.Hostname("app.terraform.io"), map[string]interface{}{ "login.v1": map[string]interface{}{ // On app.terraform.io we use password-based authorization. @@ -96,16 +95,16 @@ func TestLogin(t *testing.T) { }, } - test(t, c, ui, func(data string) { - t.Helper() - inputBuf.WriteString(data) - }) + test(t, c, ui) } } - t.Run("defaulting to app.terraform.io with password flow", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string)) { - // Enter "yes" at the consent prompt, then a username and then a password. - inp("yes\nfoo\nbar\n") + t.Run("defaulting to app.terraform.io with password flow", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { + defer testInputMap(t, map[string]string{ + "approve": "yes", + "username": "foo", + "password": "bar", + })() status := c.Run(nil) if status != 0 { t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) @@ -121,9 +120,11 @@ func TestLogin(t *testing.T) { } })) - t.Run("example.com with authorization code flow", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string)) { + t.Run("example.com with authorization code flow", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { // Enter "yes" at the consent prompt. - inp("yes\n") + defer testInputMap(t, map[string]string{ + "approve": "yes", + })() status := c.Run([]string{"example.com"}) if status != 0 { t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) @@ -139,10 +140,13 @@ func TestLogin(t *testing.T) { } })) - t.Run("TFE host without login support", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string)) { + t.Run("TFE host without login support", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { // Enter "yes" at the consent prompt, then paste a token with some // accidental whitespace. - inp("yes\n good-token \n") + defer testInputMap(t, map[string]string{ + "approve": "yes", + "token": " good-token ", + })() status := c.Run([]string{"tfe.acme.com"}) if status != 0 { t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) @@ -158,9 +162,12 @@ func TestLogin(t *testing.T) { } })) - t.Run("TFE host without login support, incorrectly pasted token", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string)) { + t.Run("TFE host without login support, incorrectly pasted token", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { // Enter "yes" at the consent prompt, then paste an invalid token. - inp("yes\ngood-tok\n") + defer testInputMap(t, map[string]string{ + "approve": "yes", + "token": "good-tok", + })() status := c.Run([]string{"tfe.acme.com"}) if status != 1 { t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) @@ -176,7 +183,7 @@ func TestLogin(t *testing.T) { } })) - t.Run("host without login or TFE API support", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi, inp func(string)) { + t.Run("host without login or TFE API support", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { status := c.Run([]string{"unsupported.example.net"}) if status == 0 { t.Fatalf("successful exit; want error") @@ -186,4 +193,34 @@ func TestLogin(t *testing.T) { t.Fatalf("missing expected error message\nwant: %s\nfull output:\n%s", want, got) } })) + + t.Run("answering no cancels", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { + // Enter "no" at the consent prompt + defer testInputMap(t, map[string]string{ + "approve": "no", + })() + status := c.Run(nil) + if status != 1 { + t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) + } + + if got, want := ui.ErrorWriter.String(), "Login cancelled"; !strings.Contains(got, want) { + t.Fatalf("missing expected error message\nwant: %s\nfull output:\n%s", want, got) + } + })) + + t.Run("answering y cancels", loginTestCase(func(t *testing.T, c *LoginCommand, ui *cli.MockUi) { + // Enter "y" at the consent prompt + defer testInputMap(t, map[string]string{ + "approve": "y", + })() + status := c.Run(nil) + if status != 1 { + t.Fatalf("unexpected error code %d\nstderr:\n%s", status, ui.ErrorWriter.String()) + } + + if got, want := ui.ErrorWriter.String(), "Login cancelled"; !strings.Contains(got, want) { + t.Fatalf("missing expected error message\nwant: %s\nfull output:\n%s", want, got) + } + })) } diff --git a/command/ui_input.go b/command/ui_input.go index 5a09bb164..962fa76de 100644 --- a/command/ui_input.go +++ b/command/ui_input.go @@ -15,7 +15,9 @@ import ( "sync/atomic" "unicode" + "github.com/bgentry/speakeasy" "github.com/hashicorp/terraform/terraform" + "github.com/mattn/go-isatty" "github.com/mitchellh/colorstring" ) @@ -128,8 +130,14 @@ func (i *UIInput) Input(ctx context.Context, opts *terraform.InputOpts) (string, } defer atomic.CompareAndSwapInt32(&i.listening, 1, 0) - buf := bufio.NewReader(r) - line, err := buf.ReadString('\n') + var line string + var err error + if opts.Secret && isatty.IsTerminal(os.Stdin.Fd()) { + line, err = speakeasy.Ask("") + } else { + buf := bufio.NewReader(r) + line, err = buf.ReadString('\n') + } if err != nil { log.Printf("[ERR] UIInput scan err: %s", err) } diff --git a/go.mod b/go.mod index 6bcd5f8ab..5c37c0b0b 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/armon/go-radix v1.0.0 // indirect github.com/aws/aws-sdk-go v1.31.9 github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f // indirect + github.com/bgentry/speakeasy v0.1.0 github.com/blang/semver v3.5.1+incompatible github.com/bmatcuk/doublestar v1.1.5 github.com/boltdb/bolt v1.3.1 // indirect @@ -90,6 +91,7 @@ require ( github.com/masterzen/simplexml v0.0.0-20190410153822-31eea3082786 // indirect github.com/masterzen/winrm v0.0.0-20200615185753-c42b5136ff88 github.com/mattn/go-colorable v0.1.1 + github.com/mattn/go-isatty v0.0.5 github.com/mattn/go-shellwords v1.0.4 github.com/miekg/dns v1.0.8 // indirect github.com/mitchellh/cli v1.0.0 diff --git a/go.sum b/go.sum index d4acafc08..4185216e1 100644 --- a/go.sum +++ b/go.sum @@ -363,8 +363,6 @@ github.com/masterzen/simplexml v0.0.0-20160608183007-4572e39b1ab9 h1:SmVbOZFWAly github.com/masterzen/simplexml v0.0.0-20160608183007-4572e39b1ab9/go.mod h1:kCEbxUJlNDEBNbdQMkPSp6yaKcRXVI6f4ddk8Riv4bc= github.com/masterzen/simplexml v0.0.0-20190410153822-31eea3082786 h1:2ZKn+w/BJeL43sCxI2jhPLRv73oVVOjEKZjKkflyqxg= github.com/masterzen/simplexml v0.0.0-20190410153822-31eea3082786/go.mod h1:kCEbxUJlNDEBNbdQMkPSp6yaKcRXVI6f4ddk8Riv4bc= -github.com/masterzen/winrm v0.0.0-20190223112901-5e5c9a7fe54b h1:/1RFh2SLCJ+tEnT73+Fh5R2AO89sQqs8ba7o+hx1G0Y= -github.com/masterzen/winrm v0.0.0-20190223112901-5e5c9a7fe54b/go.mod h1:wr1VqkwW0AB5JS0QLy5GpVMS9E3VtRoSYXUYyVk46KY= github.com/masterzen/winrm v0.0.0-20200615185753-c42b5136ff88 h1:cxuVcCvCLD9yYDbRCWw0jSgh1oT6P6mv3aJDKK5o7X4= github.com/masterzen/winrm v0.0.0-20200615185753-c42b5136ff88/go.mod h1:a2HXwefeat3evJHxFXSayvRHpYEPJYtErl4uIzfaUqY= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= @@ -436,8 +434,6 @@ github.com/onsi/ginkgo v1.8.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v0.0.0-20190113212917-5533ce8a0da3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -github.com/packer-community/winrmcp v0.0.0-20180102160824-81144009af58 h1:m3CEgv3ah1Rhy82L+c0QG/U3VyY1UsvsIdkh0/rU97Y= -github.com/packer-community/winrmcp v0.0.0-20180102160824-81144009af58/go.mod h1:f6Izs6JvFTdnRbziASagjZ2vmf55NSIkC/weStxCHqk= github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db h1:9uViuKtx1jrlXLBW/pMnhOfzn3iSEdLase/But/IZRU= github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db/go.mod h1:f6Izs6JvFTdnRbziASagjZ2vmf55NSIkC/weStxCHqk= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c h1:Lgl0gzECD8GnQ5QCWA8o6BtfL6mDH5rQgM4/fX3avOs= @@ -549,8 +545,6 @@ golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191202143827-86a70503ff7e/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20191206172530-e9b2fee46413/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 h1:cg5LA/zNPRzIXIWSCxQW10Rvpy94aQh3LT/ShoCpkHw= -golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 h1:vEg9joUBmeBcK9iSJftGNf3coIG4HqZElCPehJsfAYM= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -587,8 +581,6 @@ golang.org/x/net v0.0.0-20191009170851-d66e71096ffb/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200202094626-16171245cfb2 h1:CCH4IOTTfewWjGOlSp+zGcjutRKlBEZQ6wTn8ozI/nI= golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20200301022130-244492dfa37a h1:GuSPYbZzB5/dcLNCwLQLsg3obCJtX9IJhpXkvY7kzk0= -golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200602114024-627f9648deb9 h1:pNX+40auqi2JqRfOP1akLGtYcn15TUbkhwuCO3foqqM= golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -623,8 +615,6 @@ golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191128015809-6d18c012aee9 h1:ZBzSG/7F4eNKz2L3GE9o300RX0Az1Bw5HF7PDraD+qU= golang.org/x/sys v0.0.0-20191128015809-6d18c012aee9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 h1:uYVVQ9WP/Ds2ROhcaGPeIdVq0RIXVLwsHlnvJ+cT1So= -golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/terraform/ui_input.go b/terraform/ui_input.go index f6790d9e5..688bcf71e 100644 --- a/terraform/ui_input.go +++ b/terraform/ui_input.go @@ -25,4 +25,8 @@ type InputOpts struct { // Default will be the value returned if no data is entered. Default string + + // Secret should be true if we are asking for sensitive input. + // If attached to a TTY, Terraform will disable echo. + Secret bool } diff --git a/vendor/modules.txt b/vendor/modules.txt index cb3baadd7..de9d548ce 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -168,6 +168,7 @@ github.com/aws/aws-sdk-go/service/sts/stsiface # github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d github.com/bgentry/go-netrc/netrc # github.com/bgentry/speakeasy v0.1.0 +## explicit github.com/bgentry/speakeasy # github.com/blang/semver v3.5.1+incompatible ## explicit @@ -476,6 +477,7 @@ github.com/masterzen/winrm/soap ## explicit github.com/mattn/go-colorable # github.com/mattn/go-isatty v0.0.5 +## explicit github.com/mattn/go-isatty # github.com/mattn/go-shellwords v1.0.4 ## explicit