Skip to content

Adding MsalTokenCredential implementation of azure.core.credentials.AsyncTokenProvider - #565

Merged
Rodrigo Brandão (rodrigobr-msft) merged 16 commits into
mainfrom
users/robrandao/msal-token-cred
Aug 27, 2026
Merged

Adding MsalTokenCredential implementation of azure.core.credentials.AsyncTokenProvider#565
Rodrigo Brandão (rodrigobr-msft) merged 16 commits into
mainfrom
users/robrandao/msal-token-cred

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request introduces a new MsalTokenCredential class that provides an asynchronous Azure Core token credential backed by MSAL, along with related updates to authentication logic, documentation, and tests. The changes enhance compatibility with Azure SDKs, improve access token handling, and add comprehensive integration and unit tests.

New Azure Core Token Credential and Authentication Improvements:

  • Added MsalTokenCredential implementing AsyncTokenCredential, enabling use of agent authentication with Azure SDKs and other libraries that accept Azure Core credentials. This includes resource extraction from scopes and error handling for missing scopes.
  • Updated MsalAuth to return AccessToken objects with proper expiration (expires_on), and refactored token acquisition logic to support the new credential class. [1] [2] [3] [4]
  • Exposed MsalTokenCredential in the package’s public API and documented its usage in the readme.md, including example code and usage notes. [1] [2]

Testing and Integration:

  • Added integration and unit tests for MsalTokenCredential, including scope/resource handling, error propagation, and Azure SDK compatibility. [1] [2]
  • Improved test utilities and mocks to support new expiration handling and environment variable checks. [1] [2] [3] [4]

These changes make it significantly easier to use agent authentication with Azure SDKs and ensure robust, well-tested token acquisition logic.

Copilot AI lite review requested due to automatic review settings August 27, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an MSAL-backed Azure Core AsyncTokenCredential (MsalTokenCredential) to the microsoft-agents-authentication-msal package, updates MsalAuth to produce Azure AccessToken objects with proper expiry handling, and introduces unit/integration coverage to validate Azure SDK compatibility.

Changes:

  • Introduces MsalTokenCredential implementing Azure Core AsyncTokenCredential and exports it from the package public API.
  • Refactors MsalAuth token acquisition to compute expires_on from expires_in and return Azure AccessToken internally.
  • Adds unit + integration tests and improves integration-test env var gating utilities.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/authentication_msal/test_msal_token_credential.py Adds unit tests for scope/resource handling, error cases, and MSAL forwarding.
tests/authentication_msal/test_msal_auth.py Adds unit test validating expires_inexpires_on conversion.
tests/_common/testing_objects/mocks/mock_msal_auth.py Updates MSAL auth mock payload defaults for expiration-aware tokens.
libraries/microsoft-agents-authentication-msal/readme.md Documents the new MsalTokenCredential API and usage example.
libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_token_credential.py Adds the new Azure Core credential implementation.
libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py Adds AccessToken support and expiry computation via expires_in.
libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/init.py Exposes MsalTokenCredential from the package root.
dev/integration/tests/utils/pytest.py Refines env var skip marker helper for integration tests.
dev/integration/tests/auth/test_msal_token_credential.py Adds an integration test validating real token acquisition returns an AccessToken.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libraries/microsoft-agents-authentication-msal/readme.md Outdated
Comment thread tests/_common/testing_objects/mocks/mock_msal_auth.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 18:45
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py:145

  • _get_access_token() currently requires expires_in and derives expires_on as time.time() + expires_in. MSAL token payloads may supply expires_on directly (epoch seconds), and using it when present avoids failures when expires_in is missing and reduces reliance on local clock math.
            expires_in = auth_result_payload.get("expires_in")
            if expires_in is None:
                raise ValueError("Token response does not include an expiration.")

            return AccessToken(res, int(time.time()) + int(expires_in))

dev/integration/tests/utils/pytest.py:24

  • When load_root_env_file=True, the merge order makes .env values override real environment variables ({**os.environ, **dotenv_values('.env')}). This is surprising in CI/local shells where exported env vars should take precedence over .env defaults.
    if load_root_env_file:
        # Load environment variables from the root .env file if specified
        environ = {**os.environ, **dotenv_values(".env")}
    environment = os.environ if environ is None else environ

libraries/microsoft-agents-authentication-msal/readme.md:251

  • The docs state the first scope "must be an absolute resource URI", but MsalTokenCredential.get_token() currently accepts any string and only strips a trailing /.default. Either enforce the constraint in code or soften the wording here to match actual behavior.
