OAuth: Use redirectable jobs for oauth token management
authorChristian Kamm <mail@ckamm.de>
Fri, 8 Sep 2017 14:43:59 +0000 (16:43 +0200)
committerRoeland Jago Douma <roeland@famdouma.nl>
Thu, 5 Oct 2017 20:01:32 +0000 (22:01 +0200)
src/gui/creds/oauth.cpp
src/libsync/abstractnetworkjob.cpp
src/libsync/abstractnetworkjob.h
src/libsync/account.cpp
src/libsync/account.h
src/libsync/creds/httpcredentials.cpp
src/libsync/networkjobs.cpp
src/libsync/networkjobs.h

index e711755b577643519d64a3c834d4fd571f38df71..9926700f7db5fa8885cc2a6d572cf2d3dd305de6 100644 (file)
@@ -20,6 +20,7 @@
 #include <QJsonObject>
 #include <QJsonDocument>
 #include "theme.h"
+#include "networkjobs.h"
 
 namespace OCC {
 
@@ -82,9 +83,8 @@ void OAuth::start()
                 requestToken.setPassword(Theme::instance()->oauthClientSecret());
                 QNetworkRequest req;
                 req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
-                auto reply = _account->sendRequest("POST", requestToken, req);
-                QTimer::singleShot(30 * 1000, reply, &QNetworkReply::abort);
-                QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, socket] {
+                auto job = _account->sendRequest("POST", requestToken, req);
+                QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this, socket](QNetworkReply *reply) {
                     auto jsonData = reply->readAll();
                     QJsonParseError jsonParseError;
                     QJsonObject json = QJsonDocument::fromJson(jsonData, &jsonParseError).object();
index 070a0e68951b16201669866d5af1c09d17989fe9..55bbb0a72c04f55d5c54479f094843b34a6333a5 100644 (file)
@@ -120,15 +120,20 @@ QNetworkReply *AbstractNetworkJob::addTimer(QNetworkReply *reply)
 QNetworkReply *AbstractNetworkJob::sendRequest(const QByteArray &verb, const QUrl &url,
     QNetworkRequest req, QIODevice *requestBody)
 {
-    auto reply = _account->sendRequest(verb, url, req, requestBody);
+    auto reply = _account->sendRawRequest(verb, url, req, requestBody);
     _requestBody = requestBody;
     if (_requestBody) {
         _requestBody->setParent(reply);
     }
+    adoptRequest(reply);
+    return reply;
+}
+
+void AbstractNetworkJob::adoptRequest(QNetworkReply *reply)
+{
     addTimer(reply);
     setReply(reply);
     setupConnections(reply);
-    return reply;
 }
 
 QUrl AbstractNetworkJob::makeAccountUrl(const QString &relativePath) const
index 03de2d45884d950aa169cdb42fbdd8a7eb3c8fb4..ae61e0c588063d9f729d596aab6b3ca4dfdaa178 100644 (file)
@@ -126,6 +126,13 @@ protected:
         QNetworkRequest req = QNetworkRequest(),
         QIODevice *requestBody = 0);
 
+    /** Makes this job drive a pre-made QNetworkReply
+     *
+     * This reply cannot have a QIODevice request body because we can't get
+     * at it and thus not resend it in case of redirects.
+     */
+    void adoptRequest(QNetworkReply *reply);
+
     /// Creates a url for the account from a relative path
     QUrl makeAccountUrl(const QString &relativePath) const;
 
index 2caa58713f2fff187d0bc052c35422cb2ffb2b79..d70e7ac9e864e46c37a190391f285b25ec2b6250 100644 (file)
@@ -226,7 +226,7 @@ QSharedPointer<QNetworkAccessManager> Account::sharedNetworkAccessManager()
     return _am;
 }
 
