-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add support to login to ACR with current identity #2245
Conversation
Definitely a good change in the right direction. I left some comments, but nothing blocking. Mostly curious about the strategy for removing the admin credentials path, if it is indeed something we don't want to support long-term. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely a good change in the right direction. I left some comments, but nothing blocking.
Mostly curious about the strategy for removing the admin credentials path, if it is indeed something we don't want to support long-term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement! I agree with Wei's feedback about trying to use the azure pipeline here instead of the default HTTP Client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. If needed, let's make sure to follow-up on how user input validation is handled for loginServer
.
2638103
to
b00fdd0
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference (preview)
|
With the change from #2245 we are now able to disable the ACR admin user by default. This is a security best practice. More information available in azure docs
Today
azd
only support logging in with ACR admin user. This is a bad practice and a security vulnerability.This PR addresses this issue by using the current identity to push containers.
This approach leverages exchanging an AAD token for an ACR refresh token based on the published ACR docs
Other enhancements
--password-stdin
instead of using--password
param on docker CLIResolves #2244