Refactor ui package to not wrap os.Stdout and os.Stdin and allow easier mocking

Author: Marton6

internals/cli/ui/ask.go

@@ -51,7 +51,7 @@ func AskWithDefault(io IO, question, defaultValue string) (string, error) {
 
 // AskSecret prints out the question and reads back the input,
 // without echoing it back. Useful for passwords and other sensitive inputs.
-func AskSecret(io IO, question string) (string, error) {
+func AskSecret(io IO, passwordReader PasswordReader, question string) (string, error) {
 	promptIn, promptOut, err := io.Prompts()
 	if err != nil {
 		return "", err
@@ -62,14 +62,14 @@ func AskSecret(io IO, question string) (string, error) {
 		return "", err
 	}
 
-	raw, err := promptIn.ReadPassword()
+	raw, err := passwordReader.Read(promptIn)
 	if err != nil {
 		return "", ErrReadInput(err)
 	}
 
 	fmt.Fprintln(promptOut, "")
 
-	return string(raw), nil
+	return raw, nil
 }
 
 // AskMultiline prints out the question and reads back the input until an EOF is reached.
@@ -145,14 +145,14 @@ func ConfirmCaseInsensitive(io IO, question string, expected ...string) (bool, e
 // the answers still haven't matched after trying n times, the error
 // ErrPassphrasesDoNotMatch is returned. For the empty answer ("") no
 // confirmation is asked.
-func AskPassphrase(io IO, question string, repeatPhrase string, n int) (string, error) {
+func AskPassphrase(io IO, passwordReader PasswordReader, question string, repeatPhrase string, n int) (string, error) {
 	_, promptOut, err := io.Prompts()
 	if err != nil {
 		return "", err
 	}
 
 	for i := 0; i < n; i++ {
-		answer, err := AskSecret(io, question)
+		answer, err := AskSecret(io, passwordReader, question)
 		if err != nil {
 			return "", err
 		}
@@ -161,7 +161,7 @@ func AskPassphrase(io IO, question string, repeatPhrase string, n int) (string,
 			return answer, nil
 		}
 
-		confirmed, err := AskSecret(io, repeatPhrase)
+		confirmed, err := AskSecret(io, passwordReader, repeatPhrase)
 		if err != nil {
 			return "", err
 		}

internals/cli/ui/io.go

@@ -18,35 +18,37 @@ var (
 
 // IO is an interface to work with input/output.
 type IO interface {
-	Stdin() Reader
-	Stdout() Writer
-	Prompts() (Reader, Writer, error)
+	Stdin() io.Reader
+	Stdout() io.Writer
+	Prompts() (io.Reader, io.Writer, error)
+	IsStdinPiped() bool
+	IsStdoutPiped() bool
 }
 
 // UserIO is a middleware between input and output to the CLI program.
 // It implements userIO.Prompter and can be passed to libraries.
 type UserIO struct {
-	Input        Reader
-	Output       Writer
-	tty          file
+	Input        *os.File
+	Output       *os.File
+	tty          *os.File
 	ttyAvailable bool
 }
 
 // NewStdUserIO creates a new UserIO middleware only from os.Stdin and os.Stdout.
 func NewStdUserIO() UserIO {
 	return UserIO{
-		Input:  file{os.Stdin},
-		Output: file{os.Stdout},
+		Input:  os.Stdin,
+		Output: os.Stdout,
 	}
 }
 
 // Stdin returns the UserIO's Input.
-func (o UserIO) Stdin() Reader {
+func (o UserIO) Stdin() io.Reader {
 	return o.Input
 }
 
 // Stdout returns the UserIO's Output.
-func (o UserIO) Stdout() Writer {
+func (o UserIO) Stdout() io.Writer {
 	return o.Output
 }
 
@@ -55,8 +57,8 @@ func (o UserIO) Stdout() Writer {
 // bypass stdin and stdout by connecting to /dev/tty on Unix systems when
 // available. On systems where tty is not available and when either input
 // or output is piped, prompting is not possible so an error is returned.
-func (o UserIO) Prompts() (Reader, Writer, error) {
-	if o.Input.IsPiped() || o.Output.IsPiped() {
+func (o UserIO) Prompts() (io.Reader, io.Writer, error) {
+	if o.IsStdoutPiped() || o.IsStdinPiped() {
 		if o.ttyAvailable {
 			return o.tty, o.tty, nil
 		}
@@ -65,50 +67,58 @@ func (o UserIO) Prompts() (Reader, Writer, error) {
 	return o.Input, o.Output, nil
 }
 
-// Reader can read input for a CLI program.
-type Reader interface {
-	io.Reader
-	// ReadPassword reads a line of input from a terminal without local echo.
-	ReadPassword() ([]byte, error)
-	IsPiped() bool
+func (o UserIO) IsStdinPiped() bool {
+	return isPiped(o.Input)
 }
 
-// Readln reads 1 line of input from a io.Reader. The newline character is not included in the response.
-func Readln(r io.Reader) (string, error) {
-	s := bufio.NewScanner(r)
-	s.Scan()
-	err := s.Err()
-	if err != nil {
-		return "", ErrReadInput(err)
-	}
-	return s.Text(), nil
+func (o UserIO) IsStdoutPiped() bool {
+	return isPiped(o.Output)
 }
 
-// Writer can write output for a CLI program.
-type Writer interface {
-	io.Writer
-	IsPiped() bool
+type PasswordReader interface {
+	Read(reader io.Reader) (string, error)
 }
 
-// file implements the Reader and Writer interface.
-type file struct {
-	*os.File
+type passwordReader struct{}
+
+// NewPasswordReader returns a reader that reads a string from the terminal without echoing the user input.
+func NewPasswordReader() *passwordReader {
+	return &passwordReader{}
 }
 
-// ReadPassword reads from a terminal without echoing back the typed input.
-func (f file) ReadPassword() ([]byte, error) {
+// Read reads one line of input from the terminal without echoing the user input.
+func (pr *passwordReader) Read(r io.Reader) (string, error) {
+	file, ok := r.(*os.File)
+	if !ok {
+		return "", ErrCannotAsk
+	}
 	// this case happens among other things when input is piped and ReadPassword is called.
-	if !terminal.IsTerminal(int(f.Fd())) {
-		return nil, ErrCannotAsk
+	if !terminal.IsTerminal(int(file.Fd())) {
+		return "", ErrCannotAsk
+	}
+
+	password, err := terminal.ReadPassword(int(file.Fd()))
+	if err != nil {
+		return "", err
 	}
+	return string(password), nil
+}
 
-	return terminal.ReadPassword(int(f.Fd()))
+// Readln reads 1 line of input from a io.Reader. The newline character is not included in the response.
+func Readln(r io.Reader) (string, error) {
+	s := bufio.NewScanner(r)
+	s.Scan()
+	err := s.Err()
+	if err != nil {
+		return "", ErrReadInput(err)
+	}
+	return s.Text(), nil
 }
 
-// IsPiped checks whether the file is a pipe.
+// isPiped checks whether the file is a pipe.
 // If the file does not exist, it returns false.
-func (f file) IsPiped() bool {
-	stat, err := f.Stat()
+func isPiped(file *os.File) bool {
+	stat, err := file.Stat()
 	if err != nil {
 		return false
 	}

internals/cli/ui/io_unix.go

@@ -9,9 +9,9 @@ func NewUserIO() UserIO {
 	tty, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
 	if err == nil {
 		return UserIO{
-			Input:        file{os.Stdin},
-			Output:       file{os.Stdout},
-			tty:          file{tty},
+			Input:        os.Stdin,
+			Output:       os.Stdout,
+			tty:          tty,
 			ttyAvailable: true,
 		}
 	}

internals/cli/ui/testing.go

@@ -3,8 +3,10 @@
 package ui
 
 import (
+	"bufio"
 	"bytes"
 	"errors"
+	"io"
 )
 
 // FakeIO is a helper type for testing that implements the ui.IO interface
@@ -35,20 +37,33 @@ func NewFakeIO() *FakeIO {
 }
 
 // Stdin returns the mocked StdIn.
-func (f *FakeIO) Stdin() Reader {
+func (f *FakeIO) Stdin() io.Reader {
 	return f.StdIn
 }
 
 // Stdout returns the mocked StdOut.
-func (f *FakeIO) Stdout() Writer {
+func (f *FakeIO) Stdout() io.Writer {
 	return f.StdOut
 }
 
 // Prompts returns the mocked prompts and error.
-func (f *FakeIO) Prompts() (Reader, Writer, error) {
+func (f *FakeIO) Prompts() (io.Reader, io.Writer, error) {
 	return f.PromptIn, f.PromptOut, f.PromptErr
 }
 
+func (f *FakeIO) IsStdinPiped() bool {
+	return f.StdIn.Piped
+}
+
+func (f *FakeIO) IsStdoutPiped() bool {
+	return f.StdOut.Piped
+}
+
+func (f *FakeIO) ReadPassword() ([]byte, error) {
+	line, _, err := bufio.NewReader(f.PromptIn).ReadLine()
+	return line, err
+}
+
 // FakeReader implements the Reader interface.
 type FakeReader struct {
 	*bytes.Buffer
@@ -97,3 +112,9 @@ type FakeWriter struct {
 func (f *FakeWriter) IsPiped() bool {
 	return f.Piped
 }
+
+type FakePasswordReader struct{}
+
+func (f FakePasswordReader) Read(reader io.Reader) (string, error) {
+	return Readln(reader)
+}

internals/secrethub/account_init.go

@@ -36,6 +36,7 @@ var (
 // AccountInitCommand creates, stores and outputs a credential.
 type AccountInitCommand struct {
 	io              ui.IO
+	passwordReader  ui.PasswordReader
 	useClipboard    bool
 	noWait          bool
 	isContinue      bool
@@ -50,6 +51,7 @@ type AccountInitCommand struct {
 func NewAccountInitCommand(io ui.IO, newClient newClientFunc, credentialStore CredentialConfig) *AccountInitCommand {
 	return &AccountInitCommand{
 		io:              io,
+		passwordReader:  ui.NewPasswordReader(),
 		credentialStore: credentialStore,
 		clipper:         clip.NewClipboard(),
 		progressPrinter: progress.NewPrinter(io.Stdout(), 500*time.Millisecond),
@@ -160,7 +162,7 @@ func (cmd *AccountInitCommand) Run() error {
 		var passphrase string
 		if !cmd.credentialStore.IsPassphraseSet() && !cmd.force {
 			var err error
-			passphrase, err = ui.AskPassphrase(cmd.io, "Please enter a passphrase to protect your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
+			passphrase, err = ui.AskPassphrase(cmd.io, cmd.passwordReader, "Please enter a passphrase to protect your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
 			if err != nil {
 				return err
 			}

internals/secrethub/config_update_passphrase.go

@@ -11,13 +11,15 @@ import (
 
 type ConfigUpdatePassphraseCommand struct {
 	io              ui.IO
+	passwordReader  ui.PasswordReader
 	credentialStore CredentialConfig
 }
 
 // NewConfigUpdatePassphraseCommand creates a new ConfigUpdatePassphraseCommand.
 func NewConfigUpdatePassphraseCommand(io ui.IO, credentialStore CredentialConfig) *ConfigUpdatePassphraseCommand {
 	return &ConfigUpdatePassphraseCommand{
 		io:              io,
+		passwordReader:  ui.NewPasswordReader(),
 		credentialStore: credentialStore,
 	}
 }
@@ -58,7 +60,7 @@ func (cmd *ConfigUpdatePassphraseCommand) Run() error {
 		return err
 	}
 
-	passphrase, err := ui.AskPassphrase(cmd.io, "Please enter a passphrase to (re)encrypt your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
+	passphrase, err := ui.AskPassphrase(cmd.io, cmd.passwordReader, "Please enter a passphrase to (re)encrypt your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
 	if err != nil {
 		return err
 	}

internals/secrethub/init.go

@@ -19,6 +19,7 @@ type InitCommand struct {
 	backupCode                  string
 	force                       bool
 	io                          ui.IO
+	passwordReader              ui.PasswordReader
 	newClient                   newClientFunc
 	newClientWithoutCredentials func(credentials.Provider) (secrethub.ClientInterface, error)
 	credentialStore             CredentialConfig
@@ -29,6 +30,7 @@ type InitCommand struct {
 func NewInitCommand(io ui.IO, newClient newClientFunc, newClientWithoutCredentials func(credentials.Provider) (secrethub.ClientInterface, error), credentialStore CredentialConfig) *InitCommand {
 	return &InitCommand{
 		io:                          io,
+		passwordReader:              ui.NewPasswordReader(),
 		newClient:                   newClient,
 		newClientWithoutCredentials: newClientWithoutCredentials,
 		credentialStore:             credentialStore,
@@ -167,7 +169,7 @@ func (cmd *InitCommand) Run() error {
 		var passphrase string
 		if !cmd.credentialStore.IsPassphraseSet() && !cmd.force {
 			var err error
-			passphrase, err = ui.AskPassphrase(cmd.io, "Please enter a passphrase to protect your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
+			passphrase, err = ui.AskPassphrase(cmd.io, cmd.passwordReader, "Please enter a passphrase to protect your local credential (leave empty for no passphrase): ", "Enter the same passphrase again: ", 3)
 			if err != nil {
 				return err
 			}

internals/secrethub/inject.go

@@ -91,7 +91,7 @@ func (cmd *InjectCommand) Run() error {
 			return ErrReadFile(cmd.inFile, err)
 		}
 	} else {
-		if !cmd.io.Stdin().IsPiped() {
+		if !cmd.io.IsStdinPiped() {
 			return ErrNoDataOnStdin
 		}
 
@@ -139,7 +139,7 @@ func (cmd *InjectCommand) Run() error {
 	} else if cmd.outFile != "" {
 		_, err := os.Stat(cmd.outFile)
 		if err == nil && !cmd.force {
-			if cmd.io.Stdout().IsPiped() {
+			if cmd.io.IsStdoutPiped() {
 				return ErrFileAlreadyExists
 			}