Fixes issue where role banned players can still play roles they're banned from. (#77738)

## About The Pull Request

`is_banned_from(...)` expects a ckey.

`/obj/effect/mob_spawn/ghost_role/attack_ghost(...)` checks for role
bans by using key instead.

This can lead to players whose keys and ckeys are different being able
to evade certain ghost role bans; by accident or otherwise.

This PR takes a two-pronged approach. The first fixes ghost roles
passing in the key instead of the ckey to is_banned_from. This fixes the
bug, and makes it consistent with all other cases of
`is_banned_from(...)` being called.

The second is to redefine the behaviour of `is_banned_from(...)` to
accept either a ckey OR a key, since converting from key to canonical
key should be a fairly trivial operation. This prevents this specific
bug from ever occuring again, by making it intended functionality to
pass either key or ckey similar to how the roles param accepts either a
string role or a list of roles.

### ***Please review the code carefully, my changes to
`is_banned_from(...)` have not been tested. No logical flow should have
been changed.***
## Why It's Good For The Game

Ban systems working good.
## Changelog
🆑
fix: Fixes an issue where role banned players would be able to accept
certain ghost roles they're meant to be banned from.
/🆑

code/modules/admin/sql_ban_system.dm

@@ -3,54 +3,88 @@
 
 #define MAX_REASON_LENGTH 600
 
-//checks client ban cache or DB ban table if ckey is banned from one or more roles
-//doesn't return any details, use only for if statements
-/proc/is_banned_from(player_ckey, list/roles)
-	if(!player_ckey)
-		return
+/**
+ * Checks client ban cache or, if it doesn't exist, queries the DB ban table to see if the player's
+ * ckey is banned from at least one of the provided roles.
+ *
+ * Returns TRUE if the player matches with one or more role bans.
+ * Returns FALSE if the player doesn't match with any role bans. Possible errors states also return FALSE.
+ *
+ * Args:
+ * * player_key - Either key or ckey of the player you want to check for role bans.
+ * * roles - Accepts either a single role string, or a list of role strings.
+ */
+/proc/is_banned_from(player_key, list/roles)
+	if(!player_key)
+		stack_trace("Called is_banned_from without specifying a ckey.")
+		return FALSE
+
+	// Convert to ckey. This allows is_banned_from to take either key or ckey interchangably,
+	// and is officially a feature of the proc.
+	var/player_ckey = ckey(player_key)
+
 	var/client/player_client = GLOB.directory[player_ckey]
+
+	// If there's a player client, we try to set up their ban cache (if it doesn't already exist) and test from that.
 	if(player_client)
 		var/list/ban_cache = retrieve_ban_cache(player_client)
+
+		// If this isn't a list, the client disconnected while building it.
 		if(!islist(ban_cache))
-			return // Disconnected while building the list.
+			return FALSE
+
+		// If it is a list, check each role.
 		if(islist(roles))
 			for(var/role in roles)
 				if(role in ban_cache)
 					return TRUE //they're banned from at least one role, no need to keep checking
-		else if(roles in ban_cache)
-			return TRUE
+
+			return FALSE
+
+		// Otherwise, it's just a single role string. Return if it's in the ban cache.
+		return (roles in ban_cache)
+
+	// If there's no player client, we'll ask the database.
+	var/values = list(
+		"player_ckey" = player_ckey,
+		"must_apply_to_admins" = !!(GLOB.admin_datums[player_ckey] || GLOB.deadmins[player_ckey]),
+	)
+
+	var/sql_roles
+	if(islist(roles))
+		var/list/sql_roles_list = list()
+		for (var/i in 1 to roles.len)
+			values["role[i]"] = roles[i]
+			sql_roles_list += ":role[i]"
+		sql_roles = sql_roles_list.Join(", ")
 	else
-		var/values = list(
-			"player_ckey" = player_ckey,
-			"must_apply_to_admins" = !!(GLOB.admin_datums[player_ckey] || GLOB.deadmins[player_ckey]),
-		)
-		var/sql_roles
-		if(islist(roles))
-			var/list/sql_roles_list = list()
-			for (var/i in 1 to roles.len)
-				values["role[i]"] = roles[i]
-				sql_roles_list += ":role[i]"
-			sql_roles = sql_roles_list.Join(", ")
-		else
-			values["role"] = roles
-			sql_roles = ":role"
-		var/datum/db_query/query_check_ban = SSdbcore.NewQuery({"
-			SELECT 1
-			FROM [format_table_name("ban")]
-			WHERE
-				ckey = :player_ckey AND
-				role IN ([sql_roles]) AND
-				unbanned_datetime IS NULL AND
-				(expiration_time IS NULL OR expiration_time > NOW())
-				AND (NOT :must_apply_to_admins OR applies_to_admins = 1)
-		"}, values)
-		if(!query_check_ban.warn_execute())
-			qdel(query_check_ban)
-			return
-		if(query_check_ban.NextRow())
-			qdel(query_check_ban)
-			return TRUE
+		values["role"] = roles
+		sql_roles = ":role"
+
+	var/datum/db_query/query_check_ban = SSdbcore.NewQuery({"
+		SELECT 1
+		FROM [format_table_name("ban")]
+		WHERE
+			ckey = :player_ckey AND
+			role IN ([sql_roles]) AND
+			unbanned_datetime IS NULL AND
+			(expiration_time IS NULL OR expiration_time > NOW())
+			AND (NOT :must_apply_to_admins OR applies_to_admins = 1)
+	"}, values)
+
+	// If there's an SQL error, return FALSE.
+	if(!query_check_ban.warn_execute())
 		qdel(query_check_ban)
+		return FALSE
+
+	// If there are any rows, we got a match and they're role banned from at least one role.
+	if(query_check_ban.NextRow())
+		qdel(query_check_ban)
+		return TRUE
+
+	// Otherwise, they're not banned from any roles in the DB.
+	qdel(query_check_ban)
+	return FALSE
 
 //checks DB ban table if a ckey, ip and/or cid is banned from a specific role
 //returns an associative nested list of each matching row's ban id, bantime, ban round id, expiration time, ban duration, applies to admins, reason, key, ip, cid and banning admin's key in that order