Authentication flow: move construction, slim down classes (#5312)

- Remove extra members in ForgeHandshake

src/ClientHandle.cpp

@@ -67,7 +67,6 @@ float cClientHandle::FASTBREAK_PERCENTAGE;
 // cClientHandle:
 
 cClientHandle::cClientHandle(const AString & a_IPString, int a_ViewDistance) :
-	m_ForgeHandshake(this),
 	m_CurrentViewDistance(a_ViewDistance),
 	m_RequestedViewDistance(a_ViewDistance),
 	m_IPString(a_IPString),
@@ -334,7 +333,7 @@ bool cClientHandle::BungeeAuthenticate()
 
 
 
-void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
+void cClientHandle::Authenticate(AString && a_Name, const cUUID & a_UUID, Json::Value && a_Properties)
 {
 	{
 		cCSLock lock(m_CSState);
@@ -356,7 +355,7 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c
 			return;
 		}
 
-		m_Username = a_Name;
+		m_Username = std::move(a_Name);
 
 		// Only assign UUID and properties if not already pre-assigned (BungeeCord sends those in the Handshake packet):
 		if (m_UUID.IsNil())
@@ -365,19 +364,19 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c
 		}
 		if (m_Properties.empty())
 		{
-			m_Properties = a_Properties;
+			m_Properties = std::move(a_Properties);
 		}
 
 		// Send login success (if the protocol supports it):
 		m_Protocol->SendLoginSuccess();
 
-		if (m_ForgeHandshake.m_IsForgeClient)
+		if (m_ForgeHandshake.IsForgeClient)
 		{
-			m_ForgeHandshake.BeginForgeHandshake(a_Name, a_UUID, a_Properties);
+			m_ForgeHandshake.BeginForgeHandshake(*this);
 		}
 		else
 		{
-			FinishAuthenticate(a_Name, a_UUID, a_Properties);
+			FinishAuthenticate();
 		}
 	}
 }
@@ -386,7 +385,7 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c
 
 
 
-void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
+void cClientHandle::FinishAuthenticate()
 {
 	// Serverside spawned player (so data are loaded).
 	std::unique_ptr<cPlayer> Player;
@@ -422,8 +421,8 @@ void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_U
 
 	if (!cRoot::Get()->GetPluginManager()->CallHookPlayerJoined(*m_Player))
 	{
-		cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", a_Name.c_str()));
+		cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", m_Username.c_str()));
-		LOGINFO("Player %s has joined the game", a_Name.c_str());
+		LOGINFO("Player %s has joined the game", m_Username.c_str());
 	}
 
 	// TODO: this accesses the world spawn from the authenticator thread
@@ -747,7 +746,7 @@ bool cClientHandle::HandleLogin()
 	}  // lock(m_CSState)
 
 	// Schedule for authentication; until then, let the player wait (but do not block)
-	cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), GetUsername(), m_Protocol->GetAuthServerID());
+	cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), std::move(m_Username), m_Protocol->GetAuthServerID());
 	return true;
 }
 
@@ -887,7 +886,7 @@ void cClientHandle::HandlePluginMessage(const AString & a_Channel, const Contigu
 	}
 	else if (a_Channel == "FML|HS")
 	{
-		m_ForgeHandshake.DataReceived(this, a_Message);
+		m_ForgeHandshake.DataReceived(*this, a_Message);
 	}
 	else if (!HasPluginChannel(a_Channel))
 	{

src/ClientHandle.h

@@ -121,8 +121,8 @@ public:  // tolua_export
 	/** Authenticates the specified user with the bungee proxy server */
 	bool BungeeAuthenticate();
 
-	/** Authenticates the specified user, called by cAuthenticator */
+	/** Authenticates ourselves, called by cAuthenticator supplying player details from Mojang. */
-	void Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties);
+	void Authenticate(AString && a_Name, const cUUID & a_UUID, Json::Value && a_Properties);
 
 	/** Sends a set number of new chunks to the player on every invocation, until all chunks in the view distance have been sent. */
 	void StreamNextChunks();
@@ -276,20 +276,20 @@ public:  // tolua_export
 	const AStringMap & GetForgeMods(void) const { return m_ForgeMods; }
 
 	/** Returns true if the client is modded with Forge. */
