Use MSAL to mint tokens #203

rido-min · 2025-11-13T16:53:12Z

This pull request introduces significant changes to the authentication mechanism and dependency injection setup in the Teams API and Apps libraries. The main update is the refactoring of the ClientCredentials class to use IAuthorizationHeaderProvider for token acquisition, moving away from direct management of client ID/secret. Additionally, the application initialization and dependency injection patterns are updated to support these changes, and related code paths for credentials setup are commented out to reflect the new approach. Package references are also updated to support these changes.

Authentication Refactor:

Refactored ClientCredentials to depend on IAuthorizationHeaderProvider, removing direct handling of client ID, client secret, and tenant ID, and updating the Resolve method to use the new provider for token acquisition.
Updated NuGet dependencies: added Microsoft.Identity.Web.AgentIdentities and updated System.IdentityModel.Tokens.Jwt to a newer version to support the new authentication flow. [1] [2]

Dependency Injection and App Initialization:

Modified App and AppBuilder to require IHttpCredentials via dependency injection, and adjusted the construction and registration of App instances to use DI instead of manual instantiation. [1] [2] [3]
Updated DI-related code in extension methods and configuration classes, commenting out old credential setup logic and registration patterns that are no longer compatible with the new approach. [1] [2] [3] [4]

Codebase Consistency:

Added explicit IHttpClientFactory alias imports across all API client classes for consistency and clarity. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Project File and Copyright

Minor project file updates, including copyright header fixes.

Miscellaneous

Commented out unused or obsolete using statements and credential setup code in extension/configuration classes. [1] [2]

These changes collectively modernize the authentication flow, improve testability and maintainability via dependency injection, and update the codebase for compatibility with newer identity libraries.

rido-min · 2025-11-13T16:59:07Z

Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs

replaced to use M.I.W that already handles different credential types

rido-min · 2025-11-13T16:59:52Z

Libraries/Microsoft.Teams.Api/Clients/ActivityClient.cs

 using Microsoft.Teams.Api.Activities;
 using Microsoft.Teams.Common.Http;

+using IHttpClientFactory = Microsoft.Teams.Common.Http.IHttpClientFactory;


could not find a better way to avoid namespace conflicts

rido-min · 2025-11-13T17:00:38Z

Libraries/Microsoft.Teams.Apps/App.cs

+    internal App() : this(null!, null)
+    { }
+
+    public App(IHttpCredentials credentials, AppOptions? options = null)


we need to register our credentials in the DI, so we can get receive the IAuthorizationHeaderProvider

rido-min · 2025-11-13T17:01:15Z

Libraries/Microsoft.Teams.Apps/AppBuilder.cs

+    private readonly IServiceProvider _serviceProvider;
    protected AppOptions _options;

+    public AppBuilder(IServiceProvider serviceProvider)


this is an idea to consolidate our AppBuilder with the service provider.. not perfect, but it works :)

Copilot

Pull Request Overview

This pull request modernizes the authentication mechanism by migrating from custom OAuth2 token acquisition to Microsoft Authentication Library (MSAL) via IAuthorizationHeaderProvider. The changes introduce dependency injection patterns throughout the codebase, update NuGet packages to support MSAL, and refactor the App and AppBuilder classes to require credentials through constructor injection rather than options.

Refactored ClientCredentials to use MSAL's IAuthorizationHeaderProvider instead of manually calling token endpoints
Updated dependency injection setup across plugins and extensions to support the new authentication flow
Modified App constructor signature to require IHttpCredentials as a constructor parameter

Reviewed Changes

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

Show a summary per file

