From cd81181db011a54ac0e028f83e5d4f79f456d6d5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 18:27:10 +0000 Subject: [PATCH 1/3] Initial plan From a8a349f8de77f7469525e4b4233c03a00fed7ffd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 18:32:09 +0000 Subject: [PATCH 2/3] Add comprehensive test coverage for _resolve_authority and _resolve_tenant_id methods; fix regex bug Co-authored-by: MattB-msft <10568244+MattB-msft@users.noreply.github.com> --- .../authentication/msal/msal_auth.py | 2 +- tests/authentication_msal/test_msal_auth.py | 149 ++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py b/libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py index 8b875846..8c46a7f9 100644 --- a/libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py +++ b/libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py @@ -164,7 +164,7 @@ def _resolve_authority( if config.AUTHORITY: return re.sub( - r"/\/(?:common|[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})(?=\/|$)/", + r"/(?:common|[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})(?=/|$)", f"/{tenant_id}", config.AUTHORITY, ) diff --git a/tests/authentication_msal/test_msal_auth.py b/tests/authentication_msal/test_msal_auth.py index 4368c63e..4addc35e 100644 --- a/tests/authentication_msal/test_msal_auth.py +++ b/tests/authentication_msal/test_msal_auth.py @@ -3,6 +3,10 @@ from microsoft_agents.authentication.msal import MsalAuth from microsoft_agents.hosting.core import Connections +from microsoft_agents.hosting.core.authorization import ( + AgentAuthConfiguration, + AuthTypes, +) from tests._common.testing_objects import MockMsalAuth @@ -63,6 +67,151 @@ async def test_acquire_token_on_behalf_of_confidential(self, mocker): ) +class TestMsalAuthTenantResolution: + """ + Test suite for testing tenant resolution methods in MsalAuth. + These methods are critical for multi-tenant authentication support. + """ + + def test_resolve_tenant_id_with_tenant_id_parameter(self): + """Test that tenant_id parameter takes precedence when provided""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc" + ) + result = MsalAuth._resolve_tenant_id(config, "tenant-override") + assert result == "tenant-override" + + def test_resolve_tenant_id_with_common_and_tenant_parameter(self): + """Test that tenant_id parameter is used when config.TENANT_ID is 'common'""" + config = AgentAuthConfiguration(tenant_id="common") + result = MsalAuth._resolve_tenant_id(config, "specific-tenant") + assert result == "specific-tenant" + + def test_resolve_tenant_id_with_common_no_tenant_parameter(self): + """Test that None is returned when config.TENANT_ID is 'common' and no tenant_id provided""" + config = AgentAuthConfiguration(tenant_id="common") + result = MsalAuth._resolve_tenant_id(config, None) + assert result is None + + def test_resolve_tenant_id_with_specific_tenant(self): + """Test that config.TENANT_ID is returned when it's a specific value""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc" + ) + result = MsalAuth._resolve_tenant_id(config, None) + assert result == "12345678-1234-1234-1234-123456789abc" + + def test_resolve_tenant_id_no_config_tenant_with_parameter(self): + """Test that tenant_id parameter is used when config.TENANT_ID is not set""" + config = AgentAuthConfiguration() + result = MsalAuth._resolve_tenant_id(config, "fallback-tenant") + assert result == "fallback-tenant" + + def test_resolve_tenant_id_no_config_tenant_no_parameter(self): + """Test that ValueError is raised when neither config.TENANT_ID nor tenant_id are set""" + config = AgentAuthConfiguration() + with pytest.raises( + ValueError, match="TENANT_ID is not set in the configuration" + ): + MsalAuth._resolve_tenant_id(config, None) + + def test_resolve_authority_with_common_replacement(self): + """Test that /common is replaced with the resolved tenant_id in authority URL""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc", + authority="https://login.microsoftonline.com/common", + ) + result = MsalAuth._resolve_authority(config, None) + assert ( + result + == "https://login.microsoftonline.com/12345678-1234-1234-1234-123456789abc" + ) + + def test_resolve_authority_with_tenant_guid_replacement(self): + """Test that existing tenant GUID is replaced with new tenant_id in authority URL""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc", + authority="https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + ) + result = MsalAuth._resolve_authority( + config, "new-tenant-11111111-2222-3333-4444-555555555555" + ) + assert ( + result + == "https://login.microsoftonline.com/new-tenant-11111111-2222-3333-4444-555555555555" + ) + + def test_resolve_authority_with_common_and_tenant_parameter(self): + """Test that /common is replaced with provided tenant_id parameter""" + config = AgentAuthConfiguration( + tenant_id="common", authority="https://login.microsoftonline.com/common" + ) + result = MsalAuth._resolve_authority( + config, "override-22222222-3333-4444-5555-666666666666" + ) + assert ( + result + == "https://login.microsoftonline.com/override-22222222-3333-4444-5555-666666666666" + ) + + def test_resolve_authority_no_authority_configured(self): + """Test fallback to default URL when no authority is configured""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc" + ) + result = MsalAuth._resolve_authority(config, None) + assert ( + result + == "https://login.microsoftonline.com/12345678-1234-1234-1234-123456789abc" + ) + + def test_resolve_authority_no_authority_with_tenant_override(self): + """Test fallback to default URL with tenant override when no authority is configured""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc" + ) + result = MsalAuth._resolve_authority( + config, "override-99999999-8888-7777-6666-555555555555" + ) + assert ( + result + == "https://login.microsoftonline.com/override-99999999-8888-7777-6666-555555555555" + ) + + def test_resolve_authority_with_common_no_tenant_parameter(self): + """Test behavior when config.TENANT_ID is 'common' and no tenant_id parameter""" + config = AgentAuthConfiguration( + tenant_id="common", authority="https://login.microsoftonline.com/common" + ) + # When tenant_id is None after resolution, should return original authority + result = MsalAuth._resolve_authority(config, None) + assert result == "https://login.microsoftonline.com/common" + + def test_resolve_authority_regex_with_trailing_slash(self): + """Test that regex correctly handles authority URLs with trailing slashes""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc", + authority="https://login.microsoftonline.com/common/", + ) + result = MsalAuth._resolve_authority(config, None) + assert ( + result + == "https://login.microsoftonline.com/12345678-1234-1234-1234-123456789abc/" + ) + + def test_resolve_authority_regex_preserves_path(self): + """Test that regex correctly replaces tenant while preserving additional path segments""" + config = AgentAuthConfiguration( + tenant_id="12345678-1234-1234-1234-123456789abc", + authority="https://login.microsoftonline.com/common/oauth2/v2.0/authorize", + ) + result = MsalAuth._resolve_authority(config, None) + assert ( + result + == "https://login.microsoftonline.com/12345678-1234-1234-1234-123456789abc/oauth2/v2.0/authorize" + ) + + # class TestMsalAuthAgentic: # @pytest.mark.asyncio From 0ea6e491087c8e312e5c1012153daa6da06d64b0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Feb 2026 18:32:55 +0000 Subject: [PATCH 3/3] Address code review feedback: improve test naming and add clarifying comment Co-authored-by: MattB-msft <10568244+MattB-msft@users.noreply.github.com> --- tests/authentication_msal/test_msal_auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/authentication_msal/test_msal_auth.py b/tests/authentication_msal/test_msal_auth.py index 4addc35e..e9e5f8ec 100644 --- a/tests/authentication_msal/test_msal_auth.py +++ b/tests/authentication_msal/test_msal_auth.py @@ -73,7 +73,7 @@ class TestMsalAuthTenantResolution: These methods are critical for multi-tenant authentication support. """ - def test_resolve_tenant_id_with_tenant_id_parameter(self): + def test_resolve_tenant_id_with_override_parameter(self): """Test that tenant_id parameter takes precedence when provided""" config = AgentAuthConfiguration( tenant_id="12345678-1234-1234-1234-123456789abc" @@ -102,7 +102,8 @@ def test_resolve_tenant_id_with_specific_tenant(self): assert result == "12345678-1234-1234-1234-123456789abc" def test_resolve_tenant_id_no_config_tenant_with_parameter(self): - """Test that tenant_id parameter is used when config.TENANT_ID is not set""" + """Test that tenant_id parameter is used when config.TENANT_ID is not set. + Note: tenant_id can be any string, not just GUID format.""" config = AgentAuthConfiguration() result = MsalAuth._resolve_tenant_id(config, "fallback-tenant") assert result == "fallback-tenant"