Log server ID in connection logs table (#21911)

Content.Server.Database/Migrations/Postgres/20231126234054_ConnectionLogServer.cs

@@ -0,0 +1,55 @@
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace Content.Server.Database.Migrations.Postgres
+{
+    /// <inheritdoc />
+    public partial class ConnectionLogServer : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<int>(
+                name: "server_id",
+                table: "connection_log",
+                type: "integer",
+                nullable: false,
+                defaultValue: 0);
+
+            // Note: EF Core automatically makes indexes for all FKs.
+            // That's really dumb, and there's no simple way to disable this.
+            // So we drop the index creation command from the migration here,
+            // as we don't want this index:
+
+            // migrationBuilder.CreateIndex(
+            //     name: "IX_connection_log_server_id",
+            //     table: "connection_log",
+            //     column: "server_id");
+
+            migrationBuilder.AddForeignKey(
+                name: "FK_connection_log_server_server_id",
+                table: "connection_log",
+                column: "server_id",
+                principalTable: "server",
+                principalColumn: "server_id",
+                onDelete: ReferentialAction.SetNull);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropForeignKey(
+                name: "FK_connection_log_server_server_id",
+                table: "connection_log");
+
+            // migrationBuilder.DropIndex(
+            //     name: "IX_connection_log_server_id",
+            //     table: "connection_log");
+
+            migrationBuilder.DropColumn(
+                name: "server_id",
+                table: "connection_log");
+        }
+    }
+}

Content.Server.Database/Migrations/Postgres/PostgresServerDbContextModelSnapshot.cs

@@ -528,6 +528,12 @@ namespace Content.Server.Database.Migrations.Postgres
                         .HasColumnType("bytea")
                         .HasColumnName("hwid");
 
+                    b.Property<int>("ServerId")
+                        .ValueGeneratedOnAdd()
+                        .HasColumnType("integer")
+                        .HasDefaultValue(0)
+                        .HasColumnName("server_id");
+
                     b.Property<DateTime>("Time")
                         .HasColumnType("timestamp with time zone")
                         .HasColumnName("time");
@@ -544,6 +550,9 @@ namespace Content.Server.Database.Migrations.Postgres
                     b.HasKey("Id")
                         .HasName("PK_connection_log");
 
+                    b.HasIndex("ServerId")
+                        .HasDatabaseName("IX_connection_log_server_id");
+
                     b.HasIndex("UserId");
 
                     b.ToTable("connection_log", null, t =>
@@ -1466,6 +1475,18 @@ namespace Content.Server.Database.Migrations.Postgres
                     b.Navigation("Profile");
                 });
 
+            modelBuilder.Entity("Content.Server.Database.ConnectionLog", b =>
+                {
+                    b.HasOne("Content.Server.Database.Server", "Server")
+                        .WithMany("ConnectionLogs")
+                        .HasForeignKey("ServerId")
+                        .OnDelete(DeleteBehavior.SetNull)
+                        .IsRequired()
+                        .HasConstraintName("FK_connection_log_server_server_id");
+
+                    b.Navigation("Server");
+                });
+
             modelBuilder.Entity("Content.Server.Database.Job", b =>
                 {
                     b.HasOne("Content.Server.Database.Profile", "Profile")
@@ -1712,6 +1733,8 @@ namespace Content.Server.Database.Migrations.Postgres
 
             modelBuilder.Entity("Content.Server.Database.Server", b =>
                 {
+                    b.Navigation("ConnectionLogs");
+
                     b.Navigation("Rounds");
                 });
 

Content.Server.Database/Migrations/Sqlite/20231126234049_ConnectionLogServer.cs

@@ -0,0 +1,55 @@
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace Content.Server.Database.Migrations.Sqlite
+{
+    /// <inheritdoc />
+    public partial class ConnectionLogServer : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<int>(
+                name: "server_id",
+                table: "connection_log",
+                type: "INTEGER",
+                nullable: false,
+                defaultValue: 0);
+
+            // Note: EF Core automatically makes indexes for all FKs.
+            // That's really dumb, and there's no simple way to disable this.
+            // So we drop the index creation command from the migration here,
+            // as we don't want this index:
+
+            // migrationBuilder.CreateIndex(
+            //     name: "IX_connection_log_server_id",
+            //     table: "connection_log",
+            //     column: "server_id");
+
+            migrationBuilder.AddForeignKey(
+                name: "FK_connection_log_server_server_id",
+                table: "connection_log",
+                column: "server_id",
+                principalTable: "server",
+                principalColumn: "server_id",
+                onDelete: ReferentialAction.SetNull);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropForeignKey(
+                name: "FK_connection_log_server_server_id",
+                table: "connection_log");
+
+            // migrationBuilder.DropIndex(
+            //     name: "IX_connection_log_server_id",
+            //     table: "connection_log");
+
+            migrationBuilder.DropColumn(
+                name: "server_id",
+                table: "connection_log");
+        }
+    }
+}

Content.Server.Database/Migrations/Sqlite/SqliteServerDbContextModelSnapshot.cs

@@ -498,6 +498,12 @@ namespace Content.Server.Database.Migrations.Sqlite
                         .HasColumnType("BLOB")
                         .HasColumnName("hwid");
 
+                    b.Property<int>("ServerId")
+                        .ValueGeneratedOnAdd()
+                        .HasColumnType("INTEGER")
+                        .HasDefaultValue(0)
+                        .HasColumnName("server_id");
+
                     b.Property<DateTime>("Time")
                         .HasColumnType("TEXT")
                         .HasColumnName("time");
@@ -514,6 +520,9 @@ namespace Content.Server.Database.Migrations.Sqlite
                     b.HasKey("Id")
                         .HasName("PK_connection_log");
 
+                    b.HasIndex("ServerId")
+                        .HasDatabaseName("IX_connection_log_server_id");
+
                     b.HasIndex("UserId");
 
                     b.ToTable("connection_log", (string)null);
@@ -1398,6 +1407,18 @@ namespace Content.Server.Database.Migrations.Sqlite
                     b.Navigation("Profile");
                 });
 
+            modelBuilder.Entity("Content.Server.Database.ConnectionLog", b =>
+                {
+                    b.HasOne("Content.Server.Database.Server", "Server")
+                        .WithMany("ConnectionLogs")
+                        .HasForeignKey("ServerId")
+                        .OnDelete(DeleteBehavior.SetNull)
+                        .IsRequired()
+                        .HasConstraintName("FK_connection_log_server_server_id");
+
+                    b.Navigation("Server");
+                });
+
             modelBuilder.Entity("Content.Server.Database.Job", b =>
                 {
                     b.HasOne("Content.Server.Database.Profile", "Profile")
@@ -1644,6 +1665,8 @@ namespace Content.Server.Database.Migrations.Sqlite
 
             modelBuilder.Entity("Content.Server.Database.Server", b =>
                 {
+                    b.Navigation("ConnectionLogs");
+
                     b.Navigation("Rounds");
                 });
 

Content.Server.Database/Model.cs

@@ -168,6 +168,15 @@ namespace Content.Server.Database
             modelBuilder.Entity<ConnectionLog>()
                 .HasIndex(p => p.UserId);
 
+            modelBuilder.Entity<ConnectionLog>()
+                .Property(p => p.ServerId)
+                .HasDefaultValue(0);
+
+            modelBuilder.Entity<ConnectionLog>()
+                .HasOne(p => p.Server)
+                .WithMany(p => p.ConnectionLogs)
+                .OnDelete(DeleteBehavior.SetNull);
+
             // SetNull is necessary for created by/edited by-s here,
             // so you can safely delete admins (GDPR right to erasure) while keeping the notes intact
 
@@ -494,6 +503,9 @@ namespace Content.Server.Database
 
         [InverseProperty(nameof(Round.Server))]
         public List<Round> Rounds { get; set; } = default!;
+
+        [InverseProperty(nameof(ConnectionLog.Server))]
+        public List<ConnectionLog> ConnectionLogs { get; set; } = default!;
     }
 
     [Index(nameof(Type))]
@@ -749,7 +761,19 @@ namespace Content.Server.Database
 
         public ConnectionDenyReason? Denied { get; set; }
 
+        /// <summary>
+        /// ID of the <see cref="Server"/> that the connection was attempted to.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// The default value of this column is set to <c>0</c>, which is the ID of the "<c>unknown</c>" server.
+        /// This is intended for old entries (that didn't track this) and if the server name isn't configured.
+        /// </para>
+        /// </remarks>
+        public int ServerId { get; set; }
+
         public List<ServerBanHit> BanHits { get; set; } = null!;
+        public Server Server { get; set; } = null!;
     }
 
     public enum ConnectionDenyReason : byte

