Skip to content
This repository was archived by the owner on Mar 4, 2023. It is now read-only.

Commit e19d0c0

Browse files
committed
added accept ack message to prevent conflicts
1 parent 6e0152d commit e19d0c0

File tree

11 files changed

+212
-43
lines changed

11 files changed

+212
-43
lines changed

Diff for: modeling/network_add_device.qmodel

+133-30
Large diffs are not rendered by default.

Diff for: src/datasync/messages/grantmessage.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ GrantMessage::GrantMessage() :
88
secret()
99
{}
1010

11-
GrantMessage::GrantMessage(const QUuid &deviceId, const AcceptMessage &message) :
12-
AccountMessage(deviceId),
11+
GrantMessage::GrantMessage(const AcceptMessage &message) :
12+
AccountMessage(message.deviceId),
1313
index(message.index),
1414
scheme(message.scheme),
1515
secret(message.secret)

Diff for: src/datasync/messages/grantmessage_p.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class Q_DATASYNC_EXPORT GrantMessage : public AccountMessage
1717

1818
public:
1919
GrantMessage();
20-
GrantMessage(const QUuid &deviceId, const AcceptMessage &message);
20+
GrantMessage(const AcceptMessage &message);
2121

2222
quint32 index;
2323
QByteArray scheme;

Diff for: src/datasync/messages/proofmessage.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,14 @@ const QMetaObject *AcceptMessage::getMetaObject() const
5656
{
5757
return &staticMetaObject;
5858
}
59+
60+
61+
62+
AcceptAckMessage::AcceptAckMessage(const AcceptMessage &message) :
63+
deviceId(message.deviceId)
64+
{}
65+
66+
const QMetaObject *AcceptAckMessage::getMetaObject() const
67+
{
68+
return &staticMetaObject;
69+
}

Diff for: src/datasync/messages/proofmessage_p.h

+16
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,26 @@ class Q_DATASYNC_EXPORT AcceptMessage : public Message
7676
const QMetaObject *getMetaObject() const override;
7777
};
7878

79+
class Q_DATASYNC_EXPORT AcceptAckMessage : public Message
80+
{
81+
Q_GADGET
82+
83+
Q_PROPERTY(QUuid deviceId MEMBER deviceId)
84+
85+
public:
86+
AcceptAckMessage(const AcceptMessage &message = {});
87+
88+
QUuid deviceId;
89+
90+
protected:
91+
const QMetaObject *getMetaObject() const override;
92+
};
93+
7994
}
8095

8196
Q_DECLARE_METATYPE(QtDataSync::ProofMessage)
8297
Q_DECLARE_METATYPE(QtDataSync::DenyMessage)
8398
Q_DECLARE_METATYPE(QtDataSync::AcceptMessage)
99+
Q_DECLARE_METATYPE(QtDataSync::AcceptAckMessage)
84100

85101
#endif // QTDATASYNC_PROOFMESSAGE_P_H

Diff for: src/datasync/remoteconnector.cpp

+11-3
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,7 @@ void RemoteConnector::loginReply(const QUuid &deviceId, bool accept)
288288
AcceptMessage message(deviceId);
289289
tie(message.index, message.scheme, message.secret) = _cryptoController->encryptSecretKey(crypto.data(), crypto->encryptionKey());
290290
sendSignedMessage(message);
291-
emit accountAccessGranted(deviceId);
292-
logInfo() << "Granted access to account for device" << deviceId;
291+
logDebug() << "Granting access to account for device" << deviceId;
293292
} else {
294293
sendMessage(DenyMessage{deviceId});
295294
logInfo() << "Rejected access to account for device" << deviceId;
@@ -465,6 +464,8 @@ void RemoteConnector::binaryMessageReceived(const QByteArray &message)
465464
onRemoved(Message::deserializeMessage<RemoveAckMessage>(stream));
466465
else if(Message::isType<ProofMessage>(name))
467466
onProof(Message::deserializeMessage<ProofMessage>(stream));
467+
else if(Message::isType<AcceptAckMessage>(name))
468+
onAcceptAck(Message::deserializeMessage<AcceptAckMessage>(stream));
468469
else if(Message::isType<MacUpdateAckMessage>(name))
469470
onMacUpdateAck(Message::deserializeMessage<MacUpdateAckMessage>(stream));
470471
else if(Message::isType<DeviceKeysMessage>(name))
@@ -1139,9 +1140,16 @@ void RemoteConnector::onProof(const ProofMessage &message)
11391140
}
11401141
}
11411142

