diff --git a/packages/apps/src/microsoft_teams/apps/app.py b/packages/apps/src/microsoft_teams/apps/app.py index 3b05cb0b..da082016 100644 --- a/packages/apps/src/microsoft_teams/apps/app.py +++ b/packages/apps/src/microsoft_teams/apps/app.py @@ -63,7 +63,7 @@ from .routing.activity_context import ActivityContext from .socket_mode import SocketModeAdapter, SocketModeOptions from .state import create_state_loader -from .token_manager import DEFAULT_TENANT_FOR_GRAPH_TOKEN, TokenManager +from .token_manager import TokenManager from .token_provider import AppTokenProvider from .utils import create_graph_client, derive_graph_base_url from .utils.thread import to_threaded_conversation_id @@ -804,10 +804,7 @@ async def _get_bot_token(self): return await self._token_provider.get_app_token() async def _get_graph_token(self, tenant_id: Optional[str] = None) -> Optional[TokenProtocol]: - return await self._token_provider.get_app_token( - self.cloud.graph_scope, - tenant_id or (self.credentials.tenant_id if self.credentials else None) or DEFAULT_TENANT_FOR_GRAPH_TOKEN, - ) + return await self._token_provider.get_app_token(self.cloud.graph_scope, tenant_id) async def _get_agentic_graph_token( self, identity: AgenticIdentity, tenant_id: Optional[str] = None diff --git a/packages/apps/src/microsoft_teams/apps/token_manager.py b/packages/apps/src/microsoft_teams/apps/token_manager.py index 74ae4757..695e6d19 100644 --- a/packages/apps/src/microsoft_teams/apps/token_manager.py +++ b/packages/apps/src/microsoft_teams/apps/token_manager.py @@ -55,20 +55,25 @@ def __init__( async def get_bot_token(self) -> Optional[TokenProtocol]: """Refresh the bot authentication token.""" - return await self.get_app_token(self._cloud.bot_scope, default_tenant_id=self._cloud.login_tenant) + return await self.get_app_token(self._cloud.bot_scope) async def get_app_token( self, scope: str, tenant_id: Optional[str] = None, *, - default_tenant_id: str | None = None, caller_name: str | None = None, ) -> Optional[TokenProtocol]: - """Get an app token for the requested scope.""" - resolved_tenant_id = self._resolve_tenant_id(tenant_id, default_tenant_id or self._cloud.login_tenant) + """ + Get an app token for the requested scope. + + The tenant resolves as `tenant_id` -> credentials tenant -> a default for the scope: the cloud's login + tenant for the Bot Framework scope and "common" for the Graph scope. Any other scope has no default and + requires an explicit or configured tenant. + """ + resolved_tenant_id = self._resolve_tenant_id(tenant_id, self._default_tenant_id_for_scope(scope)) if resolved_tenant_id is None: - raise ValueError("tenant_id is required to get an app token") + raise ValueError(f"tenant_id is required to get an app token for scope {scope}") return await self._get_token( scope, tenant_id=resolved_tenant_id, @@ -86,11 +91,7 @@ async def get_graph_token(self, tenant_id: Optional[str] = None) -> Optional[Tok Returns: The graph token or None if not available """ - return await self.get_app_token( - self._cloud.graph_scope, - tenant_id=tenant_id, - default_tenant_id=DEFAULT_TENANT_FOR_GRAPH_TOKEN, - ) + return await self.get_app_token(self._cloud.graph_scope, tenant_id) async def get_agentic_user_token( self, @@ -431,5 +432,12 @@ def _get_managed_identity_client( ) return self._managed_identity_client + def _default_tenant_id_for_scope(self, scope: str) -> str | None: + if scope == self._cloud.bot_scope: + return self._cloud.login_tenant + if scope == self._cloud.graph_scope: + return DEFAULT_TENANT_FOR_GRAPH_TOKEN + return None + def _resolve_tenant_id(self, tenant_id: str | None, default_tenant_id: str | None) -> str | None: return tenant_id or (self._credentials.tenant_id if self._credentials else None) or default_tenant_id diff --git a/packages/apps/src/microsoft_teams/apps/token_provider.py b/packages/apps/src/microsoft_teams/apps/token_provider.py index 6282680d..e163dd79 100644 --- a/packages/apps/src/microsoft_teams/apps/token_provider.py +++ b/packages/apps/src/microsoft_teams/apps/token_provider.py @@ -28,7 +28,12 @@ async def get_app_token( scope: str | None = None, tenant_id: str | None = None, ) -> TokenProtocol | None: - """Acquire an app-only token.""" + """ + Acquire an app-only token, defaulting to the Bot Framework scope. + + Without a `tenant_id` or configured tenant, the Bot Framework scope falls back to the cloud's login tenant + and the Graph scope to "common"; any other scope raises `ValueError`. + """ return await self._token_manager.get_app_token(scope or self._cloud.bot_scope, tenant_id) async def get_agentic_user_token( diff --git a/packages/apps/tests/test_app.py b/packages/apps/tests/test_app.py index 70be6aa1..0540572e 100644 --- a/packages/apps/tests/test_app.py +++ b/packages/apps/tests/test_app.py @@ -526,13 +526,34 @@ def test_app_exposes_token_provider(self): assert not hasattr(app, "_auth_provider") @pytest.mark.asyncio - async def test_get_graph_token_uses_credentials_tenant(self): - app = App(client_id="test-client-id", client_secret="test-secret", tenant_id="credentials-tenant") - - with patch.object(app.token_provider, "get_app_token", autospec=True, return_value=None) as get_app_token: + @pytest.mark.parametrize( + "configured_tenant,expected_tenant", + [ + ("credentials-tenant", "credentials-tenant"), + (None, "common"), + ], + ) + async def test_graph_token_paths_resolve_the_same_tenant( + self, configured_tenant: Optional[str], expected_tenant: str + ): + with patch.dict("os.environ", {"TENANT_ID": ""}, clear=False): + app = App(client_id="test-client-id", client_secret="test-secret", tenant_id=configured_tenant) + msal_app = MagicMock() + # Minimal unsigned JWT: {"alg": "none"}.{} + msal_app.acquire_token_for_client.return_value = {"access_token": "eyJhbGciOiJub25lIn0.e30."} + + with patch( + "microsoft_teams.apps.token_manager.ConfidentialClientApplication", return_value=msal_app + ) as msal_class: await app._get_graph_token() + await app.token_provider.get_app_token(app.cloud.graph_scope) - get_app_token.assert_awaited_once_with(PUBLIC.graph_scope, "credentials-tenant") + # Both paths share one cached MSAL client, built for the Graph tenant rather than the Bot Framework tenant. + msal_class.assert_called_once_with( + "test-client-id", + client_credential="test-secret", + authority=f"https://login.microsoftonline.com/{expected_tenant}", + ) # The guard on `_get_agentic_graph_token`. A blueprint-level identity names no agentic user, so there is nobody to # acquire as, and the guard must refuse WITHOUT reaching the token provider. That shape is reachable rather than diff --git a/packages/apps/tests/test_token_manager.py b/packages/apps/tests/test_token_manager.py index c98babb9..9111e9c3 100644 --- a/packages/apps/tests/test_token_manager.py +++ b/packages/apps/tests/test_token_manager.py @@ -395,6 +395,80 @@ async def test_get_graph_token_with_tenant(self): # Should have been called with different-tenant-id assert any("different-tenant-id" in str(call) for call in calls) + @pytest.mark.asyncio + @pytest.mark.parametrize( + "scope,expected_tenant", + [ + (None, "botframework.com"), + (PUBLIC.bot_scope, "botframework.com"), + (PUBLIC.graph_scope, "common"), + ], + ) + async def test_app_token_provider_default_tenant_depends_on_scope(self, scope: str | None, expected_tenant: str): + """A multi-tenant app falls back to the default tenant for the requested resource.""" + credentials = ClientCredentials(client_id="test-client-id", client_secret="test-client-secret") + mock_msal_app = MagicMock() + mock_msal_app.acquire_token_for_client.return_value = {"access_token": VALID_TEST_TOKEN} + + with patch( + "microsoft_teams.apps.token_manager.ConfidentialClientApplication", return_value=mock_msal_app + ) as mock_msal_class: + provider = AppTokenProvider(TokenManager(credentials=credentials), PUBLIC) + token = await provider.get_app_token(scope) + + assert str(token) == VALID_TEST_TOKEN + mock_msal_class.assert_called_once_with( + "test-client-id", + client_credential="test-client-secret", + authority=f"https://login.microsoftonline.com/{expected_tenant}", + ) + + @pytest.mark.asyncio + async def test_app_token_provider_requires_tenant_for_other_scopes(self): + """A scope with no known default tenant must not borrow the Bot Framework login tenant.""" + credentials = ClientCredentials(client_id="test-client-id", client_secret="test-client-secret") + + with patch("microsoft_teams.apps.token_manager.ConfidentialClientApplication") as mock_msal_class: + provider = AppTokenProvider(TokenManager(credentials=credentials), PUBLIC) + with pytest.raises(ValueError, match="tenant_id is required"): + await provider.get_app_token("api://custom-resource/.default") + + mock_msal_class.assert_not_called() + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "credentials_tenant,tenant_id,expected_tenant", + [ + ("credentials-tenant", None, "credentials-tenant"), + ("credentials-tenant", "explicit-tenant", "explicit-tenant"), + (None, "explicit-tenant", "explicit-tenant"), + ], + ) + async def test_app_token_provider_other_scopes_use_explicit_or_configured_tenant( + self, credentials_tenant: str | None, tenant_id: str | None, expected_tenant: str + ): + credentials = ClientCredentials( + client_id="test-client-id", + client_secret="test-client-secret", + tenant_id=credentials_tenant, + ) + mock_msal_app = MagicMock() + mock_msal_app.acquire_token_for_client.return_value = {"access_token": VALID_TEST_TOKEN} + + with patch( + "microsoft_teams.apps.token_manager.ConfidentialClientApplication", return_value=mock_msal_app + ) as mock_msal_class: + provider = AppTokenProvider(TokenManager(credentials=credentials), PUBLIC) + token = await provider.get_app_token("api://custom-resource/.default", tenant_id) + + assert str(token) == VALID_TEST_TOKEN + mock_msal_app.acquire_token_for_client.assert_called_once_with(["api://custom-resource/.default"]) + mock_msal_class.assert_called_once_with( + "test-client-id", + client_credential="test-client-secret", + authority=f"https://login.microsoftonline.com/{expected_tenant}", + ) + @pytest.mark.asyncio @pytest.mark.parametrize( "get_token_method,expected_resource",