From c96c68440062d0d2f5357063b5ee9bf8d1d69206 Mon Sep 17 00:00:00 2001 From: Zach Fox <fox@highfidelity.io> Date: Thu, 5 Dec 2019 11:55:24 -0800 Subject: [PATCH] Initial path to fixing screenshare auth race condition --- domain-server/src/DomainServer.cpp | 16 ++++++-- domain-server/src/DomainServer.h | 4 +- .../ScreenshareScriptingInterface.cpp | 28 +++++++++++++ .../scripting/ScreenshareScriptingInterface.h | 5 +++ libraries/networking/src/AccountManager.cpp | 41 ++++++++++++++++--- libraries/networking/src/AccountManager.h | 4 +- 6 files changed, 86 insertions(+), 12 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index de41927fe2..9a8033e03c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -3634,10 +3634,10 @@ void DomainServer::processAvatarZonePresencePacket(QSharedPointer<ReceivedMessag return; } static const int SCREENSHARE_EXPIRATION_SECONDS = 24 * 60 * 60; - screensharePresence(zone.isNull() ? "" : zone.toString(), verifiedUsername, SCREENSHARE_EXPIRATION_SECONDS); + screensharePresence(zone.isNull() ? "" : zone.toString(), verifiedUsername, avatar, SCREENSHARE_EXPIRATION_SECONDS); } -void DomainServer::screensharePresence(QString roomname, QString username, int expirationSeconds) { +void DomainServer::screensharePresence(QString roomname, QString username, QUuid avatarID, int expirationSeconds) { if (!DependencyManager::get<AccountManager>()->hasValidAccessToken()) { static std::once_flag presenceAuthorityWarning; std::call_once(presenceAuthorityWarning, [] { @@ -3649,6 +3649,10 @@ void DomainServer::screensharePresence(QString roomname, QString username, int e callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "handleSuccessfulScreensharePresence"; callbackParams.errorCallbackMethod = "handleFailedScreensharePresence"; + QJsonObject callbackData; + callbackData.insert("roomname", roomname); + callbackData.insert("avatarID", avatarID.toString()); + callbackParams.callbackData = callbackData; const QString PATH = "api/v1/domains/%1/screenshare"; QString domain_id = uuidStringWithoutCurlyBraces(getID()); QJsonObject json, screenshare; @@ -3666,11 +3670,17 @@ void DomainServer::screensharePresence(QString roomname, QString username, int e ); } -void DomainServer::handleSuccessfulScreensharePresence(QNetworkReply* requestReply) { +void DomainServer::handleSuccessfulScreensharePresence(QNetworkReply* requestReply, QJsonObject callbackData) { QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); if (jsonObject["status"].toString() != "success") { qCWarning(domain_server) << "screensharePresence api call failed:" << QJsonDocument(jsonObject).toJson(QJsonDocument::Compact); + return; } + + auto nodeList = DependencyManager::get<LimitedNodeList>(); + auto packet = NLPacket::create(PacketType::AvatarZonePresence, NUM_BYTES_RFC4122_UUID, true); + packet->write(QUuid(callbackData["roomname"].toString()).toRfc4122()); + nodeList->sendPacket(std::move(packet), *(nodeList->nodeWithUUID(QUuid(callbackData["avatarID"].toString())))); } void DomainServer::handleFailedScreensharePresence(QNetworkReply* requestReply) { diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 034ccb5a18..92ef5a90b3 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -78,7 +78,7 @@ public: bool isAssetServerEnabled(); - void screensharePresence(QString roomname, QString username, int expiration_seconds = 0); + void screensharePresence(QString roomname, QString username, QUuid avatarID, int expiration_seconds = 0); public slots: /// Called by NodeList to inform us a node has been added @@ -132,7 +132,7 @@ private slots: void handleSuccessfulICEServerAddressUpdate(QNetworkReply* requestReply); void handleFailedICEServerAddressUpdate(QNetworkReply* requestReply); - void handleSuccessfulScreensharePresence(QNetworkReply* requestReply); + void handleSuccessfulScreensharePresence(QNetworkReply* requestReply, QJsonObject callbackData); void handleFailedScreensharePresence(QNetworkReply* requestReply); void updateReplicatedNodes(); diff --git a/interface/src/scripting/ScreenshareScriptingInterface.cpp b/interface/src/scripting/ScreenshareScriptingInterface.cpp index 37365c1b89..a5f7595599 100644 --- a/interface/src/scripting/ScreenshareScriptingInterface.cpp +++ b/interface/src/scripting/ScreenshareScriptingInterface.cpp @@ -37,14 +37,41 @@ ScreenshareScriptingInterface::ScreenshareScriptingInterface() { _requestScreenshareInfoRetryTimer->setSingleShot(true); _requestScreenshareInfoRetryTimer->setInterval(SCREENSHARE_INFO_REQUEST_RETRY_TIMEOUT_MS); connect(_requestScreenshareInfoRetryTimer, &QTimer::timeout, this, &ScreenshareScriptingInterface::requestScreenshareInfo); + + auto nodeList = DependencyManager::get<NodeList>(); + PacketReceiver& packetReceiver = nodeList->getPacketReceiver(); + packetReceiver.registerListener(PacketType::AvatarZonePresence, this, "processAvatarZonePresencePacketOnClient"); }; ScreenshareScriptingInterface::~ScreenshareScriptingInterface() { stopScreenshare(); } +void ScreenshareScriptingInterface::processAvatarZonePresencePacketOnClient(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode) { + QUuid zone = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + + if (zone.isNull()) { + qWarning() << "Ignoring avatar zone presence packet that doesn't specify a zone."; + return; + } + + _lastAuthorizedZoneID = zone; + + if (_waitingForAuthorization) { + requestScreenshareInfo(); + } +} + static const int MAX_NUM_SCREENSHARE_INFO_REQUEST_RETRIES = 5; void ScreenshareScriptingInterface::requestScreenshareInfo() { + if (_screenshareZoneID != _lastAuthorizedZoneID) { + qDebug() << "Client not yet authorized to screenshare. Waiting for authorization message from domain server..."; + _waitingForAuthorization = true; + return; + } + + _waitingForAuthorization = false; + _requestScreenshareInfoRetries++; if (_requestScreenshareInfoRetries >= MAX_NUM_SCREENSHARE_INFO_REQUEST_RETRIES) { @@ -174,6 +201,7 @@ void ScreenshareScriptingInterface::stopScreenshare() { _projectAPIKey = ""; _sessionID = ""; _isPresenter = false; + _waitingForAuthorization = false; } // Called when the Metaverse returns the information necessary to start/view a screen share. diff --git a/interface/src/scripting/ScreenshareScriptingInterface.h b/interface/src/scripting/ScreenshareScriptingInterface.h index 2e9cd79fcf..41e93d770d 100644 --- a/interface/src/scripting/ScreenshareScriptingInterface.h +++ b/interface/src/scripting/ScreenshareScriptingInterface.h @@ -41,6 +41,8 @@ private slots: void handleFailedScreenshareInfoGet(QNetworkReply* reply); private: + void processAvatarZonePresencePacketOnClient(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode); + #if DEV_BUILD #ifdef Q_OS_WIN const QString SCREENSHARE_EXE_PATH{ PathUtils::projectRootPath() + "/screenshare/hifi-screenshare-win32-x64/hifi-screenshare.exe" }; @@ -82,6 +84,9 @@ private: QUuid _screenshareZoneID; QUuid _smartboardEntityID; bool _isPresenter{ false }; + + QUuid _lastAuthorizedZoneID; + bool _waitingForAuthorization{ false }; }; #endif // hifi_ScreenshareScriptingInterface_h diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index 5473f1a010..095dfebf5c 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -61,6 +61,18 @@ JSONCallbackParameters::JSONCallbackParameters(QObject* callbackReceiver, } +JSONCallbackParameters::JSONCallbackParameters(QObject* callbackReceiver, + const QString& jsonCallbackMethod, + const QString& errorCallbackMethod, + const QJsonObject& callbackData) : + callbackReceiver(callbackReceiver), + jsonCallbackMethod(jsonCallbackMethod), + errorCallbackMethod(errorCallbackMethod), + callbackData(callbackData) +{ + +} + QJsonObject AccountManager::dataObjectFromResponse(QNetworkReply* requestReply) { QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); @@ -348,9 +360,17 @@ void AccountManager::sendRequest(const QString& path, [callbackParams, networkReply] { if (networkReply->error() == QNetworkReply::NoError) { if (!callbackParams.jsonCallbackMethod.isEmpty()) { - bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, - qPrintable(callbackParams.jsonCallbackMethod), - Q_ARG(QNetworkReply*, networkReply)); + bool invoked = false; + if (callbackParams.callbackData.isEmpty()) { + invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, + qPrintable(callbackParams.jsonCallbackMethod), + Q_ARG(QNetworkReply*, networkReply)); + } else { + invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, + qPrintable(callbackParams.jsonCallbackMethod), + Q_ARG(QNetworkReply*, networkReply), + Q_ARG(QJsonObject, callbackParams.callbackData)); + } if (!invoked) { QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* " @@ -366,9 +386,18 @@ void AccountManager::sendRequest(const QString& path, } } else { if (!callbackParams.errorCallbackMethod.isEmpty()) { - bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, - qPrintable(callbackParams.errorCallbackMethod), - Q_ARG(QNetworkReply*, networkReply)); + bool invoked = false; + if (callbackParams.callbackData.isEmpty()) { + invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, + qPrintable(callbackParams.errorCallbackMethod), + Q_ARG(QNetworkReply*, networkReply)); + } + else { + invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, + qPrintable(callbackParams.errorCallbackMethod), + Q_ARG(QNetworkReply*, networkReply), + Q_ARG(QJsonObject, callbackParams.callbackData)); + } if (!invoked) { QString error = "Could not invoke " + callbackParams.errorCallbackMethod + " with QNetworkReply* " diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index 2e94ccf0ea..33c9190561 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -31,13 +31,15 @@ class JSONCallbackParameters { public: JSONCallbackParameters(QObject* callbackReceiver = nullptr, const QString& jsonCallbackMethod = QString(), - const QString& errorCallbackMethod = QString()); + const QString& errorCallbackMethod = QString(), + const QJsonObject& callbackData = QJsonObject()); bool isEmpty() const { return !callbackReceiver; } QObject* callbackReceiver; QString jsonCallbackMethod; QString errorCallbackMethod; + QJsonObject callbackData; }; namespace AccountManagerAuth {