Adds In Game Rank Editing (Permissions Panel Cleanup) (#91873)

## About The Pull Request

Ok there's a lot here, sorry bout that.

- Cleaned up the permissions panel backend pretty signficantly
- Added some extra security measures to said code, mostly proc call
checks
- Properly implemented filtering code jordie wrote years and years ago
for permissions logs
- Cleaned up the permissions ui generally, more bars, nicer lookin
stuff, etc
- Fixed the Management panel's relationship with combined roles, and
renamed it to Housekeeping. Its display is expanded too.
- Added tracking to rank datums on where exactly they came from
- Added a new tab to the permissions panel which allows the modification
and deletion of ranks
- Beefed up rank modification to try and avoid accidential temp rank
additions to the db

I'm doing my best to avoid perms escalation issues, tho they are always
possible right.
Also, got mad at some query cleanup handling, did a pass on it. this
isn't nearly all of em, but it's some.

## Why It's Good For The Game

I realized there is no way to, in game, cleanly edit/create ranks, and
that the way the existing system worked was quite opaque.
I'm trying to fix that here. It does mean potentially opening up DB rank
deletion/modification to bad actors, but frankly I am not overly worried
about that. Admin modification has always been a vulnerability so like.

Here's a video with my changes (mostly, it's lightly outdated)
https://file.house/XqME7KWKk0ULj4ZUkJ5reg==.mp4

## Changelog
🆑
refactor: Fucked with admin rank setup very slightly, please yell at me
if anything is wrong.
admin: Updated the permissions panel to be a good bit more user
friendly, added rank management support to it.
server: I've added code that gives the game modification/deletion perms
for the rank table, be made aware.
/🆑

---------

Co-authored-by: san7890 <the@san7890.com>

code/modules/admin/admin_ranks.dm

@@ -5,13 +5,20 @@ GLOBAL_LIST_EMPTY(protected_ranks) //admin ranks loaded from txt
 GLOBAL_PROTECT(protected_ranks)
 
 /datum/admin_rank
+	/// Rank name, key'd to the db
 	var/name = "NoRank"
+	/// Rank source, see RANK_SOURCE_TXT and friends
+	var/source = null
+	/// Our functional rights, these are what we actually use in game
 	var/rights = R_DEFAULT
-	var/exclude_rights = NONE
+	/// Rights we're allowed to use, pre filtering
 	var/include_rights = NONE
+	/// These are the rights of include_rights we aren't allowed to use. Frankly I have no idea why this exists
+	var/exclude_rights = NONE
+	/// Rights we're allowed to edit on other folks, impact of this is dependent on R_PERMISSIONS and R_DBRANKS
 	var/can_edit_rights = NONE
 
-/datum/admin_rank/New(init_name, init_rights, init_exclude_rights, init_edit_rights)
+/datum/admin_rank/New(init_name, init_source, init_rights, init_exclude_rights, init_edit_rights)
 	if(IsAdminAdvancedProcCall())
 		alert_to_permissions_elevation_attempt(usr)
 		if (name == "NoRank") //only del if this is a true creation (and not just a New() proc call), other wise trialmins/coders could abuse this to deadmin other admins
@@ -19,6 +26,10 @@ GLOBAL_PROTECT(protected_ranks)
 			CRASH("Admin proc call creation of admin datum")
 		return
 	name = init_name
+	source = init_source
+	if(!source)
+		qdel(src)
+		CRASH("Admin rank created without a source.")
 	if(!name)
 		qdel(src)
 		CRASH("Admin rank created without name.")
@@ -102,11 +113,24 @@ GLOBAL_PROTECT(protected_ranks)
 			if(3)
 				can_edit_rights |= flag
 
+/datum/admin_rank/proc/pretty_print_source()
+	switch(source)
+		if(RANK_SOURCE_LOCAL)
+			return "Localhost"
+		if(RANK_SOURCE_TXT)
+			return "admin_ranks.txt"
+		if(RANK_SOURCE_DB)
+			return "Database"
+		if(RANK_SOURCE_BACKUP)
+			return "Backup JSON"
+		if(RANK_SOURCE_TEMPORARY)
+			return "Temporary"
+
 /// Loads admin ranks.
 ///	Return a list containing the backup data if they were loaded from the database backup json
 /proc/load_admin_ranks(dbfail, no_update)
 	if(IsAdminAdvancedProcCall())
