Merge pull request #387 from stevenh/d862d7922e9e0f70d912519d05e42c2b0c3dfc93

Groups, Permissions, Ban logic fixes

TShockAPI/Commands.cs

@@ -870,25 +870,14 @@ namespace TShockAPI
 			var ban = TShock.Bans.GetBanByName(plStr);
 			if (ban != null)
 			{
-				if (TShock.Bans.RemoveBan(ban.IP))
+				if (TShock.Bans.RemoveBan(ban.IP, true))
-					args.Player.SendMessage(string.Format("Unbanned {0} ({1})!", ban.Name, ban.IP), Color.Red);
-				else
-					args.Player.SendMessage(string.Format("Failed to unban {0} ({1})!", ban.Name, ban.IP), Color.Red);
-			}
-			else if (!TShock.Config.EnableBanOnUsernames)
-			{
-				ban = TShock.Bans.GetBanByIp(plStr);
-
-				if (ban == null)
-					args.Player.SendMessage(string.Format("Failed to unban {0}, not found.", args.Parameters[0]), Color.Red);
-				else if (TShock.Bans.RemoveBan(ban.IP))
 					args.Player.SendMessage(string.Format("Unbanned {0} ({1})!", ban.Name, ban.IP), Color.Red);
 				else
 					args.Player.SendMessage(string.Format("Failed to unban {0} ({1})!", ban.Name, ban.IP), Color.Red);
 			}
 			else
 			{
-				args.Player.SendMessage("Invalid player!", Color.Red);
+				args.Player.SendMessage(string.Format("No bans for player {0} exist", plStr), Color.Red);
 			}
 		}
 
@@ -947,8 +936,8 @@ namespace TShockAPI
 				return;
 			}
 
-			string plStr = args.Parameters[0];
+			var ip = args.Parameters[0];
-			var ban = TShock.Bans.GetBanByIp(plStr);
+			var ban = TShock.Bans.GetBanByIp(ip);
 			if (ban != null)
 			{
 				if (TShock.Bans.RemoveBan(ban.IP))
@@ -958,7 +947,7 @@ namespace TShockAPI
 			}
 			else
 			{
-				args.Player.SendMessage("Invalid player!", Color.Red);
+				args.Player.SendMessage(string.Format("No bans for ip {0} exist", ip), Color.Red);
 			}
 		}
 

TShockAPI/DB/BanManager.cs

@@ -40,15 +40,15 @@ namespace TShockAPI.DB
 			                                  db.GetSqlType() == SqlType.Sqlite
 			                                  	? (IQueryBuilder) new SqliteQueryCreator()
 			                                  	: new MysqlQueryCreator());
-								try{
+			try
-			creator.EnsureExists(table);
+			{
-								}
+				creator.EnsureExists(table);
-								catch (DllNotFoundException ex)
+			}
-{
+			catch (DllNotFoundException)
-System.Console.WriteLine("Possible problem with your database - is Sqlite3.dll present?");
+			{
-throw new Exception("Could not find a database library (probably Sqlite3.dll)");
+				System.Console.WriteLine("Possible problem with your database - is Sqlite3.dll present?");
-}
+				throw new Exception("Could not find a database library (probably Sqlite3.dll)");
-
+			}
 		}
 
 		public Ban GetBanByIp(string ip)
@@ -110,7 +110,7 @@ throw new Exception("Could not find a database library (probably Sqlite3.dll)");
 			return null;
 		}
 
-		public bool AddBan(string ip, string name = "", string reason = "")
+		public bool AddBan(string ip, string name = "", string reason = "", bool exceptions = false)
 		{
 			try
 			{
@@ -118,12 +118,15 @@ throw new Exception("Could not find a database library (probably Sqlite3.dll)");
 			}
 			catch (Exception ex)
 			{
+				if (exceptions)
+					throw ex;
 				Log.Error(ex.ToString());
 			}
 			return false;
 		}
 		
-		public bool RemoveBan(string match, bool byName = false, bool casesensitive = true)
+
+		public bool RemoveBan(string match, bool byName = false, bool casesensitive = true, bool exceptions = false)
 		{
 			try
 			{
@@ -135,6 +138,8 @@ throw new Exception("Could not find a database library (probably Sqlite3.dll)");
 			}
 			catch (Exception ex)
 			{
+				if (exceptions)
+					throw ex;
 				Log.Error(ex.ToString());
 			}
 			return false;

TShockAPI/DB/GroupManager.cs

@@ -29,7 +29,7 @@ namespace TShockAPI.DB
 	{
 		private IDbConnection database;
 
-		public List<Group> groups = new List<Group>();
+		public readonly List<Group> groups = new List<Group>();
 
 		public GroupManager(IDbConnection db)
 		{
@@ -49,14 +49,23 @@ namespace TShockAPI.DB
 			                                  	: new MysqlQueryCreator());
 			creator.EnsureExists(table);
 
-			//Add default groups
+			// Load Permissions from the DB
-			AddGroup("guest", "canbuild,canregister,canlogin,canpartychat,cantalkinthird");
+			LoadPermisions();
-			AddGroup("default", "guest", "warp,canchangepassword");
+
-			AddGroup("newadmin", "default", "kick,editspawn,reservedslot");
+			// Add default groups if they don't exist
-			AddGroup("admin", "newadmin",
+			AddDefaultGroup("guest", "", "canbuild,canregister,canlogin,canpartychat,cantalkinthird");
+			AddDefaultGroup("default", "guest", "warp,canchangepassword");
+			AddDefaultGroup("newadmin", "default", "kick,editspawn,reservedslot");
+			AddDefaultGroup("admin", "newadmin",
 			         "ban,unban,whitelist,causeevents,spawnboss,spawnmob,managewarp,time,tp,pvpfun,kill,logs,immunetokick,tphere");
-			AddGroup("trustedadmin", "admin", "maintenance,cfg,butcher,item,heal,immunetoban,usebanneditem,manageusers");
+			AddDefaultGroup("trustedadmin", "admin", "maintenance,cfg,butcher,item,heal,immunetoban,usebanneditem,manageusers");
-			AddGroup("vip", "default", "reservedslot");
+			AddDefaultGroup("vip", "default", "reservedslot");
+		}
+
+		private void AddDefaultGroup(string name, string parent, string permissions)
+		{
+			if (!GroupExists(name))
+				AddGroup(name, parent, permissions);
 		}
 
 
@@ -92,9 +101,8 @@ namespace TShockAPI.DB
 		/// <param name="permissions">permissions</param>
 		/// <param name="chatcolor">chatcolor</param>
 		/// <param name="exceptions">exceptions true indicates use exceptions for errors false otherwise</param>
-		public String AddGroup(String name, string parentname, String permissions, String chatcolor, bool exceptions = false)
+		public String AddGroup(String name, string parentname, String permissions, String chatcolor = "255,255,255", bool exceptions = false)
 		{
-			String message = "";
 			if (GroupExists(name))
 			{
 				if (exceptions)
@@ -103,13 +111,13 @@ namespace TShockAPI.DB
 			}
 
 			var group = new Group(name, null, chatcolor);
-			group.permissions.Add(permissions);
+			group.Permissions = permissions;
 			if (!string.IsNullOrWhiteSpace(parentname))
 			{
 				var parent = groups.FirstOrDefault(gp => gp.Name == parentname);
 				if (parent == null)
 				{
-					var error = "Invalid parent {0} for group {1}".SFormat(group.Name, parentname);
+					var error = "Invalid parent {0} for group {1}".SFormat(parentname, group.Name);
 					if (exceptions)
 						throw new GroupManagerException(error);
 					Log.ConsoleError(error);
@@ -123,13 +131,13 @@ namespace TShockAPI.DB
 			               	: "INSERT IGNORE INTO GroupList SET GroupName=@0, Parent=@1, Commands=@2, ChatColor=@3";
 			if (database.Query(query, name, parentname, permissions, chatcolor) == 1)
 			{
-				message = "Group " + name + " has been created successfully.";
 				groups.Add(group);
+				return "Group " + name + " has been created successfully.";
 			}
 			else if (exceptions)
 				throw new GroupManagerException("Failed to add group '" + name + "'");
 
-			return message;
+			return "";
 		}
 
 		public String AddGroup(String name, String permissions)
@@ -137,14 +145,38 @@ namespace TShockAPI.DB
 			return AddGroup(name, "", permissions, "255,255,255");
 		}
 
-		public String AddGroup(String name, string parent, String permissions)
+		/// <summary>
+		/// Updates a group including permissions
+		/// </summary>
+		/// <param name="name">name of the group to update</param>
+		/// <param name="parentname">parent of group</param>
+		/// <param name="permissions">permissions</param>
+		/// <param name="chatcolor">chatcolor</param>
+		public void UpdateGroup(string name, string parentname, string permissions, string chatcolor)
 		{
-			return AddGroup(name, parent, permissions, "255,255,255");
+			if (!GroupExists(name))
+				throw new GroupNotExistException(name);
+
+			Group parent = null;
+			if (!string.IsNullOrWhiteSpace(parentname))
+			{
+				parent = groups.FirstOrDefault(gp => gp.Name == parentname);
+				if (null == parent)
+					throw new GroupManagerException("Invalid parent {0} for group {1}".SFormat(parentname, name));
+			}
+
+			// NOTE: we use newgroup.XYZ to ensure any validation is also persisted to the DB
+			var newgroup = new Group(name, parent, chatcolor, permissions);
+			string query = "UPDATE GroupList SET Parent=@0, Commands=@1, ChatColor=@2 WHERE GroupName=@3";
+			if (database.Query(query, parentname, newgroup.Permissions, newgroup.ChatColor, name) != 1)
+				throw new GroupManagerException("Failed to update group '" + name + "'");
+
+			groups.Remove(TShock.Utils.GetGroup(name));
+			groups.Add(newgroup);
 		}
 
 		public String DeleteGroup(String name, bool exceptions = false)
 		{
-			String message = "";
 			if (!GroupExists(name))
 			{
 				if (exceptions)
@@ -154,60 +186,60 @@ namespace TShockAPI.DB
 
 			if (database.Query("DELETE FROM GroupList WHERE GroupName=@0", name) == 1)
 			{
-				message = "Group " + name + " has been deleted successfully.";
 				groups.Remove(TShock.Utils.GetGroup(name));
+				return "Group " + name + " has been deleted successfully.";
 			}
 			else if (exceptions)
 				throw new GroupManagerException("Failed to delete group '" + name + "'");
 
-			return message;
+			return "";
 		}
 
 		public String AddPermissions(String name, List<String> permissions)
 		{
-			String message = "";
 			if (!GroupExists(name))
 				return "Error: Group doesn't exists.";
 
 			var group = TShock.Utils.GetGroup(name);
-			//Add existing permissions (without duplicating)
+			var oldperms = group.Permissions; // Store old permissions in case of error
-			permissions.AddRange(group.permissions.Where(s => !permissions.Contains(s)));
+			permissions.ForEach(p => group.AddPermission(p));
  
-			if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", String.Join(",", permissions), name) != 0)
+			if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", group.Permissions, name) == 1)
-			{
+				return "Group " + name + " has been modified successfully.";
-				message = "Group " + name + " has been modified successfully.";
+
-				group.SetPermission(permissions);
+			// Restore old permissions so DB and internal object are in a consistent state
-			}
+			group.Permissions = oldperms;
-			return message;
+			return "";
 		}
 
 		public String DeletePermissions(String name, List<String> permissions)
 		{
-			String message = "";
 			if (!GroupExists(name))
 				return "Error: Group doesn't exists.";
 
 			var group = TShock.Utils.GetGroup(name);
+			var oldperms = group.Permissions; // Store old permissions in case of error
+			permissions.ForEach(p => group.RemovePermission(p));
 
-			//Only get permissions that exist in the group.
+			if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", group.Permissions, name) == 1)
-			var newperms = group.permissions.Except(permissions);
+				return "Group " + name + " has been modified successfully.";
 	
-			if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", String.Join(",", newperms), name) != 0)
+			// Restore old permissions so DB and internal object are in a consistent state
-			{
+			group.Permissions = oldperms;
-				message = "Group " + name + " has been modified successfully.";
+			return "";
-				group.SetPermission(newperms.ToList());
-			}
-			return message;
 		}
 
 		public void LoadPermisions()
 		{
-			//Create a temporary list so if there is an error it doesn't override the currently loaded groups with broken groups.
+			// Create a temporary list so if there is an error it doesn't override the currently loaded groups with broken groups.
 			var tempgroups = new List<Group>();
 			tempgroups.Add(new SuperAdminGroup());
 
 			if (groups == null || groups.Count < 2)
-				groups = tempgroups;
+			{
+				groups.Clear();
+				groups.AddRange(tempgroups);
+			}
 
 			try
 			{
@@ -216,34 +248,9 @@ namespace TShockAPI.DB
 				{
 					while (reader.Read())
 					{
-						string groupname = reader.Get<String>("GroupName");
+						var group = new Group(reader.Get<String>("GroupName"), null, reader.Get<String>("ChatColor"), reader.Get<String>("Commands"));
-						var group = new Group(groupname);
-
 						group.Prefix = reader.Get<String>("Prefix");
 						group.Suffix = reader.Get<String>("Suffix");
-
-						//Inherit Given commands
-						String[] commands = reader.Get<String>("Commands").Split(',');
-						foreach (var t in commands)
-						{
-							var str = t.Trim();
-							if (str.StartsWith("!"))
-							{
-								group.NegatePermission(str.Substring(1));
-							}
-							else
-							{
-								group.AddPermission(str);
-							}
-						}
-						String[] chatcolour = (reader.Get<String>("ChatColor") ?? "").Split(',');
-						if (chatcolour.Length == 3)
-						{
-							byte.TryParse(chatcolour[0], out group.R);
-							byte.TryParse(chatcolour[1], out group.G);
-							byte.TryParse(chatcolour[2], out group.B);
-						}
-
 						groupsparents.Add(Tuple.Create(group, reader.Get<string>("Parent")));
 					}
 				}
@@ -257,7 +264,7 @@ namespace TShockAPI.DB
 						var parent = groupsparents.FirstOrDefault(gp => gp.Item1.Name == parentname);
 						if (parent == null)
 						{
-							Log.ConsoleError("Invalid parent {0} for group {1}".SFormat(group.Name, parentname));
+							Log.ConsoleError("Invalid parent {0} for group {1}".SFormat(parentname, group.Name));
 							return;
 						}
 						group.Parent = parent.Item1;
@@ -265,8 +272,8 @@ namespace TShockAPI.DB
 					tempgroups.Add(group);
 				}
 
-
+				groups.Clear();
-				groups = tempgroups;
+				groups.AddRange(tempgroups);
 			}
 			catch (Exception ex)
 			{
@@ -289,21 +296,21 @@ namespace TShockAPI.DB
 		}
 	}
 
-    [Serializable]
+	[Serializable]
-    public class GroupExistsException : GroupManagerException
+	public class GroupExistsException : GroupManagerException
-    {
+	{
-        public GroupExistsException(string name)
+		public GroupExistsException(string name)
-            : base("Group '" + name + "' already exists")
+			: base("Group '" + name + "' already exists")
-        {
+		{
-        }
+		}
-    }
+	}
 
-    [Serializable]
+	[Serializable]
-    public class GroupNotExistException : GroupManagerException
+	public class GroupNotExistException : GroupManagerException
-    {
+	{
-        public GroupNotExistException(string name)
+		public GroupNotExistException(string name)
-            : base("Group '" + name + "' does not exist")
+			: base("Group '" + name + "' does not exist")
-        {
+		{
-        }
+		}
-    }
+	}
 } 

TShockAPI/DB/UserManager.cs

@@ -20,6 +20,7 @@ using System.Data;
 using System.IO;
 using System.Collections.Generic;
 using MySql.Data.MySqlClient;
+using System.Text.RegularExpressions;
 
 namespace TShockAPI.DB
 {
@@ -62,6 +63,9 @@ namespace TShockAPI.DB
 			}
 			catch (Exception ex)
 			{
+				// Detect duplicate user using a regexp as Sqlite doesn't have well structured exceptions
+				if (Regex.IsMatch(ex.Message, "Username.*not unique"))
+					throw new UserExistsException(user.Name);
 				throw new UserManagerException("AddUser SQL returned an error (" + ex.Message + ")", ex);
 			}
 
@@ -254,15 +258,22 @@ namespace TShockAPI.DB
 					result = database.QueryReader("SELECT * FROM Users WHERE IP=@0", user.Address);
 				}
 
-				using (var reader = result)
+				if (result.Read())
 				{
-					if (reader.Read())
+					user = LoadUserFromResult(user, result);
-						return LoadUserFromResult(user, result);
+					// Check for multiple matches
+					if (!result.Read())
+						return user;
+
+					if (string.IsNullOrEmpty(user.Address))
+						throw new UserManagerException(String.Format("Multiple users found for name '{0}'", user.Name));
+					else
+						throw new UserManagerException(String.Format("Multiple users found for ip '{0}'", user.Address));
 				}
 			}
 			catch (Exception ex)
 			{
-				throw new UserManagerException("GetUserID SQL returned an error", ex);
+				throw new UserManagerException("GetUser SQL returned an error (" + ex.Message + ")", ex);
 			}
 			throw new UserNotExistException(string.IsNullOrEmpty(user.Address) ? user.Name : user.Address);
 		}

