Introduce msal #191

heyitsaamir · 2025-10-29T03:31:21Z

In this PR, we introduce MSAL as a dependency to take care of authentication for us. We replace the use of bot token client with this library. It still uses secrets by default, but uses MSAL to do it.

Tested the general flows (regular + graph)

PR Dependency Tree

PR Introduce msal #191 👈
- PR Add User Managed Identity support in MSAL #192
  - PR Add Federated Identity Credentials support #203

This tree was auto-generated by Charcoal

packages/apps/src/microsoft/teams/apps/token_manager.py

## Context This fixes #184 which stated that the token wasn't being refreshed after an hour. This was because we weren't refreshing it on use. In typescript, we refresh the token on every [app.process](https://github.com/microsoft/teams.ts/blob/1f2d735ce02d0add1dc308f9e9df28f0c3fb6985/packages/apps/src/app.process.ts#L31). But this actually still leaves _proactive scenarios_ to used an cached, potentially expired, tokens. To remediate this, the token that gets passed to the API is a factory which refreshes the token if the cached token is expired. For the record C# sets the token value to be [refreshable](https://github.com/microsoft/teams.net/blob/19e4df96dac1524ae99d6c06bd4891fa5535ca67/Libraries/Microsoft.Teams.Apps/App.cs#L67C1-L67C46) (just as this PR is attempting to do). ## Changes This PR includes several changes 1. No more graph token manager. Instead we now have a TokenManager which manages all tokens. Soon, this might change to msal doing the changes 2. The app no longer refreshes token on start. But it does it the first time the token is being used. Because of this, the id field is now from the credentials, and the "name" field had to be removed. I don't think this should cause a big deal because name is honestly, not a very well used (or documented) field. **This is a breaking change though**. 3. The token that's being passed around now is an async function that either gets the token from the cache, or refreshes it if it's expired. ## Testing 1. Unit tests 2. Sanity tests to make sure we can send messages etc normally. 3. Tested to make sure app token refreshes automatically after an hour 4. Tested user graph tokens 5. Tested app graph tokens #### PR Dependency Tree * **PR #187** 👈 * **PR #191** * **PR #192** * **PR #193** This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal)

Copilot

Pull Request Overview

This PR migrates the TokenManager from using a custom BotTokenClient to using the Microsoft Authentication Library (MSAL) for acquiring authentication tokens. The changes remove the dependency on http_client and implement direct MSAL integration with token caching by tenant ID.

Key Changes:

Replaced custom HTTP-based token acquisition with MSAL's ConfidentialClientApplication
Removed token caching logic in favor of letting MSAL handle it internally
Added MSAL dependency (>=1.33.0) to the project and configured type stubs

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`packages/apps/pyproject.toml`	Added `msal>=1.33.0` dependency
`uv.lock`	Updated lockfile to include MSAL dependency
`stubs/msal/__init__.pyi`	Added type stubs for MSAL's `ConfidentialClientApplication`
`pyrightconfig.json`	Configured stub path for type checking
`packages/apps/src/microsoft/teams/apps/token_manager.py`	Refactored token acquisition to use MSAL instead of `BotTokenClient`
`packages/apps/src/microsoft/teams/apps/app.py`	Removed `http_client` parameter from `TokenManager` initialization
`packages/apps/tests/test_token_manager.py`	Updated tests to mock MSAL instead of `BotTokenClient`

Comments suppressed due to low confidence (2)

packages/apps/src/microsoft/teams/apps/token_manager.py:51

The docstring references a force parameter that no longer exists in the function signature (removed in line 45). This documentation should be removed.

            force: Force refresh even if token is not expired

packages/apps/src/microsoft/teams/apps/token_manager.py:82

Variable tenant_id_param is not used.

            tenant_id_param = tenant_id or credentials.tenant_id or "botframework.com"

packages/apps/src/microsoft/teams/apps/token_manager.py

heyitsaamir · 2025-11-05T18:47:41Z

@copilot can you review this PR again

Copilot · 2025-11-05T18:47:52Z

@heyitsaamir I've opened a new pull request, #204, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

packages/apps/src/microsoft/teams/apps/token_manager.py:51

The docstring still references a force parameter that no longer exists in the function signature. This line should be removed from the documentation.

            force: Force refresh even if token is not expired

packages/apps/src/microsoft/teams/apps/token_manager.py:82

Variable tenant_id_param is not used.

            tenant_id_param = tenant_id or credentials.tenant_id or "botframework.com"

packages/apps/src/microsoft/teams/apps/token_manager.py

Copilot

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

packages/apps/src/microsoft/teams/apps/token_manager.py:51