File	Description
Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs	Replaced manual OAuth2 token acquisition with MSAL `IAuthorizationHeaderProvider`
Libraries/Microsoft.Teams.Api/Microsoft.Teams.Api.csproj	Added Microsoft.Identity.Web.AgentIdentities and updated System.IdentityModel.Tokens.Jwt
Libraries/Microsoft.Teams.Apps/App.cs	Changed constructor to require `IHttpCredentials` as first parameter
Libraries/Microsoft.Teams.Apps/AppBuilder.cs	Added service provider-based constructor and updated Build() to inject credentials
Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/HostApplicationBuilder.cs	Added MSAL token acquisition services and configured DI for credentials
Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/ApplicationBuilder.cs	Updated App instantiation to pass credentials via constructor
Libraries/Microsoft.Teams.Extensions/Microsoft.Teams.Extensions.Hosting/Microsoft.Teams.Apps.Extensions/ServiceCollection.cs	Commented out old AddTeams overload that created App without DI
Libraries/Microsoft.Teams.Extensions/Microsoft.Teams.Extensions.Hosting/Microsoft.Teams.Apps.Extensions/HostApplicationBuilder.cs	Commented out manual ClientCredentials setup, switched to DI-based App registration
Libraries/Microsoft.Teams.Extensions/Microsoft.Teams.Extensions.Configuration/Microsoft.Teams.Apps.Extensions/TeamsSettings.cs	Commented out credential setup logic in Apply method
Tests/Microsoft.Teams.Apps.Tests/AppTests.cs	Updated all test cases to pass credentials as constructor parameter
Samples/Samples.Echo/appsettings.json	Updated configuration structure for MSAL with Instance, TenantId, and ClientCredentials array
Samples/Samples.Echo/appsettings.Development.json	Updated configuration structure for MSAL
Samples/Samples.Echo/Program.cs	Simplified sample to use new Teams extension methods
Libraries/Microsoft.Teams.Api/Clients/*.cs	Added IHttpClientFactory alias for consistency (12 client files)
Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore.DevTools/Microsoft.Teams.Plugins.AspNetCore.DevTools.csproj	Updated System.IdentityModel.Tokens.Jwt version

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-13T17:02:40Z

...rosoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/HostApplicationBuilder.cs

+#pragma warning disable ASP0000 // Use 'new(...)'
+        AppBuilder appBuilder = new AppBuilder(builder.Services.BuildServiceProvider());
+#pragma warning restore ASP0000
+
+        builder.AddTeamsCore(appBuilder);


The BuildServiceProvider() call creates a service provider during configuration, which is an anti-pattern in ASP.NET Core. This can lead to:

Memory leaks from captured scoped services

Multiple service provider instances with inconsistent state

Services resolved before the final container is built

Consider refactoring to avoid building the service provider here. Instead, register the AppBuilder as a factory or use a different approach to defer the App creation until after the container is fully configured.

Suggested change

#pragma warning disable ASP0000 // Use 'new(...)'

AppBuilder appBuilder = new AppBuilder(builder.Services.BuildServiceProvider());

#pragma warning restore ASP0000

builder.AddTeamsCore(appBuilder);

// Register AppBuilder as a singleton using the final service provider

builder.Services.AddSingleton<AppBuilder>(sp => new AppBuilder(sp));

builder.AddTeamsCore();

Copilot · 2025-11-13T17:02:41Z

Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs

+        var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(scopes[0], options, cancellationToken);
+        return new TokenResponse
+        {
+            AccessToken = tokenResult.Substring("Bearer ".Length),


The Substring call will throw an ArgumentOutOfRangeException if tokenResult is shorter than "Bearer " (7 characters) or doesn't start with "Bearer ". Consider using a safer approach like:

AccessToken = tokenResult.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase) ? tokenResult.Substring("Bearer ".Length) : tokenResult,

Suggested change

AccessToken = tokenResult.Substring("Bearer ".Length),

AccessToken = tokenResult.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)

? tokenResult.Substring("Bearer ".Length)

: tokenResult,

Copilot · 2025-11-13T17:02:41Z

Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs

-
-        var res = await client.SendAsync<TokenResponse>(request, cancellationToken);
-        return res.Body;
+        var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(scopes[0], options, cancellationToken);


Only the first scope (scopes[0]) is being used, but the method accepts an array. This could lead to unexpected behavior if callers pass multiple scopes. Consider either:

Joining all scopes with a space delimiter (standard OAuth2 practice): string.Join(" ", scopes)

Changing the parameter type to string scope if only one scope is supported

Documenting that only the first scope is used

Suggested change

var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(scopes[0], options, cancellationToken);

var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(string.Join(" ", scopes), options, cancellationToken);

Copilot · 2025-11-13T17:02:42Z

Libraries/Microsoft.Teams.Apps/AppBuilder.cs

    public App Build()
    {
-        return new App(_options);
+        return new App(_serviceProvider.GetService<IHttpCredentials>()!, _options);


The Build() method will throw a NullReferenceException when called on an AppBuilder instance created with the parameterless constructor (line 21-25), since _serviceProvider is set to null!. Consider adding a null check and appropriate error handling:

public App Build() { var credentials = _serviceProvider?.GetService<IHttpCredentials>(); return new App(credentials!, _options); }

Or throw a more descriptive exception if _serviceProvider is null.

Suggested change

return new App(_serviceProvider.GetService<IHttpCredentials>()!, _options);

IHttpCredentials? credentials = null;

if (_serviceProvider != null)

{

credentials = _serviceProvider.GetService<IHttpCredentials>();

}

if (credentials == null)

{

credentials = _options.Credentials;

}

if (credentials == null)

{

throw new InvalidOperationException("No IHttpCredentials available. Provide credentials via AddCredentials or ensure they are registered in the service provider.");

}

return new App(credentials, _options);

Copilot · 2025-11-13T17:02:42Z

Tests/Microsoft.Teams.Apps.Tests/AppTests.cs

            Credentials = credentials.Object,
        };
-        var app = new App(options);
+        var app = new App(credentials.Object,options);


Missing space after comma in constructor call. Should be:

var app = new App(credentials.Object, options);

Suggested change

var app = new App(credentials.Object,options);

var app = new App(credentials.Object, options);

Copilot · 2025-11-13T17:02:42Z

Tests/Microsoft.Teams.Apps.Tests/AppTests.cs

            Credentials = null,
        };
-        var app = new App(options);
+        var app = new App(null!, options);


Using null! (null-forgiving operator) to pass credentials suggests the IHttpCredentials parameter in the App constructor should be nullable. Consider making the parameter nullable (IHttpCredentials? credentials) to better reflect that null is an acceptable value, as demonstrated by the test case "Test_App_Start_DoesNot_GetBotToken_WhenNoCredentials".

Suggested change

var app = new App(null!, options);

var app = new App(null, options);

Copilot · 2025-11-13T17:02:43Z

.../Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/ApplicationBuilder.cs

    {
        var assembly = Assembly.GetEntryAssembly() ?? Assembly.GetCallingAssembly();
-        var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<AppOptions>());
+        var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!,builder.ApplicationServices.GetService<AppOptions>());


Missing space after comma in constructor call. Should be:

var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!, builder.ApplicationServices.GetService<AppOptions>());

Suggested change

var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!,builder.ApplicationServices.GetService<AppOptions>());

var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!, builder.ApplicationServices.GetService<AppOptions>());

Copilot · 2025-11-13T17:02:43Z

Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs

+        AuthorizationHeaderProviderOptions options = new();
+        options.AcquireTokenOptions = new AcquireTokenOptions()
        {
-            { "grant_type", "client_credentials" },
-            { "client_id", ClientId },
-            { "client_secret", ClientSecret },
-            { "scope", string.Join(",", scopes) }
        };


The AcquireTokenOptions is instantiated but left empty. Consider either:

Removing these lines if no options are needed: AuthorizationHeaderProviderOptions options = new();

Adding a comment explaining why an empty options object is required

Passing null if the API supports it

This will improve code clarity and reduce unnecessary object allocation.

Copilot · 2025-11-13T17:02:43Z

Tests/Microsoft.Teams.Apps.Tests/AppTests.cs

            Credentials = null,
        };
-        var app = new App(options);
+        var app = new App(null!, options);


This assignment to app is useless, since its value is never read.

Suggested change

var app = new App(null!, options);

rido-min · 2025-11-13T17:03:42Z

...Microsoft.Teams.Extensions.Hosting/Microsoft.Teams.Apps.Extensions/HostApplicationBuilder.cs

-                settings.TenantId
-            );
-        }
+        //if (options.Credentials is null && settings.ClientId is not null && settings.ClientSecret is not null && !settings.Empty)


Credentials are now managed by M.I.W, I leave this as commented just to see the diff, if we al agree I can delete the commented code

rido-min · 2025-11-13T17:04:16Z

...Microsoft.Teams.Extensions.Hosting/Microsoft.Teams.Apps.Extensions/HostApplicationBuilder.cs

-        var app = new App(options);
-
+        //var app = new App(options);
+        builder.Services.AddSingleton<App>();


this is the key part.. to properly register our app in DI.

rido-min · 2025-11-13T17:04:42Z

...ions/Microsoft.Teams.Extensions.Hosting/Microsoft.Teams.Apps.Extensions/ServiceCollection.cs

-        collection.AddSingleton<IContext.Accessor>();
-        return collection;
-    }
+    //public static IServiceCollection AddTeams(this IServiceCollection collection, AppOptions options)


this method was not called anywhere

rido-min · 2025-11-13T17:05:37Z

...rosoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore/Extensions/HostApplicationBuilder.cs

+        builder.Services.AddSingleton<AppOptions>();
+        builder.Services.Configure<MicrosoftIdentityApplicationOptions>(builder.Configuration.GetSection(authSectionName));
+#pragma warning disable ASP0000 // Use 'new(...)'
+        AppBuilder appBuilder = new AppBuilder(builder.Services.BuildServiceProvider());


this is the worst part of this approach.. Without revisiting DI, App, AppBuilder and AppOptions I could not find a better way

rido-min · 2025-11-13T17:06:07Z

Samples/Samples.Echo/appsettings.json

      "Level": "debug"
    }
  },
  "Teams": {


This is the config provided by M.I.W. I just renamed to Teams

rido-min · 2025-11-13T17:06:23Z

Samples/Samples.Echo/Program.cs

I wanted to refactor the Echo to use minimal APIs

rido-min added 3 commits November 13, 2025 06:16

Add M.I.W package

fc0b30c

working with AzureAD section

1d1dc99

configure M.I.W in app builder

cb348e3

rido-min requested review from aacebo, Copilot and singhk97 and removed request for aacebo and singhk97 November 13, 2025 16:58

Copilot started reviewing on behalf of rido-min November 13, 2025 16:59 View session

rido-min commented Nov 13, 2025

View reviewed changes

Libraries/Microsoft.Teams.Api/Auth/ClientCredentials.cs

Copy link

Member Author

rido-min Nov 13, 2025

Choose a reason for hiding this comment

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

replaced to use M.I.W that already handles different credential types

rido-min commented Nov 13, 2025

View reviewed changes

Copilot finished reviewing on behalf of rido-min November 13, 2025 17:01

Copilot AI reviewed Nov 13, 2025

View reviewed changes

rido-min commented Nov 13, 2025

View reviewed changes

Samples/Samples.Echo/Program.cs

Copy link

Member Author

rido-min Nov 13, 2025

Choose a reason for hiding this comment

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

I wanted to refactor the Echo to use minimal APIs

Merge branch 'main' into feat/msal

0a50d15

-            AccessToken = tokenResult.Substring("Bearer ".Length),
+            AccessToken = tokenResult.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)
+                ? tokenResult.Substring("Bearer ".Length)
+                : tokenResult,

	var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(scopes[0], options, cancellationToken);
	var tokenResult = await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync(string.Join(" ", scopes), options, cancellationToken);

-        return new App(_serviceProvider.GetService<IHttpCredentials>()!, _options);
+        IHttpCredentials? credentials = null;
+        if (_serviceProvider != null)
+        {
+            credentials = _serviceProvider.GetService<IHttpCredentials>();
+        }
+        if (credentials == null)
+        {
+            credentials = _options.Credentials;
+        }
+        if (credentials == null)
+        {
+            throw new InvalidOperationException("No IHttpCredentials available. Provide credentials via AddCredentials or ensure they are registered in the service provider.");
+        }
+        return new App(credentials, _options);

	var app = new App(credentials.Object,options);
	var app = new App(credentials.Object, options);

	var app = new App(null!, options);
	var app = new App(null, options);

	var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!,builder.ApplicationServices.GetService<AppOptions>());
	var app = builder.ApplicationServices.GetService<App>() ?? new App(builder.ApplicationServices.GetService<IHttpCredentials>()!, builder.ApplicationServices.GetService<AppOptions>());

Use MSAL to mint tokens #203

Are you sure you want to change the base?

Use MSAL to mint tokens #203

Uh oh!

Conversation

rido-min commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants