Skip to content

Conversation

@heyitsaamir
Copy link
Collaborator

@heyitsaamir heyitsaamir commented Nov 10, 2025

Adds trace level to our logger, and also passes in logging options to MSAL. And moves credential creation logic to TokenManager.

PR Dependency Tree

This tree was auto-generated by Charcoal

@heyitsaamir heyitsaamir changed the title Add trace level logging to logger Add trace level logging to logger and move credential logic to tokenmanager Nov 11, 2025
@heyitsaamir heyitsaamir force-pushed the aamirj/addTrace branch 2 times, most recently from c83db7f to 96da7e8 Compare November 11, 2025 18:41
const LOG_LEVEL_TO_MSAL_LOG_LEVEL: Record<LogLevel, MSALLogLevel> = {
'error': MSALLogLevel.Error,
'warn': MSALLogLevel.Warning,
'info': MSALLogLevel.Warning,// MSAL logs are noisy, so we if logging is set to info, we set msal logging to warning
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo

Base automatically changed from aamirj/FIC to aamirj/addMsal November 11, 2025 20:30
@heyitsaamir heyitsaamir merged commit 1e5a305 into aamirj/addMsal Nov 11, 2025
1 of 5 checks passed
@heyitsaamir heyitsaamir deleted the aamirj/addTrace branch November 11, 2025 20:30
heyitsaamir added a commit that referenced this pull request Nov 12, 2025
This PR introduces MSAL as a dependency and switches our supported bot
authentication to use it. It also includes support for User Managed
Identity support and FIC such that we don't need to provide custom code
to do that anymore as we had previously suggested. Instead, we can do
that simply by the environment variables.

| CLIENT_ID | CLIENT_SECRET | MANAGED_IDENTITY_CLIENT_ID | Output |
|-|-|-|-|
| not_set | | | No-Auth |
| set | set | | SecretsAuth |
| set | not_set | | User Managed Identity Auth |
| set | not_set | set (same as CLIENT_ID) | User Managed Identity Auth |
| set | not_set | set (diff from CLIENT_ID) | FIC (user managed
identity) |
| set | not_set | "system" | FIC (system identity) |

Changes to note:
1. We are now no longer _proactively_ refreshing tokens. Instead, we put
the token fetcher inside the token resolver logic for the client. This
ends up being a call to `tokenManager.getBotToken`.
2. This fixes a bug where proactive scenarios were going to fail because
we weren't refreshing the token for them. For reactive scenarios, we
refresh the token when the request comes in, so it was fine.
3. We are deprecating the `id` and `name` fields because those used to
depend on the token being present. Now that we are not proactively
fetching the token, those are not as useful (or reliable). For now, I
have chosen to simply use the value from the manifest to populate them.
I don't think these fields were that useful anyway, so opting to
deprecate.
4. Removes graphToken from being injected into plugins. Plugins weren't
using it, and we want to remove tight dependency on graph package.
















#### PR Dependency Tree


* **PR #387** 👈
* Merged #388, #389, #392

This tree was auto-generated by
[Charcoal](https://github.com/danerwilliams/charcoal)
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.

5 participants