added accept ack message to prevent conflicts

Skycoder42 · Skycoder42 · commit e19d0c03fe59 · 2018-02-02T22:24:00.000+01:00
diff --git a/modeling/network_add_device.qmodel b/modeling/network_add_device.qmodel
diff --git a/src/datasync/messages/grantmessage.cpp b/src/datasync/messages/grantmessage.cpp
@@ -8,8 +8,8 @@ GrantMessage::GrantMessage() :
 	secret()
 {}
 
-GrantMessage::GrantMessage(const QUuid &deviceId, const AcceptMessage &message) :
-	AccountMessage(deviceId),
+GrantMessage::GrantMessage(const AcceptMessage &message) :
+	AccountMessage(message.deviceId),
 	index(message.index),
 	scheme(message.scheme),
 	secret(message.secret)
diff --git a/src/datasync/messages/grantmessage_p.h b/src/datasync/messages/grantmessage_p.h
@@ -17,7 +17,7 @@ class Q_DATASYNC_EXPORT GrantMessage : public AccountMessage
 
 public:
 	GrantMessage();
-	GrantMessage(const QUuid &deviceId, const AcceptMessage &message);
+	GrantMessage(const AcceptMessage &message);
 
 	quint32 index;
 	QByteArray scheme;
diff --git a/src/datasync/messages/proofmessage.cpp b/src/datasync/messages/proofmessage.cpp
@@ -56,3 +56,14 @@ const QMetaObject *AcceptMessage::getMetaObject() const
 {
 	return &staticMetaObject;
 }
+
+
+
+AcceptAckMessage::AcceptAckMessage(const AcceptMessage &message) :
+	deviceId(message.deviceId)
+{}
+
+const QMetaObject *AcceptAckMessage::getMetaObject() const
+{
+	return &staticMetaObject;
+}
diff --git a/src/datasync/messages/proofmessage_p.h b/src/datasync/messages/proofmessage_p.h
@@ -76,10 +76,26 @@ class Q_DATASYNC_EXPORT AcceptMessage : public Message
 	const QMetaObject *getMetaObject() const override;
 };
 
+class Q_DATASYNC_EXPORT AcceptAckMessage : public Message
+{
+	Q_GADGET
+
+	Q_PROPERTY(QUuid deviceId MEMBER deviceId)
+
+public:
+	AcceptAckMessage(const AcceptMessage &message = {});
+
+	QUuid deviceId;
+
+protected:
+	const QMetaObject *getMetaObject() const override;
+};
+
 }
 
 Q_DECLARE_METATYPE(QtDataSync::ProofMessage)
 Q_DECLARE_METATYPE(QtDataSync::DenyMessage)
 Q_DECLARE_METATYPE(QtDataSync::AcceptMessage)
+Q_DECLARE_METATYPE(QtDataSync::AcceptAckMessage)
 
 #endif // QTDATASYNC_PROOFMESSAGE_P_H
diff --git a/src/datasync/remoteconnector.cpp b/src/datasync/remoteconnector.cpp
@@ -288,8 +288,7 @@ void RemoteConnector::loginReply(const QUuid &deviceId, bool accept)
 			AcceptMessage message(deviceId);
 			tie(message.index, message.scheme, message.secret) = _cryptoController->encryptSecretKey(crypto.data(), crypto->encryptionKey());
 			sendSignedMessage(message);
-			emit accountAccessGranted(deviceId);
-			logInfo() << "Granted access to account for device" << deviceId;
+			logDebug() << "Granting access to account for device" << deviceId;
 		} else {
 			sendMessage(DenyMessage{deviceId});
 			logInfo() << "Rejected access to account for device" << deviceId;
@@ -465,6 +464,8 @@ void RemoteConnector::binaryMessageReceived(const QByteArray &message)
 			onRemoved(Message::deserializeMessage<RemoveAckMessage>(stream));
 		else if(Message::isType<ProofMessage>(name))
 			onProof(Message::deserializeMessage<ProofMessage>(stream));
+		else if(Message::isType<AcceptAckMessage>(name))
+			onAcceptAck(Message::deserializeMessage<AcceptAckMessage>(stream));
 		else if(Message::isType<MacUpdateAckMessage>(name))
 			onMacUpdateAck(Message::deserializeMessage<MacUpdateAckMessage>(stream));
 		else if(Message::isType<DeviceKeysMessage>(name))
@@ -1139,9 +1140,16 @@ void RemoteConnector::onProof(const ProofMessage &message)
 	}
 }
 
+void RemoteConnector::onAcceptAck(const AcceptAckMessage &message)
+{
+	if(checkIdle(message)) {
+		emit accountAccessGranted(message.deviceId);
+		logInfo() << "Granted access to account for device" << message.deviceId;
+	}
+}
+
 void RemoteConnector::onMacUpdateAck(const MacUpdateAckMessage &message)
 {
-	Q_UNUSED(message)
 	if(checkIdle(message)) {
 		settings()->remove(keySendCmac);
 		logDebug() << "Mac change acknowledged";
diff --git a/src/datasync/remoteconnector_p.h b/src/datasync/remoteconnector_p.h
@@ -199,6 +199,7 @@ private Q_SLOTS:
 	void onDevices(const DevicesMessage &message);
 	void onRemoved(const RemoveAckMessage &message);
 	void onProof(const ProofMessage &message);
+	void onAcceptAck(const AcceptAckMessage &message);
 	void onMacUpdateAck(const MacUpdateAckMessage &message);
 	void onDeviceKeys(const DeviceKeysMessage &message);
 	void onNewKeyAck(const NewKeyAckMessage &message);
diff --git a/tests/auto/datasync/TestAppServer/tst_appserver.cpp b/tests/auto/datasync/TestAppServer/tst_appserver.cpp
@@ -513,6 +513,10 @@ void TestAppServer::testAddDevice(MockClient *&partner, QUuid &partnerDevId, boo
 		accMsg.scheme = keyScheme;
 		accMsg.secret = keySecret;
 		client->sendSigned(accMsg, crypto);
+		QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
+			QCOMPARE(message.deviceId, partnerDevId);
+			ok = true;
+		}));
 
 		//wait for granted
 		QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -832,6 +836,10 @@ void TestAppServer::testAddDeviceInvalidKeyIndex()
 		accMsg.scheme = keyScheme;
 		accMsg.secret = keySecret;
 		client->sendSigned(accMsg, crypto);
+		QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
+			QCOMPARE(message.deviceId, partnerDevId);
+			ok = true;
+		}));
 
 		//wait for granted
 		QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -922,6 +930,10 @@ void TestAppServer::testSendDoubleAccept()
 		accMsg.scheme = keyScheme;
 		accMsg.secret = keySecret;
 		client->sendSigned(accMsg, crypto);
+		QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
+			QCOMPARE(message.deviceId, partnerDevId);
+			ok = true;
+		}));
 
 		//wait for granted
 		QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -946,6 +958,11 @@ void TestAppServer::testSendDoubleAccept()
 
 		//send another proof accept
 		client->sendSigned(accMsg, crypto);
+		//is still sent, event though no accept happend...
+		QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
+			QCOMPARE(message.deviceId, partnerDevId);
+			ok = true;
+		}));
 		QVERIFY(partner->waitForNothing());
 
 		//clean partner
diff --git a/tests/auto/datasync/TestMessages/tst_messages.cpp b/tests/auto/datasync/TestMessages/tst_messages.cpp
@@ -341,7 +341,7 @@ void TestMessages::addAllData()
 		msg.index = 42;
 		msg.scheme = "key_scheme";
 		msg.secret = "encrypted_key";
-		return GrantMessage(QUuid::createUuid(), msg);
+		return GrantMessage(msg);
 	});
 	addData<DeviceChangeMessage>([&]() {
 		DeviceChangeMessage msg("id_hash", QUuid::createUuid());
diff --git a/tests/auto/datasync/TestRemoteConnector/tst_remoteconnector.cpp b/tests/auto/datasync/TestRemoteConnector/tst_remoteconnector.cpp
@@ -679,7 +679,8 @@ void TestRemoteConnector::testAddDeviceUntrusted()
 			ok = true;
 
 			//Send from here because message needed
-			partnerConnection->send(GrantMessage(partnerDevId, message));
+			partnerConnection->send(GrantMessage(message));
+			connection->send(AcceptAckMessage(message));
 		}));
 
 		//verify preperations are done