TShockAPI/Group.cs

@@ -30,23 +30,57 @@ namespace TShockAPI
 		public int Order { get; set; }
 		public string Prefix { get; set; }
 		public string Suffix { get; set; }
+		public string ParentName { get { return (null == Parent) ? "" : Parent.Name; } }
+		public string ChatColor
+		{
+			get { return string.Format("{0}{1}{2}", R.ToString("X2"), G.ToString("X2"), B.ToString("X2")); }
+			set
+			{
+				if (null != value)
+				{
+					string[] parts = value.Split(',');
+					if (3 == parts.Length)
+					{
+						byte r, g, b;
+						if (byte.TryParse(parts[0], out r) && byte.TryParse(parts[1], out g) && byte.TryParse(parts[2], out b))
+						{
+							R = r;
+							G = g;
+							B = b;
+							return;
+						}
+					}
+				}
+			}
+		}
+
+		public string Permissions
+		{
+			get
+			{
+				List<string> all = new List<string>(permissions);
+				permissions.ForEach(p => all.Add("!" + p));
+				return string.Join(",", all);
+			}
+			set
+			{
+				permissions.Clear();
+				negatedpermissions.Clear();
+				if (null != value)
+					value.Split(',').ForEach(p => AddPermission(p.Trim()));
+			}
+		}
 
 		public byte R = 255;
 		public byte G = 255;
 		public byte B = 255;
 
-		public Group(string groupname, Group parentgroup = null, string chatcolor = "255,255,255")
+		public Group(string groupname, Group parentgroup = null, string chatcolor = "255,255,255", string permissions = null)
 		{
 			Name = groupname;
 			Parent = parentgroup;
-			byte.TryParse(chatcolor.Split(',')[0], out R);
+			ChatColor = chatcolor;
-			byte.TryParse(chatcolor.Split(',')[1], out G);
+			Permissions = permissions;
-			byte.TryParse(chatcolor.Split(',')[2], out B);
-		}
-
-		public string ChatColor()
-		{
-			return string.Format("{0}{1}{2}", R.ToString("X2"), G.ToString("X2"), B.ToString("X2"));
 		}
 
 		public virtual bool HasPermission(string permission)
@@ -73,21 +107,44 @@ namespace TShockAPI
 
 		public void NegatePermission(string permission)
 		{
-			negatedpermissions.Add(permission);
+			// Avoid duplicates
+			if (!negatedpermissions.Contains(permission))
+			{
+				negatedpermissions.Add(permission);
+				permissions.Remove(permission); // Ensure we don't have conflicting definitions for a permissions
+			}
 		}
 
 		public void AddPermission(string permission)
 		{
-			permissions.Add(permission);
+			if (permission.StartsWith("!"))
+			{
+				NegatePermission(permission.Substring(1));
+				return;
+			}
+			// Avoid duplicates
+			if (!permissions.Contains(permission))
+			{
+				permissions.Add(permission);
+				negatedpermissions.Remove(permission); // Ensure we don't have conflicting definitions for a permissions
+			}
 		}
 
 		public void SetPermission(List<string> permission)
 		{
 			permissions.Clear();
-			foreach (string s in permission)
+			negatedpermissions.Clear();
+			permission.ForEach(p => AddPermission(p));
+		}
+
+		public void RemovePermission(string permission)
+		{
+			if (permission.StartsWith("!"))
 			{
-				permissions.Add(s);
+				negatedpermissions.Remove(permission.Substring(1));
+				return;
 			}
+			permissions.Remove(permission);
 		}
 	}
 

