Skip to content

Commit 5ec3a56

Browse files
authored
Resolving Error Checks for AzurePowerShellCredential Fall Back (#30214)
1 parent ffb2f96 commit 5ec3a56

File tree

3 files changed

+36
-12
lines changed

3 files changed

+36
-12
lines changed

sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.ComponentModel;
65
using System.Diagnostics;
76
using System.Globalization;
87
using System.IO;
@@ -38,7 +37,6 @@ public class AzurePowerShellCredential : TokenCredential
3837
private const string DefaultWorkingDirNonWindows = "/bin/";
3938
private static readonly string DefaultWorkingDir = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? DefaultWorkingDirWindows : DefaultWorkingDirNonWindows;
4039
private readonly string _tenantId;
41-
private const int ERROR_FILE_NOT_FOUND = 2;
4240
private readonly bool _logPII;
4341
private readonly bool _logAccountDetails;
4442
internal const string AzurePowerShellNotLogInError = "Please run 'Connect-AzAccount' to set up account.";
@@ -105,7 +103,10 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
105103
}
106104
return scope.Succeeded(token);
107105
}
108-
catch (Win32Exception ex) when (ex.NativeErrorCode == ERROR_FILE_NOT_FOUND && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
106+
// External execution is wrapped in a "cmd /c" command which will never throw a native Win32Exception ERROR_FILE_NOT_FOUND
107+
// Check against the message for constant PowerShellNotInstalledError
108+
// Do not retry if already using legacy PowerShell to prevent delays, also used in tests to ensure a single process result
109+
catch (CredentialUnavailableException ex) when (UseLegacyPowerShell == false && ex.Message == PowerShellNotInstalledError && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
109110
{
110111
UseLegacyPowerShell = true;
111112
try
@@ -176,10 +177,6 @@ private static void CheckForErrors(string output)
176177
{
177178
throw new CredentialUnavailableException(AzurePowerShellModuleNotInstalledError);
178179
}
179-
if (output.IndexOf("is not recognized as an internal or external command", StringComparison.OrdinalIgnoreCase) != -1)
180-
{
181-
throw new Win32Exception(ERROR_FILE_NOT_FOUND);
182-
}
183180

184181
var needsLogin = output.IndexOf(RunConnectAzAccountToLogin, StringComparison.OrdinalIgnoreCase) != -1 ||
185182
output.IndexOf(NoAccountsWereFoundInTheCache, StringComparison.OrdinalIgnoreCase) != -1 ||

sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Azure.Core.TestFramework;
1212
using Azure.Identity.Tests.Mock;
1313
using NUnit.Framework;
14+
using System.Runtime.InteropServices;
1415

1516
namespace Azure.Identity.Tests
1617
{
@@ -73,9 +74,6 @@ public async Task AuthenticateWithAzurePowerShellCredential(
7374

7475
private static IEnumerable<object[]> ErrorScenarios()
7576
{
76-
yield return new object[] { "'pwsh' is not recognized", AzurePowerShellCredential.PowerShellNotInstalledError };
77-
yield return new object[] { "pwsh: command not found", AzurePowerShellCredential.PowerShellNotInstalledError };
78-
yield return new object[] { "pwsh: not found", AzurePowerShellCredential.PowerShellNotInstalledError };
7977
yield return new object[] { "Run Connect-AzAccount to login", AzurePowerShellCredential.AzurePowerShellNotLogInError };
8078
yield return new object[] { "NoAzAccountModule", AzurePowerShellCredential.AzurePowerShellModuleNotInstalledError };
8179
yield return new object[] { "Get-AzAccessToken: Run Connect-AzAccount to login.", AzurePowerShellCredential.AzurePowerShellNotLogInError };
@@ -94,6 +92,32 @@ public void AuthenticateWithAzurePowerShellCredential_ErrorScenarios(string erro
9492
Assert.AreEqual(expectedError, ex.Message);
9593
}
9694

95+
/// <summary>
96+
/// Requires 2 TestProcess results falling back from pwsh to powershell on Windows
97+
/// </summary>
98+
private static IEnumerable<object[]> FallBackErrorScenarios()
99+
{
100+
yield return new object[] { "'pwsh' is not recognized", AzurePowerShellCredential.PowerShellNotInstalledError };
101+
yield return new object[] { "pwsh: command not found", AzurePowerShellCredential.PowerShellNotInstalledError };
102+
yield return new object[] { "pwsh: not found", AzurePowerShellCredential.PowerShellNotInstalledError };
103+
}
104+
105+
[Test]
106+
[TestCaseSource(nameof(FallBackErrorScenarios))]
107+
public void AuthenticateWithAzurePowerShellCredential_FallBackErrorScenarios(string errorMessage, string expectedError)
108+
{
109+
// This will require two processes on Windows and one on other platforms
110+
// Purposefully stripping out the second process to ensure any attempt to fallback is caught on non-Windows
111+
TestProcess[] testProcesses = new TestProcess[] { new TestProcess { Error = errorMessage }, new TestProcess { Error = errorMessage } };
112+
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
113+
testProcesses = new TestProcess[] { testProcesses[0] };
114+
115+
AzurePowerShellCredential credential = InstrumentClient(
116+
new AzurePowerShellCredential(new AzurePowerShellCredentialOptions(), CredentialPipeline.GetInstance(null), new TestProcessService(testProcesses)));
117+
var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
118+
Assert.AreEqual(expectedError, ex.Message);
119+
}
120+
97121
[Test]
98122
public async Task HandlesAlternateDateTimeFormats([Values("en-US", "nl-NL")] string culture)
99123
{
@@ -151,7 +175,10 @@ public void AuthenticateWithAzurePowerShellCredential_PowerShellNotInstalled(
151175
{
152176
var testProcess = new TestProcess { Error = errorMessage };
153177
AzurePowerShellCredential credential = InstrumentClient(
154-
new AzurePowerShellCredential(new AzurePowerShellCredentialOptions(), CredentialPipeline.GetInstance(null), new TestProcessService(testProcess)));
178+
new AzurePowerShellCredential(new AzurePowerShellCredentialOptions(), CredentialPipeline.GetInstance(null), new TestProcessService(testProcess))
179+
{
180+
UseLegacyPowerShell = true
181+
});
155182
var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
156183
Assert.AreEqual(AzurePowerShellCredential.PowerShellNotInstalledError, ex.Message);
157184
}

sdk/identity/Azure.Identity/tests/DefaultAzureCredentialLiveTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public void DefaultAzureCredential_AllCredentialsHaveFailed_CredentialUnavailabl
221221
});
222222

223223
var vscAdapter = new TestVscAdapter(ExpectedServiceName, "AzureCloud", "{}");
224-
var factory = new TestDefaultAzureCredentialFactory(options, new TestFileSystemService(), new TestProcessService(new TestProcess { Error = "'az' is not recognized" }, new TestProcess{Error = "'PowerShell' is not recognized"}), vscAdapter);
224+
var factory = new TestDefaultAzureCredentialFactory(options, new TestFileSystemService(), new TestProcessService(new TestProcess { Error = "'az' is not recognized" }, new TestProcess{Error = "'PowerShell' is not recognized"}, new TestProcess{Error = "'PowerShell' is not recognized"}), vscAdapter);
225225
var credential = InstrumentClient(new DefaultAzureCredential(factory, options));
226226

227227
List<ClientDiagnosticListener.ProducedDiagnosticScope> scopes;

0 commit comments

Comments
 (0)