Merge pull request #39969 from rouault/oauth2_fixes

OAuth2 fixes: threading issuing and handling of short token validity
qgis · Nov 16, 2020 · b38f99c · b38f99c
2 parents f66ddf4 + a8af06f
commit b38f99c
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 25 deletions.
diff --git a/src/auth/oauth2/qgsauthoauth2method.cpp b/src/auth/oauth2/qgsauthoauth2method.cpp
@@ -28,6 +28,8 @@
 #include "qgsmessagelog.h"
 #include "qgssettings.h"
 
+#include <algorithm>
+
 #include <QDateTime>
 #include <QInputDialog>
 #include <QDesktopServices>
@@ -139,7 +141,10 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
     if ( o2->expires() > 0 )  // QStringLiteral("").toInt() result for tokens with no expiration
     {
       int cursecs = static_cast<int>( QDateTime::currentDateTime().toMSecsSinceEpoch() / 1000 );
-      expired = ( ( o2->expires() - cursecs ) < 120 ); // try refresh with expired or two minutes to go
+      const int lExpirationDelay = o2->expirationDelay();
+      // try refresh with expired or two minutes to go (or a fraction of the initial expiration delay if it is short)
+      const int refreshThreshold = lExpirationDelay > 0 ? std::min( 120, std::max( 2, lExpirationDelay / 10 ) ) : 120;
+      expired = ( ( o2->expires() - cursecs ) < refreshThreshold );
     }
 
     if ( expired )
@@ -157,32 +162,11 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
         QgsMessageLog::logMessage( msg, AUTH_METHOD_KEY, Qgis::MessageLevel::Info );
 
         // Try to get a refresh token first
-        // go into local event loop and wait for a fired refresh-related slot
-        QEventLoop rloop( nullptr );
-        connect( o2, &QgsO2::refreshFinished, &rloop, &QEventLoop::quit );
-
-        // add single shot timer to quit refresh after an allotted timeout
-        // this should keep the local event loop from blocking forever
-        QTimer r_timer( nullptr );
-        int r_reqtimeout = o2->oauth2config()->requestTimeout() * 1000;
-        r_timer.setInterval( r_reqtimeout );
-        r_timer.setSingleShot( true );
-        connect( &r_timer, &QTimer::timeout, &rloop, &QEventLoop::quit );
-        r_timer.start();
-
-        // Asynchronously attempt the refresh
-        // TODO: This already has a timed reply setup in O2 base class (and in QgsNetworkAccessManager!)
-        //       May need to address this or app crashes will occur!
-        o2->refresh();
-
-        // block request update until asynchronous linking loop is quit
-        rloop.exec();
-        if ( r_timer.isActive() )
-        {
-          r_timer.stop();
-        }
+        o2->refreshSynchronous();
 
         // refresh result should set o2 to (un)linked
+        if ( o2->linked() )
+          o2->computeExpirationDelay();
       }
     }
   }
@@ -249,6 +233,8 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
       QgsMessageLog::logMessage( msg, AUTH_METHOD_KEY, Qgis::MessageLevel::Warning );
       return false;
     }
+
+    o2->computeExpirationDelay();
   }
 
   if ( o2->token().isEmpty() )

diff --git a/src/auth/oauth2/qgso2.cpp b/src/auth/oauth2/qgso2.cpp
@@ -21,8 +21,11 @@
 #include "qgsauthoauth2config.h"
 #include "qgslogger.h"
 #include "qgsnetworkaccessmanager.h"
+#include "qgsblockingnetworkrequest.h"
 
 #include <QDir>
+#include <QJsonDocument>
+#include <QJsonObject>
 #include <QSettings>
 #include <QUrl>
 #include <QUrlQuery>
@@ -348,3 +351,89 @@ QNetworkAccessManager *QgsO2::getManager()
 {
   return QgsNetworkAccessManager::instance();
 }
+
+/// Parse JSON data into a QVariantMap
+static QVariantMap parseTokenResponse( const QByteArray &data )
+{
+  QJsonParseError err;
+  QJsonDocument doc = QJsonDocument::fromJson( data, &err );
+  if ( err.error != QJsonParseError::NoError )
+  {
+    qWarning() << "parseTokenResponse: Failed to parse token response due to err:" << err.errorString();
+    return QVariantMap();
+  }
+
+  if ( !doc.isObject() )
+  {
+    qWarning() << "parseTokenResponse: Token response is not an object";
+    return QVariantMap();
+  }
+
+  return doc.object().toVariantMap();
+}
+
+// Code adapted from O2::refresh(), but using QgsBlockingNetworkRequest
+void QgsO2::refreshSynchronous()
+{
+  qDebug() << "O2::refresh: Token: ..." << refreshToken().right( 7 );
+
+  if ( refreshToken().isEmpty() )
+  {
+    qWarning() << "O2::refresh: No refresh token";
+    onRefreshError( QNetworkReply::AuthenticationRequiredError );
+    return;
+  }
+  if ( refreshTokenUrl_.isEmpty() )
+  {
+    qWarning() << "O2::refresh: Refresh token URL not set";
+    onRefreshError( QNetworkReply::AuthenticationRequiredError );
+    return;
+  }
+
+  QNetworkRequest refreshRequest( refreshTokenUrl_ );
+  refreshRequest.setHeader( QNetworkRequest::ContentTypeHeader, O2_MIME_TYPE_XFORM );
+  QMap<QString, QString> parameters;
+  parameters.insert( O2_OAUTH2_CLIENT_ID, clientId_ );
+  parameters.insert( O2_OAUTH2_CLIENT_SECRET, clientSecret_ );
+  parameters.insert( O2_OAUTH2_REFRESH_TOKEN, refreshToken() );
+  parameters.insert( O2_OAUTH2_GRANT_TYPE, O2_OAUTH2_REFRESH_TOKEN );
+
+  QByteArray data = buildRequestBody( parameters );
+
+  QgsBlockingNetworkRequest blockingRequest;
+  QgsBlockingNetworkRequest::ErrorCode errCode = blockingRequest.post( refreshRequest, data, true );
+  if ( errCode == QgsBlockingNetworkRequest::NoError )
+  {
+    QByteArray reply = blockingRequest.reply().content();
+    QVariantMap tokens = parseTokenResponse( reply );
+    if ( tokens.contains( QStringLiteral( "error" ) ) )
+    {
+      qDebug() << " Error refreshing token" << tokens.value( QStringLiteral( "error" ) ).toMap().value( QStringLiteral( "message" ) ).toString().toLocal8Bit().constData();
+      unlink();
+    }
+    else
+    {
+      setToken( tokens.value( O2_OAUTH2_ACCESS_TOKEN ).toString() );
+      setExpires( QDateTime::currentMSecsSinceEpoch() / 1000 + tokens.value( O2_OAUTH2_EXPIRES_IN ).toInt() );
+      const QString refreshToken = tokens.value( O2_OAUTH2_REFRESH_TOKEN ).toString();
+      if ( !refreshToken.isEmpty() )
+        setRefreshToken( refreshToken );
+      setLinked( true );
+      qDebug() << " New token expires in" << expires() << "seconds";
+      emit linkingSucceeded();
+    }
+    emit refreshFinished( QNetworkReply::NoError );
+  }
+  else
+  {
+    unlink();
+    qDebug() << "O2::onRefreshFinished: Error" << blockingRequest.errorMessage();
+    emit refreshFinished( blockingRequest.reply().error() );
+  }
+}
+
+void QgsO2::computeExpirationDelay()
+{
+  const int lExpires = expires();
+  mExpirationDelay = lExpires > 0 ? lExpires - static_cast<int>( QDateTime::currentMSecsSinceEpoch() / 1000 ) : 0;
+}
diff --git a/src/auth/oauth2/qgso2.h b/src/auth/oauth2/qgso2.h
@@ -58,6 +58,19 @@ class QgsO2: public O2
     //! Store oauth2 state to a random value when called
     void setState( const QString &value );
 
+    //! Refresh token in a synchronous way
+    void refreshSynchronous();
+
+    /**
+     * Compute expiration delay from current timestamp and expires()
+     * Should only be called just after a refresh / link event. */
+    void computeExpirationDelay();
+
+    /** Returns expiration delay.
+     * May be 0 if it is unknown
+     */
+    int expirationDelay() const { return mExpirationDelay; }
+
   public slots:
 
     //! Clear all properties
@@ -101,6 +114,8 @@ class QgsO2: public O2
     QString state_;
     QgsAuthOAuth2Config *mOAuth2Config;
     bool mIsLocalHost = false;
+    int mExpirationDelay = 0;
+
     static QString O2_OAUTH2_STATE;
 };