BaseJob: Don't send accessToken if not needed; send again on 401

The first part closes #358; the second part is a workaround for non-standard
cases when endpoints without security by the spec turn out to be secured
(in particular, the case of authenticating media servers).

5 files changed, 66 insertions, 31 deletions

diff --git a/lib/connection.cpp b/lib/connection.cpp
index 5ee7e84e..c830557c 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -343,7 +343,8 @@ void Connection::logout()
     }
     const auto* job = callApi<LogoutJob>();
     connect(job, &LogoutJob::finished, this, [this, job, syncWasRunning] {
-        if (job->status().good() || job->error() == BaseJob::ContentAccessError) {
+        if (job->status().good() || job->error() == BaseJob::Unauthorised
+            || job->error() == BaseJob::ContentAccessError) {
             if (d->syncLoopConnection)
                 disconnect(d->syncLoopConnection);
             d->data->setToken({});
@@ -380,9 +381,9 @@ void Connection::sync(int timeout)
             });
     connect(job, &SyncJob::failure, this, [this, job] {
         d->syncJob = nullptr;
-        if (job->error() == BaseJob::ContentAccessError) {
-            qCWarning(SYNCJOB) << "Sync job failed with ContentAccessError - "
-                                  "login expired?";
+        if (job->error() == BaseJob::Unauthorised) {
+            qCWarning(SYNCJOB)
+                << "Sync job failed with Unauthorised - login expired?";
             emit loginError(job->errorString(), job->rawDataSample());
         } else
             emit syncError(job->errorString(), job->rawDataSample());
diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp
index cbc36420..e2a0f06f 100644
--- a/lib/connectiondata.cpp
+++ b/lib/connectiondata.cpp
@@ -42,6 +42,7 @@ public:
     QString lastEvent;
     QString userId;
     QString deviceId;
+    std::vector<QString> needToken;
 
     mutable unsigned int txnCounter = 0;
     const qint64 txnBase = QDateTime::currentMSecsSinceEpoch();
@@ -143,6 +144,12 @@ const QString& ConnectionData::deviceId() const { return d->deviceId; }
 
 const QString& ConnectionData::userId() const { return d->userId; }
 
+bool ConnectionData::needsToken(const QString& requestName) const
+{
+    return std::find(d->needToken.cbegin(), d->needToken.cend(), requestName)
+           != d->needToken.cend();
+}
+
 void ConnectionData::setDeviceId(const QString& deviceId)
 {
     d->deviceId = deviceId;
@@ -150,6 +157,11 @@ void ConnectionData::setDeviceId(const QString& deviceId)
 
 void ConnectionData::setUserId(const QString& userId) { d->userId = userId; }
 
+void ConnectionData::setNeedsToken(const QString& requestName)
+{
+    d->needToken.push_back(requestName);
+}
+
 QString ConnectionData::lastEvent() const { return d->lastEvent; }
 
 void ConnectionData::setLastEvent(QString identifier)
diff --git a/lib/connectiondata.h b/lib/connectiondata.h
index b367c977..000099d1 100644
--- a/lib/connectiondata.h
+++ b/lib/connectiondata.h
@@ -40,6 +40,7 @@ public:
     QUrl baseUrl() const;
     const QString& deviceId() const;
     const QString& userId() const;
+    bool needsToken(const QString& requestName) const;
     QNetworkAccessManager* nam() const;
 
     void setBaseUrl(QUrl baseUrl);
@@ -50,6 +51,7 @@ public:
     void setPort(int port);
     void setDeviceId(const QString& deviceId);
     void setUserId(const QString& userId);
+    void setNeedsToken(const QString& requestName);
 
     QString lastEvent() const;
     void setLastEvent(QString identifier);
diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 13e65188..7f558685 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -226,8 +226,9 @@ void BaseJob::Private::sendRequest()
                                          requestQuery) };
     if (!requestHeaders.contains("Content-Type"))
         req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
-    req.setRawHeader("Authorization",
-                     QByteArray("Bearer ") + connection->accessToken());
+    if (needsToken)
+        req.setRawHeader("Authorization",
+                         QByteArray("Bearer ") + connection->accessToken());
     req.setAttribute(QNetworkRequest::BackgroundRequestAttribute, inBackground);
     req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
     req.setMaximumRedirectsAllowed(10);
@@ -274,6 +275,7 @@ void BaseJob::sendRequest()
         return;
     Q_ASSERT(d->connection && status().code == Pending);
     qCDebug(d->logCat).noquote() << "Making" << d->dumpRequest();
+    d->needsToken |= d->connection->needsToken(objectName());
     emit aboutToSendRequest();
     d->sendRequest();
     Q_ASSERT(d->reply);
@@ -341,32 +343,32 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
     return false;
 }
 
-BaseJob::Status BaseJob::Status::fromHttpCode(int httpCode, QString msg)
+BaseJob::StatusCode BaseJob::Status::fromHttpCode(int httpCode)
 {
+    if (httpCode / 10 == 41) // 41x errors
+        return httpCode == 410 ? IncorrectRequestError : NotFoundError;
+    switch (httpCode) {
+    case 401:
+        return Unauthorised;
     // clang-format off
-    return { [httpCode]() -> StatusCode {
-            if (httpCode / 10 == 41) // 41x errors
-                return httpCode == 410 ? IncorrectRequestError : NotFoundError;
-            switch (httpCode) {
-            case 401: case 403: case 407:
-                return ContentAccessError;
-            case 404:
-                return NotFoundError;
-            case 400: case 405: case 406: case 426: case 428: case 505:
-            case 494: // Unofficial nginx "Request header too large"
-            case 497: // Unofficial nginx "HTTP request sent to HTTPS port"
-                return IncorrectRequestError;
-            case 429:
-                return TooManyRequestsError;
-            case 501: case 510:
-                return RequestNotImplementedError;
-            case 511:
-                return NetworkAuthRequiredError;
-            default:
-                return NetworkError;
-            }
-        }(), std::move(msg) };
-    // clang-format on
+    case 403: case 407: // clang-format on
+        return ContentAccessError;
+    case 404:
+        return NotFoundError;
+    // clang-format off
+    case 400: case 405: case 406: case 426: case 428: case 505: // clang-format on
+    case 494: // Unofficial nginx "Request header too large"
+    case 497: // Unofficial nginx "HTTP request sent to HTTPS port"
+        return IncorrectRequestError;
+    case 429:
+        return TooManyRequestsError;
+    case 501: case 510:
+        return RequestNotImplementedError;
+    case 511:
+        return NetworkAuthRequiredError;
+    default:
+        return NetworkError;
+    }
 }
 
 QDebug BaseJob::Status::dumpToLog(QDebug dbg) const
@@ -496,6 +498,16 @@ void BaseJob::finishJob()
         d->connection->submit(this);
         return;
     }
+    if (error() == Unauthorised && !d->needsToken
+        && !d->connection->accessToken().isEmpty()) {
+        // Rerun with access token (extension of the spec while
+        // https://github.com/matrix-org/matrix-doc/issues/701 is pending)
+        d->connection->setNeedsToken(objectName());
+        qCWarning(d->logCat) << this << "re-running with authentication";
+        emit retryScheduled(d->retriesTaken, 0);
+        setStatus(Pending);
+        sendRequest();
+    }
     if ((error() == NetworkError || error() == Timeout)
         && d->retriesTaken < d->maxRetries) {
         // TODO: The whole retrying thing should be put to Connection(Manager)
@@ -596,6 +608,8 @@ QString BaseJob::statusCaption() const
         return tr("Network problems");
     case TimeoutError:
         return tr("Request timed out");
+    case Unauthorised:
+        return tr("Unauthorised request");
     case ContentAccessError:
         return tr("Access error");
     case NotFoundError:
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index c4da40f5..ecbec74a 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -60,6 +60,7 @@ public:
         NetworkError = 100,
         Timeout,
         TimeoutError = Timeout,
+        Unauthorised,
         ContentAccessError,
         NotFoundError,
         IncorrectRequest,
@@ -113,7 +114,12 @@ public:
     struct Status {
         Status(StatusCode c) : code(c) {}
         Status(int c, QString m) : code(c), message(std::move(m)) {}
-        static Status fromHttpCode(int httpCode, QString msg = {});
+
+        static StatusCode fromHttpCode(int httpCode);
+        static Status fromHttpCode(int httpCode, QString msg)
+        {
+            return { fromHttpCode(httpCode), std::move(msg) };
+        }
 
         bool good() const { return code < ErrorLevel; }
         QDebug dumpToLog(QDebug dbg) const;