Skip to content

GA confidential client extensibility APIs - #6216

Merged
Gladwin Johnson VR (gladjohn) merged 5 commits into
mainfrom
gladjohn/ga-confidential-extensibility
Oct 11, 2026
Merged

Gladwin Johnson VR (gladjohn) merged 5 commits into
mainfrom
gladjohn/ga-confidential-extensibility

Conversation

@gladjohn

@gladjohn Gladwin Johnson VR (gladjohn) commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • remove the experimental feature opt-in requirement from the dynamic certificate provider API
  • GA OnMsalServiceFailure, OnCompletion, and confidential-client OnBackgroundTokenRefreshCompleted
  • update confidential-client tests to exercise these APIs without calling WithExperimentalFeatures()
  • document that static WithCertificate(X509Certificate2, ...) overloads capture one certificate and direct externally managed, rotating, or invalidatable key-handle scenarios to the dynamic certificate provider

Testing

  • full Microsoft.Identity.Test.Unit suite: .NET 8 — 2,383 passed, 25 skipped; .NET Framework 4.8 — 2,388 passed, 22 skipped
  • Microsoft.Identity.Client builds successfully across its standard target frameworks

Compatibility

  • no public API signatures changed, so no Public API baseline update is required
  • the expected Microsoft.Identity.Web sibling checkout at D:\source\microsoft-identity-web was not present; searched APIs: WithCertificate, OnMsalServiceFailure, OnCompletion, and OnBackgroundTokenRefreshCompleted

assistance: agentic-cli
type: feature
agent-tool: copilot-cli
agent-model: gpt-5.6-sol
work-item: AB#n/a

Remove the experimental feature opt-in from the dynamic certificate and confidential client callback APIs, and update unit coverage to exercise them without experimental features enabled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e29c0e10-6991-478c-a158-1d97f2a8d75f
Copilot AI balanced review requested due to automatic review settings October 10, 2026 21:07
@gladjohn Gladwin Johnson VR (gladjohn) added feature agentic-cli Terminal AI agent drove the bulk of the edits labels Oct 10, 2026

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.

🟢 Approval recommended

The gates are removed consistently and existing unit coverage directly validates the promoted behavior.

0 open findings

What changed in this PR

Promotes confidential-client extensibility APIs to GA by removing their experimental-feature opt-in requirement.

Changes:

  • Removes experimental validation from four extensibility APIs.
  • Updates configuration, integration, and proactive-refresh tests to use them without opt-in.
File Description
ConfidentialClientApplicationBuilderExtensions.cs Removes experimental gates.
ConfidentialClientApplicationExtensibilityApiTests.cs Verifies configuration without opt-in.
ConfidentialClientApplicationExtensibilityTests.cs Exercises runtime behavior without opt-in.
RefreshInTests.cs Tests refresh callbacks without opt-in.

🧠 Review effort: Balanced


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

Guide callers with externally managed, rotating, or invalidatable certificate handles from the static certificate overloads to the dynamic provider API.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e29c0e10-6991-478c-a158-1d97f2a8d75f
Copilot AI balanced review requested due to automatic review settings October 10, 2026 21:20

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.

🟡 Changes recommended

The new public documentation incorrectly states that static certificates are captured at build time rather than when WithCertificate is called.

1 open finding

🧠 Review effort: Balanced

Updated remarks to clarify certificate handling and token request process.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 10, 2026 22:19

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.

🔵 Needs a closer look

The GA dynamic certificate provider silently ignores certificate-based cache partitioning during rotation.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Dynamic provider ignores certificate token association during cache partitioning

src/​client/​Microsoft.Identity.Client/​Extensibility/​ConfidentialClientApplicationBuilderExtensions.cs:55

The GA dynamic-provider overload still silently ignores CertificateOptions.AssociateTokensWithCertificate. Unlike the static overload, this method never sets a certificate-derived cache-key component, and AcquireTokenForClientParameterBuilder.Create only partitions the cache when CertificateIdToAssociateWithToken is populated. Consequently, after rotation a cache hit can return a token associated with the previous certificate even when this option is true. Please either resolve/use the current serial for cache reads and writes or reject this option for dynamic providers until it is supported.

🧠 Review effort: Balanced

Remove doubled carriage returns introduced in the certificate remarks so documentation generation succeeds across CI targets.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e29c0e10-6991-478c-a158-1d97f2a8d75f
Copilot AI balanced review requested due to automatic review settings October 10, 2026 22:40

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.

🔵 Needs a closer look

The new public API documentation inaccurately describes when static certificates are captured and when dynamic providers run.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Low severity Correct certificate capture and token request timing documentation

src/​client/​Microsoft.Identity.Client/​AppConfig/​ConfidentialClientApplicationBuilder.cs:125

This overload stores the supplied X509Certificate2 reference as soon as WithCertificate is called; Build() does not capture or clone it. Saying capture happens at build time can mislead callers about when an externally managed handle must remain valid, and “each token request” also includes cache hits where the dynamic provider is not invoked. Match the precise wording used by the overload above.

This issue also appears on line 158 of the same file.

🧠 Review effort: Balanced

Clarify that all static certificate overloads capture the supplied certificate when called and that the dynamic provider runs only for network token requests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e29c0e10-6991-478c-a158-1d97f2a8d75f
Copilot AI balanced review requested due to automatic review settings October 10, 2026 22: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.

🟢 Approval recommended

The focused guard removals, documentation, and corresponding test updates are consistent and complete.

0 open findings

🧠 Review effort: Balanced

This was referenced Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentic-cli Terminal AI agent drove the bulk of the edits feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants