fix(db): Correct casing and escaping in DB queries

Updates the database queries to handle casing inconsistencies
and improves SQL query parameter escaping for better
security and compatibility.

Refactors group existence checks for simplicity, enhancing
readability and maintainability.

Addresses issues related to unique constraints in user
registration by improving error handling for duplicate
usernames.
This commit is contained in:
Sakura Akeno Isayeki 2025-04-29 01:38:32 +02:00
parent 2d839e3609
commit de602a91d4
No known key found for this signature in database
GPG key ID: BAB781B71FD2E7E6
2 changed files with 33 additions and 34 deletions

View file

@ -253,13 +253,7 @@ namespace TShockAPI.DB
/// </summary> /// </summary>
/// <param name="group">The group.</param> /// <param name="group">The group.</param>
/// <returns><c>true</c> if it does; otherwise, <c>false</c>.</returns> /// <returns><c>true</c> if it does; otherwise, <c>false</c>.</returns>
public bool GroupExists(string group) public bool GroupExists(string group) => group is "superadmin" || groups.Any(g => g.Name.Equals(group));
{
if (group == "superadmin")
return true;
return groups.Any(g => g.Name.Equals(group));
}
IEnumerator IEnumerable.GetEnumerator() IEnumerator IEnumerable.GetEnumerator()
{ {
@ -270,21 +264,14 @@ namespace TShockAPI.DB
/// Gets the enumerator. /// Gets the enumerator.
/// </summary> /// </summary>
/// <returns>The enumerator.</returns> /// <returns>The enumerator.</returns>
public IEnumerator<Group> GetEnumerator() public IEnumerator<Group> GetEnumerator() => groups.GetEnumerator();
{
return groups.GetEnumerator();
}
/// <summary> /// <summary>
/// Gets the group matching the specified name. /// Gets the group matching the specified name.
/// </summary> /// </summary>
/// <param name="name">The name.</param> /// <param name="name">The name.</param>
/// <returns>The group.</returns> /// <returns>The group.</returns>
public Group GetGroupByName(string name) public Group GetGroupByName(string name) => groups.FirstOrDefault(g => g.Name == name);
{
var ret = groups.Where(g => g.Name == name);
return 1 == ret.Count() ? ret.ElementAt(0) : null;
}
/// <summary> /// <summary>
/// Adds group with name and permissions if it does not exist. /// Adds group with name and permissions if it does not exist.

View file

@ -70,15 +70,22 @@ namespace TShockAPI.DB
int ret; int ret;
try try
{ {
ret = _database.Query("INSERT INTO Users (Username, Password, UUID, UserGroup, Registered) VALUES (@0, @1, @2, @3, @4);", account.Name, string query = _database.GetSqlType() switch
account.Password, account.UUID, account.Group, DateTime.UtcNow.ToString("s")); {
SqlType.Postgres => "INSERT INTO Users (\"Username\", \"Password\", \"UUID\", \"Usergroup\", \"Registered\") VALUES (@0, @1, @2, @3, @4);",
_ => "INSERT INTO Users (Username, Password, UUID, Usergroup, Registered) VALUES (@0, @1, @2, @3, @4);"
};
ret = _database.Query(query, account.Name, account.Password, account.UUID, account.Group, DateTime.UtcNow.ToString("s"));
} }
catch (Exception ex) // Detect duplicate user using a regexp as Sqlite doesn't have well structured exceptions
catch (Exception e) when (Regex.IsMatch(e.Message, "Username.*not unique|UNIQUE constraint failed: Users\\.Username"))
{ {
// Detect duplicate user using a regexp as Sqlite doesn't have well structured exceptions throw new UserAccountExistsException(account.Name);
if (Regex.IsMatch(ex.Message, "Username.*not unique|UNIQUE constraint failed: Users\\.Username")) }
throw new UserAccountExistsException(account.Name); catch (Exception e)
throw new UserAccountManagerException(GetString($"AddUser SQL returned an error ({ex.Message})"), ex); {
throw new UserAccountManagerException(GetString($"AddUser SQL returned an error ({e.Message})"), e);
} }
if (1 > ret) if (1 > ret)
@ -99,7 +106,7 @@ namespace TShockAPI.DB
TShock.Players.Where(p => p?.IsLoggedIn == true && p.Account.Name == account.Name).ForEach(p => p.Logout()); TShock.Players.Where(p => p?.IsLoggedIn == true && p.Account.Name == account.Name).ForEach(p => p.Logout());
UserAccount tempuser = GetUserAccount(account); UserAccount tempuser = GetUserAccount(account);
int affected = _database.Query("DELETE FROM Users WHERE Username=@0", account.Name); int affected = _database.Query($"DELETE FROM Users WHERE {"Username".EscapeSqlId(_database)}=@0", account.Name);
if (affected < 1) if (affected < 1)
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
@ -124,10 +131,11 @@ namespace TShockAPI.DB
{ {
account.CreateBCryptHash(password); account.CreateBCryptHash(password);
if ( if (_database.Query($"UPDATE Users SET {"Password".EscapeSqlId(_database)} = @0 WHERE {"Username".EscapeSqlId(_database)} = @1;",
_database.Query("UPDATE Users SET Password = @0 WHERE Username = @1;", account.Password, account.Password, account.Name) is 0)
account.Name) == 0) {
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
}
} }
catch (Exception ex) catch (Exception ex)
{ {
@ -144,10 +152,10 @@ namespace TShockAPI.DB
{ {
try try
{ {
if ( if (_database.Query(/*lang=sql*/$"UPDATE Users SET {"UUID".EscapeSqlId(_database)} = @0 WHERE {"Username".EscapeSqlId(_database)} = @1;", uuid, account.Name) is 0)
_database.Query("UPDATE Users SET UUID = @0 WHERE Username = @1;", uuid, {
account.Name) == 0)
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
}
} }
catch (Exception ex) catch (Exception ex)
{ {
@ -169,7 +177,7 @@ namespace TShockAPI.DB
if (AccountHooks.OnAccountGroupUpdate(account, ref grp)) if (AccountHooks.OnAccountGroupUpdate(account, ref grp))
throw new UserGroupUpdateLockedException(account.Name); throw new UserGroupUpdateLockedException(account.Name);
if (_database.Query("UPDATE Users SET UserGroup = @0 WHERE Username = @1;", grp.Name, account.Name) == 0) if (_database.Query($"UPDATE Users SET {"UserGroup".EscapeSqlId(_database)} = @0 WHERE {"Username".EscapeSqlId(_database)} = @1;", grp.Name, account.Name) == 0)
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
try try
@ -200,7 +208,7 @@ namespace TShockAPI.DB
if (AccountHooks.OnAccountGroupUpdate(account, author, ref grp)) if (AccountHooks.OnAccountGroupUpdate(account, author, ref grp))
throw new UserGroupUpdateLockedException(account.Name); throw new UserGroupUpdateLockedException(account.Name);
if (_database.Query("UPDATE Users SET UserGroup = @0 WHERE Username = @1;", grp.Name, account.Name) == 0) if (_database.Query($"UPDATE Users SET {"UserGroup".EscapeSqlId(_database)} = @0 WHERE {"Username".EscapeSqlId(_database)} = @1;", grp.Name, account.Name) == 0)
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
try try
@ -223,8 +231,12 @@ namespace TShockAPI.DB
{ {
try try
{ {
if (_database.Query("UPDATE Users SET LastAccessed = @0, KnownIps = @1 WHERE Username = @2;", DateTime.UtcNow.ToString("s"), account.KnownIps, account.Name) == 0) if (_database.Query(
$"UPDATE Users SET {"LastAccessed".EscapeSqlId(_database)} = @0, {"KnownIPs".EscapeSqlId(_database)} = @1 WHERE {"Username".EscapeSqlId(_database)} = @2;",
DateTime.UtcNow.ToString("s"), account.KnownIps, account.Name) is 0
) {
throw new UserAccountNotExistException(account.Name); throw new UserAccountNotExistException(account.Name);
}
} }
catch (Exception ex) catch (Exception ex)
{ {
@ -388,7 +400,7 @@ namespace TShockAPI.DB
account.Name = result.Get<string>("Username"); account.Name = result.Get<string>("Username");
account.Registered = result.Get<string>("Registered"); account.Registered = result.Get<string>("Registered");
account.LastAccessed = result.Get<string>("LastAccessed"); account.LastAccessed = result.Get<string>("LastAccessed");
account.KnownIps = result.Get<string>("KnownIps"); account.KnownIps = result.Get<string>("KnownIPs");
return account; return account;
} }
} }