From 160552ef296961c21468af8181a37108aed2078f Mon Sep 17 00:00:00 2001 From: Paul Van Eck Date: Thu, 26 Jan 2023 13:21:04 -0800 Subject: [PATCH 1/2] [Identity] Allow configurable process timeouts AzureCliCredential, AzureDeveloperCliCredential, and AzurePowerShellCredential now allow users to pass in a custom timeout. This addresses scenarios where these proceses can take longer than the current default timeout values. DefaultAzureCredential now also has an optional keyword argument to allow users to pass in timeout values to the underlying developer credentials. Signed-off-by: Paul Van Eck --- sdk/identity/azure-identity/CHANGELOG.md | 3 +- .../azure/identity/_credentials/azd_cli.py | 20 +++++++++---- .../azure/identity/_credentials/azure_cli.py | 15 ++++++---- .../identity/_credentials/azure_powershell.py | 14 ++++----- .../azure/identity/_credentials/default.py | 10 +++++-- .../identity/aio/_credentials/azd_cli.py | 16 +++++++--- .../identity/aio/_credentials/azure_cli.py | 11 ++++--- .../aio/_credentials/azure_powershell.py | 16 +++++++--- .../identity/aio/_credentials/default.py | 10 +++++-- .../tests/test_azd_cli_credential.py | 9 ++++-- .../tests/test_cli_credential.py | 9 ++++-- .../azure-identity/tests/test_default.py | 30 ++++++++++++++++++- .../tests/test_default_async.py | 29 +++++++++++++++++- .../tests/test_powershell_credential.py | 6 +++- 14 files changed, 153 insertions(+), 45 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index dd6a8813f014..218d722d38f4 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -4,7 +4,8 @@ ### Features Added -- Added proactive refreshing feature for managed iedentity +- Added proactive refreshing feature for managed identity +- Credentials that are implemented via launching a subprocess to acquire tokens now have configurable timeouts using the `process_timeout` keyword argument. This addresses scenarios where these proceses can take longer than the current default timeout values. The affected credentials are `AzureCliCredential`, `AzureDeveloperCliCredential`, and `AzurePowerShellCredential`. (Note: For `DefaultAzureCredential`, the `developer_credential_timeout` keyword argument allows users to propagate this option to `AzureCliCredential`, `AzureDeveloperCliCredential`, and `AzurePowerShellCredential` in the authentication chain.) ([#28290](https://github.com/Azure/azure-sdk-for-python/pull/28290)) ### Breaking Changes diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py b/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py index a5bc819ff7eb..4a9eb6db9224 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py @@ -10,7 +10,7 @@ import shutil import subprocess import sys -from typing import Any, List, Optional +from typing import Any, Dict, List, Optional import six from azure.core.credentials import AccessToken @@ -37,12 +37,20 @@ class AzureDeveloperCliCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults to 10. """ - def __init__(self, *, tenant_id: str = "", additionally_allowed_tenants: Optional[List[str]] = None): + def __init__( + self, + *, + tenant_id: str = "", + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 + ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout def __enter__(self) -> "AzureDeveloperCliCredential": return self @@ -83,7 +91,7 @@ def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: ) if tenant: command += " --tenant-id " + tenant - output = _run_command(command) + output = _run_command(command, self._process_timeout) token = parse_token(output) if not token: @@ -130,7 +138,7 @@ def sanitize_output(output): return re.sub(r"\"token\": \"(.*?)(\"|$)", "****", output) -def _run_command(command): +def _run_command(command: str, timeout: int) -> str: # Ensure executable exists in PATH first. This avoids a subprocess call that would fail anyway. if shutil.which(EXECUTABLE_NAME) is None: raise CredentialUnavailableError(message=CLI_NOT_FOUND) @@ -142,12 +150,12 @@ def _run_command(command): try: working_directory = get_safe_working_dir() - kwargs = { + kwargs: Dict[str, Any] = { "stderr": subprocess.PIPE, "cwd": working_directory, "universal_newlines": True, "env": dict(os.environ, NO_COLOR="true"), - "timeout": 10, + "timeout": timeout, } return subprocess.check_output(args, **kwargs) 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 873426775bee..250f06fef037 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py @@ -10,7 +10,7 @@ import subprocess import sys import time -from typing import List, Optional, Any +from typing import List, Optional, Any, Dict import six from azure.core.credentials import AccessToken @@ -36,16 +36,19 @@ class AzureCliCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10. """ def __init__( self, *, tenant_id: str = "", - additionally_allowed_tenants: Optional[List[str]] = None + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout def __enter__(self): return self @@ -84,7 +87,7 @@ def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: ) if tenant: command += " --tenant " + tenant - output = _run_command(command) + output = _run_command(command, self._process_timeout) token = parse_token(output) if not token: @@ -135,7 +138,7 @@ def sanitize_output(output: str) -> str: return re.sub(r"\"accessToken\": \"(.*?)(\"|$)", "****", output) -def _run_command(command): +def _run_command(command: str, timeout: int) -> str: # Ensure executable exists in PATH first. This avoids a subprocess call that would fail anyway. if shutil.which(EXECUTABLE_NAME) is None: raise CredentialUnavailableError(message=CLI_NOT_FOUND) @@ -147,11 +150,11 @@ def _run_command(command): try: working_directory = get_safe_working_dir() - kwargs = { + kwargs: Dict[str, Any] = { "stderr": subprocess.PIPE, "cwd": working_directory, "universal_newlines": True, - "timeout": 10, + "timeout": timeout, "env": dict(os.environ, AZURE_CORE_NO_COLOR="true"), } return subprocess.check_output(args, **kwargs) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py index 0ceef0f01b6e..88c96a395669 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py @@ -4,7 +4,6 @@ # ------------------------------------ import base64 import logging -import platform import subprocess import sys from typing import List, Tuple, Optional, Any @@ -51,16 +50,19 @@ class AzurePowerShellCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10. """ def __init__( self, *, tenant_id: str = "", - additionally_allowed_tenants: Optional[List[str]] = None + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout def __enter__(self): return self @@ -96,17 +98,15 @@ def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: **kwargs ) command_line = get_command_line(scopes, tenant_id) - output = run_command_line(command_line) + output = run_command_line(command_line, self._process_timeout) token = parse_token(output) return token -def run_command_line(command_line: List[str]) -> str: +def run_command_line(command_line: List[str], timeout: int) -> str: stdout = stderr = "" proc = None - kwargs = {} - if platform.python_version() >= "3.3": - kwargs["timeout"] = 10 + kwargs = {"timeout": timeout} try: proc = start_process(command_line) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 6630a2bd01b5..1615ce19c8d8 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -80,6 +80,8 @@ class DefaultAzureCredential(ChainedTokenCredential): :class:`~azure.identity.VisualStudioCodeCredential`. Defaults to the "Azure: Tenant" setting in VS Code's user settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active Directory work or school accounts. + :keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials that run + subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10**. """ def __init__(self, **kwargs: Any) -> None: # pylint: disable=too-many-statements @@ -116,6 +118,8 @@ def __init__(self, **kwargs: Any) -> None: # pylint: disable=too-many-statement "shared_cache_tenant_id", os.environ.get(EnvironmentVariables.AZURE_TENANT_ID) ) + developer_credential_timeout = kwargs.pop("developer_credential_timeout", 10) + exclude_environment_credential = kwargs.pop("exclude_environment_credential", False) exclude_managed_identity_credential = kwargs.pop("exclude_managed_identity_credential", False) exclude_shared_token_cache_credential = kwargs.pop("exclude_shared_token_cache_credential", False) @@ -138,7 +142,7 @@ def __init__(self, **kwargs: Any) -> None: # pylint: disable=too-many-statement if not exclude_managed_identity_credential: credentials.append(ManagedIdentityCredential(client_id=managed_identity_client_id, **kwargs)) if not exclude_azd_cli_credential: - credentials.append(AzureDeveloperCliCredential()) + credentials.append(AzureDeveloperCliCredential(process_timeout=developer_credential_timeout)) if not exclude_shared_token_cache_credential and SharedTokenCacheCredential.supported(): try: # username and/or tenant_id are only required when the cache contains tokens for multiple identities @@ -151,9 +155,9 @@ def __init__(self, **kwargs: Any) -> None: # pylint: disable=too-many-statement if not exclude_visual_studio_code_credential: credentials.append(VisualStudioCodeCredential(**vscode_args)) if not exclude_cli_credential: - credentials.append(AzureCliCredential()) + credentials.append(AzureCliCredential(process_timeout=developer_credential_timeout)) if not exclude_powershell_credential: - credentials.append(AzurePowerShellCredential()) + credentials.append(AzurePowerShellCredential(process_timeout=developer_credential_timeout)) if not exclude_interactive_browser_credential: if interactive_browser_client_id: credentials.append( diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py index 3c5d68b1520b..904d83cfc9d0 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py @@ -35,12 +35,20 @@ class AzureDeveloperCliCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults to 10. """ - def __init__(self, *, tenant_id: str = "", additionally_allowed_tenants: Optional[List[str]] = None): + def __init__( + self, + *, + tenant_id: str = "", + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 + ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout @log_get_token_async async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: @@ -73,7 +81,7 @@ async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: if tenant: command += " --tenant-id " + tenant - output = await _run_command(command) + output = await _run_command(command, self._process_timeout) token = parse_token(output) if not token: @@ -88,7 +96,7 @@ async def close(self) -> None: """Calling this method is unnecessary""" -async def _run_command(command: str) -> str: +async def _run_command(command: str, timeout: int) -> str: # Ensure executable exists in PATH first. This avoids a subprocess call that would fail anyway. if shutil.which(EXECUTABLE_NAME) is None: raise CredentialUnavailableError(message=CLI_NOT_FOUND) @@ -108,7 +116,7 @@ async def _run_command(command: str) -> str: cwd=working_directory, env=dict(os.environ, NO_COLOR="true") ) - stdout_b, stderr_b = await asyncio.wait_for(proc.communicate(), 10) + stdout_b, stderr_b = await asyncio.wait_for(proc.communicate(), timeout) output = stdout_b.decode() stderr = stderr_b.decode() except OSError as ex: 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 31b8fa23ee09..9743cfa3d5d0 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 @@ -35,16 +35,19 @@ class AzureCliCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10. """ def __init__( self, *, tenant_id: str = "", - additionally_allowed_tenants: Optional[List[str]] = None + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout @log_get_token_async async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: @@ -76,7 +79,7 @@ async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken: if tenant: command += " --tenant " + tenant - output = await _run_command(command) + output = await _run_command(command, self._process_timeout) token = parse_token(output) if not token: @@ -92,7 +95,7 @@ async def close(self) -> None: """Calling this method is unnecessary""" -async def _run_command(command: str) -> str: +async def _run_command(command: str, timeout: int) -> str: # Ensure executable exists in PATH first. This avoids a subprocess call that would fail anyway. if shutil.which(EXECUTABLE_NAME) is None: raise CredentialUnavailableError(message=CLI_NOT_FOUND) @@ -112,7 +115,7 @@ async def _run_command(command: str) -> str: cwd=working_directory, env=dict(os.environ, AZURE_CORE_NO_COLOR="true") ) - stdout_b, stderr_b = await asyncio.wait_for(proc.communicate(), 10) + stdout_b, stderr_b = await asyncio.wait_for(proc.communicate(), timeout) output = stdout_b.decode() stderr = stderr_b.decode() except OSError as ex: diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py index 40b4f3621c5d..54e73f92423a 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py @@ -29,12 +29,20 @@ class AzurePowerShellCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. + :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10. """ - def __init__(self, *, tenant_id: str = "", additionally_allowed_tenants: Optional[List[str]] = None): + def __init__( + self, + *, + tenant_id: str = "", + additionally_allowed_tenants: Optional[List[str]] = None, + process_timeout: int = 10 + ) -> None: self.tenant_id = tenant_id self._additionally_allowed_tenants = additionally_allowed_tenants or [] + self._process_timeout = process_timeout @log_get_token_async async def get_token( @@ -65,7 +73,7 @@ async def get_token( **kwargs ) command_line = get_command_line(scopes, tenant_id) - output = await run_command_line(command_line) + output = await run_command_line(command_line, self._process_timeout) token = parse_token(output) return token @@ -73,7 +81,7 @@ async def close(self) -> None: """Calling this method is unnecessary""" -async def run_command_line(command_line: List[str]) -> str: +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) @@ -81,7 +89,7 @@ async def run_command_line(command_line: List[str]) -> str: # 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) - stdout, stderr = await asyncio.wait_for(proc.communicate(), 10) + stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout) except OSError as ex: # failed to execute "cmd" or "/bin/sh"; Azure PowerShell may or may not be installed diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py index 5e3189228e33..694b08378e1f 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py @@ -72,6 +72,8 @@ class DefaultAzureCredential(ChainedTokenCredential): :class:`~azure.identity.aio.VisualStudioCodeCredential`. Defaults to the "Azure: Tenant" setting in VS Code's user settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active Directory work or school accounts. + :keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials that run + subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10**. """ def __init__(self, **kwargs: Any) -> None: @@ -107,6 +109,8 @@ def __init__(self, **kwargs: Any) -> None: "visual_studio_code_tenant_id", os.environ.get(EnvironmentVariables.AZURE_TENANT_ID) ) + developer_credential_timeout = kwargs.pop("developer_credential_timeout", 10) + exclude_visual_studio_code_credential = kwargs.pop("exclude_visual_studio_code_credential", True) exclude_azd_cli_credential = kwargs.pop("exclude_azd_cli_credential", False) exclude_cli_credential = kwargs.pop("exclude_cli_credential", False) @@ -128,7 +132,7 @@ def __init__(self, **kwargs: Any) -> None: if not exclude_managed_identity_credential: credentials.append(ManagedIdentityCredential(client_id=managed_identity_client_id, **kwargs)) if not exclude_azd_cli_credential: - credentials.append(AzureDeveloperCliCredential()) + credentials.append(AzureDeveloperCliCredential(process_timeout=developer_credential_timeout)) if not exclude_shared_token_cache_credential and SharedTokenCacheCredential.supported(): try: # username and/or tenant_id are only required when the cache contains tokens for multiple identities @@ -141,9 +145,9 @@ def __init__(self, **kwargs: Any) -> None: if not exclude_visual_studio_code_credential: credentials.append(VisualStudioCodeCredential(**vscode_args)) if not exclude_cli_credential: - credentials.append(AzureCliCredential()) + credentials.append(AzureCliCredential(process_timeout=developer_credential_timeout)) if not exclude_powershell_credential: - credentials.append(AzurePowerShellCredential()) + credentials.append(AzurePowerShellCredential(process_timeout=developer_credential_timeout)) super().__init__(*credentials) diff --git a/sdk/identity/azure-identity/tests/test_azd_cli_credential.py b/sdk/identity/azure-identity/tests/test_azd_cli_credential.py index bf995b320c0b..fdbbafb8ade0 100644 --- a/sdk/identity/azure-identity/tests/test_azd_cli_credential.py +++ b/sdk/identity/azure-identity/tests/test_azd_cli_credential.py @@ -134,9 +134,14 @@ def test_timeout(): from subprocess import TimeoutExpired with mock.patch("shutil.which", return_value="azd"): - with mock.patch(CHECK_OUTPUT, mock.Mock(side_effect=TimeoutExpired("", 42))): + with mock.patch(CHECK_OUTPUT, mock.Mock(side_effect=TimeoutExpired("", 42))) as check_output_mock: with pytest.raises(CredentialUnavailableError): - AzureDeveloperCliCredential().get_token("scope") + AzureDeveloperCliCredential(process_timeout=42).get_token("scope") + + # Ensure custom timeout is passed to subprocess + _, kwargs = check_output_mock.call_args + assert "timeout" in kwargs + assert kwargs["timeout"] == 42 def test_multitenant_authentication_class(): diff --git a/sdk/identity/azure-identity/tests/test_cli_credential.py b/sdk/identity/azure-identity/tests/test_cli_credential.py index 930416e7f65f..baa1e744610e 100644 --- a/sdk/identity/azure-identity/tests/test_cli_credential.py +++ b/sdk/identity/azure-identity/tests/test_cli_credential.py @@ -141,9 +141,14 @@ def test_timeout(): from subprocess import TimeoutExpired with mock.patch("shutil.which", return_value="az"): - with mock.patch(CHECK_OUTPUT, mock.Mock(side_effect=TimeoutExpired("", 42))): + with mock.patch(CHECK_OUTPUT, mock.Mock(side_effect=TimeoutExpired("", 42))) as check_output_mock: with pytest.raises(CredentialUnavailableError): - AzureCliCredential().get_token("scope") + AzureCliCredential(process_timeout=42).get_token("scope") + + # Ensure custom timeout is passed to subprocess + _, kwargs = check_output_mock.call_args + assert "timeout" in kwargs + assert kwargs["timeout"] == 42 def test_multitenant_authentication_class(): diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index 5583eb9bb202..3441e37a324a 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -161,7 +161,7 @@ def assert_credentials_not_present(chain, *excluded_credential_classes): credential = DefaultAzureCredential(exclude_powershell_credential=True) assert_credentials_not_present(credential, AzurePowerShellCredential) - + credential = DefaultAzureCredential(exclude_azd_cli_credential=True) assert_credentials_not_present(credential, AzureDeveloperCliCredential) @@ -374,6 +374,34 @@ def validate_client_id(credential): validate_client_id(mock_credential) +def test_developer_credential_timeout(): + """the credential should allow configuring a process timeout for Azure CLI and PowerShell by kwarg""" + + timeout = 42 + + with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential: + with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential: + DefaultAzureCredential(developer_credential_timeout=timeout) + + for credential in (mock_cli_credential, mock_pwsh_credential): + _, kwargs = credential.call_args + assert "process_timeout" in kwargs + assert kwargs["process_timeout"] == timeout + + +def test_developer_credential_timeout_default(): + """the credential should allow configuring a process timeout for Azure CLI and PowerShell by kwarg""" + + with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential: + with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential: + DefaultAzureCredential() + + for credential in (mock_cli_credential, mock_pwsh_credential): + _, kwargs = credential.call_args + assert "process_timeout" in kwargs + assert kwargs["process_timeout"] == 10 + + def test_unexpected_kwarg(): """the credential shouldn't raise when given an unexpected keyword argument""" DefaultAzureCredential(foo=42) diff --git a/sdk/identity/azure-identity/tests/test_default_async.py b/sdk/identity/azure-identity/tests/test_default_async.py index 3455979e8899..e43c3100ff42 100644 --- a/sdk/identity/azure-identity/tests/test_default_async.py +++ b/sdk/identity/azure-identity/tests/test_default_async.py @@ -93,7 +93,7 @@ def test_initialization(mock_credential, expect_argument): shared_cache.supported = lambda: False with patch.dict("os.environ", {}, clear=True): test_initialization(mock_credential, expect_argument=False) - + # authority should not be passed to AzureDeveloperCliCredential with patch(DefaultAzureCredential.__module__ + ".AzureDeveloperCliCredential") as mock_credential: with patch(DefaultAzureCredential.__module__ + ".SharedTokenCacheCredential") as shared_cache: @@ -282,6 +282,33 @@ def get_credential_for_shared_cache_test(expected_refresh_token, expected_access return DefaultAzureCredential(_cache=cache, transport=transport, **exclude_other_credentials, **kwargs) +def test_developer_credential_timeout(): + """the credential should allow configuring a process timeout for Azure CLI and PowerShell by kwarg""" + + timeout = 42 + + with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential: + with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential: + DefaultAzureCredential(developer_credential_timeout=timeout) + + for credential in (mock_cli_credential, mock_pwsh_credential): + _, kwargs = credential.call_args + assert "process_timeout" in kwargs + assert kwargs["process_timeout"] == timeout + +def test_developer_credential_timeout(): + """the credential should allow configuring a process timeout for Azure CLI and PowerShell by kwarg""" + + with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential: + with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential: + DefaultAzureCredential() + + for credential in (mock_cli_credential, mock_pwsh_credential): + _, kwargs = credential.call_args + assert "process_timeout" in kwargs + assert kwargs["process_timeout"] == 10 + + def test_unexpected_kwarg(): """the credential shouldn't raise when given an unexpected keyword argument""" DefaultAzureCredential(foo=42) diff --git a/sdk/identity/azure-identity/tests/test_powershell_credential.py b/sdk/identity/azure-identity/tests/test_powershell_credential.py index 6a008fd44f02..7c4b11715760 100644 --- a/sdk/identity/azure-identity/tests/test_powershell_credential.py +++ b/sdk/identity/azure-identity/tests/test_powershell_credential.py @@ -192,9 +192,13 @@ def test_timeout(): proc = Mock(communicate=Mock(side_effect=TimeoutExpired("", 42)), returncode=None) with patch(POPEN, Mock(return_value=proc)): with pytest.raises(CredentialUnavailableError): - AzurePowerShellCredential().get_token("scope") + AzurePowerShellCredential(process_timeout=42).get_token("scope") assert proc.communicate.call_count == 1 + # Ensure custom timeout is passed to subprocess + _, kwargs = proc.communicate.call_args + assert "timeout" in kwargs + assert kwargs["timeout"] == 42 def test_unexpected_error(): From 2e78270243a7d5c0c3b2be014b1a798d48f3dab8 Mon Sep 17 00:00:00 2001 From: Paul Van Eck Date: Fri, 24 Mar 2023 17:28:04 -0700 Subject: [PATCH 2/2] Update docstrings Signed-off-by: Paul Van Eck --- .../azure-identity/azure/identity/_credentials/azd_cli.py | 3 ++- .../azure-identity/azure/identity/_credentials/azure_cli.py | 2 +- .../azure/identity/_credentials/azure_powershell.py | 2 +- .../azure-identity/azure/identity/_credentials/default.py | 2 +- .../azure-identity/azure/identity/aio/_credentials/azd_cli.py | 3 ++- .../azure/identity/aio/_credentials/azure_cli.py | 2 +- .../azure/identity/aio/_credentials/azure_powershell.py | 2 +- .../azure-identity/azure/identity/aio/_credentials/default.py | 2 +- 8 files changed, 10 insertions(+), 8 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py b/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py index 4a9eb6db9224..969f1c2f58c7 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py @@ -37,7 +37,8 @@ class AzureDeveloperCliCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults + to 10 seconds. """ def __init__( 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 250f06fef037..905ce45c6c10 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py @@ -36,7 +36,7 @@ class AzureCliCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10 seconds. """ def __init__( self, diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py index 88c96a395669..5bfa5f9284de 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py @@ -50,7 +50,7 @@ class AzurePowerShellCredential: :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10 seconds. """ def __init__( self, diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 1615ce19c8d8..ff6c37f2d7b6 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -81,7 +81,7 @@ class DefaultAzureCredential(ChainedTokenCredential): settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active Directory work or school accounts. :keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials that run - subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10**. + subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10** seconds. """ def __init__(self, **kwargs: Any) -> None: # pylint: disable=too-many-statements diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py index 904d83cfc9d0..0e0e4041bb48 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azd_cli.py @@ -35,7 +35,8 @@ class AzureDeveloperCliCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure Developer CLI process to respond. Defaults + to 10 seconds. """ def __init__( 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 9743cfa3d5d0..33632b652839 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 @@ -35,7 +35,7 @@ class AzureCliCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10 seconds. """ def __init__( self, diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py index 54e73f92423a..d28b3aa3923c 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py @@ -29,7 +29,7 @@ class AzurePowerShellCredential(AsyncContextManager): :keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id" for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the application can access. - :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10. + :keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10 seconds. """ def __init__( diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py index 694b08378e1f..6eae972eee4c 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py @@ -73,7 +73,7 @@ class DefaultAzureCredential(ChainedTokenCredential): user settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active Directory work or school accounts. :keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials that run - subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10**. + subprocesses (e.g. AzureCliCredential, AzurePowerShellCredential). Defaults to **10** seconds. """ def __init__(self, **kwargs: Any) -> None: