Skip to content

Commit

Permalink
feat: remove separate get rate-limit for authenticated users
Browse files Browse the repository at this point in the history
  • Loading branch information
alexey-yarmosh committed Sep 27, 2024
1 parent 2b1dfc9 commit c5709a8
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 56 deletions.
7 changes: 3 additions & 4 deletions config/default.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,9 @@ module.exports = {
authenticatedLimit: 250,
reset: 3600,
},
get: {
anonymousLimit: 5,
authenticatedLimit: 5,
reset: 2,
getPerMeasurement: {
limit: 5,
reset: 20,
},
},
limits: {
Expand Down
5 changes: 2 additions & 3 deletions config/test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ module.exports = {
measurement: {
maxInProgressTests: 2,
rateLimit: {
get: {
anonymousLimit: 1000,
authenticatedLimit: 1000,
getPerMeasurement: {
limit: 1000,
},
},
},
Expand Down
3 changes: 0 additions & 3 deletions public/v1/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ paths:
$ref: 'components/responses.yaml#/components/responses/measurement429'
tags:
- Measurements
security:
- {}
- bearerAuth: []
/v1/probes:
get:
summary: List probes currently online
Expand Down
41 changes: 7 additions & 34 deletions src/lib/rate-limiter/rate-limiter-get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,22 @@ import type { Next } from 'koa';

const redisClient = getPersistentRedisClient();

export const anonymousRateLimiter = new RateLimiterRedis({
export const rateLimiter = new RateLimiterRedis({
storeClient: redisClient,
keyPrefix: 'rate:get:anon',
points: config.get<number>('measurement.rateLimit.get.anonymousLimit'),
duration: config.get<number>('measurement.rateLimit.get.reset'),
keyPrefix: 'rate:get',
points: config.get<number>('measurement.rateLimit.getPerMeasurement.limit'),
duration: config.get<number>('measurement.rateLimit.getPerMeasurement.reset'),
blockDuration: 5,
});

export const authenticatedRateLimiter = new RateLimiterRedis({
storeClient: redisClient,
keyPrefix: 'rate:get:auth',
points: config.get<number>('measurement.rateLimit.get.authenticatedLimit'),
duration: config.get<number>('measurement.rateLimit.get.reset'),
blockDuration: 5,
});

const getRateLimiter = (ctx: ExtendedContext, extraId?: string): {
type: 'user'| 'ip',
id: string,
rateLimiter: RateLimiterRedis
} => {
if (ctx.state.user?.id) {
return {
type: 'user',
id: extraId ? `${ctx.state.user.id}:${extraId}` : ctx.state.user.id,
rateLimiter: authenticatedRateLimiter,
};
}

const ip = requestIp.getClientIp(ctx.req) ?? '';
return {
type: 'ip',
id: extraId ? `${ip}:${extraId}` : ip,
rateLimiter: anonymousRateLimiter,
};
};

export const getMeasurementRateLimit = async (ctx: ExtendedContext, next: Next) => {
if (ctx['isAdmin']) {
return next();
}

const { rateLimiter, id } = getRateLimiter(ctx, ctx.params['id']);
const ip = requestIp.getClientIp(ctx.req) ?? '';
const measurementId = ctx.params['id'] ?? '';
const id = `${ip}:${measurementId}`;

try {
await rateLimiter.consume(id);
Expand Down
4 changes: 1 addition & 3 deletions src/measurement/route/get-measurement.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import type { DefaultContext, DefaultState, ParameterizedContext } from 'koa';
import type Router from '@koa/router';
import { getMeasurementStore } from '../store.js';
import { corsAuthHandler } from '../../lib/http/middleware/cors.js';
import { authenticate } from '../../lib/http/middleware/authenticate.js';
import { getMeasurementRateLimit } from '../../lib/rate-limiter/rate-limiter-get.js';
import createHttpError from 'http-errors';

Expand All @@ -27,5 +25,5 @@ const handle = async (ctx: ParameterizedContext<DefaultState, DefaultContext & R
};

export const registerGetMeasurementRoute = (router: Router): void => {
router.get('/measurements/:id', '/measurements/:id([a-zA-Z0-9]+)', corsAuthHandler(), authenticate(), getMeasurementRateLimit, handle);
router.get('/measurements/:id', '/measurements/:id([a-zA-Z0-9]+)', getMeasurementRateLimit, handle);
};
16 changes: 7 additions & 9 deletions test/tests/integration/ratelimit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { expect } from 'chai';
import { getTestServer, addFakeProbe, deleteFakeProbes, waitForProbesUpdate } from '../../utils/server.js';
import nockGeoIpProviders from '../../utils/nock-geo-ip.js';
import { anonymousRateLimiter as anonymousPostRateLimiter, authenticatedRateLimiter as authenticatedPostRateLimiter } from '../../../src/lib/rate-limiter/rate-limiter-post.js';
import { anonymousRateLimiter as anonymousGetRateLimiter, authenticatedRateLimiter as authenticatedGetRateLimiter } from '../../../src/lib/rate-limiter/rate-limiter-get.js';
import { rateLimiter as getRateLimiter } from '../../../src/lib/rate-limiter/rate-limiter-get.js';
import { client } from '../../../src/lib/sql/client.js';
import { GP_TOKENS_TABLE } from '../../../src/lib/http/auth.js';
import { CREDITS_TABLE } from '../../../src/lib/credits.js';
Expand Down Expand Up @@ -49,14 +49,12 @@ describe('rate limiter', () => {


afterEach(async () => {
const [ anonGetKeys, authGetKeys ] = await Promise.all([
await redis.keys(`rate:get:anon:${clientIpv6}:*`),
await redis.keys('rate:get:auth:89da69bd-a236-4ab7-9c5d-b5f52ce09959:*'),
const [ getKeys ] = await Promise.all([
await redis.keys(`rate:get:${clientIpv6}:*`),
await anonymousPostRateLimiter.delete(clientIpv6),
await authenticatedPostRateLimiter.delete('89da69bd-a236-4ab7-9c5d-b5f52ce09959'),
]);

const getKeys = [ ...anonGetKeys, ...authGetKeys ];
getKeys.length && await redis.del(getKeys);
});

Expand Down Expand Up @@ -172,7 +170,7 @@ describe('rate limiter', () => {
type: 'ping',
target: 'jsdelivr.com',
}).expect(202) as Response;
await anonymousGetRateLimiter.set(`${clientIpv6}:${id}`, 999, 0);
await getRateLimiter.set(`${clientIpv6}:${id}`, 999, 0);
const response = await requestAgent.get(`/v1/measurements/${id}`).send().expect(200) as Response;

expect(response.headers['Retry-After']).to.not.exist;
Expand Down Expand Up @@ -220,7 +218,7 @@ describe('rate limiter', () => {
type: 'ping',
target: 'jsdelivr.com',
}).expect(202) as Response;
await anonymousGetRateLimiter.set(`${clientIpv6}:${id}`, 1000, 0);
await getRateLimiter.set(`${clientIpv6}:${id}`, 1000, 0);
const response = await requestAgent.get(`/v1/measurements/${id}`).send().expect(429) as Response;

expect(response.headers['retry-after']).to.equal('5');
Expand Down Expand Up @@ -268,13 +266,13 @@ describe('rate limiter', () => {
expect(response.headers['x-ratelimit-remaining']).to.equal('1');
});

it('should fail and include Retry-After header (GET measurement)', async () => {
it('should fail and include Retry-After header if ip limit is applied (GET measurement)', async () => {
const { body: { id } } = await requestAgent.post('/v1/measurements')
.send({
type: 'ping',
target: 'jsdelivr.com',
}).expect(202) as Response;
await authenticatedGetRateLimiter.set(`89da69bd-a236-4ab7-9c5d-b5f52ce09959:${id}`, 1000, 0);
await getRateLimiter.set(`${clientIpv6}:${id}`, 1000, 0);
const response = await requestAgent.get(`/v1/measurements/${id}`)
.set('Authorization', 'Bearer qz5kdukfcr3vggv3xbujvjwvirkpkkpx')
.send().expect(429) as Response;
Expand Down

0 comments on commit c5709a8

Please sign in to comment.