This was introduced in 79141ce5eb8f7c341b03aa50f7ce4b819de20720, which
was reverted in f5962021251c127326d654a440c8b6b675e2ba0d, and re-applied
in the previous commit.
Before this patch, saving credentials worked correctly;
docker login -u thajeztah
Password:
Login Succeeded
cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "REDACTED"
}
}
}
But when resolving the credentials, the credentials stored would not be found;
docker pull -q thajeztah/private-test-image
Error response from daemon: pull access denied for thajeztah/private-test-image, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
With this patch applied:
docker pull -q thajeztah/private-test-image
docker.io/thajeztah/private-test-image:latest
Thanks to mtrmac (Miloslav Trmač) for spotting this mistake!
Suggested-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit f5962021251c127326d654a440c8b6b675e2ba0d, and reapplies
79141ce5eb8f7c341b03aa50f7ce4b819de20720.
> cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo
>
> Re-implement locally, based on the code in github.com/docker/docker/registry,
> but leaving out bits that are not used on the client-side, such as
> configuration of Mirrors, and configurable insecure-registry, which
> are not used on the client side.
This commit contains a regression due to a typo in `authConfigKey`;
const authConfigKey = "https:/index.docker.io/v1/"
Which is missing a `/` after the scheme.
Which currently fails the TestRetrieveAuthTokenFromImage test.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Re-implement locally, based on the code in github.com/docker/docker/registry,
but leaving out bits that are not used on the client-side, such as
configuration of Mirrors, and configurable insecure-registry, which
are not used on the client side.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Users have trouble understanding the different login paths on the CLI.
The default login is performed through an OAuth flow with the option to
fallback to a username and PAT login using the docker login -u <username>
option.
This patch improves the text around docker login, indicating:
- The username is shown when already authenticated
- Steps the user can take to switch user accounts are printed when
authenticated in an info.
- When not authenticated, the OAuth login flow explains the fallback
clearly to the user in an info.
- The password prompt now explicitly states that it accepts a PAT in an
info.
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
This message resulted in code-lines that were too long; move it to a
const together with the other hint. While at it, also suppress unhandled
error, and touch-up the code-comment.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Deprecation comments must have an empty line before them, otherwise tools
and linters may not recognise them. While fixing this, also updated the
reference to PromptUserForCredentials to be a docs-link to make it clickable.
Updates 6e4818e7d6d006f14ebac4c06fbe6ed958237408.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If restoring the terminal state fails, "echo" no longer works, which means
that anything the user types is no longer shown. The login itself may already
have succeeded, so we should not fail the command, but it's good to inform
the user that this happened, which may give them a clue why things no longer
work as they expect them to work.
With this patch:
docker login -u yourname
Password:
Error: failed to restore terminal state to echo input: something bad happened
Login Succeeded
We should consider printing instructions how to restore this manually (other
than restarting the shell). e.g., 'run stty echo' when in a Linux or macOS shell,
but PowerShell and CMD.exe may need different instructions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
we don't support empty passwords; when prompting the user for a password,
we already trim the result, but we didn't do the same for a password that's
passed through stdin or through the `-p` / `--password` flag.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- move trimming defaultUsername inside the if-branch, as it's the only
location where the result of the trimmed username is use.
- do the reverse for trimming argUser, because the result of trimming
argUser is used outside of the if-branch (not just for the condition).
putting it inside the condition makes it easy to assume the result is
only used locally.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
move the "post" check for username being empty inside the branch
that's handling the username, as it's the only branch where username
is mutated after checking if it's empty.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
remove isDefaultRegistry and inline it where it's used; the code-comment
already outlines what we're looking for, so the intermediate var didn't
add much currently.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function has multiple conditional branches, which makes it harder
to see at a glance whether authConfig may be partially populated. This
patch instead returns a fresh instance for error returns to prevent any
confusion.
It also removes the named output variables, as they're now no longer used,
and the returned types should already be descriptive enough to understand
what's returned.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: 2269acc7a3...164cae56ed
Co-authored-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Running `docker login` in a non-interactive environment sometimes errors
out if no username/pwd is provided. This handling is somewhat
inconsistent – this commit addresses that.
Before:
| `--username` | `--password` | Result |
|:------------:|:------------:| ------------------------------------------------------------------ |
| ✅ | ✅ | ✅ |
| ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ❌ | ✅ | hangs |
After:
| `--username` | `--password` | Result |
|:------------:|:------------:| ------------------------------------------------------------------ |
| ✅ | ✅ | ✅ |
| ❌ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ✅ | ❌ | `Error: Cannot perform an interactive login from a non TTY device` |
| ❌ | ✅ | `Error: Cannot perform an interactive login from a non TTY device` |
It's worth calling out a separate scenario – if there are previous,
valid credentials, then running `docker login` with no username or
password provided will use the previously stored credentials, and not
error out.
```console
cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "xxxxxxxxxxx"
}
}
}
⭑ docker login 0>/dev/null
Authenticating with existing credentials...
Login Succeeded
```
This commit also applies the same non-interactive handling logic to the
new web-based login flow, which means that now, if there are no prior
credentials stored and a user runs `docker login`, instead of initiating
the new web-based login flow, an error is returned.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Commit 27b2797f7deb3ca5b7f80371d825113deb1faca1 added a local implementation
of this function, so let's use the local variant to (slightly) reduce the
dependency on moby's registry package.
Also made some minor cleanups.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both these functions took the whole DockerCLI as argument, but only needed
the ConfigFile. ResolveAuthConfig also had an unused context.Context as
argument.
This patch updates both functions to accept a ConfigFile, and removes the
unused context.Context.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was deprecated in b87ed343510b1cf71d1aba02294f52268f754386,
which is part of the v24.0 release, so we can remove it from master.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Deprecate this function in favor of the implementation in the API types,
considering that to be the canonical implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Replace uses of this function in favor of the implementation in the
API types, so that we have a single, canonical implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
changes readInput() to trim whitespace. The existing code tried to be
conservative and only trimmed whitespace for username (not for password).
Passwords with leading/trailing whitespace would be _very_ unlikely, and
trimming whitespace is generally accepted.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ConfigureAuth used the readInput() utility to read the username and password.
However, this utility did not return errors it encountered, but instead did
an os.Exit(1). A result of this was that the terminal was not restored if
an error happened. When reading the password, the terminal is configured to
disable echo (i.e. characters are not printed), and failing to restore
the previous state means that the terminal is now "non-functional".
This patch:
- changes readInput() to return errors it encounters
- uses a defer() to restore terminal state
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function no longer uses the /info endpoint to resolve the registry
to use. The documentation for this function was still referring to
the (once used) special registry for Windows images, which is no longer
in use, so update the docs to reflect reality :)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was deprecated in b4ca1c7368daeead400fcc1b8f2d61951a0d9d1e,
which is part of the v23.0 release, and is no longer used, so we can remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The comment was not formatted correctly, and because of that not picked up as
being deprecated.
updates b4ca1c7368daeead400fcc1b8f2d61951a0d9d1e
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The CLI currenly calls the `/info` endpoint to get the address
of the default registry to use.
This functionality was added as part of the initial Windows implementation
of the engine. For legal reasons, Microsoft Windows (and thus Docker images
based on Windows) were not allowed to be distributed through non-Microsoft
infrastructure. As a temporary solution, a dedicated "registry-win-tp3.docker.io"
registry was created to serve Windows images.
As a result, the default registry was no longer "fixed", so a helper function
(`ElectAuthServer`) was added to allow the CLI to get the correct registry
address from the daemon. (docker/docker PR's/issues 18019, 19891, 19973)
Using separate registries was not an ideal solution, and a more permanent
solution was created by introducing "foreign image layers" in the distribution
spec, after which the "registry-win-tp3.docker.io" ceased to exist, and
removed from the engine through docker/docker PR 21100.
However, the `ElectAuthServer` was left in place, quoting from that PR;
> make the client check which default registry the daemon uses is still
> more correct than leaving it up to the client, even if it won't technically
> matter after this PR. There may be some backward compatibility scenarios
> where `ElectAuthServer` [sic] is still helpful.
That comment was 5 years ago, and given that the engine and cli are
released in tandem, and the default registry is not configurable, we
can save the extra roundtrip to the daemon by using a fixed value.
This patch deprecates the `ElectAuthServer` function, and makes it
return the default registry without calling (potentially expensie)
`/info` API endpoint.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Samuel Karp <skarp@amazon.com>
(cherry picked from commit 42d1c02750b3631402da3973e5f36b76c8c934f4)
Signed-off-by: Samuel Karp <skarp@amazon.com>
Commit f32731f9020f12ae2600a3f4c90e668565220cb6 fixed a potential panic
when an error was returned while trying to get existing credentials.
However, other code paths currently use the result of `GetDefaultAuthConfig()`
even in an error condition; this resulted in a panic, because a `nil` was
returned.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These packages are now living in their own repository. Updating
docker/docker to replace the dependencies.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Instead of using an `if else if else`, switch to a sequence of independent
`if` blocks containing a `return`.
Instead of defining a return variable and updating it in the `if` blocks
and returning at the end, make each `if` block return the desired value
independenly.
Signed-off-by: David Scott <dave.scott@docker.com>
Previously if the Docker engine was not running the behaviour of
commands would vary depending on whether the --debug flag was provided.
For example, consider `docker logout`:
$ docker logout
Not logged in to
-- note the missing server URL
$ docker --debug logout
Warning: failed to get default registry endpoint from daemon (Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?). Using system default: https://index.docker.io/v1/
Not logged in to https://index.docker.io/v1/
-- note the server URL is present
This patch makes only the debug printing conditional on the `--debug` flag,
not the return value.
Signed-off-by: David Scott <dave.scott@docker.com>
This new collection of commands supports initializing a local
engine using containerd, updating that engine, and activating
the EE product
Signed-off-by: Daniel Hiltgen <daniel.hiltgen@docker.com>
Since go 1.7, "context" is a standard package. Since go 1.9,
x/net/context merely provides some types aliased to those in
the standard context package.
The changes were performed by the following script:
for f in $(git ls-files \*.go | grep -v ^vendor/); do
sed -i 's|golang.org/x/net/context|context|' $f
goimports -w $f
for i in 1 2; do
awk '/^$/ {e=1; next;}
/\t"context"$/ {e=0;}
{if (e) {print ""; e=0}; print;}' < $f > $f.new && \
mv $f.new $f
goimports -w $f
done
done
[v2: do awk/goimports fixup twice]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
