Fixed Rate Limit and DDP Rate Limit packages. Added end to end tests which test removing rules.

packages/ddp-rate-limiter/ddp-rate-limiter-server-tests.js

@@ -0,0 +1,20 @@
+Meteor.methods({
+  resetAndAddRuleToDDPRateLimiter : function(intervalTimeInMillis) {
+    DDPRateLimiter.rateLimiter.rules = {};
+      this.ruleId = DDPRateLimiter.addRule({
+        userId: null,
+        ipAddr: function() {return true},
+        type: 'method',
+        name: 'login'
+      }, 5, intervalTimeInMillis);
+     return this.ruleId;
+	},
+
+	removeRuleFromDDPRateLimiter : function(id) {
+    return DDPRateLimiter.removeRule(id);
+	},
+
+	printCurrentListOfRules : function () {
+		console.log('Current list of rules :', DDPRateLimiter.rateLimiter.rules);
+	}
+});

packages/ddp-rate-limiter/ddp-rate-limiter-tests.js

@@ -1,14 +1,58 @@
-DDPRateLimiter.config([]);
-DDPRateLimiter.addRule({
-  userId: null,
-  IPAddr: null,
-  type: 'method',
-  name: 'login'
-}, 5, 1000);
+testAsyncMulti("passwords - basic login with password", [
+  function (test, expect) {
+    var self = this;
+    // Setup the rate limiter rules
+    Meteor.call('resetAndAddRuleToDDPRateLimiter', 1000, expect(function(error, result) {
+        self.ruleId = result;
+    }));
+    // setup
+    this.username = Random.id();
+    this.email = Random.id() + '-intercept@example.com';
+    this.password = 'password';
 
-if (Meteor.isClient) {
-  testAsyncMulti("passwords - basic login with password", [
-    function (test, expect) {
+    Accounts.createUser({
+        username: this.username,
+        email: this.email,
+        password: this.password
+      },
+      expect(function () {}));
+  },
+  function (test, expect) {
+    test.notEqual(Meteor.userId(), null);
+  },
+  function (test, expect) {
+    Meteor.logout(expect(function (error) {
+      test.equal(error, undefined);
+      test.equal(Meteor.user(), null);
+    }));
+  },
+  function (test, expect) {
+    var self = this;
+    for (var i = 0; i < 5; i++) {
+      Meteor.loginWithPassword(self.username, 'fakePassword', expect(
+        function (error) {
+          // Get 5 'User not found' 403 messages before rate limit is hit
+          test.equal(error.error, 403);
+        }));
+    }
+    Meteor.loginWithPassword(self.username, 'fakePassword', expect(
+      function (error) {
+        test.equal(error.error, 'too-many-requests');
+      }));
+    // Cleanup
+    Meteor.call('removeRuleFromDDPRateLimiter', self.ruleId, expect(function(error, result) {
+        test.equal(result,true);
+      }));
+  }
+]);
+
+testAsyncMulti("test removing rule with rateLimited client lets them send new queries", [
+    function(test, expect) {
+      var self = this;
+      // Setup the rate limiter rules
+      Meteor.call('resetAndAddRuleToDDPRateLimiter', 5000, expect(function(error, result) {
+        self.ruleId = result;
+      }));
       // setup
       this.username = Random.id();
       this.email = Random.id() + '-intercept@example.com';
@@ -35,7 +79,8 @@ if (Meteor.isClient) {
       for (var i = 0; i < 5; i++) {
         Meteor.loginWithPassword(self.username, 'fakePassword', expect(
           function (error) {
-            // Get 5 'User not found' 403 messages before rate limit is hit
+            // Call printCurrentListofRules to see all the rules on the server
+            // Meteor.call('printCurrentListOfRules');
             test.equal(error.error, 403);
           }));
       }
@@ -43,6 +88,16 @@ if (Meteor.isClient) {
         function (error) {
           test.equal(error.error, 'too-many-requests');
         }));
-    }
+      // By removing the rule from the DDP rate limiter, we no longer restrict them even though they were rate limited
+      Meteor.call('removeRuleFromDDPRateLimiter', self.ruleId, expect(function(error, result) {
+        test.equal(result,true);
+      }));
+      //
+      for (var i = 0; i < 10; i++) {
+        Meteor.loginWithPassword(self.username, 'fakePassword', expect(
+          function (error) {
+            test.equal(error.error, 403);
+        }));
+      }
+  }
   ]);
-};

packages/ddp-rate-limiter/ddp-rate-limiter.js

@@ -3,20 +3,10 @@ DDPRateLimiter = {
   errorMessage : function (rateLimitResult) {
     return "Error, too many requests. Please slow down. You must wait " + Math.ceil(
     rateLimitResult.timeToReset / 1000) + " seconds before trying again.";
-  }
+  },
+  rateLimiter : new RateLimiter()
 }
 
-DDPRateLimiter.rateLimiter = new RateLimiter();
-
-// DDPRateLimiter.rateLimiter.addRule( {
-//   userId: null,
-//   ipAddr: function (ipAddr) {
-//     return true;
-//   },
-//   type: 'sub',
-//   name: null
-// }, 5, 10000);
-
 DDPRateLimiter.getErrorMessage = function (rateLimitResult) {
   if (typeof this.errorMessage === 'function')
     return this.errorMessage(rateLimitResult);
@@ -28,22 +18,10 @@ DDPRateLimiter.setErrorMessage = function (message) {
   this.errorMessage = message;
 }
 
-DDPRateLimiter.setRules = function (rules) {
-  DDPRateLimiter.rateLimiter.rules = rules;
-};
-
 DDPRateLimiter.addRule = function (rule, numRequests, intervalTime) {
-  DDPRateLimiter.rateLimiter.addRule(rule, numRequests, intervalTime);
+  return DDPRateLimiter.rateLimiter.addRule(rule, numRequests, intervalTime);
 };
 
-
-// Add a default rule of limiting logins to 5 times per 10 seconds by IP address.
-// Override using DDPRateLimiter.config
-DDPRateLimiter.addRule({
-  userId: null,
-  ipAddr: function (ipAddr) {
-    return true;
-  },
-  type: 'method',
-  name: 'login'
-}, 5, 10000);
+DDPRateLimiter.removeRule = function (id) {
+  return DDPRateLimiter.rateLimiter.removeRule(id);
+}

packages/ddp-rate-limiter/package.js

@@ -18,9 +18,11 @@ Package.onUse(function(api) {
 });
 
 Package.onTest(function(api) {
+  api.use('underscore');
   api.use(['accounts-password', 'tinytest', 'test-helpers', 'tracker',
            'accounts-base', 'random', 'email', 'underscore', 'check',
            'ddp']);
   api.use('ddp-rate-limiter');
-  api.addFiles('ddp-rate-limiter-tests.js');
+  api.addFiles('ddp-rate-limiter-server-tests.js', 'server');
+  api.addFiles('ddp-rate-limiter-tests.js', 'client');
 });

packages/rate-limit/package.js

@@ -12,6 +12,7 @@ Package.describe({
 
 Package.onUse(function(api) {
   api.use('underscore');
+  api.use('random');
   api.addFiles('rate-limit.js');
   api.export("RateLimiter");
 });
@@ -19,6 +20,7 @@ Package.onUse(function(api) {
 Package.onTest(function(api) {
   api.use('test-helpers', ['client', 'server']);
   api.use('underscore');
+  api.use('random');
   api.use('ddp-rate-limiter');
   api.use('tinytest');
   api.use('rate-limit');

packages/rate-limit/rate-limit-tests.js

@@ -1,6 +1,6 @@
 Tinytest.add('Check empty constructor creation', function (test) {
   r = new RateLimiter();
-  test.equal(r.rules, []);
+  test.equal(r.rules, {});
 });
 
 Tinytest.add(
@@ -220,7 +220,7 @@ Tinytest.add("test matchRule method", function (test) {
     type: null,
     name: null
   }
-  r.addRule(globalRule);
+  var globalRuleId = r.addRule(globalRule);
 
   var RateLimiterInput = {
     userId: 1023,
@@ -229,7 +229,7 @@ Tinytest.add("test matchRule method", function (test) {
     name: 'getSubLists'
   };
 
-  test.equal(r.rules[0].match(RateLimiterInput), true);
+  test.equal(r.rules[globalRuleId].match(RateLimiterInput), true);
 
   var oneNotNullRule = {
     userId: 102,
@@ -238,18 +238,18 @@ Tinytest.add("test matchRule method", function (test) {
     name: null
   }
 
-  r.addRule(oneNotNullRule);
-  test.equal(r.rules[1].match(RateLimiterInput), false);
+  var oneNotId = r.addRule(oneNotNullRule);
+  test.equal(r.rules[oneNotId].match(RateLimiterInput), false);
 
   oneNotNullRule.userId = 1023;
-  test.equal(r.rules[1].match(RateLimiterInput), true);
+  test.equal(r.rules[oneNotId].match(RateLimiterInput), true);
 
   var notCompleteInput = {
     userId: 102,
     IPAddr: '127.0.0.1'
   };
-  test.equal(r.rules[0].match(notCompleteInput), true);
-  test.equal(r.rules[1].match(notCompleteInput), false);
+  test.equal(r.rules[globalRuleId].match(notCompleteInput), true);
+  test.equal(r.rules[oneNotId].match(notCompleteInput), false);
 });
 
 Tinytest.add('test generateMethodKey string', function (test) {
@@ -260,7 +260,7 @@ Tinytest.add('test generateMethodKey string', function (test) {
     type: null,
     name: null
   }
-  r.addRule(globalRule);
+  var globalRuleId = r.addRule(globalRule);
 
   var RateLimiterInput = {
     userId: 1023,
@@ -268,11 +268,11 @@ Tinytest.add('test generateMethodKey string', function (test) {
     type: 'sub',
     name: 'getSubLists'
   };
-  // test.equal(r._generateKeyString(globalRule, RateLimiterInput), "");
-  test.equal(r.rules[0]._generateKeyString(RateLimiterInput), "");
+
+  test.equal(r.rules[globalRuleId]._generateKeyString(RateLimiterInput), "");
   globalRule.userId = 1023;
 
-  test.equal(r.rules[0]._generateKeyString(RateLimiterInput),
+  test.equal(r.rules[globalRuleId]._generateKeyString(RateLimiterInput),
     "userId1023");
 
   var ruleWithFuncs = {
@@ -282,16 +282,16 @@ Tinytest.add('test generateMethodKey string', function (test) {
     IPAddr: null,
     type: null
   };
-  r.addRule(ruleWithFuncs);
-  test.equal(r.rules[1]._generateKeyString(RateLimiterInput), "");
+  var funcRuleId = r.addRule(ruleWithFuncs);
+  test.equal(r.rules[funcRuleId]._generateKeyString(RateLimiterInput), "");
   RateLimiterInput.userId = 1024;
-  test.equal(r.rules[1]._generateKeyString(RateLimiterInput),
+  test.equal(r.rules[funcRuleId]._generateKeyString(RateLimiterInput),
     "userId1024");
 
   var multipleRules = ruleWithFuncs;
   multipleRules.IPAddr = '127.0.0.1';
-  r.addRule(multipleRules);
-  test.equal(r.rules[2]._generateKeyString(RateLimiterInput),
+  var multipleRuleId = r.addRule(multipleRules);
+  test.equal(r.rules[multipleRuleId]._generateKeyString(RateLimiterInput),
     "userId1024IPAddr127.0.0.1")
 })
 

packages/rate-limit/rate-limit.js

@@ -6,6 +6,7 @@ var DEFAULT_REQUESTS_PER_INTERVAL = 10;
 var Rule = function (options, matchers) {
   var self = this;
 
+  self.id = Random.id();
   // Options contains the timeToReset and intervalTime
   self.options = options;
 
@@ -98,10 +99,10 @@ _.extend(Rule.prototype, {
 RateLimiter = function () {
   var self = this;
 
-  // List of all rules associated with this RateLimiter. Each rule object stores
-  // the rule pattern, number of requests allowed, last reset time and the rule
-  // reset interval in milliseconds.
-  self.rules = [];
+  // Dictionary of all rules associated with this RateLimiter, keyed by their
+  // id. Each rule object stores the rule pattern, number of requests allowed,
+  // last reset time and the rule reset interval in milliseconds.
+  self.rules = {};
 }
 
 /**
@@ -175,7 +176,8 @@ RateLimiter.prototype.addRule = function (rule, numRequestsAllowed, intervalTime
   }
 
   var newRule = new Rule(options, rule);
-  this.rules.push(newRule);
+  this.rules[newRule.id] = newRule;
+  return newRule.id;
 }
 
 /**
@@ -214,4 +216,14 @@ RateLimiter.prototype._findAllMatchingRules = function (input) {
       matchingRules.push(rule);
   });
   return matchingRules;
+}
+
+RateLimiter.prototype.removeRule = function (id) {
+  var self = this;
+  if (self.rules[id]) {
+    delete self.rules[id];
+    return true;
+  } else {
+    return false;
+  }
 }