-QNetworkReply *Account::sendRequest(const QByteArray &verb, const QUrl &url, QNetworkRequest req, QIODevice *data)
+QNetworkReply *Account::sendRawRequest(const QByteArray &verb, const QUrl &url, QNetworkRequest req, QIODevice *data)
 {
     req.setUrl(url);
     req.setSslConfiguration(this->getOrCreateSslConfig());
@@ -244,6 +244,13 @@ QNetworkReply *Account::sendRequest(const QByteArray &verb, const QUrl &url, QNe
     return _am->sendCustomRequest(req, verb, data);
 }
 
+SimpleNetworkJob *Account::sendRequest(const QByteArray &verb, const QUrl &url, QNetworkRequest req, QIODevice *data)
+{
+    auto job = new SimpleNetworkJob(sharedFromThis(), this);
+    job->startRequest(verb, url, req, data);
+    return job;
+}
+
 void Account::setSslConfiguration(const QSslConfiguration &config)
 {
     _sslConfiguration = config;
index fa2ccb8f98df04d62cc34a360c65970f5fbd5bb1..c9f361feb8913e87658fa1ebd17679562592ace7 100644 (file)
@@ -44,6 +44,7 @@ class Account;
 typedef QSharedPointer<Account> AccountPtr;
 class QuotaInfo;
 class AccessManager;
+class SimpleNetworkJob;
 
 
 /**
@@ -114,9 +115,22 @@ public:
     AbstractCredentials *credentials() const;
     void setCredentials(AbstractCredentials *cred);
 
+    /** Create a network request on the account's QNAM.
+     *
+     * Network requests in AbstractNetworkJobs are created through
+     * this function. Other places should prefer to use jobs or
+     * sendRequest().
+     */
+    QNetworkReply *sendRawRequest(const QByteArray &verb,
+        const QUrl &url,
+        QNetworkRequest req = QNetworkRequest(),
+        QIODevice *data = 0);
 
-    // For creating various network requests
-    QNetworkReply *sendRequest(const QByteArray &verb,
+    /** Create and start network job for a simple one-off request.
+     *
+     * More complicated requests typically create their own job types.
+     */
+    SimpleNetworkJob *sendRequest(const QByteArray &verb,
         const QUrl &url,
         QNetworkRequest req = QNetworkRequest(),
         QIODevice *data = 0);
index 82c0c768d8caf28afa8122ce8b094abb00b46619..53f4b89cefecf1f21ee1d8408b970f6ad61dd1a5 100644 (file)
@@ -303,10 +303,8 @@ bool HttpCredentials::refreshAccessToken()
     requestToken.setPassword(Theme::instance()->oauthClientSecret());
     QNetworkRequest req;
     req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
-    auto reply = _account->sendRequest("POST", requestToken, req);
-    QTimer::singleShot(30 * 1000, reply, &QNetworkReply::abort);
-    QObject::connect(reply, &QNetworkReply::finished, this, [this, reply] {
-        reply->deleteLater();
+    auto job = _account->sendRequest("POST", requestToken, req);
+    QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this](QNetworkReply *reply) {
         auto jsonData = reply->readAll();
         QJsonParseError jsonParseError;
         QJsonObject json = QJsonDocument::fromJson(jsonData, &jsonParseError).object();
index 37fd1b71a2ad7f412d182aea7ac718a4165ded93..3a87571cf94ccc45680e9c613f1ecf4efcc0f8bc 100644 (file)
@@ -886,4 +886,23 @@ void DetermineAuthTypeJob::send(const QUrl &url)
     sendRequest("GET", url, req);
 }
 
+SimpleNetworkJob::SimpleNetworkJob(AccountPtr account, QObject *parent)
+    : AbstractNetworkJob(account, QString(), parent)
+{
+}
+
+QNetworkReply *SimpleNetworkJob::startRequest(const QByteArray &verb, const QUrl &url,
+    QNetworkRequest req, QIODevice *requestBody)
+{
+    auto reply = sendRequest(verb, url, req, requestBody);
+    start();
+    return reply;
+}
+
+bool SimpleNetworkJob::finished()
+{
+    emit finishedSignal(reply());
+    return true;
+}
+
 } // namespace OCC
index 7992be25ae0ecd2397bc332884b906bee57dc4aa..2965d5ae382f47bfb8d9854e82fee29d770fd684 100644 (file)
@@ -377,6 +377,28 @@ private:
     int _redirects;
 };
 
+/**
+ * @brief A basic job around a network request without extra funtionality
+ * @ingroup libsync
+ *
+ * Primarily adds timeout and redirection handling.
+ */
+class OWNCLOUDSYNC_EXPORT SimpleNetworkJob : public AbstractNetworkJob
+{
+    Q_OBJECT
+public:
+    explicit SimpleNetworkJob(AccountPtr account, QObject *parent = 0);
+
+    QNetworkReply *startRequest(const QByteArray &verb, const QUrl &url,
+        QNetworkRequest req = QNetworkRequest(),
+        QIODevice *requestBody = 0);
+
+signals:
+    void finishedSignal(QNetworkReply *reply);
+private slots:
+    bool finished() Q_DECL_OVERRIDE;
+};
+
 } // namespace OCC
 
 #endif // NETWORKJOBS_H