Azure / ms-rest-nodeauth

node.js based authentication library for Azure with type definitions
MIT License
33 stars 33 forks source link

Breaking Change for AzureCliCredentials #122

Closed thomasmillergb closed 3 years ago

thomasmillergb commented 3 years ago

Describe the bug I have just run a npm install using this package to find a nasty surprise that i cannot log in with AzureCliCredentials.create({resource}) for mac and linux (not tested windows)_

Looking at the source code i have found that a bug in the following commit https://github.com/Azure/ms-rest-nodeauth/commit/1b8dcfb6d7e1c8a0a00044286021a841961cf636

 exec(`az ${cmd} --out json`, { encoding: "utf8" }, (error, stdout) => {
to
    execFile(`az`, [cmd, `--out json`], { encoding: "utf8" }, (error, stdout) => {

the issue being that execFile args with spaces does not work. Which i have also been able to reproduce from terminal

az "account show"
'account show' is misspelled or not recognized by the system.
Did you mean 'account' ?

However if i correct execFile without spaces, it works, not very nice looking

execFile(`az`, ['account', 'show', '--out', 'json']

To Reproduce Steps to reproduce the behavior:

await AzureCliCredentials.create({resource});

Error: Command failed: az account show --out json
ERROR: 'account show' is misspelled or not recognized by the system.
Did you mean 'account' ?

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

Expected behavior It should execute the commands without error, as this impacted all

work around

I have downgraded my package

ramya-rao-a commented 3 years ago

Thanks for reporting @thomasmillergb!

Can you try out the changes in https://github.com/Azure/ms-rest-nodeauth/pull/123?

In my local box which is Windows, the command fails as it cannot find the az command in my path. While I try and fix that, I'd appreciate it if you can try out the changes in https://github.com/Azure/ms-rest-nodeauth/pull/123 on your end.

ramya-rao-a commented 3 years ago

Alright, fixed the Windows issue in the PR. Turns out we need to use az.cmd and not just az because execFile needs the file, not just the command.

ramya-rao-a commented 3 years ago

Rolled out the fix in version 3.0.9 of this package. Please give it a try