diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 781fe4b105c3..d753337d91cd 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -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`. diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py index e18dd43ca3a6..550e36fc1754 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py @@ -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) @@ -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: diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py index 99ca177450e5..c049b1421cde 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py @@ -13,6 +13,7 @@ CLI_NOT_FOUND, COMMAND_LINE, get_safe_working_dir, + NOT_LOGGED_IN, parse_token, sanitize_output, ) @@ -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) diff --git a/sdk/identity/azure-identity/tests/test_cli_credential.py b/sdk/identity/azure-identity/tests/test_cli_credential.py index 7377d330455e..3467a451356a 100644 --- a/sdk/identity/azure-identity/tests/test_cli_credential.py +++ b/sdk/identity/azure-identity/tests/test_cli_credential.py @@ -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 @@ -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") diff --git a/sdk/identity/azure-identity/tests/test_cli_credential_async.py b/sdk/identity/azure-identity/tests/test_cli_credential_async.py index 9e8ad1f82217..ae9dec90a341 100644 --- a/sdk/identity/azure-identity/tests/test_cli_credential_async.py +++ b/sdk/identity/azure-identity/tests/test_cli_credential_async.py @@ -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 @@ -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(): @@ -27,7 +29,6 @@ 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""" @@ -35,7 +36,6 @@ async def test_no_scopes(): 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""" @@ -43,14 +43,12 @@ async def test_multiple_scopes(): 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""" @@ -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""" @@ -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""" @@ -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""" @@ -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""" @@ -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""" @@ -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""" @@ -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"""