Skip to content

Commit

Permalink
AzureCliCredential is unavailable when no account is logged in (Azure…
Browse files Browse the repository at this point in the history
  • Loading branch information
chlowell authored Jun 5, 2020
1 parent bce84db commit 717c419
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 16 deletions.
3 changes: 3 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Release History

## 1.4.0b4 (Unreleased)
- `AzureCliCredential` raises `CredentialUnavailableError` when no user is
logged in to the Azure CLI.
([#11819](https://github.com/Azure/azure-sdk-for-python/issues/11819))
- `AzureCliCredential` and `VSCodeCredential`, which enable authenticating as
the identity signed in to the Azure CLI and Visual Studio Code, respectively,
can be imported from `azure.identity` and `azure.identity.aio`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

CLI_NOT_FOUND = "Azure CLI not found on path"
COMMAND_LINE = "az account get-access-token --output json --resource {}"
NOT_LOGGED_IN = "Please run 'az login' to set up an account"

# CLI's "expiresOn" is naive, so we use this naive datetime for the epoch to calculate expires_on in UTC
EPOCH = datetime.fromtimestamp(0)
Expand Down Expand Up @@ -116,6 +117,8 @@ def _run_command(command):
# non-zero return from shell
if ex.returncode == 127 or ex.output.startswith("'az' is not recognized"):
error = CredentialUnavailableError(message=CLI_NOT_FOUND)
elif "az login" in ex.output or "az account set" in ex.output:
error = CredentialUnavailableError(message=NOT_LOGGED_IN)
else:
# return code is from the CLI -> propagate its output
if ex.output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
CLI_NOT_FOUND,
COMMAND_LINE,
get_safe_working_dir,
NOT_LOGGED_IN,
parse_token,
sanitize_output,
)
Expand Down Expand Up @@ -83,5 +84,8 @@ async def _run_command(command):
if proc.returncode == 127 or output.startswith("'az' is not recognized"):
raise CredentialUnavailableError(CLI_NOT_FOUND)

if "az login" in output or "az account set" in output:
raise CredentialUnavailableError(message=NOT_LOGGED_IN)

message = sanitize_output(output) if output else "Failed to invoke Azure CLI"
raise ClientAuthenticationError(message=message)
13 changes: 11 additions & 2 deletions sdk/identity/azure-identity/tests/test_cli_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import json

from azure.identity import AzureCliCredential, CredentialUnavailableError
from azure.identity._credentials.azure_cli import CLI_NOT_FOUND
from azure.identity._credentials.azure_cli import CLI_NOT_FOUND, NOT_LOGGED_IN
from azure.core.exceptions import ClientAuthenticationError

import subprocess
Expand Down Expand Up @@ -98,10 +98,19 @@ def test_cannot_execute_shell():


def test_not_logged_in():
"""When the CLI isn't logged in, the credential should raise an error containing the CLI's output"""
"""When the CLI isn't logged in, the credential should raise CredentialUnavailableError"""

output = "ERROR: Please run 'az login' to setup account."
with mock.patch(CHECK_OUTPUT, raise_called_process_error(1, output)):
with pytest.raises(CredentialUnavailableError, match=NOT_LOGGED_IN):
AzureCliCredential().get_token("scope")


def test_unexpected_error():
"""When the CLI returns an unexpected error, the credential should raise an error containing the CLI's output"""

output = "something went wrong"
with mock.patch(CHECK_OUTPUT, raise_called_process_error(42, output)):
with pytest.raises(ClientAuthenticationError, match=output):
AzureCliCredential().get_token("scope")

Expand Down
28 changes: 14 additions & 14 deletions sdk/identity/azure-identity/tests/test_cli_credential_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from azure.identity import CredentialUnavailableError
from azure.identity.aio import AzureCliCredential
from azure.identity._credentials.azure_cli import CLI_NOT_FOUND
from azure.identity._credentials.azure_cli import CLI_NOT_FOUND, NOT_LOGGED_IN
from azure.core.exceptions import ClientAuthenticationError
import pytest

Expand All @@ -18,6 +18,8 @@

SUBPROCESS_EXEC = AzureCliCredential.__module__ + ".asyncio.create_subprocess_exec"

pytestmark = pytest.mark.asyncio


def mock_exec(stdout, stderr="", return_code=0):
async def communicate():
Expand All @@ -27,30 +29,26 @@ async def communicate():
return mock.Mock(return_value=get_completed_future(process))


@pytest.mark.asyncio
async def test_no_scopes():
"""The credential should raise ValueError when get_token is called with no scopes"""

with pytest.raises(ValueError):
await AzureCliCredential().get_token()


@pytest.mark.asyncio
async def test_multiple_scopes():
"""The credential should raise ValueError when get_token is called with more than one scope"""

with pytest.raises(ValueError):
await AzureCliCredential().get_token("one scope", "and another")


@pytest.mark.asyncio
async def test_close():
"""The credential must define close, although it's a no-op because the credential has no transport"""

await AzureCliCredential().close()


@pytest.mark.asyncio
async def test_context_manager():
"""The credential must be a context manager, although it does nothing as one because it has no transport"""

Expand All @@ -59,7 +57,6 @@ async def test_context_manager():


@pytest.mark.skipif(not sys.platform.startswith("win"), reason="tests Windows-specific behavior")
@pytest.mark.asyncio
async def test_windows_fallback():
"""The credential should fall back to the sync implementation when not using ProactorEventLoop on Windows"""

Expand All @@ -74,7 +71,6 @@ async def test_windows_fallback():
assert sync_get_token.call_count == 1


@pytest.mark.asyncio
async def test_get_token():
"""The credential should parse the CLI's output to an AccessToken"""

Expand All @@ -100,7 +96,6 @@ async def test_get_token():
assert token.expires_on == valid_seconds


@pytest.mark.asyncio
async def test_cli_not_installed_linux():
"""The credential should raise CredentialUnavailableError when the CLI isn't installed"""

Expand All @@ -111,7 +106,6 @@ async def test_cli_not_installed_linux():
await credential.get_token("scope")


@pytest.mark.asyncio
async def test_cli_not_installed_windows():
"""The credential should raise CredentialUnavailableError when the CLI isn't installed"""

Expand All @@ -122,7 +116,6 @@ async def test_cli_not_installed_windows():
await credential.get_token("scope")


@pytest.mark.asyncio
async def test_cannot_execute_shell():
"""The credential should raise CredentialUnavailableError when the subprocess doesn't start"""

Expand All @@ -132,19 +125,27 @@ async def test_cannot_execute_shell():
await credential.get_token("scope")


@pytest.mark.asyncio
async def test_not_logged_in():
"""When the CLI isn't logged in, the credential should raise an error containing the CLI's output"""
"""When the CLI isn't logged in, the credential should raise CredentialUnavailableError"""

output = "ERROR: Please run 'az login' to setup account."
with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=1)):
with pytest.raises(CredentialUnavailableError, match=NOT_LOGGED_IN):
credential = AzureCliCredential()
await credential.get_token("scope")


async def test_unexpected_error():
"""When the CLI returns an unexpected error, the credential should raise an error containing the CLI's output"""

output = "something went wrong"
with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=42)):
with pytest.raises(ClientAuthenticationError, match=output):
credential = AzureCliCredential()
await credential.get_token("scope")


@pytest.mark.parametrize("output", TEST_ERROR_OUTPUTS)
@pytest.mark.asyncio
async def test_parsing_error_does_not_expose_token(output):
"""Errors during CLI output parsing shouldn't expose access tokens in that output"""

Expand All @@ -158,7 +159,6 @@ async def test_parsing_error_does_not_expose_token(output):


@pytest.mark.parametrize("output", TEST_ERROR_OUTPUTS)
@pytest.mark.asyncio
async def test_subprocess_error_does_not_expose_token(output):
"""Errors from the subprocess shouldn't expose access tokens in CLI output"""

Expand Down

0 comments on commit 717c419

Please sign in to comment.