From 4d46e58865c256f1fc42e72c88c5fe78f37b9521 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Sun, 13 Jun 2021 01:09:03 -0700 Subject: [PATCH] Prevent users from removing default guest group The default guest group is critical and shouldn't be removed without either TShock doing something like automatically recreating it if it doesn't exist, or not having a huge problem if it doesn't exist. I chose to take the easiest path, preventing users from removing it. In theory the message gives enough context to imply "okay, go change the group now." This should be a relatively small edge case but I wanted to resolve it while I was here. --- CHANGELOG.md | 1 + TShockAPI/Commands.cs | 6 +++--- TShockAPI/DB/GroupManager.cs | 7 +++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a421bafc..4c8315ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * Fixed `/home` allowing players to bypass the respawn timer. (@hakusaro, @moisterrific, @Arthri) * Added the config option `SuppressPermissionFailureNotices`. When set to `true`, the server will not send warning messages to players when they fail a build permission check from `TSPlayer.HasBuildPermission` (even if `shouldWarnPlayer` is set to true. (@hakusaro) * Fixed `/warp send` failing a nullcheck if the warp didn't exist. The previous behavior may have always been buggy or broken. In other words, sending someone to a warp that doesn't exist should result in a nicer error. (@hakusaro, @punchready) +* Fixed `/group del` allowing server operators to delete the default group that guests are put into. This is a really critical group and the server doesn't behave correctly when it happens. As a result, it's better to prevent this from happening than not. Additionally, `GroupManagerException`s will be thrown if this is attempted programmatically. Finally, if the exception is thrown in response to `/group del` (or if any other exception is thrown that the command handler can handle), the stack trace will no longer be present. This was reported long ago but I can't find the report anymore. If you know who reported this or if you reported it and want credit, please send @hakusaro a note. (@hakusaro) ## TShock 4.5.4 * Fixed ridiculous typo in `GetDataHandlers` which caused TShock to read the wrong field in the packet for `usingBiomeTorches`. (@hakusaro, @Arthri) diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs index 5ea9d5ee..0dbe6980 100644 --- a/TShockAPI/Commands.cs +++ b/TShockAPI/Commands.cs @@ -3498,7 +3498,7 @@ namespace TShockAPI try { - string response = TShock.Groups.DeleteGroup(args.Parameters[1]); + string response = TShock.Groups.DeleteGroup(args.Parameters[1], true); if (response.Length > 0) { args.Player.SendSuccessMessage(response); @@ -3506,7 +3506,7 @@ namespace TShockAPI } catch (GroupManagerException ex) { - args.Player.SendErrorMessage(ex.ToString()); + args.Player.SendErrorMessage(ex.Message); } } #endregion @@ -3542,7 +3542,7 @@ namespace TShockAPI } catch (GroupManagerException ex) { - args.Player.SendErrorMessage(ex.ToString()); + args.Player.SendErrorMessage(ex.Message); } } #endregion diff --git a/TShockAPI/DB/GroupManager.cs b/TShockAPI/DB/GroupManager.cs index 3bc66adb..7b3b62dd 100644 --- a/TShockAPI/DB/GroupManager.cs +++ b/TShockAPI/DB/GroupManager.cs @@ -456,6 +456,13 @@ namespace TShockAPI.DB return "Error: Group doesn't exist."; } + if (name == Group.DefaultGroup.Name) + { + if (exceptions) + throw new GroupManagerException("Unable to remove default guest group."); + return "Error: Unable to remove the default guest group."; + } + if (database.Query("DELETE FROM GroupList WHERE GroupName=@0", name) == 1) { groups.Remove(TShock.Groups.GetGroupByName(name));