Fixes client having authority over rules popup cvars (#28655)

* Fixes client having authority over rules popup cvars

* Delete duplicate migration

* Pre-update

* Post-update

Content.Client/UserInterface/Systems/Info/InfoUIController.cs

@@ -1,13 +1,9 @@
-using System.Globalization;
 using Content.Client.Gameplay;
 using Content.Client.Info;
-using Content.Shared.Administration.Managers;
 using Content.Shared.CCVar;
 using Content.Shared.Guidebook;
 using Content.Shared.Info;
-using Robust.Client;
 using Robust.Client.Console;
-using Robust.Client.Player;
 using Robust.Client.UserInterface.Controllers;
 using Robust.Client.UserInterface.Controls;
 using Robust.Shared.Configuration;
@@ -18,24 +14,20 @@ namespace Content.Client.UserInterface.Systems.Info;
 
 public sealed class InfoUIController : UIController, IOnStateExited<GameplayState>
 {
-    [Dependency] private readonly IBaseClient _client = default!;
     [Dependency] private readonly IConfigurationManager _cfg = default!;
     [Dependency] private readonly IClientConsoleHost _consoleHost = default!;
     [Dependency] private readonly INetManager _netManager = default!;
-    [Dependency] private readonly ISharedAdminManager _adminManager = default!;
-    [Dependency] private readonly IPlayerManager _playerManager = default!;
     [Dependency] private readonly IPrototypeManager _prototype = default!;
 
     private RulesPopup? _rulesPopup;
     private RulesAndInfoWindow? _infoWindow;
 
-    private static DateTime NextRulesReadTime => DateTime.UtcNow + TimeSpan.FromDays(60);
-
     public override void Initialize()
     {
         base.Initialize();
 
-        _client.PlayerJoinedServer += OnJoinedServer;
+
+        _netManager.RegisterNetMessage<RulesAcceptedMessage>();
         _netManager.RegisterNetMessage<ShowRulesPopupMessage>(OnShowRulesPopupMessage);
 
         _consoleHost.RegisterCommand("fuckrules",
@@ -47,22 +39,6 @@ public sealed class InfoUIController : UIController, IOnStateExited<GameplayStat
         });
     }
 
-    private void OnJoinedServer(object? sender, PlayerEventArgs args)
-    {
-        if (_playerManager.LocalSession is not { } localSession)
-            return;
-
-        if (_adminManager.IsAdmin(localSession) && _cfg.GetCVar(CCVars.RulesExemptLocal))
-            return;
-
-        var nextReadTarget = DateTime.Parse(_cfg.GetCVar(CCVars.RulesNextPopupTime));
-        if (nextReadTarget >= DateTime.UtcNow)
-            return;
-
-        var time = _cfg.GetCVar(CCVars.RulesWaitTime);
-        ShowRules(time);
-    }
-
     private void OnShowRulesPopupMessage(ShowRulesPopupMessage message)
     {
         ShowRules(message.PopupTime);
@@ -100,8 +76,7 @@ public sealed class InfoUIController : UIController, IOnStateExited<GameplayStat
 
     private void OnAcceptPressed()
     {
-        _cfg.SetCVar(CCVars.RulesNextPopupTime, NextRulesReadTime.ToString(CultureInfo.InvariantCulture));
+        _netManager.ClientSendMessage(new RulesAcceptedMessage());
-        _cfg.SaveToFile();
 
         _rulesPopup?.Orphan();
         _rulesPopup = null;

Content.Server.Database/Migrations/Postgres/20240606175154_ReturnLastReadRules.cs

@@ -0,0 +1,29 @@
+using System;
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace Content.Server.Database.Migrations.Postgres
+{
+    /// <inheritdoc />
+    public partial class ReturnLastReadRules : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<DateTime>(
+                name: "last_read_rules",
+                table: "player",
+                type: "timestamp with time zone",
+                nullable: true);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropColumn(
+                name: "last_read_rules",
+                table: "player");
+        }
+    }
+}

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

@@ -648,6 +648,10 @@ namespace Content.Server.Database.Migrations.Postgres
                         .HasColumnType("timestamp with time zone")
                         .HasColumnName("first_seen_time");
 
+                    b.Property<DateTime?>("LastReadRules")
+                        .HasColumnType("timestamp with time zone")
+                        .HasColumnName("last_read_rules");
+
                     b.Property<IPAddress>("LastSeenAddress")
                         .IsRequired()
                         .HasColumnType("inet")

Content.Server.Database/Migrations/Sqlite/20240606175141_ReturnLastReadRules.cs

@@ -0,0 +1,29 @@
+using System;
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace Content.Server.Database.Migrations.Sqlite
+{
+    /// <inheritdoc />
+    public partial class ReturnLastReadRules : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<DateTime>(
+                name: "last_read_rules",
+                table: "player",
+                type: "TEXT",
+                nullable: true);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropColumn(
+                name: "last_read_rules",
+                table: "player");
+        }
+    }
+}

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

