Merge pull request #4190 from thaJeztah/command_auth_cleanups

cli/command: some cleanups / refactoring, and fixes related to auth

cli/command/container/create.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/cli/command/image"
+	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/opts"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
@@ -19,7 +20,6 @@ import (
 	"github.com/docker/docker/api/types/versions"
 	apiclient "github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/jsonmessage"
-	"github.com/docker/docker/registry"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -112,41 +112,27 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptio
 	return nil
 }
 
-func pullImage(ctx context.Context, dockerCli command.Cli, image string, platform string, out io.Writer) error {
+// FIXME(thaJeztah): this is the only code-path that uses APIClient.ImageCreate. Rewrite this to use the regular "pull" code (or vice-versa).
-	ref, err := reference.ParseNormalizedNamed(image)
+func pullImage(ctx context.Context, dockerCli command.Cli, image string, opts *createOptions) error {
+	encodedAuth, err := command.RetrieveAuthTokenFromImage(ctx, dockerCli, image)
 	if err != nil {
 		return err
 	}
 
-	// Resolve the Repository name from fqn to RepositoryInfo
+	responseBody, err := dockerCli.Client().ImageCreate(ctx, image, types.ImageCreateOptions{
-	repoInfo, err := registry.ParseRepositoryInfo(ref)
-	if err != nil {
-		return err
-	}
-
-	authConfig := command.ResolveAuthConfig(ctx, dockerCli, repoInfo.Index)
-	encodedAuth, err := command.EncodeAuthToBase64(authConfig)
-	if err != nil {
-		return err
-	}
-
-	options := types.ImageCreateOptions{
 		RegistryAuth: encodedAuth,
-		Platform:     platform,
+		Platform:     opts.platform,
-	}
+	})
-
-	responseBody, err := dockerCli.Client().ImageCreate(ctx, image, options)
 	if err != nil {
 		return err
 	}
 	defer responseBody.Close()
 
-	return jsonmessage.DisplayJSONMessagesStream(
+	out := dockerCli.Err()
-		responseBody,
+	if opts.quiet {
-		out,
+		out = io.Discard
-		dockerCli.Out().FD(),
+	}
-		dockerCli.Out().IsTerminal(),
+	return jsonmessage.DisplayJSONMessagesToStream(responseBody, streams.NewOut(out), nil)
-		nil)
 }
 
 type cidFile struct {
@@ -236,11 +222,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
 	}
 
 	pullAndTagImage := func() error {
-		pullOut := dockerCli.Err()
+		if err := pullImage(ctx, dockerCli, config.Image, opts); err != nil {
-		if opts.quiet {
-			pullOut = io.Discard
-		}
-		if err := pullImage(ctx, dockerCli, config.Image, opts.platform, pullOut); err != nil {
 			return err
 		}
 		if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {

cli/command/image/push.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
+	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
@@ -76,7 +77,7 @@ func RunPush(dockerCli command.Cli, opts pushOptions) error {
 
 	// Resolve the Auth config relevant for this server
 	authConfig := command.ResolveAuthConfig(ctx, dockerCli, repoInfo.Index)
-	encodedAuth, err := command.EncodeAuthToBase64(authConfig)
+	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
 	if err != nil {
 		return err
 	}

cli/command/image/trust.go

@@ -262,20 +262,17 @@ func getTrustedPullTargets(cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth)
 
 // imagePullPrivileged pulls the image and displays it to the output
 func imagePullPrivileged(ctx context.Context, cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth, opts PullOptions) error {
-	ref := reference.FamiliarString(imgRefAndAuth.Reference())
+	encodedAuth, err := registrytypes.EncodeAuthConfig(*imgRefAndAuth.AuthConfig())
-
-	encodedAuth, err := command.EncodeAuthToBase64(*imgRefAndAuth.AuthConfig())
 	if err != nil {
 		return err
 	}
 	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, imgRefAndAuth.RepoInfo().Index, "pull")
-	options := types.ImagePullOptions{
+	responseBody, err := cli.Client().ImagePull(ctx, reference.FamiliarString(imgRefAndAuth.Reference()), types.ImagePullOptions{
 		RegistryAuth:  encodedAuth,
 		PrivilegeFunc: requestPrivilege,
 		All:           opts.all,
 		Platform:      opts.platform,
-	}
+	})
-	responseBody, err := cli.Client().ImagePull(ctx, ref, options)
 	if err != nil {
 		return err
 	}

cli/command/plugin/install.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
+	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
@@ -86,8 +87,7 @@ func buildPullConfig(ctx context.Context, dockerCli command.Cli, opts pluginOpti
 	}
 
 	authConfig := command.ResolveAuthConfig(ctx, dockerCli, repoInfo.Index)
-
+	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
-	encodedAuth, err := command.EncodeAuthToBase64(authConfig)
 	if err != nil {
 		return types.PluginInstallOptions{}, err
 	}

cli/command/plugin/push.go

@@ -7,6 +7,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/distribution/reference"
+	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
@@ -55,8 +56,7 @@ func runPush(dockerCli command.Cli, opts pushOptions) error {
 		return err
 	}
 	authConfig := command.ResolveAuthConfig(ctx, dockerCli, repoInfo.Index)
-
+	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
-	encodedAuth, err := command.EncodeAuthToBase64(authConfig)
 	if err != nil {
 		return err
 	}

cli/command/registry.go

@@ -3,8 +3,6 @@ package command
 import (
 	"bufio"
 	"context"
-	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"io"
 	"os"
@@ -21,13 +19,9 @@ import (
 	"github.com/pkg/errors"
 )
 
-// EncodeAuthToBase64 serializes the auth configuration as JSON base64 payload
+// EncodeAuthToBase64 serializes the auth configuration as JSON base64 payload.
 func EncodeAuthToBase64(authConfig registrytypes.AuthConfig) (string, error) {
-	buf, err := json.Marshal(authConfig)
+	return registrytypes.EncodeAuthConfig(authConfig)
-	if err != nil {
-		return "", err
-	}
-	return base64.URLEncoding.EncodeToString(buf), nil
 }
 
 // RegistryAuthenticationPrivilegedFunc returns a RequestPrivilegeFunc from the specified registry index info
@@ -45,7 +39,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
 		if err != nil {
 			return "", err
 		}
-		return EncodeAuthToBase64(authConfig)
+		return registrytypes.EncodeAuthConfig(authConfig)
 	}
 }
 
@@ -89,7 +83,14 @@ func GetDefaultAuthConfig(cli Cli, checkCredStore bool, serverAddress string, is
 
 // ConfigureAuth handles prompting of user's username and password if needed
 func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes.AuthConfig, isDefaultRegistry bool) error {
-	// On Windows, force the use of the regular OS stdin stream. Fixes #14336/#14210
+	// On Windows, force the use of the regular OS stdin stream.
+	//
+	// See:
+	// - https://github.com/moby/moby/issues/14336
+	// - https://github.com/moby/moby/issues/14210
+	// - https://github.com/moby/moby/pull/17738
+	//
+	// TODO(thaJeztah): we need to confirm if this special handling is still needed, as we may not be doing this in other places.
 	if runtime.GOOS == "windows" {
 		cli.SetIn(streams.NewIn(os.Stdin))
 	}
@@ -113,8 +114,11 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
 			fmt.Fprintln(cli.Out(), "Login with your Docker ID to push and pull images from Docker Hub. If you don't have a Docker ID, head over to https://hub.docker.com to create one.")
 		}
 		promptWithDefault(cli.Out(), "Username", authconfig.Username)
-		flUser = readInput(cli.In(), cli.Out())
+		var err error
-		flUser = strings.TrimSpace(flUser)
+		flUser, err = readInput(cli.In())
+		if err != nil {
+			return err
+		}
 		if flUser == "" {
 			flUser = authconfig.Username
 		}
@@ -128,12 +132,15 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
 			return err
 		}
 		fmt.Fprintf(cli.Out(), "Password: ")
-		term.DisableEcho(cli.In().FD(), oldState)
+		_ = term.DisableEcho(cli.In().FD(), oldState)
-
+		defer func() {
-		flPassword = readInput(cli.In(), cli.Out())
+			_ = term.RestoreTerminal(cli.In().FD(), oldState)
+		}()
+		flPassword, err = readInput(cli.In())
+		if err != nil {
+			return err
+		}
 		fmt.Fprint(cli.Out(), "\n")
-
-		term.RestoreTerminal(cli.In().FD(), oldState)
 		if flPassword == "" {
 			return errors.Errorf("Error: Password Required")
 		}
@@ -145,14 +152,15 @@ func ConfigureAuth(cli Cli, flUser, flPassword string, authconfig *registrytypes
 	return nil
 }
 
-func readInput(in io.Reader, out io.Writer) string {
+// readInput reads, and returns user input from in. It tries to return a
-	reader := bufio.NewReader(in)
+// single line, not including the end-of-line bytes, and trims leading
-	line, _, err := reader.ReadLine()
+// and trailing whitespace.
+func readInput(in io.Reader) (string, error) {
+	line, _, err := bufio.NewReader(in).ReadLine()
 	if err != nil {
-		fmt.Fprintln(out, err.Error())
+		return "", errors.Wrap(err, "error while reading input")
-		os.Exit(1)
 	}
-	return string(line)
+	return strings.TrimSpace(string(line)), nil
 }
 
 func promptWithDefault(out io.Writer, prompt string, configDefault string) {
@@ -163,14 +171,19 @@ func promptWithDefault(out io.Writer, prompt string, configDefault string) {
 	}
 }
 
-// RetrieveAuthTokenFromImage retrieves an encoded auth token given a complete image
+// RetrieveAuthTokenFromImage retrieves an encoded auth token given a complete
+// image. The auth configuration is serialized as a base64url encoded RFC4648,
+// section 5) JSON string for sending through the X-Registry-Auth header.
+//
+// For details on base64url encoding, see:
+// - RFC4648, section 5:   https://tools.ietf.org/html/rfc4648#section-5
 func RetrieveAuthTokenFromImage(ctx context.Context, cli Cli, image string) (string, error) {
 	// Retrieve encoded auth token from the image reference
 	authConfig, err := resolveAuthConfigFromImage(ctx, cli, image)
 	if err != nil {
 		return "", err
 	}
-	encodedAuth, err := EncodeAuthToBase64(authConfig)
+	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
 	if err != nil {
 		return "", err
 	}

cli/command/registry/search.go

@@ -8,6 +8,7 @@ import (
 	"github.com/docker/cli/cli/command/formatter"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types"
+	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/registry"
 	"github.com/spf13/cobra"
 )
@@ -54,25 +55,19 @@ func runSearch(dockerCli command.Cli, options searchOptions) error {
 	}
 
 	ctx := context.Background()
-
 	authConfig := command.ResolveAuthConfig(ctx, dockerCli, indexInfo)
-	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, indexInfo, "search")
+	encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
-
-	encodedAuth, err := command.EncodeAuthToBase64(authConfig)
 	if err != nil {
 		return err
 	}
 
-	searchOptions := types.ImageSearchOptions{
+	requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, indexInfo, "search")
+	results, err := dockerCli.Client().ImageSearch(ctx, options.term, types.ImageSearchOptions{
 		RegistryAuth:  encodedAuth,
 		PrivilegeFunc: requestPrivilege,
 		Filters:       options.filter.Value(),
 		Limit:         options.limit,
-	}
+	})
-
-	clnt := dockerCli.Client()
-
-	results, err := clnt.ImageSearch(ctx, options.term, searchOptions)
 	if err != nil {
 		return err
 	}

cli/command/trust/sign.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/cli/cli/trust"
 	"github.com/docker/docker/api/types"
+	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/theupdateframework/notary/client"
@@ -93,7 +94,7 @@ func runSignImage(cli command.Cli, options signOptions) error {
 			fmt.Fprintf(cli.Err(), "Signing and pushing trust data for local image %s, may overwrite remote trust data\n", imageName)
 
 			authConfig := command.ResolveAuthConfig(ctx, cli, imgRefAndAuth.RepoInfo().Index)
-			encodedAuth, err := command.EncodeAuthToBase64(authConfig)
+			encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig)
 			if err != nil {
 				return err
 			}