Skip to content

Commit 49ae61d

Browse files
mtrezzamtrezza
committed
fix: Rate limit user zone key fallback and batch request bypass
Fix rate limit `user` zone always falling back to IP-based key due to checking `request.zone` instead of `route.zone` in the key generator. Replace the static batch sub-request count pre-check with actual rate limiter invocation so batch sub-requests consume tokens from the same window state as direct requests.
1 parent 90f254d commit 49ae61d

File tree

3 files changed

+185
-39
lines changed

3 files changed

+185
-39
lines changed

‎spec/RateLimit.spec.js‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,73 @@ describe('rate limit', () => {
434434
new Parse.Error(Parse.Error.CONNECTION_FAILED, 'Too many requests')
435435
);
436436
});
437+
438+
it('should rate limit per user independently with user zone', async () => {
439+
await reconfigureServer({
440+
rateLimit: {
441+
requestPath: '/functions/*path',
442+
requestTimeWindow: 10000,
443+
requestCount: 1,
444+
errorResponseMessage: 'Too many requests',
445+
includeInternalRequests: true,
446+
zone: Parse.Server.RateLimitZone.user,
447+
},
448+
});
449+
Parse.Cloud.define('test', () => 'Abc');
450+
// Sign up two different users using REST API to avoid destroying sessions
451+
const res1 = await request({
452+
method: 'POST',
453+
headers: headers,
454+
url: 'http://localhost:8378/1/users',
455+
body: JSON.stringify({ username: 'user1', password: 'password' }),
456+
});
457+
const sessionToken1 = res1.data.sessionToken;
458+
const res2 = await request({
459+
method: 'POST',
460+
headers: headers,
461+
url: 'http://localhost:8378/1/users',
462+
body: JSON.stringify({ username: 'user2', password: 'password' }),
463+
});
464+
const sessionToken2 = res2.data.sessionToken;
465+
// User 1 makes a request — should succeed
466+
const result1 = await request({
467+
method: 'POST',
468+
headers: { ...headers, 'X-Parse-Session-Token': sessionToken1 },
469+
url: 'http://localhost:8378/1/functions/test',
470+
body: JSON.stringify({}),
471+
});
472+
expect(result1.data.result).toBe('Abc');
473+
// User 2 makes a request — should also succeed (independent rate limit per user)
474+
const result2 = await request({
475+
method: 'POST',
476+
headers: { ...headers, 'X-Parse-Session-Token': sessionToken2 },
477+
url: 'http://localhost:8378/1/functions/test',
478+
body: JSON.stringify({}),
479+
});
480+
expect(result2.data.result).toBe('Abc');
481+
// User 1 makes another request — should be rate limited
482+
const result3 = await request({
483+
method: 'POST',
484+
headers: { ...headers, 'X-Parse-Session-Token': sessionToken1 },
485+
url: 'http://localhost:8378/1/functions/test',
486+
body: JSON.stringify({}),
487+
}).catch(e => e);
488+
expect(result3.data).toEqual({
489+
code: Parse.Error.CONNECTION_FAILED,
490+
error: 'Too many requests',
491+
});
492+
// User 2 makes another request — should also be rate limited
493+
const result4 = await request({
494+
method: 'POST',
495+
headers: { ...headers, 'X-Parse-Session-Token': sessionToken2 },
496+
url: 'http://localhost:8378/1/functions/test',
497+
body: JSON.stringify({}),
498+
}).catch(e => e);
499+
expect(result4.data).toEqual({
500+
code: Parse.Error.CONNECTION_FAILED,
501+
error: 'Too many requests',
502+
});
503+
});
437504
});
438505

439506
it('can validate rateLimit', async () => {
@@ -679,6 +746,94 @@ describe('rate limit', () => {
679746
});
680747
});
681748

