[Identity] Update behavior of MI Service Fabric (Azure#39848)

Service Fabric managed identity should not allow a user to pass in a user-assigned identity
since Service Fabric will always use the cluster-configured identity.

Signed-off-by: Paul Van Eck <[email protected]>

Author: pvaneck

sdk/identity/azure-identity/CHANGELOG.md

@@ -6,6 +6,8 @@
 
 ### Breaking Changes
 
+- Previously, if a `client_id` or `identity_config` was specified in `ManagedIdentityCredential` for Service Fabric managed identity, which is not supported, the `client_id` (or `resource_id`/`object_id` specified `identity_config`) would be silently ignored. Now, an exception will be raised during a token request if a `client_id` or `identity_config` is specified for Service Fabric managed identity.
+
 ### Bugs Fixed
 
 ### Other Changes

sdk/identity/azure-identity/azure/identity/_credentials/service_fabric.py

@@ -6,16 +6,37 @@
 import os
 from typing import Dict, Optional, Any
 
+from azure.core.credentials import AccessToken, AccessTokenInfo, TokenRequestOptions
+from azure.core.exceptions import ClientAuthenticationError
 from azure.core.pipeline.transport import HttpRequest
 
 from .._constants import EnvironmentVariables
 from .._internal.msal_managed_identity_client import MsalManagedIdentityClient
 
 
+SERVICE_FABRIC_ERROR_MESSAGE = (
+    "Specifying a client_id or identity_config is not supported by the Service Fabric managed identity environment. "
+    "The managed identity configuration is determined by the Service Fabric cluster resource configuration. "
+    "See https://aka.ms/servicefabricmi for more information."
+)
+
+
 class ServiceFabricCredential(MsalManagedIdentityClient):
     def get_unavailable_message(self, desc: str = "") -> str:
         return f"Service Fabric managed identity configuration not found in environment. {desc}"
 
+    def get_token(
+        self, *scopes: str, claims: Optional[str] = None, tenant_id: Optional[str] = None, **kwargs: Any
+    ) -> AccessToken:
+        if self._settings.get("client_id") or self._settings.get("identity_config"):
+            raise ClientAuthenticationError(message=SERVICE_FABRIC_ERROR_MESSAGE)
+        return super().get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
+
+    def get_token_info(self, *scopes: str, options: Optional[TokenRequestOptions] = None) -> AccessTokenInfo:
+        if self._settings.get("client_id") or self._settings.get("identity_config"):
+            raise ClientAuthenticationError(message=SERVICE_FABRIC_ERROR_MESSAGE)
+        return super().get_token_info(*scopes, options=options)
+
 
 def _get_client_args(**kwargs: Any) -> Optional[Dict]:
     url = os.environ.get(EnvironmentVariables.IDENTITY_ENDPOINT)

sdk/identity/azure-identity/azure/identity/aio/_credentials/service_fabric.py

@@ -4,9 +4,12 @@
 # ------------------------------------
 from typing import Optional, Any
 
+from azure.core.credentials import AccessToken, AccessTokenInfo, TokenRequestOptions
+from azure.core.exceptions import ClientAuthenticationError
+
 from .._internal.managed_identity_base import AsyncManagedIdentityBase
 from .._internal.managed_identity_client import AsyncManagedIdentityClient
-from ..._credentials.service_fabric import _get_client_args
+from ..._credentials.service_fabric import _get_client_args, SERVICE_FABRIC_ERROR_MESSAGE
 
 
 class ServiceFabricCredential(AsyncManagedIdentityBase):
@@ -18,3 +21,16 @@ def get_client(self, **kwargs: Any) -> Optional[AsyncManagedIdentityClient]:
 
     def get_unavailable_message(self, desc: str = "") -> str:
         return f"Service Fabric managed identity configuration not found in environment. {desc}"
+
+    async def get_token(
+        self, *scopes: str, claims: Optional[str] = None, tenant_id: Optional[str] = None, **kwargs: Any
+    ) -> AccessToken:
+        if self._client and self._client._identity_config:  # pylint:disable=protected-access
+            raise ClientAuthenticationError(message=SERVICE_FABRIC_ERROR_MESSAGE)
+        return await super().get_token(*scopes, claims=claims, tenant_id=tenant_id, **kwargs)
+
+    async def get_token_info(self, *scopes: str, options: Optional[TokenRequestOptions] = None) -> AccessTokenInfo:
+
+        if self._client and self._client._identity_config:  # pylint:disable=protected-access
+            raise ClientAuthenticationError(message=SERVICE_FABRIC_ERROR_MESSAGE)
+        return await super().get_token_info(*scopes, options=options)

sdk/identity/azure-identity/tests/test_managed_identity.py

@@ -7,6 +7,7 @@
 import logging
 from unittest import mock
 
+from azure.core.exceptions import ClientAuthenticationError
 from azure.identity import ManagedIdentityCredential, CredentialUnavailableError
 from azure.identity._constants import EnvironmentVariables
 from azure.identity._credentials.imds import IMDS_AUTHORITY, IMDS_TOKEN_PATH
@@ -820,6 +821,27 @@ def send(request, **kwargs):
         assert abs(token.expires_on - expires_on) <= 1
 
 
+@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
+def test_service_fabric_with_client_id_error(get_token_method):
+    """ManagedIdentityCredential should raise an error if a user identity is provided."""
+    endpoint = "http://localhost:42"
+    with mock.patch(
+        "os.environ",
+        {
+            EnvironmentVariables.IDENTITY_ENDPOINT: endpoint,
+            EnvironmentVariables.IDENTITY_HEADER: "secret",
+            EnvironmentVariables.IDENTITY_SERVER_THUMBPRINT: "thumbprint",
+        },
+    ):
+        cred = ManagedIdentityCredential(client_id="client_id")
+        with pytest.raises(ClientAuthenticationError):
+            getattr(cred, get_token_method)("scope")
+
+        cred = ManagedIdentityCredential(identity_config={"resource_id": "resource_id"})
+        with pytest.raises(ClientAuthenticationError):
+            getattr(cred, get_token_method)("scope")
+
+
 @pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
 def test_token_exchange(tmpdir, get_token_method):
     exchange_token = "exchange-token"

sdk/identity/azure-identity/tests/test_managed_identity_async.py

@@ -858,6 +858,29 @@ async def send(request, **kwargs):
         assert token.expires_on == expires_on
 
 
+@pytest.mark.asyncio
+@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
+async def test_service_fabric_with_client_id_error(get_token_method):
+    """ManagedIdentityCredential should raise an error if a user identity is provided."""
+    endpoint = "http://localhost:42"
+    with mock.patch(
+        "os.environ",
+        {
+            EnvironmentVariables.IDENTITY_ENDPOINT: endpoint,
+            EnvironmentVariables.IDENTITY_HEADER: "secret",
+            EnvironmentVariables.IDENTITY_SERVER_THUMBPRINT: "thumbprint",
+        },
+    ):
+
+        cred = ManagedIdentityCredential(client_id="client_id")
+        with pytest.raises(ClientAuthenticationError):
+            await getattr(cred, get_token_method)("scope")
+
+        cred = ManagedIdentityCredential(identity_config={"resource_id": "resource_id"})
+        with pytest.raises(ClientAuthenticationError):
+            await getattr(cred, get_token_method)("scope")
+
+
 @pytest.mark.asyncio
 @pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
 async def test_azure_arc(tmpdir, get_token_method):