From 0da2ad0c1044568ae78fe8ee2d81b42f64ccecb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Fri, 12 Dec 2014 20:26:19 +0100 Subject: [PATCH 1/3] Add API limits configurable in settings.json --- bin/server.js | 5 +- lib/server/authMiddleware.js | 52 ------ lib/server/middlewares/apiLimitsMiddleware.js | 85 +++++++++ lib/server/middlewares/authMiddleware.js | 42 +++++ package.json | 1 - server_config/settings.json | 6 +- test/api/apiTest.js | 163 ++++++++---------- 7 files changed, 202 insertions(+), 152 deletions(-) delete mode 100644 lib/server/authMiddleware.js create mode 100644 lib/server/middlewares/apiLimitsMiddleware.js create mode 100644 lib/server/middlewares/authMiddleware.js diff --git a/bin/server.js b/bin/server.js index 8fda8f9..f8b76aa 100644 --- a/bin/server.js +++ b/bin/server.js @@ -7,11 +7,14 @@ var server = require('http').createServer(app); var bodyParser = require('body-parser'); var compress = require('compression'); -var authMiddleware = require('../lib/server/authMiddleware'); +var authMiddleware = require('../lib/server/middlewares/authMiddleware'); +var apiLimitsMiddleware = require('../lib/server/middlewares/apiLimitsMiddleware'); + app.use(compress()); app.use(bodyParser.json()); app.use(authMiddleware); +app.use(apiLimitsMiddleware); // Initialize the controllers diff --git a/lib/server/authMiddleware.js b/lib/server/authMiddleware.js deleted file mode 100644 index fe70125..0000000 --- a/lib/server/authMiddleware.js +++ /dev/null @@ -1,52 +0,0 @@ -var config = require('../../server_config/settings.json'); - -var jwt = require('jwt-simple'); -var debug = require('debug')('authMiddleware'); - - -var authMiddleware = function(req, res, next) { - 'use strict'; - - if (req.path.indexOf('/api/') === 0) { - - // Test if it's an authorized key - if (req.headers && req.headers['x-api-key'] && isApiKeyValid(req.headers['x-api-key'])) { - next(); - return; - } - - // Test if it's an authorized token - if (req.headers && req.headers['x-api-token'] && isTokenValid(req.headers['x-api-token'])) { - next(); - return; - } - - res.status(401).send('Unauthorized'); - } -}; - - -function isApiKeyValid(apiKey) { - return (config.authorizedKeys[apiKey]) ? true : false; -} - - -function isTokenValid(token) { - - var data = null; - - try { - data = jwt.decode(token, config.tokenSalt); - } catch(err) { - debug('Error while decoding token'); - debug(err); - return false; - } - - return data.expire && - data.expire > Date.now() && - data.application && - config.authorizedApplications.indexOf(data.application) >= 0; -} - -module.exports = authMiddleware; \ No newline at end of file diff --git a/lib/server/middlewares/apiLimitsMiddleware.js b/lib/server/middlewares/apiLimitsMiddleware.js new file mode 100644 index 0000000..7c17015 --- /dev/null +++ b/lib/server/middlewares/apiLimitsMiddleware.js @@ -0,0 +1,85 @@ +var config = require('../../../server_config/settings.json'); + +var debug = require('debug')('apiLimitsMiddleware'); + + +var apiLimitsMiddleware = function(req, res, next) { + 'use strict'; + + debug('Entering API Limits Middleware with IP address %s', req.connection.remoteAddress); + + if (req.path.indexOf('/api/') === 0 && !res.locals.hasApiKey) { + + + if (req.path === '/api/runs') { + + if (!runsTable.accepts(req.connection.remoteAddress)) { + // Sorry :/ + debug('Too many tests launched from IP address %s', req.connection.remoteAddress); + res.status(429).send('Too Many Requests'); + return; + } + + } + + if (!callsTable.accepts(req.connection.remoteAddress)) { + // Sorry :/ + debug('Too many API requests from IP address %s', req.connection.remoteAddress); + res.status(429).send('Too Many Requests'); + return; + } + + debug('Not blocked by the API limits'); + // It's ok for the moment + } + + next(); +}; + + +var RecordTable = function(maxPerDay) { + var table = {}; + + // Check if the user overpassed the limit and save its visit + this.accepts = function(ipAddress) { + if (table[ipAddress]) { + + this.cleanEntry(ipAddress); + + debug('%d visits in the last 24 hours', table[ipAddress].length); + + if (table[ipAddress].length >= maxPerDay) { + return false; + } else { + table[ipAddress].push(Date.now()); + } + + } else { + table[ipAddress] = []; + table[ipAddress].push(Date.now()); + } + + return true; + }; + + // Clean the table for this guy + this.cleanEntry = function(ipAddress) { + table[ipAddress] = table[ipAddress].filter(function(date) { + return date > Date.now() - 1000*60*60*24; + }); + }; + + // Clean the entire table once in a while + this.removeOld = function() { + for (var ipAddress in table) { + this.cleanEntry(ipAddress); + } + }; + +}; + +// Init the records tables +var runsTable = new RecordTable(config.maxAnonymousRunsPerDay); +var callsTable = new RecordTable(config.maxAnonymousCallsPerDay); + +module.exports = apiLimitsMiddleware; \ No newline at end of file diff --git a/lib/server/middlewares/authMiddleware.js b/lib/server/middlewares/authMiddleware.js new file mode 100644 index 0000000..e5335e6 --- /dev/null +++ b/lib/server/middlewares/authMiddleware.js @@ -0,0 +1,42 @@ +var config = require('../../../server_config/settings.json'); + +var debug = require('debug')('authMiddleware'); + + +var authMiddleware = function(req, res, next) { + 'use strict'; + + if (req.path.indexOf('/api/') === 0) { + + + if (req.headers && req.headers['x-api-key']) { + + // Test if it's an authorized key + if (isApiKeyValid(req.headers['x-api-key'])) { + + // Come in! + debug('Authorized key: %s', req.headers['x-api-key']); + res.locals.hasApiKey = true; + + } else { + + // Sorry :/ + debug('Unauthorized key %s', req.headers['x-api-key']); + res.status(401).send('Unauthorized'); + return; + } + } else { + debug('No authorization key'); + // It's ok for the moment but you might be blocked by the apiLimitsMiddleware, dude + } + } + + next(); +}; + + +function isApiKeyValid(apiKey) { + return (config.authorizedKeys[apiKey]) ? true : false; +} + +module.exports = authMiddleware; \ No newline at end of file diff --git a/package.json b/package.json index 0e8fe6c..1c0eb59 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,6 @@ "compression": "~1.2.1", "debug": "^2.1.0", "express": "~4.10.4", - "jwt-simple": "^0.2.0", "phantomas": "1.7.0", "rimraf": "^2.2.8", "socket.io": "~1.2.0" diff --git a/server_config/settings.json b/server_config/settings.json index 70f03bf..2b05df1 100644 --- a/server_config/settings.json +++ b/server_config/settings.json @@ -5,8 +5,6 @@ "authorizedKeys": { "1234567890": "contact@gaelmetais.com" }, - "tokenSalt": "lake-city", - "authorizedApplications": [ - "frontend" - ] + "maxAnonymousRunsPerDay": 24, + "maxAnonymousCallsPerDay": 1000 } \ No newline at end of file diff --git a/test/api/apiTest.js b/test/api/apiTest.js index dacc553..3183bf2 100644 --- a/test/api/apiTest.js +++ b/test/api/apiTest.js @@ -1,13 +1,11 @@ var should = require('chai').should(); var request = require('request'); -var jwt = require('jwt-simple'); +var Q = require('q'); var config = { "authorizedKeys": { "1234567890": "contact@gaelmetais.com" - }, - "tokenSalt": "lake-city", - "authorizedApplications": ["frontend"] + } }; var apiUrl = 'http://localhost:8387/api'; @@ -16,26 +14,7 @@ var wwwUrl = 'http://localhost:8388'; describe('api', function() { var runId; - - it('should not accept a query if there is no key in headers', function(done) { - this.timeout(5000); - request({ - method: 'POST', - url: apiUrl + '/runs', - body: { - url: wwwUrl + '/simple-page.html', - waitForResponse: false - }, - json: true - }, function(error, response, body) { - if (!error && response.statusCode === 401) { - done(); - } else { - done(error || response.statusCode); - } - }); - }); it('should refuse a query with an invalid key', function(done) { this.timeout(5000); @@ -87,85 +66,81 @@ describe('api', function() { }); }); - it('should refuse an expired token', function(done) { - this.timeout(5000); + it('should accept up to 24 anonymous runs to the API', function(done) { + this.timeout(15000); - request({ - method: 'POST', - url: apiUrl + '/runs', - body: { - url: wwwUrl + '/simple-page.html', - waitForResponse: false - }, - json: true, - headers: { - 'X-Api-Token': jwt.encode({ - application: config.authorizedApplications[0], - expire: Date.now() - 60000 - }, config.tokenSalt) - } - }, function(error, response, body) { - if (!error && response.statusCode === 401) { - done(); - } else { - done(error || response.statusCode); - } - }); - }); + function launchRun() { + var deferred = Q.defer(); - it('should refuse a token from an unknown app', function(done) { - this.timeout(5000); + request({ + method: 'POST', + url: apiUrl + '/runs', + body: { + url: wwwUrl + '/simple-page.html', + waitForResponse: false + }, + json: true + }, function(error, response, body) { + if (error) { + deferred.reject(error); + } else { + deferred.resolve(response, body); + } + }); - request({ - method: 'POST', - url: apiUrl + '/runs', - body: { - url: wwwUrl + '/simple-page.html', - waitForResponse: false - }, - json: true, - headers: { - 'X-Api-Token': jwt.encode({ - application: 'unknown-app', - expire: Date.now() + 60000 - }, config.tokenSalt) - } - }, function(error, response, body) { - if (!error && response.statusCode === 401) { - done(); - } else { - done(error || response.statusCode); - } - }); - }); + return deferred.promise; + } - it('should accept a good token', function(done) { - this.timeout(5000); + launchRun() + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) - request({ - method: 'POST', - url: apiUrl + '/runs', - body: { - url: wwwUrl + '/simple-page.html', - waitForResponse: false - }, - json: true, - headers: { - 'X-Api-Token': jwt.encode({ - application: config.authorizedApplications[0], - expire: Date.now() + 60000 - }, config.tokenSalt) - } - }, function(error, response, body) { - if (!error && response.statusCode === 200) { + .then(function(response, body) { + + // Here should still be ok + response.statusCode.should.equal(200); - runId = body.runId; - runId.should.be.a('string'); + launchRun() + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + .then(launchRun) + + .then(function(response, body) { + + // It should fail now + response.statusCode.should.equal(429); done(); - } else { - done(error || response.statusCode); - } + }) + .fail(function(error) { + done(error); + }); + + }).fail(function(error) { + done(error); }); + }); + + }); From 3ad6cdef51b074e1a465483acb40f0a54b0d044d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Sat, 13 Dec 2014 12:11:33 +0100 Subject: [PATCH 2/3] Launch the test server with coverage --- Gruntfile.js | 42 +++++++++++++++++++++++++++++++++++-- bin/cli.js | 2 -- bin/server.js | 4 +--- server_config/settings.json | 2 +- test/api/apiTest.js | 18 +--------------- test/fixtures/settings.json | 10 +++++++++ 6 files changed, 53 insertions(+), 25 deletions(-) create mode 100644 test/fixtures/settings.json diff --git a/Gruntfile.js b/Gruntfile.js index b9e7296..9cf19b1 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -58,7 +58,8 @@ module.exports = function(grunt) { coverage: { files: [ {src: ['test/**'], dest: 'coverage/'}, - {src: ['lib/metadata/**'], dest: 'coverage/'} + {src: ['lib/metadata/**'], dest: 'coverage/'}, + {src: ['bin/**'], dest: 'coverage/'} ] } }, @@ -70,6 +71,10 @@ module.exports = function(grunt) { coverageLib: { src: ['lib/'], dest: 'coverage/lib/' + }, + coverageBin: { + src: ['bin/'], + dest: 'coverage/bin/' } }, mochaTest: { @@ -106,7 +111,7 @@ module.exports = function(grunt) { testServer: { options: { port: 8387, - server: './bin/server.js' + server: './coverage/bin/server.js' } }, testSuite: { @@ -118,6 +123,37 @@ module.exports = function(grunt) { } }); + + // Custom task: copies the test settings.json file to the coverage folder, and checks if there's no missing fields + grunt.registerTask('copy-test-server-settings', function() { + var mainSettingsFile = './server_config/settings.json'; + var testSettingsFile = './test/fixtures/settings.json'; + + var mainSettings = grunt.file.readJSON(mainSettingsFile); + var testSettings = grunt.file.readJSON(testSettingsFile); + + // Recursively compare keys of two objects (not the values) + function compareKeys(original, copy, context) { + for (var key in original) { + if (!copy[key] && copy[key] !== '' && copy[key] !== 0) { + grunt.fail.warn('Settings file ' + testSettingsFile + ' doesn\'t contain key ' + context + '.' + key); + } + if (original[key] !== null && typeof original[key] === 'object') { + compareKeys(original[key], copy[key], context + '.' + key); + } + } + } + + compareKeys(mainSettings, testSettings, 'settings'); + + var outputFile = './coverage/server_config/settings.json'; + grunt.file.write(outputFile, JSON.stringify(testSettings, null, 4)); + grunt.verbose.ok('File ' + outputFile + ' created'); + }); + + + + require('matchdep').filterDev('grunt-*').forEach(grunt.loadNpmTasks); grunt.registerTask('icons', [ @@ -141,6 +177,7 @@ module.exports = function(grunt) { grunt.registerTask('test', [ 'build', 'jshint', + 'copy-test-server-settings', 'express:testServer', 'express:testSuite', 'clean:coverage', @@ -153,6 +190,7 @@ module.exports = function(grunt) { grunt.registerTask('test-current-work', [ 'build', 'jshint', + 'copy-test-server-settings', 'express:testServer', 'express:testSuite', 'clean:coverage', diff --git a/bin/cli.js b/bin/cli.js index 88b615a..0524668 100644 --- a/bin/cli.js +++ b/bin/cli.js @@ -1,5 +1,3 @@ -#!/usr/bin/env node - var debug = require('debug')('ylt:cli'); var YellowLabTools = require('../lib/yellowlabtools'); diff --git a/bin/server.js b/bin/server.js index f8b76aa..95bb8a5 100644 --- a/bin/server.js +++ b/bin/server.js @@ -1,6 +1,3 @@ -// Config file -var settings = require('../server_config/settings.json'); - var express = require('express'); var app = express(); var server = require('http').createServer(app); @@ -25,6 +22,7 @@ var uiController = require('../lib/server/controllers/uiController')( // Let's start the server! if (!process.env.GRUNTED) { // The server is not launched by Grunt + var settings = require('../server_config/settings.json'); server.listen(settings.serverPort, function() { console.log('Listening on port %d', server.address().port); }); diff --git a/server_config/settings.json b/server_config/settings.json index 2b05df1..5c69980 100644 --- a/server_config/settings.json +++ b/server_config/settings.json @@ -5,6 +5,6 @@ "authorizedKeys": { "1234567890": "contact@gaelmetais.com" }, - "maxAnonymousRunsPerDay": 24, + "maxAnonymousRunsPerDay": 50, "maxAnonymousCallsPerDay": 1000 } \ No newline at end of file diff --git a/test/api/apiTest.js b/test/api/apiTest.js index 3183bf2..e23f3d5 100644 --- a/test/api/apiTest.js +++ b/test/api/apiTest.js @@ -66,7 +66,7 @@ describe('api', function() { }); }); - it('should accept up to 24 anonymous runs to the API', function(done) { + it('should accept up to 10 anonymous runs to the API', function(done) { this.timeout(15000); function launchRun() { @@ -96,13 +96,6 @@ describe('api', function() { .then(launchRun) .then(launchRun) .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) .then(function(response, body) { @@ -115,15 +108,6 @@ describe('api', function() { .then(launchRun) .then(launchRun) .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) - .then(launchRun) .then(function(response, body) { diff --git a/test/fixtures/settings.json b/test/fixtures/settings.json new file mode 100644 index 0000000..619190f --- /dev/null +++ b/test/fixtures/settings.json @@ -0,0 +1,10 @@ +{ + "serverPort": "auto", + "googleAnalyticsId": "", + + "authorizedKeys": { + "1234567890": "contact@gaelmetais.com" + }, + "maxAnonymousRunsPerDay": 10, + "maxAnonymousCallsPerDay": 1000 +} \ No newline at end of file From 8abfe93df74084b62be1a6ccdc53bc9b2c934894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Sat, 13 Dec 2014 13:56:21 +0100 Subject: [PATCH 3/3] The test itself starts the server in a before hook --- Gruntfile.js | 14 +++----------- bin/server.js | 19 ++++++++++--------- test/api/apiTest.js | 12 ++++++++++-- test/fixtures/settings.json | 2 +- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index 9cf19b1..ac5dc6a 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -108,12 +108,6 @@ module.exports = function(grunt) { showStack: true } }, - testServer: { - options: { - port: 8387, - server: './coverage/bin/server.js' - } - }, testSuite: { options: { port: 8388, @@ -148,7 +142,7 @@ module.exports = function(grunt) { var outputFile = './coverage/server_config/settings.json'; grunt.file.write(outputFile, JSON.stringify(testSettings, null, 4)); - grunt.verbose.ok('File ' + outputFile + ' created'); + grunt.log.ok('File ' + outputFile + ' created'); }); @@ -177,10 +171,9 @@ module.exports = function(grunt) { grunt.registerTask('test', [ 'build', 'jshint', - 'copy-test-server-settings', - 'express:testServer', 'express:testSuite', 'clean:coverage', + 'copy-test-server-settings', 'blanket', 'copy:coverage', 'mochaTest:test', @@ -190,10 +183,9 @@ module.exports = function(grunt) { grunt.registerTask('test-current-work', [ 'build', 'jshint', - 'copy-test-server-settings', - 'express:testServer', 'express:testSuite', 'clean:coverage', + 'copy-test-server-settings', 'blanket', 'copy:coverage', 'mochaTest:test-current-work' diff --git a/bin/server.js b/bin/server.js index 95bb8a5..cc7199c 100644 --- a/bin/server.js +++ b/bin/server.js @@ -20,13 +20,14 @@ var uiController = require('../lib/server/controllers/uiController')( // Let's start the server! -if (!process.env.GRUNTED) { - // The server is not launched by Grunt - var settings = require('../server_config/settings.json'); - server.listen(settings.serverPort, function() { - console.log('Listening on port %d', server.address().port); - }); -} +var settings = require('../server_config/settings.json'); +server.listen(settings.serverPort, function() { + console.log('Listening on port %d', server.address().port); -// For Grunt -module.exports = app; \ No newline at end of file + // For the tests + if (server.startTests) { + server.startTests(); + } +}); + +module.exports = server; \ No newline at end of file diff --git a/test/api/apiTest.js b/test/api/apiTest.js index e23f3d5..da46653 100644 --- a/test/api/apiTest.js +++ b/test/api/apiTest.js @@ -14,7 +14,12 @@ var wwwUrl = 'http://localhost:8388'; describe('api', function() { var runId; + var apiServer; + before(function(done) { + apiServer = require('../../bin/server.js'); + apiServer.startTests = done; + }); it('should refuse a query with an invalid key', function(done) { this.timeout(5000); @@ -67,7 +72,7 @@ describe('api', function() { }); it('should accept up to 10 anonymous runs to the API', function(done) { - this.timeout(15000); + this.timeout(5000); function launchRun() { var deferred = Q.defer(); @@ -126,5 +131,8 @@ describe('api', function() { }); - + after(function() { + console.log('Closing the server'); + apiServer.close(); + }); }); diff --git a/test/fixtures/settings.json b/test/fixtures/settings.json index 619190f..75b2365 100644 --- a/test/fixtures/settings.json +++ b/test/fixtures/settings.json @@ -1,5 +1,5 @@ { - "serverPort": "auto", + "serverPort": "8387", "googleAnalyticsId": "", "authorizedKeys": {