Merge lp:~online-accounts/signon-plugin-oauth2/packaging into lp:signon-plugin-oauth2

[Alberto Mardegan]

New upstream release

- OAuth2: implement a fallback mechanism when parsing replies
  (LP: #1415376)

New upstream release

- OAuth2: implement a fallback mechanism when parsing replies
  (LP: #1415376)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:71
http://jenkins.qa.ubuntu.com/job/signon-plugin-oauth2-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/signon-plugin-oauth2-vivid-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/signon-plugin-oauth2-vivid-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/signon-plugin-oauth2-vivid-armhf-ci/1/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/signon-plugin-oauth2-vivid-i386-ci/1

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/signon-plugin-oauth2-ci/16/rebuild

review: Approve (continuous-integration)

[David Barth (dbarth)]

How is the version bump and extra functions affecting binary compatibility?

[David Barth (dbarth)]

Fine for submitting to vivid.

review: Approve

[lp:~online-accounts/signon-plugin-oauth2/packaging]

70. By Alberto Mardegan on 2015-01-28

New upstream release

- OAuth2: implement a fallback mechanism when parsing replies
  (LP: #1415376) Fixes: #1415376
Approved by: PS Jenkins bot, David Barth

=== modified file 'common-vars.pri'
--- common-vars.pri	2014-09-11 09:32:32 +0000
+++ common-vars.pri	2015-01-28 12:16:47 +0000
@@ -13,7 +13,7 @@
 # Project version
 # remember to update debian/* files if you changes this
 #-----------------------------------------------------------------------------
-PROJECT_VERSION = 0.20
+PROJECT_VERSION = 0.21
 
 #-----------------------------------------------------------------------------
 # Library version

=== modified file 'debian/changelog'
--- debian/changelog	2014-11-10 09:45:53 +0000
+++ debian/changelog	2015-01-28 12:16:47 +0000
@@ -1,3 +1,14 @@
+signon-plugin-oauth2 (0.21-0ubuntu1) UNRELEASED; urgency=medium
+
+  * New upstream release
+    - Add ForceTokenRefresh flag for forcing a new token
+    - OAuth2: implement a fallback mechanism when parsing replies
+      (LP: #1415376)
+    - Fixes build with -Werror=unused-variable
+    - Improve test coverage
+
+ -- Alberto Mardegan <[e-mail]>  Wed, 28 Jan 2015 13:09:28 +0100
+
 signon-plugin-oauth2 (0.20+15.04.20141110-0ubuntu1) vivid; urgency=low
 
   [ Ubuntu daily release ]

=== modified file 'src/oauth2plugin.cpp'
--- src/oauth2plugin.cpp	2014-10-31 08:09:57 +0000
+++ src/oauth2plugin.cpp	2015-01-28 12:16:47 +0000
@@ -430,6 +430,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)
 {
@@ -446,55 +487,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
@@ -567,8 +583,6 @@
 
 void OAuth2Plugin::refreshOAuth2Token(const QString &refreshToken)
 {
-    Q_D(OAuth2Plugin);
-
     TRACE() << refreshToken;
     QUrl url;
     url.addQueryItem(GRANT_TYPE, REFRESH_TOKEN);
@@ -648,7 +662,7 @@
     TRACE() << d->m_tokens;
 }
 
-const QVariantMap OAuth2Plugin::parseJSONReply(const QByteArray &reply)
+QVariantMap OAuth2Plugin::parseJSONReply(const QByteArray &reply)
 {
     TRACE();
     bool ok = false;
@@ -666,10 +680,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-01-28 12:16:47 +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-10-31 08:09:57 +0000
+++ tests/oauth2plugintest.cpp	2015-01-28 12:16:47 +0000
@@ -847,6 +847,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) <<
@@ -934,6 +944,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()

=== modified file 'tests/tests.pro'
--- tests/tests.pro	2014-10-31 13:34:21 +0000
+++ tests/tests.pro	2015-01-28 12:16:47 +0000
@@ -36,8 +36,8 @@
         libsignon-qt5
 }
 
-target.path = /usr/bin
-testsuite.path = /usr/share/$$TARGET
+target.path = $${INSTALL_PREFIX}/bin
+testsuite.path = $${INSTALL_PREFIX}/share/$$TARGET
 testsuite.files = tests.xml
 INSTALLS += target \
     testsuite