-		to_chat(usr, "<span class='admin prefix'>Admin Reload blocked: Advanced ProcCall detected.</span>", confidential = TRUE)
+		to_chat(usr, span_adminprefix("Admin Reload blocked: Advanced ProcCall detected."), confidential = TRUE)
 		return
 	GLOB.admin_ranks.Cut()
 	GLOB.protected_ranks.Cut()
@@ -115,17 +139,17 @@ GLOBAL_PROTECT(protected_ranks)
 	var/datum/admin_rank/previous_rank
 	var/regex/admin_ranks_regex = new(@"^Name\s*=\s*(.+?)\s*\n+Include\s*=\s*([\l @]*?)\s*\n+Exclude\s*=\s*([\l @]*?)\s*\n+Edit\s*=\s*([\l @]*?)\s*\n*$", "gm")
 	while(admin_ranks_regex.Find(ranks_text))
-		var/datum/admin_rank/R = new(admin_ranks_regex.group[1])
-		if(!R)
+		var/datum/admin_rank/txt_rank = new(admin_ranks_regex.group[1], RANK_SOURCE_TXT)
+		if(QDELETED(txt_rank))
 			continue
 		var/count = 1
 		for(var/i in admin_ranks_regex.group - admin_ranks_regex.group[1])
 			if(i)
-				R.process_keyword(i, count, previous_rank)
+				txt_rank.process_keyword(i, count, previous_rank)
 			count++
-		GLOB.admin_ranks += R
-		GLOB.protected_ranks += R
-		previous_rank = R
+		GLOB.admin_ranks += txt_rank
+		GLOB.protected_ranks += txt_rank
+		previous_rank = txt_rank
 	if(!CONFIG_GET(flag/admin_legacy_system) && !dbfail)
 		if(CONFIG_GET(flag/load_legacy_ranks_only))
 			if(!no_update)
@@ -144,14 +168,15 @@ GLOBAL_PROTECT(protected_ranks)
 						if(R.name == rank_name) //this rank was already loaded from txt override
 							skip = 1
 							break
-					if(!skip)
-						var/rank_flags = text2num(query_load_admin_ranks.item[2])
-						var/rank_exclude_flags = text2num(query_load_admin_ranks.item[3])
-						var/rank_can_edit_flags = text2num(query_load_admin_ranks.item[4])
-						var/datum/admin_rank/R = new(rank_name, rank_flags, rank_exclude_flags, rank_can_edit_flags)
-						if(!R)
-							continue
-						GLOB.admin_ranks += R
+					if(skip)
+						continue
+					var/rank_flags = text2num(query_load_admin_ranks.item[2])
+					var/rank_exclude_flags = text2num(query_load_admin_ranks.item[3])
+					var/rank_can_edit_flags = text2num(query_load_admin_ranks.item[4])
+					var/datum/admin_rank/db_rank = new(rank_name, RANK_SOURCE_DB, rank_flags, rank_exclude_flags, rank_can_edit_flags)
+					if(QDELETED(db_rank))
+						continue
+					GLOB.admin_ranks += db_rank
 			qdel(query_load_admin_ranks)
 	//load ranks from backup file
 	if(dbfail)
@@ -167,10 +192,10 @@ GLOBAL_PROTECT(protected_ranks)
 					skip = TRUE
 			if(skip)
 				continue
-			var/datum/admin_rank/R = new("[J]", json["ranks"]["[J]"]["include rights"], json["ranks"]["[J]"]["exclude rights"], json["ranks"]["[J]"]["can edit rights"])
-			if(!R)
+			var/datum/admin_rank/json_rank = new("[J]", RANK_SOURCE_BACKUP, json["ranks"]["[J]"]["include rights"], json["ranks"]["[J]"]["exclude rights"], json["ranks"]["[J]"]["can edit rights"])
+			if(QDELETED(json_rank))
 				continue
-			GLOB.admin_ranks += R
+			GLOB.admin_ranks += json_rank
 		return json
 	#ifdef TESTING
 	var/msg = "Permission Sets Built:\n"
