Fix for second temporary token being interpreted as success

The code now keeps track of whether or not the final tokens were
actually requested.

Before if we received temporary tokens a second time it would behave as
if the authorization succeeded and would simply fail any actual
authorizing and would stop attempts to authorize again. By keeping
track of whether or not an actual request was made we can now tell the
temporary and the final tokens apart.

Change-Id: I4a55e7beda9e7be9d8c693739f4d3985438837ea
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Jesus Fernandez <Jesus.Fernandez@qt.io>

3 files changed, 69 insertions, 1 deletions

diff --git a/src/oauth/qoauth1.cpp b/src/oauth/qoauth1.cpp
index fcff92e..37408fc 100644
--- a/src/oauth/qoauth1.cpp
+++ b/src/oauth/qoauth1.cpp
@@ -262,6 +262,15 @@ void QOAuth1Private::_q_tokensReceived(const QVariantMap &tokens)
 {
     Q_Q(QOAuth1);
 
+    if (!tokenRequested && status == QAbstractOAuth::Status::TemporaryCredentialsReceived) {
+        // We didn't request a token yet, but in the "TemporaryCredentialsReceived" state _any_
+        // new tokens received will count as a successful authentication and we move to the
+        // 'Granted' state. To avoid this, 'status' will be temporarily set to 'NotAuthenticated'.
+        status = QAbstractOAuth::Status::NotAuthenticated;
+    }
+    if (tokenRequested) // 'Reset' tokenRequested now that we've gotten new tokens
+        tokenRequested = false;
+
     QPair<QString, QString> credential(tokens.value(Key::oauthToken).toString(),
                                        tokens.value(Key::oauthTokenSecret).toString());
     switch (status) {
@@ -675,6 +684,7 @@ QNetworkReply *QOAuth1::requestTokenCredentials(QNetworkAccessManager::Operation
                                                 const QVariantMap &parameters)
 {
     Q_D(QOAuth1);
+    d->tokenRequested = true;
     return d->requestToken(operation, url, temporaryToken, parameters);
 }
 
@@ -786,7 +796,7 @@ void QOAuth1::grant()
         qCWarning(d->loggingCategory, "authorizationGrantUrl is empty");
         return;
     }
-    if (!d->token.isEmpty()) {
+    if (!d->token.isEmpty() && status() == Status::Granted) {
         qCWarning(d->loggingCategory, "Already authenticated");
         return;
     }
diff --git a/src/oauth/qoauth1_p.h b/src/oauth/qoauth1_p.h
index ecc5cc4..ee57fba 100644
--- a/src/oauth/qoauth1_p.h
+++ b/src/oauth/qoauth1_p.h
@@ -93,6 +93,7 @@ public:
     QUrl tokenCredentialsUrl;
     QOAuth1::SignatureMethod signatureMethod = QOAuth1::SignatureMethod::Hmac_Sha1;
     const QString oauthVersion = QStringLiteral("1.0");
+    bool tokenRequested = false;
 
     struct OAuth1KeyString
     {
diff --git a/tests/auto/oauth1/tst_oauth1.cpp b/tests/auto/oauth1/tst_oauth1.cpp
index 0ca373d..2385863 100644
--- a/tests/auto/oauth1/tst_oauth1.cpp
+++ b/tests/auto/oauth1/tst_oauth1.cpp
@@ -160,6 +160,8 @@ private Q_SLOTS:
 
     void authenticatedCalls_data();
     void authenticatedCalls();
+
+    void secondTemporaryToken();
 };
 
 bool hostReachable(const QLatin1String &host)
@@ -712,5 +714,60 @@ void tst_OAuth1::authenticatedCalls()
     reply.clear();
 }
 
+void tst_OAuth1::secondTemporaryToken()
+{
+    QNetworkAccessManager networkAccessManager;
+
+    const StringPair expectedToken(qMakePair(QStringLiteral("temporaryKey"), QStringLiteral("temporaryToken")));
+    WebServer webServer([&](const WebServer::HttpRequest &request, QTcpSocket *socket) {
+        Q_UNUSED(request);
+        const QString format = "oauth_token=%1&oauth_token_secret=%2&oauth_callback_confirmed=true";
+        const QByteArray text = format.arg(expectedToken.first, expectedToken.second).toUtf8();
+        const QByteArray replyMessage {
+            "HTTP/1.0 200 OK\r\n"
+            "Content-Type: application/x-www-form-urlencoded; charset=\"utf-8\"\r\n"
+            "Content-Length: " + QByteArray::number(text.size()) + "\r\n\r\n"
+            + text
+        };
+        socket->write(replyMessage);
+    });
+
+    QOAuth1 o1(&networkAccessManager);
+
+    StringPair clientCredentials = qMakePair(QStringLiteral("user"), QStringLiteral("passwd"));
+    o1.setClientCredentials(clientCredentials);
+    o1.setTemporaryCredentialsUrl(webServer.url(QStringLiteral("temporary")));
+    o1.setAuthorizationUrl(webServer.url(QStringLiteral("authorization")));
+    o1.setTokenCredentialsUrl(webServer.url(QStringLiteral("token")));
+
+    StringPair tokenReceived;
+    connect(&o1, &QOAuth1::tokenChanged, [&tokenReceived](const QString &token) {
+        tokenReceived.first = token;
+    });
+    bool replyReceived = false;
+    connect(&o1, &QOAuth1::tokenSecretChanged, [&tokenReceived, &replyReceived](const QString &tokenSecret) {
+        tokenReceived.second = tokenSecret;
+        replyReceived = true;
+    });
+
+    o1.grant();
+    QTRY_VERIFY(replyReceived);
+
+    QVERIFY(!tokenReceived.first.isEmpty());
+    QVERIFY(!tokenReceived.second.isEmpty());
+    QCOMPARE(o1.status(), QAbstractOAuth::Status::TemporaryCredentialsReceived);
+    QCOMPARE(tokenReceived, expectedToken);
+
+    replyReceived = false; // reset this so we can 'synchronize' on it again
+    // Do the same request again, should end up in the same state!!
+    o1.grant();
+    QTRY_VERIFY(replyReceived);
+
+    QVERIFY(!tokenReceived.first.isEmpty());
+    QVERIFY(!tokenReceived.second.isEmpty());
+    QCOMPARE(o1.status(), QAbstractOAuth::Status::TemporaryCredentialsReceived);
+    QCOMPARE(tokenReceived, expectedToken);
+}
+
 QTEST_MAIN(tst_OAuth1)
 #include "tst_oauth1.moc"