@@ -608,6 +608,10 @@ namespace Content.Server.Database.Migrations.Sqlite
                         .HasColumnType("TEXT")
                         .HasColumnName("first_seen_time");
 
+                    b.Property<DateTime?>("LastReadRules")
+                        .HasColumnType("TEXT")
+                        .HasColumnName("last_read_rules");
+
                     b.Property<string>("LastSeenAddress")
                         .IsRequired()
                         .HasColumnType("TEXT")

Content.Server.Database/Model.cs

@@ -520,6 +520,8 @@ namespace Content.Server.Database
         public List<Round> Rounds { get; set; } = null!;
         public List<AdminLogPlayer> AdminLogs { get; set; } = null!;
 
+        public DateTime? LastReadRules { get; set; }
+
         public List<AdminNote> AdminNotesReceived { get; set; } = null!;
         public List<AdminNote> AdminNotesCreated { get; set; } = null!;
         public List<AdminNote> AdminNotesLastEdited { get; set; } = null!;

Content.Server/Administration/Managers/AdminManager.cs

@@ -240,7 +240,6 @@ namespace Content.Server.Administration.Managers
             _sawmill = _logManager.GetSawmill("admin");
 
             _netMgr.RegisterNetMessage<MsgUpdateAdminStatus>();
-            _netMgr.RegisterNetMessage<ShowRulesPopupMessage>();
 
             // Cache permissions for loaded console commands with the requisite attributes.
             foreach (var (cmdName, cmd) in _consoleHost.AvailableCommands)

Content.Server/Database/ServerDbBase.cs

@@ -1032,6 +1032,30 @@ INSERT INTO player_round (players_id, rounds_id) VALUES ({players[player]}, {id}
             await db.DbContext.SaveChangesAsync();
         }
 
+        public async Task<DateTimeOffset?> GetLastReadRules(NetUserId player)
+        {
+            await using var db = await GetDb();
+
+            return NormalizeDatabaseTime(await db.DbContext.Player
+                .Where(dbPlayer => dbPlayer.UserId == player)
+                .Select(dbPlayer => dbPlayer.LastReadRules)
+                .SingleOrDefaultAsync());
+        }
+
+        public async Task SetLastReadRules(NetUserId player, DateTimeOffset date)
+        {
+            await using var db = await GetDb();
+
+            var dbPlayer = await db.DbContext.Player.Where(dbPlayer => dbPlayer.UserId == player).SingleOrDefaultAsync();
+            if (dbPlayer == null)
+            {
+                return;
+            }
+
+            dbPlayer.LastReadRules = date.UtcDateTime;
+            await db.DbContext.SaveChangesAsync();
+        }
+
         #endregion
 
         #region Uploaded Resources Logs

Content.Server/Database/ServerDbManager.cs

@@ -252,6 +252,13 @@ namespace Content.Server.Database
 
         #endregion
 
+        #region Rules
+
+        Task<DateTimeOffset?> GetLastReadRules(NetUserId player);
+        Task SetLastReadRules(NetUserId player, DateTimeOffset time);
+
+        #endregion
+
         #region Admin Notes
 
         Task<int> AddAdminNote(int? roundId, Guid player, TimeSpan playtimeAtNote, string message, NoteSeverity severity, bool secret, Guid createdBy, DateTimeOffset createdAt, DateTimeOffset? expiryTime);
@@ -700,6 +707,18 @@ namespace Content.Server.Database
             return RunDbCommand(() => _db.PurgeUploadedResourceLogAsync(days));
         }
 
