Skip to content

Commit 8f709a7

Browse files
TolseeEmrysMyrddin
andauthored
fix(rate-limiter): incorrectly applying to non-configured fields (#2690)
* fix: rate limiter issue limiting non configured fields * chore: update failing test * fix: check whether the baseConfig is undefined * changeset --------- Co-authored-by: Valentin Cocaud <[email protected]>
1 parent 35539a7 commit 8f709a7

File tree

3 files changed

+58
-2
lines changed

3 files changed

+58
-2
lines changed

.changeset/eight-llamas-mate.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@envelop/rate-limiter': patch
3+
---
4+
5+
Fix rate limiting being wrongly applied to all fields with a default configuration.

packages/plugins/rate-limiter/src/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,10 @@ export const useRateLimiter = (options: RateLimiterPluginOptions): Plugin<RateLi
147147
);
148148
}
149149

150-
const rateLimitConfig = { ...(rateLimitDirective ?? fieldConfig) };
150+
const baseConfig = rateLimitDirective ?? fieldConfig;
151151

152-
if (rateLimitConfig) {
152+
if (baseConfig) {
153+
const rateLimitConfig = { ...baseConfig };
153154
rateLimitConfig.max = rateLimitConfig.max && Number(rateLimitConfig.max);
154155

155156
if (fieldConfig?.identifyFn) {

packages/plugins/rate-limiter/tests/use-rate-limiter.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,4 +448,54 @@ describe('Rate-Limiter', () => {
448448
),
449449
).toThrow(`Config error: field 'Query.foo' has both a configuration and a directive`);
450450
});
451+
it('should not rate limit fields that are not in configByField', async () => {
452+
const schema = makeExecutableSchema({
453+
typeDefs: /* GraphQL */ `
454+
type Query {
455+
limitedField: String
456+
unlimitedField: String
457+
}
458+
`,
459+
resolvers: {
460+
Query: {
461+
limitedField: () => 'limited',
462+
unlimitedField: () => 'unlimited',
463+
},
464+
},
465+
});
466+
467+
const testkit = createTestkit(
468+
[
469+
useRateLimiter({
470+
identifyFn: (ctx: any) => ctx.ip,
471+
configByField: [
472+
{
473+
type: 'Query',
474+
field: 'limitedField',
475+
max: 1,
476+
window: '60s',
477+
},
478+
],
479+
}),
480+
],
481+
schema,
482+
);
483+
484+
const context = { ip: '127.0.0.1' };
485+
486+
const result1 = await testkit.execute(`{ limitedField }`, {}, context);
487+
expect(result1).toEqual({ data: { limitedField: 'limited' } });
488+
489+
const result2 = await testkit.execute(`{ limitedField }`, {}, context);
490+
assertSingleExecutionValue(result2);
491+
expect(result2.errors?.[0]?.message).toBe("You are trying to access 'limitedField' too often");
492+
493+
// unlimitedField should not be rate limited at all, so we should be able to call it many times
494+
for (let i = 0; i < 10; i++) {
495+
const result = await testkit.execute(`{ unlimitedField }`, {}, context);
496+
assertSingleExecutionValue(result);
497+
expect(result).toEqual({ data: { unlimitedField: 'unlimited' } });
498+
expect(result.errors).toBeUndefined();
499+
}
500+
});
451501
});

0 commit comments

Comments
 (0)