diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index fd8817b9aa..97bd473568 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -145,6 +145,19 @@ internal class SqlConnectionInternal : DbConnectionInternal, IDisposable // @TODO: Probably a good idea to introduce a delegate type internal readonly Func> _accessTokenCallback; + /// + /// True when the caller supplied a federated authentication access token directly, either + /// as a literal token via or as a token provider + /// via . + /// + /// + /// Both paths represent the same "caller-supplied token" authentication mode, so they must + /// always be treated identically. Use this property rather than testing the underlying + /// fields individually. + /// + internal bool IsAccessTokenProvided => + _accessTokenInBytes != null || _accessTokenCallback != null; + // @TODO: Should be private and accessed via internal property // @TODO: Rename to match naming conventions internal bool _cleanSQLDNSCaching = false; @@ -3115,7 +3128,9 @@ private void LoginNoFailover( #if NET bool isParallel = connectionOptions.MultiSubnetFailover; #else - bool disableTnir = ShouldDisableTnir(connectionOptions); + bool disableTnir = ShouldDisableTnir( + connectionOptions, + isAccessTokenProvided: IsAccessTokenProvided); bool isParallel = connectionOptions.MultiSubnetFailover || (connectionOptions.TransparentNetworkIPResolution && !disableTnir); #endif @@ -3911,12 +3926,29 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup, } #if NETFRAMEWORK - private bool ShouldDisableTnir(SqlConnectionOptions connectionOptions) + /// + /// Determines whether Transparent Network IP Resolution (TNIR) should be disabled for this + /// connection attempt. + /// + /// The parsed connection options. + /// + /// True when the caller supplied a federated authentication access token directly, either + /// via or + /// . + /// + /// + /// True when TNIR should be disabled. TNIR is disabled by default for Azure SQL endpoints + /// and for federated authentication, but an explicit + /// TransparentNetworkIPResolution keyword always takes precedence. + /// + internal static bool ShouldDisableTnir( + SqlConnectionOptions connectionOptions, + bool isAccessTokenProvided) { bool isAzureEndPoint = ADP.IsAzureSqlServerEndpoint(connectionOptions.DataSource); // @TODO: Turn into a HashSet and just check the list instead of this MESS. - bool isFedAuthEnabled = _accessTokenInBytes != null || + bool isFedAuthEnabled = isAccessTokenProvided || #pragma warning disable 0618 // Type or member is obsolete connectionOptions.Authentication == SqlAuthenticationMethod.ActiveDirectoryPassword || #pragma warning restore 0618 // Type or member is obsolete diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index c86b30525a..280898fd7a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -763,8 +763,15 @@ public string AccessToken CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessToken(ConnectionOptions); } - // Need to call ConnectionString_Set to do proper pool group check - ConnectionString_Set(new ConnectionPoolKey(_connectionString, credential: _credential, accessToken: value, accessTokenCallback: null, sspiContextProvider: null)); + // Need to call ConnectionString_Set to do proper pool group check. + // Preserve the other authentication state so it isn't dropped from the pool key + // (see the ConnectionString setter, which is the reference for this pattern). + ConnectionString_Set(new ConnectionPoolKey( + _connectionString, + credential: _credential, + accessToken: value, + accessTokenCallback: _accessTokenCallback, + sspiContextProvider: _sspiContextProvider)); _accessToken = value; } } @@ -787,7 +794,12 @@ public Func + /// TNIR is disabled by default whenever federated authentication is in play, including when + /// the token is supplied directly through AccessToken or AccessTokenCallback, + /// unless the user explicitly specified the TNIR keyword. + /// + [Theory] + // Non-Azure endpoint, no explicit TNIR keyword: access token (or callback) disables TNIR. + [InlineData("my.test.server", false, false, false)] + [InlineData("my.test.server", true, false, true)] + // Azure endpoint always disables TNIR when the keyword is absent. + [InlineData("test.database.windows.net", false, false, true)] + [InlineData("test.database.windows.net", true, false, true)] + // An explicit TNIR keyword always wins, regardless of access token or endpoint. + [InlineData("my.test.server", true, true, false)] + [InlineData("test.database.windows.net", true, true, false)] + [InlineData("test.database.windows.net", false, true, false)] + public void TestShouldDisableTnirWithAccessToken( + string dataSource, + bool isAccessTokenProvided, + bool tnirExplicitlySpecified, + bool expectedValue) + { + SqlConnectionStringBuilder builder = new() { DataSource = dataSource }; + if (tnirExplicitlySpecified) + { + builder.TransparentNetworkIPResolution = true; + } + + SqlConnectionOptions connectionOptions = new(builder.ConnectionString); + + Assert.Equal( + expectedValue, + SqlConnectionInternal.ShouldDisableTnir(connectionOptions, isAccessTokenProvided)); + } #endif /// /// Test MSF values when set through connection string and through app context switch. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs index a082c8c7e6..f16844ea83 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs @@ -764,6 +764,65 @@ public void ConnectionTestAccessTokenCallbackCombinations() } } + /// + /// Setting one authentication-related property must not silently drop the others from the + /// connection pool key. Previously, assigning + /// rebuilt the pool key with a null (and a + /// null ), so the token was never handed to the + /// internal connection even though the public property still reported it as set. + /// + [Fact] + public void AccessTokenStateIsPreservedInPoolKeyWhenSspiContextProviderIsSet() + { + Func> callback = + (ctx, token) => Task.FromResult(new SqlAuthenticationToken("invalid", DateTimeOffset.MaxValue)); + + using (SqlConnection conn = new("Data Source=localhost")) + { + conn.AccessTokenCallback = callback; + Assert.Same(callback, conn.PoolGroup.PoolKey.AccessTokenCallback); + + conn.SspiContextProvider = null; + + Assert.Same(callback, conn.AccessTokenCallback); + Assert.Same(callback, conn.PoolGroup.PoolKey.AccessTokenCallback); + } + + using (SqlConnection conn = new("Data Source=localhost")) + { + conn.AccessToken = "token"; + Assert.Equal("token", conn.PoolGroup.PoolKey.AccessToken); + + conn.SspiContextProvider = null; + + Assert.Equal("token", conn.AccessToken); + Assert.Equal("token", conn.PoolGroup.PoolKey.AccessToken); + } + } + + /// + /// and + /// are mutually exclusive, so neither setter can ever clobber a live value of the other. + /// + [Fact] + public void AccessTokenAndAccessTokenCallbackAreMutuallyExclusive() + { + Func> callback = + (ctx, token) => Task.FromResult(new SqlAuthenticationToken("invalid", DateTimeOffset.MaxValue)); + + using (SqlConnection conn = new("Data Source=localhost")) + { + conn.AccessTokenCallback = callback; + Assert.Throws(() => conn.AccessToken = "token"); + } + + using (SqlConnection conn = new("Data Source=localhost")) + { + conn.AccessToken = "token"; + Assert.Throws(() => conn.AccessTokenCallback = callback); + } + } + [Theory] [InlineData(9, 0, 2047)] // SQL Server 2005 [InlineData(10, 0, 2531)] // SQL Server 2008