From 26e6ce07d7068973d192300e9e8b5f74407fcc44 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Mon, 3 Aug 2020 11:26:11 +1200 Subject: [PATCH] Further networking tidying --- domain-server/src/DomainGatekeeper.cpp | 17 +++++------ .../networking/src/DomainAccountManager.cpp | 29 +++++++------------ 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 16c7383bf9..25f38d6ebd 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -1101,11 +1101,11 @@ void DomainGatekeeper::getIsGroupMemberErrorCallback(QNetworkReply* requestReply void DomainGatekeeper::getDomainGroupMemberships(const QString& domainUserName) { - // ####### TODO: Get user's domain group memberships (WordPress roles) from domain. + // ####### TODO: Get user's domain group memberships (WordPress roles) from domain. [plugin groups] // This may be able to be provided at the same time as the "authenticate user" call to the domain API, in which case // a copy of some of the following code can be made there. However, this code is still needed for refreshing groups. - // ####### TODO: Check how often this method and the WordPress API is called. + // ####### TODO: Check how often this method and the WordPress API is called. [plugin groups] QStringList wordpressGroupsForUser; wordpressGroupsForUser << "silVER" << "gold" << "coal"; @@ -1161,7 +1161,7 @@ void DomainGatekeeper::getDomainOwnerFriendsListErrorCallback(QNetworkReply* req qDebug() << "getDomainOwnerFriendsList api call failed:" << requestReply->error(); } -// ####### TODO: Domain equivalent or addition +// ####### TODO: Domain equivalent or addition [plugin groups] void DomainGatekeeper::refreshGroupsCache() { // if agents are connected to this domain, refresh our cached information about groups and memberships in such. getDomainOwnerFriendsList(); @@ -1251,7 +1251,7 @@ void DomainGatekeeper::requestDomainUser(const QString& username, const QString& const QString WORDPRESS_USER_ROUTE = "wp/v2/users/me?context=edit&_fields=id,username,roles"; QUrl domainUserURL = apiBase + WORDPRESS_USER_ROUTE; - // ####### TODO: Append a random key to check in response? + // ####### TODO: Append a random key to check in response? [security] QNetworkRequest request; @@ -1265,8 +1265,6 @@ void DomainGatekeeper::requestDomainUser(const QString& username, const QString& request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); - // ####### TODO: Handle invalid URL (e.g., set timeout or similar). - QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance(); QNetworkReply* requestReply = networkAccessManager.post(request, formData); connect(requestReply, &QNetworkReply::finished, this, &DomainGatekeeper::requestDomainUserFinished); @@ -1280,6 +1278,7 @@ void DomainGatekeeper::requestDomainUserFinished() { const QJsonObject& rootObject = jsonResponse.object(); auto httpStatus = requestReply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + if (200 <= httpStatus && httpStatus < 300) { // Success. // ####### TODO: Verify Expected response. [plugin] @@ -1307,11 +1306,9 @@ void DomainGatekeeper::requestDomainUserFinished() { // Failure. // ####### TODO: Error fields to report. [plugin] - qDebug() << "Error in response for user details -" << rootObject["error"].toString(); + qDebug() << "Error in response for user details -" << httpStatus << requestReply->error() + << "-" << rootObject["error"].toString(); - // ####### TODO: Is this the best way to handle _inFlightDomainUserIdentityRequests? - // If there's a brief network glitch will it recover? - // Perhaps clear on a timer? Cancel timer upon subsequent successful responses? _inFlightDomainUserIdentityRequests.clear(); } } diff --git a/libraries/networking/src/DomainAccountManager.cpp b/libraries/networking/src/DomainAccountManager.cpp index 5cf707e732..cb67d30d3e 100644 --- a/libraries/networking/src/DomainAccountManager.cpp +++ b/libraries/networking/src/DomainAccountManager.cpp @@ -11,31 +11,23 @@ #include "DomainAccountManager.h" -// ####### TODO: Check that all #includes are still needed. - -#include - #include -#include #include #include #include #include -#include -#include "DomainAccountManager.h" +#include + #include "NetworkingConstants.h" -#include "OAuthAccessToken.h" #include "NetworkLogging.h" -#include "NodeList.h" -#include "udt/PacketHeaders.h" #include "NetworkAccessManager.h" // FIXME: Generalize to other OAuth2 sources for domain login. const bool VERBOSE_HTTP_REQUEST_DEBUGGING = false; -const QString DOMAIN_ACCOUNT_MANAGER_REQUESTED_SCOPE = "foo bar"; // ####### TODO: WordPress plugin's required scope. -// ####### TODO: Should scope be configured in domain server settings? +const QString DOMAIN_ACCOUNT_MANAGER_REQUESTED_SCOPE = "foo bar"; // ####### TODO: WordPress plugin's required scope. [plugin] +// ####### TODO: Should scope be configured in domain server settings? [plugin] // ####### TODO: Add storing domain URL and check against it when retrieving values? // ####### TODO: Add storing _authURL and check against it when retrieving values? @@ -79,7 +71,7 @@ void DomainAccountManager::requestAccessToken(const QString& username, const QSt request.setHeader(QNetworkRequest::UserAgentHeader, NetworkingConstants::VIRCADIA_USER_AGENT); request.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded"); - // ####### TODO: WordPress plugin's authorization requirements. + // ####### TODO: WordPress plugin's authorization requirements. [plugin] request.setRawHeader(QByteArray("Authorization"), QByteArray("Basic b2F1dGgtY2xpZW50LTE6b2F1dGgtY2xpZW50LXNlY3JldC0x")); QByteArray formData; @@ -87,7 +79,7 @@ void DomainAccountManager::requestAccessToken(const QString& username, const QSt formData.append("username=" + QUrl::toPercentEncoding(username) + "&"); formData.append("password=" + QUrl::toPercentEncoding(password) + "&"); formData.append("scope=" + DOMAIN_ACCOUNT_MANAGER_REQUESTED_SCOPE); - // ####### TODO: Include state? + // ####### TODO: Include state? [plugin] request.setUrl(_authURL); @@ -108,10 +100,10 @@ void DomainAccountManager::requestAccessTokenFinished() { auto httpStatus = requestReply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); if (200 <= httpStatus && httpStatus < 300) { - // ####### TODO: Check that token type == "Bearer"? - // ####### TODO: Process response state? - // ####### TODO: Process response scope? + // ####### TODO: Process response state? [plugin] + // ####### TODO: Process response scope? [plugin] + // ####### TODO: Which method are the tokens provided in? [plugin] if (rootObject.contains("access_token")) { // Success. @@ -132,7 +124,8 @@ void DomainAccountManager::requestAccessTokenFinished() { // Failure. // ####### TODO: Error object fields to report. [plugin] - qCDebug(networking) << "Error in response for password grant -" << rootObject["error"].toString(); + qCDebug(networking) << "Error in response for password grant -" << httpStatus << requestReply->error() + << "_" << rootObject["error"].toString(); emit loginFailed(); } }