Refactor admin status

Store as a per-player flag, to avoid costly processing with every network message

lib/netplay/netplay.cpp

@@ -674,6 +674,7 @@ void NET_InitPlayer(uint32_t i, bool initPosition, bool initTeams, bool initSpec
 	{
 		NetPlay.players[i].isSpectator = false;
 	}
+	NetPlay.players[i].isAdmin = false;
 }
 
 uint8_t NET_numHumanPlayers(void)
@@ -742,6 +743,7 @@ static void NETSendNPlayerInfoTo(uint32_t *index, uint32_t indexLen, unsigned to
 		NETint8_t(reinterpret_cast<int8_t*>(&NetPlay.players[index[n]].difficulty));
 		NETuint8_t(reinterpret_cast<uint8_t *>(&NetPlay.players[index[n]].faction));
 		NETbool(&NetPlay.players[index[n]].isSpectator);
+		NETbool(&NetPlay.players[index[n]].isAdmin);
 	}
 	NETend();
 	ActivityManager::instance().updateMultiplayGameData(game, ingame, NETGameIsLocked());
@@ -2542,6 +2544,7 @@ static bool NETprocessSystemMessage(NETQUEUE playerQueue, uint8_t *type)
 			int8_t difficulty = 0;
 			uint8_t faction = FACTION_NORMAL;
 			bool isSpectator = false;
+			bool isAdmin = false;
 			bool error = false;
 
 			NETbeginDecode(playerQueue, NET_PLAYER_INFO);
@@ -2586,6 +2589,7 @@ static bool NETprocessSystemMessage(NETQUEUE playerQueue, uint8_t *type)
 				NETint8_t(&difficulty);
 				NETuint8_t(&faction);
 				NETbool(&isSpectator);
+				NETbool(&isAdmin);
 
 				auto newFactionId = uintToFactionID(faction);
 				if (!newFactionId.has_value())
@@ -2605,6 +2609,7 @@ static bool NETprocessSystemMessage(NETQUEUE playerQueue, uint8_t *type)
 					NetPlay.players[index].difficulty = static_cast<AIDifficulty>(difficulty);
 					NetPlay.players[index].faction = newFactionId.value();
 					NetPlay.players[index].isSpectator = isSpectator;
+					NetPlay.players[index].isAdmin = isAdmin;
 				}
 
 				debug(LOG_NET, "%s for player %u (%s)", n == 0 ? "Receiving MSG_PLAYER_INFO" : "                      and", (unsigned int)index, NetPlay.players[index].allocated ? "human" : "AI");
@@ -4294,12 +4299,9 @@ static void NETallowJoining()
 
 			NEThostPromoteTempSocketToPermanentPlayerConnection(i, index);
 
-			bool isAdminJoining = identityMatchesAdmin(joinRequestInfo.identity);
-
 			NETbeginEncode(NETnetQueue(index), NET_ACCEPTED);
 			NETuint8_t(&index);
 			NETuint32_t(&NetPlay.hostPlayer);
-			NETbool(&isAdminJoining);
 			NETend();
 
 			// First send info about players to newcomer.

lib/netplay/netplay.h

@@ -275,6 +275,7 @@ struct PLAYER
 	bool                autoGame;           ///< if we are running a autogame (AI controls us)
 	FactionID			faction;			///< which faction the player has
 	bool				isSpectator;		///< whether this slot is a spectator slot
+	bool				isAdmin;			///< whether this slot has admin privs
 
 	// used on host-ONLY (not transmitted to other clients):
 	std::shared_ptr<std::vector<WZFile>> wzFiles = std::make_shared<std::vector<WZFile>>();            ///< for each player, we keep track of map/mod download progress
@@ -302,6 +303,7 @@ struct PLAYER
 		IPtextAddress[0] = '\0';
 		faction = FACTION_NORMAL;
 		isSpectator = false;
+		isAdmin = false;
 	}
 };
 
@@ -316,7 +318,6 @@ struct NETPLAY
 	uint32_t	hostPlayer;		///< Index of host in player array
 	bool		bComms;			///< Actually do the comms?
 	bool		isHost;			///< True if we are hosting the game
-	bool		isAdmin;		///< True if we are promoted to admin by the host
 	bool		isPortMappingEnabled;				// if we want the automatic Port mapping setup routines to run
 	bool		isHostAlive;	/// if the host is still alive
 	char gamePassword[password_string_size];		//

src/multiint.cpp

@@ -1443,13 +1443,18 @@ static void addGameOptions()
 	updateLimitIcons();
 }
 
