From a78ab6380134805e7dc02ca8caa415a192f1e6fd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 18 Jul 2024 18:15:12 +0200 Subject: [PATCH 1/2] login: don't print "unencrypted" warning when failing to save credentials If we fail to save credentials, make sure that the error about saving doesn't get lost in the warning about credentials being stored unencrypted. Also discard errors about printing the warning, as those would be unlikely, and if they would occur, probably would fail to be printed as well. Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login.go | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 75cf7c5265..b82f0b8e79 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -18,6 +18,11 @@ 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 password will be stored unencrypted in %s. Configure a credential helper to remove this warning. See https://docs.docker.com/engine/reference/commandline/login/#credential-stores @@ -60,17 +65,6 @@ func NewLoginCommand(dockerCli command.Cli) *cobra.Command { return cmd } -// displayUnencryptedWarning 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. -func displayUnencryptedWarning(dockerCli command.Streams, filename string) error { - _, err := fmt.Fprintln(dockerCli.Err(), fmt.Sprintf(unencryptedWarning, filename)) - - return err -} - type isFileStore interface { IsFileStore() bool GetFilename() string @@ -143,19 +137,15 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err creds := dockerCli.ConfigFile().GetCredentialsStore(serverAddress) - store, isDefault := creds.(isFileStore) - // Display a warning if we're storing the users password (not a token) - if isDefault && authConfig.Password != "" { - err = displayUnencryptedWarning(dockerCli, store.GetFilename()) - if err != nil { - return err - } - } - 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) } From fcefe44bda91e868c0d19d14b22a7bee22337224 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 18 Jul 2024 18:07:52 +0200 Subject: [PATCH 2/2] login: slightly cleanup warning about unencrypted store - Add an empty line before the warning to separate it from the command's output - Use the `/go/` redirect URL that we have available. - Put quotes around the filename used for storage. - Use present tense for the message, as the message is printed while saving. - User "credentials" instead of "password" for consistency with "credentials-store" Before: docker login myregistry.example.com Username: thajeztah Password: WARNING! Your password will be stored unencrypted in /root/.docker/config.json. Configure a credential helper to remove this warning. See https://docs.docker.com/engine/reference/commandline/login/#credential-stores Login Succeeded After: docker login myregistry.example.com Username: thajeztah Password: WARNING! Your credentials are stored unencrypted in '/root/.docker/config.json'. Configure a credential helper to remove this warning. See https://docs.docker.com/go/credential-store/ Login Succeeded Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index b82f0b8e79..f76c0a1cdc 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -23,9 +23,10 @@ import ( // 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 password will be stored unencrypted in %s. +const unencryptedWarning = ` +WARNING! Your credentials are stored unencrypted in '%s'. Configure a credential helper to remove this warning. See -https://docs.docker.com/engine/reference/commandline/login/#credential-stores +https://docs.docker.com/go/credential-store/ ` type loginOptions struct {