Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[Identity] Check exit code for PowershellCredential
Signed-off-by: Paul Van Eck <[email protected]>
  • Loading branch information
pvaneck committed Feb 12, 2024
commit d2b8a907340531364ee4529dc969c38ce718402d
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

- Fixed an issue in `AzurePowerShellCredential` where if `pwsh` isn't available and the Command Prompt language is not English, it would not fall back to `powershell`. ([#34271](https://github.com/Azure/azure-sdk-for-python/pull/34271))

### Other Changes

## 1.16.0b1 (2024-02-06)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def run_command_line(command_line: List[str], timeout: int) -> str:
try:
proc = start_process(command_line)
stdout, stderr = proc.communicate(**kwargs)
if sys.platform.startswith("win") and "' is not recognized" in stderr:
if sys.platform.startswith("win") and ("' is not recognized" in stderr or proc.returncode == 9009):
# pwsh.exe isn't on the path; try powershell.exe
command_line[-1] = command_line[-1].replace("pwsh", "powershell", 1)
proc = start_process(command_line)
Expand Down Expand Up @@ -192,7 +192,7 @@ def get_command_line(scopes: Tuple[str, ...], tenant_id: str) -> List[str]:

command = "pwsh -NoProfile -NonInteractive -EncodedCommand " + encoded_script
if sys.platform.startswith("win"):
return ["cmd", "/c", command]
return ["cmd", "/c", command + " & exit"]
return ["/bin/sh", "-c", command]


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ async def run_command_line(command_line: List[str], timeout: int) -> str:
try:
proc = await start_process(command_line)
stdout, stderr = await asyncio.wait_for(proc.communicate(), 10)
if sys.platform.startswith("win") and b"' is not recognized" in stderr:
if sys.platform.startswith("win") and (b"' is not recognized" in stderr or proc.returncode == 9009):
# pwsh.exe isn't on the path; try powershell.exe
command_line[-1] = command_line[-1].replace("pwsh", "powershell", 1)
proc = await start_process(command_line)
Expand Down
13 changes: 10 additions & 3 deletions sdk/identity/azure-identity/tests/test_powershell_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,14 @@ def emit(self, record):
assert False, "Credential should have included stderr in a DEBUG level message"


def test_windows_powershell_fallback():
@pytest.mark.parametrize(
"error_message",
(
"'pwsh' is not recognized as an internal or external command,\r\noperable program or batch file.",
"some other message",
),
)
def test_windows_powershell_fallback(error_message):
"""On Windows, the credential should fall back to powershell.exe when pwsh.exe isn't on the path"""

class Fake:
Expand All @@ -262,8 +269,8 @@ def Popen(args, **kwargs):
if args[-1].startswith("pwsh"):
assert Fake.calls == 1, 'credential should invoke "pwsh" only once'
stdout = ""
stderr = "'pwsh' is not recognized as an internal or external command,\r\noperable program or batch file."
return_code = 1
stderr = error_message
return_code = 9009
else:
assert args[-1].startswith("powershell"), 'credential should fall back to "powershell"'
stdout = NO_AZ_ACCOUNT_MODULE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,14 @@ async def test_windows_event_loop():


@pytest.mark.skipif(not sys.platform.startswith("win"), reason="tests Windows-specific behavior")
async def test_windows_powershell_fallback():
@pytest.mark.parametrize(
"error_message",
(
"'pwsh' is not recognized as an internal or external command,\r\noperable program or batch file.",
"some other message",
),
)
async def test_windows_powershell_fallback(error_message):
"""On Windows, the credential should fall back to powershell.exe when pwsh.exe isn't on the path"""

calls = 0
Expand All @@ -259,8 +266,8 @@ async def mock_exec(*args, **kwargs):
if args[-1].startswith("pwsh"):
assert calls == 1, 'credential should invoke "pwsh" only once'
stdout = ""
stderr = "'pwsh' is not recognized as an internal or external command,\r\noperable program or batch file."
return_code = 1
stderr = error_message
return_code = 9009
else:
assert args[-1].startswith("powershell"), 'credential should fall back to "powershell"'
stdout = NO_AZ_ACCOUNT_MODULE
Expand All @@ -270,10 +277,11 @@ async def mock_exec(*args, **kwargs):
communicate = Mock(return_value=get_completed_future((stdout.encode(), stderr.encode())))
return Mock(communicate=communicate, returncode=return_code)

credential = AzurePowerShellCredential()
with pytest.raises(CredentialUnavailableError, match=AZ_ACCOUNT_NOT_INSTALLED):
with patch(CREATE_SUBPROCESS_EXEC, mock_exec):
await credential.get_token("scope")
with patch(AzurePowerShellCredential.__module__ + ".sys.platform", "win32"):
credential = AzurePowerShellCredential()
with pytest.raises(CredentialUnavailableError, match=AZ_ACCOUNT_NOT_INSTALLED):
with patch(CREATE_SUBPROCESS_EXEC, mock_exec):
await credential.get_token("scope")

assert calls == 2

Expand Down