+static bool isHostOrAdmin()
+{
+	return NetPlay.isHost || NetPlay.players[selectedPlayer].isAdmin;
+}
+
 // ////////////////////////////////////////////////////////////////////////////
 // Colour functions
 
 static bool safeToUseColour(unsigned player, unsigned otherPlayer)
 {
 	// Player wants to take the colour from otherPlayer. May not take from a human otherPlayer, unless we're the host/admin.
-	return player == otherPlayer || (NetPlay.isHost || NetPlay.isAdmin) || !isHumanPlayer(otherPlayer);
+	return player == otherPlayer || isHostOrAdmin() || !isHumanPlayer(otherPlayer);
 }
 
 static int getPlayerTeam(int i)
@@ -1982,7 +1987,7 @@ static std::set<uint32_t> validPlayerIdxTargetsForPlayerPositionMove(uint32_t pl
 	for (uint32_t i = 0; i < game.maxPlayers; i++)
 	{
 		if (player != i
-			&& (NetPlay.isHost || (!isHumanPlayer(i) || NetPlay.isAdmin)) // host/admin can move a player to any slot, player can only move to empty slots
+			&& (isHostOrAdmin() || !isHumanPlayer(i)) // host/admin can move a player to any slot, player can only move to empty slots
 			&& !isSpectatorOnlySlot(i)) // target cannot be a spectator only slot (for player position changes)
 		{
 			validTargetPlayerIdx.insert(i);
@@ -2114,7 +2119,7 @@ void WzMultiplayerOptionsTitleUI::openTeamChooser(uint32_t player)
 
 	UDWORD i;
 	int disallow = allPlayersOnSameTeam(player);
-	if (bIsTrueMultiplayerGame && (NetPlay.isHost || NetPlay.isAdmin))
+	if (bIsTrueMultiplayerGame && isHostOrAdmin())
 	{
 		// allow configuration of all teams in true multiplayer mode (by host/admin), even if they would block the game starting
 		// (i.e. even if all players would be configured to be on the same team)
@@ -2513,18 +2518,18 @@ bool recvTeamRequest(NETQUEUE queue)
 		return false;
 	}
 
-	if (whosResponsible(player) != queue.index && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (whosResponsible(player) != queue.index && !NetPlay.players[queue.index].isAdmin)
 	{
 		HandleBadParam("NET_TEAMREQUEST given incorrect params.", player, queue.index);
 		return false;
 	}
 
-	if (locked.teams && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (locked.teams)
 	{
 		return false;
 	}
 
-	if (senderHasLobbyCommandAdminPrivs(queue.index) && (queue.index != player || locked.teams))
+	if (NetPlay.players[queue.index].isAdmin && (queue.index != player))
 	{
 		sendRoomSystemMessage(astringf("Admin %s changed team of player [%d] %s to %d",
 			NetPlay.players[queue.index].name,
@@ -2631,6 +2636,7 @@ bool changeReadyStatus(UBYTE player, bool bReady)
 
 static bool changePosition(UBYTE player, UBYTE position)
 {
+	ASSERT_HOST_ONLY(return false);
 	ASSERT(player < MAX_PLAYERS, "Invalid player idx: %" PRIu8, player);
 	int i;
 
@@ -2782,7 +2788,7 @@ bool recvFactionRequest(NETQUEUE queue)
 		return false;
 	}
 
-	if (whosResponsible(player) != queue.index && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (whosResponsible(player) != queue.index && !NetPlay.players[queue.index].isAdmin)
 	{
 		HandleBadParam("NET_FACTIONREQUEST given incorrect params.", player, queue.index);
 		return false;
@@ -2795,7 +2801,7 @@ bool recvFactionRequest(NETQUEUE queue)
 		return false;
 	}
 
-	if (senderHasLobbyCommandAdminPrivs(queue.index) && queue.index != player)
+	if (NetPlay.players[queue.index].isAdmin && (queue.index != player))
 	{
 		sendRoomSystemMessage(astringf("Admin %s changed faction of player [%d] %s to %d",
 			NetPlay.players[queue.index].name,
@@ -2835,13 +2841,13 @@ bool recvColourRequest(NETQUEUE queue)
 		return false;
 	}
 
-	if (whosResponsible(player) != queue.index && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (whosResponsible(player) != queue.index && !NetPlay.players[queue.index].isAdmin)
 	{
 		HandleBadParam("NET_COLOURREQUEST given incorrect params.", player, queue.index);
 		return false;
 	}
 
-	if (senderHasLobbyCommandAdminPrivs(queue.index) && (queue.index != player))
+	if (NetPlay.players[queue.index].isAdmin && (queue.index != player))
 	{
 		sendRoomSystemMessage(astringf("Admin %s changed color of player [%d] %s to %d",
 			NetPlay.players[queue.index].name,
@@ -2858,7 +2864,7 @@ bool recvColourRequest(NETQUEUE queue)
 
 	resetReadyStatus(false, true);
 
-	return changeColour(player, col, senderHasLobbyCommandAdminPrivs(queue.index));
+	return changeColour(player, col, NetPlay.players[queue.index].isAdmin);
 }
 
 bool recvPositionRequest(NETQUEUE queue)
@@ -2880,18 +2886,18 @@ bool recvPositionRequest(NETQUEUE queue)
 		return false;
 	}
 
-	if (whosResponsible(player) != queue.index && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (whosResponsible(player) != queue.index && !NetPlay.players[queue.index].isAdmin)
 	{
 		HandleBadParam("NET_POSITIONREQUEST given incorrect params.", player, queue.index);
 		return false;
 	}
 
-	if (locked.position && !senderHasLobbyCommandAdminPrivs(queue.index))
+	if (locked.position)
 	{
 		return false;
 	}
 
-	if (senderHasLobbyCommandAdminPrivs(queue.index) && (queue.index != player || (locked.position)))
+	if (NetPlay.players[queue.index].isAdmin && (queue.index != player))
 	{
 		sendRoomSystemMessage(astringf("Admin %s changed position of player [%d] %s to %d",
 			NetPlay.players[queue.index].name,
@@ -3329,7 +3335,7 @@ static SwapPlayerIndexesResult recvSwapPlayerIndexes(NETQUEUE queue, const std::
 
 static bool canChooseTeamFor(int i)
 {
-	return (i == selectedPlayer || (NetPlay.isHost || NetPlay.isAdmin));
+	return (i == selectedPlayer || isHostOrAdmin());
 }
 
 // ////////////////////////////////////////////////////////////////////////////
@@ -4056,7 +4062,7 @@ class WzPlayerRow : public WIDGET
 		widget->colorButton->addOnClickHandler([playerIdx, titleUI](W_BUTTON& button){
 			auto strongTitleUI = titleUI.lock();
 			ASSERT_OR_RETURN(, strongTitleUI != nullptr, "Title UI is gone?");
-			if (playerIdx == selectedPlayer || (NetPlay.isHost || NetPlay.isAdmin))
+			if (playerIdx == selectedPlayer || isHostOrAdmin())
 			{
 				if (!NetPlay.players[playerIdx].isSpectator) // not a spectator
 				{
@@ -4091,13 +4097,13 @@ class WzPlayerRow : public WIDGET
 		widget->playerInfo->addOnClickHandler([playerIdx, titleUI](W_BUTTON& button){
 			auto strongTitleUI = titleUI.lock();
 			ASSERT_OR_RETURN(, strongTitleUI != nullptr, "Title UI is gone?");
-			if (playerIdx == selectedPlayer || (NetPlay.isHost || NetPlay.isAdmin))
+			if (playerIdx == selectedPlayer || isHostOrAdmin())
 			{
 				uint32_t player = playerIdx;
 				// host can move any player, clients can request to move themselves if there are available slots
 				// admins can move people as if they are host
 				if (((player == selectedPlayer && validPlayerIdxTargetsForPlayerPositionMove(player).size() > 0) ||
-						(NetPlay.players[player].allocated && (NetPlay.isHost || NetPlay.isAdmin)))
+						(NetPlay.players[player].allocated && isHostOrAdmin()))
 					&& !locked.position
 					&& player < MAX_PLAYERS
 					&& !isSpectatorOnlySlot(player))
@@ -8889,6 +8895,7 @@ inline void to_json(nlohmann::json& j, const PLAYER& p) {
 	// Do not persist IPtextAddress
 	j["faction"] = static_cast<uint8_t>(p.faction);
 	j["isSpectator"] = p.isSpectator;
+	j["isAdmin"] = p.isAdmin;
 }
 
 inline void from_json(const nlohmann::json& j, PLAYER& p) {
@@ -8919,6 +8926,15 @@ inline void from_json(const nlohmann::json& j, PLAYER& p) {
 	auto factionUint = j.at("faction").get<uint8_t>();
 	p.faction = static_cast<FactionID>(factionUint); // TODO CHECK
 	p.isSpectator = j.at("isSpectator").get<bool>();
+	if (j.contains("isAdmin"))
+	{
+		p.isAdmin = j.at("isAdmin").get<bool>();
+	}
+	else
+	{
+		// default to the old (pre-4.5.4) default value of false
+		p.isAdmin = false;
+	}
 }
 
 static nlohmann::json DataHashToJSON()

src/multilobbycommands.cpp

@@ -58,7 +58,8 @@ bool removeLobbyAdminPublicKey(const std::string& publicKeyB64Str)
 }
 
 // checks for specific identity being an admin
-bool identityMatchesAdmin(const EcKey& identity) {
+bool identityMatchesAdmin(const EcKey& identity)
+{
 	std::string senderIdentityHash = identity.publicHashString();
 	std::string senderPublicKeyB64 = base64Encode(identity.toBytes(EcKey::Public));
 	return lobbyAdminPublicKeys.count(senderPublicKeyB64) != 0 || lobbyAdminPublicHashStrings.count(senderIdentityHash) != 0;
@@ -82,18 +83,11 @@ static bool senderApparentlyMatchesAdmin(uint32_t playerIdx)
 	{
 		return false;
 	}
-	std::string senderIdentityHash = identity.publicHashString();
-	std::string senderPublicKeyB64 = base64Encode(identity.toBytes(EcKey::Public));
-	if (lobbyAdminPublicKeys.count(senderPublicKeyB64) == 0 && lobbyAdminPublicHashStrings.count(senderIdentityHash) == 0)
-	{
-		return false; // identity hash is not in permitted lists
-	}
-
-	return true;
+	return identityMatchesAdmin(identity);
 }
 
 // **THIS** is the function that should be used to determine whether a sender currently has permission to execute admin commands
-bool senderHasLobbyCommandAdminPrivs(uint32_t playerIdx)
+static bool senderHasLobbyCommandAdminPrivs(uint32_t playerIdx)
 {
 	if (playerIdx >= MAX_CONNECTED_PLAYERS)
 	{

src/multilobbycommands.h

@@ -53,7 +53,6 @@ void cmdInterfaceLogChatMsg(const NetworkTextMessage& message, const char* log_p
 bool processChatLobbySlashCommands(const NetworkTextMessage& message, HostLobbyOperationsInterface& cmdInterface);
 
 bool identityMatchesAdmin(const EcKey& identity);
-bool senderHasLobbyCommandAdminPrivs(uint32_t playerIdx);
 
 bool addLobbyAdminIdentityHash(const std::string& playerIdentityHash);
 bool removeLobbyAdminIdentityHash(const std::string& playerIdentityHash);

src/multiplay.cpp

@@ -1031,7 +1031,7 @@ HandleMessageAction getMessageHandlingAction(NETQUEUE& queue, uint8_t type)
 	}
 
 	bool senderIsSpectator = NetPlay.players[queue.index].isSpectator;
-	bool senderIsAdmin = senderHasLobbyCommandAdminPrivs(queue.index);
+	bool senderIsAdmin = NetPlay.players[queue.index].isAdmin;
 
 	if (type > NET_MIN_TYPE && type < NET_MAX_TYPE)
 	{
@@ -1050,15 +1050,20 @@ HandleMessageAction getMessageHandlingAction(NETQUEUE& queue, uint8_t type)
 				}
 				break;
 			case NET_KICK:
-			case NET_AITEXTMSG:
-			case NET_BEACONMSG:
 			case NET_TEAMREQUEST: // spectators should not be allowed to request a team / non-spectator slot status
 			case NET_POSITIONREQUEST:
 				if (senderIsSpectator && !senderIsAdmin)
 				{
 					return HandleMessageAction::Disallow_And_Kick_Sender;
 				}
 				break;
+			case NET_AITEXTMSG:
+			case NET_BEACONMSG:
+				if (senderIsSpectator)
+				{
+					return HandleMessageAction::Disallow_And_Kick_Sender;
+				}
+				break;
 			case NET_TEXTMSG:
 				// Normal chat messages are available to spectators in the game room / lobby chat, but *not* in-game
 				if (senderIsSpectator && GetGameMode() == GS_NORMAL)

src/multistat.cpp

@@ -40,6 +40,7 @@
 #include "multistat.h"
 #include "urlrequest.h"
 #include "stdinreader.h"
+#include "multilobbycommands.h"
 
 #include <utility>
 #include <memory>
@@ -377,12 +378,26 @@ bool multiStatsSetIdentity(uint32_t playerIndex, const EcKey::Key &identity, boo
 			{
 				// Record that the identity has been verified
 				ingame.VerifiedIdentity[playerIndex] = true;
+				if (NetPlay.isHost)
+				{
+					NetPlay.players[playerIndex].isAdmin = identityMatchesAdmin(playerStats[playerIndex].identity);
+					// Do not broadcast player info here, as it's assumed caller will do it
+				}
 
 				// Do *not* output to stdinterface here - the join event is still being processed
 			}
 			else
 			{
 				ingame.VerifiedIdentity[playerIndex] = false;
+				if (NetPlay.isHost)
+				{
+					// when verified identity is cleared, so is admin status (until new identity is verified)
+					if (NetPlay.players[playerIndex].isAdmin)
+					{
+						NetPlay.players[playerIndex].isAdmin = false;
+						NETBroadcastPlayerInfo(playerIndex);
+					}
+				}
 
 				// Output to stdinterface, if enabled
 				if (!identity.empty())