Content.Server/Connection/ConnectionManager.cs

@@ -29,6 +29,7 @@ namespace Content.Server.Connection
         [Dependency] private readonly IServerDbManager _db = default!;
         [Dependency] private readonly IConfigurationManager _cfg = default!;
         [Dependency] private readonly ILocalizationManager _loc = default!;
+        [Dependency] private readonly ServerDbEntryManager _serverDbEntry = default!;
 
         public void Initialize()
         {
@@ -66,11 +67,13 @@ namespace Content.Server.Connection
             var addr = e.IP.Address;
             var userId = e.UserId;
 
+            var serverId = (await _serverDbEntry.ServerEntity).Id;
+
             if (deny != null)
             {
                 var (reason, msg, banHits) = deny.Value;
 
-                var id = await _db.AddConnectionLogAsync(userId, e.UserName, addr, e.UserData.HWId, reason);
+                var id = await _db.AddConnectionLogAsync(userId, e.UserName, addr, e.UserData.HWId, reason, serverId);
                 if (banHits is { Count: > 0 })
                     await _db.AddServerBanHitsAsync(id, banHits);
 
@@ -78,7 +81,7 @@ namespace Content.Server.Connection
             }
             else
             {
-                await _db.AddConnectionLogAsync(userId, e.UserName, addr, e.UserData.HWId, null);
+                await _db.AddConnectionLogAsync(userId, e.UserName, addr, e.UserData.HWId, null, serverId);
 
                 if (!ServerPreferencesManager.ShouldStorePrefs(e.AuthType))
                     return;

Content.Server/Database/ServerDbBase.cs

@@ -570,7 +570,8 @@ namespace Content.Server.Database
             string userName,
             IPAddress address,
             ImmutableArray<byte> hwId,
-            ConnectionDenyReason? denied);
+            ConnectionDenyReason? denied,
+            int serverId);
 
         public async Task AddServerBanHitsAsync(int connection, IEnumerable<ServerBanDef> bans)
         {

Content.Server/Database/ServerDbEntryManager.cs

@@ -0,0 +1,39 @@
+using System.Threading.Tasks;
+using Content.Shared.CCVar;
+using Robust.Shared.Configuration;
+
+namespace Content.Server.Database;
+
+/// <summary>
+/// Stupid tiny manager whose sole purpose is keeping track of the <see cref="Server"/> database entry for this server.
+/// </summary>
+/// <remarks>
+/// This allows the value to be cached,
+/// so it can be easily retrieved by later code that needs to log the server ID to the database.
+/// </remarks>
+public sealed class ServerDbEntryManager
+{
+    [Dependency] private readonly IConfigurationManager _cfg = default!;
+    [Dependency] private readonly IServerDbManager _db = default!;
+    [Dependency] private readonly ILogManager _logManager = default!;
+
+    private Task<Server>? _serverEntityTask;
+
+    /// <summary>
+    /// The entity that represents this server in the database.
+    /// </summary>
+    /// <remarks>
+    /// This value is cached when first requested. Do not re-use this entity; if you need data like the rounds,
+    /// request it manually with <see cref="IServerDbManager.AddOrGetServer"/>.
+    /// </remarks>
+    public Task<Server> ServerEntity => _serverEntityTask ??= GetServerEntity();
+
+    private async Task<Server> GetServerEntity()
+    {
+        var name = _cfg.GetCVar(CCVars.AdminLogsServerName);
+        var server = await _db.AddOrGetServer(name);
+
+        _logManager.GetSawmill("db").Verbose("Server name: {Name}, ID in database: {Id}", server, server.Id);
+        return server;
+    }
+}

Content.Server/Database/ServerDbManager.cs

@@ -185,7 +185,8 @@ namespace Content.Server.Database
             string userName,
             IPAddress address,
             ImmutableArray<byte> hwId,
-            ConnectionDenyReason? denied);
+            ConnectionDenyReason? denied,
+            int serverId);
 
         Task AddServerBanHitsAsync(int connection, IEnumerable<ServerBanDef> bans);
 