@@ -307,7 +332,7 @@ GLOBAL_PROTECT(protected_ranks)
 	set waitfor = FALSE
 
 	if(IsAdminAdvancedProcCall())
-		to_chat(usr, "<span class='admin prefix'>Admin rank DB Sync blocked: Advanced ProcCall detected.</span>", confidential = TRUE)
+		to_chat(usr, span_adminprefix("Admin rank DB Sync blocked: Advanced ProcCall detected."), confidential = TRUE)
 		return
 
 	var/list/sql_ranks = list()
@@ -351,7 +376,7 @@ GLOBAL_PROTECT(protected_ranks)
 
 /proc/sync_admins_with_db()
 	if(IsAdminAdvancedProcCall())
-		to_chat(usr, "<span class='admin prefix'>Admin rank DB Sync blocked: Advanced ProcCall detected.</span>")
+		to_chat(usr, span_adminprefix("Admin rank DB Sync blocked: Advanced ProcCall detected."))
 		return
 
 	if(CONFIG_GET(flag/admin_legacy_system) || !SSdbcore.IsConnected()) //we're already using legacy system so there's nothing to save
@@ -369,7 +394,7 @@ GLOBAL_PROTECT(protected_ranks)
 
 /proc/save_admin_backup()
 	if(IsAdminAdvancedProcCall())
-		to_chat(usr, "<span class='admin prefix'>Admin rank DB Sync blocked: Advanced ProcCall detected.</span>")
+		to_chat(usr, span_adminprefix("Admin rank DB Sync blocked: Advanced ProcCall detected."))
 		return
 
 	if(CONFIG_GET(flag/admin_legacy_system)) //we're already using legacy system so there's nothing to save

code/modules/admin/known_alts.dm

@@ -48,7 +48,7 @@ GLOBAL_DATUM_INIT(known_alts, /datum/known_alts, new)
 				return
 
 			var/already_exists_row = query_already_exists.NextRow()
-			qdel(query_already_exists)
+			QDEL_NULL(query_already_exists)
 
 			if (already_exists_row)
 				alert(usr, "Those two are already in the list of known alts!")
@@ -71,7 +71,7 @@ GLOBAL_DATUM_INIT(known_alts, /datum/known_alts, new)
 				cached_known_alts = null
 				load_known_alts()
 
-			qdel(query_add_known_alt)
+			QDEL_NULL(query_add_known_alt)
 			show_panel(usr.client)
 
 			if (!is_banned_from(ckey2, "Server"))
@@ -101,7 +101,7 @@ GLOBAL_DATUM_INIT(known_alts, /datum/known_alts, new)
 				return
 
 			var/list/result = query_known_alt_info.item
-			qdel(query_known_alt_info)
+			QDEL_NULL(query_known_alt_info)
 
 			if (alert("Are you sure you want to delete the alt connection between [result[1]] and [result[2]]?",,"Yes", "No") != "Yes")
 				return
@@ -121,7 +121,7 @@ GLOBAL_DATUM_INIT(known_alts, /datum/known_alts, new)
 				cached_known_alts = null
 				load_known_alts()
 
-			qdel(query_delete_known_alt)
+			QDEL_NULL(query_delete_known_alt)
 			show_panel(usr.client)
 
 /// Returns the list of known alts, will return an empty list if the DB could not be connected to.
@@ -152,8 +152,8 @@ GLOBAL_DATUM_INIT(known_alts, /datum/known_alts, new)
 			query_known_alts.item[1],
 		))
 
+	QDEL_NULL(query_known_alts)
 	COOLDOWN_START(src, cache_cooldown, 10 SECONDS)
-	qdel(query_known_alts)
 
 	return cached_known_alts
 

code/modules/admin/sql_ban_system.dm

@@ -177,7 +177,7 @@
 		if(is_admin && !text2num(query_build_ban_cache.item[2]))
 			continue
 		ban_cache[query_build_ban_cache.item[1]] = TRUE
-	qdel(query_build_ban_cache)
+	QDEL_NULL(query_build_ban_cache)
 	if(QDELETED(player_client)) // Disconnected while working with the DB.
 		return
 	player_client.ban_cache = ban_cache