TShockAPI/Rest/Rest.cs

@@ -21,6 +21,7 @@ using System.ComponentModel;
 using System.Net;
 using System.Text;
 using System.Text.RegularExpressions;
+using System.Reflection;
 using HttpServer;
 using HttpServer.Headers;
 using Newtonsoft.Json;
@@ -41,6 +42,7 @@ namespace Rests
 	{
 		private readonly List<RestCommand> commands = new List<RestCommand>();
 		private HttpListener listener;
+		private StringHeader serverHeader;
 		public IPAddress Ip { get; set; }
 		public int Port { get; set; }
 
@@ -48,6 +50,9 @@ namespace Rests
 		{
 			Ip = ip;
 			Port = port;
+			string appName = this.GetType().Assembly.GetName().Version.ToString();
+			AssemblyName ass = this.GetType().Assembly.GetName();
+			serverHeader = new StringHeader("Server", String.Format("{0}/{1}", ass.Name, ass.Version));
 		}
 
 		public virtual void Start()
@@ -117,14 +122,14 @@ namespace Rests
 				throw new NullReferenceException("obj");
 
 			if (OnRestRequestCall(e))
-			return;
+				return;
 
 			var str = JsonConvert.SerializeObject(obj, Formatting.Indented);
 			e.Response.Connection.Type = ConnectionType.Close;
-			e.Response.Add(new ContentTypeHeader("application/json"));
+			e.Response.ContentType = new ContentTypeHeader("application/json");
+			e.Response.Add(serverHeader);
 			e.Response.Body.Write(Encoding.ASCII.GetBytes(str), 0, str.Length);
 			e.Response.Status = HttpStatusCode.OK;
-			return;
 		}
 
 		protected virtual object ProcessRequest(object sender, RequestEventArgs e)

