From 31aa0f4de3214b4efd2b16c07f739bbb2a55e583 Mon Sep 17 00:00:00 2001 From: Andrew Starr-Bochicchio Date: Fri, 6 Jan 2023 12:11:08 -0500 Subject: [PATCH] Modernize auth init prompt. (#1337) --- commands/auth.go | 53 ++++++++----- commands/auth_test.go | 57 ++++++++++++++ go.mod | 2 +- integration/auth_test.go | 46 ++++++++--- .../x/crypto/ssh/terminal/terminal.go | 76 ------------------- vendor/modules.txt | 1 - 6 files changed, 128 insertions(+), 107 deletions(-) delete mode 100644 vendor/golang.org/x/crypto/ssh/terminal/terminal.go diff --git a/commands/auth.go b/commands/auth.go index fdf6cb516..39a969188 100644 --- a/commands/auth.go +++ b/commands/auth.go @@ -21,20 +21,23 @@ import ( "path/filepath" "sort" "strings" - "syscall" "github.com/digitalocean/doctl" - - "golang.org/x/crypto/ssh/terminal" + "github.com/digitalocean/doctl/commands/charm/input" + "github.com/digitalocean/doctl/commands/charm/template" "github.com/spf13/cobra" "github.com/spf13/viper" + "golang.org/x/term" yaml "gopkg.in/yaml.v2" ) const ( // TokenValidationServer is the default server used to validate an OAuth token TokenValidationServer = "https://cloud.digitalocean.com" + + legacyTokenLength = 64 + v1TokenLength = 71 ) // ErrUnknownTerminal signifies an unknown terminal. It is returned when doit @@ -47,18 +50,30 @@ var ( // retrieveUserTokenFromCommandLine is a function that can retrieve a token. By default, // it will prompt the user. In test, you can replace this with code that returns the appropriate response. func retrieveUserTokenFromCommandLine() (string, error) { - if !terminal.IsTerminal(int(os.Stdout.Fd())) { + if !term.IsTerminal(int(os.Stdout.Fd())) { return "", ErrUnknownTerminal } - fmt.Print("Please authenticate doctl for use with your DigitalOcean account. You can generate a token in the control panel at https://cloud.digitalocean.com/account/api/tokens\n\n") - fmt.Print("Enter your access token: ") - passwdBytes, err := terminal.ReadPassword(int(syscall.Stdin)) - if err != nil { - return "", err - } + template.Print( + `Please authenticate doctl for use with your DigitalOcean account. You can generate a token in the control panel at {{underline "https://cloud.digitalocean.com/account/api/tokens"}}{{nl}}{{nl}}`, + nil, + ) + + prompt := input.New("Enter your access token: ", + input.WithHidden(), + input.WithRequired(), + input.WithValidator(tokenInputValidator), + ) - return string(passwdBytes), nil + return prompt.Prompt() +} + +// tokenInputValidator is used to do basic validation of a token while it is being typed. +func tokenInputValidator(input string) error { + if len(input) == legacyTokenLength || (strings.HasPrefix(input, "do") && len(input) == v1TokenLength) { + return nil + } + return errors.New("") } // UnknownSchemeError signifies an unknown HTTP scheme. @@ -131,6 +146,10 @@ To create new contexts, see the help for `+"`"+`doctl auth init`+"`"+`.`, Writer func RunAuthInit(retrieveUserTokenFunc func() (string, error)) func(c *CmdConfig) error { return func(c *CmdConfig) error { token := c.getContextAccessToken() + context := Context + if context == "" { + context = viper.GetString("context") + } if token == "" { in, err := retrieveUserTokenFunc() @@ -139,14 +158,12 @@ func RunAuthInit(retrieveUserTokenFunc func() (string, error)) func(c *CmdConfig } token = strings.TrimSpace(in) } else { - fmt.Fprintf(c.Out, "Using token [%v]", token) - fmt.Fprintln(c.Out) + template.Render(c.Out, `Using token for context {{highlight .}}{{nl}}`, context) } c.setContextAccessToken(token) - fmt.Fprintln(c.Out) - fmt.Fprint(c.Out, "Validating token... ") + template.Render(c.Out, `{{nl}}Validating token... `, nil) // need to initial the godo client since we've changed the configuration. if err := c.initServices(c); err != nil { @@ -159,13 +176,11 @@ func RunAuthInit(retrieveUserTokenFunc func() (string, error)) func(c *CmdConfig } if _, err := c.OAuth().TokenInfo(server); err != nil { - fmt.Fprintln(c.Out, "invalid token") - fmt.Fprintln(c.Out) + template.Render(c.Out, `{{error crossmark}}{{nl}}{{nl}}`, nil) return fmt.Errorf("Unable to use supplied token to access API: %s", err) } - fmt.Fprintln(c.Out, "OK") - fmt.Fprintln(c.Out) + template.Render(c.Out, `{{success checkmark}}{{nl}}{{nl}}`, nil) return writeConfig() } diff --git a/commands/auth_test.go b/commands/auth_test.go index b8745401c..9132f7ec8 100644 --- a/commands/auth_test.go +++ b/commands/auth_test.go @@ -189,6 +189,63 @@ func Test_displayAuthContexts(t *testing.T) { } } +func TestTokenInputValidator(t *testing.T) { + tests := []struct { + name string + token string + valid bool + }{ + { + name: "valid legacy token", + token: "53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84", + valid: true, + }, + { + name: "valid v1 pat", + token: "dop_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84", + valid: true, + }, + { + name: "valid v1 oauth", + token: "doo_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84", + valid: true, + }, + { + name: "too short legacy token", + token: "53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adca", + }, + { + name: "too long legacy token", + token: "53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84a2d45", + }, + { + name: "too short v1 pat", + token: "dop_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae", + }, + { + name: "too short v1 oauth", + token: "doo_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adc84", + }, + { + name: "too long v1 pat", + token: "dop_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84sdsd", + }, + { + name: "too long v1 oauth", + token: "doo_v1_53918d3cd735062ca6ea791427900af10cf595f18dc6016c1cb0c3a11adcae84sd", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.valid { + assert.NoError(t, tokenInputValidator(tt.token)) + } else { + assert.Error(t, tokenInputValidator(tt.name)) + } + }) + } +} + type testConfig map[string]interface{} type nopWriteCloser struct { diff --git a/go.mod b/go.mod index ec575e88c..640942d34 100644 --- a/go.mod +++ b/go.mod @@ -57,6 +57,7 @@ require ( github.com/muesli/reflow v0.3.0 github.com/muesli/termenv v0.12.0 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 + golang.org/x/term v0.3.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -116,7 +117,6 @@ require ( github.com/subosito/gotenv v1.2.0 // indirect go.opencensus.io v0.23.0 // indirect golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect - golang.org/x/term v0.3.0 // indirect golang.org/x/text v0.5.0 // indirect golang.org/x/time v0.3.0 // indirect golang.org/x/tools v0.1.12 // indirect diff --git a/integration/auth_test.go b/integration/auth_test.go index e3bcb9a2a..de22ebad0 100644 --- a/integration/auth_test.go +++ b/integration/auth_test.go @@ -19,6 +19,7 @@ import ( "github.com/creack/pty" "github.com/sclevine/spec" "github.com/stretchr/testify/require" + "golang.org/x/term" ) var _ = suite("auth/init", func(t *testing.T, when spec.G, it spec.S) { @@ -37,7 +38,8 @@ var _ = suite("auth/init", func(t *testing.T, when spec.G, it spec.S) { case "/v1/oauth/token/info": auth := req.Header.Get("Authorization") - if auth == "Bearer first-token" || auth == "Bearer second-token" || auth == "Bearer some-magic-token" { + if auth == "Bearer first-token" || auth == "Bearer second-token" || + auth == "Bearer some-magic-token" || auth == "Bearer some-magic-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1" { w.Write([]byte(`{"resource_owner_id":123}`)) return } @@ -79,22 +81,30 @@ var _ = suite("auth/init", func(t *testing.T, when spec.G, it spec.S) { ptmx, err := pty.Start(cmd) expect.NoError(err) + // Set the terminal to raw mode so that we can send the carriage return + fd := int(ptmx.Fd()) + oldState, err := term.MakeRaw(fd) + if err != nil { + panic(err) + } + defer func() { _ = term.Restore(fd, oldState) }() + go func() { - ptmx.Write([]byte("some-magic-token\n")) + ptmx.Write([]byte("some-magic-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1\r")) }() buf := bytes.NewBuffer([]byte{}) count, _ := io.Copy(buf, ptmx) // yes, ignore error intentionally expect.NotZero(count) - ptmx.Close() - expect.Contains(buf.String(), "Validating token... OK") + expect.Contains(buf.String(), "Validating token...") + expect.Contains(buf.String(), "✔") fileBytes, err := ioutil.ReadFile(testConfig) expect.NoError(err) - expect.Contains(string(fileBytes), "access-token: some-magic-token") + expect.Contains(string(fileBytes), "access-token: some-magic-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1") }) }) @@ -161,9 +171,16 @@ context: default ptmx, err := pty.Start(cmd) expect.NoError(err) + // Set the terminal to raw mode so that we can send the carriage return + fd := int(ptmx.Fd()) + oldState, err := term.MakeRaw(fd) + if err != nil { + panic(err) + } + defer func() { _ = term.Restore(fd, oldState) }() go func() { - ptmx.Write([]byte("some-magic-token\n")) + ptmx.Write([]byte("some-magic-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1\r")) }() buf := bytes.NewBuffer([]byte{}) @@ -172,7 +189,8 @@ context: default expect.NotZero(count) ptmx.Close() - expect.Contains(buf.String(), "Validating token... OK") + expect.Contains(buf.String(), "Validating token...") + expect.Contains(buf.String(), "✔") location, err := getDefaultConfigLocation() expect.NoError(err) @@ -180,7 +198,7 @@ context: default fileBytes, err := ioutil.ReadFile(location) expect.NoError(err) - expect.Contains(string(fileBytes), "access-token: some-magic-token") + expect.Contains(string(fileBytes), "access-token: some-magic-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1") err = os.Remove(location) expect.NoError(err) @@ -203,9 +221,16 @@ context: default ptmx, err := pty.Start(cmd) expect.NoError(err) + // Set the terminal to raw mode so that we can send the carriage return + fd := int(ptmx.Fd()) + oldState, err := term.MakeRaw(fd) + if err != nil { + panic(err) + } + defer func() { _ = term.Restore(fd, oldState) }() go func() { - ptmx.Write([]byte("some-bad-token\n")) + ptmx.Write([]byte("some-bad-token-that-is-64-characters-long-1a1a1a1a1a11a1a1a1a1a1\r")) }() buf := bytes.NewBuffer([]byte{}) @@ -214,7 +239,8 @@ context: default expect.NotZero(count) ptmx.Close() - expect.Contains(buf.String(), "Validating token... invalid token") + expect.Contains(buf.String(), "Validating token...") + expect.Contains(buf.String(), "✘") expect.Contains(buf.String(), fmt.Sprintf("Unable to use supplied token to access API: GET %s/v1/oauth/token/info: 401", server.URL)) }) }) diff --git a/vendor/golang.org/x/crypto/ssh/terminal/terminal.go b/vendor/golang.org/x/crypto/ssh/terminal/terminal.go deleted file mode 100644 index a4d1919a9..000000000 --- a/vendor/golang.org/x/crypto/ssh/terminal/terminal.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2011 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Package terminal provides support functions for dealing with terminals, as -// commonly found on UNIX systems. -// -// Deprecated: this package moved to golang.org/x/term. -package terminal - -import ( - "io" - - "golang.org/x/term" -) - -// EscapeCodes contains escape sequences that can be written to the terminal in -// order to achieve different styles of text. -type EscapeCodes = term.EscapeCodes - -// Terminal contains the state for running a VT100 terminal that is capable of -// reading lines of input. -type Terminal = term.Terminal - -// NewTerminal runs a VT100 terminal on the given ReadWriter. If the ReadWriter is -// a local terminal, that terminal must first have been put into raw mode. -// prompt is a string that is written at the start of each input line (i.e. -// "> "). -func NewTerminal(c io.ReadWriter, prompt string) *Terminal { - return term.NewTerminal(c, prompt) -} - -// ErrPasteIndicator may be returned from ReadLine as the error, in addition -// to valid line data. It indicates that bracketed paste mode is enabled and -// that the returned line consists only of pasted data. Programs may wish to -// interpret pasted data more literally than typed data. -var ErrPasteIndicator = term.ErrPasteIndicator - -// State contains the state of a terminal. -type State = term.State - -// IsTerminal returns whether the given file descriptor is a terminal. -func IsTerminal(fd int) bool { - return term.IsTerminal(fd) -} - -// ReadPassword reads a line of input from a terminal without local echo. This -// is commonly used for inputting passwords and other sensitive data. The slice -// returned does not include the \n. -func ReadPassword(fd int) ([]byte, error) { - return term.ReadPassword(fd) -} - -// MakeRaw puts the terminal connected to the given file descriptor into raw -// mode and returns the previous state of the terminal so that it can be -// restored. -func MakeRaw(fd int) (*State, error) { - return term.MakeRaw(fd) -} - -// Restore restores the terminal connected to the given file descriptor to a -// previous state. -func Restore(fd int, oldState *State) error { - return term.Restore(fd, oldState) -} - -// GetState returns the current state of a terminal which may be useful to -// restore the terminal after a signal. -func GetState(fd int) (*State, error) { - return term.GetState(fd) -} - -// GetSize returns the dimensions of the given terminal. -func GetSize(fd int) (width, height int, err error) { - return term.GetSize(fd) -} diff --git a/vendor/modules.txt b/vendor/modules.txt index 40fe1560c..728e65d71 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -423,7 +423,6 @@ golang.org/x/crypto/internal/poly1305 golang.org/x/crypto/internal/subtle golang.org/x/crypto/ssh golang.org/x/crypto/ssh/internal/bcrypt_pbkdf -golang.org/x/crypto/ssh/terminal # golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 ## explicit; go 1.17 golang.org/x/mod/internal/lazyregexp