jason-johnson / azure-pipelines-tasks-terraform

Azure Pipelines extension for Terraform
MIT License
127 stars 55 forks source link

Terraform CLI: Run Azure CLI Login bugged with secret starting with dash #284

Open ste-camp opened 2 years ago

ste-camp commented 2 years ago

On a Terraform CLI Task, when "Run Azure CLI Login" flag is checked and you use a Service Principal to login to Azure, the login will fail if the Service Principal password/secret starts with a -

To Reproduce Steps to reproduce the behavior:

  1. Generate a new secret for your Service Principal (try multiple times until it generates one starting with a dash)
  2. Execute pipeline
  3. See error where it states that the az login command failed

Expected behavior Pipeline successfully executes az login.

Pipeline Logs ` [command]/usr/bin/az login --service-principal -t -u -p *** ERROR: argument --password/-p: expected one argument

Examples from AI knowledge base: az login --service-principal -u http://azure-cli-2016-08-05-14-31-15 -p VerySecret --tenant contoso.onmicrosoft.com Log in with a service principal using client secret. Use -p=secret if the first character of the password is '-'.

az login --service-principal -u http://azure-cli-2016-08-05-14-31-15 -p ~/mycertfile.pem --tenant contoso.onmicrosoft.com Log in with a service principal using client certificate.

az login -u johndoe@contoso.com -p VerySecret Log in with user name and password. This doesn't work with Microsoft accounts or accounts that have two-factor authentication enabled. Use -p=secret if the first character of the password is '-'.

https://aka.ms/cli_ref Read more about the command in reference docs `

Agent Configuration

Additional context As stated in the error itself, when the password starts with a dash the az login command should change syntax and use -p=SECRET instead of -p SECRET

jason-johnson commented 2 years ago

Can you verify that the login works when using az cli directly? Just to make sure the bug is here and not with az cli

stmpn commented 1 year ago

Hi, I wanted to chime in as I've stumbled onto this issue as well.

This is not a bug with terraform task, it's also not a bug in the CLI - this thread shouldn't be a bug report and rather a feature request.

Even when the bug itself shows up and in theory it should make the pipeline fail - it does not. There seems to be a failsafe on the azcli side, that in case there is a password starting with dash (so, on the first glance it looks like additional parameter right after "-p" flag), it manages to log in successfuly despite errors being thrown.

"Log in with a service principal using client secret. Use -p=secret if the first character of the password is '-'." which is clearly a log message produced by azcli suggest that azcli is smart enough to recognize situation like that and it probably retries, although this time with "-p=secret" syntaxt instead of "-p secret" hardcoded inside terraform task.

Back to the actual request: can you simply make "-p=secret" syntax the default one? It should resolve any issues caused by the secret starting with dash, while working exactly the same with "regular" secrets.

If changing the default behavior is not feasible, at least adding some kind of switch to force terraform task to use the different syntax would be greatly appreciated, as at the moment it paints the logs red, indicates issues but works anyways, which does not seem like intended behavior.

timothycrall commented 1 year ago

Also running into this problem. Since we use automatically generated service principal secrets, there's no way for us to ensure that they don't start with a dash. '-p=secret' should be the syntax used for logging in to account for this possibility.

Not sure what previous user is describing in terms of retries, but in my pipelines, it it very much failing the pipeline. So I would say that this really is a bug that breaks basic functionality (when the secret does happen to start with a dash).

jason-johnson commented 1 year ago

There is a PR already made. We just need to finish the migration to multiple versions (e.g. terraformCli@1, @0, etc.) so we're able to test PRs to make sure it didn't break anything else.

stewartgordonsitka commented 7 months ago

It looks like the previous PR was closed and then reverted back because it broke a test. Any chance this issue is still going to still get resolved in the future?

jason-johnson commented 7 months ago

Absolutely. The PR just needs to be modified to fix the tests so the build can go through.