Skip to content

Commit

Permalink
[auth] Add mutex; remove qApp parents; do not set parent in wrong thread
Browse files Browse the repository at this point in the history
  • Loading branch information
elpaso authored and dakcarto committed Oct 27, 2017
1 parent 7f96c5b commit 6354563
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 10 deletions.
7 changes: 5 additions & 2 deletions src/auth/oauth2/qgsauthoauth2config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ QgsStringMap QgsAuthOAuth2Config::mapOAuth2Configs(
return configs;
}

QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extradir )
QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache(QObject *parent, const QString &extradir )
{
QgsStringMap configs;
bool ok = false;
Expand All @@ -701,7 +701,7 @@ QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extra
continue;
}
QgsStringMap newconfigs = QgsAuthOAuth2Config::mapOAuth2Configs(
configdirinfo.canonicalFilePath(), qApp, QgsAuthOAuth2Config::JSON, &ok );
configdirinfo.canonicalFilePath(), parent, QgsAuthOAuth2Config::JSON, &ok );
if ( ok )
{
QgsStringMap::const_iterator i = newconfigs.constBegin();
Expand Down Expand Up @@ -735,6 +735,7 @@ QString QgsAuthOAuth2Config::configTypeString( QgsAuthOAuth2Config::ConfigType c
case QgsAuthOAuth2Config::Custom:
return tr( "Custom" );
case QgsAuthOAuth2Config::Predefined:
default:
return tr( "Predefined" );
}
}
Expand All @@ -749,6 +750,7 @@ QString QgsAuthOAuth2Config::grantFlowString( QgsAuthOAuth2Config::GrantFlow flo
case QgsAuthOAuth2Config::Implicit:
return tr( "Implicit" );
case QgsAuthOAuth2Config::ResourceOwner:
default:
return tr( "Resource Owner" );
}
}
Expand All @@ -763,6 +765,7 @@ QString QgsAuthOAuth2Config::accessMethodString( QgsAuthOAuth2Config::AccessMeth
case QgsAuthOAuth2Config::Form:
return tr( "Form (POST only)" );
case QgsAuthOAuth2Config::Query:
default:
return tr( "URL Query" );
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/auth/oauth2/qgsauthoauth2config.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class QgsAuthOAuth2Config : public QObject
bool *ok = nullptr );

//! Load and parse standard directories of configs (e.g. JSON) to a mapped cache
static QgsStringMap mappedOAuth2ConfigsCache( const QString &extradir = QString::null );
static QgsStringMap mappedOAuth2ConfigsCache( QObject *parent, const QString &extradir = QString::null );

//!
static QString oauth2ConfigsPkgDataDir();
Expand Down
2 changes: 1 addition & 1 deletion src/auth/oauth2/qgsauthoauth2edit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ void QgsAuthOAuth2Edit::updateDefinedConfigsCache()
{
QString extradir = leDefinedDirPath->text();
mDefinedConfigsCache.clear();
mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );
}

// slot
Expand Down
17 changes: 11 additions & 6 deletions src/auth/oauth2/qgsauthoauth2method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <QDir>
#include <QEventLoop>
#include <QString>
#include <QMutexLocker>


static const QString AUTH_METHOD_KEY = QStringLiteral( "OAuth2" );
Expand Down Expand Up @@ -105,6 +106,8 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
{
Q_UNUSED( dataprovider )

QMutexLocker locker( &mNetworkRequestMutex );

QString msg;

QgsO2 *o2 = getOAuth2Bundle( authcfg );
Expand Down Expand Up @@ -144,7 +147,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const

// Try to get a refresh token first
// go into local event loop and wait for a fired refresh-related slot
QEventLoop rloop( qApp );
QEventLoop rloop( nullptr );
connect( o2, &QgsO2::refreshFinished, &rloop, &QEventLoop::quit );

// Asynchronously attempt the refresh
Expand Down Expand Up @@ -182,13 +185,13 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
settings.setValue( timeoutkey, reqtimeout );

// go into local event loop and wait for a fired linking-related slot
QEventLoop loop( qApp );
QEventLoop loop( nullptr );
connect( o2, &QgsO2::linkingFailed, &loop, &QEventLoop::quit );
connect( o2, &QgsO2::linkingSucceeded, &loop, &QEventLoop::quit );

// add singlshot timer to quit linking after an alloted timeout
// this should keep the local event loop from blocking forever
QTimer timer( this );
QTimer timer( nullptr );
timer.setInterval( reqtimeout * 5 );
timer.setSingleShot( true );
connect( &timer, &QTimer::timeout, o2, &QgsO2::linkingFailed );
Expand Down Expand Up @@ -270,6 +273,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
bool QgsAuthOAuth2Method::updateNetworkReply( QNetworkReply *reply, const QString &authcfg, const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( &mNetworkRequestMutex );

// TODO: handle token refresh error on the reply, see O2Requestor::onRequestError()
// Is this doable if the errors are also handled in qgsapp (and/or elsewhere)?
Expand Down Expand Up @@ -389,6 +393,7 @@ void QgsAuthOAuth2Method::onReplyFinished()

void QgsAuthOAuth2Method::onNetworkError( QNetworkReply::NetworkError err )
{
QMutexLocker locker( &mNetworkRequestMutex );
QString msg;
QNetworkReply *reply = qobject_cast<QNetworkReply *>( sender() );
if ( !reply )
Expand Down Expand Up @@ -489,7 +494,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
return sOAuth2ConfigCache.value( authcfg );
}

QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( qApp );
QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( );
QgsO2 *nullbundle = nullptr;

// else build oauth2 config
Expand Down Expand Up @@ -542,7 +547,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
QgsDebugMsg( QStringLiteral( "No custom defined dir path to load OAuth2 config" ) );
}

QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );

if ( !definedcache.contains( definedid ) )
{
Expand Down Expand Up @@ -595,7 +600,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
QgsDebugMsg( QStringLiteral( "Loading authenticator object with %1 flow properties of OAuth2 config: %2" )
.arg( QgsAuthOAuth2Config::grantFlowString( config->grantFlow() ), authcfg ) );

QgsO2 *o2 = new QgsO2( authcfg, config, qApp, QgsNetworkAccessManager::instance() );
QgsO2 *o2 = new QgsO2( authcfg, config, nullptr, QgsNetworkAccessManager::instance() );

// cache bundle
putOAuth2Bundle( authcfg, o2 );
Expand Down
3 changes: 3 additions & 0 deletions src/auth/oauth2/qgsauthoauth2method.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <QDialog>
#include <QEventLoop>
#include <QTimer>
#include <QMutex>

#include "qgsauthmethod.h"

Expand Down Expand Up @@ -77,6 +78,8 @@ class QgsAuthOAuth2Method : public QgsAuthMethod
static QMap<QString, QgsO2 *> sOAuth2ConfigCache;

QgsO2 *authO2( const QString &authcfg );

QMutex mNetworkRequestMutex;
};

#endif // QGSAUTHOAUTH2METHOD_H

0 comments on commit 6354563

Please sign in to comment.