Merge pull request #5260 from thaJeztah/27.1_backport_fix-cli-login

[27.0 backport] fix: ctx cancellation on login prompt
This commit is contained in:
Sebastiaan van Stijn 2024-07-19 12:05:16 +02:00 committed by GitHub
commit 7e972d2f5c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 160 additions and 34 deletions

View File

@ -1,10 +1,8 @@
package command package command
import ( import (
"bufio"
"context" "context"
"fmt" "fmt"
"io"
"os" "os"
"runtime" "runtime"
"strings" "strings"
@ -18,7 +16,6 @@ import (
"github.com/docker/docker/api/types" "github.com/docker/docker/api/types"
registrytypes "github.com/docker/docker/api/types/registry" registrytypes "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/registry" "github.com/docker/docker/registry"
"github.com/moby/term"
"github.com/pkg/errors" "github.com/pkg/errors"
) )
@ -44,7 +41,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
default: default:
} }
err = ConfigureAuth(cli, "", "", &authConfig, isDefaultRegistry) err = ConfigureAuth(ctx, cli, "", "", &authConfig, isDefaultRegistry)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -90,7 +87,7 @@ func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serve
} }
// ConfigureAuth handles prompting of user's username and password if needed // ConfigureAuth handles prompting of user's username and password if needed
func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes.AuthConfig, isDefaultRegistry bool) error { func ConfigureAuth(ctx context.Context, cli Cli, flUser, flPassword string, authconfig *registrytypes.AuthConfig, isDefaultRegistry bool) error {
// On Windows, force the use of the regular OS stdin stream. // On Windows, force the use of the regular OS stdin stream.
// //
// See: // See:
@ -125,9 +122,15 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
fmt.Fprintln(cli.Out()) fmt.Fprintln(cli.Out())
} }
} }
promptWithDefault(cli.Out(), "Username", authconfig.Username)
var prompt string
if authconfig.Username == "" {
prompt = "Username: "
} else {
prompt = fmt.Sprintf("Username (%s): ", authconfig.Username)
}
var err error var err error
flUser, err = readInput(cli.In()) flUser, err = PromptForInput(ctx, cli.In(), cli.Out(), prompt)
if err != nil { if err != nil {
return err return err
} }
@ -139,16 +142,13 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
return errors.Errorf("Error: Non-null Username Required") return errors.Errorf("Error: Non-null Username Required")
} }
if flPassword == "" { if flPassword == "" {
oldState, err := term.SaveState(cli.In().FD()) restoreInput, err := DisableInputEcho(cli.In())
if err != nil { if err != nil {
return err return err
} }
fmt.Fprintf(cli.Out(), "Password: ") defer restoreInput()
_ = term.DisableEcho(cli.In().FD(), oldState)
defer func() { flPassword, err = PromptForInput(ctx, cli.In(), cli.Out(), "Password: ")
_ = term.RestoreTerminal(cli.In().FD(), oldState)
}()
flPassword, err = readInput(cli.In())
if err != nil { if err != nil {
return err return err
} }
@ -164,25 +164,6 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
return nil return nil
} }
// readInput reads, and returns user input from in. It tries to return a
// single line, not including the end-of-line bytes, and trims leading
// and trailing whitespace.
func readInput(in io.Reader) (string, error) {
line, _, err := bufio.NewReader(in).ReadLine()
if err != nil {
return "", errors.Wrap(err, "error while reading input")
}
return strings.TrimSpace(string(line)), nil
}
func promptWithDefault(out io.Writer, prompt string, configDefault string) {
if configDefault == "" {
fmt.Fprintf(out, "%s: ", prompt)
} else {
fmt.Fprintf(out, "%s (%s): ", prompt, configDefault)
}
}
// RetrieveAuthTokenFromImage retrieves an encoded auth token given a complete // RetrieveAuthTokenFromImage retrieves an encoded auth token given a complete
// image. The auth configuration is serialized as a base64url encoded RFC4648, // image. The auth configuration is serialized as a base64url encoded RFC4648,
// section 5) JSON string for sending through the X-Registry-Auth header. // section 5) JSON string for sending through the X-Registry-Auth header.

View File

@ -121,7 +121,7 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err
response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig) response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig)
} }
if err != nil || authConfig.Username == "" || authConfig.Password == "" { if err != nil || authConfig.Username == "" || authConfig.Password == "" {
err = command.ConfigureAuth(dockerCli, opts.user, opts.password, &authConfig, isDefaultRegistry) err = command.ConfigureAuth(ctx, dockerCli, opts.user, opts.password, &authConfig, isDefaultRegistry)
if err != nil { if err != nil {
return err return err
} }

View File

@ -6,7 +6,10 @@ import (
"errors" "errors"
"fmt" "fmt"
"testing" "testing"
"time"
"github.com/creack/pty"
"github.com/docker/cli/cli/command"
configtypes "github.com/docker/cli/cli/config/types" configtypes "github.com/docker/cli/cli/config/types"
"github.com/docker/cli/cli/streams" "github.com/docker/cli/cli/streams"
"github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test"
@ -185,3 +188,41 @@ func TestRunLogin(t *testing.T) {
}) })
} }
} }
func TestLoginTermination(t *testing.T) {
p, tty, err := pty.Open()
assert.NilError(t, err)
t.Cleanup(func() {
_ = tty.Close()
_ = p.Close()
})
cli := test.NewFakeCli(&fakeClient{}, func(fc *test.FakeCli) {
fc.SetOut(streams.NewOut(tty))
fc.SetIn(streams.NewIn(tty))
})
tmpFile := fs.NewFile(t, "test-login-termination")
defer tmpFile.Remove()
configFile := cli.ConfigFile()
configFile.Filename = tmpFile.Path()
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
runErr := make(chan error)
go func() {
runErr <- runLogin(ctx, cli, loginOptions{})
}()
// Let the prompt get canceled by the context
cancel()
select {
case <-time.After(1 * time.Second):
t.Fatal("timed out after 1 second. `runLogin` did not return")
case err := <-runErr:
assert.ErrorIs(t, err, command.ErrPromptTerminated)
}
}