@@ -689,7 +690,8 @@ void TestRemoteConnector::testAddDeviceUntrusted()
 		QCOMPARE(grantedSpy.takeFirst()[0].toUuid(), partnerDevId);
 
 		//verify grant message received successfully
-		QVERIFY(partnerImportSpy.wait());
+		if(partnerImportSpy.isEmpty())
+			QVERIFY(partnerImportSpy.wait());
 		QCOMPARE(partnerImportSpy.size(), 1);
 
 		//wait for mac update message
@@ -781,7 +783,8 @@ void TestRemoteConnector::testAddDeviceTrusted()
 			ok = true;
 
 			//Send from here because message needed
-			partnerCon->send(GrantMessage(devId2, message));
+			partnerCon->send(GrantMessage(message));
+			connection->send(AcceptAckMessage(message));
 		}));
 
 		//verify preperations are done
@@ -791,7 +794,8 @@ void TestRemoteConnector::testAddDeviceTrusted()
 		QCOMPARE(grantedSpy.takeFirst()[0].toUuid(), devId2);
 
 		//verify grant message received successfully
-		QVERIFY(partnerImportSpy.wait());
+		if(partnerImportSpy.isEmpty())
+			QVERIFY(partnerImportSpy.wait());
 		QCOMPARE(partnerImportSpy.size(), 1);
 
 		//wait for mac update message
@@ -1540,7 +1544,7 @@ void TestRemoteConnector::testUnexpectedMessage_data()
 									<< true;
 	QTest::newRow("WelcomeMessage") << create<WelcomeMessage>(false)
 									<< true;
-	QTest::newRow("GrantMessage") << create<GrantMessage>(devId, AcceptMessage(partnerDevId))
+	QTest::newRow("GrantMessage") << create<GrantMessage>(AcceptMessage(partnerDevId))
 								  << true;
 	QTest::newRow("ChangeAckMessage") << create<ChangeAckMessage>(ChangeMessage("test"))
 									  << false;
@@ -1558,6 +1562,8 @@ void TestRemoteConnector::testUnexpectedMessage_data()
 									  << false;
 	QTest::newRow("ProofMessage") << create<ProofMessage>(AccessMessage(), partnerDevId)
 								  << false;
+	QTest::newRow("AcceptAckMessage") << create<AcceptAckMessage>(AcceptMessage(partnerDevId))
+									  << false;
 	QTest::newRow("MacUpdateAckMessage") << create<MacUpdateAckMessage>()
 										 << false;
 	QTest::newRow("DeviceKeysMessage") << create<DeviceKeysMessage>(7)
diff --git a/tools/appserver/client.cpp b/tools/appserver/client.cpp
@@ -147,6 +147,12 @@ void Client::proofResult(bool success, const AcceptMessage &message)
 		} else {
 			qDebug() << "Proof completed with result:" << success;
 			if(success) {
+				if(message.deviceId != _deviceId) {
+					qWarning() << "Message device id does not match. Own id:"
+							   << _deviceId << "agains message id:" << message.deviceId;
+					return;
+				}
+
 				auto pDevId = _cachedAccessRequest.partnerId;
 				_database->addNewDeviceToUser(_deviceId,
 											  pDevId,
@@ -160,7 +166,7 @@ void Client::proofResult(bool success, const AcceptMessage &message)
 				_cachedFingerPrint.clear();
 
 				qDebug() << "Created new device and added to account of device" << pDevId;
-				sendMessage(GrantMessage{_deviceId, message});
+				sendMessage(GrantMessage{message});
 				_state = Idle;
 				emit connected(_deviceId);
 			} else
@@ -546,6 +552,7 @@ void Client::onAccept(const AcceptMessage &message, QDataStream &stream)
 	}
 
 	emit proofDone(message.deviceId, true, message);
+	sendMessage(AcceptAckMessage(message));
 }
 
 void Client::onDeny(const DenyMessage &message)
