Refactored rate-limit package.

Changed rate-limit package to take generic rules and match generic inputs to those rules.
Now, users can use the rate-limit package for whatever they would like. Tests still need to
be updated and need to change location of ddp-rate-limiter to include subscriptions.
Need to remove duplicate code in rate-limit from previous implementation which hardcoded DDP types
into the rate limiting package by making the input be a DDPCommon.MethodInvocation object.

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 );

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(

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,

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
+  };
+
 }