View File

@ -19,6 +19,7 @@ import (
"github.com/docker/docker/api/types/versions" "github.com/docker/docker/api/types/versions"
"github.com/docker/docker/errdefs" "github.com/docker/docker/errdefs"
"github.com/moby/sys/sequential" "github.com/moby/sys/sequential"
"github.com/moby/term"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/spf13/pflag" "github.com/spf13/pflag"
) )
@ -76,6 +77,48 @@ func PrettyPrint(i any) string {
var ErrPromptTerminated = errdefs.Cancelled(errors.New("prompt terminated")) var ErrPromptTerminated = errdefs.Cancelled(errors.New("prompt terminated"))
// DisableInputEcho disables input echo on the provided streams.In.
// This is useful when the user provides sensitive information like passwords.
// The function returns a restore function that should be called to restore the
// terminal state.
func DisableInputEcho(ins *streams.In) (restore func() error, err error) {
oldState, err := term.SaveState(ins.FD())
if err != nil {
return nil, err
}
restore = func() error {
return term.RestoreTerminal(ins.FD(), oldState)
}
return restore, term.DisableEcho(ins.FD(), oldState)
}
// PromptForInput requests input from the user.
//
// If the user terminates the CLI with SIGINT or SIGTERM while the prompt is
// active, the prompt will return an empty string ("") with an ErrPromptTerminated error.
// When the prompt returns an error, the caller should propagate the error up
// the stack and close the io.Reader used for the prompt which will prevent the
// background goroutine from blocking indefinitely.
func PromptForInput(ctx context.Context, in io.Reader, out io.Writer, message string) (string, error) {
_, _ = fmt.Fprint(out, message)
result := make(chan string)
go func() {
scanner := bufio.NewScanner(in)
if scanner.Scan() {
result <- strings.TrimSpace(scanner.Text())
}
}()
select {
case <-ctx.Done():
_, _ = fmt.Fprintln(out, "")
return "", ErrPromptTerminated
case r := <-result:
return r, nil
}
}
// PromptForConfirmation requests and checks confirmation from the user. // PromptForConfirmation requests and checks confirmation from the user.
// This will display the provided message followed by ' [y/N] '. If the user // This will display the provided message followed by ' [y/N] '. If the user
// input 'y' or 'Y' it returns true otherwise false. If no message is provided, // input 'y' or 'Y' it returns true otherwise false. If no message is provided,

View File

@ -15,6 +15,7 @@ import (
"time" "time"
"github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/streams"
"github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test"
"github.com/pkg/errors" "github.com/pkg/errors"
"gotest.tools/v3/assert" "gotest.tools/v3/assert"
@ -80,6 +81,66 @@ func TestValidateOutputPath(t *testing.T) {
} }
} }
func TestPromptForInput(t *testing.T) {
t.Run("case=cancelling the context", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
reader, _ := io.Pipe()
buf := new(bytes.Buffer)
bufioWriter := bufio.NewWriter(buf)
wroteHook := make(chan struct{}, 1)
promptOut := test.NewWriterWithHook(bufioWriter, func(p []byte) {
wroteHook <- struct{}{}
})
promptErr := make(chan error, 1)
go func() {
_, err := command.PromptForInput(ctx, streams.NewIn(reader), streams.NewOut(promptOut), "Enter something")
promptErr <- err
}()
select {
case <-time.After(1 * time.Second):
t.Fatal("timeout waiting for prompt to write to buffer")
case <-wroteHook:
cancel()
}
select {
case <-time.After(1 * time.Second):
t.Fatal("timeout waiting for prompt to be canceled")
case err := <-promptErr:
assert.ErrorIs(t, err, command.ErrPromptTerminated)
}
})
t.Run("case=user input should be properly trimmed", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
t.Cleanup(cancel)
reader, writer := io.Pipe()
buf := new(bytes.Buffer)
bufioWriter := bufio.NewWriter(buf)
wroteHook := make(chan struct{}, 1)
promptOut := test.NewWriterWithHook(bufioWriter, func(p []byte) {
wroteHook <- struct{}{}
})
go func() {
<-wroteHook
writer.Write([]byte(" foo \n"))
}()
answer, err := command.PromptForInput(ctx, streams.NewIn(reader), streams.NewOut(promptOut), "Enter something")
assert.NilError(t, err)
assert.Equal(t, answer, "foo")
})
}
func TestPromptForConfirmation(t *testing.T) { func TestPromptForConfirmation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel) t.Cleanup(cancel)