@@ -514,10 +515,11 @@ namespace Content.Server.Database
             string userName,
             IPAddress address,
             ImmutableArray<byte> hwId,
-            ConnectionDenyReason? denied)
+            ConnectionDenyReason? denied,
+            int serverId)
         {
             DbWriteOpsMetric.Inc();
-            return RunDbCommand(() => _db.AddConnectionLogAsync(userId, userName, address, hwId, denied));
+            return RunDbCommand(() => _db.AddConnectionLogAsync(userId, userName, address, hwId, denied, serverId));
         }
 
         public Task AddServerBanHitsAsync(int connection, IEnumerable<ServerBanDef> bans)

Content.Server/Database/ServerDbPostgres.cs

@@ -463,7 +463,8 @@ namespace Content.Server.Database
             string userName,
             IPAddress address,
             ImmutableArray<byte> hwId,
-            ConnectionDenyReason? denied)
+            ConnectionDenyReason? denied,
+            int serverId)
         {
             await using var db = await GetDbImpl();
 
@@ -475,6 +476,7 @@ namespace Content.Server.Database
                 UserName = userName,
                 HWId = hwId.ToArray(),
                 Denied = denied,
+                ServerId = serverId
             };
 
             db.PgDbContext.ConnectionLog.Add(connectionLog);

Content.Server/Database/ServerDbSqlite.cs

@@ -418,7 +418,8 @@ namespace Content.Server.Database
             string userName,
             IPAddress address,
             ImmutableArray<byte> hwId,
-            ConnectionDenyReason? denied)
+            ConnectionDenyReason? denied,
+            int serverId)
         {
             await using var db = await GetDbImpl();
 
@@ -429,7 +430,8 @@ namespace Content.Server.Database
                 UserId = userId.UserId,
                 UserName = userName,
                 HWId = hwId.ToArray(),
-                Denied = denied
+                Denied = denied,
+                ServerId = serverId
             };
 
             db.SqliteDbContext.ConnectionLog.Add(connectionLog);

Content.Server/GameTicking/GameTicker.GamePreset.cs

@@ -316,7 +316,7 @@ namespace Content.Server.GameTicking
             // This whole setup logic should be made asynchronous so we can properly wait on the DB AAAAAAAAAAAAAH
             var task = Task.Run(async () =>
             {
-                var server = await _db.AddOrGetServer(serverName);
+                var server = await _dbEntryManager.ServerEntity;
                 return await _db.AddNewRound(server, playerIds);
             });
 

Content.Server/GameTicking/GameTicker.cs

@@ -70,6 +70,7 @@ namespace Content.Server.GameTicking
         [Dependency] private readonly UserDbDataManager _userDb = default!;
         [Dependency] private readonly MetaDataSystem _metaData = default!;
         [Dependency] private readonly SharedRoleSystem _roles = default!;
+        [Dependency] private readonly ServerDbEntryManager _dbEntryManager = default!;
 
         [ViewVariables] private bool _initialized;
         [ViewVariables] private bool _postInitialized;

Content.Server/IoC/ServerContentIoC.cs

@@ -57,6 +57,7 @@ namespace Content.Server.IoC
             IoCManager.Register<ServerInfoManager>();
             IoCManager.Register<PoissonDiskSampler>();
             IoCManager.Register<DiscordWebhook>();
+            IoCManager.Register<ServerDbEntryManager>();
         }
     }
 }