Add pardon command and tests (#3190)

* Add pardon command

* Make pardoning check for an existing ban and unban first

* Add pardon test and documentation

Content.IntegrationTests/Tests/Commands/PardonCommand.cs

@@ -0,0 +1,127 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Content.Server.Database;
+using NUnit.Framework;
+using Robust.Server.Console;
+using Robust.Server.Player;
+
+namespace Content.IntegrationTests.Tests.Commands
+{
+    [TestFixture]
+    [TestOf(typeof(PardonCommand))]
+    public class PardonCommand : ContentIntegrationTest
+    {
+        private static readonly TimeSpan MarginOfError = TimeSpan.FromMinutes(1);
+
+        [Test]
+        public async Task PardonTest()
+        {
+            var (client, server) = await StartConnectedServerClientPair();
+
+            await Task.WhenAll(client.WaitIdleAsync(), server.WaitIdleAsync());
+
+            var sPlayerManager = server.ResolveDependency<IPlayerManager>();
+            var sConsole = server.ResolveDependency<IServerConsoleHost>();
+            var sDatabase = server.ResolveDependency<IServerDbManager>();
+
+            await server.WaitAssertion(async () =>
+            {
+                var clientSession = sPlayerManager.GetAllPlayers().Single();
+                var clientId = clientSession.UserId;
+
+                // No bans on record
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Null);
+                Assert.That(await sDatabase.GetServerBanAsync(1), Is.Null);
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Is.Empty);
+
+                // Try to pardon a ban that does not exist
+                sConsole.ExecuteCommand("pardon 1");
+
+                // Still no bans on record
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Null);
+                Assert.That(await sDatabase.GetServerBanAsync(1), Is.Null);
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Is.Empty);
+
+                var banReason = "test";
+
+                // Ban the client for 24 hours
+                sConsole.ExecuteCommand($"ban {clientSession.Name} {banReason} 1440");
+
+                // Should have one ban on record now
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Not.Null);
+                Assert.That(await sDatabase.GetServerBanAsync(1), Is.Not.Null);
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Has.Count.EqualTo(1));
+
+                // Try to pardon a ban that does not exist
+                sConsole.ExecuteCommand("pardon 2");
+
+                // The existing ban is unaffected
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Not.Null);
+
+                var ban = await sDatabase.GetServerBanAsync(1);
+                Assert.That(ban, Is.Not.Null);
+
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Has.Count.EqualTo(1));
+
+                // Check that it matches
+                Assert.That(ban.Id, Is.EqualTo(1));
+                Assert.That(ban.UserId, Is.EqualTo(clientId));
+                Assert.That(ban.BanTime.UtcDateTime - DateTime.UtcNow, Is.LessThanOrEqualTo(MarginOfError));
+                Assert.NotNull(ban.ExpirationTime);
+                Assert.That(ban.ExpirationTime.Value.UtcDateTime - DateTime.UtcNow.AddHours(24), Is.LessThanOrEqualTo(MarginOfError));
+                Assert.That(ban.Reason, Is.EqualTo(banReason));
+
+                // Done through the console
+                Assert.That(ban.BanningAdmin, Is.Null);
+
+                Assert.That(ban.Unban, Is.Null);
+
+                // Pardon the actual ban
+                sConsole.ExecuteCommand("pardon 1");
+
+                // No bans should be returned
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Null);
+
+                // Direct id lookup returns a pardoned ban
+                var pardonedBan = await sDatabase.GetServerBanAsync(1);
+                Assert.That(pardonedBan, Is.Not.Null);
+
+                // The list is still returned since that ignores pardons
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Has.Count.EqualTo(1));
+
+                // Check that it matches
+                Assert.That(pardonedBan.Id, Is.EqualTo(1));
+                Assert.That(pardonedBan.UserId, Is.EqualTo(clientId));
+                Assert.That(pardonedBan.BanTime.UtcDateTime - DateTime.UtcNow, Is.LessThanOrEqualTo(MarginOfError));
+                Assert.NotNull(pardonedBan.ExpirationTime);
+                Assert.That(pardonedBan.ExpirationTime.Value.UtcDateTime - DateTime.UtcNow.AddHours(24), Is.LessThanOrEqualTo(MarginOfError));
+                Assert.That(pardonedBan.Reason, Is.EqualTo(banReason));
+
+                // Done through the console
+                Assert.That(pardonedBan.BanningAdmin, Is.Null);
+
+                Assert.That(pardonedBan.Unban, Is.Not.Null);
+                Assert.That(pardonedBan.Unban.BanId, Is.EqualTo(1));
+
+                // Done through the console
+                Assert.That(pardonedBan.Unban.UnbanningAdmin, Is.Null);
+
+                Assert.That(pardonedBan.Unban.UnbanTime.UtcDateTime - DateTime.UtcNow, Is.LessThanOrEqualTo(MarginOfError));
+
+                // Try to pardon it again
+                sConsole.ExecuteCommand("pardon 1");
+
+                // Nothing changes
+                // No bans should be returned
+                Assert.That(await sDatabase.GetServerBanAsync(null, clientId), Is.Null);
+
+                // Direct id lookup returns a pardoned ban
+                Assert.That(await sDatabase.GetServerBanAsync(1), Is.Not.Null);
+
+                // The list is still returned since that ignores pardons
+                Assert.That(await sDatabase.GetServerBansAsync(null, clientId), Has.Count.EqualTo(1));
+            });
+        }
+    }
+}

