Fix AccessTokenCallback to match AccessToken behavior for TNIR and connection pool keys

[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.

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").

2. Pool key no longer drops sibling authentication state.
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. These now preserve sibling state, matching the ConnectionString setter, which already did this correctly.

AccessToken and AccessTokenCallback are already mutually exclusive (validated in CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessToken*), so that pairing was benign; SspiContextProvider is not mutually exclusive with either, so that one was a live defect.

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.TestShouldDisableTnirWithAccessToken (netfx only) — token/no-token x Azure/non-Azure endpoint x explicit/absent TNIR keyword.
• ConnectionTests.AccessTokenStateIsPreservedInPoolKeyWhenSspiContextProviderIsSet — verified to fail without the SqlConnection.cs change.
• ConnectionTests.AccessTokenAndAccessTokenCallbackAreMutuallyExclusive — pins the invariant that makes the token pairing safe.

Compatibility

Behavior change is limited to .NET Framework, and only for 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.

Checklist

• Tests added or updated
• Public API changes documented (no public API changes)
• Verified against customer repro (if applicable)
• Ensure no breaking changes introduced

Suggested release note

Fixed SqlConnection.AccessTokenCallback not disabling Transparent Network IP Resolution by default on .NET Framework, making it consistent with SqlConnection.AccessToken. Also fixed the AccessToken, AccessTokenCallback and SspiContextProvider setters discarding each other's values 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 (11) pass locally on net8.0; 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.

[mdaigle]

SSPI is an alternative to token auth. We shouldn't preserve access token or callback. The miss was that it should have been treated as mutually exclusive and thrown an exception if either of the others was already set.