Merge pull request #10845 from birarda/bug/domain-cookie-restart

fix for OAuth OPTIONS firing every XHR 302

domain-server/resources/web/js/tables.js

@@ -2,11 +2,11 @@ $(document).ready(function(){
   // setup the underscore templates
   var nodeTemplate = _.template($('#nodes-template').html());
   var queuedTemplate = _.template($('#queued-template').html());
-  
+
   // setup a function to grab the assignments
   function getNodesAndAssignments() {
     $.getJSON("nodes.json", function(json){
-      
+
       json.nodes.sort(function(a, b){
           if (a.type === b.type) {
             if (a.uptime < b.uptime) {
@@ -16,36 +16,50 @@ $(document).ready(function(){
             } else {
               return 0;
             }
-          } 
+          }
-              
+
           if (a.type === "agent" && b.type !== "agent") {
             return 1;
           } else if (b.type === "agent" && a.type !== "agent") {
             return -1;
           }
-          
+
           if (a.type > b.type) {
             return 1;
           }
-          
+
           if (a.type < b.type) {
             return -1;
-          }   
+          }
       });
-      
+
       $('#nodes-table tbody').html(nodeTemplate(json));
+    }).fail(function(jqXHR, textStatus, errorThrown) {
+      // we assume a 401 means the DS has restarted
+      // and no longer has our OAuth produced uuid
+      // so just reload and re-auth
+      if (jqXHR.status == 401) {
+        location.reload();
+      }
     });
-    
+
-    $.getJSON("assignments.json", function(json){      
+    $.getJSON("assignments.json", function(json){
       $('#assignments-table tbody').html(queuedTemplate(json));
+    }).fail(function(jqXHR, textStatus, errorThrown) {
+      // we assume a 401 means the DS has restarted
+      // and no longer has our OAuth produced uuid
+      // so just reload and re-auth
+      if (jqXHR.status == 401) {
+        location.reload();
+      }
     });
   }
-  
+
   // do the first GET on page load
   getNodesAndAssignments();
   // grab the new assignments JSON every two seconds
   var getNodesAndAssignmentsInterval = setInterval(getNodesAndAssignments, 2000);
-  
+
   // hook the node delete to the X button
   $(document.body).on('click', '.glyphicon-remove', function(){
     // fire off a delete for this node
@@ -57,10 +71,10 @@ $(document).ready(function(){
         }
     });
   });
-  
+
   $(document.body).on('click', '#kill-all-btn', function() {
     var confirmed_kill = confirm("Are you sure?");
-    
+
     if (confirmed_kill == true) {
       $.ajax({
         url: "/nodes/",

domain-server/src/DomainServer.cpp

@@ -1974,7 +1974,8 @@ bool DomainServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
     return _settingsManager.handleAuthenticatedHTTPRequest(connection, url);
 }
 
-const QString HIFI_SESSION_COOKIE_KEY = "DS_WEB_SESSION_UUID";
+static const QString HIFI_SESSION_COOKIE_KEY = "DS_WEB_SESSION_UUID";
+static const QString STATE_QUERY_KEY = "state";
 
 bool DomainServer::handleHTTPSRequest(HTTPSConnection* connection, const QUrl &url, bool skipSubHandler) {
     qDebug() << "HTTPS request received at" << url.toString();
@@ -1985,10 +1986,9 @@ bool DomainServer::handleHTTPSRequest(HTTPSConnection* connection, const QUrl &u
         const QString CODE_QUERY_KEY = "code";
         QString authorizationCode = codeURLQuery.queryItemValue(CODE_QUERY_KEY);
 
-        const QString STATE_QUERY_KEY = "state";
         QUuid stateUUID = QUuid(codeURLQuery.queryItemValue(STATE_QUERY_KEY));
 
-        if (!authorizationCode.isEmpty() && !stateUUID.isNull()) {
+        if (!authorizationCode.isEmpty() && !stateUUID.isNull() && _webAuthenticationStateSet.remove(stateUUID)) {
             // fire off a request with this code and state to get an access token for the user
 
             const QString OAUTH_TOKEN_REQUEST_PATH = "/oauth/token";
@@ -2006,47 +2006,83 @@ bool DomainServer::handleHTTPSRequest(HTTPSConnection* connection, const QUrl &u
             tokenRequest.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
 
             QNetworkReply* tokenReply = NetworkAccessManager::getInstance().post(tokenRequest, tokenPostBody.toLocal8Bit());
+            connect(tokenReply, &QNetworkReply::finished, this, &DomainServer::tokenGrantFinished);
 
-            if (_webAuthenticationStateSet.remove(stateUUID)) {
+            // add this connection to our list of pending connections so that we can hold the response
-                // this is a web user who wants to auth to access web interface
+            _pendingOAuthConnections.insert(stateUUID, connection);
-                // we hold the response back to them until we get their profile information
-                // and can decide if they are let in or not
 
-                QEventLoop loop;
+            // set the state UUID on the reply so that we can associate the response with the connection later
-                connect(tokenReply, &QNetworkReply::finished, &loop, &QEventLoop::quit);
+            tokenReply->setProperty(STATE_QUERY_KEY.toLocal8Bit(), stateUUID);
 
-                // start the loop for the token request
+            return true;
-                loop.exec();
+        } else {
+            connection->respond(HTTPConnection::StatusCode400);
 
-                QNetworkReply* profileReply = profileRequestGivenTokenReply(tokenReply);
+            return true;
+        }
+    } else {
+        return false;
+    }
+}
 
-                // stop the loop once the profileReply is complete
+HTTPSConnection* DomainServer::connectionFromReplyWithState(QNetworkReply* reply) {
-                connect(profileReply, &QNetworkReply::finished, &loop, &QEventLoop::quit);
+    // grab the UUID state property from the reply
+    QUuid stateUUID = reply->property(STATE_QUERY_KEY.toLocal8Bit()).toUuid();
 
-                // restart the loop for the profile request
+    if (!stateUUID.isNull()) {
-                loop.exec();
+        return _pendingOAuthConnections.take(stateUUID);
+    } else {
+        return nullptr;
+    }
+}
 
+void DomainServer::tokenGrantFinished() {
+    auto tokenReply = qobject_cast<QNetworkReply*>(sender());
+
+    if (tokenReply) {
+        if (tokenReply->error() == QNetworkReply::NoError) {
+            // now that we have a token for this profile, send off a profile request
+            QNetworkReply* profileReply = profileRequestGivenTokenReply(tokenReply);
+
+            // forward along the state UUID that we kept with the token request
+            profileReply->setProperty(STATE_QUERY_KEY.toLocal8Bit(), tokenReply->property(STATE_QUERY_KEY.toLocal8Bit()));
+
+            connect(profileReply, &QNetworkReply::finished, this, &DomainServer::profileRequestFinished);
+        } else {
+            // the token grant failed, send back a 500 (assuming the connection is still around)
+            auto connection = connectionFromReplyWithState(tokenReply);
+
+            if (connection) {
+                connection->respond(HTTPConnection::StatusCode500);
+            }
+        }
+
+        tokenReply->deleteLater();
+    }
+}
+
+void DomainServer::profileRequestFinished() {
+
+    auto profileReply = qobject_cast<QNetworkReply*>(sender());
+
+    if (profileReply) {
+        auto connection = connectionFromReplyWithState(profileReply);
+
+        if (connection) {
+            if (profileReply->error() == QNetworkReply::NoError) {
                 // call helper method to get cookieHeaders
                 Headers cookieHeaders = setupCookieHeadersFromProfileReply(profileReply);
 
                 connection->respond(HTTPConnection::StatusCode302, QByteArray(),
                                     HTTPConnection::DefaultContentType, cookieHeaders);
 
-                delete tokenReply;
+            } else {
-                delete profileReply;
+                // the profile request failed, send back a 500 (assuming the connection is still around)
-
+                connection->respond(HTTPConnection::StatusCode500);
-                // we've redirected the user back to our homepage
-                return true;
-
             }
         }
 
-        // respond with a 200 code indicating that login is complete
+        profileReply->deleteLater();
-        connection->respond(HTTPConnection::StatusCode200);
-
-        return true;
-    } else {
-        return false;
     }
 }
 
@@ -2106,22 +2142,31 @@ bool DomainServer::isAuthenticatedRequest(HTTPConnection* connection, const QUrl
             // the user does not have allowed username or role, return 401
             return false;
         } else {
-            // re-direct this user to OAuth page
+            static const QByteArray REQUESTED_WITH_HEADER = "X-Requested-With";
+            static const QString XML_REQUESTED_WITH = "XMLHttpRequest";
 
-            // generate a random state UUID to use
+            if (connection->requestHeaders().value(REQUESTED_WITH_HEADER) == XML_REQUESTED_WITH) {
-            QUuid stateUUID = QUuid::createUuid();
+                // unauthorized XHR requests get a 401 and not a 302, since there isn't an XHR
+                // path to OAuth authorize
+                connection->respond(HTTPConnection::StatusCode401, UNAUTHENTICATED_BODY);
+            } else {
+                // re-direct this user to OAuth page
 
-            // add it to the set so we can handle the callback from the OAuth provider
+                // generate a random state UUID to use
-            _webAuthenticationStateSet.insert(stateUUID);
+                QUuid stateUUID = QUuid::createUuid();
 
-            QUrl authURL = oauthAuthorizationURL(stateUUID);
+                // add it to the set so we can handle the callback from the OAuth provider
+                _webAuthenticationStateSet.insert(stateUUID);
 
-            Headers redirectHeaders;
+                QUrl authURL = oauthAuthorizationURL(stateUUID);
 
-            redirectHeaders.insert("Location", authURL.toEncoded());
+                Headers redirectHeaders;
 
-            connection->respond(HTTPConnection::StatusCode302,
+                redirectHeaders.insert("Location", authURL.toEncoded());
-                                QByteArray(), HTTPConnection::DefaultContentType, redirectHeaders);
+
+                connection->respond(HTTPConnection::StatusCode302,
+                                    QByteArray(), HTTPConnection::DefaultContentType, redirectHeaders);
+            }
 
             // we don't know about this user yet, so they are not yet authenticated
             return false;

domain-server/src/DomainServer.h

@@ -111,6 +111,9 @@ private slots:
     void updateDownstreamNodes();
     void updateUpstreamNodes();
 
+    void tokenGrantFinished();
+    void profileRequestFinished();
+
 signals:
     void iceServerChanged();
     void userConnected();
@@ -178,6 +181,8 @@ private:
 
     void updateReplicationNodes(ReplicationServerDirection direction);
 
+    HTTPSConnection* connectionFromReplyWithState(QNetworkReply* reply);
+
     SubnetList _acSubnetWhitelist;
 
     std::vector<QString> _replicatedUsernames;
@@ -235,6 +240,8 @@ private:
 
     bool _sendICEServerAddressToMetaverseAPIInProgress { false };
     bool _sendICEServerAddressToMetaverseAPIRedo { false };
+
+    QHash<QUuid, QPointer<HTTPSConnection>> _pendingOAuthConnections;
 };