Content.Server/Administration/Commands/BanCommand.cs

@@ -48,7 +48,7 @@ namespace Content.Server.Administration.Commands
                 expires = DateTimeOffset.Now + TimeSpan.FromMinutes(duration);
             }
 
-            await dbMan.AddServerBanAsync(new ServerBanDef(null, targetUid, null, DateTimeOffset.Now, expires, reason, player?.UserId));
+            await dbMan.AddServerBanAsync(new ServerBanDef(null, targetUid, null, DateTimeOffset.Now, expires, reason, player?.UserId, null));
 
             if (plyMgr.TryGetSessionById(targetUid, out var targetPlayer))
             {

Content.Server/Administration/Commands/PardonCommand.cs

@@ -0,0 +1,65 @@
+using System;
+using System.Text;
+using Content.Server.Database;
+using Content.Shared.Administration;
+using Robust.Server.Player;
+using Robust.Shared.Console;
+using Robust.Shared.IoC;
+
+#nullable enable
+
+namespace Content.Server.Administration.Commands
+{
+    [AdminCommand(AdminFlags.Ban)]
+    public class PardonCommand : IConsoleCommand
+    {
+        public string Command => "pardon";
+        public string Description => "Pardons somebody's ban";
+        public string Help => $"Usage: {Command} <ban id>";
+
+        public async void Execute(IConsoleShell shell, string argStr, string[] args)
+        {
+            var player = shell.Player as IPlayerSession;
+            var dbMan = IoCManager.Resolve<IServerDbManager>();
+
+            if (args.Length != 1)
+            {
+                shell.WriteLine(Help);
+                return;
+            }
+
+            if (!int.TryParse(args[0], out var banId))
+            {
+                shell.WriteLine($"Unable to parse {args[1]} as a ban id integer.\n{Help}");
+                return;
+            }
+
+            var ban = await dbMan.GetServerBanAsync(banId);
+
+            if (ban == null)
+            {
+                shell.WriteLine($"No ban found with id {banId}");
+                return;
+            }
+
+            if (ban.Unban != null)
+            {
+                var response = new StringBuilder("This ban has already been pardoned");
+
+                if (ban.Unban.UnbanningAdmin != null)
+                {
+                    response.Append($" by {ban.Unban.UnbanningAdmin.Value}");
+                }
+
+                response.Append($" in {ban.Unban.UnbanTime}.");
+
+                shell.WriteLine(response.ToString());
+                return;
+            }
+
+            await dbMan.AddServerUnbanAsync(new ServerUnbanDef(banId, player?.UserId, DateTimeOffset.Now));
+
+            shell.WriteLine($"Pardoned ban with id {banId}");
+        }
+    }
+}

Content.Server/Database/ServerBanDef.cs

@@ -16,8 +16,17 @@ namespace Content.Server.Database
         public DateTimeOffset? ExpirationTime { get; }
         public string Reason { get; }
         public NetUserId? BanningAdmin { get; }
+        public ServerUnbanDef? Unban { get; }
 
-        public ServerBanDef(int? id, NetUserId? userId, (IPAddress, int)? address, DateTimeOffset banTime, DateTimeOffset? expirationTime, string reason, NetUserId? banningAdmin)
+        public ServerBanDef(
+            int? id,
+            NetUserId? userId,
+            (IPAddress, int)? address,
+            DateTimeOffset banTime,
+            DateTimeOffset? expirationTime,
+            string reason,
+            NetUserId? banningAdmin,
+            ServerUnbanDef? unban)
         {
             if (userId == null && address == null)
             {
@@ -38,6 +47,7 @@ namespace Content.Server.Database
             ExpirationTime = expirationTime;
             Reason = reason;
             BanningAdmin = banningAdmin;
+            Unban = unban;
         }
     }
 }

