Merge pull request #5259 from thaJeztah/move_file_warning

cli/config/credentials: move warning to fileStore

Author: laurazard

cli/command/registry/login.go

@@ -18,17 +18,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// unencryptedWarning warns the user when using an insecure credential storage.
-// After a deprecation period, user will get prompted if stdin and stderr are a terminal.
-// Otherwise, we'll assume they want it (sadly), because people may have been scripting
-// insecure logins and we don't want to break them. Maybe they'll see the warning in their
-// logs and fix things.
-const unencryptedWarning = `
-WARNING! Your credentials are stored unencrypted in '%s'.
-Configure a credential helper to remove this warning. See
-https://docs.docker.com/go/credential-store/
-`
-
 type loginOptions struct {
 	serverAddress string
 	user          string
@@ -66,11 +55,6 @@ func NewLoginCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-type isFileStore interface {
-	IsFileStore() bool
-	GetFilename() string
-}
-
 func verifyloginOptions(dockerCli command.Cli, opts *loginOptions) error {
 	if opts.password != "" {
 		fmt.Fprintln(dockerCli.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
@@ -137,16 +121,10 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err
 	}
 
 	creds := dockerCli.ConfigFile().GetCredentialsStore(serverAddress)
-
 	if err := creds.Store(configtypes.AuthConfig(authConfig)); err != nil {
 		return errors.Errorf("Error saving credentials: %v", err)
 	}
 
-	if store, isDefault := creds.(isFileStore); isDefault && authConfig.Password != "" {
-		// Display a warning if we're storing the users password (not a token)
-		_, _ = fmt.Fprintln(dockerCli.Err(), fmt.Sprintf(unencryptedWarning, store.GetFilename()))
-	}
-
 	if response.Status != "" {
 		fmt.Fprintln(dockerCli.Out(), response.Status)
 	}

cli/config/credentials/file_store.go

@@ -1,8 +1,10 @@
 package credentials
 
 import (
+	"fmt"
 	"net"
 	"net/url"
+	"os"
 	"strings"
 
 	"github.com/docker/cli/cli/config/types"
@@ -52,19 +54,33 @@ func (c *fileStore) GetAll() (map[string]types.AuthConfig, error) {
 	return c.file.GetAuthConfigs(), nil
 }
 
+// unencryptedWarning warns the user when using an insecure credential storage.
+// After a deprecation period, user will get prompted if stdin and stderr are a terminal.
+// Otherwise, we'll assume they want it (sadly), because people may have been scripting
+// insecure logins and we don't want to break them. Maybe they'll see the warning in their
+// logs and fix things.
+const unencryptedWarning = `
+WARNING! Your credentials are stored unencrypted in '%s'.
+Configure a credential helper to remove this warning. See
+https://docs.docker.com/go/credential-store/
+`
+
 // Store saves the given credentials in the file store.
 func (c *fileStore) Store(authConfig types.AuthConfig) error {
 	authConfigs := c.file.GetAuthConfigs()
 	authConfigs[authConfig.ServerAddress] = authConfig
-	return c.file.Save()
-}
+	if err := c.file.Save(); err != nil {
+		return err
+	}
 
-func (c *fileStore) GetFilename() string {
-	return c.file.GetFilename()
-}
+	if authConfig.Password != "" {
+		// Display a warning if we're storing the users password (not a token).
+		//
+		// FIXME(thaJeztah): make output configurable instead of hardcoding to os.Stderr
+		_, _ = fmt.Fprintln(os.Stderr, fmt.Sprintf(unencryptedWarning, c.file.GetFilename()))
+	}
 
-func (c *fileStore) IsFileStore() bool {
-	return true
+	return nil
 }
 
 // ConvertToHostname converts a registry url which has http|https prepended