Skip to content
This repository has been archived by the owner on Sep 16, 2024. It is now read-only.

feat: support for subscription based rate limit customization #561

Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fern/docs/contents/rate-limits.mdx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
### Rate Limits

Revert will return a rate limit error with the HTTP status code `429` when the request rate limit for an application or an individual IP address has been exceeded.
Revert will return a rate limit error with the HTTP status code `429` when the request rate limit for a tenant has been exceeded.

As a default, 4 requests per minute are allowed from a single IP address but if you're needs are higher please get in touch and we can tailor it to your use-case.
As a default, 100 requests per minute per connection are allowed for a single tenant but if you're needs are higher please get in touch and we can tailor it to your use-case.
3 changes: 2 additions & 1 deletion packages/backend/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ MS_DYNAMICS_SALES_ORG_URL=
OPEN_INT_API_KEY=
TWENTY_ACCOUNT_ID=
BITBUCKET_CLIENT_ID=
BITBUCKET_CLIENT_SECRET=
BITBUCKET_CLIENT_SECRET=
DEFAULT_RATE_LIMIT_DEVELOPER_PLAN=
1 change: 1 addition & 0 deletions packages/backend/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const config = {
OPEN_INT_API_KEY: process.env.OPEN_INT_API_KEY,
OPEN_INT_BASE_API_URL: process.env.OPEN_INT_BASE_API_URL,
TWENTY_ACCOUNT_ID: process.env.TWENTY_ACCOUNT_ID,
DEFAULT_RATE_LIMIT_DEVELOPER_PLAN: process.env.DEFAULT_RATE_LIMIT_DEVELOPER_PLAN,
};

export default config;
1 change: 1 addition & 0 deletions packages/backend/helpers/authMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const revertAuthMiddleware = () => async (req: Request, res: Response, next: ()
include: {
environments: true,
accountFieldMappingConfig: true,
subscription: true,
},
});
if (!account || !account.length) {
Expand Down
50 changes: 50 additions & 0 deletions packages/backend/helpers/rateLimitMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { Request, Response } from 'express';
import { RateLimiterRedis, IRateLimiterStoreOptions } from 'rate-limiter-flexible';

import redis from '../redis/client';
import { skipRateLimitRoutes } from './utils';
import config from '../config';

// In Memory Cache for storing RateLimiterRedis instances to prevent the creation of a new instance for each request.
// The cache key is the 'rate_limit' value derived from the subscriptions table. Currently, we cache by the 'rate_limit' value for simplicity.
// Using 'subscriptionId' as a key would be more precise but would add complexity in keeping the cache in sync with the database.
const rateLimiters = new Map<number, RateLimiterRedis>();

//We can make this dynamic based on the subscription as well
const RATE_LIMIT_DURATION_IN_MINUTES = 1;

const getRateLimiter = (rateLimit: number): RateLimiterRedis => {
if (!rateLimiters.has(rateLimit)) {
const opts: IRateLimiterStoreOptions = {
storeClient: redis,
points: rateLimit, // Points represent the maximum number of requests allowed within the set duration.
duration: RATE_LIMIT_DURATION_IN_MINUTES * 60, // Converts minutes to seconds for the duration.
};
rateLimiters.set(rateLimit, new RateLimiterRedis(opts));
}
return rateLimiters.get(rateLimit)!;
};

const rateLimitMiddleware = () => async (req: Request, res: Response, next: Function) => {
if (skipRateLimitRoutes(req)) next();
try {
const { 'x-revert-t-id': tenantId } = req.headers;
const { subscription, id: accountId } = res.locals.account; // Subscription details are retrieved from response locals set earlier in the revertAuthMiddleware.
const rateLimit = subscription?.rate_limit ?? config.DEFAULT_RATE_LIMIT_DEVELOPER_PLAN; //incase subscription undefined, we will use the default rate limit this is to make sure backward compatibility as currently some accounts might not have subscription attached to them. We can remove the optional chaining and nullish coalescing once we are sure that all accounts have subscription attached to them
jatinsandilya marked this conversation as resolved.
Show resolved Hide resolved
const rateLimiter = getRateLimiter(rateLimit);
//added accountId to make the key unique
const uniqueKey = `${accountId}-${tenantId}`;
// rate limit is per tenantId
await rateLimiter.consume(uniqueKey, 1); // consume 1 point for each request
jatinsandilya marked this conversation as resolved.
Show resolved Hide resolved
next();
} catch (rejRes) {
//currently not handling the redis failure here explicitly as it is getting handled in the redis client. If required in future we can handle it here so service does't go down due to rate limit middleware failure which might occur due to redis failure
if (rejRes instanceof Error) {
throw rejRes;
} else {
res.status(429).send('Rate limit reached.');
}
}
};

export default rateLimitMiddleware;
17 changes: 17 additions & 0 deletions packages/backend/helpers/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Request } from 'express';

