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.
• Full net8.0 unit test suite was still running locally when this was opened; CI will cover it. net462 tests cannot be executed on the macOS dev machine used here, though the net462 target builds clean.

[Copilot]

Pull request overview

This PR fixes a behavioral inconsistency in the .NET Framework TNIR (Transparent Network IP Resolution) defaulting logic by treating SqlConnection.AccessTokenCallback the same as SqlConnection.AccessToken, and hardens connection pooling behavior by ensuring authentication-related properties don’t silently drop each other from the ConnectionPoolKey.

Changes:

• Added a single internal “token was supplied” signal (SqlConnectionInternal.IsAccessTokenProvided) and used it in TNIR-related and pre-login handshake logic.
• Updated AccessToken, AccessTokenCallback, and SspiContextProvider setters to preserve sibling authentication state when rebuilding the connection pool key.
• Added/extended unit tests to cover TNIR disabling logic (netfx) and pool-key/auth-property invariants.

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs	Adds regression tests ensuring pool key preserves access-token state when setting SspiContextProvider, and that AccessToken/AccessTokenCallback remain mutually exclusive.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionOptionsTest.cs	Adds netfx-only unit test coverage for ShouldDisableTnir across token-provided, Azure endpoint, and explicit TNIR keyword cases.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	Uses IsAccessTokenProvided for fed-auth-required detection and certificate-validation gating during pre-login handshake.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs	Preserves sibling authentication values when rebuilding ConnectionPoolKey in AccessToken, AccessTokenCallback, and SspiContextProvider setters.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Introduces IsAccessTokenProvided and updates netfx TNIR decision logic to take a unified access-token-provided 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 1 comment.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs:3938

• <see cref="SqlConnection.AccessToken"/> / <see cref="SqlConnection.AccessTokenCallback"/> in this new XML doc block is likely unresolved in this namespace, which can raise doc warnings (CS1574) and fail builds due to warnings-as-errors. Use fully-qualified cref targets here as well.

        /// <param name="isAccessTokenProvided">
        /// True when the caller supplied a federated authentication access token directly, either
        /// via <see cref="SqlConnection.AccessToken"/> or
        /// <see cref="SqlConnection.AccessTokenCallback"/>.
        /// </param>

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs:158

• The XML doc comment for IsAccessTokenProvided nests <remarks> inside <summary>, which is invalid XML documentation structure and can lead to doc build warnings or the remarks being ignored. Close the <summary> before starting <remarks>.

        /// <summary>
        /// True when the caller supplied a federated authentication access token directly, either
        /// as a literal token via <see cref="global::Microsoft.Data.SqlClient.SqlConnection.AccessToken"/> or as a token provider
        /// via <see cref="global::Microsoft.Data.SqlClient.SqlConnection.AccessTokenCallback"/>.
        /// <remarks>

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs:3937

• The XML doc <see cref="SqlConnection.*"/> references in this file are not in scope (this namespace doesn’t using Microsoft.Data.SqlClient;), which will produce CS1574 XML comment warnings; with TreatWarningsAsErrors enabled, that can break net462 builds. Use fully-qualified global::Microsoft.Data.SqlClient.SqlConnection.* like the other XML docs in this file.

        /// via <see cref="SqlConnection.AccessToken"/> or
        /// <see cref="SqlConnection.AccessTokenCallback"/>.

[cheenamalhotra]

Superseded by #4520, which is branched directly in dotnet/SqlClient (same commits) so the full CI and ADO pipeline validation runs against it.