From a18f9dbe90b3fefc7310f1673f5d5d141e95d39d Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Fri, 16 Dec 2016 16:32:07 -0800 Subject: [PATCH] PR comments; need particular fix on Master for full QA --- domain-server/src/DomainServerSettingsManager.cpp | 15 +++------------ interface/resources/qml/hifi/Pal.qml | 6 +++--- libraries/networking/src/NodeList.cpp | 3 +-- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 6af8f12dfa..8da7d030cb 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -785,7 +785,6 @@ void DomainServerSettingsManager::processUsernameFromIDRequestPacket(QSharedPoin // From the packet, pull the UUID we're identifying QUuid nodeUUID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); - // If the UUID isn't NULL... if (!nodeUUID.isNull()) { // First, make sure we actually have a node with this UUID auto limitedNodeList = DependencyManager::get(); @@ -796,33 +795,25 @@ void DomainServerSettingsManager::processUsernameFromIDRequestPacket(QSharedPoin // It's time to figure out the username QString verifiedUsername = matchingNode->getPermissions().getVerifiedUserName(); - // If the verified username is Empty... if (verifiedUsername.isEmpty()) { // Make sure we're using an empty string as the Verified Username verifiedUsername = ""; } // Setup the packet auto usernameFromIDReplyPacket = NLPacket::create(PacketType::UsernameFromIDReply, NUM_BYTES_RFC4122_UUID + sizeof(verifiedUsername), true); - - // write the node ID to the packet usernameFromIDReplyPacket->write(nodeUUID.toRfc4122()); - // write the username to the packet usernameFromIDReplyPacket->writeString(verifiedUsername); // Ship it! limitedNodeList->sendPacket(std::move(usernameFromIDReplyPacket), *sendingNode); - } - else { + } else { qWarning() << "Node username request received for unknown node. Refusing to process."; } - } - else { - // This isn't a UUID we can use + } else { qWarning() << "Node username request received for invalid node ID. Refusing to process."; } - } - else { + } else { qWarning() << "Refusing to process a username request packet from node" << uuidStringWithoutCurlyBraces(sendingNode->getUUID()) << "that does not have kick permissions."; } diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index 62e7ba1571..7a8dc4722e 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -76,18 +76,18 @@ Rectangle { case 'updateUsername': // The User ID (UUID) is the first parameter in the message. var userId = message.params[0]; - // The Username String (name + UUID) is the second parameter in the message. + // The text that goes in the userName field is the second parameter in the message. var userName = message.params[1]; // If the userId is empty, we're updating "myData". if (!userId) { myData.userName = userName; - myCard.userName = userName; + myCard.userName = userName; // Defensive programming } else { // Get the index in userModel and userData associated with the passed UUID var userIndex = findSessionIndex(userId); // Set the userName appropriately userModel.get(userIndex).userName = userName; - userData[userIndex].userName = userName; + userData[userIndex].userName = userName; // Defensive programming } break; default: diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index a984da1648..f76189b13a 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -912,8 +912,7 @@ void NodeList::requestUsernameFromSessionID(const QUuid& nodeID) { qDebug() << "Sending packet to get username of node" << uuidStringWithoutCurlyBraces(nodeID); sendPacket(std::move(usernameFromIDRequestPacket), _domainHandler.getSockAddr()); - } - else { + } else { qWarning() << "You do not have permissions to kick in this domain." << "Request to get the username of node" << uuidStringWithoutCurlyBraces(nodeID) << "will not be sent"; } diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index ee42398666..d3347f8e53 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -44,7 +44,7 @@ const QSet NON_SOURCED_PACKETS = QSet() PacketVersion versionForPacketType(PacketType packetType) { switch (packetType) { case PacketType::DomainList: - return static_cast(DomainListVersion::PermissionsGrid); + return static_cast(DomainListVersion::GetUsernameFromUUIDSupport); case PacketType::EntityAdd: case PacketType::EntityEdit: case PacketType::EntityData: diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 07319bcd74..4a16e8ffe8 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -229,7 +229,8 @@ enum class DomainServerAddedNodeVersion : PacketVersion { enum class DomainListVersion : PacketVersion { PrePermissionsGrid = 18, - PermissionsGrid + PermissionsGrid, + GetUsernameFromUUIDSupport }; enum class AudioVersion : PacketVersion {