-	bool IsForgeClient(void) const { return m_ForgeHandshake.m_IsForgeClient; }
+	bool IsForgeClient(void) const { return m_ForgeHandshake.IsForgeClient; }
 
 	// tolua_end
 
 	/** Add the Forge mod list to the server ping response. */
 	void ForgeAugmentServerListPing(Json::Value & a_Response)
 	{
-		m_ForgeHandshake.AugmentServerListPing(a_Response);
+		m_ForgeHandshake.AugmentServerListPing(*this, a_Response);
 	}
 
 	/** Mark a client connection as using Forge. Set by the protocol. */
 	void SetIsForgeClient()
 	{
-		m_ForgeHandshake.m_IsForgeClient = true;
+		m_ForgeHandshake.IsForgeClient = true;
 	}
 
 	/** Returns true if the client wants the chunk specified to be sent (in m_ChunksToSend) */
@@ -418,8 +418,6 @@ public:  // tolua_export
 
 private:
 
-	friend class cServer;  // Needs access to SetSelf()
-
 	friend class cForgeHandshake;   // Needs access to FinishAuthenticate()
 
 	/** The type used for storing the names of registered plugin channels. */
@@ -573,7 +571,7 @@ private:
 	float m_BreakProgress;
 
 	/** Finish logging the user in after authenticating. */
-	void FinishAuthenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties);
+	void FinishAuthenticate();
 
 	/** Returns true if the rate block interactions is within a reasonable limit (bot protection) */
 	bool CheckBlockInteractionsRate(void);

src/Protocol/Authenticator.cpp

@@ -57,17 +57,17 @@ void cAuthenticator::ReadSettings(cSettingsRepositoryInterface & a_Settings)
 
 
 
-void cAuthenticator::Authenticate(int a_ClientID, const AString & a_UserName, const AString & a_ServerHash)
+void cAuthenticator::Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash)
 {
 	if (!m_ShouldAuthenticate)
 	{
-		Json::Value Value;
+		const auto UUID = cClientHandle::GenerateOfflineUUID(a_Username);
-		cRoot::Get()->AuthenticateUser(a_ClientID, a_UserName, cClientHandle::GenerateOfflineUUID(a_UserName), Value);
+		cRoot::Get()->GetServer()->AuthenticateUser(a_ClientID, std::move(a_Username), UUID, Json::Value{});
 		return;
 	}
 
-	cCSLock LOCK(m_CS);
+	cCSLock Lock(m_CS);
-	m_Queue.emplace_back(a_ClientID, a_UserName, a_ServerHash);
+	m_Queue.emplace_back(a_ClientID, std::move(a_Username), a_ServerHash);
 	m_QueueNonempty.Set();
 }
 
@@ -112,20 +112,19 @@ void cAuthenticator::Execute(void)
 		}
 		ASSERT(!m_Queue.empty());
 
-		cAuthenticator::cUser & User = m_Queue.front();
+		cAuthenticator::cUser User = std::move(m_Queue.front());
-		int ClientID = User.m_ClientID;
+		int & ClientID = User.m_ClientID;
-		AString UserName = User.m_Name;
+		AString & UserName = User.m_Name;
-		AString ServerID = User.m_ServerID;
+		AString & ServerID = User.m_ServerID;
 		m_Queue.pop_front();
 		Lock.Unlock();
 
-		AString NewUserName = UserName;
 		cUUID UUID;
 		Json::Value Properties;
-		if (AuthWithYggdrasil(NewUserName, ServerID, UUID, Properties))
+		if (AuthWithYggdrasil(UserName, ServerID, UUID, Properties))
 		{
-			LOGINFO("User %s authenticated with UUID %s", NewUserName.c_str(), UUID.ToShortString().c_str());
+			LOGINFO("User %s authenticated with UUID %s", UserName.c_str(), UUID.ToShortString().c_str());
-			cRoot::Get()->AuthenticateUser(ClientID, NewUserName, UUID, Properties);
+			cRoot::Get()->GetServer()->AuthenticateUser(ClientID, std::move(UserName), UUID, std::move(Properties));
 		}
 		else
 		{

src/Protocol/Authenticator.h

@@ -41,7 +41,7 @@ public:
 	void ReadSettings(cSettingsRepositoryInterface & a_Settings);
 
 	/** Queues a request for authenticating a user. If the auth fails, the user will be kicked */
-	void Authenticate(int a_ClientID, const AString & a_UserName, const AString & a_ServerHash);
+	void Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash);
 
 	/** Starts the authenticator thread. The thread may be started and stopped repeatedly */
 	void Start(cSettingsRepositoryInterface & a_Settings);