The docstring references a force parameter that no longer exists in the function signature. This outdated documentation should be removed.

            tenant_id: If provided, gets a tenant-specific token. Otherwise uses app's default.
            force: Force refresh even if token is not expired

packages/apps/src/microsoft/teams/apps/token_manager.py:82

Variable tenant_id_param is not used.

            tenant_id_param = tenant_id or credentials.tenant_id or "botframework.com"

packages/apps/src/microsoft/teams/apps/token_manager.py

packages/apps/tests/test_token_manager.py

Co-authored-by: Copilot <[email protected]>

corinagum · 2025-11-06T17:46:17Z

packages/apps/src/microsoft/teams/apps/token_manager.py

        """
-        if not self._credentials:
-            self._logger.debug("No credentials provided for graph token refresh")
+        return await self._get_token("https://graph.microsoft.com/.default", tenant_id)


nit: should we have these urls in a list of consts to avoid magic strings?

Yup we should. That might make @lilyydu's life easier

packages/apps/src/microsoft/teams/apps/token_manager.py

lilyydu · 2025-11-06T21:12:33Z

packages/apps/src/microsoft/teams/apps/token_manager.py

+
+            return JsonWebToken(access_token)
+
+    def _get_msal_client_for_tenant(self, tenant_id: str) -> ConfidentialClientApplication:


do you know if this / MSAL supports regional bots

stubs/msal/__init__.pyi

heyitsaamir mentioned this pull request Oct 29, 2025

Use refreshable app tokens #187

Merged

heyitsaamir force-pushed the aamirj/msal branch from e41253e to 4a83f10 Compare October 29, 2025 03:38

heyitsaamir mentioned this pull request Oct 29, 2025

Add User Managed Identity support in MSAL #192

Open

rido-min reviewed Oct 29, 2025

View reviewed changes

packages/apps/src/microsoft/teams/apps/token_manager.py Show resolved Hide resolved

heyitsaamir mentioned this pull request Oct 30, 2025

Add support for certificate files #193

Closed

Base automatically changed from aamirj/appToken to main October 30, 2025 22:21

heyitsaamir force-pushed the aamirj/msal branch from b4b57a3 to 1a84d4d Compare October 31, 2025 06:42

heyitsaamir mentioned this pull request Nov 5, 2025

Add Federated Identity Credentials support #203

Open

heyitsaamir added 4 commits November 4, 2025 18:08

Introduce msal

828d3b6

remove user token client

ea7717c

remove user token client

b7360af

fix tests

d086aea

heyitsaamir force-pushed the aamirj/msal branch from 1a84d4d to d086aea Compare November 5, 2025 02:08

Copilot AI review requested due to automatic review settings November 5, 2025 02:08

Copilot AI reviewed Nov 5, 2025

View reviewed changes

heyitsaamir force-pushed the aamirj/msal branch from 9c39533 to d086aea Compare November 5, 2025 06:14

Copilot AI mentioned this pull request Nov 5, 2025

[WIP] Introduce MSAL for authentication management #204

Closed

heyitsaamir requested a review from Copilot November 5, 2025 18:48

Copilot AI reviewed Nov 5, 2025

View reviewed changes

packages/apps/src/microsoft/teams/apps/token_manager.py Outdated Show resolved Hide resolved

reveal_type -> type

1605344

heyitsaamir requested a review from Copilot November 5, 2025 19:07

Copilot AI reviewed Nov 5, 2025

View reviewed changes

packages/apps/src/microsoft/teams/apps/token_manager.py Outdated Show resolved Hide resolved

packages/apps/tests/test_token_manager.py Outdated Show resolved Hide resolved

heyitsaamir and others added 2 commits November 5, 2025 18:23

Update packages/apps/tests/test_token_manager.py

c0806da

Co-authored-by: Copilot <[email protected]>

remove dep in default connection name

7d5efa8

corinagum reviewed Nov 6, 2025

View reviewed changes

corinagum approved these changes Nov 6, 2025

View reviewed changes

lilyydu reviewed Nov 6, 2025

View reviewed changes


		return JsonWebToken(access_token)

		def _get_msal_client_for_tenant(self, tenant_id: str) -> ConfidentialClientApplication:

Introduce msal #191

Are you sure you want to change the base?

Introduce msal #191

Conversation

heyitsaamir commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Dependency Tree

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

heyitsaamir commented Nov 5, 2025

Uh oh!

Copilot AI commented Nov 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

corinagum Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

heyitsaamir Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lilyydu Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

heyitsaamir commented Oct 29, 2025 •

edited

Loading