Content.Server/Database/ServerDbBase.cs

@@ -7,9 +7,9 @@ using System.Threading;
 using System.Threading.Tasks;
 using Content.Shared.Preferences;
 using Microsoft.EntityFrameworkCore;
+using Robust.Shared.Localization.Macros;
 using Robust.Shared.Maths;
 using Robust.Shared.Network;
-using Robust.Shared.Localization.Macros;
 
 namespace Content.Server.Database
 {
@@ -229,9 +229,36 @@ namespace Content.Server.Database
         /*
          * BAN STUFF
          */
+        /// <summary>
+        ///     Looks up a ban by id.
+        ///     This will return a pardoned ban as well.
+        /// </summary>
+        /// <param name="id">The ban id to look for.</param>
+        /// <returns>The ban with the given id or null if none exist.</returns>
+        public abstract Task<ServerBanDef?> GetServerBanAsync(int id);
+
+        /// <summary>
+        ///     Looks up an user's most recent received un-pardoned ban.
+        ///     This will NOT return a pardoned ban.
+        ///     One of <see cref="address"/> or <see cref="userId"/> need to not be null.
+        /// </summary>
+        /// <param name="address">The ip address of the user.</param>
+        /// <param name="userId">The id of the user.</param>
+        /// <returns>The user's latest received un-pardoned ban, or null if none exist.</returns>
         public abstract Task<ServerBanDef?> GetServerBanAsync(IPAddress? address, NetUserId? userId);
+
+        /// <summary>
+        ///     Looks up an user's ban history.
+        ///     This will return pardoned bans as well.
+        ///     One of <see cref="address"/> or <see cref="userId"/> need to not be null.
+        /// </summary>
+        /// <param name="address">The ip address of the user.</param>
+        /// <param name="userId">The id of the user.</param>
+        /// <returns>The user's ban history.</returns>
         public abstract Task<List<ServerBanDef>> GetServerBansAsync(IPAddress? address, NetUserId? userId);
+
         public abstract Task AddServerBanAsync(ServerBanDef serverBan);
+        public abstract Task AddServerUnbanAsync(ServerUnbanDef serverUnban);
 
         /*
          * PLAYER RECORDS

Content.Server/Database/ServerDbManager.cs

@@ -15,8 +15,8 @@ using Robust.Shared.ContentPack;
 using Robust.Shared.IoC;
 using Robust.Shared.Log;
 using Robust.Shared.Network;
-using MSLogLevel = Microsoft.Extensions.Logging.LogLevel;
 using LogLevel = Robust.Shared.Log.LogLevel;
+using MSLogLevel = Microsoft.Extensions.Logging.LogLevel;
 
 #nullable enable
 
@@ -41,9 +41,36 @@ namespace Content.Server.Database
         Task<NetUserId?> GetAssignedUserIdAsync(string name);
 
         // Ban stuff
+        /// <summary>
+        ///     Looks up a ban by id.
+        ///     This will return a pardoned ban as well.
+        /// </summary>
+        /// <param name="id">The ban id to look for.</param>
+        /// <returns>The ban with the given id or null if none exist.</returns>
+        Task<ServerBanDef?> GetServerBanAsync(int id);
+
+        /// <summary>
+        ///     Looks up an user's most recent received un-pardoned ban.
+        ///     This will NOT return a pardoned ban.
+        ///     One of <see cref="address"/> or <see cref="userId"/> need to not be null.
+        /// </summary>
+        /// <param name="address">The ip address of the user.</param>
+        /// <param name="userId">The id of the user.</param>
+        /// <returns>The user's latest received un-pardoned ban, or null if none exist.</returns>
         Task<ServerBanDef?> GetServerBanAsync(IPAddress? address, NetUserId? userId);
+
+        /// <summary>
+        ///     Looks up an user's ban history.
+        ///     This will return pardoned bans as well.
+        ///     One of <see cref="address"/> or <see cref="userId"/> need to not be null.
+        /// </summary>
+        /// <param name="address">The ip address of the user.</param>
+        /// <param name="userId">The id of the user.</param>
+        /// <returns>The user's ban history.</returns>
         Task<List<ServerBanDef>> GetServerBansAsync(IPAddress? address, NetUserId? userId);
+
         Task AddServerBanAsync(ServerBanDef serverBan);
+        Task AddServerUnbanAsync(ServerUnbanDef serverBan);
 
         // Player records
         Task UpdatePlayerRecordAsync(NetUserId userId, string userName, IPAddress address);
@@ -139,6 +166,11 @@ namespace Content.Server.Database
             return _db.GetAssignedUserIdAsync(name);
         }
 
+        public Task<ServerBanDef?> GetServerBanAsync(int id)
+        {
+            return _db.GetServerBanAsync(id);
+        }
+
         public Task<ServerBanDef?> GetServerBanAsync(IPAddress? address, NetUserId? userId)
         {
             return _db.GetServerBanAsync(address, userId);
@@ -154,6 +186,11 @@ namespace Content.Server.Database
             return _db.AddServerBanAsync(serverBan);
         }
 
+        public Task AddServerUnbanAsync(ServerUnbanDef serverUnban)
+        {
+            return _db.AddServerUnbanAsync(serverUnban);
+        }
+
         public Task UpdatePlayerRecordAsync(NetUserId userId, string userName, IPAddress address)
         {
             return _db.UpdatePlayerRecord(userId, userName, address);

Content.Server/Database/ServerDbPostgres.cs

@@ -35,6 +35,19 @@ namespace Content.Server.Database
             });
         }
 
+        public override async Task<ServerBanDef?> GetServerBanAsync(int id)
+        {
+            await using var db = await GetDbImpl();
+
+            var query = db.PgDbContext.Ban
+                .Include(p => p.Unban)
+                .Where(p => p.Id == id);
+
+            var ban = await query.SingleOrDefaultAsync();
+
+            return ConvertBan(ban);
+        }
+
         public override async Task<ServerBanDef?> GetServerBanAsync(IPAddress? address, NetUserId? userId)
         {
             if (address == null && userId == null)
@@ -144,6 +157,8 @@ namespace Content.Server.Database
                 aUid = new NetUserId(aGuid);
             }
 
+            var unbanDef = ConvertUnban(ban.Unban);
+
             return new ServerBanDef(
                 ban.Id,
                 uid,
@@ -151,7 +166,27 @@ namespace Content.Server.Database
                 ban.BanTime,
                 ban.ExpirationTime,
                 ban.Reason,
-                aUid);
+                aUid,
+                unbanDef);
+        }
+
+        private static ServerUnbanDef? ConvertUnban(PostgresServerUnban? unban)
+        {
+            if (unban == null)
+            {
+                return null;
+            }
+
+            NetUserId? aUid = null;
+            if (unban.UnbanningAdmin is {} aGuid)
+            {
+                aUid = new NetUserId(aGuid);
+            }
+
+            return new ServerUnbanDef(
+                unban.Id,
+                aUid,
+                unban.UnbanTime);
         }
 
         public override async Task AddServerBanAsync(ServerBanDef serverBan)
@@ -171,6 +206,20 @@ namespace Content.Server.Database
             await db.PgDbContext.SaveChangesAsync();
         }
 
+        public override async Task AddServerUnbanAsync(ServerUnbanDef serverUnban)
+        {
+            await using var db = await GetDbImpl();
+
+            db.PgDbContext.Unban.Add(new PostgresServerUnban
+            {
+                 BanId = serverUnban.BanId,
+                 UnbanningAdmin = serverUnban.UnbanningAdmin?.UserId,
+                 UnbanTime = serverUnban.UnbanTime.UtcDateTime
+            });
+
+            await db.PgDbContext.SaveChangesAsync();
+        }
+
         public override async Task UpdatePlayerRecord(NetUserId userId, string userName, IPAddress address)
         {
             await using var db = await GetDbImpl();

Content.Server/Database/ServerDbSqlite.cs

@@ -46,6 +46,18 @@ namespace Content.Server.Database
             }
         }
 
+        public override async Task<ServerBanDef?> GetServerBanAsync(int id)
+        {
+            await using var db = await GetDbImpl();
+
+            var ban = await db.SqliteDbContext.Ban
+                .Include(p => p.Unban)
+                .Where(p => p.Id == id)
+                .SingleOrDefaultAsync();
+
+            return ConvertBan(ban);
+        }
+
         public override async Task<ServerBanDef?> GetServerBanAsync(IPAddress? address, NetUserId? userId)
         {
             await using var db = await GetDbImpl();
@@ -132,6 +144,20 @@ namespace Content.Server.Database
             await db.SqliteDbContext.SaveChangesAsync();
         }
 
+        public override async Task AddServerUnbanAsync(ServerUnbanDef serverUnban)
+        {
+            await using var db = await GetDbImpl();
+
+            db.SqliteDbContext.Unban.Add(new SqliteServerUnban
+            {
+                BanId = serverUnban.BanId,
+                UnbanningAdmin = serverUnban.UnbanningAdmin?.UserId,
+                UnbanTime = serverUnban.UnbanTime.UtcDateTime
+            });
+
+            await db.SqliteDbContext.SaveChangesAsync();
+        }
+
         public override async Task UpdatePlayerRecord(NetUserId userId, string userName, IPAddress address)
         {
             await using var db = await GetDbImpl();
@@ -218,6 +244,8 @@ namespace Content.Server.Database
                     int.Parse(ban.Address.AsSpan(idx + 1), provider: CultureInfo.InvariantCulture));
             }
 
+            var unban = ConvertUnban(ban.Unban);
+
             return new ServerBanDef(
                 ban.Id,
                 uid,
@@ -225,7 +253,27 @@ namespace Content.Server.Database
                 ban.BanTime,
                 ban.ExpirationTime,
                 ban.Reason,
-                aUid);
+                aUid,
+                unban);
+        }
+
+        private static ServerUnbanDef? ConvertUnban(SqliteServerUnban? unban)
+        {
+            if (unban == null)
+            {
+                return null;
+            }
+
+            NetUserId? aUid = null;
+            if (unban.UnbanningAdmin is {} aGuid)
+            {
+                aUid = new NetUserId(aGuid);
+            }
+
+            return new ServerUnbanDef(
+                unban.Id,
+                aUid,
+                unban.UnbanTime);
         }
 
         public override async Task AddConnectionLogAsync(NetUserId userId, string userName, IPAddress address)

Content.Server/Database/ServerUnbanDef.cs

@@ -0,0 +1,21 @@
+using System;
+using Robust.Shared.Network;
+
+namespace Content.Server.Database
+{
+    public sealed class ServerUnbanDef
+    {
+        public int BanId { get; }
+
+        public NetUserId? UnbanningAdmin { get; }
+
+        public DateTimeOffset UnbanTime { get; }
+
+        public ServerUnbanDef(int banId, NetUserId? unbanningAdmin, DateTimeOffset unbanTime)
+        {
+            BanId = banId;
+            UnbanningAdmin = unbanningAdmin;
+            UnbanTime = unbanTime;
+        }
+    }
+}