Fix AccessTokenCallback TNIR behavior and make SspiContextProvider mutually exclusive with token auth

[cheenamalhotra]

Follow-up to the review discussion on #4493, where it was noted that the TNIR behavior documented there does not actually apply when SqlConnection.AccessTokenCallback is used:

Since this is a fairly new introduction, we should fix it in our driver to match what happens in the AccessToken usecase. Documenting this would not be needed in that case.

The bug

On .NET Framework the driver disables Transparent Network IP Resolution by default whenever federated authentication is in use, unless the caller explicitly specified the TransparentNetworkIPResolution keyword. SqlConnectionInternal.ShouldDisableTnir only tested _accessTokenInBytes (SqlConnection.AccessToken) and ignored _accessTokenCallback (SqlConnection.AccessTokenCallback), so the two token-supplying APIs behaved differently for no good reason.

Investigating the fix surfaced a second, coupled defect in how the authentication setters build the connection pool key. Both are fixed here.

Changes

1. Single source of truth for "a token was supplied."
Added SqlConnectionInternal.IsAccessTokenProvided and used it in all three places that previously inlined the field checks — ShouldDisableTnir plus two spots in TdsParser.ConsumePreLoginHandshake. The duplicated hand-written expression is exactly what let these paths drift apart, and the existing @TODO in OnFedAuthInfo predicted this ("we're gonna forget one in one spot and cause a big ol bug someday").

The property's doc comment is deliberately scoped: it is the right check for prelogin FEDAUTHREQUIRED, server certificate validation and TNIR, but not for login feature-extension negotiation, which must keep testing the fields individually because _accessTokenCallback selects FedAuthLibrary.MSAL while _accessTokenInBytes selects FedAuthLibrary.SecurityToken. Calling that out prevents a future "cleanup" from collapsing those two sites and breaking fedauth login.

2. SspiContextProvider is now mutually exclusive with token authentication.
The AccessToken, AccessTokenCallback and SspiContextProvider setters each rebuilt the ConnectionPoolKey with the sibling authentication values hard-coded to null. Setting SspiContextProvider silently dropped a previously assigned access token or callback from the pool key, so it never reached the internal connection even though the public property still reported it as set — which would also have defeated the TNIR fix above.

SSPI is an alternative to token-based authentication rather than a complement to it, so the correct fix is not to preserve both but to reject the combination outright:

• CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessToken / ...AccessTokenCallback now throw if a context provider is already set.
• New CheckAndThrowOnInvalidCombinationOfConnectionOptionAndSspiContextProvider, called from the SspiContextProvider setter, throws if either token property is already set.
• Two new resource strings, one per direction, so the message names the property the caller actually set.

AccessToken and AccessTokenCallback were already mutually exclusive (validated in CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessToken*), so that pairing was always benign; SspiContextProvider was the live defect.

The setters still pass the sibling values through when constructing the ConnectionPoolKey. With the new validation in place this is a no-op whenever a non-null value is assigned — the siblings are guaranteed null — so it matters only on the clearing path. Validation only runs when value != null (matching the existing convention in the token setters), so conn.AccessToken = "t"; conn.SspiContextProvider = null; is a clear, not a combination, and does not throw. Passing literal nulls there would wipe the token from the pool key while conn.AccessToken still reported it as set, which is the same class of bug this PR is fixing.

3. Tests.
ShouldDisableTnir is now internal static so the decision matrix can be unit tested directly (constructing a SqlConnectionInternal in a unit test is impractical). Added:

• SqlConnectionOptionsTest.TestShouldDisableTnirWithCallerSuppliedToken (netfx only) — token/no-token x Azure/non-Azure endpoint x explicit/absent TNIR keyword.
• ConnectionTests.SspiContextProviderAndAccessTokenStateAreMutuallyExclusive — all four assignment orderings across both token properties.
• ConnectionTests.ClearingOneAuthPropertyPreservesTheOthersInPoolKey — pins the clearing-path behavior described above; verified to fail without the SqlConnection.cs change.
• ConnectionTests.AccessTokenAndAccessTokenCallbackAreMutuallyExclusive — pins the pre-existing invariant that makes the token pairing safe.

Compatibility

The TNIR change is limited to .NET Framework, and only to connections using AccessTokenCallback, which now get the same TNIR default as AccessToken. Users who explicitly set TransparentNetworkIPResolution in the connection string are unaffected — the explicit keyword still takes precedence, so the escape hatch documented in #4493 continues to work.

