Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

login: slightly cleanup warning about unencrypted store #5258

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 18, 2024

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.

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

- A picture of a cute animal (not mandatory but encouraged)

…ials

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 <[email protected]>
@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Jul 18, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jul 18, 2024
- 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 <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.49%. Comparing base (07baebe) to head (fcefe44).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5258      +/-   ##
==========================================
- Coverage   61.49%   61.49%   -0.01%     
==========================================
  Files         299      299              
  Lines       20828    20822       -6     
==========================================
- Hits        12808    12804       -4     
+ Misses       7110     7109       -1     
+ Partials      910      909       -1     

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

@thaJeztah thaJeztah merged commit 26b412e into docker:master Jul 18, 2024
94 checks passed
@thaJeztah thaJeztah deleted the cleanup_unencrypted_warning branch July 18, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants