From a3f18d61582e84336b7a8543d022ffe3e5ee6588 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Mon, 19 Apr 2021 10:57:50 -0700 Subject: [PATCH] refactor: change limiter.Try() to .removeToken() --- src/node/routes/login.ts | 9 +++------ test/unit/routes/login.test.ts | 13 +++++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts index 809c31f07..f02c9c08d 100644 --- a/src/node/routes/login.ts +++ b/src/node/routes/login.ts @@ -21,11 +21,8 @@ export class RateLimiter { return this.minuteLimiter.getTokensRemaining() > 0 || this.hourLimiter.getTokensRemaining() > 0 } - public try(): boolean { - if (this.canTry()) { - return this.minuteLimiter.tryRemoveTokens(1) || this.hourLimiter.tryRemoveTokens(1) - } - return false + public removeToken(): boolean { + return this.minuteLimiter.tryRemoveTokens(1) || this.hourLimiter.tryRemoveTokens(1) } } @@ -91,7 +88,7 @@ router.post("/", async (req, res) => { // Note: successful logins should not count against the RateLimiter // which is why this logic must come after the successful login logic - if (!limiter.try()) { + if (!limiter.removeToken()) { throw new Error("Login rate limited!") } diff --git a/test/unit/routes/login.test.ts b/test/unit/routes/login.test.ts index fde02b2c2..2a2e20466 100644 --- a/test/unit/routes/login.test.ts +++ b/test/unit/routes/login.test.ts @@ -4,20 +4,20 @@ describe("login", () => { describe("RateLimiter", () => { it("should allow one try ", () => { const limiter = new RateLimiter() - expect(limiter.try()).toBe(true) + expect(limiter.removeToken()).toBe(true) }) it("should pull tokens from both limiters (minute & hour)", () => { const limiter = new RateLimiter() // Try twice, which pulls two from the minute bucket - limiter.try() - limiter.try() + limiter.removeToken() + limiter.removeToken() // Check that we can still try // which should be true since there are 12 remaining in the hour bucket expect(limiter.canTry()).toBe(true) - expect(limiter.try()).toBe(true) + expect(limiter.removeToken()).toBe(true) }) it("should not allow more than 14 tries in less than an hour", () => { @@ -27,10 +27,11 @@ describe("login", () => { // so if we run it 15 times, 14 should return true and the last // should return false for (let i = 1; i <= 14; i++) { - expect(limiter.try()).toBe(true) + expect(limiter.removeToken()).toBe(true) } - expect(limiter.try()).toBe(false) + expect(limiter.canTry()).toBe(false) + expect(limiter.removeToken()).toBe(false) }) }) })