From 0f5bb8528f54f83149e537903141f0ac53858105 Mon Sep 17 00:00:00 2001 From: Oren Robinson Date: Tue, 22 Dec 2015 15:25:24 -0800 Subject: [PATCH 1/7] Highlight failing tests --- test/errors.test.js | 8 ++++---- test/resources.test.js | 2 +- test/validator-remote.test.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/errors.test.js b/test/errors.test.js index 3d8fae8..c339a73 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -8,7 +8,7 @@ const jws = require('jws'); const keys = require('./test-keys'); const util = require('util'); -var UNREACHABLE = 'http://nope.example.org/'; // not sure how to do this with nock +var UNREACHABLE = 'http://example.org/'; var ORIGIN = 'https://example.org'; var httpScope = function() { nock.cleanAll(); @@ -179,7 +179,7 @@ test('validateHosted', function (t) { }); validator.validateHosted(assertion, function(err, data) { t.ok(err, 'should have error'); - t.same(err.code, 'unreachable'); + t.same(err.code, 'unreachable'); // Fails when UNREACHABLE = 'http://nope.example.org' t.ok(err.message, 'has message'); t.end(); }); @@ -268,7 +268,7 @@ test('validateHostedUrl', function (t) { httpScope() .get('/assertion').reply(404); validator.validateHostedUrl(ORIGIN + '/assertion', function(err, data) { - t.ok(err, 'should have error'); + t.ok(err, 'should have error'); // Times out t.same(err.code, 'http-status'); t.same(err.url, ORIGIN + '/assertion'); t.same(err.received, 404); @@ -282,7 +282,7 @@ test('validateHostedUrl', function (t) { t.test('assertion url unreachable', function(t) { validator.validateHostedUrl(UNREACHABLE + '/assertion', function(err, data) { t.ok(err, 'should have error'); - t.same(err.code, 'unreachable'); + t.same(err.code, 'unreachable'); // Fails when UNREACHABLE = 'http://nope.example.org' t.same(err.url, UNREACHABLE + '/assertion'); t.ok(err.reason, 'has reason'); t.ok(err.message, 'has message'); diff --git a/test/resources.test.js b/test/resources.test.js index bf743a2..8dcf228 100644 --- a/test/resources.test.js +++ b/test/resources.test.js @@ -159,7 +159,7 @@ test('resources', function (t) { 'c.image': { required: true }, 'd.does.not.exist': { optional: false } }, function (err, results) { - t.same(results['a.root'].toString(), 'a.root'); + t.same(results['a.root'].toString(), 'a.root'); // Times out t.same(results['a.nested.url'].toString(), 'a.nested.url'); t.same(err['b.image'].code, 'content-type'); diff --git a/test/validator-remote.test.js b/test/validator-remote.test.js index 6fec15f..f0c6a41 100644 --- a/test/validator-remote.test.js +++ b/test/validator-remote.test.js @@ -58,7 +58,7 @@ test('validator.getLinkedResources, all errors', function (t) { issuer: generators['1.0.0-issuer']() }; validator.getLinkedResources(structures, function (err, results) { - t.same(err.code, 'resources', 'code should be resources'); + t.same(err.code, 'resources', 'code should be resources'); // Times out t.same(err.extra['assertion.image'].code, 'unreachable'); t.same(err.extra['assertion.verify.url'].code, 'unreachable'); t.same(err.extra['badge.image'].code, 'unreachable'); From 16ca1264dc57618d12b57d100ccbe2148bf6f271 Mon Sep 17 00:00:00 2001 From: Oren Robinson Date: Tue, 22 Dec 2015 15:25:51 -0800 Subject: [PATCH 2/7] Update jws to ~3.1.0 --- index.js | 4 +++- package.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 680f139..69a8907 100644 --- a/index.js +++ b/index.js @@ -388,6 +388,7 @@ function unpackJWS(signature, callback) { const payload = jsonParse(parts.payload); if (!payload) return callback(makeError('jws-payload-parse')); + payload.header = parts.header; return callback(null, payload) } @@ -457,9 +458,10 @@ function fullValidateSignedAssertion(signature, callback) { return getLinkedResources(structures, callback); }, function verifySignature(resources, callback) { + var algorithm = data.structures.assertion.header.alg; data.resources = resources; const publicKey = resources['assertion.verify.url']; - if (!jws.verify(signature, publicKey)) + if (!jws.verify(signature, algorithm, publicKey)) return callback(makeError('verify-signature')) return callback(null, resources); }, diff --git a/package.json b/package.json index 81ba746..6448e93 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "dataurl": "~0.1.0", "request": "~2.14.0", "async": "~0.2.5", - "jws": "0.2.2", + "jws": "~3.1.0", "deep-equal": "0.0.0" }, "devDependencies": { From 830f24113136c23e9ab36ef266ad5c2837f4198a Mon Sep 17 00:00:00 2001 From: Oren Robinson Date: Tue, 22 Dec 2015 15:35:54 -0800 Subject: [PATCH 3/7] Update failing tests --- test/errors.test.js | 2 +- test/resources.test.js | 2 +- test/validator-remote.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/errors.test.js b/test/errors.test.js index c339a73..18a91bf 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -268,7 +268,7 @@ test('validateHostedUrl', function (t) { httpScope() .get('/assertion').reply(404); validator.validateHostedUrl(ORIGIN + '/assertion', function(err, data) { - t.ok(err, 'should have error'); // Times out + t.ok(err, 'should have error'); t.same(err.code, 'http-status'); t.same(err.url, ORIGIN + '/assertion'); t.same(err.received, 404); diff --git a/test/resources.test.js b/test/resources.test.js index 8dcf228..bf743a2 100644 --- a/test/resources.test.js +++ b/test/resources.test.js @@ -159,7 +159,7 @@ test('resources', function (t) { 'c.image': { required: true }, 'd.does.not.exist': { optional: false } }, function (err, results) { - t.same(results['a.root'].toString(), 'a.root'); // Times out + t.same(results['a.root'].toString(), 'a.root'); t.same(results['a.nested.url'].toString(), 'a.nested.url'); t.same(err['b.image'].code, 'content-type'); diff --git a/test/validator-remote.test.js b/test/validator-remote.test.js index f0c6a41..fa78874 100644 --- a/test/validator-remote.test.js +++ b/test/validator-remote.test.js @@ -58,7 +58,7 @@ test('validator.getLinkedResources, all errors', function (t) { issuer: generators['1.0.0-issuer']() }; validator.getLinkedResources(structures, function (err, results) { - t.same(err.code, 'resources', 'code should be resources'); // Times out + t.same(err.code, 'resources', 'code should be resources'); // Times out consistently t.same(err.extra['assertion.image'].code, 'unreachable'); t.same(err.extra['assertion.verify.url'].code, 'unreachable'); t.same(err.extra['badge.image'].code, 'unreachable'); From 2d29d0aeb5874c3510bd28ac8301a14071968a67 Mon Sep 17 00:00:00 2001 From: baisong Date: Wed, 23 Dec 2015 09:51:10 -0800 Subject: [PATCH 4/7] Upgrade nock to ~1.0.0, update header test --- package.json | 4 ++-- test/validator-remote.test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 6448e93..2ec1148 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "deep-equal": "0.0.0" }, "devDependencies": { - "nock": "~0.22.1", + "nock": "~1.0.0", "tap": "~0.4.0", "underscore": "~1.5.1", "sinon": "~1.7.3", @@ -23,7 +23,7 @@ "cover": "0.2.9" }, "engines": { - "node": "~0.10.0" + "node": "~0.12.7" }, "scripts": { "test": "node node_modules/tap/bin/tap test/*.test.js" diff --git a/test/validator-remote.test.js b/test/validator-remote.test.js index fa78874..7b632af 100644 --- a/test/validator-remote.test.js +++ b/test/validator-remote.test.js @@ -133,7 +133,7 @@ test('validator.unpackJWS: bad payload', function (t) { }); test('validator.unpackJWS: everything good', function (t) { - const expect = {sup: 'lol'}; + const expect = {sup: 'lol', header: { alg: 'rs256' }}; const signature = jws.sign({ header: { alg: 'rs256' }, payload: expect, From 27634193095eb41b183c615bb22282fb78b8f3ca Mon Sep 17 00:00:00 2001 From: baisong Date: Wed, 23 Dec 2015 09:56:36 -0800 Subject: [PATCH 5/7] Clean up changes... --- package.json | 2 +- test/validator-remote.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 2ec1148..5cfce14 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "cover": "0.2.9" }, "engines": { - "node": "~0.12.7" + "node": "~0.10.0" }, "scripts": { "test": "node node_modules/tap/bin/tap test/*.test.js" diff --git a/test/validator-remote.test.js b/test/validator-remote.test.js index 7b632af..ad1417a 100644 --- a/test/validator-remote.test.js +++ b/test/validator-remote.test.js @@ -58,7 +58,7 @@ test('validator.getLinkedResources, all errors', function (t) { issuer: generators['1.0.0-issuer']() }; validator.getLinkedResources(structures, function (err, results) { - t.same(err.code, 'resources', 'code should be resources'); // Times out consistently + t.same(err.code, 'resources', 'code should be resources'); t.same(err.extra['assertion.image'].code, 'unreachable'); t.same(err.extra['assertion.verify.url'].code, 'unreachable'); t.same(err.extra['badge.image'].code, 'unreachable'); From 5789624b129cfce15b02a3aebe4b58dd21e3296c Mon Sep 17 00:00:00 2001 From: baisong Date: Wed, 23 Dec 2015 09:59:17 -0800 Subject: [PATCH 6/7] More cleaning... --- test/errors.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/errors.test.js b/test/errors.test.js index 18a91bf..2120445 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -8,7 +8,7 @@ const jws = require('jws'); const keys = require('./test-keys'); const util = require('util'); -var UNREACHABLE = 'http://example.org/'; +var UNREACHABLE = 'http://nope.example.org/'; var ORIGIN = 'https://example.org'; var httpScope = function() { nock.cleanAll(); @@ -179,7 +179,7 @@ test('validateHosted', function (t) { }); validator.validateHosted(assertion, function(err, data) { t.ok(err, 'should have error'); - t.same(err.code, 'unreachable'); // Fails when UNREACHABLE = 'http://nope.example.org' + t.same(err.code, 'unreachable'); t.ok(err.message, 'has message'); t.end(); }); @@ -282,7 +282,7 @@ test('validateHostedUrl', function (t) { t.test('assertion url unreachable', function(t) { validator.validateHostedUrl(UNREACHABLE + '/assertion', function(err, data) { t.ok(err, 'should have error'); - t.same(err.code, 'unreachable'); // Fails when UNREACHABLE = 'http://nope.example.org' + t.same(err.code, 'unreachable'); t.same(err.url, UNREACHABLE + '/assertion'); t.ok(err.reason, 'has reason'); t.ok(err.message, 'has message'); From 9f0491a92b7a74415a817870ce1f98c7a2bc1f04 Mon Sep 17 00:00:00 2001 From: baisong Date: Wed, 23 Dec 2015 10:02:59 -0800 Subject: [PATCH 7/7] Fewest changes possible --- test/errors.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/errors.test.js b/test/errors.test.js index 2120445..3d8fae8 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -8,7 +8,7 @@ const jws = require('jws'); const keys = require('./test-keys'); const util = require('util'); -var UNREACHABLE = 'http://nope.example.org/'; +var UNREACHABLE = 'http://nope.example.org/'; // not sure how to do this with nock var ORIGIN = 'https://example.org'; var httpScope = function() { nock.cleanAll();