Skip to content

Commit 014b0c8

Browse files
committed
feat: Fix rate limit issue
Signed-off-by: ABHAY PANDEY <pandeyabhay967@gmail.com>
1 parent 0c2a1a8 commit 014b0c8

10 files changed

Lines changed: 177 additions & 6 deletions

resources/default-settings.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,19 @@ limits:
112112
- "::1"
113113
- "10.10.10.1"
114114
- "::ffff:10.10.10.1"
115+
admin:
116+
rateLimits:
117+
- description: 30 admin requests/min
118+
period: 60000
119+
rate: 30
120+
loginRateLimits:
121+
- description: 10 login attempts per 15 minutes
122+
period: 900000
123+
rate: 10
124+
ipWhitelist:
125+
- "::1"
126+
- "10.10.10.1"
127+
- "::ffff:10.10.10.1"
115128
connection:
116129
rateLimits:
117130
- period: 1000

src/@types/settings.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,17 @@ export interface AdmissionCheckLimits {
141141
ipWhitelist?: string[]
142142
}
143143

144+
export interface AdminLimits {
145+
rateLimits?: RateLimit[]
146+
loginRateLimits?: RateLimit[]
147+
ipWhitelist?: string[]
148+
}
149+
144150
export interface Limits {
145151
rateLimiter?: RateLimiterSettings
146152
invoice?: InvoiceLimits
147153
admissionCheck?: AdmissionCheckLimits
154+
admin?: AdminLimits
148155
connection?: ConnectionLimits
149156
client?: ClientLimits
150157
event?: EventLimits

src/controllers/admin/get-health-controller.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
11
import { Request, Response } from 'express'
22

33
import { IController } from '../../@types/controllers'
4+
import { Settings } from '../../@types/settings'
5+
import { IRateLimiter } from '../../@types/utils'
46
import { collectAdminHealthSnapshot } from '../../utils/admin-health'
7+
import { isAdminRateLimited } from '../../utils/admin-rate-limit'
58

69
export class GetAdminHealthController implements IController {
7-
public async handleRequest(_request: Request, response: Response): Promise<void> {
10+
public constructor(
11+
private readonly settings: () => Settings,
12+
private readonly rateLimiter: () => IRateLimiter,
13+
) {}
14+
15+
public async handleRequest(request: Request, response: Response): Promise<void> {
16+
const currentSettings = this.settings()
17+
18+
if (await isAdminRateLimited(request, currentSettings, this.rateLimiter, 'admin')) {
19+
response.status(429).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Too many requests' }))
20+
return
21+
}
22+
823
const health = await collectAdminHealthSnapshot()
924
response.status(200).setHeader('content-type', 'application/json').send(JSON.stringify(health))
1025
}

src/controllers/admin/get-session-controller.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,25 @@ import { Request, Response } from 'express'
22

33
import { IAdminAuthProvider } from '../../@types/admin'
44
import { IController } from '../../@types/controllers'
5+
import { Settings } from '../../@types/settings'
6+
import { IRateLimiter } from '../../@types/utils'
7+
import { isAdminRateLimited } from '../../utils/admin-rate-limit'
58

69
export class GetAdminSessionController implements IController {
7-
public constructor(private readonly authProvider: IAdminAuthProvider) {}
10+
public constructor(
11+
private readonly authProvider: IAdminAuthProvider,
12+
private readonly settings: () => Settings,
13+
private readonly rateLimiter: () => IRateLimiter,
14+
) {}
815

916
public async handleRequest(request: Request, response: Response): Promise<void> {
17+
const currentSettings = this.settings()
18+
19+
if (await isAdminRateLimited(request, currentSettings, this.rateLimiter, 'admin')) {
20+
response.status(429).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Too many requests' }))
21+
return
22+
}
23+
1024
if (!this.authProvider.isRequestAuthenticated(request)) {
1125
response.status(401).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Unauthorized' }))
1226
return

src/controllers/admin/post-login-controller.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,25 @@ import { Request, Response } from 'express'
22

33
import { IAdminAuthProvider } from '../../@types/admin'
44
import { IController } from '../../@types/controllers'
5+
import { Settings } from '../../@types/settings'
6+
import { IRateLimiter } from '../../@types/utils'
7+
import { isAdminRateLimited } from '../../utils/admin-rate-limit'
58

69
export class PostAdminLoginController implements IController {
7-
public constructor(private readonly authProvider: IAdminAuthProvider) {}
10+
public constructor(
11+
private readonly authProvider: IAdminAuthProvider,
12+
private readonly settings: () => Settings,
13+
private readonly rateLimiter: () => IRateLimiter,
14+
) {}
815

916
public async handleRequest(request: Request, response: Response): Promise<void> {
17+
const currentSettings = this.settings()
18+
19+
if (await isAdminRateLimited(request, currentSettings, this.rateLimiter, 'login')) {
20+
response.status(429).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Too many requests' }))
21+
return
22+
}
23+
1024
await this.authProvider.handleLogin(request, response)
1125
}
1226
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { GetAdminHealthController } from '../../controllers/admin/get-health-controller'
22
import { IController } from '../../@types/controllers'
3+
import { createSettings } from '../settings-factory'
4+
import { rateLimiterFactory } from '../rate-limiter-factory'
35

46
export const createGetAdminHealthController = (): IController => {
5-
return new GetAdminHealthController()
7+
return new GetAdminHealthController(createSettings, rateLimiterFactory)
68
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { GetAdminSessionController } from '../../controllers/admin/get-session-controller'
22
import { IController } from '../../@types/controllers'
33
import { createAdminAuthProvider } from '../admin-auth-provider-factory'
4+
import { createSettings } from '../settings-factory'
5+
import { rateLimiterFactory } from '../rate-limiter-factory'
46

57
export const createGetAdminSessionController = (): IController => {
6-
return new GetAdminSessionController(createAdminAuthProvider())
8+
return new GetAdminSessionController(createAdminAuthProvider(), createSettings, rateLimiterFactory)
79
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { PostAdminLoginController } from '../../controllers/admin/post-login-controller'
22
import { IController } from '../../@types/controllers'
33
import { createAdminAuthProvider } from '../admin-auth-provider-factory'
4+
import { createSettings } from '../settings-factory'
5+
import { rateLimiterFactory } from '../rate-limiter-factory'
46

57
export const createPostAdminLoginController = (): IController => {
6-
return new PostAdminLoginController(createAdminAuthProvider())
8+
return new PostAdminLoginController(createAdminAuthProvider(), createSettings, rateLimiterFactory)
79
}

src/utils/admin-rate-limit.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { path } from 'ramda'
2+
import { Request } from 'express'
3+
4+
import { Settings } from '../@types/settings'
5+
import { IRateLimiter } from '../@types/utils'
6+
import { createLogger } from '../factories/logger-factory'
7+
import { getRemoteAddress } from './http'
8+
9+
const logger = createLogger('admin-rate-limit')
10+
11+
export async function isAdminRateLimited(
12+
request: Request,
13+
settings: Settings,
14+
rateLimiterFactory: () => IRateLimiter,
15+
scope: 'login' | 'admin',
16+
): Promise<boolean> {
17+
const rateLimitsKey = scope === 'login' ? 'loginRateLimits' : 'rateLimits'
18+
const rateLimits = path(['limits', 'admin', rateLimitsKey], settings)
19+
if (!Array.isArray(rateLimits) || !rateLimits.length) {
20+
return false
21+
}
22+
23+
const ipWhitelist = path(['limits', 'admin', 'ipWhitelist'], settings)
24+
const remoteAddress = getRemoteAddress(request, settings)
25+
26+
let limited = false
27+
if (Array.isArray(ipWhitelist) && !ipWhitelist.includes(remoteAddress)) {
28+
const rateLimiter = rateLimiterFactory()
29+
for (const { rate, period } of rateLimits) {
30+
if (await rateLimiter.hit(`${remoteAddress}:admin-${scope}:${period}`, 1, { period, rate })) {
31+
logger('rate limited %s: %d in %d milliseconds', remoteAddress, rate, period)
32+
limited = true
33+
}
34+
}
35+
}
36+
37+
return limited
38+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import chai from 'chai'
2+
import sinon from 'sinon'
3+
import sinonChai from 'sinon-chai'
4+
5+
import { isAdminRateLimited } from '../../../src/utils/admin-rate-limit'
6+
7+
chai.use(sinonChai)
8+
const { expect } = chai
9+
10+
describe('isAdminRateLimited', () => {
11+
const baseSettings = {
12+
network: {
13+
remoteIpHeader: 'x-forwarded-for',
14+
},
15+
limits: {
16+
admin: {
17+
rateLimits: [{ rate: 30, period: 60000 }],
18+
loginRateLimits: [{ rate: 10, period: 900000 }],
19+
ipWhitelist: [],
20+
},
21+
},
22+
}
23+
24+
const makeRequest = (remoteAddress = '1.2.3.4') =>
25+
({
26+
headers: {},
27+
connection: { remoteAddress },
28+
socket: { remoteAddress },
29+
}) as any
30+
31+
it('returns false when no rate limits are configured', async () => {
32+
const rateLimiter = { hit: sinon.stub().resolves(false) }
33+
const settings = { ...baseSettings, limits: { admin: { ipWhitelist: [] } } }
34+
35+
const limited = await isAdminRateLimited(makeRequest(), settings as any, () => rateLimiter, 'admin')
36+
37+
expect(limited).to.equal(false)
38+
expect(rateLimiter.hit).not.to.have.been.called
39+
})
40+
41+
it('returns true when login rate limit is exceeded', async () => {
42+
const rateLimiter = { hit: sinon.stub().resolves(true) }
43+
44+
const limited = await isAdminRateLimited(makeRequest(), baseSettings as any, () => rateLimiter, 'login')
45+
46+
expect(limited).to.equal(true)
47+
expect(rateLimiter.hit).to.have.been.calledOnceWithExactly('1.2.3.4:admin-login:900000', 1, {
48+
period: 900000,
49+
rate: 10,
50+
})
51+
})
52+
53+
it('returns true when admin route rate limit is exceeded', async () => {
54+
const rateLimiter = { hit: sinon.stub().resolves(true) }
55+
56+
const limited = await isAdminRateLimited(makeRequest(), baseSettings as any, () => rateLimiter, 'admin')
57+
58+
expect(limited).to.equal(true)
59+
expect(rateLimiter.hit).to.have.been.calledOnceWithExactly('1.2.3.4:admin-admin:60000', 1, {
60+
period: 60000,
61+
rate: 30,
62+
})
63+
})
64+
})

0 commit comments

Comments
 (0)