From c1cb440531602b5984c00ffdcd34334551d57b07 Mon Sep 17 00:00:00 2001 From: Aidan Oldershaw Date: Tue, 30 Jun 2020 22:41:23 -0400 Subject: [PATCH] fly: behaviour: accept arbitrary length tokens, take 3 I'm surprised how difficult it's proven to be to just read input from the terminal without hitting buffer limits. The unit tests on our Windows worker highlighted some undesirable behaviour with the peterh/liner library. It didn't seem to successfully detect the number of columns (at least on Windows), which caused: a) The password prompt to fail b) The regular prompt, well, not printing the prompt c) Not using raw mode, which defeats the whole purpose of this fix! I'm not sure exactly why the columns were detected to be 0, and why the library even cares about the number of columns for something as simple as our use case. peterh/liner does a lot of cool stuff, but we were using practically none of it - so why not go with a simpler solution that we control! In place of peterh/liner, just use a raw mode terminal + our own line reading logic. Unfortunately, since we're using raw mode, new lines don't also come with carriage returns, so I needed to manually add them throughout the print statements. Signed-off-by: Aidan Oldershaw --- fly/commands/login.go | 66 ++++++++++++++++++++++--------------------- fly/pty/readline.go | 43 ++++++++++++++++++++++++++++ go.mod | 1 - go.sum | 4 --- 4 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 fly/pty/readline.go diff --git a/fly/commands/login.go b/fly/commands/login.go index 5c1bfe588..ee3d84165 100644 --- a/fly/commands/login.go +++ b/fly/commands/login.go @@ -9,15 +9,17 @@ import ( "io/ioutil" "net" "net/http" + "os" "strings" "github.com/concourse/concourse/atc" + "github.com/concourse/concourse/fly/pty" "github.com/concourse/concourse/fly/rc" "github.com/concourse/concourse/go-concourse/concourse" semisemanticversion "github.com/cppforlife/go-semi-semantic/version" - "github.com/peterh/liner" "github.com/skratchdot/open-golang/open" "github.com/vito/go-interact/interact" + "golang.org/x/crypto/ssh/terminal" "golang.org/x/oauth2" ) @@ -113,21 +115,32 @@ func (command *LoginCommand) Execute(args []string) error { return err } - lineReader := liner.NewLiner() - defer lineReader.Close() - lineReader.SetCtrlCAborts(true) + if pty.IsTerminal() && !command.BrowserOnly { + state, err := terminal.MakeRaw(int(os.Stdin.Fd())) + if err != nil { + return err + } + defer func() { + terminal.Restore(int(os.Stdin.Fd()), state) + fmt.Println("\r") + }() + } if semver.Compare(legacySemver) <= 0 && semver.Compare(devSemver) != 0 { // Legacy Auth Support - tokenType, tokenValue, err = command.legacyAuth(lineReader, target, command.BrowserOnly) + tokenType, tokenValue, err = command.legacyAuth(target, command.BrowserOnly) } else { if command.Username != "" && command.Password != "" { tokenType, tokenValue, err = command.passwordGrant(client, command.Username, command.Password) } else { - tokenType, tokenValue, err = command.authCodeGrant(lineReader, client.URL(), command.BrowserOnly) + tokenType, tokenValue, err = command.authCodeGrant(client.URL(), command.BrowserOnly) } } + if errors.Is(err, pty.ErrInterrupted) { + return nil + } + if err != nil { return err } @@ -198,8 +211,7 @@ func (command *LoginCommand) passwordGrant(client concourse.Client, username, pa return token.TokenType, idToken, nil } -func (command *LoginCommand) authCodeGrant(lineReader *liner.State, targetUrl string, browserOnly bool) (string, string, error) { - +func (command *LoginCommand) authCodeGrant(targetUrl string, browserOnly bool) (string, string, error) { var tokenStr string stdinChannel := make(chan string) @@ -213,12 +225,12 @@ func (command *LoginCommand) authCodeGrant(lineReader *liner.State, targetUrl st var openURL string - fmt.Println("navigate to the following URL in your browser:") - fmt.Println("") + fmt.Println("navigate to the following URL in your browser:\r") + fmt.Println("\r") openURL = fmt.Sprintf("%s/login?fly_port=%s", targetUrl, port) - fmt.Printf(" %s\n", openURL) + fmt.Printf(" %s\r\n", openURL) if command.OpenBrowser { // try to open the browser window, but don't get all hung up if it @@ -227,7 +239,7 @@ func (command *LoginCommand) authCodeGrant(lineReader *liner.State, targetUrl st } if !browserOnly { - go waitForTokenInput(lineReader, stdinChannel, errorChannel) + go waitForTokenInput(stdinChannel, errorChannel) } select { @@ -288,23 +300,13 @@ type tcpKeepAliveListener struct { *net.TCPListener } -func waitForTokenInput(lineReader *liner.State, tokenChannel chan string, errorChannel chan error) { +func waitForTokenInput(tokenChannel chan string, errorChannel chan error) { fmt.Println() - passwordPromptSupported := liner.TerminalSupported() for { - var token string - var err error - if passwordPromptSupported { - token, err = lineReader.PasswordPrompt("or enter token manually (input hidden): ") - if err != nil && err != liner.ErrPromptAborted && err != io.EOF { - passwordPromptSupported = false - continue - } - } else { - token, err = lineReader.Prompt("or enter token manually: ") - } - token = strings.TrimSpace(token) + fmt.Print("or enter token manually (input hidden): ") + tokenBytes, err := pty.ReadLine(os.Stdin) + token := strings.TrimSpace(string(tokenBytes)) if len(token) == 0 && err == io.EOF { return } @@ -315,7 +317,7 @@ func waitForTokenInput(lineReader *liner.State, tokenChannel chan string, errorC parts := strings.Split(token, " ") if len(parts) != 2 { - fmt.Println("token must be of the format 'TYPE VALUE', e.g. 'Bearer ...'") + fmt.Println("\rtoken must be of the format 'TYPE VALUE', e.g. 'Bearer ...'\r") continue } @@ -340,12 +342,12 @@ func (command *LoginCommand) saveTarget(url string, token *rc.TargetToken, caCer return err } - fmt.Println("target saved") + fmt.Println("\rtarget saved\r") return nil } -func (command *LoginCommand) legacyAuth(lineReader *liner.State, target rc.Target, browserOnly bool) (string, string, error) { +func (command *LoginCommand) legacyAuth(target rc.Target, browserOnly bool) (string, string, error) { httpClient := target.Client().HTTPClient() @@ -423,9 +425,9 @@ func (command *LoginCommand) legacyAuth(lineReader *liner.State, target rc.Targe theURL := fmt.Sprintf("%s&fly_local_port=%s\n", chosenMethod.AuthURL, port) - fmt.Println("navigate to the following URL in your browser:") + fmt.Println("navigate to the following URL in your browser:\r") fmt.Println("") - fmt.Printf(" %s", theURL) + fmt.Printf(" %s\r\n", theURL) if command.OpenBrowser { // try to open the browser window, but don't get all hung up if it @@ -434,7 +436,7 @@ func (command *LoginCommand) legacyAuth(lineReader *liner.State, target rc.Targe } if !browserOnly { - go waitForTokenInput(lineReader, stdinChannel, errorChannel) + go waitForTokenInput(stdinChannel, errorChannel) } select { diff --git a/fly/pty/readline.go b/fly/pty/readline.go new file mode 100644 index 000000000..a460fb452 --- /dev/null +++ b/fly/pty/readline.go @@ -0,0 +1,43 @@ +package pty + +import ( + "errors" + "io" +) + +var ErrInterrupted = errors.New("interrupted") + +const ( + keyCtrlC = 3 + keyBackspace = 127 +) + +func ReadLine(reader io.Reader) ([]byte, error) { + var buf [1]byte + var ret []byte + + for { + n, err := reader.Read(buf[:]) + if n > 0 { + switch buf[0] { + case '\b', keyBackspace: + if len(ret) > 0 { + ret = ret[:len(ret)-1] + } + case '\r', '\n': + return ret, nil + case keyCtrlC: + return nil, ErrInterrupted + default: + ret = append(ret, buf[0]) + } + continue + } + if err != nil { + if err == io.EOF && len(ret) > 0 { + return ret, nil + } + return ret, err + } + } +} diff --git a/go.mod b/go.mod index 4c0132021..8e70fdec2 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,6 @@ require ( github.com/opencontainers/runc v0.1.1 // indirect github.com/opencontainers/runtime-spec v1.0.1 github.com/patrickmn/go-cache v2.1.0+incompatible - github.com/peterh/liner v1.2.0 github.com/peterhellberg/link v1.0.0 github.com/pkg/errors v0.8.1 github.com/pkg/term v0.0.0-20190109203006-aa71e9d9e942 diff --git a/go.sum b/go.sum index 7237d5427..7ab4e0227 100644 --- a/go.sum +++ b/go.sum @@ -462,8 +462,6 @@ github.com/mattn/go-isatty v0.0.10 h1:qxFzApOv4WsAL965uUPIsXzAKCZxN2p9UqdhFS4ZW1 github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcMEpPG5Rm84= github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/mattn/go-runewidth v0.0.3 h1:a+kO+98RDGEfo6asOGMmpodZq4FNtnGP54yps8BzLR4= -github.com/mattn/go-runewidth v0.0.3/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= github.com/mattn/go-sqlite3 v0.0.0-20160907162043-3fb7a0e792ed/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= github.com/mattn/go-sqlite3 v1.10.0 h1:jbhqpg7tQe4SupckyijYiy0mJJ/pRyHvXf7JdWK860o= github.com/mattn/go-sqlite3 v1.10.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= @@ -544,8 +542,6 @@ github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaR github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pelletier/go-toml v1.5.0/go.mod h1:5N711Q9dKgbdkxHL+MEfF31hpT7l0S0s/t2kKREewys= -github.com/peterh/liner v1.2.0 h1:w/UPXyl5GfahFxcTOz2j9wCIHNI+pUPr2laqpojKNCg= -github.com/peterh/liner v1.2.0/go.mod h1:CRroGNssyjTd/qIG2FyxByd2S8JEAZXBl4qUrZf8GS0= github.com/peterhellberg/link v1.0.0 h1:mUWkiegowUXEcmlb+ybF75Q/8D2Y0BjZtR8cxoKhaQo= github.com/peterhellberg/link v1.0.0/go.mod h1:gtSlOT4jmkY8P47hbTc8PTgiDDWpdPbFYl75keYyBB8= github.com/pierrec/lz4 v2.0.5+incompatible h1:2xWsjqPFWcplujydGg4WmhC/6fZqK42wMM8aXeqhl0I=