From 9dee0aee7d6b1a401c8855d60035cc8fc01615d9 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Fri, 1 Dec 2017 23:12:06 -0700
Subject: [PATCH 01/11] Rewrote the /ban add sub command

/ban add now replaces add, addtemp, and addip.
New syntax: /ban add <target> [time] [reason]

Examples:
- /ban add Shank 10d Rewrote the ban system.
- /ban add Ash
- /ban add 127.0.0.1 5d Go work on homework.

Note that if you want to specify a reason and a permaban, you need
to use 0 (zero) as the duration.

Examples:
- /ban add Ash 0 Love ya.
- /ban add Shank 0 Hacking.

Closes #1510
---
 TShockAPI/Commands.cs | 341 +++++++++++++++++++++---------------------
 1 file changed, 171 insertions(+), 170 deletions(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 493aa956..87559392 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -35,6 +35,7 @@ using Terraria.GameContent.Events;
 using Microsoft.Xna.Framework;
 using OTAPI.Tile;
 using TShockAPI.Localization;
+using System.Text.RegularExpressions;
 
 namespace TShockAPI
 {
@@ -1273,200 +1274,202 @@ namespace TShockAPI
 			switch (subcmd)
 			{
 				case "add":
-					#region Add ban
+					#region Add Ban
 					{
 						if (args.Parameters.Count < 2)
 						{
-							args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}ban add <player> [reason]", Specifier);
+							args.Player.SendErrorMessage("Invalid command. Format: {0}ban add <player> [time] [reason]", Specifier);
+							args.Player.SendErrorMessage("Example: {0}ban add Shank 10d Hacking and cheating", Specifier);
+							args.Player.SendErrorMessage("Example: {0}ban add Ash", Specifier);
+							args.Player.SendErrorMessage("Use the time 0 (zero) for a permanent ban.");
 							return;
 						}
 
+						// Used only to notify if a ban was successful and who the ban was about
+						bool success = false;
+						string targetGeneralizedName = "";
+
+						// What if caller is console? Name?
+
+						// Effective ban target assignment
 						List<TSPlayer> players = TShock.Utils.FindPlayer(args.Parameters[1]);
-						string reason = args.Parameters.Count > 2 ? String.Join(" ", args.Parameters.Skip(2)) : "Misbehavior.";
-						if (players.Count == 0)
+						User offlineUser = TShock.Users.GetUserByName(args.Parameters[1]);
+
+						// Determines if the caller is effective root for overriding permissions
+						bool callerIsEffectiveRoot = false;
+
+						// A ban source is effective root if they are not not real and not rest
+						// Super admins ingame who attempt to run this will have to remove
+						// the immune to ban permission if they want to ban an immune player
+						if (!(args.Player is TSRestPlayer) && !args.Player.RealPlayer)
 						{
-							var user = TShock.Users.GetUserByName(args.Parameters[1]);
-							if (user != null)
-							{
-								bool force = !args.Player.RealPlayer;
-
-								if (user.Name == args.Player.Name && !force)
-								{
-									args.Player.SendErrorMessage("You can't ban yourself!");
-									return;
-								}
-
-								if (TShock.Groups.GetGroupByName(user.Group).HasPermission(Permissions.immunetoban) && !force)
-									args.Player.SendErrorMessage("You can't ban {0}!", user.Name);
-								else
-								{
-									if (user.KnownIps == null)
-									{
-										args.Player.SendErrorMessage("Cannot ban {0} because they have no IPs to ban.", user.Name);
-										return;
-									}
-									var knownIps = JsonConvert.DeserializeObject<List<string>>(user.KnownIps);
-									TShock.Bans.AddBan(knownIps.Last(), user.Name, user.UUID, reason, false, args.Player.User.Name);
-									if (String.IsNullOrWhiteSpace(args.Player.User.Name))
-									{
-										if (args.Silent)
-										{
-											args.Player.SendInfoMessage("{0} was {1}banned for '{2}'.", user.Name, force ? "Force " : "", reason);
-										}
-										else
-										{
-											TSPlayer.All.SendInfoMessage("{0} was {1}banned for '{2}'.", user.Name, force ? "Force " : "", reason);
-										}
-									}
-									else
-									{
-										if (args.Silent)
-										{
-											args.Player.SendInfoMessage("{0}banned {1} for '{2}'.", force ? "Force " : "", user.Name, reason);
-										}
-										else
-										{
-											TSPlayer.All.SendInfoMessage("{0} {1}banned {2} for '{3}'.", args.Player.Name, force ? "Force " : "", user.Name, reason);
-										}
-									}
-								}
-							}
-							else
-								args.Player.SendErrorMessage("Invalid player or account!");
+							callerIsEffectiveRoot = true;
 						}
-						else if (players.Count > 1)
+
+						// The ban reason the ban is going to have
+						string banReason = "Unknown.";
+
+						// The default ban length
+						// 0 is permanent ban, otherwise temp ban
+						int banLengthInSeconds = 0;
+
+						// Figure out if param 2 is a time or 0 or garbage
+						if (args.Parameters.Count >= 3)
+						{
+							bool parsedOkay = false;
+							if (!(args.Parameters[2] == "0"))
+							{
+								parsedOkay = TShock.Utils.TryParseTime(args.Parameters[2], out banLengthInSeconds);
+							} else {
+								parsedOkay = true;
+							}
+
+							if (!parsedOkay)
+							{
+								args.Player.SendErrorMessage("Invalid time format. Example: 10d+5h+3m-2s.");
+								args.Player.SendErrorMessage("Use 0 (zero) for a permanent ban.");
+								return;
+							}
+						}
+
+						// If a reason exists, use the given reason.
+						if (args.Parameters.Count > 3)
+						{
+							banReason = String.Join(" ", args.Parameters.Skip(3));
+						}
+
+						// Bad case: Players contains more than 1 person so we can't ban them
+						if (players.Count > 1)
+						{
 							TShock.Utils.SendMultipleMatchError(args.Player, players.Select(p => p.Name));
-						else
-						{
-							if (!TShock.Utils.Ban(players[0], reason, !args.Player.RealPlayer, args.Player.User.Name))
-								args.Player.SendErrorMessage("You can't ban {0}!", players[0].Name);
-						}
-					}
-					#endregion
-					return;
-				case "addip":
-					#region Add IP ban
-					{
-						if (args.Parameters.Count < 2)
-						{
-							args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}ban addip <ip> [reason]", Specifier);
 							return;
 						}
 
-						string ip = args.Parameters[1];
-						string reason = args.Parameters.Count > 2
-											? String.Join(" ", args.Parameters.GetRange(2, args.Parameters.Count - 2))
-											: "Manually added IP address ban.";
-						TShock.Bans.AddBan(ip, "", "", reason, false, args.Player.User.Name);
-						args.Player.SendSuccessMessage("Banned IP {0}.", ip);
-					}
-					#endregion
-					return;
-				case "addtemp":
-					#region Add temp ban
-					{
-						if (args.Parameters.Count < 3)
+						// Good case: Online ban for matching character.
+						if (players.Count == 1)
 						{
-							args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}ban addtemp <player> <time> [reason]", Specifier);
-							return;
-						}
+							TSPlayer target = players[0];
 
-						int time;
-						if (!TShock.Utils.TryParseTime(args.Parameters[2], out time))
-						{
-							args.Player.SendErrorMessage("Invalid time string! Proper format: _d_h_m_s, with at least one time specifier.");
-							args.Player.SendErrorMessage("For example, 1d and 10h-30m+2m are both valid time strings, but 2 is not.");
-							return;
-						}
-
-						string reason = args.Parameters.Count > 3
-											? String.Join(" ", args.Parameters.Skip(3))
-											: "Misbehavior.";
-
-						List<TSPlayer> players = TShock.Utils.FindPlayer(args.Parameters[1]);
-						if (players.Count == 0)
-						{
-							var user = TShock.Users.GetUserByName(args.Parameters[1]);
-							if (user != null)
+							if (target.HasPermission(Permissions.immunetoban) && !callerIsEffectiveRoot)
 							{
-								bool force = !args.Player.RealPlayer;
-								if (TShock.Groups.GetGroupByName(user.Group).HasPermission(Permissions.immunetoban) && !force)
-									args.Player.SendErrorMessage("You can't ban {0}!", user.Name);
-								else
-								{
-									var knownIps = JsonConvert.DeserializeObject<List<string>>(user.KnownIps);
-									TShock.Bans.AddBan(knownIps.Last(), user.Name, user.UUID, reason, false, args.Player.User.Name, DateTime.UtcNow.AddSeconds(time).ToString("s"));
-									if (String.IsNullOrWhiteSpace(args.Player.User.Name))
-									{
-										if (args.Silent)
-										{
-											args.Player.SendInfoMessage("{0} was {1}banned for '{2}'.", user.Name, force ? "force " : "", reason);
-										}
-										else
-										{
-											TSPlayer.All.SendInfoMessage("{0} was {1}banned for '{2}'.", user.Name, force ? "force " : "", reason);
-										}
-									}
-									else
-									{
-										if (args.Silent)
-										{
-											args.Player.SendInfoMessage("{0} was {1}banned for '{2}'.", user.Name, force ? "force " : "", reason);
-										}
-										else
-										{
-											TSPlayer.All.SendInfoMessage("{0} {1}banned {2} for '{3}'.", args.Player.Name, force ? "force " : "", user.Name, reason);
-										}
-									}
-								}
-							}
-							else
-							{
-								args.Player.SendErrorMessage("Invalid player or account!");
-							}
-						}
-						else if (players.Count > 1)
-							TShock.Utils.SendMultipleMatchError(args.Player, players.Select(p => p.Name));
-						else
-						{
-							if (args.Player.RealPlayer && players[0].HasPermission(Permissions.immunetoban))
-							{
-								args.Player.SendErrorMessage("You can't ban {0}!", players[0].Name);
+								args.Player.SendErrorMessage("Permission denied. Target {0} is immune to ban.", target.Name);
 								return;
 							}
 
-							if (TShock.Bans.AddBan(players[0].IP, players[0].Name, players[0].UUID, reason,
-								false, args.Player.Name, DateTime.UtcNow.AddSeconds(time).ToString("s")))
+							targetGeneralizedName = target.Name;
+							success = TShock.Bans.AddBan(target.IP, target.Name, target.UUID, banReason, false, args.Player.User.Name,
+								banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+
+							// Since this is an online ban, we need to dc the player and tell them now.
+							if (success)
 							{
-								players[0].Disconnect(String.Format("Banned: {0}", reason));
-								string verb = args.Player.RealPlayer ? "Force " : "";
-								if (args.Player.RealPlayer)
-									if (args.Silent)
-									{
-										args.Player.SendSuccessMessage("{0}banned {1} for '{2}'", verb, players[0].Name, reason);
-									}
-									else
-									{
-										TSPlayer.All.SendSuccessMessage("{0} {1}banned {2} for '{3}'", args.Player.Name, verb, players[0].Name, reason);
-									}
+								if (banLengthInSeconds == 0)
+								{
+									target.Disconnect(String.Format("Permanently banned for {0}", banReason));
+								}
 								else
 								{
-									if (args.Silent)
-									{
-										args.Player.SendSuccessMessage("{0}banned {1} for '{2}'", verb, players[0].Name, reason);
-									}
-									else
-									{
-										TSPlayer.All.SendSuccessMessage("{0} was {1}banned for '{2}'", players[0].Name, verb, reason);
-									}
+									target.Disconnect(String.Format("Banned for {0} seconds for {1}", banLengthInSeconds, banReason));
 								}
 							}
-							else
-								args.Player.SendErrorMessage("Failed to ban {0}, check logs.", players[0].Name);
 						}
+
+						// Case: Players & user are invalid, could be IP?
+						// Note: Order matters. If this method is above the online player check,
+						// This enables you to ban an IP even if the player exists in the database as a player.
+						// You'll get two bans for the price of one, in theory, because both IP and user named IP will be banned.
+						// ??? edge cases are weird, but this is going to happen
+						// The only way around this is to either segregate off the IP code or do something else.
+						if (players.Count == 0)
+						{
+							// If the target is a valid IP...
+							string pattern = @"^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$";
+							Regex r = new Regex(pattern, RegexOptions.IgnoreCase);
+							if (r.IsMatch(args.Parameters[1])) {
+								targetGeneralizedName = String.Join(" ", "IP:", args.Parameters[1]);
+								success = TShock.Bans.AddBan(args.Parameters[1], "", "", banReason,
+									false, args.Player.User.Name, banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+								if (success && offlineUser != null)
+								{
+									args.Player.SendSuccessMessage("Target IP {0} was banned successfully.", targetGeneralizedName);
+									args.Player.SendErrorMessage("Note: An account named with this IP address also exists.");
+									args.Player.SendErrorMessage("Note: It will also be banned.");
+								}
+							} else {
+								// Apparently there is no way to not IP ban someone
+								// This means that where we would normally just ban a "character name" here
+								// We can't because it requires some IP as a primary key.
+								if (offlineUser == null)
+								{
+									args.Player.SendErrorMessage("Unable to ban target {0}.", args.Parameters[1]);
+									args.Player.SendErrorMessage("Target is not a valid IP address, a valid online player, or a known offline user.");
+									return;
+								}
+							}
+
+						}
+
+						// Case: Offline ban
+						if (players.Count == 0 && offlineUser != null)
+						{
+							// Catch: we don't know an offline player's last login character name
+							// This means that we're banning their *user name* on the assumption that
+							// user name == character name
+							// (which may not be true)
+							// This needs to be fixed in a future implementation.
+							targetGeneralizedName = offlineUser.Name;
+
+							if (TShock.Groups.GetGroupByName(offlineUser.Group).HasPermission(Permissions.immunetoban) && 
+								!callerIsEffectiveRoot)
+							{
+								args.Player.SendErrorMessage("Permission denied. Target {0} is immune to ban.", targetGeneralizedName);
+								return;
+							}
+
+							if (offlineUser.KnownIps == null)
+							{
+								args.Player.SendErrorMessage("Unable to ban target {0} because they have no valid IP to ban.", targetGeneralizedName);
+								return;
+							}
+
+							string lastIP = JsonConvert.DeserializeObject<List<string>>(offlineUser.KnownIps).Last();
+
+							success = 
+								TShock.Bans.AddBan(lastIP,
+									offlineUser.Name, offlineUser.UUID, banReason, false, args.Player.User.Name,
+									banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+						}
+
+
+						if (success)
+						{
+							args.Player.SendSuccessMessage("{0} was successfully banned.", targetGeneralizedName);
+							args.Player.SendInfoMessage("Length: {0}", banLengthInSeconds == 0 ? "Permanent." : banLengthInSeconds + " seconds.");
+							args.Player.SendInfoMessage("Reason: {0}", banReason);
+							if (!args.Silent)
+							{
+								if (banLengthInSeconds == 0)
+								{
+									TSPlayer.All.SendErrorMessage("{0} was permanently banned by {1} for: {2}",
+										targetGeneralizedName, args.Player.User.Name, banReason);
+								}
+								else
+								{
+									TSPlayer.All.SendErrorMessage("{0} was temp banned for {1} seconds by {2} for: {3}",
+										targetGeneralizedName, banLengthInSeconds, args.Player.User.Name, banReason);
+								}
+							}
+						}
+						else
+						{
+							args.Player.SendErrorMessage("{0} was NOT banned due to a database error or other system problem.", targetGeneralizedName);
+							args.Player.SendErrorMessage("If this player is online, they have NOT been kicked.");
+							args.Player.SendErrorMessage("Check the system logs for details.");
+						}
+
+						return;
 					}
 					#endregion
-					return;
 				case "del":
 					#region Delete ban
 					{
@@ -1522,9 +1525,7 @@ namespace TShockAPI
 
 						var lines = new List<string>
 						{
-							"add <player> [reason] - Bans a player or user account if the player is not online.",
-							"addip <ip> [reason] - Bans an IP.",
-							"addtemp <player> <time> [reason] - Temporarily bans a player.",
+							"add <target> <time> [reason] - Bans a player or user account if the player is not online.",
 							"del <player> - Unbans a player.",
 							"delip <ip> - Unbans an IP.",
 							"list [page] - Lists all player bans.",

From d10c8d65f61763ee3d0f511cebc60deaa2110238 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Fri, 1 Dec 2017 23:20:34 -0700
Subject: [PATCH 02/11] Removed minor comment

Turns out that ServerConsole gets a name if caller is unknown.
---
 TShockAPI/Commands.cs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 87559392..67b0643c 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1289,8 +1289,6 @@ namespace TShockAPI
 						bool success = false;
 						string targetGeneralizedName = "";
 
-						// What if caller is console? Name?
-
 						// Effective ban target assignment
 						List<TSPlayer> players = TShock.Utils.FindPlayer(args.Parameters[1]);
 						User offlineUser = TShock.Users.GetUserByName(args.Parameters[1]);

From ec2a5425649463152c8743c9c6900bd0b5ba53f3 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Fri, 1 Dec 2017 23:27:51 -0700
Subject: [PATCH 03/11] Remove silly string join since snoons is right

> String.Join(" ", str1, str2) is far less clear than str1 + " " + str2
---
 TShockAPI/Commands.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 67b0643c..26ed7fc5 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1384,7 +1384,7 @@ namespace TShockAPI
 							string pattern = @"^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$";
 							Regex r = new Regex(pattern, RegexOptions.IgnoreCase);
 							if (r.IsMatch(args.Parameters[1])) {
-								targetGeneralizedName = String.Join(" ", "IP:", args.Parameters[1]);
+								targetGeneralizedName = "IP: " + args.Parameters[1];
 								success = TShock.Bans.AddBan(args.Parameters[1], "", "", banReason,
 									false, args.Player.User.Name, banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
 								if (success && offlineUser != null)

From 6b2c9c47c3b5cff8674cfbfdb8893fa8b6f8cc3f Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 17:38:43 -0700
Subject: [PATCH 04/11] Update chagnelog with ban stuff

---
 CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ecbd7138..534e143b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,7 +14,8 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin
 * Added a warning notifying users of the minimum memory required to run TShock (@bartico6)
 * Added /group rename to allow changing group names (@ColinBohn, @ProfessorXZ)
 * Added /region rename and OnRegionRenamed hook (@koneko-nyan, @deadsurgeon42)
-
+* Rebuilt /ban add. New syntax is /ban add <target> [time] [reason] where target is the target online player, offline player, or IP; where time is the time format or 0 for permanent; and where [reason] is the reason. (@hakusaro)
+* Removed /ban addip and /ban addtemp. Now covered under /ban add. (@hakusaro)
 
 ## TShock 4.3.24
 * Updated OpenTerraria API to 1.3.5.3 (@DeathCradle)

From 0edfc6834f8f5c329d50e2bf7a7624dd3ee01e88 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 19:19:17 -0700
Subject: [PATCH 05/11] Add technical infrastructure for adding account bans

---
 TShockAPI/DB/BanManager.cs | 63 ++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/TShockAPI/DB/BanManager.cs b/TShockAPI/DB/BanManager.cs
index 8bfe9d86..8fa40375 100644
--- a/TShockAPI/DB/BanManager.cs
+++ b/TShockAPI/DB/BanManager.cs
@@ -43,6 +43,7 @@ namespace TShockAPI.DB
 									new SqlColumn("IP", MySqlDbType.String, 16) { Primary = true },
 									new SqlColumn("Name", MySqlDbType.Text),
 									new SqlColumn("UUID", MySqlDbType.Text),
+									new SqlColumn("AccountName", MySqlDbType.Text),
 									new SqlColumn("Reason", MySqlDbType.Text),
 									new SqlColumn("BanningUser", MySqlDbType.Text),
 									new SqlColumn("Date", MySqlDbType.Text),
@@ -75,7 +76,7 @@ namespace TShockAPI.DB
 				using (var reader = database.QueryReader("SELECT * FROM Bans WHERE IP=@0", ip))
 				{
 					if (reader.Read())
-						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
+						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("AccountName"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
 				}
 			}
 			catch (Exception ex)
@@ -104,7 +105,7 @@ namespace TShockAPI.DB
 				{
 					while (reader.Read())
 					{
-						banlist.Add(new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration")));
+						banlist.Add(new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("AccountName"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration")));
 					}
 
 					banlist.Sort(new BanComparer(sortMethod));
@@ -135,7 +136,7 @@ namespace TShockAPI.DB
 				using (var reader = database.QueryReader("SELECT * FROM Bans WHERE " + namecol + "=@0", name))
 				{
 					if (reader.Read())
-						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
+						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("AccountName"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
 				}
 			}
 			catch (Exception ex)
@@ -157,7 +158,7 @@ namespace TShockAPI.DB
 				using (var reader = database.QueryReader("SELECT * FROM Bans WHERE UUID=@0", uuid))
 				{
 					if (reader.Read())
-						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
+						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("AccountName"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
 				}
 			}
 			catch (Exception ex)
@@ -179,6 +180,22 @@ namespace TShockAPI.DB
 		/// <param name="banner">Banner.</param>
 		/// <param name="expiration">Expiration date.</param>
 		public bool AddBan(string ip, string name = "", string uuid = "", string reason = "", bool exceptions = false, string banner = "", string expiration = "")
+		{
+			return AddBan2(ip, name, uuid, "", reason, exceptions, banner, expiration);
+		}
+
+		/// <summary>
+		/// Adds a ban.
+		/// </summary>
+		/// <returns><c>true</c>, if ban was added, <c>false</c> otherwise.</returns>
+		/// <param name="ip">Ip.</param>
+		/// <param name="name">Name.</param>
+		/// <param name="uuid">UUID.</param>
+		/// <param name="reason">Reason.</param>
+		/// <param name="exceptions">If set to <c>true</c> enable throwing exceptions.</param>
+		/// <param name="banner">Banner.</param>
+		/// <param name="expiration">Expiration date.</param>
+		public bool AddBan2(string ip, string name = "", string uuid = "", string accountName = "", string reason = "", bool exceptions = false, string banner = "", string expiration = "")
 		{
 			try
 			{
@@ -192,7 +209,7 @@ namespace TShockAPI.DB
 				}
 				else
 				{
-					return database.Query("INSERT INTO Bans (IP, Name, UUID, Reason, BanningUser, Date, Expiration) VALUES (@0, @1, @2, @3, @4, @5, @6);", ip, name, uuid, reason, banner, DateTime.UtcNow.ToString("s"), expiration) != 0;
+					return database.Query("INSERT INTO Bans (IP, Name, UUID, AccountName, Reason, BanningUser, Date, Expiration) VALUES (@0, @1, @2, @3, @4, @5, @6, @7);", ip, name, uuid, accountName, reason, banner, DateTime.UtcNow.ToString("s"), expiration) != 0;
 				}
 			}
 			catch (Exception ex)
@@ -231,6 +248,26 @@ namespace TShockAPI.DB
 			return false;
 		}
 
+		/// <summary>
+		/// Removes a ban by account name (not character/player name).
+		/// </summary>
+		/// <returns><c>true</c>, if ban was removed, <c>false</c> otherwise.</returns>
+		/// <param name="match">Match.</param>
+		/// <param name="casesensitive">If set to <c>true</c> casesensitive.</param>
+		public bool RemoveBanByAccountName(string match, bool casesensitive = true)
+		{
+			try
+			{
+				var namecol = casesensitive ? "AccountName" : "UPPER(AccountName)";
+				return database.Query("DELETE FROM Bans WHERE " + namecol + "=@0", casesensitive ? match : match.ToUpper()) != 0;
+			}
+			catch (Exception ex)
+			{
+				TShock.Log.Error(ex.ToString());
+			}
+			return false;
+		}
+
 		/// <summary>
 		/// Clears bans.
 		/// </summary>
@@ -380,6 +417,12 @@ namespace TShockAPI.DB
 		/// <value>The UUID</value>
 		public string UUID { get; set; }
 
+		/// <summary>
+		/// Gets or sets the account name of the ban
+		/// </summary>
+		/// <value>The account name</value>
+		public String AccountName { get; set; }
+
 		/// <summary>
 		/// Gets or sets the ban reason.
 		/// </summary>
@@ -424,11 +467,12 @@ namespace TShockAPI.DB
 		/// <param name="banner">Banner.</param>
 		/// <param name="date">UTC ban date.</param>
 		/// <param name="exp">Expiration time</param>
-		public Ban(string ip, string name, string uuid, string reason, string banner, string date, string exp)
+		public Ban(string ip, string name, string uuid, string accountName, string reason, string banner, string date, string exp)
 		{
 			IP = ip;
 			Name = name;
 			UUID = uuid;
+			AccountName = accountName;
 			Reason = reason;
 			BanningUser = banner;
 			Date = date;
@@ -454,10 +498,11 @@ namespace TShockAPI.DB
 			IP = string.Empty;
 			Name = string.Empty;
 			UUID = string.Empty;
+			AccountName = string.Empty;
 			Reason = string.Empty;
-			BanningUser = "";
-			Date = "";
-			Expiration = "";
+			BanningUser = string.Empty;
+			Date = string.Empty;
+			Expiration = string.Empty;
 		}
 	}
 }

From 718525904a43bc50137e25ee445404301d74fe26 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 20:41:14 -0700
Subject: [PATCH 06/11] Add remaining backbone for banning accounts (sort of)

Frontend still doesn't work properly. In particular:

1. Need a way to list bans by account name.
2. Need a way to unban by account name.
3. Really need a way to change the IP on a ban lol.

Ban system still needs to be be rebuilt fully, but at least this
doesn't necessarily character ban someone if you were going for an IP
ban.

Fixes #1412
---
 TShockAPI/Commands.cs         |  8 ++++----
 TShockAPI/DB/BanManager.cs    | 33 ++++++++++++++++++++++++++++++---
 TShockAPI/Rest/RestManager.cs |  4 ++--
 TShockAPI/TShock.cs           | 24 +++++++++++++++++++++++-
 TShockAPI/Utils.cs            |  2 +-
 5 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 630927e3..f11e3326 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1367,7 +1367,7 @@ namespace TShockAPI
 							}
 
 							targetGeneralizedName = target.Name;
-							success = TShock.Bans.AddBan(target.IP, target.Name, target.UUID, banReason, false, args.Player.User.Name,
+							success = TShock.Bans.AddBan2(target.IP, target.Name, target.UUID, target.User.Name, banReason, false, args.Player.User.Name,
 								banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
 
 							// Since this is an online ban, we need to dc the player and tell them now.
@@ -1397,7 +1397,7 @@ namespace TShockAPI
 							Regex r = new Regex(pattern, RegexOptions.IgnoreCase);
 							if (r.IsMatch(args.Parameters[1])) {
 								targetGeneralizedName = "IP: " + args.Parameters[1];
-								success = TShock.Bans.AddBan(args.Parameters[1], "", "", banReason,
+								success = TShock.Bans.AddBan2(args.Parameters[1], "", "", "", banReason,
 									false, args.Player.User.Name, banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
 								if (success && offlineUser != null)
 								{
@@ -1445,8 +1445,8 @@ namespace TShockAPI
 							string lastIP = JsonConvert.DeserializeObject<List<string>>(offlineUser.KnownIps).Last();
 
 							success = 
-								TShock.Bans.AddBan(lastIP,
-									offlineUser.Name, offlineUser.UUID, banReason, false, args.Player.User.Name,
+								TShock.Bans.AddBan2(lastIP,
+									"", offlineUser.UUID, offlineUser.Name, banReason, false, args.Player.User.Name,
 									banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
 						}
 
diff --git a/TShockAPI/DB/BanManager.cs b/TShockAPI/DB/BanManager.cs
index 8fa40375..7ee0fa29 100644
--- a/TShockAPI/DB/BanManager.cs
+++ b/TShockAPI/DB/BanManager.cs
@@ -43,11 +43,11 @@ namespace TShockAPI.DB
 									new SqlColumn("IP", MySqlDbType.String, 16) { Primary = true },
 									new SqlColumn("Name", MySqlDbType.Text),
 									new SqlColumn("UUID", MySqlDbType.Text),
-									new SqlColumn("AccountName", MySqlDbType.Text),
 									new SqlColumn("Reason", MySqlDbType.Text),
 									new SqlColumn("BanningUser", MySqlDbType.Text),
 									new SqlColumn("Date", MySqlDbType.Text),
-									new SqlColumn("Expiration", MySqlDbType.Text)
+									new SqlColumn("Expiration", MySqlDbType.Text),
+									new SqlColumn("AccountName", MySqlDbType.Text)
 				);
 			var creator = new SqlTableCreator(db,
 				db.GetSqlType() == SqlType.Sqlite
@@ -146,6 +146,32 @@ namespace TShockAPI.DB
 			return null;
 		}
 
+		/// <summary>
+		/// Gets a ban by account name (not player/character name).
+		/// </summary>
+		/// <param name="name">The name.</param>
+		/// <param name="casesensitive">Whether to check with case sensitivity.</param>
+		/// <returns>The ban.</returns>
+		public Ban GetBanByAccountName(string name, bool casesensitive = false)
+		{
+			try
+			{
+				var namecol = casesensitive ? "AccountName" : "UPPER(AccountName)";
+				if (!casesensitive)
+					name = name.ToUpper();
+				using (var reader = database.QueryReader("SELECT * FROM Bans WHERE " + namecol + "=@0", name))
+				{
+					if (reader.Read())
+						return new Ban(reader.Get<string>("IP"), reader.Get<string>("Name"), reader.Get<string>("UUID"), reader.Get<string>("AccountName"), reader.Get<string>("Reason"), reader.Get<string>("BanningUser"), reader.Get<string>("Date"), reader.Get<string>("Expiration"));
+				}
+			}
+			catch (Exception ex)
+			{
+				TShock.Log.Error(ex.ToString());
+			}
+			return null;
+		}
+
 		/// <summary>
 		/// Gets a ban by UUID.
 		/// </summary>
@@ -179,6 +205,7 @@ namespace TShockAPI.DB
 		/// <param name="exceptions">If set to <c>true</c> enable throwing exceptions.</param>
 		/// <param name="banner">Banner.</param>
 		/// <param name="expiration">Expiration date.</param>
+		[Obsolete("Use AddBan2 instead of AddBan.", true)]
 		public bool AddBan(string ip, string name = "", string uuid = "", string reason = "", bool exceptions = false, string banner = "", string expiration = "")
 		{
 			return AddBan2(ip, name, uuid, "", reason, exceptions, banner, expiration);
@@ -209,7 +236,7 @@ namespace TShockAPI.DB
 				}
 				else
 				{
-					return database.Query("INSERT INTO Bans (IP, Name, UUID, AccountName, Reason, BanningUser, Date, Expiration) VALUES (@0, @1, @2, @3, @4, @5, @6, @7);", ip, name, uuid, accountName, reason, banner, DateTime.UtcNow.ToString("s"), expiration) != 0;
+					return database.Query("INSERT INTO Bans (IP, Name, UUID, Reason, BanningUser, Date, Expiration, AccountName) VALUES (@0, @1, @2, @3, @4, @5, @6, @7);", ip, name, uuid, reason, banner, DateTime.UtcNow.ToString("s"), expiration, accountName) != 0;
 				}
 			}
 			catch (Exception ex)
diff --git a/TShockAPI/Rest/RestManager.cs b/TShockAPI/Rest/RestManager.cs
index bd10de2d..9c2c818a 100644
--- a/TShockAPI/Rest/RestManager.cs
+++ b/TShockAPI/Rest/RestManager.cs
@@ -645,7 +645,7 @@ namespace TShockAPI
 
 			try
 			{
-				TShock.Bans.AddBan(ip, name, "", args.Parameters["reason"], true, args.TokenData.Username);
+				TShock.Bans.AddBan2(ip, name, "", "", args.Parameters["reason"], true, args.TokenData.Username);
 			}
 			catch (Exception e)
 			{
@@ -1022,7 +1022,7 @@ namespace TShockAPI
 
 			TSPlayer player = (TSPlayer)ret;
 			var reason = null == args.Parameters["reason"] ? "Banned via web" : args.Parameters["reason"];
-			TShock.Bans.AddBan(player.IP, player.Name, "", reason);
+			TShock.Bans.AddBan2(player.IP, player.Name, "", "", reason);
 			TShock.Utils.ForceKick(player, reason, false, true);
 			return RestResponse("Player " + player.Name + " was banned");
 		}
diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs
index 7c2d1ab8..d07a466c 100644
--- a/TShockAPI/TShock.cs
+++ b/TShockAPI/TShock.cs
@@ -472,6 +472,28 @@ namespace TShockAPI
 
 			args.Player.User.KnownIps = JsonConvert.SerializeObject(KnownIps, Formatting.Indented);
 			Users.UpdateLogin(args.Player.User);
+
+			Ban potentialBan = Bans.GetBanByAccountName(args.Player.User.Name);
+
+			if (potentialBan != null)
+			{
+				// A user just signed in successfully despite being banned by account name.
+				// We should fix the ban database so that all of their ban info is up to date.
+				Bans.AddBan2(args.Player.IP, args.Player.Name, args.Player.UUID, args.Player.User.Name,
+					potentialBan.Reason, false, potentialBan.BanningUser, potentialBan.Expiration);
+
+				// And then get rid of them.
+				if (potentialBan.Expiration == "")
+				{
+					Utils.ForceKick(args.Player, String.Format("Permanently banned by {0} for {1}", potentialBan.BanningUser
+						,potentialBan.Reason), false, false);
+				}
+				else
+				{
+					Utils.ForceKick(args.Player, String.Format("Still banned by {0} for {1}", potentialBan.BanningUser,
+						potentialBan.Reason), false, false);
+				}
+			}
 		}
 
 		/// <summary>OnAccountDelete - Internal hook fired on account delete.</summary>
@@ -488,7 +510,7 @@ namespace TShockAPI
 			CharacterDB.SeedInitialData(Users.GetUser(args.User));
 		}
 
-		/// <summary>OnPlayerPreLogin - Internal hook fired when on player pre login.</summary>
+		/// <summary>OnPlayerPreLoginOnPlayerPreLogin - Internal hook fired when on player pre login.</summary>
 		/// <param name="args">args - The PlayerPreLoginEventArgs object.</param>
 		private void OnPlayerPreLogin(Hooks.PlayerPreLoginEventArgs args)
 		{
diff --git a/TShockAPI/Utils.cs b/TShockAPI/Utils.cs
index 5081f8e4..9efa8b99 100644
--- a/TShockAPI/Utils.cs
+++ b/TShockAPI/Utils.cs
@@ -675,7 +675,7 @@ namespace TShockAPI
 				string ip = player.IP;
 				string uuid = player.UUID;
 				string playerName = player.Name;
-				TShock.Bans.AddBan(ip, playerName, uuid, reason, false, adminUserName);
+				TShock.Bans.AddBan2(ip, playerName, uuid, "", reason, false, adminUserName);
 				player.Disconnect(string.Format("Banned: {0}", reason));
 				string verb = force ? "force " : "";
 				if (string.IsNullOrWhiteSpace(adminUserName))

From 7160e758b2fc485174c0c7037baf4440813ba9a4 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 20:44:17 -0700
Subject: [PATCH 07/11] Unbreak comment OnPlayerPreLoginOnPlayerPreLogin

---
 TShockAPI/TShock.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs
index d07a466c..f808ce44 100644
--- a/TShockAPI/TShock.cs
+++ b/TShockAPI/TShock.cs
@@ -510,7 +510,7 @@ namespace TShockAPI
 			CharacterDB.SeedInitialData(Users.GetUser(args.User));
 		}
 
-		/// <summary>OnPlayerPreLoginOnPlayerPreLogin - Internal hook fired when on player pre login.</summary>
+		/// <summary>OnPlayerPreLogin - Internal hook fired when on player pre login.</summary>
 		/// <param name="args">args - The PlayerPreLoginEventArgs object.</param>
 		private void OnPlayerPreLogin(Hooks.PlayerPreLoginEventArgs args)
 		{

From 35c61e53f1e9d85623eca283f1c844e9beaee5a7 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 20:45:55 -0700
Subject: [PATCH 08/11] Remove some space left around

---
 TShockAPI/Commands.cs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index f11e3326..461af221 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1450,7 +1450,6 @@ namespace TShockAPI
 									banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
 						}
 
-
 						if (success)
 						{
 							args.Player.SendSuccessMessage("{0} was successfully banned.", targetGeneralizedName);

From 39733a116d8b21e1c3e79643dd99c25445daf569 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sat, 2 Dec 2017 20:59:30 -0700
Subject: [PATCH 09/11] Fix issue where temp bans were stored incorrectly

Bans were being stored as seconds, should be a DateTime in s format
with seconds added on to existing time.
---
 TShockAPI/Commands.cs | 6 +++---
 TShockAPI/TShock.cs   | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 461af221..8abcbedd 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1368,7 +1368,7 @@ namespace TShockAPI
 
 							targetGeneralizedName = target.Name;
 							success = TShock.Bans.AddBan2(target.IP, target.Name, target.UUID, target.User.Name, banReason, false, args.Player.User.Name,
-								banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+								banLengthInSeconds == 0 ? "" : DateTime.UtcNow.AddSeconds(banLengthInSeconds).ToString("s"));
 
 							// Since this is an online ban, we need to dc the player and tell them now.
 							if (success)
@@ -1398,7 +1398,7 @@ namespace TShockAPI
 							if (r.IsMatch(args.Parameters[1])) {
 								targetGeneralizedName = "IP: " + args.Parameters[1];
 								success = TShock.Bans.AddBan2(args.Parameters[1], "", "", "", banReason,
-									false, args.Player.User.Name, banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+									false, args.Player.User.Name, banLengthInSeconds == 0 ? "" : DateTime.UtcNow.AddSeconds(banLengthInSeconds).ToString("s"));
 								if (success && offlineUser != null)
 								{
 									args.Player.SendSuccessMessage("Target IP {0} was banned successfully.", targetGeneralizedName);
@@ -1447,7 +1447,7 @@ namespace TShockAPI
 							success = 
 								TShock.Bans.AddBan2(lastIP,
 									"", offlineUser.UUID, offlineUser.Name, banReason, false, args.Player.User.Name,
-									banLengthInSeconds == 0 ? "" : banLengthInSeconds.ToString());
+									banLengthInSeconds == 0 ? "" : DateTime.UtcNow.AddSeconds(banLengthInSeconds).ToString("s"));
 						}
 
 						if (success)
diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs
index f808ce44..e4fdf0ff 100644
--- a/TShockAPI/TShock.cs
+++ b/TShockAPI/TShock.cs
@@ -1387,7 +1387,7 @@ namespace TShockAPI
 					DateTime exp;
 					if (!DateTime.TryParse(ban.Expiration, out exp))
 					{
-						player.Disconnect("You are banned forever: " + ban.Reason);
+						player.Disconnect("Permanently banned for: " + ban.Reason);
 					}
 					else
 					{

From 3204ef2a83bfd14a336b6593cc2f7a8363245007 Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sun, 3 Dec 2017 09:49:45 -0700
Subject: [PATCH 10/11] callerIsEffectiveRoot -> callerIsServerConsole in ban
 system

This code clarity improvement helps make the code base clearer.

Thanks @deadsurgeon42
---
 TShockAPI/Commands.cs | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs
index 8abcbedd..2b2a9b73 100644
--- a/TShockAPI/Commands.cs
+++ b/TShockAPI/Commands.cs
@@ -1305,15 +1305,13 @@ namespace TShockAPI
 						List<TSPlayer> players = TShock.Utils.FindPlayer(args.Parameters[1]);
 						User offlineUser = TShock.Users.GetUserByName(args.Parameters[1]);
 
-						// Determines if the caller is effective root for overriding permissions
-						bool callerIsEffectiveRoot = false;
-
-						// A ban source is effective root if they are not not real and not rest
-						// Super admins ingame who attempt to run this will have to remove
-						// the immune to ban permission if they want to ban an immune player
-						if (!(args.Player is TSRestPlayer) && !args.Player.RealPlayer)
+						// Storage variable to determine if the command executor is the server console
+						// If it is, we assume they have full control and let them override permission checks
+						bool callerIsServerConsole = false;
+						
+						if (args.Player is TSServerPlayer)
 						{
-							callerIsEffectiveRoot = true;
+							callerIsServerConsole = true;
 						}
 
 						// The ban reason the ban is going to have
@@ -1360,7 +1358,7 @@ namespace TShockAPI
 						{
 							TSPlayer target = players[0];
 
-							if (target.HasPermission(Permissions.immunetoban) && !callerIsEffectiveRoot)
+							if (target.HasPermission(Permissions.immunetoban) && !callerIsServerConsole)
 							{
 								args.Player.SendErrorMessage("Permission denied. Target {0} is immune to ban.", target.Name);
 								return;
@@ -1430,7 +1428,7 @@ namespace TShockAPI
 							targetGeneralizedName = offlineUser.Name;
 
 							if (TShock.Groups.GetGroupByName(offlineUser.Group).HasPermission(Permissions.immunetoban) && 
-								!callerIsEffectiveRoot)
+								!callerIsServerConsole)
 							{
 								args.Player.SendErrorMessage("Permission denied. Target {0} is immune to ban.", targetGeneralizedName);
 								return;

From ec59549f2f78598e7d5d39ca8e2690c7cc48a24a Mon Sep 17 00:00:00 2001
From: Lucas Nicodemus <shank@shanked.me>
Date: Sun, 3 Dec 2017 15:55:40 -0700
Subject: [PATCH 11/11] Update changelog to add note about account bans

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 05ff1fe8..14a0931d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin
 * Removed /ban addip and /ban addtemp. Now covered under /ban add. (@hakusaro)
 * Added /su, which temporarily elevates players with the tshock.su permission to super admin. In addition added, a new group, owner, that is suggested for new users to setup TShock with as opposed to superadmin. Finally, /su is implemented such that a 10 minute timeout will occur preventing people from just camping with it on. (@hakusaro)
 * Added /sudo, which runs a command as the superadmin group. If a user fails to execute a command but can sudo, they'll be told that they can override the permission check with sudo. Much better than just telling them to run /su and then re-run the command. (@hakusaro)
+* Added the ability to ban by account name instead of just banning a character name assuming its an account name. (@hakusaro)
 
 ## TShock 4.3.24
 * Updated OpenTerraria API to 1.3.5.3 (@DeathCradle)