code/modules/admin/stickyban.dm

@@ -124,7 +124,7 @@
 					list("ckey" = ckey, "alt" = alt)
 				)
 				query_remove_stickyban_alt.warn_execute()
-				qdel(query_remove_stickyban_alt)
+				QDEL_NULL(query_remove_stickyban_alt)
 
 			log_admin_private("[key_name(usr)] has disassociated [alt] from [ckey]'s sticky ban")
 			message_admins(span_adminnotice("[key_name_admin(usr)] has disassociated [alt] from [ckey]'s sticky ban"))
@@ -158,7 +158,7 @@
 					list("reason" = reason, "ckey" = ckey)
 				)
 				query_edit_stickyban.warn_execute()
-				qdel(query_edit_stickyban)
+				QDEL_NULL(query_edit_stickyban)
 
 			log_admin_private("[key_name(usr)] has edited [ckey]'s sticky ban reason from [oldreason] to [reason]")
 			message_admins(span_adminnotice("[key_name_admin(usr)] has edited [ckey]'s sticky ban reason from [oldreason] to [reason]"))
@@ -208,7 +208,7 @@
 					list("ckey" = ckey, "alt" = alt)
 				)
 				query_exempt_stickyban_alt.warn_execute()
-				qdel(query_exempt_stickyban_alt)
+				QDEL_NULL(query_exempt_stickyban_alt)
 
 			log_admin_private("[key_name(usr)] has exempted [alt] from [ckey]'s sticky ban")
 			message_admins(span_adminnotice("[key_name_admin(usr)] has exempted [alt] from [ckey]'s sticky ban"))
@@ -258,7 +258,7 @@
 					list("ckey" = ckey, "alt" = alt)
 				)
 				query_unexempt_stickyban_alt.warn_execute()
-				qdel(query_unexempt_stickyban_alt)
+				QDEL_NULL(query_unexempt_stickyban_alt)
 
 			log_admin_private("[key_name(usr)] has unexempted [alt] from [ckey]'s sticky ban")
 			message_admins(span_adminnotice("[key_name_admin(usr)] has unexempted [alt] from [ckey]'s sticky ban"))

code/modules/admin/topic.dm

@@ -49,19 +49,28 @@
 		cmd_show_exp_panel(M.client)
 
 	else if(href_list["editrightsbrowser"])
-		edit_admin_permissions(0)
+		edit_admin_permissions(PERMISSIONS_PAGE_PERMISSIONS)
 
-	else if(href_list["editrightsbrowserlog"])
-		edit_admin_permissions(1, href_list["editrightstarget"], href_list["editrightsoperation"], href_list["editrightspage"])
+	else if(href_list["editrightsbrowserranks"])
+		if(href_list["editrightsaddrank"])
+			add_rank()
+		else if(href_list["editrightsremoverank"])
+			remove_rank(href_list["editrightsremoverank"])
+		else if(href_list["editrightseditrank"])
+			change_rank(href_list["editrightseditrank"])
+		edit_admin_permissions(PERMISSIONS_PAGE_RANKS)
 
-	if(href_list["editrightsbrowsermanage"])
+	else if(href_list["editrightsbrowserlogging"])
+		edit_admin_permissions(PERMISSIONS_PAGE_LOGGING, href_list["editrightslogtarget"], href_list["editrightslogactor"], href_list["editrightslogoperation"], href_list["editrightslogpage"])
+
+	if(href_list["editrightsbrowserhousekeep"])
 		if(href_list["editrightschange"])
 			change_admin_rank(ckey(href_list["editrightschange"]), href_list["editrightschange"], TRUE)
 		else if(href_list["editrightsremove"])
 			remove_admin(ckey(href_list["editrightsremove"]), href_list["editrightsremove"], TRUE)
 		else if(href_list["editrightsremoverank"])
 			remove_rank(href_list["editrightsremoverank"])
-		edit_admin_permissions(2)
+		edit_admin_permissions(PERMISSIONS_PAGE_HOUSEKEEPING)
 
 	else if(href_list["editrights"])
 		edit_rights_topic(href_list)