From f23445845310b9f47ea5592f1dea92ca463ba284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Tue, 18 Apr 2017 12:58:35 +0200 Subject: 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 Reviewed-by: Timur Pocheptsov Reviewed-by: Jesus Fernandez --- src/oauth/qoauth1.cpp | 12 ++++++++- src/oauth/qoauth1_p.h | 1 + tests/auto/oauth1/tst_oauth1.cpp | 57 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) 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 credential(tokens.value(Key::oauthToken).toString(), tokens.value(Key::oauthTokenSecret).toString()); switch (status) { @@ -675,6 +684,7 @@ QNetworkReply *QOAuth1::requestTokenCredentials(QNetworkAccessManager::Operation const QVariantMap ¶meters) { 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" -- cgit v1.2.3