refactor: change limiter.Try() to .removeToken()
This commit is contained in:
parent
7928dc2bff
commit
a3f18d6158
@ -21,12 +21,9 @@ export class RateLimiter {
|
||||
return this.minuteLimiter.getTokensRemaining() > 0 || this.hourLimiter.getTokensRemaining() > 0
|
||||
}
|
||||
|
||||
public try(): boolean {
|
||||
if (this.canTry()) {
|
||||
public removeToken(): boolean {
|
||||
return this.minuteLimiter.tryRemoveTokens(1) || this.hourLimiter.tryRemoveTokens(1)
|
||||
}
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
const getRoot = async (req: Request, error?: Error): Promise<string> => {
|
||||
@ -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!")
|
||||
}
|
||||
|
||||
|
@ -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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
Reference in New Issue
Block a user