From a62df9c73db6dc71781ef1f330cc0ec6b95640ba Mon Sep 17 00:00:00 2001 From: Paul Van Eck Date: Mon, 23 Sep 2024 14:28:00 -0700 Subject: [PATCH] [Identity] Update AzurePipelinesCredential request headers (#37510) Now we include the `X-TFS-FedAuthRedirect' header in OIDC requests to avoid redirect responses. Signed-off-by: Paul Van Eck --- sdk/identity/azure-identity/CHANGELOG.md | 2 ++ .../azure-identity/TROUBLESHOOTING.md | 2 +- .../identity/_credentials/azure_pipelines.py | 7 ++++- .../tests/test_azure_pipelines_credential.py | 27 ++++++++++++++++++ .../test_azure_pipelines_credential_async.py | 28 +++++++++++++++++++ 5 files changed, 64 insertions(+), 2 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 2a3c86e13144..4a424ce46c0d 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +- Fixed the request sent in `AzurePipelinesCredential` so it doesn't result in a redirect response when an invalid system access token is provided. ([#37510](https://github.com/Azure/azure-sdk-for-python/pull/37510)) + ### Other Changes ## 1.18.0 (2024-09-19) diff --git a/sdk/identity/azure-identity/TROUBLESHOOTING.md b/sdk/identity/azure-identity/TROUBLESHOOTING.md index 5a5ce9f52e18..dcb83c2307ad 100644 --- a/sdk/identity/azure-identity/TROUBLESHOOTING.md +++ b/sdk/identity/azure-identity/TROUBLESHOOTING.md @@ -295,7 +295,7 @@ Get-AzAccessToken -ResourceUrl "https://management.core.windows.net" | --- | --- | --- | | AADSTS900023: Specified tenant identifier `` is neither a valid DNS name, nor a valid external domain. | The Microsoft Entra tenant ID passed to the credential is invalid. | Verify the tenant ID is valid. If the service connection federated identity credential (FIC) was configured via a user-assigned managed identity, the tenant is the one in which managed identity was registered. If the service connection FIC is configured via an app registration, the tenant should be the one in which the app registration is registered. | | No service connection found with identifier ``. | The service connection ID provided is incorrect. | Verify the `service_connection_id` provided. For details on finding this value, check the [Find a service connection ID](#find-a-service-connection-id) section. | -| ClientAuthenticationError: OIDC token not found in response. Response = Object moved to here. Status Code: 302. | The system access token seems to be malformed when passing in as a parameter to the credential. | `System.AccessToken` is a required system variable in the Azure Pipelines task and should be provided in the pipeline task, [as mentioned in the docs](https://learn.microsoft.com/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml#systemaccesstoken). Verify that the system access token value provided is the predefined variable in Azure Pipelines and isn't malformed. | +| ClientAuthenticationError: 401 (Unauthorized) response from OIDC endpoint. | The system access token seems to be malformed/invalid when passing in as a parameter to the credential. | `System.AccessToken` is a required system variable in the Azure Pipelines task and should be provided in the pipeline task, [as mentioned in the docs](https://learn.microsoft.com/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml#systemaccesstoken). Verify that the system access token value provided is the predefined variable in Azure Pipelines and isn't malformed. | | ClientAuthenticationError: OIDC token not found in response. Response = {"$id":"1", "innerException":null, "message":"``", "typeName":"Microsoft.VisualStudio.Services.WebApi.VssInvalidPreviewVersionException, Microsoft.VisualStudio.Services.WebApi", "typeKey":"VssInvalidPreviewVersionException", "errorCode":0} | When the OIDC token request fails, the OIDC token API throws an error. More details about the specific error are specified in the "message" field of the response. | Mitigation usually depends on the scenario based on what [error message](https://learn.microsoft.com/azure/devops/pipelines/release/troubleshoot-workload-identity?view=azure-devops#error-messages) is being thrown. Make sure you use the [recommended Azure Pipelines task](https://learn.microsoft.com/azure/devops/pipelines/release/troubleshoot-workload-identity?view=azure-devops#review-pipeline-tasks). | | CredentialUnavailableError: Missing value for the `SYSTEM_OIDCREQUESTURI` environment variable. | This code isn't running inside of an Azure Pipelines environment. You might be running this code locally or on some other environment. | This credential is only designed to run inside the Azure Pipelines environment for the federated identity to work. | | AuthenticationRequiredError: unauthorized_client: 700016 - AADSTS700016: Application with identifier `` was not found in the directory 'Microsoft'. This error can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You may have sent your authentication request to the wrong tenant.| The `client_id` provided is invalid. | Verify the client ID argument is valid. If the service connection's federated identity was registered via a user-assigned managed identity, the client ID of the managed identity should be provided. If the service connection's federated identity is registered via an app registration, the application (client) ID from your app registration should be provided. | diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_pipelines.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_pipelines.py index a981ecff7a4c..3501f626ae62 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_pipelines.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_pipelines.py @@ -24,7 +24,12 @@ def build_oidc_request(service_connection_id: str, access_token: str) -> HttpRequest: base_uri = os.environ[SYSTEM_OIDCREQUESTURI].rstrip("/") url = f"{base_uri}?api-version={OIDC_API_VERSION}&serviceConnectionId={service_connection_id}" - headers = {"Content-Type": "application/json", "Authorization": f"Bearer {access_token}"} + headers = { + "Content-Type": "application/json", + "Authorization": f"Bearer {access_token}", + # Prevents the service from responding with a redirect HTTP status code (useful for automation). + "X-TFS-FedAuthRedirect": "Suppress", + } return HttpRequest("POST", url, headers=headers) diff --git a/sdk/identity/azure-identity/tests/test_azure_pipelines_credential.py b/sdk/identity/azure-identity/tests/test_azure_pipelines_credential.py index 1f9e27dab2d5..bb2cce7458d5 100644 --- a/sdk/identity/azure-identity/tests/test_azure_pipelines_credential.py +++ b/sdk/identity/azure-identity/tests/test_azure_pipelines_credential.py @@ -8,6 +8,7 @@ import pytest from azure.core.rest import HttpRequest +from azure.core.exceptions import ClientAuthenticationError from azure.identity import ( AzurePipelinesCredential, ChainedTokenCredential, @@ -134,3 +135,29 @@ def test_azure_pipelines_credential_authentication(get_token_method): token = getattr(credential, get_token_method)(scope) assert token.token assert isinstance(token.expires_on, int) + + +@pytest.mark.live_test_only("Requires Azure Pipelines environment with configured service connection") +@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS) +def test_azure_pipelines_credential_authentication_invalid_token(get_token_method): + system_access_token = "invalid" + service_connection_id = os.environ.get("AZURE_SERVICE_CONNECTION_ID", "") + tenant_id = os.environ.get("AZURE_SERVICE_CONNECTION_TENANT_ID", "") + client_id = os.environ.get("AZURE_SERVICE_CONNECTION_CLIENT_ID", "") + + scope = "https://vault.azure.net/.default" + + if not all([service_connection_id, tenant_id, client_id]): + pytest.skip("This test requires environment variables to be set") + + credential = AzurePipelinesCredential( + system_access_token=system_access_token, + tenant_id=tenant_id, + client_id=client_id, + service_connection_id=service_connection_id, + ) + + with pytest.raises(ClientAuthenticationError) as ex: + getattr(credential, get_token_method)(scope) + + assert ex.value.status_code == 401 diff --git a/sdk/identity/azure-identity/tests/test_azure_pipelines_credential_async.py b/sdk/identity/azure-identity/tests/test_azure_pipelines_credential_async.py index 471bc4f32189..0ee3b6ff2775 100644 --- a/sdk/identity/azure-identity/tests/test_azure_pipelines_credential_async.py +++ b/sdk/identity/azure-identity/tests/test_azure_pipelines_credential_async.py @@ -7,6 +7,7 @@ from unittest.mock import AsyncMock, patch import pytest +from azure.core.exceptions import ClientAuthenticationError from azure.identity import CredentialUnavailableError from azure.identity._credentials.azure_pipelines import SYSTEM_OIDCREQUESTURI from azure.identity.aio import AzurePipelinesCredential, ChainedTokenCredential, ClientAssertionCredential @@ -117,3 +118,30 @@ async def test_azure_pipelines_credential_authentication(get_token_method): token = await getattr(credential, get_token_method)(scope) assert token.token assert isinstance(token.expires_on, int) + + +@pytest.mark.asyncio +@pytest.mark.live_test_only("Requires Azure Pipelines environment with configured service connection") +@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS) +async def test_azure_pipelines_credential_authentication_invalid_token(get_token_method): + system_access_token = "invalid" + service_connection_id = os.environ.get("AZURE_SERVICE_CONNECTION_ID", "") + tenant_id = os.environ.get("AZURE_SERVICE_CONNECTION_TENANT_ID", "") + client_id = os.environ.get("AZURE_SERVICE_CONNECTION_CLIENT_ID", "") + + scope = "https://vault.azure.net/.default" + + if not all([service_connection_id, tenant_id, client_id]): + pytest.skip("This test requires environment variables to be set") + + credential = AzurePipelinesCredential( + system_access_token=system_access_token, + tenant_id=tenant_id, + client_id=client_id, + service_connection_id=service_connection_id, + ) + + with pytest.raises(ClientAuthenticationError) as ex: + await getattr(credential, get_token_method)(scope) + + assert ex.value.status_code == 401