From d5cdfe33a246299fd4799a0b34691f19787b0dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 25 Jan 2024 12:39:08 +0100 Subject: [PATCH] Http2: fix 401 authentication required w/o challenge The code did not handle the path where we didn't have a challenge. We cannot recover from that so we just have to fail the request. Amends fe1b668861e8a3ef99e126821fcd3eeaa6044b54 Pick-to: 6.6 6.6.2 6.5 6.2 Fixes: QTBUG-121515 Change-Id: Ie39a92e7439785a09cad28e8f81599a51de5e27f Reviewed-by: Qt CI Bot Reviewed-by: Timur Pocheptsov (cherry picked from commit ffe0271a21e9574d1c9eab5fb9803573e17e0f22) Reviewed-by: Qt Cherry-pick Bot --- src/network/access/qhttp2protocolhandler.cpp | 11 ++++++++++ tests/auto/network/access/http2/http2srv.cpp | 8 +++++++ tests/auto/network/access/http2/http2srv.h | 3 +++ tests/auto/network/access/http2/tst_http2.cpp | 21 +++++++++++++------ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp index 60f7ea12a6c..e887b460042 100644 --- a/src/network/access/qhttp2protocolhandler.cpp +++ b/src/network/access/qhttp2protocolhandler.cpp @@ -1216,6 +1216,17 @@ void QHttp2ProtocolHandler::handleAuthorization(Stream &stream) // closed, so we don't need to call sendRequest ourselves. return true; } // else: Authentication failed or was cancelled + } else { + // No authentication header, but we got a 401/407 so we cannot + // succeed. We need to emit signals for headers and data, and then + // finishWithError. + emit httpReply->headerChanged(); + emit httpReply->readyRead(); + QNetworkReply::NetworkError error = httpReply->statusCode() == 401 + ? QNetworkReply::AuthenticationRequiredError + : QNetworkReply::ProxyAuthenticationRequiredError; + finishStreamWithError(stream, QNetworkReply::AuthenticationRequiredError, + m_connection->d_func()->errorDetail(error, m_socket)); } return false; }; diff --git a/tests/auto/network/access/http2/http2srv.cpp b/tests/auto/network/access/http2/http2srv.cpp index d7c8c2fd8e2..80eef2a341b 100644 --- a/tests/auto/network/access/http2/http2srv.cpp +++ b/tests/auto/network/access/http2/http2srv.cpp @@ -105,6 +105,12 @@ void Http2Server::setAuthenticationHeader(const QByteArray &authentication) authenticationHeader = authentication; } +void Http2Server::setAuthenticationRequired(bool enable) +{ + Q_ASSERT(!enable || authenticationHeader.isEmpty()); + authenticationRequired = enable; +} + void Http2Server::setRedirect(const QByteArray &url, int count) { redirectUrl = url; @@ -864,6 +870,8 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody) } else if (!authenticationHeader.isEmpty() && !hasAuth) { header.push_back({ ":status", "401" }); header.push_back(HPack::HeaderField("www-authenticate", authenticationHeader)); + } else if (authenticationRequired) { + header.push_back({ ":status", "401" }); } else { header.push_back({":status", "200"}); } diff --git a/tests/auto/network/access/http2/http2srv.h b/tests/auto/network/access/http2/http2srv.h index cc5353c8556..11a75c5ef94 100644 --- a/tests/auto/network/access/http2/http2srv.h +++ b/tests/auto/network/access/http2/http2srv.h @@ -65,6 +65,8 @@ public: void setContentEncoding(const QByteArray &contentEncoding); // No authentication data is generated for the method, the full header value must be set void setAuthenticationHeader(const QByteArray &authentication); + // Authentication always required, no challenge provided + void setAuthenticationRequired(bool enable); // Set the redirect URL and count. The server will return a redirect response with the url // 'count' amount of times void setRedirect(const QByteArray &redirectUrl, int count); @@ -202,6 +204,7 @@ private: QByteArray contentEncoding; QByteArray authenticationHeader; + bool authenticationRequired = false; QByteArray redirectUrl; int redirectCount = 0; diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp index 85bcfcb8c42..1b52905f73a 100644 --- a/tests/auto/network/access/http2/tst_http2.cpp +++ b/tests/auto/network/access/http2/tst_http2.cpp @@ -1064,13 +1064,18 @@ void tst_Http2::authenticationRequired_data() { QTest::addColumn("success"); QTest::addColumn("responseHEADOnly"); + QTest::addColumn("withChallenge"); - QTest::addRow("failed-auth") << false << true; - QTest::addRow("successful-auth") << true << true; + QTest::addRow("failed-auth") << false << true << true; + QTest::addRow("successful-auth") << true << true << true; // Include a DATA frame in the response from the remote server. An example would be receiving a // JSON response on a request along with the 401 error. - QTest::addRow("failed-auth-with-response") << false << false; - QTest::addRow("successful-auth-with-response") << true << false; + QTest::addRow("failed-auth-with-response") << false << false << true; + QTest::addRow("successful-auth-with-response") << true << false << true; + + // Don't provide a challenge header. This is valid if you are actually just + // denied access for whatever reason. + QTest::addRow("no-challenge") << false << false << false; } void tst_Http2::authenticationRequired() @@ -1081,11 +1086,15 @@ void tst_Http2::authenticationRequired() POSTResponseHEADOnly = responseHEADOnly; QFETCH(const bool, success); + QFETCH(const bool, withChallenge); ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType())); QByteArray responseBody = "Hello"_ba; targetServer->setResponseBody(responseBody); - targetServer->setAuthenticationHeader("Basic realm=\"Shadow\""); + if (withChallenge) + targetServer->setAuthenticationHeader("Basic realm=\"Shadow\""); + else + targetServer->setAuthenticationRequired(true); QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection); runEventLoop(); @@ -1142,7 +1151,7 @@ void tst_Http2::authenticationRequired() QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); // else: no error (is checked in tst_Http2::replyFinished) - QVERIFY(authenticationRequested); + QVERIFY(authenticationRequested || !withChallenge); const auto isAuthenticated = [](const QByteArray &bv) { return bv == "Basic YWRtaW46YWRtaW4="; // admin:admin