The SSPI change is a deliberate behavior change on all targets: assigning SspiContextProvider alongside AccessToken/AccessTokenCallback now throws InvalidOperationException instead of silently producing an ambiguous authentication state. Any code relying on the old behavior was already broken — the token was being dropped from the pool key — so this converts silent misbehavior into a clear, actionable error. Clearing a property (assigning null) is unaffected and never throws.

Checklist

• Tests added or updated
• Public API changes documented (no public API surface changes)
• Verified against customer repro (if applicable)
• Ensure no breaking changes introduced — see the SSPI note under Compatibility: combining SSPI with token authentication now throws rather than silently dropping the token. Flagging explicitly for reviewer sign-off.

Suggested release note

Fixed SqlConnection.AccessTokenCallback not disabling Transparent Network IP Resolution by default on .NET Framework, making it consistent with SqlConnection.AccessToken. SqlConnection.SspiContextProvider is now correctly treated as mutually exclusive with AccessToken and AccessTokenCallback and throws when combined, instead of silently discarding the token from the connection pool key.

Notes for reviewers

• Docs are intentionally untouched: docs: correct TransparentNetworkIPResolution behavior wording (Fixes #4489) #4493 already updates the TNIR wording to say TNIR is disabled when Entra ID auth or an access token is used, and this change makes that statement true for the callback path.
• Targeted unit tests pass locally on net8.0, including the existing SspiTests suite; the net462 target builds clean but its tests cannot be executed on the macOS dev machine used here, so CI covers that leg.
• Supersedes Fix AccessTokenCallback to match AccessToken behavior for TNIR and connection pool keys #4518, which was opened from a fork; this PR is branched directly in dotnet/SqlClient so the full CI/ADO pipeline suite runs.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR tightens behavior around federated authentication state by ensuring access-token state is consistently represented in connection pooling keys and by unifying “caller-supplied token” detection for TNIR and pre-login logic.

Changes:

• Preserve AccessToken / AccessTokenCallback / SspiContextProvider in ConnectionPoolKey updates to avoid silently dropping authentication state.
• Introduce SqlConnectionInternal.IsAccessTokenProvided and apply it in pre-login and certificate validation checks.
• Add NETFRAMEWORK TNIR tests and simulated server tests covering token/callback state and mutual exclusivity.

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs	Adds regression tests for pool-key preservation and token/callback mutual exclusivity.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionOptionsTest.cs	Adds NETFRAMEWORK theory coverage for TNIR disablement with caller-supplied tokens.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	Switches token detection to unified IsAccessTokenProvided.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs	Updates pool-key rebuilds to preserve other auth-related properties.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Adds IsAccessTokenProvided and refactors NETFRAMEWORK TNIR logic to accept it as an input.

---

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

[Copilot]

Pull request overview

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.66%. Comparing base (9b20e5e) to head (feb5e85).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4520      +/-   ##
==========================================
- Coverage   64.73%   62.66%   -2.08%     
==========================================
  Files         288      283       -5     
  Lines       44088    67057   +22969     
==========================================
+ Hits        28542    42020   +13478     
- Misses      15546    25037    +9491     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	62.66% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

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

Files not reviewed (1)

• src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

[Copilot]

Pull request overview

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

Files not reviewed (1)

• src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

[mdaigle]

Requesting changes for the clone bypass, the Credential setter pool-key divergence, and missing TNIR coverage through the connection open paths.

[mdaigle]

Could we copy _sspiContextProvider in the copy constructor and add a clone regression test? ICloneable.Clone() copies _accessToken and _accessTokenCallback, but not _sspiContextProvider, while CopyFrom retains the source PoolGroup. I verified that the clone reports SspiContextProvider == null while its pool key still carries the provider, and assigning AccessToken to the clone succeeds instead of throwing. This bypasses the new mutual-exclusivity validation and can authenticate with state that the public properties do not report.

[mdaigle]

Could we make the Credential setter preserve or validate SspiContextProvider? It still rebuilds the pool key with sspiContextProvider: null. I verified that after setting a provider and then Credential, the public property still reports the provider while the pool key has dropped it. This leaves the same property/pool-key divergence this change fixes in the other setters.

[mdaigle]

Could we add regression coverage through both Open and OpenAsync with AccessTokenCallback? This theory supplies isAccessTokenProvided as a literal, so it does not exercise IsAccessTokenProvided or LoginNoFailover. Reverting the property to _accessTokenInBytes != null leaves these tests green. Please also cover an explicit TransparentNetworkIPResolution=false; the current explicit-keyword rows only test true.