diff --git a/packages/ddp-rate-limiter/ddp-rate-limiter.js b/packages/ddp-rate-limiter/ddp-rate-limiter.js index a333785add..7487fb7d9a 100644 --- a/packages/ddp-rate-limiter/ddp-rate-limiter.js +++ b/packages/ddp-rate-limiter/ddp-rate-limiter.js @@ -8,7 +8,7 @@ DDPRateLimiter.RateLimiter = new RateLimiter(); DDPRateLimiter.RateLimiter.addRule( { userId: null, IPAddr: function( IPAddr ) { - return true + return true; }, method: 'login' }, 5, 10000 ); diff --git a/packages/ddp-server/livedata_server.js b/packages/ddp-server/livedata_server.js index 73dc855530..08596fab3a 100644 --- a/packages/ddp-server/livedata_server.js +++ b/packages/ddp-server/livedata_server.js @@ -492,7 +492,7 @@ _.extend(Session.prototype, { var self = this; if (!self.inQueue) // we have been destroyed. return; - + // console.log(msg_in); // Respond to ping and pong messages immediately without queuing. // If the negotiated DDP version is "pre1" which didn't support // pings, preserve the "pre1" behavior of responding with a "bad @@ -543,8 +543,20 @@ _.extend(Session.prototype, { processNext(); }; - if (_.has(self.protocol_handlers, msg.msg)) - self.protocol_handlers[msg.msg].call(self, msg, unblock); + if (_.has(self.protocol_handlers, msg.msg)) { + // Is it bad to do these checks here? Instead of inside session.protocol_handlers and then in method + // var rateLimiterInput = { + // userId: self.userId, + // IPAddr: self.connectionHandle.clientAddress, + // type: msg.msg, + // name: msg.name } + // DDPRateLimiter.RateLimiter.increment(rateLimiterInput); + // var rateLimitResult = DDPRateLimiter.RateLimiter.check(rateLimiterInput) + // if (!rateLimitResult.valid) + // self.sendError('too-many-requests', DDPRateLimiter.getErrorMessage(rateLimitResult)); + // else + self.protocol_handlers[msg.msg].call(self, msg, unblock); + } else self.sendError('Bad request', msg); unblock(); // in case the handler didn't already do it @@ -580,6 +592,8 @@ _.extend(Session.prototype, { return; var handler = self.server.publish_handlers[msg.name]; + + // console.log(msg); self._startSubscription(handler, msg.id, msg.params, msg.name); }, @@ -643,10 +657,14 @@ _.extend(Session.prototype, { connection: self.connectionHandle, randomSeed: randomSeed }); - invocation.method = msg.method; + // invocation.method = msg.method; + var rateLimiterInput = { + userId: self.userId, + IPAddr: self.connectionHandle.clientAddress, + method: msg.method}; try { - DDPRateLimiter.RateLimiter.increment(invocation); - var rateLimitResult = DDPRateLimiter.RateLimiter.check(invocation) + DDPRateLimiter.RateLimiter.newIncrement(rateLimiterInput); + var rateLimitResult = DDPRateLimiter.RateLimiter.newCheck(rateLimiterInput) if (!rateLimitResult.valid) { throw new Meteor.Error(429, DDPRateLimiter.getErrorMessage(rateLimitResult)); } @@ -766,7 +784,6 @@ _.extend(Session.prototype, { _startSubscription: function (handler, subId, params, name) { var self = this; - var sub = new Subscription( self, handler, subId, params, name); if (subId) @@ -1535,6 +1552,7 @@ _.extend(Server.prototype, { connection: connection, randomSeed: DDPCommon.makeRpcSeed(currentInvocation, name) }); + try { var result = DDP._CurrentInvocation.withValue(invocation, function () { return maybeAuditArgumentChecks( diff --git a/packages/rate-limit/rate-limit-tests.js b/packages/rate-limit/rate-limit-tests.js index 3c6c97678d..d07b7bcfe0 100644 --- a/packages/rate-limit/rate-limit-tests.js +++ b/packages/rate-limit/rate-limit-tests.js @@ -8,7 +8,7 @@ Tinytest.add( 'Check empty constructor creation', function( test ) { r = new RateLimiter(); test.equal( r.rules, [] ); test.equal( r.ruleId, 0 ); - test.equal( r.ruleInvocationCounters, {} ); + test.equal( r.ruleCounters, {} ); } ); Tinytest.add( @@ -170,6 +170,77 @@ Tinytest.add( "add global rule", function( test ) { test.equal( r.check( methodInvc3 ).valid, false ); } ); +Tinytest.add("test matchRule method", function (test) { + r = new RateLimiter(); + var globalRule = { + userId: null, + IPAddr: null, + type: null, + name: null + } + + var RateLimiterInput = { + userId: 1023, + IPAddr: "127.0.0.1", + type: 'sub', + name: 'getSubLists' + }; + + test.equal(r._matchRule(globalRule, RateLimiterInput), true); + + var oneNotNullRule = { + userId: 102, + IPAddr: null, + type: null, + name: null + } + + test.equal(r._matchRule(oneNotNullRule, RateLimiterInput), false); + + oneNotNullRule.userId = 1023; + test.equal(r._matchRule(oneNotNullRule, RateLimiterInput), true); + + var notCompleteInput = { userId: 102, IPAddr: '127.0.0.1'}; + test.equal(r._matchRule(globalRule, notCompleteInput), true); + test.equal(r._matchRule(oneNotNullRule, notCompleteInput), false); +}); + +Tinytest.add('test generateMethodKey string', function(test) { + r = new RateLimiter(); + var globalRule = { + userId: null, + IPAddr: null, + type: null, + name: null + } + + var RateLimiterInput = { + userId: 1023, + IPAddr: "127.0.0.1", + type: 'sub', + name: 'getSubLists' + }; + + test.equal(r._generateKeyString(globalRule, RateLimiterInput), ""); + + globalRule.userId = 1023; + test.equal(r._generateKeyString(globalRule, RateLimiterInput), "userId1023"); + + var ruleWithFuncs = { + userId: function(input) { return input % 2 === 0}, + IPAddr: null, + type: null + }; + + test.equal(r._generateKeyString(ruleWithFuncs, RateLimiterInput), ""); + RateLimiterInput.userId = 1024; + test.equal(r._generateKeyString(ruleWithFuncs, RateLimiterInput), "userId1024"); + + var multipleRules = ruleWithFuncs; + multipleRules.IPAddr = '127.0.0.1'; + test.equal(r._generateKeyString(multipleRules, RateLimiterInput), "userId1024IPAddr127.0.0.1") +}) + function createTempConnectionHandle( id, clientIP ) { return { id: id, diff --git a/packages/rate-limit/rate-limit.js b/packages/rate-limit/rate-limit.js index bd65f86af8..45fc64a2af 100644 --- a/packages/rate-limit/rate-limit.js +++ b/packages/rate-limit/rate-limit.js @@ -14,7 +14,7 @@ RateLimiter = function() { var self = this; self.rules = []; self.ruleId = 0; - self.ruleInvocationCounters = {}; + self.ruleCounters = {}; } /** @@ -37,7 +37,7 @@ RateLimiter.prototype.check = function( methodInvocation ) { if ( RateLimiter.prototype.matchRuleUsingFind( rule, methodInvocation ) ) { var matchRuleHelper = self._matchRuleHelper( rule, methodInvocation ); - var numInvocations = self.ruleInvocationCounters[ rule.ruleId ] + var numInvocations = self.ruleCounters[ rule.ruleId ] [ matchRuleHelper.methodString ]; if ( numInvocations > rule.numRequestsAllowed && @@ -65,10 +65,39 @@ RateLimiter.prototype.check = function( methodInvocation ) { return reply; } +RateLimiter.prototype.newCheck = function ( input ) { + var self = this; + var reply = { + valid: true, + timeToReset: 0, + numInvocationsLeft: Infinity + }; + + _.each( self.rules, function( rule ) { + if ( self._matchRule( rule, input )) { + var matchRuleHelper = self._newRuleHelper( rule, input ); + var numInvocations = self.ruleCounters[ rule.ruleId ][matchRuleHelper.methodString]; + if (numInvocations > rule.numRequestsAllowed && matchRuleHelper.timeSinceLastReset < rule.intervalTime) { + if ( reply.timeToReset < matchRuleHelper.timeToNextReset ) { + reply.timeToReset = matchRuleHelper.timeToNextReset; + }; + reply.valid = false; + reply.numInvocationsLeft = 0; + } else { + if (rule.numRequestsAllowed - numInvocations < reply.numInvocationsLeft && reply.valid) { + reply.valid = true; + reply.timeToReset = matchRuleHelper.timeToNextReset < 0 ? rule.intervalTime : matchRuleHelper.timeToNextReset; + reply.numInvocationsLeft = rule.numRequestsAllowed - numInvocations; + } + } + } + }); + return reply; +} + /** - * Adds a rule to with a specified identifier query to list of rules that are - * checked against on every method invocation - * @param {object} identifierQuery Specified domain for rate limit rule + * Appends a rule to list of rules that are checked against on every method invocation + * @param {object} rule Specified domain for rate limit rule * { userId: ID | function() | null, * IPAddr: ID | function() | null, * method: name | function() | null @@ -76,15 +105,15 @@ RateLimiter.prototype.check = function( methodInvocation ) { * @param {integer} numRequestsAllowed Number of requests allowed per interval * @param {integer} intervalTime Number of milliseconds before interval is reset */ -RateLimiter.prototype.addRule = function( identifierQuery, numRequestsAllowed, +RateLimiter.prototype.addRule = function( rule, numRequestsAllowed, intervalTime ) { - identifierQuery.ruleId = this._createNewRuleId(); - identifierQuery.numRequestsAllowed = numRequestsAllowed || + rule.ruleId = this._createNewRuleId(); + rule.numRequestsAllowed = numRequestsAllowed || DEFAULT_REQUESTS_PER_INTERVAL; - identifierQuery.intervalTime = intervalTime || + rule.intervalTime = intervalTime || DEFAULT_INTERVAL_TIME_IN_MILLISECONDS; - identifierQuery._lastResetTime = new Date().getTime(); - this.rules.push( identifierQuery ); + rule._lastResetTime = new Date().getTime(); + this.rules.push( rule ); } /** @@ -119,6 +148,33 @@ RateLimiter.prototype.matchRuleUsingFind = function( rule, methodInvocation ) { return ruleMatches; } +RateLimiter.prototype._matchRule = function ( rule, input ) { + var self = this; + var ruleMatches = true; + _.find( rule, function ( value, key) { + if (value !== null && key != 'ruleId' && key != '_lastResetTime' && key != 'numRequestsAllowed' && key != 'intervalTime') { + if (!(key in input)) { + ruleMatches = false; + return true; + } else { + if (typeof value === 'function') { + if (!(value(input[key]))) { + ruleMatches = false; + return true; + } + } else { + if (value !== input[key]) { + ruleMatches = false; + return true; + } + } + } + } + }); + return ruleMatches; +} + + /** * Increment appropriate counters on every method invocation * @param {object} methodInvocation DDPCommon.MethodInvocation object with @@ -135,29 +191,59 @@ RateLimiter.prototype.increment = function( methodInvocation ) { if ( matchRuleHelper.timeSinceLastReset > rule.intervalTime ) { rule._lastResetTime = new Date().getTime(); // Reset all the counters for this rule - _.each( self.ruleInvocationCounters[ rule.ruleId ], function( + _.each( self.ruleCounters[ rule.ruleId ], function( value, methodString ) { - self.ruleInvocationCounters[ rule.ruleId ][ methodString ] = + self.ruleCounters[ rule.ruleId ][ methodString ] = 0; } ); } - if ( rule.ruleId in self.ruleInvocationCounters ) { - if ( matchRuleHelper.methodString in self.ruleInvocationCounters[ + if ( rule.ruleId in self.ruleCounters ) { + if ( matchRuleHelper.methodString in self.ruleCounters[ rule.ruleId ] ) { - self.ruleInvocationCounters[ rule.ruleId ][ matchRuleHelper.methodString ]++; + self.ruleCounters[ rule.ruleId ][ matchRuleHelper.methodString ]++; } else { - self.ruleInvocationCounters[ rule.ruleId ][ matchRuleHelper.methodString ] = + self.ruleCounters[ rule.ruleId ][ matchRuleHelper.methodString ] = 1; } } else { - self.ruleInvocationCounters[ rule.ruleId ] = {}; - self.ruleInvocationCounters[ rule.ruleId ][ matchRuleHelper.methodString ] = + self.ruleCounters[ rule.ruleId ] = {}; + self.ruleCounters[ rule.ruleId ][ matchRuleHelper.methodString ] = 1; } } } ); } +RateLimiter.prototype.newIncrement = function ( input ) { + var self = this; + + // Only increment rule counters that match this input + _.each ( self.rules, function ( rule ) { + if (self._matchRule( rule , input ) ) { + var matchRuleHelper = self._newRuleHelper( rule, input ); + + if ( matchRuleHelper.timeSinceLastReset > rule.intervalTime) { + // Reset all the counters since the rule has reset + rule._lastResetTime = new Date().getTime(); + _.each( self.ruleCounters [ rule.ruleId ], function (value, keyString) { + self.ruleCounters[ rule.ruleId][keyString ] = 0; + }); + } + + if ( rule.ruleId in self.ruleCounters ) { + if ( matchRuleHelper.methodString in self.ruleCounters[rule.ruleId] ) { + self.ruleCounters[ rule.ruleId ] [ matchRuleHelper.methodString ]++; + } else { + self.ruleCounters[ rule.ruleId ] [ matchRuleHelper.methodString ] = 1; + } + } else { + self.ruleCounters [ rule.ruleId ] = {}; + self.ruleCounters [ rule.ruleId ] [matchRuleHelper.methodString ] = 1; + } + } + }); +} + // Creates new unique rule id RateLimiter.prototype._createNewRuleId = function() { return this.ruleId++; @@ -191,6 +277,22 @@ RateLimiter.prototype._generateMethodInvocationKeyStringFromRuleMapping = return returnString; } +RateLimiter.prototype._generateKeyString = function (rule, input) { + var self = this; + var returnString = ""; + _.each( rule, function ( value, key) { + if (value !== null) { + if (typeof value === 'function') { + if (value(input[key])) + returnString += key + input[key]; + } + else + returnString += key + input[key]; + } + }); + return returnString; +} + /** * Helper method that uses the RATE_LIMITING_DICT to create a fast way to * access values in methodInvocation without manually parsing the paths @@ -223,4 +325,17 @@ RateLimiter.prototype._matchRuleHelper = function( rule, methodInvocation ) { timeSinceLastReset: timeSinceLastReset, timeToNextReset: timeToNextReset }; +} + +RateLimiter.prototype._newRuleHelper = function (rule, input) { + var self = this; + var keyString = self._generateKeyString(rule, input); + var timeSinceLastReset = new Date().getTime() - rule._lastResetTime; + var timeToNextReset = rule.intervalTime - timeSinceLastReset; + return { + methodString: keyString, + timeSinceLastReset: timeSinceLastReset, + timeToNextReset: timeToNextReset + }; + } \ No newline at end of file