From 8ee2e793f8cc6b815600698a4b18ab3d050b9815 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 29 Jan 2019 15:40:42 +1000 Subject: [PATCH] Make network authentication request handling thread safe, avoid races and crashes when auth requests occur in non-main thread Like the recent SSL error handling, this commit abstracts out the network authentication handling into a thread safe approach. --- .../qgsnetworkaccessmanager.sip.in | 22 ++- src/app/CMakeLists.txt | 1 + src/app/qgisapp.cpp | 70 +-------- src/app/qgisapp.h | 1 - src/app/qgsappauthrequesthandler.cpp | 82 +++++++++++ src/app/qgsappauthrequesthandler.h | 30 ++++ src/core/qgsnetworkaccessmanager.cpp | 139 ++++++++++++++---- src/core/qgsnetworkaccessmanager.h | 113 +++++++++++++- .../src/core/testqgsnetworkaccessmanager.cpp | 129 +++++++++++++++- 9 files changed, 485 insertions(+), 102 deletions(-) create mode 100644 src/app/qgsappauthrequesthandler.cpp create mode 100644 src/app/qgsappauthrequesthandler.h diff --git a/python/core/auto_generated/qgsnetworkaccessmanager.sip.in b/python/core/auto_generated/qgsnetworkaccessmanager.sip.in index 8967a3e822a..c1a6d41fbf4 100644 --- a/python/core/auto_generated/qgsnetworkaccessmanager.sip.in +++ b/python/core/auto_generated/qgsnetworkaccessmanager.sip.in @@ -268,6 +268,22 @@ This signal is propagated to the main thread QgsNetworkAccessManager instance, s only to connect to the main thread's signal in order to receive notifications about requests created in any thread. +.. versionadded:: 3.6 +%End + + void requestRequiresAuth( int requestId, const QString &realm ); +%Docstring +Emitted when a network request prompts an authentication request. + +The ``requestId`` argument reflects the unique ID identifying the original request which the authentication relates to. + +This signal is propagated to the main thread QgsNetworkAccessManager instance, so it is necessary +only to connect to the main thread's signal in order to receive notifications about authentication requests +from any thread. + +This signal is for debugging and logging purposes only, and cannot be used to respond to the +requests. See QgsNetworkAuthenticationHandler for details on how to handle authentication requests. + .. versionadded:: 3.6 %End @@ -276,12 +292,15 @@ created in any thread. %Docstring Emitted when a network request encounters SSL ``errors``. -The ``requestId`` argument reflects the unique ID identifying the original request which the progress report relates to. +The ``requestId`` argument reflects the unique ID identifying the original request which the SSL error relates to. This signal is propagated to the main thread QgsNetworkAccessManager instance, so it is necessary only to connect to the main thread's signal in order to receive notifications about SSL errors from any thread. +This signal is for debugging and logging purposes only, and cannot be used to respond to the errors. +See QgsSslErrorHandler for details on how to handle SSL errors and potentially ignore them. + .. versionadded:: 3.6 %End @@ -296,6 +315,7 @@ from any thread. void requestTimedOut( QNetworkReply * ); + protected: virtual QNetworkReply *createRequest( QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *outgoingData = 0 ); diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index f823033268a..d15575874de 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -8,6 +8,7 @@ SET(QGIS_APP_SRCS qgisappstylesheet.cpp qgsabout.cpp qgsalignrasterdialog.cpp + qgsappauthrequesthandler.cpp qgsappbrowserproviders.cpp qgsapplayertreeviewmenuprovider.cpp qgsappwindowmanager.cpp diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp index b6b1186c58f..bec38a33f5c 100644 --- a/src/app/qgisapp.cpp +++ b/src/app/qgisapp.cpp @@ -142,6 +142,7 @@ Q_GUI_EXPORT extern int qt_defaultDpiX(); #include "qgisplugin.h" #include "qgsabout.h" #include "qgsalignrasterdialog.h" +#include "qgsappauthrequesthandler.h" #include "qgsappbrowserproviders.h" #include "qgsapplayertreeviewmenuprovider.h" #include "qgsapplication.h" @@ -13883,85 +13884,18 @@ void QgisApp::namSetup() { QgsNetworkAccessManager *nam = QgsNetworkAccessManager::instance(); - connect( nam, &QNetworkAccessManager::authenticationRequired, - this, &QgisApp::namAuthenticationRequired ); - connect( nam, &QNetworkAccessManager::proxyAuthenticationRequired, this, &QgisApp::namProxyAuthenticationRequired ); connect( nam, qgis::overload< QgsNetworkRequestParameters >::of( &QgsNetworkAccessManager::requestTimedOut ), this, &QgisApp::namRequestTimedOut ); + nam->setAuthHandler( qgis::make_unique() ); #ifndef QT_NO_SSL nam->setSslErrorHandler( qgis::make_unique() ); #endif } -void QgisApp::namAuthenticationRequired( QNetworkReply *inReply, QAuthenticator *auth ) -{ - QPointer reply( inReply ); - Q_ASSERT( qApp->thread() == QThread::currentThread() ); - - QString username = auth->user(); - QString password = auth->password(); - - if ( username.isEmpty() && password.isEmpty() && reply->request().hasRawHeader( "Authorization" ) ) - { - QByteArray header( reply->request().rawHeader( "Authorization" ) ); - if ( header.startsWith( "Basic " ) ) - { - QByteArray auth( QByteArray::fromBase64( header.mid( 6 ) ) ); - int pos = auth.indexOf( ':' ); - if ( pos >= 0 ) - { - username = auth.left( pos ); - password = auth.mid( pos + 1 ); - } - } - } - - for ( ;; ) - { - bool ok; - - { - QMutexLocker lock( QgsCredentials::instance()->mutex() ); - ok = QgsCredentials::instance()->get( - QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), - username, password, - tr( "Authentication required" ) ); - } - if ( !ok ) - return; - - if ( reply.isNull() || reply->isFinished() ) - return; - - if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) ) - break; - - // credentials didn't change - stored ones probably wrong? clear password and retry - { - QMutexLocker lock( QgsCredentials::instance()->mutex() ); - QgsCredentials::instance()->put( - QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), - username, QString() ); - } - } - - // save credentials - { - QMutexLocker lock( QgsCredentials::instance()->mutex() ); - QgsCredentials::instance()->put( - QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), - username, password - ); - } - - auth->setUser( username ); - auth->setPassword( password ); -} - void QgisApp::namProxyAuthenticationRequired( const QNetworkProxy &proxy, QAuthenticator *auth ) { QgsSettings settings; diff --git a/src/app/qgisapp.h b/src/app/qgisapp.h index 1a9240d996c..c8a6e50896f 100644 --- a/src/app/qgisapp.h +++ b/src/app/qgisapp.h @@ -879,7 +879,6 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow void setAppStyleSheet( const QString &stylesheet ); //! request credentials for network manager - void namAuthenticationRequired( QNetworkReply *reply, QAuthenticator *auth ); void namProxyAuthenticationRequired( const QNetworkProxy &proxy, QAuthenticator *auth ); void namRequestTimedOut( const QgsNetworkRequestParameters &request ); diff --git a/src/app/qgsappauthrequesthandler.cpp b/src/app/qgsappauthrequesthandler.cpp new file mode 100644 index 00000000000..33baefc7df7 --- /dev/null +++ b/src/app/qgsappauthrequesthandler.cpp @@ -0,0 +1,82 @@ +/*************************************************************************** + qgsappauthrequesthandler.cpp + --------------------------- + begin : January 2019 + copyright : (C) 2019 by Nyall Dawson + email : nyall dot dawson at gmail dot com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ + +#include "qgsappauthrequesthandler.h" +#include "qgslogger.h" +#include "qgsauthcertutils.h" +#include "qgsapplication.h" +#include "qgsauthmanager.h" +#include "qgisapp.h" +#include "qgscredentials.h" +#include + +void QgsAppAuthRequestHandler::handleAuthRequest( QNetworkReply *reply, QAuthenticator *auth ) +{ + QString username = auth->user(); + QString password = auth->password(); + + if ( username.isEmpty() && password.isEmpty() && reply->request().hasRawHeader( "Authorization" ) ) + { + QByteArray header( reply->request().rawHeader( "Authorization" ) ); + if ( header.startsWith( "Basic " ) ) + { + QByteArray auth( QByteArray::fromBase64( header.mid( 6 ) ) ); + int pos = auth.indexOf( ':' ); + if ( pos >= 0 ) + { + username = auth.left( pos ); + password = auth.mid( pos + 1 ); + } + } + } + + for ( ;; ) + { + bool ok; + + { + QMutexLocker lock( QgsCredentials::instance()->mutex() ); + ok = QgsCredentials::instance()->get( + QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), + username, password, + QObject::tr( "Authentication required" ) ); + } + if ( !ok ) + return; + + if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) ) + break; + + // credentials didn't change - stored ones probably wrong? clear password and retry + { + QMutexLocker lock( QgsCredentials::instance()->mutex() ); + QgsCredentials::instance()->put( + QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), + username, QString() ); + } + } + + // save credentials + { + QMutexLocker lock( QgsCredentials::instance()->mutex() ); + QgsCredentials::instance()->put( + QStringLiteral( "%1 at %2" ).arg( auth->realm(), reply->url().host() ), + username, password + ); + } + + auth->setUser( username ); + auth->setPassword( password ); +} diff --git a/src/app/qgsappauthrequesthandler.h b/src/app/qgsappauthrequesthandler.h new file mode 100644 index 00000000000..ed651f67a02 --- /dev/null +++ b/src/app/qgsappauthrequesthandler.h @@ -0,0 +1,30 @@ +/*************************************************************************** + qgsappauthrequesthandler.h + ------------------------- + begin : January 2019 + copyright : (C) 2019 by Nyall Dawson + email : nyall dot dawson at gmail dot com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ +#ifndef QGSAPPAUTHREQUESTHANDLER_H +#define QGSAPPAUTHREQUESTHANDLER_H + +#include "qgsnetworkaccessmanager.h" + +class QgsAppAuthRequestHandler : public QgsNetworkAuthenticationHandler +{ + + public: + + void handleAuthRequest( QNetworkReply *reply, QAuthenticator *auth ); + +}; + + +#endif // QGSAPPAUTHREQUESTHANDLER_H diff --git a/src/core/qgsnetworkaccessmanager.cpp b/src/core/qgsnetworkaccessmanager.cpp index cfabcb0a173..c082e3874e2 100644 --- a/src/core/qgsnetworkaccessmanager.cpp +++ b/src/core/qgsnetworkaccessmanager.cpp @@ -130,6 +130,12 @@ void QgsNetworkAccessManager::setSslErrorHandler( std::unique_ptr handler ) +{ + Q_ASSERT( sMainNAM == this ); + mAuthHandler = std::move( handler ); +} + void QgsNetworkAccessManager::insertProxyFactory( QNetworkProxyFactory *factory ) { mProxyFactories.insert( 0, factory ); @@ -280,7 +286,7 @@ void QgsNetworkAccessManager::abortRequest() QgsDebugMsgLevel( QStringLiteral( "Abort [reply:%1] %2" ).arg( reinterpret_cast< qint64 >( reply ), 0, 16 ).arg( reply->url().toString() ), 3 ); QgsMessageLog::logMessage( tr( "Network request %1 timed out" ).arg( reply->url().toString() ), tr( "Network" ) ); // Notify the application - emit requestTimedOut( QgsNetworkRequestParameters( reply->operation(), reply->request(), reply->property( "requestId" ).toInt() ) ); + emit requestTimedOut( QgsNetworkRequestParameters( reply->operation(), reply->request(), getRequestId( reply ) ) ); emit requestTimedOut( reply ); } @@ -293,10 +299,7 @@ void QgsNetworkAccessManager::onReplyDownloadProgress( qint64 bytesReceived, qin { if ( QNetworkReply *reply = qobject_cast< QNetworkReply *>( sender() ) ) { - bool ok = false; - int requestId = reply->property( "requestId" ).toInt( &ok ); - if ( ok ) - emit downloadProgress( requestId, bytesReceived, bytesTotal ); + emit downloadProgress( getRequestId( reply ), bytesReceived, bytesTotal ); } } @@ -307,16 +310,10 @@ void QgsNetworkAccessManager::onReplySslErrors( const QList &errors ) Q_ASSERT( reply ); Q_ASSERT( reply->manager() == this ); - QTimer *timer = reply->findChild( QStringLiteral( "timeoutTimer" ) ); - if ( timer && timer->isActive() ) - { - QgsDebugMsg( QStringLiteral( "Stopping network reply timeout whilst SSL error is handled" ) ); - timer->stop(); - } - bool ok = false; - int requestId = reply->property( "requestId" ).toInt( &ok ); - if ( ok ) - emit requestEncounteredSslErrors( requestId, errors ); + QgsDebugMsg( QStringLiteral( "Stopping network reply timeout whilst SSL error is handled" ) ); + pauseTimeout( reply ); + + emit requestEncounteredSslErrors( getRequestId( reply ), errors ); // in main thread this will trigger SSL error handler immediately and return once the errors are handled, // while in worker thread the signal will be queued (and return immediately) -- hence the need to lock the thread in the next block @@ -336,15 +333,8 @@ void QgsNetworkAccessManager::afterSslErrorHandled( QNetworkReply *reply ) { if ( reply->manager() == this ) { - // restart reply timeout - QTimer *timer = reply->findChild( QStringLiteral( "timeoutTimer" ) ); - if ( timer ) - { - Q_ASSERT( !timer->isActive() ); - QgsDebugMsg( QStringLiteral( "Restarting network reply timeout" ) ); - timer->setSingleShot( true ); - timer->start( QgsSettings().value( QStringLiteral( "/qgis/networkAndProxy/networkTimeout" ), 60000 ).toInt() ); - } + restartTimeout( reply ); + emit sslErrorsHandled( reply ); } else if ( this == sMainNAM ) { @@ -353,13 +343,94 @@ void QgsNetworkAccessManager::afterSslErrorHandled( QNetworkReply *reply ) } } +void QgsNetworkAccessManager::unlockAfterAuthRequestHandled() +{ + Q_ASSERT( QThread::currentThread() == QApplication::instance()->thread() ); + mAuthRequestWaitCondition.wakeOne(); +} + +void QgsNetworkAccessManager::afterAuthRequestHandled( QNetworkReply *reply ) +{ + if ( reply->manager() == this ) + { + restartTimeout( reply ); + emit authRequestHandled( reply ); + } + else if ( this == sMainNAM ) + { + // notify other threads to allow them to handle the reply + qobject_cast< QgsNetworkAccessManager *>( reply->manager() )->unlockAfterAuthRequestHandled(); // safe to call directly - the other thread will be stuck waiting for us + } +} + +void QgsNetworkAccessManager::pauseTimeout( QNetworkReply *reply ) +{ + Q_ASSERT( reply->manager() == this ); + + QTimer *timer = reply->findChild( QStringLiteral( "timeoutTimer" ) ); + if ( timer && timer->isActive() ) + { + timer->stop(); + } +} + +void QgsNetworkAccessManager::restartTimeout( QNetworkReply *reply ) +{ + Q_ASSERT( reply->manager() == this ); + // restart reply timeout + QTimer *timer = reply->findChild( QStringLiteral( "timeoutTimer" ) ); + if ( timer ) + { + Q_ASSERT( !timer->isActive() ); + QgsDebugMsg( QStringLiteral( "Restarting network reply timeout" ) ); + timer->setSingleShot( true ); + timer->start( QgsSettings().value( QStringLiteral( "/qgis/networkAndProxy/networkTimeout" ), 60000 ).toInt() ); + } +} + +int QgsNetworkAccessManager::getRequestId( QNetworkReply *reply ) +{ + return reply->property( "requestId" ).toInt(); +} + void QgsNetworkAccessManager::handleSslErrors( QNetworkReply *reply, const QList &errors ) { mSslErrorHandler->handleSslErrors( reply, errors ); afterSslErrorHandled( reply ); } + #endif +void QgsNetworkAccessManager::onAuthRequired( QNetworkReply *reply, QAuthenticator *auth ) +{ + Q_ASSERT( reply ); + Q_ASSERT( reply->manager() == this ); + + QgsDebugMsg( QStringLiteral( "Stopping network reply timeout whilst auth request is handled" ) ); + pauseTimeout( reply ); + + emit requestRequiresAuth( getRequestId( reply ), auth->realm() ); + + // in main thread this will trigger auth handler immediately and return once the request is satisfied, + // while in worker thread the signal will be queued (and return immediately) -- hence the need to lock the thread in the next block + emit authRequestOccurred( reply, auth ); + if ( this != sMainNAM ) + { + // lock thread and wait till error is handled. If we return from this slot now, then the reply will resume + // without actually giving the main thread the chance to act on the ssl error and possibly ignore it. + mAuthRequestHandlerMutex.lock(); + mAuthRequestWaitCondition.wait( &mAuthRequestHandlerMutex ); + mAuthRequestHandlerMutex.unlock(); + afterAuthRequestHandled( reply ); + } +} + +void QgsNetworkAccessManager::handleAuthRequest( QNetworkReply *reply, QAuthenticator *auth ) +{ + mAuthHandler->handleAuthRequest( reply, auth ); + afterAuthRequestHandled( reply ); +} + QString QgsNetworkAccessManager::cacheLoadControlName( QNetworkRequest::CacheLoadControl control ) { switch ( control ) @@ -406,10 +477,6 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache( Qt::ConnectionType conn if ( sMainNAM != this ) { - connect( this, &QNetworkAccessManager::authenticationRequired, - sMainNAM, &QNetworkAccessManager::authenticationRequired, - connectionType ); - connect( this, &QNetworkAccessManager::proxyAuthenticationRequired, sMainNAM, &QNetworkAccessManager::proxyAuthenticationRequired, connectionType ); @@ -435,16 +502,22 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache( Qt::ConnectionType conn connect( this, &QgsNetworkAccessManager::requestEncounteredSslErrors, sMainNAM, &QgsNetworkAccessManager::requestEncounteredSslErrors ); #endif + + connect( this, &QgsNetworkAccessManager::requestRequiresAuth, sMainNAM, &QgsNetworkAccessManager::requestRequiresAuth ); } else { #ifndef QT_NO_SSL setSslErrorHandler( qgis::make_unique< QgsSslErrorHandler >() ); #endif + setAuthHandler( qgis::make_unique< QgsNetworkAuthenticationHandler>() ); } #ifndef QT_NO_SSL connect( this, &QgsNetworkAccessManager::sslErrorsOccurred, sMainNAM, &QgsNetworkAccessManager::handleSslErrors ); #endif + connect( this, &QNetworkAccessManager::authenticationRequired, this, &QgsNetworkAccessManager::onAuthRequired ); + connect( this, &QgsNetworkAccessManager::authRequestOccurred, sMainNAM, &QgsNetworkAccessManager::handleAuthRequest ); + connect( this, &QNetworkAccessManager::finished, this, &QgsNetworkAccessManager::onReplyFinished ); // check if proxy is enabled @@ -559,3 +632,13 @@ void QgsSslErrorHandler::handleSslErrors( QNetworkReply *reply, const QListrequest().url().toString() ) ); } + +// +// QgsNetworkAuthenticationHandler +// + +void QgsNetworkAuthenticationHandler::handleAuthRequest( QNetworkReply *reply, QAuthenticator * ) +{ + Q_UNUSED( reply ); + QgsDebugMsg( QStringLiteral( "Network reply required authentication, but no handler was in place to provide this authentication request while accessing the URL:\n%1" ).arg( reply->request().url().toString() ) ); +} diff --git a/src/core/qgsnetworkaccessmanager.h b/src/core/qgsnetworkaccessmanager.h index 5949148761c..3805e6dcfa8 100644 --- a/src/core/qgsnetworkaccessmanager.h +++ b/src/core/qgsnetworkaccessmanager.h @@ -183,6 +183,48 @@ class CORE_EXPORT QgsSslErrorHandler */ virtual void handleSslErrors( QNetworkReply *reply, const QList &errors ); +}; + +/** + * \class QgsNetworkAuthenticationHandler + * \brief Network authentication handler, used for responding to network authentication requests during network requests. + * \ingroup core + * + * QgsNetworkAuthenticationHandler responds to authentication requests encountered during network requests. The + * base QgsNetworkAuthenticationHandler class responds to requests only by logging the request, + * but does not provide any username or password to allow the request to proceed. + * + * Subclasses can override this behavior by implementing their own handleAuthRequest() + * method. QgsNetworkAuthenticationHandler are ONLY ever called from the main thread, so it + * is safe to utilize gui widgets and dialogs during handleAuthRequest (e.g. to + * present prompts to users requesting the username and password). + * + * If a reply is coming from background thread, that thread is blocked while handleAuthRequest() + * is running. + * + * An application instance can only have a single network authentication handler. The current + * authentication handler is set by calling QgsNetworkAccessManager::setAuthHandler(). + * By default an instance of the logging-only QgsNetworkAuthenticationHandler base class is used. + * + * \since QGIS 3.6 + */ +class CORE_EXPORT QgsNetworkAuthenticationHandler +{ + + public: + + virtual ~QgsNetworkAuthenticationHandler() = default; + + /** + * Called whenever network authentication requests are encountered during a network \a reply. + * + * Subclasses should reimplement this method to implement their own logic + * regarding how to handle the requests and whether they should be presented to users. + * + * The base class method just logs the request but does not provide any username/password resolution. + */ + virtual void handleAuthRequest( QNetworkReply *reply, QAuthenticator *auth ); + }; #endif @@ -251,6 +293,24 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager * \since QGIS 3.6 */ void setSslErrorHandler( std::unique_ptr< QgsSslErrorHandler > handler ); + + /** + * Sets the application network authentication \a handler, which is used to respond to network + * authentication prompts during network requests. + * + * Ownership of \a handler is transferred to the main thread QgsNetworkAccessManager instance. + * + * This method must ONLY be called on the main thread QgsNetworkAccessManager. It is not + * necessary to set handlers for background threads -- the main thread QgsNetworkAuthenticationHandler will + * automatically be used in a thread-safe manner for any authentication requests encountered on background threads. + * + * The default QgsNetworkAuthenticationHandler responds to request only by logging the request, + * but does not provide any username or password resolution. + * + * \note Not available in Python bindings. + * \since QGIS 3.6 + */ + void setAuthHandler( std::unique_ptr< QgsNetworkAuthenticationHandler > handler ); #endif //! insert a factory into the proxy factories list @@ -354,17 +414,36 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager */ void downloadProgress( int requestId, qint64 bytesReceived, qint64 bytesTotal ); + /** + * Emitted when a network request prompts an authentication request. + * + * The \a requestId argument reflects the unique ID identifying the original request which the authentication relates to. + * + * This signal is propagated to the main thread QgsNetworkAccessManager instance, so it is necessary + * only to connect to the main thread's signal in order to receive notifications about authentication requests + * from any thread. + * + * This signal is for debugging and logging purposes only, and cannot be used to respond to the + * requests. See QgsNetworkAuthenticationHandler for details on how to handle authentication requests. + * + * \since QGIS 3.6 + */ + void requestRequiresAuth( int requestId, const QString &realm ); + #ifndef QT_NO_SSL /** * Emitted when a network request encounters SSL \a errors. * - * The \a requestId argument reflects the unique ID identifying the original request which the progress report relates to. + * The \a requestId argument reflects the unique ID identifying the original request which the SSL error relates to. * * This signal is propagated to the main thread QgsNetworkAccessManager instance, so it is necessary * only to connect to the main thread's signal in order to receive notifications about SSL errors * from any thread. * + * This signal is for debugging and logging purposes only, and cannot be used to respond to the errors. + * See QgsSslErrorHandler for details on how to handle SSL errors and potentially ignore them. + * * \since QGIS 3.6 */ void requestEncounteredSslErrors( int requestId, const QList &errors ); @@ -386,6 +465,14 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager void requestTimedOut( QNetworkReply * ); +#ifndef SIP_RUN +///@cond PRIVATE + // these signals are for internal use only - it's not safe to connect by external code + void authRequestOccurred( QNetworkReply *, QAuthenticator *auth ); + void authRequestHandled( QNetworkReply *reply ); +///@endcond +#endif + private slots: void abortRequest(); @@ -398,6 +485,10 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager void handleSslErrors( QNetworkReply *reply, const QList &errors ); #endif + + void onAuthRequired( QNetworkReply *reply, QAuthenticator *auth ); + void handleAuthRequest( QNetworkReply *reply, QAuthenticator *auth ); + protected: QNetworkReply *createRequest( QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *outgoingData = nullptr ) override; @@ -406,6 +497,14 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager void unlockAfterSslErrorHandled(); void afterSslErrorHandled( QNetworkReply *reply ); #endif + + void unlockAfterAuthRequestHandled(); + void afterAuthRequestHandled( QNetworkReply *reply ); + + void pauseTimeout( QNetworkReply *reply ); + void restartTimeout( QNetworkReply *reply ); + static int getRequestId( QNetworkReply *reply ); + QList mProxyFactories; QNetworkProxy mFallbackProxy; QStringList mExcludedURLs; @@ -414,10 +513,18 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager static QgsNetworkAccessManager *sMainNAM; // ssl error handler, will be set for main thread ONLY std::unique_ptr< QgsSslErrorHandler > mSslErrorHandler; - // only in use by work threads, unused in main thread + // only in use by worker threads, unused in main thread QMutex mSslErrorHandlerMutex; - // only in use by work threads, unused in main thread + // only in use by worker threads, unused in main thread QWaitCondition mSslErrorWaitCondition; + + // auth request handler, will be set for main thread ONLY + std::unique_ptr< QgsNetworkAuthenticationHandler > mAuthHandler; + // only in use by worker threads, unused in main thread + QMutex mAuthRequestHandlerMutex; + // only in use by worker threads, unused in main thread + QWaitCondition mAuthRequestWaitCondition; + }; #endif // QGSNETWORKACCESSMANAGER_H diff --git a/tests/src/core/testqgsnetworkaccessmanager.cpp b/tests/src/core/testqgsnetworkaccessmanager.cpp index e6499926fc8..f444dbb3567 100644 --- a/tests/src/core/testqgsnetworkaccessmanager.cpp +++ b/tests/src/core/testqgsnetworkaccessmanager.cpp @@ -23,7 +23,7 @@ #include "qgstest.h" #include "qgssettings.h" #include - +#include class BackgroundRequest : public QThread { @@ -65,12 +65,36 @@ class TestSslErrorHandler : public QgsSslErrorHandler void handleSslErrors( QNetworkReply *reply, const QList &errors ) override { + Q_ASSERT( QThread::currentThread() == QApplication::instance()->thread() ); QCOMPARE( errors.at( 0 ).error(), QSslError::SelfSignedCertificate ); reply->ignoreSslErrors(); } }; +class TestAuthRequestHandler : public QgsNetworkAuthenticationHandler +{ + public: + + TestAuthRequestHandler( const QString &user, const QString &password ) + : mUser( user ) + , mPassword( password ) + {} + + void handleAuthRequest( QNetworkReply *, QAuthenticator *auth ) override + { + Q_ASSERT( QThread::currentThread() == QApplication::instance()->thread() ); + if ( !mUser.isEmpty() ) + auth->setUser( mUser ); + if ( !mPassword.isEmpty() ) + auth->setPassword( mPassword ); + } + + QString mUser; + QString mPassword; + +}; + class TestQgsNetworkAccessManager : public QObject { Q_OBJECT @@ -89,6 +113,7 @@ class TestQgsNetworkAccessManager : public QObject void fetchPost(); void fetchBadSsl(); void testSslErrorHandler(); + void testAuthRequestHandler(); void fetchTimeout(); }; @@ -483,6 +508,108 @@ void TestQgsNetworkAccessManager::testSslErrorHandler() QgsNetworkAccessManager::instance()->setSslErrorHandler( qgis::make_unique< QgsSslErrorHandler >() ); } +void TestQgsNetworkAccessManager::testAuthRequestHandler() +{ + if ( QgsTest::isTravis() ) + QSKIP( "This test is disabled on Travis CI environment" ); + + // initially this request should fail -- we aren't providing the username and password required + QgsNetworkAccessManager::instance()->setAuthHandler( qgis::make_unique< TestAuthRequestHandler >( QString(), QString() ) ); + + QObject context; + bool loaded = false; + bool gotRequestAboutToBeCreatedSignal = false; + bool gotAuthRequest = false; + int requestId = -1; + QUrl u = QUrl( QStringLiteral( "http://httpbin.org/basic-auth/me/secret" ) ); + QNetworkReply::NetworkError expectedError = QNetworkReply::NoError; + connect( QgsNetworkAccessManager::instance(), qgis::overload< QgsNetworkRequestParameters >::of( &QgsNetworkAccessManager::requestAboutToBeCreated ), &context, [&]( const QgsNetworkRequestParameters & params ) + { + gotRequestAboutToBeCreatedSignal = true; + requestId = params.requestId(); + QVERIFY( requestId > 0 ); + QCOMPARE( params.operation(), QNetworkAccessManager::GetOperation ); + QCOMPARE( params.request().url(), u ); + } ); + connect( QgsNetworkAccessManager::instance(), qgis::overload< QgsNetworkReplyContent >::of( &QgsNetworkAccessManager::finished ), &context, [&]( const QgsNetworkReplyContent & reply ) + { + QCOMPARE( reply.error(), expectedError ); + QCOMPARE( reply.requestId(), requestId ); + QCOMPARE( reply.request().url(), u ); + loaded = true; + } ); + + connect( QgsNetworkAccessManager::instance(), &QgsNetworkAccessManager::requestRequiresAuth, &context, [&]( int authRequestId, const QString & realm ) + { + QCOMPARE( authRequestId, requestId ); + QCOMPARE( realm, QStringLiteral( "Fake Realm" ) ); + gotAuthRequest = true; + } ); + + expectedError = QNetworkReply::AuthenticationRequiredError; + QgsNetworkAccessManager::instance()->get( QNetworkRequest( u ) ); + + while ( !loaded || !gotAuthRequest || !gotRequestAboutToBeCreatedSignal ) + { + qApp->processEvents(); + } + + QVERIFY( gotRequestAboutToBeCreatedSignal ); + + // now try in a thread + loaded = false; + gotAuthRequest = false; + gotRequestAboutToBeCreatedSignal = false; + + + BackgroundRequest *thread = new BackgroundRequest( QNetworkRequest( u ) ); + + thread->start(); + + while ( !loaded || !gotAuthRequest || !gotRequestAboutToBeCreatedSignal ) + { + qApp->processEvents(); + } + QVERIFY( gotRequestAboutToBeCreatedSignal ); + thread->exit(); + thread->wait(); + thread->deleteLater(); + + // try with username and password specified + QgsNetworkAccessManager::instance()->setAuthHandler( qgis::make_unique< TestAuthRequestHandler >( QStringLiteral( "me" ), QStringLiteral( "secret" ) ) ); + loaded = false; + gotAuthRequest = false; + gotRequestAboutToBeCreatedSignal = false; + expectedError = QNetworkReply::NoError; + QgsNetworkAccessManager::instance()->get( QNetworkRequest( u ) ); + + while ( !loaded || !gotAuthRequest || !gotRequestAboutToBeCreatedSignal ) + { + qApp->processEvents(); + } + + // correct username and password, in a thread + QgsNetworkAccessManager::instance()->setAuthHandler( qgis::make_unique< TestAuthRequestHandler >( QStringLiteral( "me2" ), QStringLiteral( "secret2" ) ) ); + u = QUrl( QStringLiteral( "http://httpbin.org/basic-auth/me2/secret2" ) ); + loaded = false; + gotAuthRequest = false; + gotRequestAboutToBeCreatedSignal = false; + expectedError = QNetworkReply::NoError; + + thread = new BackgroundRequest( QNetworkRequest( u ) ); + thread->start(); + while ( !loaded || !gotAuthRequest || !gotRequestAboutToBeCreatedSignal ) + { + qApp->processEvents(); + } + QVERIFY( gotRequestAboutToBeCreatedSignal ); + thread->exit(); + thread->wait(); + thread->deleteLater(); + + QgsNetworkAccessManager::instance()->setAuthHandler( qgis::make_unique< QgsNetworkAuthenticationHandler >() ); +} + void TestQgsNetworkAccessManager::fetchTimeout() { if ( QgsTest::isTravis() )