From e47b8ffacabf42b8a023860b2f3c3ea28509d66f Mon Sep 17 00:00:00 2001
From: Christian Kamm <mail@ckamm.de>
Date: Mon, 11 Sep 2017 12:24:29 +0200
Subject: [PATCH] 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.
---
 src/gui/creds/oauth.cpp               | 16 +++++++++++-----
 src/libsync/abstractnetworkjob.cpp    | 11 +++++++++++
 src/libsync/creds/httpcredentials.cpp | 18 ++++++++++++------
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/gui/creds/oauth.cpp b/src/gui/creds/oauth.cpp
index 4f0b72a9f..386f90190 100644
--- 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 55bbb0a72..de68182ef 100644
--- 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 53f4b89ce..d88791d72 100644
--- 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;
-- 
2.30.2