Seb Ruiz avatar Seb Ruiz committed ce41018 Merge

Merged in fixes (pull request #21)

fixes

Comments (0)

Files changed (4)

     // auto-register routes for each webhook in the descriptor
     var webhooks = self.descriptor.modules.webhooks;
     if (webhooks) {
+        if (!_.isArray(webhooks)) {
+            webhooks = [webhooks];
+        }
         webhooks.forEach(function (webhook) {
             var webhookUrl = basePath + webhook.url;
             self.app.post(

lib/internal/jwt.js

     return segments.join('.');
 };
 
-jwt.createCanonicalRequest = function createQueryStringHash(req) {
-    return canonicalizeMethod(req) + '&' + canonicalizeUri(req) + '&' + canonicalizeQueryString(req);
+jwt.createCanonicalRequest = function createQueryStringHash(req, checkBodyForParams) {
+    return canonicalizeMethod(req) + '&' + canonicalizeUri(req) + '&' + canonicalizeQueryString(req, checkBodyForParams);
 };
 
-jwt.createQueryStringHash = function createQueryStringHash(req) {
-    return crypto.createHash(algorithmMap.HS256).update(this.createCanonicalRequest(req)).digest('hex');
+jwt.createQueryStringHash = function createQueryStringHash(req, checkBodyForParams) {
+    return crypto.createHash(algorithmMap.HS256).update(this.createCanonicalRequest(req, checkBodyForParams)).digest('hex');
 };
 
 
     return path;
 }
 
-function canonicalizeQueryString(req) {
+function canonicalizeQueryString(req, checkBodyForParams) {
+    var queryParams = req.query,
+            method = req.method.toUpperCase();
+
+    // Apache HTTP client (or something) sometimes likes to take the query string and put it into the request body
+    // if the method is PUT or POST
+    if (checkBodyForParams && _.isEmpty(queryParams) && (method === 'POST' || method === 'PUT')) {
+        queryParams = req.body;
+    }
+
     var sortedQueryString = [],
-            query = _.extend({}, req.query);
+        query = _.extend({}, queryParams);
     if (!_.isEmpty(query)) {
         // remote the 'jwt' query string param
         delete query['jwt'];

lib/middleware/authentication.js

                 return;
             }
 
-            var expectedHash = jwt.createQueryStringHash(request);
+            // First check query string params
+            var expectedHash = jwt.createQueryStringHash(request, false);
             var signatureHashVerified = verifiedClaims.qsh === expectedHash;
             if (!signatureHashVerified) {
-                sendError(401, 'Query hash does not match. Received: "' + verifiedClaims.qsh + '" but calculated "' + expectedHash + '". ' +
-                               'Canonical query was: "' + jwt.createCanonicalRequest(request) + '".');
-                return;
+                // Send the error message for the first verification - it's 90% more likely to be the one we want.
+                var error = 'Auth failure: Query hash mismatch: Received: "' + verifiedClaims.qsh + '" but calculated "' + expectedHash + '". ' +
+                                                   'Canonical query was: "' + jwt.createCanonicalRequest(request);
+                // If that didn't verify, it might be a post/put - check the request body too
+                expectedHash = jwt.createQueryStringHash(request, true);
+                signatureHashVerified = verifiedClaims.qsh === expectedHash;
+                if (!signatureHashVerified) {
+                    addon.logger.error(error);
+                    sendError(401, 'Authentication failed: query hash does not match.');
+                    return;
+                }
             }
 
             success(verifiedClaims, remoteBaseUrl);
         assert.equal(qsh, expectedHash);
         done();
     });
+
+    // apache http client likes to do this
+    it('should correctly create qsh with POST body query string', function (done) {
+        var req = {
+            method: 'post',
+            path: '/hello-world',
+            query: {},
+            body: qs.parse('lic=none&tz=Australia%2FSydney&cp=%2Fjira&user_key=&loc=en-US&user_id=&jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzODY5MTEzNTYsImlzcyI6ImppcmE6MTU0ODk1OTUiLCJxc2giOiI4MDYzZmY0Y2ExZTQxZGY3YmM5MGM4YWI2ZDBmNjIwN2Q0OTFjZjZkYWQ3YzY2ZWE3OTdiNDYxNGI3MTkyMmU5IiwiaWF0IjoxMzg2OTExMTc2fQ.rAsxpHv0EvpXkhjnZnSV14EXJgDx3KSQjgYRjfKnFt8&xdm_e=http%3A%2F%2Fstorm%3A2990&xdm_c=channel-servlet-hello-world&xdm_p=1')
+        };
+        var expectedHash = "d7e7f00660965fc15745b2c423a89b85d0853c4463faca362e0371d008eb0927";
+
+        var qsh = jwt.createQueryStringHash(req, true);
+        assert.equal(qsh, expectedHash);
+        done();
+    });
+
+    // apache http client likes to do this
+    it('should not correctly create qsh with POST body query string if not instructed to', function (done) {
+        var req = {
+            method: 'post',
+            path: '/hello-world',
+            query: {},
+            body: qs.parse('lic=none&tz=Australia%2FSydney&cp=%2Fjira&user_key=&loc=en-US&user_id=&jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzODY5MTEzNTYsImlzcyI6ImppcmE6MTU0ODk1OTUiLCJxc2giOiI4MDYzZmY0Y2ExZTQxZGY3YmM5MGM4YWI2ZDBmNjIwN2Q0OTFjZjZkYWQ3YzY2ZWE3OTdiNDYxNGI3MTkyMmU5IiwiaWF0IjoxMzg2OTExMTc2fQ.rAsxpHv0EvpXkhjnZnSV14EXJgDx3KSQjgYRjfKnFt8&xdm_e=http%3A%2F%2Fstorm%3A2990&xdm_c=channel-servlet-hello-world&xdm_p=1')
+        };
+        var expectedHash = "6f95f3738e1b037a3bebbe0ad237d80fdbc1d5ae452e98ce03a9c004c178ebb4";
+
+        var qsh = jwt.createQueryStringHash(req, false);
+        assert.equal(qsh, expectedHash);
+        done();
+    });
 });
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.