At least one scope is required. The first scope must be an absolute resource
URI (typically ending in `/.default`). For client-credential flows, all requested
scopes are passed to MSAL; managed identity uses the derived resource.

Copilot AI review requested due to automatic review settings August 27, 2026 20:33
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) marked this pull request as ready for review August 27, 2026 20:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

libraries/microsoft-agents-authentication-msal/readme.md:246

  • The README example uses await at top-level, which isn’t valid in a normal Python script. Adjusting the snippet to define an async entry point (and run it via asyncio.run) makes it directly runnable and clearer.
Explore working examples in the [Python samples repository](https://github.com/microsoft/Agents/tree/main/samples/python):

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_token_credential.py:55

  • MsalTokenCredential eagerly constructs MsalAuth in init, so calling get_token() with no scopes still creates the auth provider. This both does unnecessary work and makes the new unit test test_get_token_requires_at_least_one_scope fail (it expects MsalAuth is never constructed when scopes are missing). Lazy-initialize MsalAuth after validating scopes.
        self._config = config
        self._provider = MsalAuth(config)

    async def get_token(self, *scopes: str, **kwargs) -> AccessToken:
        """Acquire an access token for the specified scopes.

Copilot AI review requested due to automatic review settings August 27, 2026 20:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py:144

  • The ValueError raised when the token response lacks expiration information is not very actionable, and it also misses the opportunity to indicate what the response contained without leaking the access token. Consider including the response keys (or other non-sensitive metadata) so callers can diagnose MSAL/AAD responses more easily.
            expires_in = auth_result_payload.get("expires_in")
            if expires_in is None:
                raise ValueError("Token response does not include an expiration.")

changelog.md:9

  • The v1.6.0 release notes only mention the resource extraction bug fix, but this PR also introduces a new public API (MsalTokenCredential) and changes token handling to return AccessToken with expires_on. Please capture the new API/behavior in the changelog so consumers can discover it.
## Bug Fixes

- **MSAL Resource Extraction**: Preserved `api://` and other non-default scopes while removing only a trailing `/.default` when deriving the authentication resource.

Copilot AI review requested due to automatic review settings August 27, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread tests/authentication_msal/test_msal_token_credential.py Outdated
Comment thread changelog.md Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 20:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (3)

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_token_credential.py:4

  • If get_token() is called concurrently on the same credential instance, the lazy _provider initialization can race. Introducing an async lock for provider initialization requires importing asyncio.
import logging

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_token_credential.py:36

  • MsalTokenCredential lazily initializes _provider without synchronization. Adding an asyncio.Lock on the instance avoids races when multiple tasks call get_token() concurrently on the same credential.
        self._config = config
        self._provider: MsalAuth | None = None

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_token_credential.py:55

  • Lazy initialization of _provider is not concurrency-safe: two concurrent get_token() calls can create multiple MsalAuth instances and lose token-cache reuse. Guard initialization with an async lock and prefer is None checks over truthiness checks.
        if not self._provider:
            self._provider = MsalAuth(self._config)

Copilot AI review requested due to automatic review settings August 27, 2026 21:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

libraries/microsoft-agents-authentication-msal/microsoft_agents/authentication/msal/msal_auth.py:145

  • MSAL token responses may include an absolute expires_on field (epoch seconds) instead of (or in addition to) expires_in. Requiring expires_in unconditionally can raise even when the response includes a usable expiration, and it can block token acquisition for some MSAL flows.
            expires_in = auth_result_payload.get("expires_in")
            if expires_in is None:
                raise ValueError("Token response does not include an expiration.")

            return AccessToken(res, int(time.time()) + int(expires_in))

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 21:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

dev/integration/tests/utils/pytest.py:23

  • When load_root_env_file=True, the merge order gives .env values precedence over real environment variables (os.environ). This is inconsistent with _ENVIRONMENT = {**dotenv_values('.env'), **os.environ} used elsewhere and can cause CI/secret-injected env vars to be overridden by a checked-in .env file. Swap the merge order so os.environ wins.
        environ = {**os.environ, **dotenv_values(".env")}

tests/_common/testing_objects/mocks/mock_msal_auth.py:18

  • acquire_token_for_client_return uses a mutable dict as a default argument, which can be shared across instances and lead to cross-test contamination if it’s ever mutated. Use None as the default and create a new dict inside the constructor.
        acquire_token_for_client_return={
            "access_token": "token",
            "expires_in": 3600,
        },

@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) merged commit b0f0083 into main Aug 27, 2026
11 checks passed
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) deleted the users/robrandao/msal-token-cred branch August 27, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support to cast a user token or service token as an Azure TokenCredential type

3 participants