From 4285e5931e6a23a7fc2870709474e662efb71259 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 22 May 2026 22:29:23 +0100 Subject: [PATCH 1/4] RAII primitive scaffolding * Add an UnescapedName property to DatabaseObject and use it in XEventsTracingTest. * Create ServerLogin primitive. * Create DatabaseUser primitive. --- .../DatabaseObjects/DatabaseObject.cs | 2 + .../Fixtures/DatabaseObjects/DatabaseUser.cs | 58 ++++++++++++ .../Fixtures/DatabaseObjects/ServerLogin.cs | 90 +++++++++++++++++++ .../TracingTests/XEventsTracingTest.cs | 2 +- 4 files changed, 151 insertions(+), 1 deletion(-) 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 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 2dc539b5f5..8e913cfb41 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs @@ -20,6 +20,8 @@ public abstract class DatabaseObject : IDisposable public string Name { get; } + public string UnescapedName => Name.Substring(1, Name.Length - 2).Replace("]]", "]"); + protected DatabaseObject(SqlConnection connection, string name, string definition, bool shouldCreate, bool shouldDrop) { _shouldDrop = 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..21d7a4839b --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs @@ -0,0 +1,58 @@ +// 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 +{ + private readonly string _databaseName; + + /// + /// 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 server login which the database user will be associated with. + public DatabaseUser(SqlConnection connection, ServerLogin login) + : base(connection, login.Name, $"FOR LOGIN {login.Name}", shouldCreate: true, shouldDrop: true) + { + _databaseName = Connection.Database; + } + + protected override void CreateObject(string definition) + { + using SqlCommand createCommand = new($"CREATE USER {Name} {definition}", Connection); + + createCommand.ExecuteNonQuery(); + } + + protected override void DropObject() + { + string? originalDatabase = _databaseName == Connection.Database ? null : _databaseName; + using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); + + try + { + if (originalDatabase is not null) + { + Connection.ChangeDatabase(_databaseName); + } + + dropCommand.ExecuteNonQuery(); + } + finally + { + if (originalDatabase is not null) + { + 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..e668a3c8d3 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -0,0 +1,90 @@ +// 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 server login, created at the start of its scope and dropped when disposed. +/// +public sealed class ServerLogin : DatabaseObject +{ + public string Password { get; } + + /// + /// 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), shouldCreate: true, shouldDrop: true) + { + Password = password; + } + + 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]; + Random rnd = new(); + + for(int i = 0; i < 5; i++) + { + passwordDigits[i] = (char)(UpperCaseStart + rnd.Next(26)); + } + for (int i = 5; i < 10; i++) + { + passwordDigits[i] = (char)(LowerCaseStart + rnd.Next(26)); + } + for (int i = 10; i < PasswordLength; i++) + { + passwordDigits[i] = (char)(DigitsStart + rnd.Next(10)); + } + + return passwordDigits.ToString(); + } + + 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/TracingTests/XEventsTracingTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs index b03fc8b224..82a023b5fa 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs @@ -120,7 +120,7 @@ public void XEventActivityIDConsistentWithTracing_RpcStarting() // Our stored procedure name is an escaped SQL Server object name. This will not match the object_name data // in the XEvent XML, which records it as an unescaped name. - string unescapedProcedureName = sp.Name.Substring(1, sp.Name.Length - 2).Replace("]]", "]"); + string unescapedProcedureName = sp.UnescapedName; VerifyXEventActivityIDConsistentWithTracing(unescapedProcedureName, System.Data.CommandType.StoredProcedure, "rpc_starting"); } From 7d6f87fb7a1b4dbd340e72718e4853f415b57f3a Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 22 May 2026 22:43:05 +0100 Subject: [PATCH 2/4] Add relevant test conditions --- .../ManualTests/DataCommon/DataTestUtility.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 1e5458e130..7f8fe9bda4 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -98,6 +98,10 @@ public static class DataTestUtility private static bool? s_isVectorSupported; private static bool? s_isVectorFloat16Supported; + // 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 @@ -231,6 +235,20 @@ private static bool CheckVectorFloat16Supported() } } + 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(); @@ -531,6 +549,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 From f03e34fe7017c82f1929bd5418c93aad16e36274 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 23 May 2026 16:57:22 +0100 Subject: [PATCH 3/4] Modify DatabaseObject to support state This makes DatabaseUser simpler to work with. It can store a reference to the database the user should be created in, and the class can handle switching to/from that database. --- .../DatabaseObjects/DatabaseObject.cs | 23 ++++++++++++++-- .../Fixtures/DatabaseObjects/DatabaseUser.cs | 26 ++++++++++++------- .../Fixtures/DatabaseObjects/ServerLogin.cs | 7 +++-- 3 files changed, 40 insertions(+), 16 deletions(-) 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 8e913cfb41..d913724b50 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseObject.cs @@ -12,21 +12,28 @@ namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; /// Base class for a transient database object (such as a table, type or /// stored procedure.) /// -public abstract class DatabaseObject : IDisposable +/// +/// 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, bool shouldCreate, bool shouldDrop) + protected DatabaseObject(SqlConnection connection, string name, string definition, TState state, bool shouldCreate, bool shouldDrop) { _shouldDrop = shouldDrop; Connection = connection; + State = state; Name = name; if (shouldCreate) @@ -263,3 +270,15 @@ public void Dispose() 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 index 21d7a4839b..dbf7b61afc 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs @@ -10,48 +10,54 @@ namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; /// /// This class assumes that the associated server login already exists. /// -public sealed class DatabaseUser : DatabaseObject +public sealed class DatabaseUser : DatabaseObject { - private readonly string _databaseName; + 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, ServerLogin login) - : base(connection, login.Name, $"FOR LOGIN {login.Name}", shouldCreate: true, shouldDrop: true) + public DatabaseUser(SqlConnection connection, string database, ServerLogin login) + : base(connection, login.Name, $"FOR LOGIN {login.Name}", database, shouldCreate: true, shouldDrop: true) { - _databaseName = Connection.Database; } protected override void CreateObject(string definition) { using SqlCommand createCommand = new($"CREATE USER {Name} {definition}", Connection); - createCommand.ExecuteNonQuery(); + ExecuteCommandInDatabase(createCommand); } protected override void DropObject() { - string? originalDatabase = _databaseName == Connection.Database ? null : _databaseName; using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); + ExecuteCommandInDatabase(dropCommand); + } + + private void ExecuteCommandInDatabase(SqlCommand command) + { + string? originalDatabase = DatabaseName == command.Connection.Database ? null : command.Connection.Database; + try { if (originalDatabase is not null) { - Connection.ChangeDatabase(_databaseName); + command.Connection.ChangeDatabase(DatabaseName); } - dropCommand.ExecuteNonQuery(); + command.ExecuteNonQuery(); } finally { if (originalDatabase is not null) { - Connection.ChangeDatabase(originalDatabase); + 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 index e668a3c8d3..d2aedcd68e 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -7,9 +7,9 @@ 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 sealed class ServerLogin : DatabaseObject { - public string Password { get; } + public string Password => State; /// /// Initializes a new instance of the ServerLogin class using the specified SQL connection, login name prefix, and default database. @@ -24,9 +24,8 @@ public ServerLogin(SqlConnection connection, string namePrefix, string? defaultD } private ServerLogin(SqlConnection connection, string namePrefix, string password, string? defaultDatabase) - : base(connection, namePrefix, GenerateDefinition(password, defaultDatabase), shouldCreate: true, shouldDrop: true) + : base(connection, namePrefix, GenerateDefinition(password, defaultDatabase), password, shouldCreate: true, shouldDrop: true) { - Password = password; } private static string GenerateDefinition(string password, string? defaultDatabase) => From 9e71953bdf48aa4aed2d331c2967374f740bb43d Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sun, 24 May 2026 13:19:23 +0100 Subject: [PATCH 4/4] Fix bug and add regression tests Fix bug by checking HAS_PERMS_BY_NAME over the sys.all_columns view. This is safe on all supported platforms (including Synapse Analytics dedicated and shared pools - documentation is inconsistent.) Regression tests create a low-privileged login and user in on-premise environments, then use those credentials to perform a SqlBulkCopy. --- .../Microsoft/Data/SqlClient/SqlBulkCopy.cs | 55 +++-- .../Extensions/CodeAnalysis.netfx.cs | 20 ++ ...icrosoft.Data.SqlClient.ManualTests.csproj | 2 + .../SQL/SqlBulkCopyTest/CopyAllFromReader.cs | 8 +- .../SQL/SqlBulkCopyTest/UnprivilegedLogin.cs | 198 ++++++++++++++++++ 5 files changed, 258 insertions(+), 25 deletions(-) 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 414be57eb0..ed9813389c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs @@ -477,11 +477,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 @@ -526,6 +527,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; @@ -535,6 +541,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_Permissions INT = HAS_PERMS_BY_NAME('{catalogNameStringLiteral}.[sys].[all_columns]', 'OBJECT', 'SELECT'); CREATE TABLE #Column_Aliases ( @@ -554,28 +561,35 @@ 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)'; - - EXEC sp_executesql N' - INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name]) - SELECT [name], [column_id], ''$to_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8 - UNION ALL - SELECT [name], [column_id], ''$from_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5 - UNION ALL - SELECT [name], [column_id], ''$edge_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%'' - UNION ALL - SELECT [name], [column_id], ''$node_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''', - N'@Object_ID INT', @Object_ID = @Object_ID -END -ELSE +IF @Has_Permissions = 1 BEGIN - SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID'; + IF EXISTS (SELECT TOP 1 * FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = OBJECT_ID('{catalogNameStringLiteral}.[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)'; + + EXEC sp_executesql N' + INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name]) + SELECT [name], [column_id], ''$to_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8 + UNION ALL + SELECT [name], [column_id], ''$from_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5 + UNION ALL + SELECT [name], [column_id], ''$edge_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%'' + UNION ALL + SELECT [name], [column_id], ''$node_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''', + N'@Object_ID INT', @Object_ID = @Object_ID + 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; + + DELETE FROM #Column_Aliases + WHERE [Aliased_Column_Name] IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID) 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; @@ -586,7 +600,6 @@ UNION ALL SELECT [Canonical_Column_Name], [Aliased_Column_Name] FROM #Column_Aliases -WHERE [Aliased_Column_Name] NOT IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID) ORDER BY [Canonical_Column_Id] ASC DROP TABLE #Column_Aliases 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..8af4066577 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/CodeAnalysis.netfx.cs @@ -0,0 +1,20 @@ +// 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; + +#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/Microsoft.Data.SqlClient.ManualTests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj index 1145fa4790..5e1cedb47c 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 @@ -45,6 +45,7 @@ + @@ -57,6 +58,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 7dcb0a8111..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,10 +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() ? 1 : 0; - long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 12; - long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 14; - long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 1 : 0; + 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; 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..6a6308df45 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/UnprivilegedLogin.cs @@ -0,0 +1,198 @@ +// 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 System.Runtime.CompilerServices; +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. +/// +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 {_unprivilegedMasterUser.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(); + + 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 < 5; i++) + { + srcDataTable.Rows.Add($"Description {i}"); + } + + nodeCopy.DestinationTableName = dstTable.Name; + nodeCopy.ColumnMappings.Add("Description", "Description"); + nodeCopy.WriteToServer(srcDataTable); + } + + [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(); + } +}