1143+
void RemoteConnector::onAcceptAck(const AcceptAckMessage &message)
1144+
{
1145+
if(checkIdle(message)) {
1146+
emit accountAccessGranted(message.deviceId);
1147+
logInfo() << "Granted access to account for device" << message.deviceId;
1148+
}
1149+
}
1150+
11421151
void RemoteConnector::onMacUpdateAck(const MacUpdateAckMessage &message)
11431152
{
1144-
Q_UNUSED(message)
11451153
if(checkIdle(message)) {
11461154
settings()->remove(keySendCmac);
11471155
logDebug() << "Mac change acknowledged";

Diff for: src/datasync/remoteconnector_p.h

+1
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ private Q_SLOTS:
199199
void onDevices(const DevicesMessage &message);
200200
void onRemoved(const RemoveAckMessage &message);
201201
void onProof(const ProofMessage &message);
202+
void onAcceptAck(const AcceptAckMessage &message);
202203
void onMacUpdateAck(const MacUpdateAckMessage &message);
203204
void onDeviceKeys(const DeviceKeysMessage &message);
204205
void onNewKeyAck(const NewKeyAckMessage &message);

Diff for: tests/auto/datasync/TestAppServer/tst_appserver.cpp

+17
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,10 @@ void TestAppServer::testAddDevice(MockClient *&partner, QUuid &partnerDevId, boo
513513
accMsg.scheme = keyScheme;
514514
accMsg.secret = keySecret;
515515
client->sendSigned(accMsg, crypto);
516+
QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
517+
QCOMPARE(message.deviceId, partnerDevId);
518+
ok = true;
519+
}));
516520

517521
//wait for granted
518522
QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -832,6 +836,10 @@ void TestAppServer::testAddDeviceInvalidKeyIndex()
832836
accMsg.scheme = keyScheme;
833837
accMsg.secret = keySecret;
834838
client->sendSigned(accMsg, crypto);
839+
QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
840+
QCOMPARE(message.deviceId, partnerDevId);
841+
ok = true;
842+
}));
835843

836844
//wait for granted
837845
QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -922,6 +930,10 @@ void TestAppServer::testSendDoubleAccept()
922930
accMsg.scheme = keyScheme;
923931
accMsg.secret = keySecret;
924932
client->sendSigned(accMsg, crypto);
933+
QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
934+
QCOMPARE(message.deviceId, partnerDevId);
935+
ok = true;
936+
}));
925937

926938
//wait for granted
927939
QVERIFY(partner->waitForReply<GrantMessage>([&](GrantMessage message, bool &ok) {
@@ -946,6 +958,11 @@ void TestAppServer::testSendDoubleAccept()
946958

947959
//send another proof accept
948960
client->sendSigned(accMsg, crypto);
961+
//is still sent, event though no accept happend...
962+
QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
963+
QCOMPARE(message.deviceId, partnerDevId);
964+
ok = true;
965+
}));
949966
QVERIFY(partner->waitForNothing());
950967

951968
//clean partner

Diff for: tests/auto/datasync/TestMessages/tst_messages.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ void TestMessages::addAllData()
341341
msg.index = 42;
342342
msg.scheme = "key_scheme";
343343
msg.secret = "encrypted_key";
344-
return GrantMessage(QUuid::createUuid(), msg);
344+
return GrantMessage(msg);
345345
});
346346
addData<DeviceChangeMessage>([&]() {
347347
DeviceChangeMessage msg("id_hash", QUuid::createUuid());

Diff for: tests/auto/datasync/TestRemoteConnector/tst_remoteconnector.cpp

+11-5
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,8 @@ void TestRemoteConnector::testAddDeviceUntrusted()
679679
ok = true;
680680

681681
//Send from here because message needed
682-
partnerConnection->send(GrantMessage(partnerDevId, message));
682+
partnerConnection->send(GrantMessage(message));
683+
connection->send(AcceptAckMessage(message));
683684
}));
684685