const skipRateLimitRoutes = (req: Request) => {
const nonSecurePaths = ['/oauth-callback', '/oauth/refresh'];
const nonSecurePathsPartialMatch = ['/integration-status', '/trello-request-token'];
const allowedRoutes = ['/health-check'];
if (
nonSecurePaths.includes(req.path) ||
nonSecurePathsPartialMatch.some((path) => req.path.includes(path)) ||
allowedRoutes.includes(req.baseUrl + req.path)
) {
return true;
}
return false;
};

export { skipRateLimitRoutes };
25 changes: 0 additions & 25 deletions packages/backend/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,8 @@ import versionMiddleware, { manageRouterVersioning } from './helpers/versionMidd
import { ShortloopSDK } from '@shortloop/node';
import endpointLogger from './helpers/endPointLoggerMiddleWare';

const rateLimit = require('express-rate-limit');
const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 60, // Limit each IP to 60 requests per `window` (here, per 15 minutes)
standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
legacyHeaders: false, // Disable the `X-RateLimit-*` headers
message: async () => {
return JSON.stringify({ message: 'Rate limit reached.' });
},
skip: (req: Request, _res: Response) => {
const basePath = req.baseUrl + req.path;
const allowedRoutes = ['/health-check'];
if (allowedRoutes.includes(basePath)) {
return true;
}
return false;
},
});

const app: Express = express();

// Debug rate limiting / trust proxy issue as per: https://github.com/express-rate-limit/express-rate-limit/wiki/Troubleshooting-Proxy-Issues
app.set('trust proxy', 1);
app.get('/ip', (request, response) => response.send(request.ip));
app.get('/x-forwarded-for', (request, response) => response.send(request.headers['x-forwarded-for']));

