Azure Contianer Registry login scenario discussion

[shhsu]

Hi Docker team:

I am opening this issue as a channel to discuss with docker team the best way forward with the Azure scenario. This issue is an extension of a closed PR. #105

To reiterate, we want to enable to user to take advantage of our AAD device login feature for docker. We are also hoping that the user would not need to install azure cli for this scenario. Namely, we want to user to be able to call "docker login" to login to an Azure Container Registry, but the login process would be Azure specific.

Following suggestions from @friism, I created an azure credential helper prototype which can be wrapped around any credential helper. https://github.com/shhsu/docker-acr-cred-helper/blob/master/program.go

In order for this component to work properly though, there are 3 changes that need to be made on the docker cli side:

1. Because azure credential helper interact with the user and prompt them to perform device login, we need to pipe the stderr to docker cli (Caller of cred helpers would now see the stderr returns from cred helpers docker-credential-helpers#64)
2. Azure credential helper should freely support any credential helper as its backend for storing the credentials. Since information can be written to the active docker config file, we need a way for the credential helper to know which config file is currently active. We could do this by setting the current cli config location in a DOCKER_CONFIG variable in the shell session when we invoke the credential helper
3. This is the most intrusive change: https://github.com/shhsu/cli-1/blob/v2_credhelper/cli/command/registry.go#L86-L103. When given no -p and -u during login, ConfigureAuth method currently always prompt user for username and password. The username retrieved from the cred store is only used as the default value during prompt and password is not used. This will not work for us because our credential helper has already produced a pair of username and password and prompting again breaks our scenario.

We'll like to open a conversation with docker team and see if you guys are open to allow Azure Container team make these 3 changes.

Alternatively, we are also thinking of introduce a new component of docker called docker-login-manager. Similar to docker-credential-helper, this login manager would be a plugin component. However, the only active that this login manager would perform is to get the AuthConfig object. The ConfigureAuth method would first go to the user configured login manager to login. If for any reason the login manager fails to retrieve the username and password, cli would then step in and prompt for username and password.

Please let us know what's the most suitable path forward.

Thanks
Peter

[shhsu]

@sajayantony @DavidObando

[sajayantony]

Bringing in @friism since @shhsu got the recommendation to use the credential manager and hit quite a few design issues when enabling interactive login with AAD with the approach as discussed above.
/cc @SteveLasker

[friism]

I'm sorry that I wasn't clear in my first message.

We are also hoping that the user would not need to install azure cli for this scenario. Namely, we want to user to be able to call "docker login" to login to an Azure Container Registry, but the login process would be Azure specific.

I don't think this is a good idea because it means that Docker will carry a bunch of Azure-specific login code. In my previous comment I detailed why I think that's the case:

Imagine, for example, that a security flaw was found in the Azure login flow. With the change you propose in Docker, you'd have to scramble to fix that in Azure tools and CLIs, and you'd have to get a fix into Docker and goad us into action to urgently ship the fix.

If we accepted similar contributions from other registry vendors then that would be hard to scale for us, and if you got similar changes into more software like Docker, it would be equally hard for you to scale fixing problems all the places that implemented Azure login.

Quoting from my previous comment:

Users would configure your credential-helper for a wildcard domain that matches all Azure registries, and then they can start doing docker push/pull without further ado. They just have to be logged in to Azure with azure login (or the PowerShell equivalent).

Please let me know if I missed anything.

[shhsu]

Thanks @friism
To be clear, we are not doing anything Azure specific in cli and docker-credential-helper. Going back to the list of 3 changes we are requesting to make, their only purpose is to allow a credential-helper to prompt/fetch user credential interactively though.

[sajayantony]

I believe the basic issue with the docker CLI credential manager is its inability to actually do any interactive operation and even if it does and is able to get new credentials the cli would prompt again.
So in short we need an extensibility point in the cli that would allow a any third party providers to prompt/return credentials and the CLI prompt needs to know not to prompt again and delete this to the underlying provider.

[friism]

@sajayantony @shhsu I think I understand the flow, but for the benefit of @thaJeztah and @nass, can you help me spell it out? I think it's something like:

1. User installs Docker
2. User uses standard Docker credential helper for their platform
3. User adds azure credential helper
4. User does docker login <some-azure-registry>
5. Docker delegates to azure credential helper
6. azure credential helper prompts user to visit azure device login website (like the azure cli does) and blocks
7. user visits azure device login website, completes login
8. azure credential helper unblocks and docker login completes
9. The resulting azure auth is cached in the standard (non-azure) credential helper

Please correct any of the above that I got wrong.

[shhsu]

@friism
This is correct.
Note that our azure auth token has a relatively short timeout period but can be refreshed, in our get operation in azure credential helper, we implemented a refresh mechanism.

Also, I have talked to @DavidObando offline about https://github.com/shhsu/cli-1/blob/v2_credhelper/cli/command/registry.go#L86-L103. As the code stands now, if the user does not provide -u, the cred store credential would always be used and user would never be prompted. While this code works perfectly for us because our cred store always ensure the correct token is returned, it might not be desirable for other cred store implementations:

Suppose in a typical cred store, user password expires. With the old code path, the user can just call azure login <registry> to login again and it would prompt for the new username and password. If my code change is used, the user would need to call azure logout first and then azure login

The new and more complicated proposal is that, if the user does not provide -u, docker would use always use the cred store credential to log in first if exists. Only when the login fails, would user be prompted. Basically login would perform a lot more similar as push, pull, etc..

I am working on that solution currently. I will update the thread once I have finished it.

[shhsu]

As discussed #139

[shhsu]

Closing this issue as we able to see a path forward