src/Protocol/ForgeHandshake.cpp

@@ -44,10 +44,9 @@ namespace ServerPhase
 
 
 
-cForgeHandshake::cForgeHandshake(cClientHandle *a_Client) :
+cForgeHandshake::cForgeHandshake() :
-	m_IsForgeClient(false),
+	IsForgeClient(false),
-	m_Errored(false),
+	m_Errored(false)
-	m_Client(a_Client)
 {
 }
 
@@ -65,9 +64,9 @@ void cForgeHandshake::SetError(const AString & message)
 
 
 
-void cForgeHandshake::AugmentServerListPing(Json::Value & a_ResponseValue)
+void cForgeHandshake::AugmentServerListPing(cClientHandle & a_Client, Json::Value & a_ResponseValue)
 {
-	auto ProtocolVersion = m_Client->GetProtocolVersion();
+	auto ProtocolVersion = a_Client.GetProtocolVersion();
 	auto & Mods = cRoot::Get()->GetServer()->GetRegisteredForgeMods(ProtocolVersion);
 
 	if (Mods.empty())
@@ -97,13 +96,9 @@ void cForgeHandshake::AugmentServerListPing(Json::Value & a_ResponseValue)
 
 
 
-void cForgeHandshake::BeginForgeHandshake(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
+void cForgeHandshake::BeginForgeHandshake(cClientHandle & a_Client)
 {
-	ASSERT(m_IsForgeClient);
+	ASSERT(IsForgeClient);
-
-	m_Name = a_Name;
-	m_UUID = a_UUID;
-	m_Properties = a_Properties;
 
 	static const std::array<std::string_view, 5> Channels{{ "FML|HS", "FML", "FML|MP", "FML", "FORGE" }};
 	ContiguousByteBuffer ChannelsString;
@@ -113,15 +108,15 @@ void cForgeHandshake::BeginForgeHandshake(const AString & a_Name, const cUUID &
 		ChannelsString.push_back(std::byte(0));
 	}
 
-	m_Client->SendPluginMessage("REGISTER", ChannelsString);
+	a_Client.SendPluginMessage("REGISTER", ChannelsString);
-	SendServerHello();
+	SendServerHello(a_Client);
 }
 
 
 
 
 
-void cForgeHandshake::SendServerHello()
+void cForgeHandshake::SendServerHello(cClientHandle & a_Client)
 {
 	cByteBuffer Buf(6);
 	// Discriminator | Byte | Always 0 for ServerHello
@@ -134,7 +129,7 @@ void cForgeHandshake::SendServerHello()
 	ContiguousByteBuffer Message;
 	Buf.ReadAll(Message);
 
-	m_Client->SendPluginMessage("FML|HS", Message);
+	a_Client.SendPluginMessage("FML|HS", Message);
 }
 
 
@@ -183,7 +178,7 @@ AStringMap cForgeHandshake::ParseModList(const ContiguousByteBufferView a_Data)
 
 
 
-void cForgeHandshake::HandleClientHello(cClientHandle * a_Client, const ContiguousByteBufferView a_Data)
+void cForgeHandshake::HandleClientHello(cClientHandle & a_Client, const ContiguousByteBufferView a_Data)
 {
 	if (a_Data.size() == 2)
 	{
@@ -204,7 +199,7 @@ void cForgeHandshake::HandleClientHello(cClientHandle * a_Client, const Contiguo
 
 
 
-void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousByteBufferView a_Data)
+void cForgeHandshake::HandleModList(cClientHandle & a_Client, const ContiguousByteBufferView a_Data)
 {
 	LOGD("Received ModList");
 
@@ -217,10 +212,10 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy
 
 	LOG("Client connected with %zu mods: %s", ClientMods.size(), ClientModsString.c_str());
 
-	m_Client->m_ForgeMods = ClientMods;
+	a_Client.m_ForgeMods = ClientMods;
 
 	// Let the plugins know about this event, they may refuse the player:
-	if (cRoot::Get()->GetPluginManager()->CallHookLoginForge(*a_Client, ClientMods))
+	if (cRoot::Get()->GetPluginManager()->CallHookLoginForge(a_Client, ClientMods))
 	{
 		SetError("Modded client refused by plugin");
 		return;
@@ -229,7 +224,7 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy
 	// Send server ModList
 
 	// Send server-side Forge mods registered by plugins
-	const auto & ServerMods = m_Client->GetForgeMods();
+	const auto & ServerMods = a_Client.GetForgeMods();
 
 	const auto ModCount = ServerMods.size();
 
@@ -246,14 +241,14 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy
 	ContiguousByteBuffer ServerModList;
 	Buf.ReadAll(ServerModList);
 
-	m_Client->SendPluginMessage("FML|HS", ServerModList);
+	a_Client.SendPluginMessage("FML|HS", ServerModList);
 }
 
 
 
 
 
-void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const ContiguousByteBufferView a_Data)
+void cForgeHandshake::HandleHandshakeAck(cClientHandle & a_Client, const ContiguousByteBufferView a_Data)
 {
 	if (a_Data.size() != 2)
 	{
@@ -286,7 +281,7 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu
 
 			ContiguousByteBuffer RegistryData;
 			Buf.ReadAll(RegistryData);
-			m_Client->SendPluginMessage("FML|HS", RegistryData);
+			a_Client.SendPluginMessage("FML|HS", RegistryData);
 			break;
 		}
 
@@ -297,7 +292,7 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu
 			ContiguousByteBuffer Ack;
 			Ack.push_back(std::byte(Discriminator::HandshakeAck));
 			Ack.push_back(ServerPhase::WAITINGCACK);
-			m_Client->SendPluginMessage("FML|HS", Ack);
+			a_Client.SendPluginMessage("FML|HS", Ack);
 			break;
 		}
 
@@ -308,15 +303,15 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu
 			ContiguousByteBuffer Ack;
 			Ack.push_back(std::byte(Discriminator::HandshakeAck));
 			Ack.push_back(ServerPhase::COMPLETE);
-			m_Client->SendPluginMessage("FML|HS", Ack);
+			a_Client.SendPluginMessage("FML|HS", Ack);
 
 			break;
 		}
 
 		case ClientPhase::COMPLETE:
 		{
-			// Now finish logging in
+			// Now finish logging in:
-			m_Client->FinishAuthenticate(m_Name, m_UUID, m_Properties);
+			a_Client.FinishAuthenticate();
 			break;
 		}
 
@@ -332,9 +327,9 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu
 
 
 
-void cForgeHandshake::DataReceived(cClientHandle * a_Client, const ContiguousByteBufferView a_Data)
+void cForgeHandshake::DataReceived(cClientHandle & a_Client, const ContiguousByteBufferView a_Data)
 {
-	if (!m_IsForgeClient)
+	if (!IsForgeClient)
 	{
 		SetError(Printf("Received unexpected Forge data from non-Forge client (%zu bytes)", a_Data.size()));
 		return;

src/Protocol/ForgeHandshake.h

@@ -9,6 +9,10 @@
 #include "../UUID.h"
 #include "json/json.h"
 
+
+
+
+
 // fwd:
 class cClientHandle;
 
@@ -19,38 +23,32 @@ class cClientHandle;
 class cForgeHandshake
 {
 public:
-	/** True if the client advertised itself as a Forge client. */
-	bool m_IsForgeClient;
 
-	cForgeHandshake(cClientHandle * client);
+	/** True if the client advertised itself as a Forge client. */
+	bool IsForgeClient;
+
+	cForgeHandshake();
 
 	/** Add the registered Forge mods to the server ping list packet. */
-	void AugmentServerListPing(Json::Value & ResponseValue);
+	void AugmentServerListPing(cClientHandle & a_Client, Json::Value & ResponseValue);
 
 	/** Begin the Forge Modloader Handshake (FML|HS) sequence. */
-	void BeginForgeHandshake(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties);
+	void BeginForgeHandshake(cClientHandle & a_Client);
 
 	/** Send the ServerHello packet in the Forge handshake. */
-	void SendServerHello();
+	void SendServerHello(cClientHandle & a_Client);
 
 	/** Process received data from the client advancing the Forge handshake. */
-	void DataReceived(cClientHandle * a_Client, ContiguousByteBufferView a_Data);
+	void DataReceived(cClientHandle & a_Client, ContiguousByteBufferView a_Data);
 
 private:
+
 	/** True if the Forge handshake is in an errored state. */
 	bool m_Errored;
 
-	/** The client handle undergoing this Forge handshake. */
+	void HandleClientHello(cClientHandle & a_Client, ContiguousByteBufferView a_Data);
-	cClientHandle * m_Client;
+	void HandleModList(cClientHandle & a_Client, ContiguousByteBufferView a_Data);
-
+	void HandleHandshakeAck(cClientHandle & a_Client, ContiguousByteBufferView a_Data);
-	/** Values saved from BeginForgeHandshake() for continuing the normal handshake after Forge completes. */
-	AString m_Name;
-	cUUID m_UUID;
-	Json::Value m_Properties;
-
-	void HandleClientHello(cClientHandle * a_Client, ContiguousByteBufferView a_Data);
-	void HandleModList(cClientHandle * a_Client, ContiguousByteBufferView a_Data);
-	void HandleHandshakeAck(cClientHandle * a_Client, ContiguousByteBufferView a_Data);
 
 	/** Set errored state to prevent further handshake message processing. */
 	void SetError(const AString & message);

src/Protocol/Protocol_1_8.cpp

@@ -2161,6 +2161,7 @@ void cProtocol_1_8_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer)
 	auto NumPlayers = static_cast<signed>(Server->GetNumPlayers());
 	auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
 	AString Favicon = Server->GetFaviconData();
+
 	cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);
 
 	// Version:
@@ -2177,7 +2178,7 @@ void cProtocol_1_8_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer)
 
 	// Description:
 	Json::Value Description;
-	Description["text"] = ServerDescription.c_str();
+	Description["text"] = std::move(ServerDescription);
 
 	// Create the response:
 	Json::Value ResponseValue;

src/Root.cpp

@@ -558,15 +558,6 @@ void cRoot::KickUser(int a_ClientID, const AString & a_Reason)
 
 
 
-void cRoot::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
-{
-	m_Server->AuthenticateUser(a_ClientID, a_Name, a_UUID, a_Properties);
-}
-
-
-
-
-
 size_t cRoot::GetTotalChunkCount(void)
 {
 	size_t Count = 0;

src/Root.h

@@ -130,9 +130,6 @@ public:
 	/** Kicks the user, no matter in what world they are. Used from cAuthenticator */
 	void KickUser(int a_ClientID, const AString & a_Reason);
 
-	/** Called by cAuthenticator to auth the specified user */
-	void AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties);
-
 	/** Returns the number of chunks loaded */
 	size_t GetTotalChunkCount(void);  // tolua_export
 

src/Server.cpp

@@ -709,7 +709,7 @@ void cServer::KickUser(int a_ClientID, const AString & a_Reason)
 
 
 
-void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties)
+void cServer::AuthenticateUser(int a_ClientID, AString && a_Username, const cUUID & a_UUID, Json::Value && a_Properties)
 {
 	cCSLock Lock(m_CSClients);
 
@@ -720,14 +720,14 @@ void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUU
 		return;
 	}
 
-	for (auto itr = m_Clients.begin(); itr != m_Clients.end(); ++itr)
+	for (const auto & Client : m_Clients)
 	{
-		if ((*itr)->GetUniqueID() == a_ClientID)
+		if (Client->GetUniqueID() == a_ClientID)
 		{
-			(*itr)->Authenticate(a_Name, a_UUID, a_Properties);
+			Client->Authenticate(std::move(a_Username), a_UUID, std::move(a_Properties));
 			return;
 		}
-	}  // for itr - m_Clients[]
+	}
 }
 
 

src/Server.h

@@ -118,8 +118,8 @@ public:
 
 	void KickUser(int a_ClientID, const AString & a_Reason);
 
-	/** Authenticates the specified user, called by cAuthenticator */
+	/** Authenticates the specified user, called by cAuthenticator supplying player details from Mojang. */
-	void AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties);
+	void AuthenticateUser(int a_ClientID, AString && a_Username, const cUUID & a_UUID, Json::Value && a_Properties);
 
 	const AString & GetServerID(void) const { return m_ServerID; }  // tolua_export