Sentry.init({
dsn: config.SENTRY_DSN,
integrations: [
Expand Down Expand Up @@ -107,7 +83,6 @@ app.use(
})
);

app.use(limiter);
app.use(versionMiddleware());

ShortloopSDK.init({
Expand Down
3 changes: 2 additions & 1 deletion packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"concurrently": "^8.2.1",
"eslint-plugin-deprecation": "^1.3.2",
"jest": "^29.6.4",
"ts-jest": "^29.1.2",
"ts-node": "^10.9.1",
"ts-node-dev": "^2.0.0",
"typescript": "^4.8.4"
Expand All @@ -69,14 +70,14 @@
"dayjs": "^1.11.10",
"dotenv": "^16.3.1",
"express": "^4.18.1",
"express-rate-limit": "^7.1.5",
"ip": "^1.1.8",
"lodash": "^4.17.21",
"moesif-nodejs": "^3.5.3",
"morgan": "^1.10.0",
"node-cron": "^3.0.2",
"oauth": "^0.10.0",
"prisma": "^4.16.0",
"rate-limiter-flexible": "^5.0.3",
"redis": "^4.6.8",
"svix": "^1.14.0",
"winston": "^3.11.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
-- AlterTable
ALTER TABLE "accounts" ADD COLUMN "subscriptionId" TEXT;

-- CreateTable
CREATE TABLE "subscriptions" (
"id" TEXT NOT NULL,
"name" TEXT NOT NULL,
"rate_limit" INTEGER NOT NULL,
"rate_limit_duration_in_minutes" INTEGER NOT NULL,
"price" DOUBLE PRECISION NOT NULL,
"features" JSONB,
"createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"updatedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,

CONSTRAINT "subscriptions_pkey" PRIMARY KEY ("id")
);

-- CreateIndex
CREATE UNIQUE INDEX "subscriptions_name_key" ON "subscriptions"("name");

-- AddForeignKey
ALTER TABLE "accounts" ADD CONSTRAINT "accounts_subscriptionId_fkey" FOREIGN KEY ("subscriptionId") REFERENCES "subscriptions"("id") ON DELETE SET NULL ON UPDATE CASCADE;
16 changes: 16 additions & 0 deletions packages/backend/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ model accounts {
accountFieldMappingConfig accountFieldMappingConfig?
createdAt DateTime @default(now())
updatedAt DateTime @default(now()) @updatedAt
subscription subscriptions? @relation(fields: [subscriptionId], references: [id])
subscriptionId String?
}

model environments {
Expand Down Expand Up @@ -155,3 +157,17 @@ model telemetry {
createdAt DateTime @default(now())
updatedAt DateTime @default(now()) @updatedAt
}

model subscriptions {
id String @id @default(uuid())
name String
rate_limit Int
rate_limit_duration_in_minutes Int
price Float
features Json?
accounts accounts[]
createdAt DateTime @default(now())
updatedAt DateTime @default(now()) @updatedAt

@@unique([name])
}
7 changes: 4 additions & 3 deletions packages/backend/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { logDebug } from '../helpers/logger';
import crmRouter from './v1/crm';
import config from '../config';
import revertAuthMiddleware from '../helpers/authMiddleware';
import rateLimitMiddleware from '../helpers/rateLimitMiddleware';
import { register } from '../generated/typescript';
import { metadataService } from '../services/metadata';
import { accountService } from '../services/Internal/account';
Expand Down Expand Up @@ -140,11 +141,11 @@ router.get('/connection/integration-status/:publicToken', async (req, res) => {
}
});

router.use('/crm', cors(), revertAuthMiddleware(), crmRouter);
router.use('/crm', cors(), [revertAuthMiddleware(), rateLimitMiddleware()], crmRouter);

router.use('/chat', cors(), revertAuthMiddleware(), chatRouter);
router.use('/chat', cors(), [revertAuthMiddleware(), rateLimitMiddleware()], chatRouter);

router.use('/ticket', cors(), revertAuthMiddleware(), ticketRouter);
router.use('/ticket', cors(), [revertAuthMiddleware(), rateLimitMiddleware()], ticketRouter);

register(router, {
metadata: metadataService,
Expand Down
56 changes: 56 additions & 0 deletions packages/backend/tests/rateLimitMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { Response } from 'express';
import rateLimitMiddleware from '../helpers/rateLimitMiddleware';
import { skipRateLimitRoutes } from '../helpers/utils';

jest.mock('../redis/client', () => {
return {
createClient: jest.fn().mockReturnThis(),
on: jest.fn(),
connect: jest.fn(),
};
});
jest.mock('../helpers/utils', () => {
return {
skipRateLimitRoutes: jest.fn(),
};
});
jest.mock('rate-limiter-flexible', () => {
return {
RateLimiterRedis: jest.fn().mockImplementation(() => ({
consume: jest.fn(),
})),
};
});
//TODO: some more extensive test case are required
describe('Rate Limit Middleware', () => {
const mockRequest = (options = {}) => ({
headers: { 'x-revert-t-id': 'tenant123' },
...options,
});
const mockResponse = () => {
const res = {
locals: { account: { subscription: { rate_limit: 100 }, id: 'account123' } },
} as unknown as Response;
res.status = jest.fn().mockReturnValue(res);
res.send = jest.fn().mockReturnValue(res);
return res;
};
const nextFunction = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
});

it('should call next if route should be skipped', async () => {
//@ts-ignore
skipRateLimitRoutes.mockReturnValue(true);
const req = mockRequest();
const res = mockResponse();

//@ts-ignore
await rateLimitMiddleware()(req, res, nextFunction);

expect(skipRateLimitRoutes).toHaveBeenCalled();
expect(nextFunction).toHaveBeenCalled();
});
});
Loading
Loading