749+
it('should enforce rate limit across direct requests and batch sub-requests', async () => {
750+
await reconfigureServer({
751+
rateLimit: [
752+
{
753+
requestPath: '/classes/*path',
754+
requestTimeWindow: 10000,
755+
requestCount: 2,
756+
errorResponseMessage: 'Too many requests',
757+
includeInternalRequests: true,
758+
},
759+
],
760+
});
761+
// First direct request — should succeed (count: 1)
762+
const obj = new Parse.Object('MyObject');
763+
await obj.save();
764+
// Batch with 1 sub-request — should succeed (count: 2)
765+
const response1 = await request({
766+
method: 'POST',
767+
headers: headers,
768+
url: 'http://localhost:8378/1/batch',
769+
body: JSON.stringify({
770+
requests: [
771+
{ method: 'POST', path: '/1/classes/MyObject', body: { key: 'value1' } },
772+
],
773+
}),
774+
});
775+
expect(response1.data.length).toBe(1);
776+
expect(response1.data[0].success).toBeDefined();
777+
// Another batch with 1 sub-request — should be rate limited (count would be 3)
778+
const response2 = await request({
779+
method: 'POST',
780+
headers: headers,
781+
url: 'http://localhost:8378/1/batch',
782+
body: JSON.stringify({
783+
requests: [
784+
{ method: 'POST', path: '/1/classes/MyObject', body: { key: 'value2' } },
785+
],
786+
}),
787+
}).catch(e => e);
788+
expect(response2.data).toEqual({
789+
code: Parse.Error.CONNECTION_FAILED,
790+
error: 'Too many requests',
791+
});
792+
});
793+
794+
it('should enforce rate limit for multiple batch requests in same window', async () => {
795+
await reconfigureServer({
796+
rateLimit: [
797+
{
798+
requestPath: '/classes/*path',
799+
requestTimeWindow: 10000,
800+
requestCount: 2,
801+
errorResponseMessage: 'Too many requests',
802+
includeInternalRequests: true,
803+
},
804+
],
805+
});
806+
// First batch with 2 sub-requests — should succeed (count: 2)
807+
const response1 = await request({
808+
method: 'POST',
809+
headers: headers,
810+
url: 'http://localhost:8378/1/batch',
811+
body: JSON.stringify({
812+
requests: [
813+
{ method: 'POST', path: '/1/classes/MyObject', body: { key: 'value1' } },
814+
{ method: 'POST', path: '/1/classes/MyObject', body: { key: 'value2' } },
815+
],
816+
}),
817+
});
818+
expect(response1.data.length).toBe(2);
819+
expect(response1.data[0].success).toBeDefined();
820+
// Second batch with 1 sub-request — should be rate limited (count would be 3)
821+
const response2 = await request({
822+
method: 'POST',
823+
headers: headers,
824+
url: 'http://localhost:8378/1/batch',
825+
body: JSON.stringify({
826+
requests: [
827+
{ method: 'POST', path: '/1/classes/MyObject', body: { key: 'value3' } },
828+
],
829+
}),
830+
}).catch(e => e);
831+
expect(response2.data).toEqual({
832+
code: Parse.Error.CONNECTION_FAILED,
833+
error: 'Too many requests',
834+
});
835+
});
836+
682837
it('should not reject batch when sub-requests target non-rate-limited paths', async () => {
683838
await reconfigureServer({
684839
rateLimit: [

‎src/batch.js‎

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ function makeBatchRoutingPathFunction(originalUrl, serverURL, publicServerURL) {
6363

6464
// Returns a promise for a {response} object.
6565
// TODO: pass along auth correctly
66-
function handleBatch(router, req) {
66+
async function handleBatch(router, req) {
6767
if (!Array.isArray(req.body?.requests)) {
6868
throw new Parse.Error(Parse.Error.INVALID_JSON, 'requests must be an array');
6969
}
@@ -83,47 +83,38 @@ function handleBatch(router, req) {
8383
req.config.publicServerURL
8484
);
8585

86-
// Check if batch sub-requests would exceed any configured rate limits.
87-
// Count how many sub-requests target each rate-limited path and reject
88-
// the entire batch if any path's count exceeds its requestCount.
86+
// Enforce rate limits for each batch sub-request by invoking the
87+
// rate limit handler. This ensures sub-requests consume tokens from
88+
// the same window state as direct requests.
8989
const rateLimits = req.config.rateLimits || [];
90-
for (const limit of rateLimits) {
91-
// Skip rate limit if master key is used and includeMasterKey is not set
92-
if (req.auth?.isMaster && !limit.includeMasterKey) {
93-
continue;
94-
}
95-
// Skip rate limit for internal requests if includeInternalRequests is not set
96-
if (req.config.ip === '127.0.0.1' && !limit.includeInternalRequests) {
97-
continue;
98-
}
99-
const pathExp = limit.path.regexp || limit.path;
100-
let matchCount = 0;
101-
for (const restRequest of req.body.requests) {
102-
// Check if sub-request method matches the rate limit's requestMethods filter
103-
if (limit.requestMethods) {
104-
const method = restRequest.method?.toUpperCase();
105-
if (Array.isArray(limit.requestMethods)) {
106-
if (!limit.requestMethods.includes(method)) {
107-
continue;
108-
}
109-
} else {
110-
const regExp = new RegExp(limit.requestMethods);
111-
if (!regExp.test(method)) {
112-
continue;
113-
}
114-
}
90+
for (const restRequest of req.body.requests) {
91+
const routablePath = makeRoutablePath(restRequest.path);
92+
for (const limit of rateLimits) {
93+
const pathExp = limit.path.regexp || limit.path;
94+
if (!pathExp.test(routablePath)) {
95+
continue;
11596
}
116-
const routablePath = makeRoutablePath(restRequest.path);
117-
if (pathExp.test(routablePath)) {
118-
matchCount++;
97+
const fakeReq = {
98+
ip: req.ip || req.config?.ip || '127.0.0.1',
99+
method: (restRequest.method || 'GET').toUpperCase(),
100+
config: req.config,
101+
auth: req.auth,
102+
info: req.info,
103+
};
104+
const fakeRes = { setHeader() {} };
105+
try {
106+
await limit.handler(fakeReq, fakeRes, err => {
107+
if (err) {
108+
throw err;
109+
}
110+
});
111+
} catch {
112+
throw new Parse.Error(
113+
Parse.Error.CONNECTION_FAILED,
114+
limit.errorResponseMessage || 'Too many requests'
115+
);
119116
}
120117
}
121-
if (matchCount > limit.requestCount) {
122-
throw new Parse.Error(
123-
Parse.Error.CONNECTION_FAILED,
124-
limit.errorResponseMessage || 'Batch request exceeds rate limit for endpoint'
125-
);
126-
}
127118
}
128119

129120
const batch = transactionRetries => {

‎src/middlewares.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ export const addRateLimit = (route, config, cloud) => {
650650
if (!request.auth) {
651651
await new Promise(resolve => handleParseSession(request, null, resolve));
652652
}
653-
if (request.auth?.user?.id && request.zone === 'user') {
653+
if (request.auth?.user?.id && route.zone === 'user') {
654654
return request.auth.user.id;
655655
}
656656
}

0 commit comments

Comments
 (0)
⚡