docker / cli

The Docker CLI
Apache License 2.0
4.89k stars 1.92k forks source link

Protocol should not be stripped out of the registry URL on login #190

Open n4ss opened 7 years ago

n4ss commented 7 years ago

When a user provides a registry URL to authenticate through docker login, we currently strip the protocol: https://github.com/docker/cli/blob/master/cli/command/registry.go#L85

Change has been introduced in: https://github.com/moby/moby/commit/67d752ac55869eab42280947ad37ff5e1468d458 but I can't see the purpose.

Implementation of ConvertToHostname is here: https://github.com/moby/moby/blob/8874f80e67c560f44322233bfc22ecd86b85e9e2/registry/auth.go#L212

All these commands are treated the same way on the credentials-storing level: > docker login https://foo.bar > docker login http://foo.bar > docker login foo.bar

And docker currently appends an "http" after stripping the protocol, which is not okay at all.

Decisions to store them as HTTP or HTTPS should not be done by the underlying docker-credential-helpers API.

This needs to be fixed very soon as we made the decision to default to HTTPS when no protocol is present and that happens when you specify docker login http://foo.bar too..

thaJeztah commented 7 years ago

ping @runcom perhaps you recall the reason for this change

runcom commented 7 years ago

The whole conversation on why we needed this starts here https://github.com/moby/moby/pull/23100#issuecomment-223066185