From e70643df877c7185ee2a64ea1c88e51711215664 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 23 Jun 2026 06:24:00 +0100 Subject: [PATCH 1/9] Fix | Reenable SqlBulkCopy in least-privilege environments (#4306) --- .../Microsoft/Data/SqlClient/SqlBulkCopy.cs | 29 +- .../DatabaseObjects/DatabaseObject.cs | 282 ++++++++++++++++++ .../Fixtures/DatabaseObjects/DatabaseUser.cs | 67 +++++ .../Fixtures/DatabaseObjects/ServerLogin.cs | 99 ++++++ .../ManualTests/DataCommon/DataTestUtility.cs | 42 +++ .../Extensions/CodeAnalysis.netfx.cs | 25 ++ .../SQL/SqlBulkCopyTest/CopyAllFromReader.cs | 12 +- .../SQL/SqlBulkCopyTest/UnprivilegedLogin.cs | 208 +++++++++++++ 8 files changed, 751 insertions(+), 13 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs index 48d2e38d32..02e3e51606 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs @@ -472,11 +472,12 @@ private string CreateInitialQuery() } else if (!string.IsNullOrEmpty(CatalogName)) { - CatalogName = SqlServerEscapeHelper.EscapeStringAsLiteral(SqlServerEscapeHelper.EscapeIdentifier(CatalogName)); + CatalogName = SqlServerEscapeHelper.EscapeIdentifier(CatalogName); } string objectName = ADP.BuildMultiPartName(parts); string escapedObjectName = SqlServerEscapeHelper.EscapeStringAsLiteral(objectName); + string catalogNameStringLiteral = CatalogName is null ? null : SqlServerEscapeHelper.EscapeStringAsLiteral(CatalogName); // Specify the column names explicitly. This is to ensure that we can map to hidden // columns (e.g. columns in temporal tables.) If the target table doesn't exist, // OBJECT_ID will return NULL and @Column_Names will remain non-null. The subsequent @@ -512,6 +513,11 @@ private string CreateInitialQuery() // we use STRING_AGG in that case and the COALESCE method otherwise. // // See: https://learn.microsoft.com/en-us/sql/t-sql/functions/serverproperty-transact-sql + // + // All of this is wrapped in an test against HAS_PERMS_BY_NAME. This test verifies that + // the user possesses the necessary permissions to access sys.all_columns. If they do not + // @Column_Names will remain NULL (and be coalesced to *) and SqlBulkCopy will degrade + // gracefully, silently dropping support for hidden columns and column aliases. return $""" SELECT @@TRANCOUNT; @@ -521,6 +527,7 @@ private string CreateInitialQuery() DECLARE @Column_Name_Query_SORT NVARCHAR(MAX); DECLARE @Column_Name_Query NVARCHAR(MAX); DECLARE @Column_Names NVARCHAR(MAX) = NULL; +DECLARE @Has_Sys_All_Columns_Permissions INT = HAS_PERMS_BY_NAME('{catalogNameStringLiteral}.[sys].[all_columns]', 'OBJECT', 'SELECT'); IF CAST(SERVERPROPERTY('EngineEdition') AS INT) = 6 BEGIN @@ -533,17 +540,21 @@ IF CAST(SERVERPROPERTY('EngineEdition') AS INT) = 6 SET @Column_Name_Query_SORT = N'ORDER BY [column_id] ASC'; END -IF EXISTS (SELECT TOP 1 * FROM sys.all_columns WHERE [object_id] = OBJECT_ID('sys.all_columns') AND [name] = 'graph_type') -BEGIN - SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7)'; -END -ELSE +IF @Has_Sys_All_Columns_Permissions = 1 BEGIN - SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID'; + IF EXISTS (SELECT TOP 1 * FROM sys.all_columns WHERE [object_id] = OBJECT_ID('sys.all_columns') AND [name] = 'graph_type') + BEGIN + SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7)'; + END + ELSE + BEGIN + SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID'; + END + SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {catalogNameStringLiteral}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';' + + EXEC sp_executesql @Column_Name_Query, N'@Object_ID INT, @Column_Names NVARCHAR(MAX) OUTPUT', @Object_ID = @Object_ID, @Column_Names = @Column_Names OUTPUT; END -SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {CatalogName}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';' -EXEC sp_executesql @Column_Name_Query, N'@Object_ID INT, @Column_Names NVARCHAR(MAX) OUTPUT', @Object_ID = @Object_ID, @Column_Names = @Column_Names OUTPUT; SELECT @Column_Names = COALESCE(@Column_Names, '*'); SET FMTONLY ON; diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs new file mode 100644 index 0000000000..a74da79697 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs @@ -0,0 +1,282 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Text; + +namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; + +/// +/// Base class for a transient database object (such as a table, type or +/// stored procedure.) +/// +/// +/// The type of the internal state accessible to derived types at the point of object creation +/// via the property. +/// +public abstract class DatabaseObject : IDisposable +{ + private readonly bool _shouldDrop; + + protected SqlConnection Connection { get; } + + protected TState State { get; } + + public string Name { get; } + + public string UnescapedName => Name.Substring(1, Name.Length - 2).Replace("]]", "]"); + + protected DatabaseObject(SqlConnection connection, string name, string definition, TState state, bool shouldCreate, bool shouldDrop) + { + _shouldDrop = shouldDrop; + + Connection = connection; + State = state; + Name = name; + + if (shouldCreate) + { + EnsureConnectionOpen(); + DropObject(); + CreateObject(definition); + } + } + + private void EnsureConnectionOpen() + { + const int MaxWaits = 2; + int counter = MaxWaits; + + if (Connection.State is System.Data.ConnectionState.Closed) + { + Connection.Open(); + } + while (counter-- > 0 && Connection.State is System.Data.ConnectionState.Connecting) + { + Thread.Sleep(80); + } + } + + /// + /// Generate a new GUID and return the characters from its 1st and 4th + /// parts, as shown here: + /// + /// + /// 7ff01cb8-88c7-11f0-b433-00155d7e531e + /// ^^^^^^^^ ^^^^ + /// + /// + /// These 12 characters are concatenated together without any + /// separators. These 2 parts typically comprise a timestamp and clock + /// sequence, most likely to be unique for tests that generate names in + /// quick succession. + /// + private static string GetGuidParts() + { + var guid = Guid.NewGuid().ToString(); + // GOTCHA: The slice operator is inclusive of the start index and + // exclusive of the end index! + return guid.Substring(0, 8) + guid.Substring(19, 4); + } + + /// + /// Generate a long unique database object name, whose maximum length is + /// 96 characters, with the format: + /// + /// {Prefix}_{GuidParts}_{UserName}_{MachineName} + /// + /// The Prefix will be truncated to satisfy the overall maximum length. + /// + /// The GUID Parts will be the characters from the 1st and 4th blocks + /// from a traditional string representation, as shown here: + /// + /// + /// 7ff01cb8-88c7-11f0-b433-00155d7e531e + /// ^^^^^^^^ ^^^^ + /// + /// + /// These 2 parts typically comprise a timestamp and clock sequence, + /// most likely to be unique for tests that generate names in quick + /// succession. The 12 characters are concatenated together without any + /// separators. + /// + /// The UserName and MachineName are obtained from the Environment, + /// and will be truncated to satisfy the maximum overall length. + /// + /// + /// + /// The prefix to use when generating the unique name, truncated to at + /// most 32 characters. + /// + /// This should not contain any characters that cannot be used in + /// database object names. See: + /// + /// https://learn.microsoft.com/en-us/sql/relational-databases/databases/database-identifiers?view=sql-server-ver17#rules-for-regular-identifiers + /// + /// + /// + /// When true, the entire generated name will be enclosed in square + /// brackets, for example: + /// + /// [MyPrefix_7ff01cb811f0_test_user_ci_agent_machine_name] + /// + /// + /// + /// A unique database object name, no more than 96 characters long. + /// + public static string GenerateLongName(string prefix, bool escape = true) + { + StringBuilder name = new(96); + + if (escape) + { + name.Append('['); + } + + if (prefix.Length > 32) + { + prefix = prefix.Substring(0, 32); + } + + name.Append(prefix); + name.Append('_'); + name.Append(GetGuidParts()); + name.Append('_'); + + var suffix = + Environment.UserName + '_' + + Environment.MachineName; + + int maxSuffixLength = 96 - name.Length; + if (escape) + { + --maxSuffixLength; + } + if (suffix.Length > maxSuffixLength) + { + suffix = suffix.Substring(0, maxSuffixLength); + } + + name.Append(suffix); + + if (escape) + { + name.Append(']'); + } + + return name.ToString(); + } + + /// + /// Generate a short unique database object name, whose maximum length + /// is 30 characters, with the format: + /// + /// {Prefix}_{GuidParts} + /// + /// The Prefix will be truncated to satisfy the overall maximum length. + /// + /// The GUID parts will be the characters from the 1st and 4th blocks + /// from a traditional string representation, as shown here: + /// + /// + /// 7ff01cb8-88c7-11f0-b433-00155d7e531e + /// ^^^^^^^^ ^^^^ + /// + /// + /// These 2 parts typically comprise a timestamp and clock sequence, + /// most likely to be unique for tests that generate names in quick + /// succession. The 12 characters are concatenated together without any + /// separators. + /// + /// + /// + /// The prefix to use when generating the unique name, truncated to at + /// most 18 characters when withBracket is false, and 16 characters when + /// withBracket is true. + /// + /// This should not contain any characters that cannot be used in + /// database object names. See: + /// + /// https://learn.microsoft.com/en-us/sql/relational-databases/databases/database-identifiers?view=sql-server-ver17#rules-for-regular-identifiers + /// + /// + /// + /// When true, the entire generated name will be enclosed in square + /// brackets, for example: + /// + /// [MyPrefix_7ff01cb811f0] + /// + /// + /// + /// A unique database object name, no more than 30 characters long. + /// + public static string GenerateShortName(string prefix, bool escape = true) + { + StringBuilder name = new(30); + + if (escape) + { + name.Append('['); + } + + int maxPrefixLength = escape ? 16 : 18; + if (prefix.Length > maxPrefixLength) + { + prefix = prefix.Substring(0, maxPrefixLength); + } + + name.Append(prefix); + name.Append('_'); + name.Append(GetGuidParts()); + + if (escape) + { + name.Append(']'); + } + + return name.ToString(); + } + + /// + /// Creates the object with a given definition. + /// + /// Definition of the object to create. + /// + /// By the time this is called, will be open. + /// + protected abstract void CreateObject(string definition); + + /// + /// Drops the object created by . + /// + /// + /// By the time this is called, will be open. + /// Must not throw an exception if the object does not exist. + /// + protected abstract void DropObject(); + + public void Dispose() + { + if (_shouldDrop) + { + EnsureConnectionOpen(); + DropObject(); + } + // This explicitly does not drop the wrapped SqlConnection; this is sometimes + // used in a loop to create multiple UDTs. + + GC.SuppressFinalize(this); + } +} + +/// +/// Base class for a transient database object (such as a table, type or +/// stored procedure.) +/// +public abstract class DatabaseObject : DatabaseObject +{ + protected DatabaseObject(SqlConnection connection, string name, string definition, bool shouldCreate, bool shouldDrop) + : base(connection, name, definition, state: null, shouldCreate, shouldDrop) + { + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs new file mode 100644 index 0000000000..bc94bc78ef --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; + +/// +/// A transient database user, created at the start of its scope and dropped when disposed. +/// +/// +/// This class assumes that the associated server login already exists. +/// +public sealed class DatabaseUser : DatabaseObject +{ + public string DatabaseName => State; + + /// + /// Initializes a new instance of the DatabaseUser class using the specified SQL connection + /// and associated server login. + /// + /// The SQL connection used to interact with the database. + /// The name of the database where the user will be created. + /// The server login which the database user will be associated with. + public DatabaseUser(SqlConnection connection, string database, ServerLogin login) + : base(connection, login.Name, $"FOR LOGIN {login.Name}", state: database, shouldCreate: true, shouldDrop: true) + { + } + + protected override void CreateObject(string definition) + { + using SqlCommand createCommand = new($"CREATE USER {Name} {definition}", Connection); + + ExecuteCommandInDatabase(createCommand); + } + + protected override void DropObject() + { + using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); + + ExecuteCommandInDatabase(dropCommand); + } + + private void ExecuteCommandInDatabase(SqlCommand command) + { + // In some cases, we want to manage a database user in a different database to the one that the connection + // currently has open. In such a case, we need to change the database context for the command execution + // and then restore it afterwards. + string? originalDatabase = DatabaseName == command.Connection.Database ? null : command.Connection.Database; + + try + { + if (originalDatabase is not null) + { + command.Connection.ChangeDatabase(DatabaseName); + } + + command.ExecuteNonQuery(); + } + finally + { + if (originalDatabase is not null) + { + command.Connection.ChangeDatabase(originalDatabase); + } + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs new file mode 100644 index 0000000000..154899e9b3 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -0,0 +1,99 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Security.Cryptography; + +namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; + +/// +/// A transient server login, created at the start of its scope and dropped when disposed. +/// +public sealed class ServerLogin : DatabaseObject +{ + public string Password => State; + + /// + /// Initializes a new instance of the ServerLogin class using the specified SQL connection, login name prefix, and default database. + /// The login will be created with a randomly generated password that meets SQL Server's password complexity requirements. + /// + /// The SQL connection used to interact with the database. + /// The prefix for the login name. + /// The default database for the login. If null, not set. + public ServerLogin(SqlConnection connection, string namePrefix, string? defaultDatabase = null) + : this(connection, GenerateLongName(namePrefix), GeneratePassword(), defaultDatabase) + { + } + + private ServerLogin(SqlConnection connection, string namePrefix, string password, string? defaultDatabase) + : base(connection, namePrefix, GenerateDefinition(password, defaultDatabase), state: password, shouldCreate: true, shouldDrop: true) + { + } + + private static string GenerateDefinition(string password, string? defaultDatabase) => + $"WITH PASSWORD='{password}'" + + (string.IsNullOrEmpty(defaultDatabase) ? string.Empty : $", DEFAULT_DATABASE=[{defaultDatabase}]"); + + /// + /// Generates a password which meets the SQL Server password complexity requirements, which are: + /// + /// Minimum length of 8 characters + /// Must contain characters from three of the following four categories: + /// + /// Uppercase letters (A-Z) + /// Lowercase letters (a-z) + /// Digits (0-9) + /// Non-alphanumeric characters (e.g. !, $, #, %) + /// + /// + /// + /// A compliant password. + private static string GeneratePassword() + { + const int PasswordLength = 16; + const char UpperCaseStart = 'A'; + const char LowerCaseStart = 'a'; + const char DigitsStart = '0'; + + // First 5 characters are uppercase letters, next 5 are lowercase letters, and the last 6 are digits + Span passwordDigits = stackalloc char[PasswordLength]; + byte[] randomBytes = new byte[PasswordLength]; + using RandomNumberGenerator rndGen = RandomNumberGenerator.Create(); + + rndGen.GetBytes(randomBytes); + for(int i = 0; i < 5; i++) + { + passwordDigits[i] = (char)(UpperCaseStart + Scale(randomBytes[i], byte.MaxValue, 26)); + } + for (int i = 5; i < 10; i++) + { + passwordDigits[i] = (char)(LowerCaseStart + Scale(randomBytes[i], byte.MaxValue, 26)); + } + for (int i = 10; i < PasswordLength; i++) + { + passwordDigits[i] = (char)(DigitsStart + Scale(randomBytes[i], byte.MaxValue, 10)); + } + + return passwordDigits.ToString(); + + // Scales a byte value from the range [0, 255) to the range [0, newMaximum) + static int Scale(byte value, int existingMaximum, int newMaximum) + { + return (int)((double)value / (existingMaximum + 1) * newMaximum); + } + } + + protected override void CreateObject(string definition) + { + using SqlCommand createCommand = new($"CREATE LOGIN {Name} {definition}", Connection); + + createCommand.ExecuteNonQuery(); + } + + protected override void DropObject() + { + using SqlCommand dropCommand = new($"IF SUSER_ID('{UnescapedName}') IS NOT NULL DROP LOGIN {Name}", Connection); + + dropCommand.ExecuteNonQuery(); + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index fc310bf3d3..49eecd37f1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -95,6 +95,10 @@ public static class DataTestUtility private static bool? s_isDataClassificationSupported; private static bool? s_isVectorSupported; + // Login permissions + private static bool? s_isSysAdmin; + private static bool? s_isSecurityAdmin; + // Azure Synapse EngineEditionId == 6 // More could be read at https://learn.microsoft.com/en-us/sql/t-sql/functions/serverproperty-transact-sql?view=sql-server-ver16#propertyname public static bool IsAzureSynapse @@ -155,6 +159,20 @@ public static string SQLServerVersion s_isVectorSupported ??= IsTCPConnStringSetup() && IsTypePresent("vector"); + public static bool IsSysAdmin => + s_isSysAdmin ??= IsTCPConnStringSetup() && + IsServerRoleMember("sysadmin"); + + public static bool IsSecurityAdmin => + s_isSecurityAdmin ??= IsTCPConnStringSetup() && + IsServerRoleMember("securityadmin"); + + public static bool CanCreateLogins => + IsSysAdmin || IsSecurityAdmin; + + public static bool CanUseSqlAuthentication => + IsSysAdmin && GetAuthenticationMode() == 2; + static DataTestUtility() { Config c = Config.Load(); @@ -457,6 +475,30 @@ public static bool IsTypePresent(string typeName) return (int)command.ExecuteScalar() > 0; } + public static bool IsServerRoleMember(string roleName) + { + using SqlConnection connection = new(TCPConnectionString); + using SqlCommand command = new("SELECT IS_SRVROLEMEMBER(@role)", connection); + + connection.Open(); + command.Parameters.AddWithValue("@role", roleName); + + // IS_SRVROLEMEMBER returns 1 if the caller is a member of the specified server role, 0 if not, and DBNull.Value if the role is not valid. + return command.ExecuteScalar() is int result && result == 1; + } + + public static int GetAuthenticationMode() + { + using SqlConnection connection = new(TCPConnectionString); + + connection.Open(); + using SqlCommand command = new("EXEC xp_instance_regread N'HKEY_LOCAL_MACHINE', N'Software\\Microsoft\\MSSQLServer\\MSSQLServer', N'LoginMode'", connection); + using SqlDataReader reader = command.ExecuteReader(); + + reader.Read(); + return reader.GetInt32(1); + } + public static bool IsAdmin { get diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs new file mode 100644 index 0000000000..da40f53141 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#if NETFRAMEWORK + +namespace System.Diagnostics.CodeAnalysis; + +// These classes are provided to provide compile-time support for Microsoft.CodeAnalysis +// attributes. These attributes are native to netcore and available for netfx as a nuget +// package - but only for net472. As such, until net462 support is dropped, these placeholder +// classes will need to exist if we want to use them for static analysis. + +#nullable enable + +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] +internal sealed class MemberNotNullAttribute : Attribute +{ + public MemberNotNullAttribute(string member) => Members = [member]; + + public MemberNotNullAttribute(params string[] members) => Members = members; + + public string[] Members { get; } +} +#endif diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs index a0ccf69301..409407444d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs @@ -40,6 +40,10 @@ public static void Test(string srcConstr, string dstConstr, string dstTable) using (DbDataReader reader = srcCmd.ExecuteReader()) { IDictionary stats; + long expectedIduCount = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0; + long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 13; + long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 15; + long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0; using (SqlBulkCopy bulkcopy = new SqlBulkCopy(dstConn)) { bulkcopy.DestinationTableName = dstTable; @@ -60,12 +64,12 @@ public static void Test(string srcConstr, string dstConstr, string dstTable) DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersReceived"], "Unexpected BuffersReceived value."); DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersSent"], "Unexpected BuffersSent value."); - DataTestUtility.AssertEqualsWithDescription((long)0, stats["IduCount"], "Unexpected IduCount value."); - DataTestUtility.AssertEqualsWithDescription((long)11, stats["SelectCount"], "Unexpected SelectCount value."); + DataTestUtility.AssertEqualsWithDescription(expectedIduCount, stats["IduCount"], "Unexpected IduCount value."); + DataTestUtility.AssertEqualsWithDescription(expectedSelectCount, stats["SelectCount"], "Unexpected SelectCount value."); DataTestUtility.AssertEqualsWithDescription((long)3, stats["ServerRoundtrips"], "Unexpected ServerRoundtrips value."); - DataTestUtility.AssertEqualsWithDescription((long)14, stats["SelectRows"], "Unexpected SelectRows value."); + DataTestUtility.AssertEqualsWithDescription(expectedSelectRows, stats["SelectRows"], "Unexpected SelectRows value."); DataTestUtility.AssertEqualsWithDescription((long)2, stats["SumResultSets"], "Unexpected SumResultSets value."); - DataTestUtility.AssertEqualsWithDescription((long)0, stats["Transactions"], "Unexpected Transactions value."); + DataTestUtility.AssertEqualsWithDescription(expectedTransactions, stats["Transactions"], "Unexpected Transactions value."); } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs new file mode 100644 index 0000000000..08915e30ab --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs @@ -0,0 +1,208 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Data; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; +using Xunit; + +namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SqlBulkCopyTests; + +#nullable enable + +/// +/// Some unprivileged users may not have permissions to query sys.all_columns, which is used to +/// handle column aliases. These tests verify that bulk copy operations can handle such situations +/// gracefully, falling back and ignoring column aliases. +/// +[Trait("Set", "2")] +public sealed class UnprivilegedLogin : IDisposable +{ + private readonly SqlConnection? _managementConnection; + private readonly ServerLogin? _unprivilegedLogin; + private readonly DatabaseUser? _unprivilegedMasterUser; + private readonly DatabaseUser? _unprivilegedAppUser; + + private readonly string? _unprivilegedConnectionString; + + public static bool CanRunTests => + DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotAzureServer() + && DataTestUtility.CanCreateLogins && DataTestUtility.CanUseSqlAuthentication; + + public UnprivilegedLogin() + { + // xUnit will instantiate the class before evaluating the test condition - make sure that we don't + // attempt to make use of these capabilities if the server doesn't support them. + // This has the expected nullability implications, so AssertEnvironmentCreated needs to be called + // before any test logic. The compiler asserts these for us. + if (!CanRunTests) + { + return; + } + + // We require two connections: a "management" connection which has permissions to create logins, + // create users and modify permissions; and an "unprivileged" connection, which is used to perform + // the actual tests. The user associated with the latter connection will be denied SELECT permissions + // over master.sys.all_columns. + _managementConnection = new SqlConnection(DataTestUtility.TCPConnectionString); + _managementConnection.Open(); + + _unprivilegedLogin = new ServerLogin(_managementConnection, nameof(UnprivilegedLogin), _managementConnection.Database); + _unprivilegedAppUser = new DatabaseUser(_managementConnection, _managementConnection.Database, _unprivilegedLogin); + _unprivilegedMasterUser = new DatabaseUser(_managementConnection, "master", _unprivilegedLogin); + + using (SqlCommand permissionsModificationCommand = _managementConnection.CreateCommand()) + { + permissionsModificationCommand.CommandText = $"DENY SELECT ON [master].[sys].[all_columns] TO {_unprivilegedMasterUser.Name}"; + permissionsModificationCommand.ExecuteNonQuery(); + + permissionsModificationCommand.CommandText = $"DENY SELECT ON [{_managementConnection.Database}].[sys].[all_columns] TO {_unprivilegedAppUser.Name}"; + permissionsModificationCommand.ExecuteNonQuery(); + } + + SqlConnectionStringBuilder tcpConnectionBuilder = new(DataTestUtility.TCPConnectionString) + { + IntegratedSecurity = false, + UserID = _unprivilegedLogin.UnescapedName, + Password = _unprivilegedLogin.Password, + // Disable connection pooling - we'll be dropping this login once the tests complete, and this can only happen once all connections + // using it are closed. + Pooling = false + }; + + _unprivilegedConnectionString = tcpConnectionBuilder.ConnectionString; + } + + /// + /// This method enables nullability assertions to operate as expected. It'll always pass if CanRunTests is + /// true - the constructor will have initialized the relevant fields. + /// + [MemberNotNull(nameof(_managementConnection), + nameof(_unprivilegedLogin), + nameof(_unprivilegedMasterUser), + nameof(_unprivilegedAppUser), + nameof(_unprivilegedConnectionString))] + private void AssertEnvironmentCreated() + { + Assert.NotNull(_managementConnection); + Assert.NotNull(_unprivilegedLogin); + Assert.NotNull(_unprivilegedMasterUser); + Assert.NotNull(_unprivilegedAppUser); + Assert.NotNull(_unprivilegedConnectionString); + } + + /// + /// Verifies that a bulk copy operation succeeds when performed by a user with only SELECT and INSERT permissions, + /// without requiring access to metadata views. + /// + [ConditionalFact(nameof(CanRunTests))] + public void BulkCopyWithoutMetadataPermission_Succeeds() + { + AssertEnvironmentCreated(); + + const int BulkCopyRowCount = 5; + + using DataTable srcDataTable = new() + { + Columns = { new DataColumn("Description", typeof(string)) } + }; + using Table dstTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_Succeeds), "([Description] VARCHAR(100))"); + using (SqlCommand permissionsConfigurationCommand = new($"GRANT SELECT, INSERT ON {dstTable.Name} TO {_unprivilegedAppUser.Name}", _managementConnection)) + { + permissionsConfigurationCommand.ExecuteNonQuery(); + } + + using SqlBulkCopy nodeCopy = new(_unprivilegedConnectionString); + + for (int i = 0; i < BulkCopyRowCount; i++) + { + srcDataTable.Rows.Add($"Description {i}"); + } + + nodeCopy.DestinationTableName = dstTable.Name; + nodeCopy.ColumnMappings.Add("Description", "Description"); + nodeCopy.WriteToServer(srcDataTable); + + int resultantRowCount; + using (SqlCommand rowCountQuery = new($"SELECT COUNT(*) FROM {dstTable.Name}", _managementConnection)) + { + resultantRowCount = (int)rowCountQuery.ExecuteScalar(); + } + + Assert.Equal(BulkCopyRowCount, resultantRowCount); + } + + [ConditionalFact(nameof(CanRunTests))] + public void BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases() + { + AssertEnvironmentCreated(); + + using DataTable edges = new DataTable() + { + Columns = { new DataColumn("To_ID", typeof(string)), new DataColumn("From_ID", typeof(string)), new DataColumn("Description", typeof(string)) } + }; + + // Use the management connection to create the source and the destination tables, grant the + // unprivileged user permissions over them and insert some sample data into the source table. + using Table srcNodeTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases), "([Name] VARCHAR(100)) AS NODE"); + using Table dstEdgeTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases), "([Description] VARCHAR(100)) AS EDGE"); + using (SqlCommand permissionsConfigurationCommand = _managementConnection.CreateCommand()) + { + permissionsConfigurationCommand.CommandText = $"GRANT SELECT, INSERT ON {srcNodeTable.Name} TO {_unprivilegedAppUser.Name}"; + permissionsConfigurationCommand.ExecuteNonQuery(); + + permissionsConfigurationCommand.CommandText = $"GRANT SELECT, INSERT ON {dstEdgeTable.Name} TO {_unprivilegedAppUser.Name}"; + permissionsConfigurationCommand.ExecuteNonQuery(); + } + + string sampleNodeDataCommand = @$"INSERT INTO {srcNodeTable.Name} ([Name]) SELECT LEFT([name], 100) FROM sys.sysobjects"; + using (SqlCommand insertSampleNodes = new(sampleNodeDataCommand, _managementConnection)) + { + insertSampleNodes.ExecuteNonQuery(); + } + + using (SqlCommand nodeQuery = new($"SELECT $node_id FROM {srcNodeTable.Name}", _managementConnection)) + using (SqlDataReader reader = nodeQuery.ExecuteReader()) + { + bool firstRead = reader.Read(); + string toId; + string fromId; + + Assert.True(firstRead); + toId = reader.GetString(0); + + while (reader.Read()) + { + fromId = reader.GetString(0); + + edges.Rows.Add(toId, fromId, "Test Description"); + toId = fromId; + } + } + + // With all source data populated, try to use the unprivileged connection to perform a bulk copy + // using aliases in the column mappings. This should fail - the permissions error will be caught, + // and SqlBulkCopy should simply report that the destination column doesn't exist. + using SqlBulkCopy edgeCopy = new(_unprivilegedConnectionString); + + edgeCopy.DestinationTableName = dstEdgeTable.Name; + edgeCopy.ColumnMappings.Add("To_ID", "$to_id"); + edgeCopy.ColumnMappings.Add("From_ID", "$from_id"); + edgeCopy.ColumnMappings.Add("Description", "Description"); + + Action failingEdgeCopy = () => edgeCopy.WriteToServer(edges); + InvalidOperationException missingColumnException = Assert.Throws(failingEdgeCopy); + + Assert.Contains("'$to_id,$from_id'", missingColumnException.Message); + } + + public void Dispose() + { + _unprivilegedAppUser?.Dispose(); + _unprivilegedMasterUser?.Dispose(); + _unprivilegedLogin?.Dispose(); + _managementConnection?.Dispose(); + } +} From 2c5dd98ea2796f102ec6ae599e95daf6a72976ca Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 24 Jun 2026 07:49:48 +0100 Subject: [PATCH 2/9] Post-merge fixes Correct compile errors and test with Azure Synapse Analytics --- .../DatabaseObjects/DatabaseObject.cs | 2 + .../Fixtures/DatabaseObjects/ServerLogin.cs | 1 + .../Common/Fixtures/DatabaseObjects/Table.cs | 61 +++++++++++++++++++ ...icrosoft.Data.SqlClient.ManualTests.csproj | 4 +- .../SQL/SqlBulkCopyTest/CopyAllFromReader.cs | 8 +-- .../SQL/SqlBulkCopyTest/UnprivilegedLogin.cs | 1 - 6 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs index a74da79697..a7e934390c 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Text; +using System.Threading; namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs index 154899e9b3..f11d2ae5a9 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Security.Cryptography; namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs new file mode 100644 index 0000000000..2838ae2272 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs @@ -0,0 +1,61 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; + +/// +/// A transient table, created at the start of its scope and dropped when disposed. +/// +public sealed class Table : DatabaseObject +{ + /// + /// Initializes a new instance of the Table class using the specified SQL connection, table name prefix, and table + /// definition. + /// + /// + /// If a table with the specified name already exists, it will be dropped automatically before + /// creation. + /// + /// The SQL connection used to interact with the database. + /// The prefix for the table name. Can begin with '#' or '##' to indicate a temporary table. + /// The SQL definition describing the structure of the table, including columns and data types. + public Table(SqlConnection connection, string prefix, string definition) + : base(connection, GenerateLongName(prefix), definition, shouldCreate: true, shouldDrop: true) + { + } + + protected override void CreateObject(string definition) + { + using SqlCommand createCommand = new($"CREATE TABLE {Name} {definition}", Connection); + + createCommand.ExecuteNonQuery(); + } + + protected override void DropObject() + { + using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TABLE {Name}", Connection); + + dropCommand.ExecuteNonQuery(); + } + + /// + /// Deletes all data from the table. + /// + public void DeleteData() + { + using SqlCommand deleteCommand = new($"DELETE FROM {Name}", Connection); + + deleteCommand.ExecuteNonQuery(); + } + + /// + /// Truncates the table. + /// + public void Truncate() + { + using SqlCommand truncateCommand = new($"TRUNCATE TABLE {Name}", Connection); + + truncateCommand.ExecuteNonQuery(); + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj index 336838b068..3d9418992e 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj @@ -1,4 +1,4 @@ - + ManualTests @@ -45,6 +45,7 @@ + @@ -179,6 +180,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs index 409407444d..62cc93536b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs @@ -40,10 +40,8 @@ public static void Test(string srcConstr, string dstConstr, string dstTable) using (DbDataReader reader = srcCmd.ExecuteReader()) { IDictionary stats; - long expectedIduCount = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0; - long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 13; + long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 3 : 12; long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 15; - long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0; using (SqlBulkCopy bulkcopy = new SqlBulkCopy(dstConn)) { bulkcopy.DestinationTableName = dstTable; @@ -64,12 +62,12 @@ public static void Test(string srcConstr, string dstConstr, string dstTable) DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersReceived"], "Unexpected BuffersReceived value."); DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersSent"], "Unexpected BuffersSent value."); - DataTestUtility.AssertEqualsWithDescription(expectedIduCount, stats["IduCount"], "Unexpected IduCount value."); + DataTestUtility.AssertEqualsWithDescription((long)0, stats["IduCount"], "Unexpected IduCount value."); DataTestUtility.AssertEqualsWithDescription(expectedSelectCount, stats["SelectCount"], "Unexpected SelectCount value."); DataTestUtility.AssertEqualsWithDescription((long)3, stats["ServerRoundtrips"], "Unexpected ServerRoundtrips value."); DataTestUtility.AssertEqualsWithDescription(expectedSelectRows, stats["SelectRows"], "Unexpected SelectRows value."); DataTestUtility.AssertEqualsWithDescription((long)2, stats["SumResultSets"], "Unexpected SumResultSets value."); - DataTestUtility.AssertEqualsWithDescription(expectedTransactions, stats["Transactions"], "Unexpected Transactions value."); + DataTestUtility.AssertEqualsWithDescription((long)0, stats["Transactions"], "Unexpected Transactions value."); } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs index 08915e30ab..7a5b9894c2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs @@ -17,7 +17,6 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SqlBulkCopyTests; /// handle column aliases. These tests verify that bulk copy operations can handle such situations /// gracefully, falling back and ignoring column aliases. /// -[Trait("Set", "2")] public sealed class UnprivilegedLogin : IDisposable { private readonly SqlConnection? _managementConnection; From 45eb7c835b3605f1a823509dd94f89af281cf762 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 24 Jun 2026 08:25:09 +0100 Subject: [PATCH 3/9] Only run SQL Graph-based test on SQL 2017 --- .../ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs index 7a5b9894c2..81f3fccd6c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs @@ -30,6 +30,8 @@ public sealed class UnprivilegedLogin : IDisposable DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotAzureServer() && DataTestUtility.CanCreateLogins && DataTestUtility.CanUseSqlAuthentication; + public static bool IsAtLeastSQL2017 => DataTestUtility.IsAtLeastSQL2017(); + public UnprivilegedLogin() { // xUnit will instantiate the class before evaluating the test condition - make sure that we don't @@ -133,7 +135,7 @@ public void BulkCopyWithoutMetadataPermission_Succeeds() Assert.Equal(BulkCopyRowCount, resultantRowCount); } - [ConditionalFact(nameof(CanRunTests))] + [ConditionalFact(nameof(CanRunTests), nameof(IsAtLeastSQL2017))] public void BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases() { AssertEnvironmentCreated(); From 8b1c181845c3c45c313107d9c5560f726792b858 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 22:14:11 +0100 Subject: [PATCH 4/9] Code review: grammar fix --- .../src/Microsoft/Data/SqlClient/SqlBulkCopy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs index 02e3e51606..da38af9a91 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs @@ -514,7 +514,7 @@ private string CreateInitialQuery() // // See: https://learn.microsoft.com/en-us/sql/t-sql/functions/serverproperty-transact-sql // - // All of this is wrapped in an test against HAS_PERMS_BY_NAME. This test verifies that + // All of this is wrapped in a test against HAS_PERMS_BY_NAME. This test verifies that // the user possesses the necessary permissions to access sys.all_columns. If they do not // @Column_Names will remain NULL (and be coalesced to *) and SqlBulkCopy will degrade // gracefully, silently dropping support for hidden columns and column aliases. From 94b68afb8991dc6d1a51b72df944d2c4fad0df66 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 22:15:17 +0100 Subject: [PATCH 5/9] Code review: copy/paste mismatch --- .../tests/ManualTests/Extensions/CodeAnalysis.netfx.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs index da40f53141..101ac8ee8c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs @@ -6,7 +6,7 @@ namespace System.Diagnostics.CodeAnalysis; -// These classes are provided to provide compile-time support for Microsoft.CodeAnalysis +// These classes are provided to provide compile-time support for System.Diagnostics.CodeAnalysis // attributes. These attributes are native to netcore and available for netfx as a nuget // package - but only for net472. As such, until net462 support is dropped, these placeholder // classes will need to exist if we want to use them for static analysis. From 8d116b86741e1cecccfb4328781f07bdd89e49c1 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:09:35 +0100 Subject: [PATCH 6/9] Code review: parameterise object name (Table) --- .../tests/Common/Fixtures/DatabaseObjects/Table.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs index 2838ae2272..36b6dc369e 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs @@ -34,7 +34,8 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TABLE {Name}", Connection); + using SqlCommand dropCommand = new($"IF (OBJECT_ID(@Table_Name) IS NOT NULL) DROP TABLE {Name}", Connection); + dropCommand.Parameters.AddWithValue("@Table_Name", Name); dropCommand.ExecuteNonQuery(); } From 71ee8a9dab789a45a0300fa035aca2428f5b7d55 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:09:49 +0100 Subject: [PATCH 7/9] Code review: parameterise object name (ServerLogin) --- .../tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs index f11d2ae5a9..0cd7f4ef1e 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -93,7 +93,8 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF SUSER_ID('{UnescapedName}') IS NOT NULL DROP LOGIN {Name}", Connection); + using SqlCommand dropCommand = new($"IF SUSER_ID(@Login_Name) IS NOT NULL DROP LOGIN {Name}", Connection); + dropCommand.Parameters.AddWithValue("@Login_Name", UnescapedName); dropCommand.ExecuteNonQuery(); } From 4119f2b6320c66ecafc94999787bf0a2f38b1609 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:10:02 +0100 Subject: [PATCH 8/9] Code review: parameterise object name (DatabaseUser) --- .../tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs index bc94bc78ef..6e111649cb 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs @@ -35,7 +35,8 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); + using SqlCommand dropCommand = new($"IF USER_ID(@User_Name) IS NOT NULL DROP USER {Name}", Connection); + dropCommand.Parameters.AddWithValue("@User_Name", UnescapedName); ExecuteCommandInDatabase(dropCommand); } From bf84bb83c48ff79af7b502fba17b95b0300eb6f8 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:10:33 +0100 Subject: [PATCH 9/9] Code review: replace xp_instance_regread --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 49eecd37f1..403263cf2a 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -94,6 +94,7 @@ public static class DataTestUtility // SQL Server capabilities private static bool? s_isDataClassificationSupported; private static bool? s_isVectorSupported; + private static bool? s_isSqlAuthenticationSupported; // Login permissions private static bool? s_isSysAdmin; @@ -171,7 +172,8 @@ public static string SQLServerVersion IsSysAdmin || IsSecurityAdmin; public static bool CanUseSqlAuthentication => - IsSysAdmin && GetAuthenticationMode() == 2; + s_isSqlAuthenticationSupported ??= IsTCPConnStringSetup() && + SupportsSqlAuthentication(); static DataTestUtility() { @@ -487,16 +489,16 @@ public static bool IsServerRoleMember(string roleName) return command.ExecuteScalar() is int result && result == 1; } - public static int GetAuthenticationMode() + public static bool SupportsSqlAuthentication() { using SqlConnection connection = new(TCPConnectionString); connection.Open(); - using SqlCommand command = new("EXEC xp_instance_regread N'HKEY_LOCAL_MACHINE', N'Software\\Microsoft\\MSSQLServer\\MSSQLServer', N'LoginMode'", connection); + using SqlCommand command = new("select SERVERPROPERTY('IsIntegratedSecurityOnly'), ISNULL(SERVERPROPERTY('IsExternalAuthenticationOnly'), 0)", connection); using SqlDataReader reader = command.ExecuteReader(); reader.Read(); - return reader.GetInt32(1); + return reader.GetInt32(0) == 0 && reader.GetInt32(1) == 0; } public static bool IsAdmin