From 3e97b07fc21d7554a44b9de91462b6ae168beccf Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 21 Jun 2017 15:14:16 -0700 Subject: [PATCH 1/3] Revert "force domain-server to use new ID if node requests connect" This reverts commit cd023e038406965dd9d5a69c3143ece8475fe26a. --- domain-server/src/DomainGatekeeper.cpp | 28 ++++++++++++++++++++------ domain-server/src/DomainGatekeeper.h | 3 ++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index a3692c974e..c187239351 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -435,8 +435,23 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect return SharedNodePointer(); } + QUuid hintNodeID; + + // in case this is a node that's failing to connect + // double check we don't have a node whose sockets match exactly already in the list + limitedNodeList->eachNodeBreakable([&nodeConnection, &hintNodeID](const SharedNodePointer& node){ + if (node->getPublicSocket() == nodeConnection.publicSockAddr + && node->getLocalSocket() == nodeConnection.localSockAddr) { + // we have a node that already has these exact sockets - this occurs if a node + // is unable to connect to the domain + hintNodeID = node->getUUID(); + return false; + } + return true; + }); + // add the connecting node (or re-use the matched one from eachNodeBreakable above) - SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection); + SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection, hintNodeID); // set the edit rights for this user newNode->setPermissions(userPerms); @@ -464,12 +479,11 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect return newNode; } -SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const NodeConnectionData& nodeConnection) { +SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const NodeConnectionData& nodeConnection, + QUuid nodeID) { HifiSockAddr discoveredSocket = nodeConnection.senderSockAddr; SharedNetworkPeer connectedPeer = _icePeers.value(nodeConnection.connectUUID); - QUuid nodeID; - if (connectedPeer) { // this user negotiated a connection with us via ICE, so re-use their ICE client ID nodeID = nodeConnection.connectUUID; @@ -479,8 +493,10 @@ SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const Node discoveredSocket = *connectedPeer->getActiveSocket(); } } else { - // we got a connectUUID we didn't recognize, randomly generate a new one - nodeID = QUuid::createUuid(); + // we got a connectUUID we didn't recognize, either use the hinted node ID or randomly generate a new one + if (nodeID.isNull()) { + nodeID = QUuid::createUuid(); + } } auto limitedNodeList = DependencyManager::get(); diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index e2d36c4cea..163f255411 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -76,7 +76,8 @@ private: SharedNodePointer processAgentConnectRequest(const NodeConnectionData& nodeConnection, const QString& username, const QByteArray& usernameSignature); - SharedNodePointer addVerifiedNodeFromConnectRequest(const NodeConnectionData& nodeConnection); + SharedNodePointer addVerifiedNodeFromConnectRequest(const NodeConnectionData& nodeConnection, + QUuid nodeID = QUuid()); bool verifyUserSignature(const QString& username, const QByteArray& usernameSignature, const HifiSockAddr& senderSockAddr); From 9285c88e4681b6f5ea462b15936bc42f88f56b05 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 21 Jun 2017 15:34:43 -0700 Subject: [PATCH 2/3] check if usernames do not match before using hint ID --- domain-server/src/DomainGatekeeper.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index c187239351..dfe3370e91 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -438,14 +438,20 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect QUuid hintNodeID; // in case this is a node that's failing to connect - // double check we don't have a node whose sockets match exactly already in the list - limitedNodeList->eachNodeBreakable([&nodeConnection, &hintNodeID](const SharedNodePointer& node){ - if (node->getPublicSocket() == nodeConnection.publicSockAddr - && node->getLocalSocket() == nodeConnection.localSockAddr) { - // we have a node that already has these exact sockets - this occurs if a node - // is unable to connect to the domain - hintNodeID = node->getUUID(); - return false; + // double check we don't have the same node whose sockets match exactly already in the list + limitedNodeList->eachNodeBreakable([&nodeConnection, &hintNodeID, &username](const SharedNodePointer& node){ + if (node->getPublicSocket() == nodeConnection.publicSockAddr && node->getLocalSocket() == nodeConnection.localSockAddr) { + // we have a node that already has these exact sockets - this can occur if a node + // is failing to connect to the domain + + // we'll re-use the existing node ID + // as long as the user hasn't changed their username (by logging in or logging out) + auto existingNodeData = static_cast(node->getLinkedData()); + + if (existingNodeData->getUsername() == username) { + hintNodeID = node->getUUID(); + return false; + } } return true; }); From e39e5c8c3b0918355cfc3c6f84a758e97e388056 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 21 Jun 2017 16:21:40 -0700 Subject: [PATCH 3/3] simplify lambda capture in gate keeper --- domain-server/src/DomainGatekeeper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index dfe3370e91..62f56184f4 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -439,7 +439,7 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect // in case this is a node that's failing to connect // double check we don't have the same node whose sockets match exactly already in the list - limitedNodeList->eachNodeBreakable([&nodeConnection, &hintNodeID, &username](const SharedNodePointer& node){ + limitedNodeList->eachNodeBreakable([&](const SharedNodePointer& node){ if (node->getPublicSocket() == nodeConnection.publicSockAddr && node->getLocalSocket() == nodeConnection.localSockAddr) { // we have a node that already has these exact sockets - this can occur if a node // is failing to connect to the domain