TShockAPI/TShock.cs

@@ -172,7 +172,6 @@ namespace TShockAPI
 				Warps = new WarpManager(DB);
 				Users = new UserManager(DB);
 				Groups = new GroupManager(DB);
-				Groups.LoadPermisions();
 				Regions = new RegionManager(DB);
 				Itembans = new ItemManager(DB);
 				RememberedPos = new RemeberedPosManager(DB);
@@ -388,11 +387,11 @@ namespace TShockAPI
 					Console.WriteLine("Startup parameter overrode REST port.");
 
 				}
-                if ((parms[i].ToLower() == "-maxplayers")||(parms[i].ToLower() == "-players"))
+				if ((parms[i].ToLower() == "-maxplayers")||(parms[i].ToLower() == "-players"))
-                {
+				{
-                    Config.MaxSlots = Convert.ToInt32(parms[++i]);
+					Config.MaxSlots = Convert.ToInt32(parms[++i]);
-                    Console.WriteLine("Startup parameter overrode maximum player slot configuration value.");
+					Console.WriteLine("Startup parameter overrode maximum player slot configuration value.");
-                }
+				}
 			}
 		}
 
@@ -659,10 +658,18 @@ namespace TShockAPI
 				return;
 			}
 
-			var nameban = Bans.GetBanByName(player.Name);
 			Ban ban = null;
-			if (nameban != null && Config.EnableBanOnUsernames)
+			if (Config.EnableBanOnUsernames)
-				ban = nameban;
+			{
+				var newban = Bans.GetBanByName(player.Name);
+				if (null != newban)
+					ban = newban;
+			}
+
+			if (Config.EnableIPBans && null == ban)
+			{
+				ban = Bans.GetBanByIp(player.IP);
+			}
 
 			if (ban != null)
 			{