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).

2 files changed, 47 insertions, 27 deletions

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;