+        public Task<DateTimeOffset?> GetLastReadRules(NetUserId player)
+        {
+            DbReadOpsMetric.Inc();
+            return RunDbCommand(() => _db.GetLastReadRules(player));
+        }
+
+        public Task SetLastReadRules(NetUserId player, DateTimeOffset time)
+        {
+            DbWriteOpsMetric.Inc();
+            return RunDbCommand(() => _db.SetLastReadRules(player, time));
+        }
+
         public Task<int> AddAdminNote(int? roundId, Guid player, TimeSpan playtimeAtNote, string message, NoteSeverity severity, bool secret, Guid createdBy, DateTimeOffset createdAt, DateTimeOffset? expiryTime)
         {
             DbWriteOpsMetric.Inc();

Content.Server/Entry/EntryPoint.cs

@@ -10,6 +10,7 @@ using Content.Server.EUI;
 using Content.Server.GameTicking;
 using Content.Server.GhostKick;
 using Content.Server.GuideGenerator;
+using Content.Server.Info;
 using Content.Server.IoC;
 using Content.Server.Maps;
 using Content.Server.NodeContainer.NodeGroups;
@@ -135,6 +136,7 @@ namespace Content.Server.Entry
                 IoCManager.Resolve<RecipeManager>().Initialize();
                 IoCManager.Resolve<IAdminManager>().Initialize();
                 IoCManager.Resolve<IAfkManager>().Initialize();
+                IoCManager.Resolve<RulesManager>().Initialize();
                 _euiManager.Initialize();
 
                 IoCManager.Resolve<IGameMapManager>().Initialize();

Content.Server/Info/RulesManager.cs

@@ -0,0 +1,44 @@
+using System.Net;
+using Content.Server.Database;
+using Content.Shared.CCVar;
+using Content.Shared.Info;
+using Robust.Shared.Configuration;
+using Robust.Shared.Network;
+
+namespace Content.Server.Info;
+
+public sealed class RulesManager
+{
+    [Dependency] private readonly IServerDbManager _dbManager = default!;
+    [Dependency] private readonly INetManager _netManager = default!;
+    [Dependency] private readonly IConfigurationManager _cfg = default!;
+
+    private static DateTime LastValidReadTime => DateTime.UtcNow - TimeSpan.FromDays(60);
+
+    public void Initialize()
+    {
+        _netManager.Connected += OnConnected;
+        _netManager.RegisterNetMessage<ShowRulesPopupMessage>();
+        _netManager.RegisterNetMessage<RulesAcceptedMessage>(OnRulesAccepted);
+    }
+
+    private async void OnConnected(object? sender, NetChannelArgs e)
+    {
+        if (IPAddress.IsLoopback(e.Channel.RemoteEndPoint.Address) && _cfg.GetCVar(CCVars.RulesExemptLocal))
+            return;
+
+        var lastRead = await _dbManager.GetLastReadRules(e.Channel.UserId);
+        if (lastRead > LastValidReadTime)
+            return;
+
+        var message = new ShowRulesPopupMessage();
+        message.PopupTime = _cfg.GetCVar(CCVars.RulesWaitTime);
+        _netManager.ServerSendMessage(message, e.Channel);
+    }
+
+    private async void OnRulesAccepted(RulesAcceptedMessage message)
+    {
+        var date = DateTime.UtcNow;
+        await _dbManager.SetLastReadRules(message.MsgChannel.UserId, date);
+    }
+}

Content.Server/IoC/ServerContentIoC.cs

@@ -9,6 +9,7 @@ using Content.Server.Database;
 using Content.Server.Discord;
 using Content.Server.EUI;
 using Content.Server.GhostKick;
+using Content.Server.Info;
 using Content.Server.Maps;
 using Content.Server.MoMMI;
 using Content.Server.NodeContainer.NodeGroups;
@@ -46,6 +47,7 @@ namespace Content.Server.IoC
             IoCManager.Register<IPlayerLocator, PlayerLocator>();
             IoCManager.Register<IAfkManager, AfkManager>();
             IoCManager.Register<IGameMapManager, GameMapManager>();
+            IoCManager.Register<RulesManager, RulesManager>();
             IoCManager.Register<IBanManager, BanManager>();
             IoCManager.Register<ContentNetworkResourceManager>();
             IoCManager.Register<IAdminNotesManager, AdminNotesManager>();

Content.Shared/CCVar/CCVars.cs

@@ -1796,13 +1796,7 @@ namespace Content.Shared.CCVar
         /// Don't show rules to localhost/loopback interface.
         /// </summary>
         public static readonly CVarDef<bool> RulesExemptLocal =
-            CVarDef.Create("rules.exempt_local", true, CVar.CLIENT);
+            CVarDef.Create("rules.exempt_local", false, CVar.SERVERONLY);
-
-        /// <summary>
-        /// The next time the rules will popup for this client, expressed in minutes
-        /// </summary>
-        public static readonly CVarDef<string> RulesNextPopupTime =
-            CVarDef.Create("rules.next_popup_time", "Jan 1, 1997", CVar.CLIENTONLY | CVar.ARCHIVE);
 
 
         /*

Content.Shared/Info/RulesMessages.cs

@@ -23,3 +23,19 @@ public sealed class ShowRulesPopupMessage : NetMessage
         buffer.Write(PopupTime);
     }
 }
+
+/// <summary>
+///     Sent by the client when it has accepted the rules.
+/// </summary>
+public sealed class RulesAcceptedMessage : NetMessage
+{
+    public override MsgGroups MsgGroup => MsgGroups.Command;
+
+    public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer serializer)
+    {
+    }
+
+    public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer serializer)
+    {
+    }
+}