Skip to content

Commit d590e98

Browse files
elpasoelpaso
authoredNov 16, 2017
Merge pull request #5644 from boundlessgeo/bd-2648-auth-thread-safety
[auth] Thread safe auth methods with recursive mutex
·
ltr-3_40final-3_0_0
2 parents 29c8f7c + 6f06597 commit d590e98

File tree

10 files changed

+38
-24
lines changed

10 files changed

+38
-24
lines changed
 

‎python/core/auth/qgsauthmethod.sip

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ Increment this if method is significantly updated, allow updater code to be writ
151151
Non-public since this is an abstract base class
152152
%End
153153

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

175+
174176
};
175177
QFlags<QgsAuthMethod::Expansion> operator|(QgsAuthMethod::Expansion f1, QFlags<QgsAuthMethod::Expansion> f2);
176178

‎src/auth/basic/qgsauthbasicmethod.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ QgsAuthBasicMethod::QgsAuthBasicMethod()
4444
<< QStringLiteral( "wms" )
4545
<< QStringLiteral( "ogr" )
4646
<< QStringLiteral( "proxy" ) );
47+
4748
}
4849

4950
QString QgsAuthBasicMethod::key() const
@@ -65,7 +66,7 @@ bool QgsAuthBasicMethod::updateNetworkRequest( QNetworkRequest &request, const Q
6566
const QString &dataprovider )
6667
{
6768
Q_UNUSED( dataprovider )
68-
69+
QMutexLocker locker( &mMutex );
6970
QgsAuthMethodConfig mconfig = getMethodConfig( authcfg );
7071
if ( !mconfig.isValid() )
7172
{
@@ -86,6 +87,8 @@ bool QgsAuthBasicMethod::updateNetworkRequest( QNetworkRequest &request, const Q
8687
bool QgsAuthBasicMethod::updateDataSourceUriItems( QStringList &connectionItems, const QString &authcfg,
8788
const QString &dataprovider )
8889
{
90+
Q_UNUSED( dataprovider )
91+
QMutexLocker locker( &mMutex );
8992
QgsAuthMethodConfig mconfig = getMethodConfig( authcfg );
9093
if ( !mconfig.isValid() )
9194
{
@@ -277,6 +280,7 @@ bool QgsAuthBasicMethod::updateDataSourceUriItems( QStringList &connectionItems,
277280
bool QgsAuthBasicMethod::updateNetworkProxy( QNetworkProxy &proxy, const QString &authcfg, const QString &dataprovider )
278281
{
279282
Q_UNUSED( dataprovider )
283+
QMutexLocker locker( &mMutex );
280284

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

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

320325
QgsAuthMethodConfig QgsAuthBasicMethod::getMethodConfig( const QString &authcfg, bool fullconfig )
321326
{
322-
QMutexLocker locker( &mConfigMutex );
327+
QMutexLocker locker( &mMutex );
323328
QgsAuthMethodConfig mconfig;
324329

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

340345
// cache bundle
341-
locker.unlock();
342346
putMethodConfig( authcfg, mconfig );
343347

344348
return mconfig;
345349
}
346350

347351
void QgsAuthBasicMethod::putMethodConfig( const QString &authcfg, const QgsAuthMethodConfig &mconfig )
348352
{
349-
QMutexLocker locker( &mConfigMutex );
353+
QMutexLocker locker( &mMutex );
350354
QgsDebugMsg( QString( "Putting basic config for authcfg: %1" ).arg( authcfg ) );
351355
sAuthConfigCache.insert( authcfg, mconfig );
352356
}
353357

354358
void QgsAuthBasicMethod::removeMethodConfig( const QString &authcfg )
355359
{
356-
QMutexLocker locker( &mConfigMutex );
360+
QMutexLocker locker( &mMutex );
357361
if ( sAuthConfigCache.contains( authcfg ) )
358362
{
359363
sAuthConfigCache.remove( authcfg );

‎src/auth/basic/qgsauthbasicmethod.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ class QgsAuthBasicMethod : public QgsAuthMethod
6363

6464
static QMap<QString, QgsAuthMethodConfig> sAuthConfigCache;
6565

66-
QMutex mConfigMutex;
6766
};
6867

6968
#endif // QGSAUTHBASICMETHOD_H

‎src/auth/identcert/qgsauthidentcertmethod.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ QgsAuthIdentCertMethod::QgsAuthIdentCertMethod()
5252

5353
QgsAuthIdentCertMethod::~QgsAuthIdentCertMethod()
5454
{
55-
QMutexLocker locker( &mConfigMutex );
55+
QMutexLocker locker( &mMutex );
5656
qDeleteAll( sPkiConfigBundleCache );
5757
sPkiConfigBundleCache.clear();
5858
}
@@ -76,6 +76,7 @@ bool QgsAuthIdentCertMethod::updateNetworkRequest( QNetworkRequest &request, con
7676
const QString &dataprovider )
7777
{
7878
Q_UNUSED( dataprovider )
79+
QMutexLocker locker( &mMutex );
7980

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

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

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

209211
void QgsAuthIdentCertMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
210212
{
213+
QMutexLocker locker( &mMutex );
211214
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
212215
{
213216
QgsDebugMsg( "Updating old style auth method config" );
@@ -222,7 +225,7 @@ void QgsAuthIdentCertMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
222225

223226
QgsPkiConfigBundle *QgsAuthIdentCertMethod::getPkiConfigBundle( const QString &authcfg )
224227
{
225-
QMutexLocker locker( &mConfigMutex );
228+
QMutexLocker locker( &mMutex );
226229
QgsPkiConfigBundle *bundle = nullptr;
227230

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

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

270-
locker.unlock();
271273
// cache bundle
272274
putPkiConfigBundle( authcfg, bundle );
273275

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

277279
void QgsAuthIdentCertMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
278280
{
279-
QMutexLocker locker( &mConfigMutex );
281+
QMutexLocker locker( &mMutex );
280282
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
281283
sPkiConfigBundleCache.insert( authcfg, pkibundle );
282284
}
283285

284286
void QgsAuthIdentCertMethod::removePkiConfigBundle( const QString &authcfg )
285287
{
286-
QMutexLocker locker( &mConfigMutex );
288+
QMutexLocker locker( &mMutex );
287289
if ( sPkiConfigBundleCache.contains( authcfg ) )
288290
{
289291
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

‎src/auth/identcert/qgsauthidentcertmethod.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class QgsAuthIdentCertMethod : public QgsAuthMethod
5959

6060
static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
6161

62-
QMutex mConfigMutex;
6362
};
6463

6564
#endif // QGSAUTHIDENTCERTMETHOD_H

‎src/auth/pkipaths/qgsauthpkipathsmethod.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ QgsAuthPkiPathsMethod::QgsAuthPkiPathsMethod()
5353

5454
QgsAuthPkiPathsMethod::~QgsAuthPkiPathsMethod()
5555
{
56-
QMutexLocker locker( &mConfigMutex );
56+
QMutexLocker locker( &mMutex );
5757
qDeleteAll( sPkiConfigBundleCache );
5858
sPkiConfigBundleCache.clear();
5959
}
@@ -77,6 +77,7 @@ bool QgsAuthPkiPathsMethod::updateNetworkRequest( QNetworkRequest &request, cons
7777
const QString &dataprovider )
7878
{
7979
Q_UNUSED( dataprovider )
80+
QMutexLocker locker( &mMutex );
8081

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

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

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

238240
void QgsAuthPkiPathsMethod::clearCachedConfig( const QString &authcfg )
239241
{
242+
QMutexLocker locker( &mMutex );
240243
removePkiConfigBundle( authcfg );
241244
}
242245

243246
void QgsAuthPkiPathsMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
244247
{
248+
QMutexLocker locker( &mMutex );
245249
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
246250
{
247251
QgsDebugMsg( "Updating old style auth method config" );
@@ -258,7 +262,7 @@ void QgsAuthPkiPathsMethod::updateMethodConfig( QgsAuthMethodConfig &mconfig )
258262

259263
QgsPkiConfigBundle *QgsAuthPkiPathsMethod::getPkiConfigBundle( const QString &authcfg )
260264
{
261-
QMutexLocker locker( &mConfigMutex );
265+
QMutexLocker locker( &mMutex );
262266
QgsPkiConfigBundle *bundle = nullptr;
263267

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

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

304-
locker.unlock();
305308
// cache bundle
306309
putPkiConfigBundle( authcfg, bundle );
307310

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

311314
void QgsAuthPkiPathsMethod::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
312315
{
313-
QMutexLocker locker( &mConfigMutex );
316+
QMutexLocker locker( &mMutex );
314317
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
315318
sPkiConfigBundleCache.insert( authcfg, pkibundle );
316319
}
317320

318321
void QgsAuthPkiPathsMethod::removePkiConfigBundle( const QString &authcfg )
319322
{
320-
QMutexLocker locker( &mConfigMutex );
323+
QMutexLocker locker( &mMutex );
321324
if ( sPkiConfigBundleCache.contains( authcfg ) )
322325
{
323326
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

‎src/auth/pkipaths/qgsauthpkipathsmethod.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ class QgsAuthPkiPathsMethod : public QgsAuthMethod
5959

6060
static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
6161

62-
QMutex mConfigMutex;
63-
6462
};
6563

6664
#endif // QGSAUTHPKIPATHSMETHOD_H

‎src/auth/pkipkcs12/qgsauthpkcs12method.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ QgsAuthPkcs12Method::QgsAuthPkcs12Method()
5353

5454
QgsAuthPkcs12Method::~QgsAuthPkcs12Method()
5555
{
56-
QMutexLocker locker( &mConfigMutex );
5756
qDeleteAll( sPkiConfigBundleCache );
5857
sPkiConfigBundleCache.clear();
5958
}
@@ -77,6 +76,7 @@ bool QgsAuthPkcs12Method::updateNetworkRequest( QNetworkRequest &request, const
7776
const QString &dataprovider )
7877
{
7978
Q_UNUSED( dataprovider )
79+
QMutexLocker locker( &mMutex );
8080

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

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

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

238239
void QgsAuthPkcs12Method::clearCachedConfig( const QString &authcfg )
239240
{
241+
QMutexLocker locker( &mMutex );
240242
removePkiConfigBundle( authcfg );
241243
}
242244

243245
void QgsAuthPkcs12Method::updateMethodConfig( QgsAuthMethodConfig &mconfig )
244246
{
247+
QMutexLocker locker( &mMutex );
245248
if ( mconfig.hasConfig( QStringLiteral( "oldconfigstyle" ) ) )
246249
{
247250
QgsDebugMsg( "Updating old style auth method config" );
@@ -257,7 +260,7 @@ void QgsAuthPkcs12Method::updateMethodConfig( QgsAuthMethodConfig &mconfig )
257260

258261
QgsPkiConfigBundle *QgsAuthPkcs12Method::getPkiConfigBundle( const QString &authcfg )
259262
{
260-
QMutexLocker locker( &mConfigMutex );
263+
QMutexLocker locker( &mMutex );
261264
QgsPkiConfigBundle *bundle = nullptr;
262265

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

332335
void QgsAuthPkcs12Method::putPkiConfigBundle( const QString &authcfg, QgsPkiConfigBundle *pkibundle )
333336
{
334-
QMutexLocker locker( &mConfigMutex );
337+
QMutexLocker locker( &mMutex );
335338
QgsDebugMsg( QString( "Putting PKI bundle for authcfg %1" ).arg( authcfg ) );
336339
sPkiConfigBundleCache.insert( authcfg, pkibundle );
337340
}
338341

339342
void QgsAuthPkcs12Method::removePkiConfigBundle( const QString &authcfg )
340343
{
341-
QMutexLocker locker( &mConfigMutex );
344+
QMutexLocker locker( &mMutex );
342345
if ( sPkiConfigBundleCache.contains( authcfg ) )
343346
{
344347
QgsPkiConfigBundle *pkibundle = sPkiConfigBundleCache.take( authcfg );

‎src/auth/pkipkcs12/qgsauthpkcs12method.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ class QgsAuthPkcs12Method : public QgsAuthMethod
6060

6161
static QMap<QString, QgsPkiConfigBundle *> sPkiConfigBundleCache;
6262

63-
QMutex mConfigMutex;
6463
};
6564

6665
#endif // QGSAUTHPKCS12METHOD_H

‎src/core/auth/qgsauthmethod.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <QNetworkRequest>
2424
#include <QStringList>
2525
#include <QUrl>
26+
#include <QMutex>
2627

2728
#include "qgis_core.h"
2829

@@ -174,8 +175,10 @@ class CORE_EXPORT QgsAuthMethod : public QObject
174175
explicit QgsAuthMethod()
175176
: mExpansions( QgsAuthMethod::Expansions( nullptr ) )
176177
, mDataProviders( QStringList() )
178+
, mMutex( QMutex::RecursionMode::Recursive )
177179
{}
178180

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

@@ -190,6 +193,8 @@ class CORE_EXPORT QgsAuthMethod : public QObject
190193
QgsAuthMethod::Expansions mExpansions;
191194
QStringList mDataProviders;
192195
int mVersion = 0;
196+
QMutex mMutex;
197+
193198
};
194199
Q_DECLARE_OPERATORS_FOR_FLAGS( QgsAuthMethod::Expansions )
195200

0 commit comments

Comments
 (0)
Please sign in to comment.