AgentAuthConfiguration validation based on auth type - #551
AgentAuthConfiguration validation based on auth type#551Rodrigo Brandão (rodrigobr-msft) wants to merge 5 commits into
AgentAuthConfiguration validation based on auth type#551Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces configuration-time validation to AgentAuthConfiguration, aiming to fail fast when required settings for certain authentication modes are missing.
Changes:
- Invoke a new
_validate()method duringAgentAuthConfiguration.__init__. - Add auth-type-specific required-field checks (e.g., certificate PFX path, federated client ID).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/hosting_core/test_auth_configuration.py:124
pytest.raises(..., match=expected_message)treatsexpected_messageas a regex. Since the messages contain regex metacharacters (e.g. the trailing.), the assertion can pass even if the error message changes unexpectedly. Escaping the message makes the test stricter and avoids accidental regex behavior.
"FEDERATED_TOKEN_FILE is required for "
"workload_identity authentication.",
),
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py:230
- The
_validatedocstring claims it validates “any required fields”, but the implementation only enforces auth-type-specific requirements (currently certificate + federated credentials). Tightening the docstring will keep it accurate as validation rules evolve.
"""
Validates the configuration. Raises ValueError if any required fields are missing or invalid.
"""
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py:242
AgentAuthConfiguration._validate()is missing the workload identity requirement check described in the PR and expected by the new tests. As written,AuthTypes.workload_identitywill not raise whenFEDERATED_TOKEN_FILEis missing, so configuration can be created in an invalid state.
if (
self.AUTH_TYPE == AuthTypes.federated_credentials
and not self.FEDERATED_CLIENT_ID
):
raise ValueError(
tests/authentication_msal/test_msal_auth.py:291
- The error-message regex here is missing the trailing period used by the other validation messages (and by the new tests in
tests/hosting_core/test_auth_configuration.py). OnceAgentAuthConfigurationvalidates workload identity, this test should match the exact message.
with pytest.raises(
ValueError,
match="FEDERATED_TOKEN_FILE is required for workload_identity authentication",
):
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/hosting_core/test_auth_configuration.py:128
pytest.raises(..., match=...)treatsexpected_messageas a regex; the trailing.in these messages is a wildcard and can let the test pass even if the exception text changes. Escaping the dot makes the assertion check the literal message more reliably.
with pytest.raises(ValueError, match=expected_message):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py:230
- The
_validatedocstring says it checks for missing or invalid fields, but the implementation only checks for missing required fields. Adjusting the wording avoids documenting behavior that doesn't exist.
"""
Validates the configuration. Raises ValueError if any required fields are missing or invalid.
"""
This pull request introduces validation for authentication configuration settings in the
AgentAuthConfigurationclass to improve reliability and developer experience. It ensures that required fields are present for certificate, federated credential, and workload identity authentication types. The update also adds comprehensive tests to verify that missing or valid credentials are handled correctly, and documents these changes in the changelog.Authentication configuration validation:
_validatemethod toAgentAuthConfigurationthat checks for required fields based on the authentication type and raises aValueErrorif any are missing (certificate:CERT_PFX_FILE, federated credentials:FEDERATED_CLIENT_ID, workload identity:FEDERATED_TOKEN_FILE). (libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py)Testing improvements:
tests/hosting_core/test_auth_configuration.py)ValueErrorwhen the workload identity token file is missing, aligning with the new validation logic. (tests/authentication_msal/test_msal_auth.py)Documentation:
changelog.md)