685686
//verify preperations are done
@@ -689,7 +690,8 @@ void TestRemoteConnector::testAddDeviceUntrusted()
689690
QCOMPARE(grantedSpy.takeFirst()[0].toUuid(), partnerDevId);
690691

691692
//verify grant message received successfully
692-
QVERIFY(partnerImportSpy.wait());
693+
if(partnerImportSpy.isEmpty())
694+
QVERIFY(partnerImportSpy.wait());
693695
QCOMPARE(partnerImportSpy.size(), 1);
694696

695697
//wait for mac update message
@@ -781,7 +783,8 @@ void TestRemoteConnector::testAddDeviceTrusted()
781783
ok = true;
782784

783785
//Send from here because message needed
784-
partnerCon->send(GrantMessage(devId2, message));
786+
partnerCon->send(GrantMessage(message));
787+
connection->send(AcceptAckMessage(message));
785788
}));
786789

787790
//verify preperations are done
@@ -791,7 +794,8 @@ void TestRemoteConnector::testAddDeviceTrusted()
791794
QCOMPARE(grantedSpy.takeFirst()[0].toUuid(), devId2);
792795

793796
//verify grant message received successfully
794-
QVERIFY(partnerImportSpy.wait());
797+
if(partnerImportSpy.isEmpty())
798+
QVERIFY(partnerImportSpy.wait());
795799
QCOMPARE(partnerImportSpy.size(), 1);
796800

797801
//wait for mac update message
@@ -1540,7 +1544,7 @@ void TestRemoteConnector::testUnexpectedMessage_data()
15401544
<< true;
15411545
QTest::newRow("WelcomeMessage") << create<WelcomeMessage>(false)
15421546
<< true;
1543-
QTest::newRow("GrantMessage") << create<GrantMessage>(devId, AcceptMessage(partnerDevId))
1547+
QTest::newRow("GrantMessage") << create<GrantMessage>(AcceptMessage(partnerDevId))
15441548
<< true;
15451549
QTest::newRow("ChangeAckMessage") << create<ChangeAckMessage>(ChangeMessage("test"))
15461550
<< false;
@@ -1558,6 +1562,8 @@ void TestRemoteConnector::testUnexpectedMessage_data()
15581562
<< false;
15591563
QTest::newRow("ProofMessage") << create<ProofMessage>(AccessMessage(), partnerDevId)
15601564
<< false;
1565+
QTest::newRow("AcceptAckMessage") << create<AcceptAckMessage>(AcceptMessage(partnerDevId))
1566+
<< false;
15611567
QTest::newRow("MacUpdateAckMessage") << create<MacUpdateAckMessage>()
15621568
<< false;
15631569
QTest::newRow("DeviceKeysMessage") << create<DeviceKeysMessage>(7)

Diff for: tools/appserver/client.cpp

+8-1
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,12 @@ void Client::proofResult(bool success, const AcceptMessage &message)
147147
} else {
148148
qDebug() << "Proof completed with result:" << success;
149149
if(success) {
150+
if(message.deviceId != _deviceId) {
151+
qWarning() << "Message device id does not match. Own id:"
152+
<< _deviceId << "agains message id:" << message.deviceId;
153+
return;
154+
}
155+
150156
auto pDevId = _cachedAccessRequest.partnerId;
151157
_database->addNewDeviceToUser(_deviceId,
152158
pDevId,
@@ -160,7 +166,7 @@ void Client::proofResult(bool success, const AcceptMessage &message)
160166
_cachedFingerPrint.clear();
161167

162168
qDebug() << "Created new device and added to account of device" << pDevId;
163-
sendMessage(GrantMessage{_deviceId, message});
169+
sendMessage(GrantMessage{message});
164170
_state = Idle;
165171
emit connected(_deviceId);
166172
} else
@@ -546,6 +552,7 @@ void Client::onAccept(const AcceptMessage &message, QDataStream &stream)
546552
}
547553

548554
emit proofDone(message.deviceId, true, message);
555+
sendMessage(AcceptAckMessage(message));
549556
}
550557

551558
void Client::onDeny(const DenyMessage &message)

0 commit comments

Comments
 (0)