Fix AAD authentication in ACA environments for RedTeam #44938
Conversation
github-actions
bot
added
the
Evaluation
slister1001
force-pushed
the
fix/aca-entra-credential-auth
branch
3 times, most recently
from
60565c6 to
ec98609
Compare
There was a problem hiding this comment.
Pull request overview
Summary:
This PR fixes AAD authentication failures in Azure Container Apps (ACA) environments for RedTeam operations. The fix allows passing an explicit credential from the RedTeam instance to get_chat_target(), avoiding reliance on DefaultAzureCredential which fails when Azure CLI is not available in containerized environments.
Changes:
- Added optional
credentialparameter toget_chat_target()function for explicit credential passing - Modified RedTeam.scan() to pass
self.credentialtoget_chat_target()for use in ACA environments - Added comprehensive test coverage for credential-based authentication with different priority scenarios
- Updated AzureOpenAIModelConfiguration TypedDict to include credential field
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| azure/ai/evaluation/red_team/_utils/strategy_utils.py | Added credential parameter to get_chat_target() function with proper authentication priority handling (api_key > credential > DefaultAzureCredential). Includes code formatting improvements. |
| azure/ai/evaluation/red_team/_red_team.py | Modified scan() method to pass self.credential to get_chat_target(). Includes extensive code formatting improvements throughout the file. |
| tests/unittests/test_redteam/test_strategy_utils.py | Added three new test cases covering credential parameter usage, precedence testing, and various authentication scenarios. Includes code formatting improvements. |
| @patch("azure.ai.evaluation.red_team._utils.strategy_utils.OpenAIChatTarget") | ||
| def test_get_chat_target_azure_openai_api_key_takes_precedence( | ||
| self, mock_openai_chat_target | ||
| ): | ||
| """Test that api_key takes precedence over credential when both are provided.""" | ||
| mock_instance = MagicMock() | ||
| mock_openai_chat_target.return_value = mock_instance | ||
|
|
||
| mock_credential = MagicMock() | ||
|
|
||
| config = { | ||
| "azure_deployment": "gpt-35-turbo", | ||
| "azure_endpoint": "https://example.openai.azure.com", | ||
| "api_key": "test-api-key", | ||
| "credential": mock_credential, | ||
| } | ||
|
|
||
| result = get_chat_target(config) | ||
|
|
||
| # Should use api_key, not credential | ||
| mock_openai_chat_target.assert_called_once_with( | ||
| model_name="gpt-35-turbo", | ||
| endpoint="https://example.openai.azure.com", | ||
| api_key="test-api-key", | ||
| api_version="2024-06-01", | ||
| ) | ||
| # Credential should not be used | ||
| mock_credential.get_token.assert_not_called() | ||
|
|
||
| assert result == mock_instance |
There was a problem hiding this comment.
Consider adding a test case for when both target["credential"] and the credential parameter are provided simultaneously. According to the implementation at line 191 in strategy_utils.py, the target dict credential should take precedence over the parameter credential (due to the or operator), but this behavior is not currently tested. This would help ensure the priority order is well-understood and maintained.
| Used as a fallback when target doesn't have an api_key. This is useful in ACA environments | ||
| where DefaultAzureCredential is not available. |
There was a problem hiding this comment.
The documentation for the credential parameter states it's "Used as a fallback when target doesn't have an api_key" but the actual implementation at line 191 shows it's also a fallback when target doesn't have a credential field. The documentation should be more precise to reflect that the credential parameter is used when BOTH api_key and target["credential"] are absent. Consider updating to: "Used as a fallback when target doesn't have an api_key or credential field. This is useful in ACA environments where DefaultAzureCredential is not available."
| Used as a fallback when target doesn't have an api_key. This is useful in ACA environments | |
| where DefaultAzureCredential is not available. | |
| Used as a fallback when target doesn't have an api_key or credential field. This is useful | |
| in ACA environments where DefaultAzureCredential is not available. |
slister1001
force-pushed
the
fix/aca-entra-credential-auth
branch
from
ec98609 to
ae92eb7
Compare
When running red team scans in ACA environments, DefaultAzureCredential fails because Azure CLI is not available. This fix allows passing an explicit credential from the RedTeam instance to get_chat_target(). Changes: - Add credential parameter to get_chat_target() function - Add _create_token_provider() helper to create callable from credential - Pass self.credential from RedTeam.scan() to get_chat_target() - Add credential field to AzureOpenAIModelConfiguration TypedDict - Add tests for credential-based authentication paths Auth priority: api_key > credential > use_aad_auth (DefaultAzureCredential)
slister1001
force-pushed
the
fix/aca-entra-credential-auth
branch
from
ae92eb7 to
99ee749
Compare
2 participants
When running red team scans in ACA environments, DefaultAzureCredential fails because Azure CLI is not available. This fix allows passing an explicit credential from the RedTeam instance to get_chat_target().
Changes:
Auth priority: api_key > credential > use_aad_auth (DefaultAzureCredential)
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines