Merge lp:~mardy/signon-plugin-oauth2/lp1417429-rtm into lp:signon-plugin-oauth2/rtm-14.09

[Alberto Mardegan]

OAuth2: implement a fallback mechanism when parsing replies

OAuth2: implement a fallback mechanism when parsing replies

[David Barth (dbarth)]

review: Approve

=== modified file 'src/oauth2plugin.cpp'
--- src/oauth2plugin.cpp	2014-09-09 13:47:22 +0000
+++ src/oauth2plugin.cpp	2015-02-04 13:10:21 +0000
@@ -421,6 +421,47 @@
     }
 }
 
+QVariantMap OAuth2Plugin::parseReply(const QByteArray &contentType,
+                                     const QByteArray &replyContent)
+{
+    typedef QVariantMap (OAuth2Plugin::*Parser)(const QByteArray &replyContent);
+    Parser preferredParser;
+    Parser fallbackParser;
+
+    QVariantMap map;
+
+    // Handling application/json content type
+    if (contentType.startsWith(CONTENT_APP_JSON)) {
+        TRACE() << "application/json content received";
+        preferredParser = &OAuth2Plugin::parseJSONReply;
+        fallbackParser = &OAuth2Plugin::parseTextReply;
+    }
+    // Added for facebook Graph API's (handling text/plain content type)
+    else if (contentType.startsWith(CONTENT_TEXT_PLAIN) ||
+               contentType.startsWith(CONTENT_APP_URLENCODED)) {
+        TRACE() << contentType << "content received";
+        preferredParser = &OAuth2Plugin::parseTextReply;
+        fallbackParser = &OAuth2Plugin::parseJSONReply;
+    } else {
+        TRACE() << "Unsupported content type received: " << contentType;
+        Q_EMIT error(Error(Error::OperationFailed,
+                           QString("Unsupported content type received")));
+        return map;
+    }
+
+    map = (this->*preferredParser)(replyContent);
+    if (Q_UNLIKELY(map.isEmpty())) {
+        TRACE() << "Parse failed, trying fallback parser";
+        map = (this->*fallbackParser)(replyContent);
+        if (Q_UNLIKELY(map.isEmpty())) {
+            TRACE() << "Parse failed";
+            Q_EMIT error(Error(Error::NotAuthorized,
+                               QString("No access token found")));
+        }
+    }
+    return map;
+}
+
 // Method to handle responses for OAuth 2.0 requests
 void OAuth2Plugin::serverReply(QNetworkReply *reply)
 {
@@ -437,55 +478,30 @@
 
     // Handling 200 OK response (HTTP_STATUS_OK) WITH content
     if (reply->hasRawHeader(CONTENT_TYPE)) {
-
-        // Handling application/json content type
-        if (reply->rawHeader(CONTENT_TYPE).startsWith(CONTENT_APP_JSON)) {
-            TRACE()<< "application/json content received";
-            QVariantMap map = parseJSONReply(replyContent);
-            QByteArray accessToken = map["access_token"].toByteArray();
-            QVariant expiresIn = map["expires_in"];
-            QByteArray refreshToken = map["refresh_token"].toByteArray();
-
-            if (accessToken.isEmpty()) {
-                TRACE()<< "Access token is empty";
-                emit error(Error(Error::NotAuthorized,
-                                 QString("Access token is empty")));
-            }
-            else {
-                OAuth2PluginTokenData response;
-                response.setAccessToken(accessToken);
-                response.setRefreshToken(refreshToken);
-                response.setExpiresIn(expiresIn.toInt());
-                storeResponse(response);
-                emit result(response);
-            }
-        }
-        // Added to test with facebook Graph API's (handling text/plain content type)
-        else if (reply->rawHeader(CONTENT_TYPE).startsWith(CONTENT_TEXT_PLAIN) ||
-                 reply->rawHeader(CONTENT_TYPE).startsWith(CONTENT_APP_URLENCODED)) {
-            TRACE()<< reply->rawHeader(CONTENT_TYPE) << "content received";
-            QMap<QString,QString> map = parseTextReply(replyContent);
-            QByteArray accessToken = map["access_token"].toAscii();
-            QByteArray expiresIn = map["expires"].toAscii();
-            QByteArray refreshToken = map["refresh_token"].toAscii();
-
-            if (accessToken.isEmpty()) {
-                TRACE()<< "Access token is empty";
-                emit error(Error(Error::NotAuthorized,
-                                 QString("Access token is empty")));
-            }
-            else {
-                OAuth2PluginTokenData response;
-                response.setAccessToken(accessToken);
-                response.setRefreshToken(refreshToken);
-                response.setExpiresIn(expiresIn.toInt());
-                storeResponse(response);
-                emit result(response);
-            }
-        }
-        else {
-            TRACE()<< "Unsupported content type received: " << reply->rawHeader(CONTENT_TYPE);
-            emit error(Error(Error::OperationFailed, QString("Unsupported content type received")));
+        QVariantMap map = parseReply(reply->rawHeader(CONTENT_TYPE), replyContent);
+        if (Q_UNLIKELY(map.isEmpty())) {
+            // The error has already been delivered
+            return;
+        }
+        QByteArray accessToken = map["access_token"].toByteArray();
+        int expiresIn = map["expires_in"].toInt();
+        if (expiresIn == 0) {
+            // Facebook uses just "expires" as key
+            expiresIn = map["expires"].toInt();
+        }
+        QByteArray refreshToken = map["refresh_token"].toByteArray();
+
+        if (accessToken.isEmpty()) {
+            TRACE()<< "Access token is empty";
+            Q_EMIT error(Error(Error::NotAuthorized,
+                               QString("Access token is empty")));
+        } else {
+            OAuth2PluginTokenData response;
+            response.setAccessToken(accessToken);
+            response.setRefreshToken(refreshToken);
+            response.setExpiresIn(expiresIn);
+            storeResponse(response);
+            emit result(response);
         }
     }
     // Handling 200 OK response (HTTP_STATUS_OK) WITHOUT content
@@ -639,7 +655,7 @@
     TRACE() << d->m_tokens;
 }
 
-const QVariantMap OAuth2Plugin::parseJSONReply(const QByteArray &reply)
+QVariantMap OAuth2Plugin::parseJSONReply(const QByteArray &reply)
 {
     TRACE();
     bool ok = false;
@@ -657,10 +673,10 @@
     return QVariantMap();
 }
 
-const QMap<QString, QString> OAuth2Plugin::parseTextReply(const QByteArray &reply)
+QVariantMap OAuth2Plugin::parseTextReply(const QByteArray &reply)
 {
     TRACE();
-    QMap<QString, QString> map;
+    QVariantMap map;
     QList<QByteArray> items = reply.split('&');
     foreach (QByteArray item, items) {
         int idx = item.indexOf("=");

=== modified file 'src/oauth2plugin.h'
--- src/oauth2plugin.h	2014-09-09 13:47:22 +0000
+++ src/oauth2plugin.h	2015-02-04 13:10:21 +0000
@@ -75,8 +75,10 @@
     void sendOAuth2PostRequest(QUrl &postData,
                                GrantType::e grantType);
     void storeResponse(const OAuth2PluginTokenData &response);
-    const QVariantMap parseJSONReply(const QByteArray &reply);
-    const QMap<QString, QString> parseTextReply(const QByteArray &reply);
+    QVariantMap parseReply(const QByteArray &contentType,
+                           const QByteArray &replyContent);
+    QVariantMap parseJSONReply(const QByteArray &reply);
+    QVariantMap parseTextReply(const QByteArray &reply);
     void handleOAuth2Error(const QByteArray &reply);
     QString urlEncode(QString strData);
 

=== modified file 'tests/oauth2plugintest.cpp'
--- tests/oauth2plugintest.cpp	2014-09-09 14:02:38 +0000
+++ tests/oauth2plugintest.cpp	2015-02-04 13:10:21 +0000
@@ -650,6 +650,16 @@
         "something else" <<
         QVariantMap();
 
+    QTest::newRow("reply code, no access token") <<
+        "http://localhost/resp.html?code=c0d3" <<
+        int(Error::NotAuthorized) <<
+        "https://localhost/access_token" <<
+        "grant_type=authorization_code&code=c0d3&redirect_uri=http://localhost/resp.html" <<
+        int(200) <<
+        "application/json" <<
+        "{ \"expires_in\": 3600 }" <<
+        QVariantMap();
+
     QTest::newRow("reply code, no content type") <<
         "http://localhost/resp.html?code=c0d3" <<
         int(Error::OperationFailed) <<
@@ -737,6 +747,20 @@
         "application/json" <<
         "{ \"access_token\":\"t0k3n\", \"expires_in\": 3600 }" <<
         response;
+
+    response.clear();
+    response.insert("AccessToken", "t0k3n");
+    response.insert("ExpiresIn", int(3600));
+    response.insert("RefreshToken", QString());
+    QTest::newRow("username-password, valid token, wrong content type") <<
+        "http://localhost/resp.html?username=us3r&password=s3cr3t" <<
+        int(-1) <<
+        "https://localhost/access_token" <<
+        "grant_type=user_basic&username=us3r&password=s3cr3t" <<
+        int(200) <<
+        "text/plain" <<
+        "{ \"access_token\":\"t0k3n\", \"expires_in\": 3600 }" <<
+        response;
 }
 
 void OAuth2PluginTest::testPluginWebserverUserActionFinished()