Improve rate limiting (#15272)

* Improve rate limiting

* make rate limiter config names more consistent

* fix tests and add new one

* correct math
This commit is contained in:
Phillip Thelen
2024-08-06 19:45:27 +02:00
committed by GitHub
parent afd00a8ab6
commit 1fe4bd2de7
2 changed files with 64 additions and 1 deletions
@@ -54,6 +54,7 @@ describe('rateLimiter middleware', () => {
it('does not throw when there are available points', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
await attachRateLimiter(req, res, next);
@@ -71,6 +72,7 @@ describe('rateLimiter middleware', () => {
it('does not throw when an unknown error is thrown by the rate limiter', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
sandbox.stub(logger, 'error');
sandbox.stub(RateLimiterMemory.prototype, 'consume')
.returns(Promise.reject(new Error('Unknown error.')));
@@ -104,6 +106,7 @@ describe('rateLimiter middleware', () => {
it('limits when LIVELINESS_PROBE_KEY is incorrect', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns('abc');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
req.query.liveliness = 'das';
@@ -120,6 +123,7 @@ describe('rateLimiter middleware', () => {
it('limits when LIVELINESS_PROBE_KEY is not set', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns(undefined);
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
await attachRateLimiter(req, res, next);
@@ -135,6 +139,7 @@ describe('rateLimiter middleware', () => {
it('throws when LIVELINESS_PROBE_KEY is blank', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns('');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
req.query.liveliness = '';
@@ -150,6 +155,7 @@ describe('rateLimiter middleware', () => {
it('throws when there are no available points remaining', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
// call for 31 times
@@ -173,6 +179,7 @@ describe('rateLimiter middleware', () => {
it('uses the user id if supplied or the ip address', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
req.ip = 1;
@@ -199,4 +206,51 @@ describe('rateLimiter middleware', () => {
'X-RateLimit-Reset': sinon.match(Date),
});
});
it('applies increased cost for registration calls with and without user id', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_REGISTRATION_COST').returns(3);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
req.path = '/api/v4/user/auth/local/register';
req.ip = 1;
await attachRateLimiter(req, res, next);
req.headers['x-api-user'] = 'user-1';
await attachRateLimiter(req, res, next);
await attachRateLimiter(req, res, next);
// user id an ip are counted as separate sources
expect(res.set).to.have.been.calledWithMatch({
'X-RateLimit-Limit': 30,
'X-RateLimit-Remaining': 27, // 2 calls with user id
'X-RateLimit-Reset': sinon.match(Date),
});
req.headers['x-api-user'] = undefined;
await attachRateLimiter(req, res, next);
await attachRateLimiter(req, res, next);
expect(res.set).to.have.been.calledWithMatch({
'X-RateLimit-Limit': 30,
'X-RateLimit-Remaining': 24, // 3 calls with only ip
'X-RateLimit-Reset': sinon.match(Date),
});
});
it('applies increased cost for unauthenticated API calls', async () => {
nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true');
nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(10);
const attachRateLimiter = requireAgain(pathToRateLimiter).default;
req.ip = 1;
await attachRateLimiter(req, res, next);
await attachRateLimiter(req, res, next);
expect(res.set).to.have.been.calledWithMatch({
'X-RateLimit-Limit': 30,
'X-RateLimit-Remaining': 10,
'X-RateLimit-Reset': sinon.match(Date),
});
});
});
+10 -1
View File
@@ -22,6 +22,8 @@ const REDIS_HOST = nconf.get('REDIS_HOST');
const REDIS_PASSWORD = nconf.get('REDIS_PASSWORD');
const REDIS_PORT = nconf.get('REDIS_PORT');
const LIVELINESS_PROBE_KEY = nconf.get('LIVELINESS_PROBE_KEY');
const REGISTRATION_COST = nconf.get('RATE_LIMITER_REGISTRATION_COST') || 5;
const IP_RATE_LIMIT_COST = nconf.get('RATE_LIMITER_IP_COST') || 5;
let redisClient;
let rateLimiter;
@@ -76,7 +78,14 @@ export default function rateLimiterMiddleware (req, res, next) {
const userId = req.header('x-api-user');
return rateLimiter.consume(userId || req.ip)
let cost = 1;
if (req.path === '/api/v4/user/auth/local/register' || req.path === '/api/v3/user/auth/local/register') {
cost = REGISTRATION_COST;
} else if (!userId) {
cost = IP_RATE_LIMIT_COST;
}
return rateLimiter.consume(userId || req.ip, cost)
.then(rateLimiterRes => {
setResponseHeaders(res, rateLimiterRes);
return next();