From aabed35882254f4787644ca8a70a67fb8d8d1598 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 11:02:27 -0700 Subject: [PATCH 01/10] add interactive_browser_client_id --- sdk/identity/azure-identity/CHANGELOG.md | 2 ++ .../azure/identity/_credentials/default.py | 11 ++++++++++- sdk/identity/azure-identity/tests/test_default.py | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 43075dd8f743..bc728cf02ee3 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,6 +8,8 @@ - `OnBehalfOfCredential` supports the on-behalf-of authentication flow for accessing resources on behalf of users ([#19308](https://github.com/Azure/azure-sdk-for-python/issues/19308)) +- `DefaultAzureCredential` allows specifying the client ID of interactive browser via keyword argument `interactive_browser_client_id` + ([#20487](https://github.com/Azure/azure-sdk-for-python/issues/20487)) ### Breaking Changes diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index bc74b37ecc89..3ca6a6ba924d 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -70,6 +70,8 @@ class DefaultAzureCredential(ChainedTokenCredential): AZURE_TENANT_ID, if any. If unspecified, users will authenticate in their home tenants. :keyword str managed_identity_client_id: The client ID of a user-assigned managed identity. Defaults to the value of the environment variable AZURE_CLIENT_ID, if any. If not specified, a system-assigned identity will be used. + :keyword str interactive_browser_client_id: The client ID to be used in interactive browser credential. If not + specified, a system-assigned identity will be used. :keyword str shared_cache_username: Preferred username for :class:`~azure.identity.SharedTokenCacheCredential`. Defaults to the value of environment variable AZURE_USERNAME, if any. :keyword str shared_cache_tenant_id: Preferred tenant for :class:`~azure.identity.SharedTokenCacheCredential`. @@ -102,6 +104,7 @@ def __init__(self, **kwargs): managed_identity_client_id = kwargs.pop( "managed_identity_client_id", os.environ.get(EnvironmentVariables.AZURE_CLIENT_ID) ) + interactive_browser_client_id = kwargs.pop("interactive_browser_client_id") shared_cache_username = kwargs.pop("shared_cache_username", os.environ.get(EnvironmentVariables.AZURE_USERNAME)) shared_cache_tenant_id = kwargs.pop( @@ -137,7 +140,13 @@ def __init__(self, **kwargs): if not exclude_powershell_credential: credentials.append(AzurePowerShellCredential(**kwargs)) if not exclude_interactive_browser_credential: - credentials.append(InteractiveBrowserCredential(tenant_id=interactive_browser_tenant_id, **kwargs)) + if interactive_browser_client_id: + credentials.append(InteractiveBrowserCredential( + tenant_id=interactive_browser_tenant_id, + client_id=interactive_browser_client_id, + **kwargs)) + else: + credentials.append(InteractiveBrowserCredential(tenant_id=interactive_browser_tenant_id, **kwargs)) super(DefaultAzureCredential, self).__init__(*credentials) diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index a03a16cb85cc..9558245dd764 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -387,6 +387,21 @@ def validate_tenant_id(credential): validate_tenant_id(mock_credential) +def test_interactive_browser_client_id(): + """the credential should allow configuring a client ID for InteractiveBrowserCredential by kwarg""" + + client_id = "client-id" + + def validate_client_id(credential): + assert len(credential.call_args_list) == 1, "InteractiveBrowserCredential should be instantiated once" + _, kwargs = credential.call_args + assert kwargs == {"client_id": client_id, "tenant_id": "72f988bf-86f1-41af-91ab-2d7cd011db47"} + + with patch(DefaultAzureCredential.__module__ + ".InteractiveBrowserCredential") as mock_credential: + DefaultAzureCredential(exclude_interactive_browser_credential=False, interactive_browser_client_id=client_id) + validate_client_id(mock_credential) + + @pytest.mark.parametrize("expected_value", (True, False)) def test_allow_multitenant_authentication(expected_value): """the credential should pass "allow_multitenant_authentication" to the inner credentials which support it""" From 1bad811dfce94d829f5bd7aaf08798c1d392c992 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 11:32:39 -0700 Subject: [PATCH 02/10] Update sdk/identity/azure-identity/azure/identity/_credentials/default.py Co-authored-by: Charles Lowell --- .../azure-identity/azure/identity/_credentials/default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 3ca6a6ba924d..d58b57164272 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -104,7 +104,7 @@ def __init__(self, **kwargs): managed_identity_client_id = kwargs.pop( "managed_identity_client_id", os.environ.get(EnvironmentVariables.AZURE_CLIENT_ID) ) - interactive_browser_client_id = kwargs.pop("interactive_browser_client_id") + interactive_browser_client_id = kwargs.pop("interactive_browser_client_id", None) shared_cache_username = kwargs.pop("shared_cache_username", os.environ.get(EnvironmentVariables.AZURE_USERNAME)) shared_cache_tenant_id = kwargs.pop( From aaf7bffd08207b59be9e2014b3316117ee947b5f Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 11:32:50 -0700 Subject: [PATCH 03/10] Update sdk/identity/azure-identity/azure/identity/_credentials/default.py Co-authored-by: Charles Lowell --- .../azure-identity/azure/identity/_credentials/default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index d58b57164272..197c2484d2d3 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -71,7 +71,7 @@ class DefaultAzureCredential(ChainedTokenCredential): :keyword str managed_identity_client_id: The client ID of a user-assigned managed identity. Defaults to the value of the environment variable AZURE_CLIENT_ID, if any. If not specified, a system-assigned identity will be used. :keyword str interactive_browser_client_id: The client ID to be used in interactive browser credential. If not - specified, a system-assigned identity will be used. + specified, users will authenticate to an Azure development application. :keyword str shared_cache_username: Preferred username for :class:`~azure.identity.SharedTokenCacheCredential`. Defaults to the value of environment variable AZURE_USERNAME, if any. :keyword str shared_cache_tenant_id: Preferred tenant for :class:`~azure.identity.SharedTokenCacheCredential`. From f388d7ba4dbc1b6507b03c989c346790b082fbd7 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 11:37:10 -0700 Subject: [PATCH 04/10] update --- sdk/identity/azure-identity/tests/test_default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index 9558245dd764..8c8189996861 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -395,7 +395,7 @@ def test_interactive_browser_client_id(): def validate_client_id(credential): assert len(credential.call_args_list) == 1, "InteractiveBrowserCredential should be instantiated once" _, kwargs = credential.call_args - assert kwargs == {"client_id": client_id, "tenant_id": "72f988bf-86f1-41af-91ab-2d7cd011db47"} + assert kwargs["client_id"] == client_id with patch(DefaultAzureCredential.__module__ + ".InteractiveBrowserCredential") as mock_credential: DefaultAzureCredential(exclude_interactive_browser_credential=False, interactive_browser_client_id=client_id) From 85c05a370c6c19b5d091a2fb667c1f05166e6d9b Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 13:49:31 -0700 Subject: [PATCH 05/10] update --- .../azure-identity/azure/identity/_credentials/default.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 197c2484d2d3..28f0b537278e 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -4,6 +4,7 @@ # ------------------------------------ import logging import os +import six from .._constants import EnvironmentVariables from .._internal import get_default_authority, normalize_authority @@ -105,6 +106,8 @@ def __init__(self, **kwargs): "managed_identity_client_id", os.environ.get(EnvironmentVariables.AZURE_CLIENT_ID) ) interactive_browser_client_id = kwargs.pop("interactive_browser_client_id", None) + if not isinstance(interactive_browser_client_id, six.string_types): + raise ValueError('"interactive_browser_client_id" must be a string') shared_cache_username = kwargs.pop("shared_cache_username", os.environ.get(EnvironmentVariables.AZURE_USERNAME)) shared_cache_tenant_id = kwargs.pop( From 3722f87a6858f4cb5fceed3209ee8fa6977361d5 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 14:19:02 -0700 Subject: [PATCH 06/10] update --- .../azure-identity/azure/identity/_credentials/default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 28f0b537278e..efc6971f7254 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -106,7 +106,7 @@ def __init__(self, **kwargs): "managed_identity_client_id", os.environ.get(EnvironmentVariables.AZURE_CLIENT_ID) ) interactive_browser_client_id = kwargs.pop("interactive_browser_client_id", None) - if not isinstance(interactive_browser_client_id, six.string_types): + if interactive_browser_client_id and not isinstance(interactive_browser_client_id, six.string_types): raise ValueError('"interactive_browser_client_id" must be a string') shared_cache_username = kwargs.pop("shared_cache_username", os.environ.get(EnvironmentVariables.AZURE_USERNAME)) From 6acfbc784ac51698044b5353a2b5bd398d3456cc Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 14:24:26 -0700 Subject: [PATCH 07/10] update --- .../azure-identity/azure/identity/_credentials/default.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index efc6971f7254..90c21cec5f48 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -106,8 +106,6 @@ def __init__(self, **kwargs): "managed_identity_client_id", os.environ.get(EnvironmentVariables.AZURE_CLIENT_ID) ) interactive_browser_client_id = kwargs.pop("interactive_browser_client_id", None) - if interactive_browser_client_id and not isinstance(interactive_browser_client_id, six.string_types): - raise ValueError('"interactive_browser_client_id" must be a string') shared_cache_username = kwargs.pop("shared_cache_username", os.environ.get(EnvironmentVariables.AZURE_USERNAME)) shared_cache_tenant_id = kwargs.pop( @@ -144,6 +142,8 @@ def __init__(self, **kwargs): credentials.append(AzurePowerShellCredential(**kwargs)) if not exclude_interactive_browser_credential: if interactive_browser_client_id: + if not isinstance(interactive_browser_client_id, six.string_types): + raise ValueError('"interactive_browser_client_id" must be a string') credentials.append(InteractiveBrowserCredential( tenant_id=interactive_browser_tenant_id, client_id=interactive_browser_client_id, From 8377f88eda6d2f3aa5cfe2260478edbe23ab7fc6 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 14:59:48 -0700 Subject: [PATCH 08/10] update --- .../azure-identity/azure/identity/_credentials/default.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 90c21cec5f48..76388b313c30 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -142,8 +142,6 @@ def __init__(self, **kwargs): credentials.append(AzurePowerShellCredential(**kwargs)) if not exclude_interactive_browser_credential: if interactive_browser_client_id: - if not isinstance(interactive_browser_client_id, six.string_types): - raise ValueError('"interactive_browser_client_id" must be a string') credentials.append(InteractiveBrowserCredential( tenant_id=interactive_browser_tenant_id, client_id=interactive_browser_client_id, From fb8cef8a06aecf542dffd924e2c8638f63a96a45 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 15:03:31 -0700 Subject: [PATCH 09/10] Update sdk/identity/azure-identity/azure/identity/_credentials/default.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com> --- .../azure/identity/_credentials/default.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 76388b313c30..28585aacb8e7 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -142,10 +142,11 @@ def __init__(self, **kwargs): credentials.append(AzurePowerShellCredential(**kwargs)) if not exclude_interactive_browser_credential: if interactive_browser_client_id: - credentials.append(InteractiveBrowserCredential( - tenant_id=interactive_browser_tenant_id, - client_id=interactive_browser_client_id, - **kwargs)) + credentials.append( + InteractiveBrowserCredential( + tenant_id=interactive_browser_tenant_id, client_id=interactive_browser_client_id, **kwargs + ) + ) else: credentials.append(InteractiveBrowserCredential(tenant_id=interactive_browser_tenant_id, **kwargs)) From 6d0e1d1629fffb746b45dee7bbeeb9815c6b7c95 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 8 Sep 2021 15:06:17 -0700 Subject: [PATCH 10/10] update --- .../azure-identity/azure/identity/_credentials/default.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 76388b313c30..197c2484d2d3 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -4,7 +4,6 @@ # ------------------------------------ import logging import os -import six from .._constants import EnvironmentVariables from .._internal import get_default_authority, normalize_authority