Add token binding to MicrosoftIdentityMessageHandler by cpp11nullptr · Pull Request #3743 · AzureAD/microsoft-identity-web

cpp11nullptr · 2026-03-04T17:20:37Z

No description provided.

…andler-with-token-binding

bgavrilMS · 2026-03-05T15:17:11Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

            }
+            catch (MicrosoftIdentityAuthenticationException)
+            {
+                throw;


Was this forgotten?

bgavrilMS · 2026-03-05T15:18:13Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs


        /// <summary>
        /// Sends an HTTP request with authentication header injection.
+        /// When token binding (mTLS PoP) is configured via <see cref="AuthorizationHeaderProviderOptions.ProtocolScheme"/>,


nit: use a remarks section for this, it's quite advanced scenario.

bgavrilMS · 2026-03-05T15:23:44Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="MicrosoftIdentityMessageHandler"/> class with mTLS PoP token binding support.


nit: in public docs, let's mention the RFC 8705

jmprieur · 2026-03-05T16:45:01Z

docs/calling-downstream-apis/custom-apis.md

+    ],
+    "SendX5c": true
+  },
+  "SecureApi": {


Please use the same formalism as the rest of the docs:

"DownstreamApis": { "SecureApi":{ } }

jmprieur

There are a couple of things to improve:

the RequiredService seems inconsistent with the option. Either use GetService or explain why required is fine. I think it's a breaking change, personally. you don't want to break Aspire!!
Fragile code that can be improved
+some questions/remarks

jmprieur · 2026-03-05T16:46:24Z

docs/calling-downstream-apis/custom-apis.md

+    client.BaseAddress = new Uri("https://secure-api.example.com");
+})
+.AddMicrosoftIdentityMessageHandler(
+    configuration.GetSection("SecureApi"),


would become DownstreamApis:SecureApi

jmprieur · 2026-03-05T16:48:26Z

docs/calling-downstream-apis/token-binding.md

+**appsettings.json:**
+```json
+{
+  "SecureApi": {


Same thing: please have a DownstreamApis section in which the secure Api is to maintain consistency with the rest of the samples in the library.

jmprieur · 2026-03-05T16:50:10Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityHttpClientBuilderExtensions.cs

            {
                var headerProvider = sp.GetRequiredService<IAuthorizationHeaderProvider>();
-                return new MicrosoftIdentityMessageHandler(headerProvider);
+                var httpClientFactory = sp.GetRequiredService<IHttpClientFactory>();


Isn't that a breaking change? Is the IHttpClientFactory always provided?
Shouln'd you use GetService and test if it's null? Given the other constructor if MicrosoftIdentityMessageHandler accepts null, it's strange to have a required service that could throw?

jmprieur · 2026-03-05T16:50:34Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityHttpClientBuilderExtensions.cs

            {
                var headerProvider = sp.GetRequiredService<IAuthorizationHeaderProvider>();
-                return new MicrosoftIdentityMessageHandler(headerProvider, options);
+                var httpClientFactory = sp.GetRequiredService<IHttpClientFactory>();


Same question. Please explain why this is ok as writen.

jmprieur · 2026-03-05T16:50:42Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityHttpClientBuilderExtensions.cs


                var headerProvider = sp.GetRequiredService<IAuthorizationHeaderProvider>();
-                return new MicrosoftIdentityMessageHandler(headerProvider, options);
+                var httpClientFactory = sp.GetRequiredService<IHttpClientFactory>();


jmprieur · 2026-03-05T16:50:45Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityHttpClientBuilderExtensions.cs


                var headerProvider = sp.GetRequiredService<IAuthorizationHeaderProvider>();
-                return new MicrosoftIdentityMessageHandler(headerProvider, options);
+                var httpClientFactory = sp.GetRequiredService<IHttpClientFactory>();


jmprieur · 2026-03-05T16:52:44Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

+        /// If <see langword="null"/>, each request must specify its own authentication options or an exception will be thrown.
+        /// </param>
+        /// <param name="mtlsHttpClientFactory">
+        /// Optional factory for creating HTTP clients configured with mTLS client certificates for token binding


Optional ... so why do the Add use a Required service?
There is an inconsistency to resolve here?

jmprieur · 2026-03-05T16:53:45Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

+        /// </exception>
+        /// <example>
+        /// <para>Usage with mTLS PoP token binding:</para>
+        /// <code>


Please remove this sample which has no place here. You don't want customers to instanciate MicrosoftIdentityMessageHandler directly but use the extension methods.

jmprieur · 2026-03-05T16:54:51Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

            }
+            catch (MicrosoftIdentityAuthenticationException)
+            {
+                throw;


jmprieur · 2026-03-05T16:55:56Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

+        private static DownstreamApiOptions CreateDownstreamApiOptionsFromMessageHandlerOptions(
+            MicrosoftIdentityMessageHandlerOptions options, IList<string> scopes)
+        {
+            return new DownstreamApiOptions


This is fragile.
Please add a copy constructor in MicrosoftIdentityMessageHandlerOptions leveraging the AuthorizationHeaderProviderOptions copy constructor and copying the scopes (maybe with cloning the collection)?

bgavrilMS · 2026-03-05T17:42:26Z

@tlupes - this is related to pure mTLS option, a review from you would be good.

tlupes · 2026-03-05T21:56:03Z

src/Microsoft.Identity.Web.TokenAcquisition/MicrosoftIdentityMessageHandler.cs

+                string authHeader;
+
+                // Check if mTLS PoP token binding is requested
+                if (string.Equals(options.ProtocolScheme, TokenBindingProtocolScheme, StringComparison.OrdinalIgnoreCase)


This logic looks nearly identical to the logic for DownstreamApi; can we centralize this logic, or is the plan to use this message handler internally to DownstreamApi?

tlupes · 2026-03-05T22:04:52Z

@tlupes - this is related to pure mTLS option, a review from you would be good.

It looks very similar to how DownstreamApi works, which means it should be easy to transfer over. Centralizing the logic would make things easier.

Add token binding to MicrosoftIdentityMessageHandler

8be50c8

cpp11nullptr requested a review from a team as a code owner March 4, 2026 17:20

Merge branch 'master' into iepoly/extend-microsoft-identity-message-h…

b0c1184

…andler-with-token-binding

bgavrilMS reviewed Mar 5, 2026

View reviewed changes

bgavrilMS approved these changes Mar 5, 2026

View reviewed changes

jmprieur reviewed Mar 5, 2026

View reviewed changes

jmprieur requested changes Mar 5, 2026

View reviewed changes

tlupes reviewed Mar 5, 2026

View reviewed changes

Conversation

cpp11nullptr commented Mar 4, 2026

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!

jmprieur left a comment

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!

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!

bgavrilMS commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlupes commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants