From 29e1909e9816c1de8db91cf28bd38d29c2cb3fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Eggert=20Lundsteen=20Olsen?= Date: Fri, 2 Oct 2026 12:39:07 +0200 Subject: [PATCH] Review 1.3: one implementation of a service account's SQL login RepositorySetup.LoginFor/IsLocal duplicated Core's ServiceAccounts, which the web uses for the GRANT script. Setup now uses ServiceAccounts too, so the login the installer grants and the one the web shows cannot drift apart. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/SQLInsight.Agent/Setup/RepositorySetup.cs | 40 ++----------------- .../RepositorySetupTests.cs | 11 ++--- 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/src/SQLInsight.Agent/Setup/RepositorySetup.cs b/src/SQLInsight.Agent/Setup/RepositorySetup.cs index 78e2c1d..89f0912 100644 --- a/src/SQLInsight.Agent/Setup/RepositorySetup.cs +++ b/src/SQLInsight.Agent/Setup/RepositorySetup.cs @@ -2,6 +2,7 @@ using System.Diagnostics; using System.Text.RegularExpressions; using Microsoft.Data.SqlClient; using SQLInsight.Connectivity; +using SQLInsight.Core.Configuration; using SQLInsight.Repository; namespace SQLInsight.Agent.Setup; @@ -146,7 +147,7 @@ public static partial class RepositorySetup return; } - var login = LoginFor(agentAccount, instance.Server, Environment.MachineName, Environment.UserDomainName); + var login = ServiceAccounts.LoginFor(agentAccount, instance.Server, Environment.MachineName, Environment.UserDomainName); var target = new SqlConnectionStringBuilder { DataSource = instance.Server, @@ -174,43 +175,13 @@ public static partial class RepositorySetup /// public static IReadOnlyList<(string Login, string Role)> RepositoryLogins(string agentAccount, string webAccount, string sqlServer) { - var agent = LoginFor(agentAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); - var web = LoginFor(webAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); + var agent = ServiceAccounts.LoginFor(agentAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); + var web = ServiceAccounts.LoginFor(webAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); return string.Equals(agent, web, StringComparison.OrdinalIgnoreCase) ? [(agent, "db_owner")] : [(agent, "db_owner"), (web, "db_datareader")]; } - /// - /// The Windows login a service account has on . Virtual and built-in machine accounts - /// (NT SERVICE\…, LocalSystem, NETWORK SERVICE) only exist locally; on another server they arrive as the computer - /// account DOMAIN\COMPUTER$. Domain accounts and gMSAs are the same everywhere. - /// - public static string LoginFor(string account, string sqlServer, string machineName, string domain) - { - ArgumentException.ThrowIfNullOrWhiteSpace(account); - var normalized = account.Trim(); - if (normalized.Equals("LocalSystem", StringComparison.OrdinalIgnoreCase)) - { - normalized = @"NT AUTHORITY\SYSTEM"; - } - - var machineIdentity = normalized.StartsWith(@"NT SERVICE\", StringComparison.OrdinalIgnoreCase) - || normalized.StartsWith(@"NT AUTHORITY\", StringComparison.OrdinalIgnoreCase); - return machineIdentity && !IsLocal(sqlServer, machineName) ? $@"{domain}\{machineName}$" : normalized; - } - - /// Whether a SQL Server data source (server, server\instance, tcp:server,port …) is this computer. - public static bool IsLocal(string sqlServer, string machineName) - { - ArgumentNullException.ThrowIfNull(sqlServer); - var host = ProtocolPrefix().Replace(sqlServer.Trim(), string.Empty).Split('\\', ',')[0].Trim(); - return host is "" or "." or "(local)" or "(localdb)" or "127.0.0.1" or "::1" - || host.Equals("localhost", StringComparison.OrdinalIgnoreCase) - || host.Equals(machineName, StringComparison.OrdinalIgnoreCase) - || host.StartsWith(machineName + ".", StringComparison.OrdinalIgnoreCase); - } - private static async Task EnsureLoginAsync(SqlConnection connection, string login) => await ScalarAsync(connection, """ IF SUSER_ID(@login) IS NULL @@ -282,9 +253,6 @@ public static partial class RepositorySetup return i >= 0 && i + 1 < args.Length ? args[i + 1] : null; } - [GeneratedRegex(@"^(tcp|np|lpc|admin):", RegexOptions.IgnoreCase)] - private static partial Regex ProtocolPrefix(); - [GeneratedRegex(@"SET @LoginName = N'[^']*';[^\r\n]*")] private static partial Regex LoginPlaceholder(); diff --git a/tests/SQLInsight.Agent.Tests/RepositorySetupTests.cs b/tests/SQLInsight.Agent.Tests/RepositorySetupTests.cs index 6a4e346..26cb292 100644 --- a/tests/SQLInsight.Agent.Tests/RepositorySetupTests.cs +++ b/tests/SQLInsight.Agent.Tests/RepositorySetupTests.cs @@ -1,4 +1,5 @@ using SQLInsight.Agent.Setup; +using SQLInsight.Core.Configuration; namespace SQLInsight.Agent.Tests; @@ -13,29 +14,29 @@ public sealed class RepositorySetupTests [InlineData("SQLMON01")] [InlineData(@"sqlmon01\SQLEXPRESS")] [InlineData("sqlmon01.kunde.local,1433")] - public void Recognises_this_computer(string server) => Assert.True(RepositorySetup.IsLocal(server, "SQLMON01")); + public void Recognises_this_computer(string server) => Assert.True(ServiceAccounts.IsLocal(server, "SQLMON01")); [Theory] [InlineData("sql02")] [InlineData(@"sql02\PROD")] [InlineData("sqlmon011")] [InlineData("tcp:10.0.0.5,1433")] - public void Recognises_other_computers(string server) => Assert.False(RepositorySetup.IsLocal(server, "SQLMON01")); + public void Recognises_other_computers(string server) => Assert.False(ServiceAccounts.IsLocal(server, "SQLMON01")); [Fact] public void Virtual_account_keeps_its_name_on_the_same_computer() => - Assert.Equal(@"NT SERVICE\SQLInsight Agent", RepositorySetup.LoginFor(@"NT SERVICE\SQLInsight Agent", @".\SQLEXPRESS", "SQLMON01", "KUNDE")); + Assert.Equal(@"NT SERVICE\SQLInsight Agent", ServiceAccounts.LoginFor(@"NT SERVICE\SQLInsight Agent", @".\SQLEXPRESS", "SQLMON01", "KUNDE")); [Theory] [InlineData(@"NT SERVICE\SQLInsight Agent")] [InlineData(@"NT AUTHORITY\NETWORK SERVICE")] [InlineData("LocalSystem")] public void Machine_accounts_arrive_as_the_computer_account_on_another_server(string account) => - Assert.Equal(@"KUNDE\SQLMON01$", RepositorySetup.LoginFor(account, "sql02", "SQLMON01", "KUNDE")); + Assert.Equal(@"KUNDE\SQLMON01$", ServiceAccounts.LoginFor(account, "sql02", "SQLMON01", "KUNDE")); [Fact] public void Domain_accounts_and_gmsas_are_the_same_everywhere() => - Assert.Equal(@"KUNDE\svc-sqlinsight$", RepositorySetup.LoginFor(@"KUNDE\svc-sqlinsight$", "sql02", "SQLMON01", "KUNDE")); + Assert.Equal(@"KUNDE\svc-sqlinsight$", ServiceAccounts.LoginFor(@"KUNDE\svc-sqlinsight$", "sql02", "SQLMON01", "KUNDE")); [Fact] public void Grant_script_takes_the_login_as_a_parameter()