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) <noreply@anthropic.com>
This commit is contained in:
Søren Eggert Lundsteen Olsen 2026-10-02 12:39:07 +02:00
parent 5ad9355130
commit 29e1909e98
2 changed files with 10 additions and 41 deletions

View file

@ -2,6 +2,7 @@ using System.Diagnostics;
using System.Text.RegularExpressions; using System.Text.RegularExpressions;
using Microsoft.Data.SqlClient; using Microsoft.Data.SqlClient;
using SQLInsight.Connectivity; using SQLInsight.Connectivity;
using SQLInsight.Core.Configuration;
using SQLInsight.Repository; using SQLInsight.Repository;
namespace SQLInsight.Agent.Setup; namespace SQLInsight.Agent.Setup;
@ -146,7 +147,7 @@ public static partial class RepositorySetup
return; 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 var target = new SqlConnectionStringBuilder
{ {
DataSource = instance.Server, DataSource = instance.Server,
@ -174,43 +175,13 @@ public static partial class RepositorySetup
/// </summary> /// </summary>
public static IReadOnlyList<(string Login, string Role)> RepositoryLogins(string agentAccount, string webAccount, string sqlServer) public static IReadOnlyList<(string Login, string Role)> RepositoryLogins(string agentAccount, string webAccount, string sqlServer)
{ {
var agent = LoginFor(agentAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); var agent = ServiceAccounts.LoginFor(agentAccount, sqlServer, Environment.MachineName, Environment.UserDomainName);
var web = LoginFor(webAccount, sqlServer, Environment.MachineName, Environment.UserDomainName); var web = ServiceAccounts.LoginFor(webAccount, sqlServer, Environment.MachineName, Environment.UserDomainName);
return string.Equals(agent, web, StringComparison.OrdinalIgnoreCase) return string.Equals(agent, web, StringComparison.OrdinalIgnoreCase)
? [(agent, "db_owner")] ? [(agent, "db_owner")]
: [(agent, "db_owner"), (web, "db_datareader")]; : [(agent, "db_owner"), (web, "db_datareader")];
} }
/// <summary>
/// The Windows login a service account has on <paramref name="sqlServer"/>. 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.
/// </summary>
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;
}
/// <summary>Whether a SQL Server data source (server, server\instance, tcp:server,port …) is this computer.</summary>
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<string> EnsureLoginAsync(SqlConnection connection, string login) => private static async Task<string> EnsureLoginAsync(SqlConnection connection, string login) =>
await ScalarAsync(connection, """ await ScalarAsync(connection, """
IF SUSER_ID(@login) IS NULL 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; 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]*")] [GeneratedRegex(@"SET @LoginName = N'[^']*';[^\r\n]*")]
private static partial Regex LoginPlaceholder(); private static partial Regex LoginPlaceholder();

View file

@ -1,4 +1,5 @@
using SQLInsight.Agent.Setup; using SQLInsight.Agent.Setup;
using SQLInsight.Core.Configuration;
namespace SQLInsight.Agent.Tests; namespace SQLInsight.Agent.Tests;
@ -13,29 +14,29 @@ public sealed class RepositorySetupTests
[InlineData("SQLMON01")] [InlineData("SQLMON01")]
[InlineData(@"sqlmon01\SQLEXPRESS")] [InlineData(@"sqlmon01\SQLEXPRESS")]
[InlineData("sqlmon01.kunde.local,1433")] [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] [Theory]
[InlineData("sql02")] [InlineData("sql02")]
[InlineData(@"sql02\PROD")] [InlineData(@"sql02\PROD")]
[InlineData("sqlmon011")] [InlineData("sqlmon011")]
[InlineData("tcp:10.0.0.5,1433")] [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] [Fact]
public void Virtual_account_keeps_its_name_on_the_same_computer() => 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] [Theory]
[InlineData(@"NT SERVICE\SQLInsight Agent")] [InlineData(@"NT SERVICE\SQLInsight Agent")]
[InlineData(@"NT AUTHORITY\NETWORK SERVICE")] [InlineData(@"NT AUTHORITY\NETWORK SERVICE")]
[InlineData("LocalSystem")] [InlineData("LocalSystem")]
public void Machine_accounts_arrive_as_the_computer_account_on_another_server(string account) => 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] [Fact]
public void Domain_accounts_and_gmsas_are_the_same_everywhere() => 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] [Fact]
public void Grant_script_takes_the_login_as_a_parameter() public void Grant_script_takes_the_login_as_a_parameter()