Skip to content

Commit

Permalink
[auth] Thread safe auth methods with recursive mutex
Browse files Browse the repository at this point in the history 
Add a mutex to the base class and use that
mutex to protect all public methods of the
authentication methods.
  • Loading branch information
@elpaso
elpaso committed Nov 16, 2017
1 parent 29c8f7c commit 674467b
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 24 deletions.
7 changes: 7 additions & 0 deletions python/core/auth/qgsauthmethod.sip
View file
Expand Up @@ -34,6 +34,11 @@ class QgsAuthMethod : QObject
typedef QFlags<QgsAuthMethod::Expansion> Expansions;


~QgsAuthMethod();
%Docstring
Destructor: delete the mutex
%End

virtual QString key() const = 0;
%Docstring
A non-translated short name representing the auth method
Expand Down Expand Up @@ -151,6 +156,7 @@ Increment this if method is significantly updated, allow updater code to be writ
Non-public since this is an abstract base class
%End


static QString authMethodTag();
%Docstring
Tag signifying that this is an authentcation method (e.g. for use as title in message log panel output)
Expand All @@ -171,6 +177,7 @@ Set the support expansions (points in providers where the authentication is inje
Set list of data providers this auth method supports
%End


};
QFlags<QgsAuthMethod::Expansion> operator|(QgsAuthMethod::Expansion f1, QFlags<QgsAuthMethod::Expansion> f2);

Expand Down
14 changes: 9 additions & 5 deletions src/auth/basic/qgsauthbasicmethod.cpp
View file
Expand Up @@ -44,6 +44,7 @@ QgsAuthBasicMethod::QgsAuthBasicMethod()
<< QStringLiteral( "wms" )
<< QStringLiteral( "ogr" )
<< QStringLiteral( "proxy" ) );

}

QString QgsAuthBasicMethod::key() const
Expand All @@ -65,7 +66,7 @@ bool QgsAuthBasicMethod::updateNetworkRequest( QNetworkRequest &request, const Q
const QString &dataprovider )
{
Q_UNUSED( dataprovider )

QMutexLocker locker( mMutex );
QgsAuthMethodConfig mconfig = getMethodConfig( authcfg );
if ( !mconfig.isValid() )
{
Expand All @@ -86,6 +87,8 @@ bool QgsAuthBasicMethod::updateNetworkRequest( QNetworkRequest &request, const Q
bool QgsAuthBasicMethod::updateDataSourceUriItems( QStringList &connectionItems, const QString &authcfg,
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );
QgsAuthMethodConfig mconfig = getMethodConfig( authcfg );
if ( !mconfig.isValid() )
{
Expand Down Expand Up @@ -277,6 +280,7 @@ bool QgsAuthBasicMethod::updateDataSourceUriItems( QStringList &connectionItems,
bool QgsAuthBasicMethod::updateNetworkProxy( QNetworkProxy &proxy, const QString &authcfg, const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

QgsAuthMethodConfig mconfig = getMethodConfig( authcfg );
if ( !mconfig.isValid() )
Expand All @@ -298,6 +302,7 @@ bool QgsAuthBasicMethod::updateNetworkProxy( QNetworkProxy &proxy, const QString

void QgsAuthBasicMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
{
QMutexLocker locker( mMutex );
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
{
QgsDebugMsg( "Updating old style auth method config" );
Expand All @@ -319,7 +324,7 @@ void QgsAuthBasicMethod::clearCachedConfig( const QString &authcfg )

QgsAuthMethodConfig QgsAuthBasicMethod::getMethodConfig( const QString &authcfg, bool fullconfig )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsAuthMethodConfig mconfig;

// check if it is cached
Expand All @@ -338,22 +343,21 @@ QgsAuthMethodConfig QgsAuthBasicMethod::getMethodConfig( const QString &authcfg,
}

// cache bundle
locker.unlock();
putMethodConfig( authcfg, mconfig );

return mconfig;
}

void QgsAuthBasicMethod::putMethodConfig( const QString &authcfg, const QgsAuthMethodConfig &mconfig )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsDebugMsg( QString( "Putting basic config for authcfg: %1" ).arg( authcfg ) );
sAuthConfigCache.insert( authcfg, mconfig );
}

void QgsAuthBasicMethod::removeMethodConfig( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
if ( sAuthConfigCache.contains( authcfg ) )
{
sAuthConfigCache.remove( authcfg );
Expand Down
1 change: 0 additions & 1 deletion src/auth/basic/qgsauthbasicmethod.h
View file
Expand Up @@ -63,7 +63,6 @@ class QgsAuthBasicMethod : public QgsAuthMethod

static QMap<QString, QgsAuthMethodConfig> sAuthConfigCache;

QMutex mConfigMutex;
};

#endif // QGSAUTHBASICMETHOD_H
12 changes: 7 additions & 5 deletions src/auth/identcert/qgsauthidentcertmethod.cpp
View file
Expand Up @@ -52,7 +52,7 @@ QgsAuthIdentCertMethod::QgsAuthIdentCertMethod()

QgsAuthIdentCertMethod::~QgsAuthIdentCertMethod()
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
qDeleteAll( sPkiConfigBundleCache );
sPkiConfigBundleCache.clear();
}
Expand All @@ -76,6 +76,7 @@ bool QgsAuthIdentCertMethod::updateNetworkRequest( QNetworkRequest &request, con
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

// TODO: is this too restrictive, to intercept only HTTPS connections?
if ( request.url().scheme().toLower() != QLatin1String( "https" ) )
Expand Down Expand Up @@ -110,6 +111,7 @@ bool QgsAuthIdentCertMethod::updateDataSourceUriItems( QStringList &connectionIt
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

QgsDebugMsg( QString( "Update URI items for authcfg: %1" ).arg( authcfg ) );

Expand Down Expand Up @@ -208,6 +210,7 @@ void QgsAuthIdentCertMethod::clearCachedConfig( const QString &authcfg )

void QgsAuthIdentCertMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
{
QMutexLocker locker( mMutex );
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
{
QgsDebugMsg( "Updating old style auth method config" );
Expand All @@ -222,7 +225,7 @@ void QgsAuthIdentCertMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )

QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsPkiConfigBundle *bundle = nullptr;

// check if it is cached
Expand Down Expand Up @@ -267,7 +270,6 @@ QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &a

bundle = new QgsPkiConfigBundle( mconfig, clientcert, clientkey );

locker.unlock();
// cache bundle
putPkiConfigBundle( authcfg, bundle );

Expand All @@ -276,14 +278,14 @@ QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &a

void QgsAuthIdentCertMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
sPkiConfigBundleCache.insert( authcfg, pkibundle );
}

void QgsAuthIdentCertMethod::removePkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
if ( sPkiConfigBundleCache.contains( authcfg ) )
{
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );
Expand Down
1 change: 0 additions & 1 deletion src/auth/identcert/qgsauthidentcertmethod.h
View file
Expand Up @@ -59,7 +59,6 @@ class QgsAuthIdentCertMethod : public QgsAuthMethod

static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;

QMutex mConfigMutex;
};

#endif // QGSAUTHIDENTCERTMETHOD_H
13 changes: 8 additions & 5 deletions src/auth/pkipaths/qgsauthpkipathsmethod.cpp
View file
Expand Up @@ -53,7 +53,7 @@ QgsAuthPkiPathsMethod::QgsAuthPkiPathsMethod()

QgsAuthPkiPathsMethod::~QgsAuthPkiPathsMethod()
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
qDeleteAll( sPkiConfigBundleCache );
sPkiConfigBundleCache.clear();
}
Expand All @@ -77,6 +77,7 @@ bool QgsAuthPkiPathsMethod::updateNetworkRequest( QNetworkRequest &request, cons
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

// TODO: is this too restrictive, to intercept only HTTPS connections?
if ( request.url().scheme().toLower() != QLatin1String( "https" ) )
Expand Down Expand Up @@ -124,6 +125,7 @@ bool QgsAuthPkiPathsMethod::updateDataSourceUriItems( QStringList &connectionIte
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

QgsDebugMsg( QString( "Update URI items for authcfg: %1" ).arg( authcfg ) );

Expand Down Expand Up @@ -237,11 +239,13 @@ bool QgsAuthPkiPathsMethod::updateDataSourceUriItems( QStringList &connectionIte

void QgsAuthPkiPathsMethod::clearCachedConfig( const QString &authcfg )
{
QMutexLocker locker( mMutex );
removePkiConfigBundle( authcfg );
}

void QgsAuthPkiPathsMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
{
QMutexLocker locker( mMutex );
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
{
QgsDebugMsg( "Updating old style auth method config" );
Expand All @@ -258,7 +262,7 @@ void QgsAuthPkiPathsMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )

QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsPkiConfigBundle *bundle = nullptr;

// check if it is cached
Expand Down Expand Up @@ -301,7 +305,6 @@ QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &au

bundle = new QgsPkiConfigBundle( mconfig, clientcert, clientkey, QgsAuthCertUtils::casFromFile( mconfig.config( QStringLiteral( "certpath" ) ) ) );

locker.unlock();
// cache bundle
putPkiConfigBundle( authcfg, bundle );

Expand All @@ -310,14 +313,14 @@ QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &au

void QgsAuthPkiPathsMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
sPkiConfigBundleCache.insert( authcfg, pkibundle );
}

void QgsAuthPkiPathsMethod::removePkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
if ( sPkiConfigBundleCache.contains( authcfg ) )
{
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );
Expand Down
2 changes: 0 additions & 2 deletions src/auth/pkipaths/qgsauthpkipathsmethod.h
View file
Expand Up @@ -59,8 +59,6 @@ class QgsAuthPkiPathsMethod : public QgsAuthMethod

static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;

QMutex mConfigMutex;

};

#endif // QGSAUTHPKIPATHSMETHOD_H
11 changes: 7 additions & 4 deletions src/auth/pkipkcs12/qgsauthpkcs12method.cpp
View file
Expand Up @@ -53,7 +53,6 @@ QgsAuthPkcs12Method::QgsAuthPkcs12Method()

QgsAuthPkcs12Method::~QgsAuthPkcs12Method()
{
QMutexLocker locker( &mConfigMutex );
qDeleteAll( sPkiConfigBundleCache );
sPkiConfigBundleCache.clear();
}
Expand All @@ -77,6 +76,7 @@ bool QgsAuthPkcs12Method::updateNetworkRequest( QNetworkRequest &request, const
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

// TODO: is this too restrictive, to intercept only HTTPS connections?
if ( request.url().scheme().toLower() != QLatin1String( "https" ) )
Expand Down Expand Up @@ -124,6 +124,7 @@ bool QgsAuthPkcs12Method::updateDataSourceUriItems( QStringList &connectionItems
const QString &dataprovider )
{
Q_UNUSED( dataprovider )
QMutexLocker locker( mMutex );

QgsDebugMsg( QString( "Update URI items for authcfg: %1" ).arg( authcfg ) );

Expand Down Expand Up @@ -237,11 +238,13 @@ bool QgsAuthPkcs12Method::updateDataSourceUriItems( QStringList &connectionItems

void QgsAuthPkcs12Method::clearCachedConfig( const QString &authcfg )
{
QMutexLocker locker( mMutex );
removePkiConfigBundle( authcfg );
}

void QgsAuthPkcs12Method::updateMethodConfig( QgsAuthMethodConfig &mconfig )
{
QMutexLocker locker( mMutex );
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
{
QgsDebugMsg( "Updating old style auth method config" );
Expand All @@ -257,7 +260,7 @@ void QgsAuthPkcs12Method::updateMethodConfig( QgsAuthMethodConfig &mconfig )

QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsPkiConfigBundle *bundle = nullptr;

// check if it is cached
Expand Down Expand Up @@ -331,14 +334,14 @@ QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &auth

void QgsAuthPkcs12Method::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
sPkiConfigBundleCache.insert( authcfg, pkibundle );
}

void QgsAuthPkcs12Method::removePkiConfigBundle( const QString &authcfg )
{
QMutexLocker locker( &mConfigMutex );
QMutexLocker locker( mMutex );
if ( sPkiConfigBundleCache.contains( authcfg ) )
{
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );
Expand Down
1 change: 0 additions & 1 deletion src/auth/pkipkcs12/qgsauthpkcs12method.h
View file
Expand Up @@ -60,7 +60,6 @@ class QgsAuthPkcs12Method : public QgsAuthMethod

static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;

QMutex mConfigMutex;
};

#endif // QGSAUTHPKCS12METHOD_H
8 changes: 8 additions & 0 deletions src/core/auth/qgsauthmethod.h
View file
Expand Up @@ -23,6 +23,7 @@
#include <QNetworkRequest>
#include <QStringList>
#include <QUrl>
#include <QMutex>

#include "qgis_core.h"

Expand Down Expand Up @@ -58,6 +59,9 @@ class CORE_EXPORT QgsAuthMethod : public QObject
};
Q_DECLARE_FLAGS( Expansions, Expansion )

//! Destructor: delete the mutex
~QgsAuthMethod() { delete mMutex; }

//! A non-translated short name representing the auth method
virtual QString key() const = 0;

Expand Down Expand Up @@ -174,8 +178,10 @@ class CORE_EXPORT QgsAuthMethod : public QObject
explicit QgsAuthMethod()
: mExpansions( QgsAuthMethod::Expansions( nullptr ) )
, mDataProviders( QStringList() )
, mMutex( new QMutex( QMutex::RecursionMode::Recursive ) )
{}


//! Tag signifying that this is an authentcation method (e.g. for use as title in message log panel output)
static QString authMethodTag() { return QObject::tr( "Authentication method" ); }

Expand All @@ -190,6 +196,8 @@ class CORE_EXPORT QgsAuthMethod : public QObject
QgsAuthMethod::Expansions mExpansions;
QStringList mDataProviders;
int mVersion = 0;
QMutex *mMutex;

};
Q_DECLARE_OPERATORS_FOR_FLAGS( QgsAuthMethod::Expansions )

Expand Down

0 comments on commit 674467b

Please sign in to comment.