Closed dethancosta closed 10 months ago
Note that from the Docker binary's perspective, the password is being entered in plaintext on the command line, so it will emit a warning. The Docker binary provides the --password-stdin flag for passing in the password to stdin, but I'm not sure that this is possible from within the provisioner code. Another method that Docker provides is through a credential helper, which would require a rewrite of op-cli to add the necessary subcommands.
Thank you very much for your contribution!
It doesn't look like this approach will work consistently. We like the third option you suggested, and have even sketched out a solution in this comment: https://github.com/1Password/shell-plugins/pull/301#issuecomment-1702406762
I don't think this will require work on the cli side, it should be possible with just shell-plugins, but that's something that would be figured out once someone really starts to dig into this. If you don't want to take this on, it's on our radar to add but not in the immediate future, in case that influences your decision.
Ah, apologies, I didn't realize there was a previous PR with the same approach. I'll take a crack at the sketched out solution possibly after the weekend, if that's alright.
Absolutely, that would be awesome!
Alright, I think I've implemented all the credential helper functionality, but with a few issues/caveats that I'm not sure how to resolve:
provision
sub-package's Add*File, otherwise the docker binary hangs. Because of this, I couldn't include the files in the expected sections of the test file. This also means that the config file needs to be deleted or temporarily removed prior to the provisioner running.~/.docker/config.json
is a non-existent file or directory.needsAuth.ForCommand("login","push","pull")
is present, even though 'docker login' is being callederror unsupported operation
as docker tries to save the credentials; I'm unsure of what the desired behaviour would be for this, it could either be changed to not emit anything and return exit code 0, or to emit a message to the user that the credentials don't need to be saved (or whatever message is appropriate)This PR likely isn't mergeable, but I figured I'd at least document the issues I ran into attempting this approach.
Thank you so much for taking the time to try this out! You're making a compelling case for docker to be added as a plugin internally, for now I'm going to close this PR and anyone who picks it up can reference it.
Overview
Adds a plugin to authenticate the Docker CLI using a username and secret (a password or an access token).
Type of change
- [x] Created a new plugin - [ ] Improved an existing plugin - [ ] Fixed a bug in an existing plugin - [ ] Improved contributor utilities or experience ## Related Issue(s) * Resolves: #114 ## How To Testdocker login
Changelog
Authenticate to a Docker registry using Touch ID and other unlock options with 1Password Shell Plugins.