Fix handling of urls containing username/password in QNetworkAccessManager

QNetworkAccessManager was ignoring the supplied credentials, although
webkit seems to support these urls at a higher level.

Following the behaviour of browsers:
We use supplied credentials if authentication is required.
We add supplied credentials to the authentication cache.
We emit authenticationRequired signal if the credentials were wrong.
We do not use previously cached credentials for that url

Synchronous http requests fail, if the credentials were wrong.

Task-number: QTBUG-18107
Change-Id: If46e8eab1511ba8a0f4bbe0d4efaabc4df0b8ab4
Reviewed-by: Richard J. Moore <rich@kde.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp
index 8537502..7be9d27 100644 (file)
--- a/src/network/access/qnetworkaccessmanager.cpp
+++ b/src/network/access/qnetworkaccessmanager.cpp
@@ -1094,6 +1094,16 @@ void QNetworkAccessManagerPrivate::authenticationRequired(QAuthenticator *authen
     // also called when last URL is empty, e.g. on first call
     if (urlForLastAuthentication->isEmpty()
             || url != *urlForLastAuthentication) {
+        // if credentials are included in the url, then use them
+        if (!url.userName().isEmpty()
+            && !url.password().isEmpty()) {
+            authenticator->setUser(url.userName());
+            authenticator->setPassword(url.password());
+            *urlForLastAuthentication = url;
+            authenticationManager->cacheCredentials(url, authenticator);
+            return;
+        }
+
         QNetworkAuthenticationCredential cred = authenticationManager->fetchCachedCredentials(url, authenticator);
         if (!cred.isNull()) {
             authenticator->setUser(cred.user);

diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
index 2796139..279570b 100644 (file)
--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
@@ -2823,12 +2823,21 @@ void tst_QNetworkReply::ioGetFromHttpWithAuth_data()
 {
     QTest::addColumn<QUrl>("url");
     QTest::addColumn<QByteArray>("expectedData");
+    QTest::addColumn<int>("expectedAuth");
 
     QFile reference(testDataDir + "/rfc3252.txt");
     reference.open(QIODevice::ReadOnly);
     QByteArray referenceData = reference.readAll();
-    QTest::newRow("basic") << QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt") << referenceData;
-    QTest::newRow("digest") << QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/auth-digest/") << QByteArray("digest authentication successful\n");
+    QTest::newRow("basic") << QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt") << referenceData << 1;
+    QTest::newRow("digest") << QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/auth-digest/") << QByteArray("digest authentication successful\n") << 1;
+    //if url contains username & password, then it should be used
+    QTest::newRow("basic-in-url") << QUrl("http://httptest:httptest@" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt") << referenceData << 0;
+    QTest::newRow("digest-in-url") << QUrl("http://httptest:httptest@" + QtNetworkSettings::serverName() + "/qtest/auth-digest/") << QByteArray("digest authentication successful\n") << 0;
+    // if url contains incorrect credentials, expect QNAM to ask for good ones (even if cached - matches behaviour of browsers)
+    QTest::newRow("basic-bad-user-in-url") << QUrl("http://baduser:httptest@" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt") << referenceData << 3;
+    QTest::newRow("basic-bad-password-in-url") << QUrl("http://httptest:wrong@" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt") << referenceData << 3;
+    QTest::newRow("digest-bad-user-in-url") << QUrl("http://baduser:httptest@" + QtNetworkSettings::serverName() + "/qtest/auth-digest/") << QByteArray("digest authentication successful\n") << 3;
+    QTest::newRow("digest-bad-password-in-url") << QUrl("http://httptest:wrong@" + QtNetworkSettings::serverName() + "/qtest/auth-digest/") << QByteArray("digest authentication successful\n") << 3;
 }
 
 void tst_QNetworkReply::ioGetFromHttpWithAuth()
@@ -2839,6 +2848,7 @@ void tst_QNetworkReply::ioGetFromHttpWithAuth()
 
     QFETCH(QUrl, url);
     QFETCH(QByteArray, expectedData);
+    QFETCH(int, expectedAuth);
     QNetworkRequest request(url);
     {
         QNetworkReplyPtr reply1 = manager.get(request);
@@ -2862,7 +2872,8 @@ void tst_QNetworkReply::ioGetFromHttpWithAuth()
         QCOMPARE(reader1.data, expectedData);
         QCOMPARE(reader2.data, expectedData);
 
-        QCOMPARE(authspy.count(), 1);
+        QCOMPARE(authspy.count(), (expectedAuth ? 1 : 0));
+        expectedAuth = qMax(0, expectedAuth - 1);
     }
 
     // rinse and repeat:
@@ -2882,7 +2893,8 @@ void tst_QNetworkReply::ioGetFromHttpWithAuth()
         QCOMPARE(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200);
         QCOMPARE(reader.data, expectedData);
 
-        QCOMPARE(authspy.count(), 0);
+        QCOMPARE(authspy.count(), (expectedAuth ? 1 : 0));
+        expectedAuth = qMax(0, expectedAuth - 1);
     }
 
     // now check with synchronous calls:
@@ -2894,14 +2906,45 @@ void tst_QNetworkReply::ioGetFromHttpWithAuth()
         QSignalSpy authspy(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*)));
         QNetworkReplyPtr replySync = manager.get(request);
         QVERIFY(replySync->isFinished()); // synchronous
-        QCOMPARE(authspy.count(), 0);
+        if (expectedAuth) {
+            // bad credentials in a synchronous request should just fail
+            QCOMPARE(replySync->error(), QNetworkReply::AuthenticationRequiredError);
+        } else {
+            QCOMPARE(authspy.count(), 0);
 
-        // we cannot use a data reader here, since that connects to the readyRead signal,
-        // just use readAll()
+            // we cannot use a data reader here, since that connects to the readyRead signal,
+            // just use readAll()
 
-        // the only thing we check here is that the auth cache was used when using synchronous requests
-        QCOMPARE(replySync->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200);
-        QCOMPARE(replySync->readAll(), expectedData);
+            // the only thing we check here is that the auth cache was used when using synchronous requests
+            QCOMPARE(replySync->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200);
+            QCOMPARE(replySync->readAll(), expectedData);
+        }
+    }
+
+    // check that credentials are used from cache if the same url is requested without credentials
+    {
+        url.setUserInfo(QString());
+        request.setUrl(url);
+        request.setAttribute(
+                QNetworkRequest::SynchronousRequestAttribute,
+                true);
+
+        QSignalSpy authspy(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*)));
+        QNetworkReplyPtr replySync = manager.get(request);
+        QVERIFY(replySync->isFinished()); // synchronous
+        if (expectedAuth) {
+            // bad credentials in a synchronous request should just fail
+            QCOMPARE(replySync->error(), QNetworkReply::AuthenticationRequiredError);
+        } else {
+            QCOMPARE(authspy.count(), 0);
+
+            // we cannot use a data reader here, since that connects to the readyRead signal,
+            // just use readAll()
+
+            // the only thing we check here is that the auth cache was used when using synchronous requests
+            QCOMPARE(replySync->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200);
+            QCOMPARE(replySync->readAll(), expectedData);
+        }
     }
 }