Skip to content

Commit 92516e3

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

7 files changed

Lines changed: 166 additions & 5 deletions

File tree

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
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { NextFunction, Request, Response } from 'express'
2+
3+
import { createSettings } from '../../factories/settings-factory'
4+
import { rateLimiterFactory } from '../../factories/rate-limiter-factory'
5+
import { isAdminRateLimited } from '../../utils/admin-rate-limit'
6+
7+
const sendTooManyRequests = (response: Response) => {
8+
response.status(429).setHeader('content-type', 'application/json').send(JSON.stringify({ error: 'Too many requests' }))
9+
}
10+
11+
export const adminLoginRateLimitMiddleware = async (request: Request, response: Response, next: NextFunction) => {
12+
if (await isAdminRateLimited(request, createSettings(), rateLimiterFactory, 'login')) {
13+
sendTooManyRequests(response)
14+
return
15+
}
16+
17+
next()
18+
}
19+
20+
export const adminRateLimitMiddleware = async (request: Request, response: Response, next: NextFunction) => {
21+
if (await isAdminRateLimited(request, createSettings(), rateLimiterFactory, 'admin')) {
22+
sendTooManyRequests(response)
23+
return
24+
}
25+
26+
next()
27+
}

src/routes/admin/index.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import { createGetAdminHealthController } from '../../factories/controllers/get-
55
import { createGetAdminSessionController } from '../../factories/controllers/get-admin-session-controller-factory'
66
import { createPostAdminLoginController } from '../../factories/controllers/post-admin-login-controller-factory'
77
import { createSettings } from '../../factories/settings-factory'
8+
import {
9+
adminLoginRateLimitMiddleware,
10+
adminRateLimitMiddleware,
11+
} from '../../handlers/request-handlers/admin-rate-limit-middleware'
812
import { rateLimiterMiddleware } from '../../handlers/request-handlers/rate-limiter-middleware'
913
import { withController } from '../../handlers/request-handlers/with-controller-request-handler'
1014

@@ -32,8 +36,8 @@ const requireAdminAuth = (request: Request, response: Response, next: NextFuncti
3236
router.use(requireAdminEnabled)
3337
router.use(rateLimiterMiddleware)
3438

35-
router.post('/login', json(), withController(createPostAdminLoginController))
36-
router.get('/session', requireAdminAuth, withController(createGetAdminSessionController))
37-
router.get('/health', requireAdminAuth, withController(createGetAdminHealthController))
39+
router.post('/login', adminLoginRateLimitMiddleware, json(), withController(createPostAdminLoginController))
40+
router.get('/session', adminRateLimitMiddleware, requireAdminAuth, withController(createGetAdminSessionController))
41+
router.get('/health', adminRateLimitMiddleware, requireAdminAuth, withController(createGetAdminHealthController))
3842

3943
export default router

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

test/unit/routes/admin.spec.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import Sinon from 'sinon'
55

66
import * as getAdminHealthControllerFactory from '../../../src/factories/controllers/get-admin-health-controller-factory'
77
import { hashAdminPassword } from '../../../src/utils/admin-password'
8+
import * as adminRateLimitMiddleware from '../../../src/handlers/request-handlers/admin-rate-limit-middleware'
89
import * as rateLimiterMiddleware from '../../../src/handlers/request-handlers/rate-limiter-middleware'
910
import * as settingsFactory from '../../../src/factories/settings-factory'
1011

@@ -14,6 +15,8 @@ describe('admin router', () => {
1415
let createGetAdminHealthControllerStub: Sinon.SinonStub
1516
let createSettingsStub: Sinon.SinonStub
1617
let rateLimiterMiddlewareStub: Sinon.SinonStub
18+
let adminRateLimitMiddlewareStub: Sinon.SinonStub
19+
let adminLoginRateLimitMiddlewareStub: Sinon.SinonStub
1720
let server: any
1821

1922
const loadAdminRouter = () => {
@@ -43,9 +46,12 @@ describe('admin router', () => {
4346
},
4447
} as any)
4548
createSettingsStub = Sinon.stub(settingsFactory, 'createSettings').returns(settings as any)
46-
rateLimiterMiddlewareStub = Sinon.stub(rateLimiterMiddleware, 'rateLimiterMiddleware').callsFake(async (_request, _response, next) => {
49+
const passthrough = async (_request: any, _response: any, next: any) => {
4750
next()
48-
})
51+
}
52+
rateLimiterMiddlewareStub = Sinon.stub(rateLimiterMiddleware, 'rateLimiterMiddleware').callsFake(passthrough)
53+
adminRateLimitMiddlewareStub = Sinon.stub(adminRateLimitMiddleware, 'adminRateLimitMiddleware').callsFake(passthrough)
54+
adminLoginRateLimitMiddlewareStub = Sinon.stub(adminRateLimitMiddleware, 'adminLoginRateLimitMiddleware').callsFake(passthrough)
4955
const router = loadAdminRouter()
5056
const app = express()
5157
app.use('/admin', router)
@@ -61,6 +67,8 @@ describe('admin router', () => {
6167
createGetAdminHealthControllerStub?.restore()
6268
createSettingsStub?.restore()
6369
rateLimiterMiddlewareStub?.restore()
70+
adminRateLimitMiddlewareStub?.restore()
71+
adminLoginRateLimitMiddlewareStub?.restore()
6472
delete require.cache[require.resolve('../../../src/routes/admin/index')]
6573
delete require.cache[require.resolve('../../../src/routes/admin')]
6674

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)