From 925ea6c490333cda62a42ea20dbbf7a6eb387ca8 Mon Sep 17 00:00:00 2001 From: Christopher Scott Date: Tue, 13 Feb 2024 17:02:03 -0600 Subject: [PATCH] Handle error when pwsh does not exist language agnostic (#41910) --- sdk/identity/Azure.Identity/CHANGELOG.md | 1 + .../src/Credentials/AzurePowerShellCredential.cs | 11 ++++++----- sdk/identity/Azure.Identity/src/ProcessRunner.cs | 1 + .../tests/AzurePowerShellCredentialsTests.cs | 5 ++++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md index 8af1afc9ca13d..9cff38bc68455 100644 --- a/sdk/identity/Azure.Identity/CHANGELOG.md +++ b/sdk/identity/Azure.Identity/CHANGELOG.md @@ -7,6 +7,7 @@ ### Breaking Changes ### Bugs Fixed +- `AzurePowerShellCredential` now handles the case where it falls back to legacy powershell without relying on the error message string. ### Other Changes diff --git a/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs index fff09cee106a1..e9fb0041ae779 100644 --- a/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs +++ b/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs @@ -159,7 +159,7 @@ private async ValueTask RequestAzurePowerShellAccessTokenAsync(bool try { output = async ? await processRunner.RunAsync().ConfigureAwait(false) : processRunner.Run(); - CheckForErrors(output); + CheckForErrors(output, processRunner.ExitCode); ValidateResult(output); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) @@ -168,7 +168,7 @@ private async ValueTask RequestAzurePowerShellAccessTokenAsync(bool } catch (InvalidOperationException exception) { - CheckForErrors(exception.Message); + CheckForErrors(exception.Message, processRunner.ExitCode); if (_isChainedCredential) { throw new CredentialUnavailableException($"{AzurePowerShellFailedError} {exception.Message}"); @@ -181,9 +181,10 @@ private async ValueTask RequestAzurePowerShellAccessTokenAsync(bool return DeserializeOutput(output); } - private static void CheckForErrors(string output) + private static void CheckForErrors(string output, int exitCode) { - bool noPowerShell = (output.IndexOf("not found", StringComparison.OrdinalIgnoreCase) != -1 || + int notFoundExitCode = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 9009 : 127; + bool noPowerShell = (exitCode == notFoundExitCode || output.IndexOf("not found", StringComparison.OrdinalIgnoreCase) != -1 || output.IndexOf("is not recognized", StringComparison.OrdinalIgnoreCase) != -1) && // If the error contains AADSTS, this should be treated as a general error to be bubbled to the user output.IndexOf("AADSTS", StringComparison.OrdinalIgnoreCase) == -1; @@ -264,7 +265,7 @@ private void GetFileNameAndArguments(string resource, string tenantId, out strin if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { fileName = Path.Combine(DefaultWorkingDirWindows, "cmd.exe"); - argument = $"/d /c \"{powershellExe} \"{commandBase64}\" \""; + argument = $"/d /c \"{powershellExe} \"{commandBase64}\" \" & exit"; } else { diff --git a/sdk/identity/Azure.Identity/src/ProcessRunner.cs b/sdk/identity/Azure.Identity/src/ProcessRunner.cs index d92d0654d4dbf..cabedd438e9a5 100644 --- a/sdk/identity/Azure.Identity/src/ProcessRunner.cs +++ b/sdk/identity/Azure.Identity/src/ProcessRunner.cs @@ -23,6 +23,7 @@ internal sealed class ProcessRunner : IDisposable private readonly CancellationTokenSource _timeoutCts; private CancellationTokenRegistration _ctRegistration; private bool _logPII; + public int ExitCode => _process.ExitCode; public ProcessRunner(IProcess process, TimeSpan timeout, bool logPII, CancellationToken cancellationToken) { diff --git a/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs b/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs index f683d1b72ce74..33905e0883af3 100644 --- a/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs +++ b/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs @@ -119,6 +119,7 @@ private static IEnumerable FallBackErrorScenarios() yield return new object[] { "'pwsh' is not recognized", AzurePowerShellCredential.PowerShellNotInstalledError }; yield return new object[] { "pwsh: command not found", AzurePowerShellCredential.PowerShellNotInstalledError }; yield return new object[] { "pwsh: not found", AzurePowerShellCredential.PowerShellNotInstalledError }; + yield return new object[] { "foo bar", AzurePowerShellCredential.PowerShellNotInstalledError }; } [Test] @@ -127,7 +128,9 @@ public void AuthenticateWithAzurePowerShellCredential_FallBackErrorScenarios(str { // This will require two processes on Windows and one on other platforms // Purposefully stripping out the second process to ensure any attempt to fallback is caught on non-Windows - TestProcess[] testProcesses = new TestProcess[] { new TestProcess { Error = errorMessage }, new TestProcess { Error = errorMessage } }; + int exitCode = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 9009 : 127; + + TestProcess[] testProcesses = new TestProcess[] { new TestProcess { Error = errorMessage, CodeOnExit = exitCode }, new TestProcess { Error = errorMessage, CodeOnExit = exitCode } }; if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) testProcesses = new TestProcess[] { testProcesses[0] };