OAuth: Don't use implicit POST bodies

The query args of POST requests become the request body. If there's a
redirect, the redirected url will therefore not contain the query
arguments. Use an explicit request body to make the redirection work.

diff --git a/src/gui/creds/oauth.cpp b/src/gui/creds/oauth.cpp
index 4f0b72a9f39466abf248a02cd648cb5df00bc141..386f90190e873cfcefb384dadbed687fb869b510 100644 (file)
--- a/src/gui/creds/oauth.cpp
+++ b/src/gui/creds/oauth.cpp
@@ -15,6 +15,7 @@
 #include <QDesktopServices>
 #include <QNetworkReply>
 #include <QTimer>
+#include <QBuffer>
 #include "account.h"
 #include "creds/oauth.h"
 #include <QJsonObject>
@@ -75,16 +76,21 @@ void OAuth::start()
 
                 QString code = rx.cap(1); // The 'code' is the first capture of the regexp
 
-                QUrl requestToken(_account->url().toString()
-                    + QLatin1String("/index.php/apps/oauth2/api/v1/token?grant_type=authorization_code&code=")
-                    + code
-                    + QLatin1String("&redirect_uri=http://localhost:") + QString::number(_server.serverPort()));
+                QUrl requestToken(_account->url().toString() + QLatin1String("/index.php/apps/oauth2/api/v1/token"));
                 QNetworkRequest req;
                 req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
+
                 QString basicAuth = QString("%1:%2").arg(
                     Theme::instance()->oauthClientId(), Theme::instance()->oauthClientSecret());
                 req.setRawHeader("Authorization", "Basic " + basicAuth.toUtf8().toBase64());
-                auto job = _account->sendRequest("POST", requestToken, req);
+
+                auto requestBody = new QBuffer;
+                QUrlQuery arguments(QString(
+                    "grant_type=authorization_code&code=%1&redirect_uri=http://localhost:%2")
+                                        .arg(code, QString::number(_server.serverPort())));
+                requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1());
+
+                auto job = _account->sendRequest("POST", requestToken, req, requestBody);
                 QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this, socket](QNetworkReply *reply) {
                     auto jsonData = reply->readAll();
                     QJsonParseError jsonParseError;

diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp
index 55bbb0a72c04f55d5c54479f094843b34a6333a5..de68182ef08deaeaab1f76a3612d6bd905ce0bd5 100644 (file)
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -173,6 +173,17 @@ void AbstractNetworkJob::slotFinished()
     if (_followRedirects && !redirectUrl.isEmpty()) {
         _redirectCount++;
 
+        // For POST requests where the target url has query arguments, Qt automatically
+        // moves these arguments to the body if no explicit body is specified.
+        // This can cause problems with redirected requests, because the redirect url
+        // will no longer contain these query arguments.
+        if (reply()->operation() == QNetworkAccessManager::PostOperation
+            && requestedUrl.hasQuery()
+            && !redirectUrl.hasQuery()
+            && !_requestBody) {
+            qCWarning(lcNetworkJob) << "Redirecting a POST request with an implicit body loses that body";
+        }
+
         // ### some of the qWarnings here should be exported via displayErrors() so they
         // ### can be presented to the user if the job executor has a GUI
         QByteArray verb = requestVerb(*reply());

diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp
index 53f4b89cefecf1f21ee1d8408b970f6ad61dd1a5..d88791d7217cd6487f6673ee2ea90bfe5cdb3026 100644 (file)
--- a/src/libsync/creds/httpcredentials.cpp
+++ b/src/libsync/creds/httpcredentials.cpp
@@ -20,6 +20,7 @@
 #include <QSslKey>
 #include <QJsonObject>
 #include <QJsonDocument>
+#include <QBuffer>
 
 #include <keychain.h>
 
@@ -296,14 +297,19 @@ bool HttpCredentials::refreshAccessToken()
     if (_refreshToken.isEmpty())
         return false;
 
-    QUrl requestToken(_account->url().toString()
-        + QLatin1String("/index.php/apps/oauth2/api/v1/token?grant_type=refresh_token&refresh_token=")
-        + _refreshToken);
-    requestToken.setUserName(Theme::instance()->oauthClientId());
-    requestToken.setPassword(Theme::instance()->oauthClientSecret());
+    QUrl requestToken(_account->url().toString() + QLatin1String("/index.php/apps/oauth2/api/v1/token"));
     QNetworkRequest req;
     req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
-    auto job = _account->sendRequest("POST", requestToken, req);
+
+    QString basicAuth = QString("%1:%2").arg(
+        Theme::instance()->oauthClientId(), Theme::instance()->oauthClientSecret());
+    req.setRawHeader("Authorization", "Basic " + basicAuth.toUtf8().toBase64());
+
+    auto requestBody = new QBuffer;
+    QUrlQuery arguments(QString("grant_type=refresh_token&refresh_token=%1").arg(_refreshToken));
+    requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1());
+
+    auto job = _account->sendRequest("POST", requestToken, req, requestBody);
     QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this](QNetworkReply *reply) {
         auto jsonData = reply->readAll();
         QJsonParseError jsonParseError;