Skip to content

Commit 4fe4611

Browse files
authored
Merge pull request #18952 from mozilla/configure-remaining-ratelimit-rules
task(auth,gql): Configure default limiting rules
2 parents 17dfc80 + 2068eb9 commit 4fe4611

23 files changed

Lines changed: 311 additions & 26 deletions

File tree

.circleci/config.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,8 @@ executors:
177177
CUSTOMS_SERVER_URL: none
178178
HUSKY_SKIP_INSTALL: 1
179179
AUTH_CLOUDTASKS_USE_LOCAL_EMULATOR: true
180-
RATE_LIMIT__RULES: ""
180+
# Seeing if clear customs approach works! RATE_LIMIT__RULES: ""
181+
RATE_LIMIT__IGNORE_EMAILS: .*@restmail.net$
181182

182183
# Contains a pre-installed fxa stack and browsers for doing ui test
183184
# automation. Perfect for running smoke tests against remote targets.

libs/accounts/rate-limit/src/lib/config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ export function parseConfigRules(
4444
// Loop overrules and group by the rule's action value.
4545
const ruleMap: Record<string, Rule[]> = {};
4646
const keys: Array<string> = [];
47+
let lineNumber = 0;
4748
for (let line of rules) {
49+
lineNumber++;
4850
// Clean up whitespace first.
4951
line = line.trim();
5052

@@ -62,7 +64,7 @@ export function parseConfigRules(
6264

6365
if (parts.length !== 5) {
6466
throw new InvalidRule(
65-
`Rules must have 5 parts separated delimited by :. But was ${line}`,
67+
`Issue detected on line ${lineNumber}. Rules must have 5 parts separated delimited by :. `,
6668
line
6769
);
6870
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import Redis from 'ioredis';
6+
import type { Redis as RedisType } from 'ioredis';
7+
8+
/**
9+
* A client that is useful to clear or emulate rate limiting scenarios. Note that this only
10+
* works when running in the test CI or locally. This WILL NOT work for smoke tests!
11+
*/
12+
export class RateLimitClient {
13+
private readonly redis: RedisType;
14+
15+
constructor() {
16+
// Only works for local host!
17+
this.redis = new Redis();
18+
}
19+
20+
/**
21+
* Resets all rate limiting counts in the Redis database. Useful during tests to avoid rate limiting.
22+
* @param action - Optional action to target. If nothing is provided, all counts are cleared.
23+
* @returns Number of keys deleted.
24+
*/
25+
async resetCounts(action?: string) {
26+
// Determine what keys to target
27+
const pattern = action ? `rate-limit:*:${action}:*` : 'rate-limit:*';
28+
29+
let cursor = '0';
30+
do {
31+
const [nextCursor, keys] = await this.redis.scan(
32+
cursor,
33+
'MATCH',
34+
pattern,
35+
'COUNT',
36+
100
37+
);
38+
cursor = nextCursor;
39+
if (keys.length > 0) {
40+
const deletedCount = await this.redis.del(...keys);
41+
console.log(`Deleted ${deletedCount} keys from redis.`);
42+
}
43+
} while (cursor !== '0');
44+
}
45+
}

packages/functional-tests/lib/targets/base.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ export abstract class BaseTarget {
4444
return this.contentServerUrl;
4545
}
4646

47-
constructor(readonly authServerUrl: string, emailUrl?: string) {
47+
constructor(
48+
readonly authServerUrl: string,
49+
emailUrl?: string
50+
) {
4851
const keyStretchVersion = parseInt(
4952
process.env.AUTH_CLIENT_KEY_STRETCH_VERSION || '1'
5053
);
@@ -65,6 +68,8 @@ export abstract class BaseTarget {
6568
});
6669
}
6770

71+
abstract clearRateLimits(): Promise<void>;
72+
6873
abstract createAccount(
6974
email: string,
7075
password: string,

packages/functional-tests/lib/targets/local.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { BoolString } from '../../../fxa-auth-client/lib/client';
66
import { TargetName } from '.';
77
import { BaseTarget, Credentials } from './base';
8+
import { RateLimitClient } from '../ratelimit';
89

910
const RELIER_CLIENT_ID = 'dcdb5ae7add825d2';
1011
const SUB_PRODUCT = 'prod_GqM9ToKK62qjkK';
@@ -23,16 +24,24 @@ export class LocalTarget extends BaseTarget {
2324
name: SUB_PRODUCT_NAME,
2425
plan: SUB_PLAN,
2526
};
27+
readonly rateLimitClient: RateLimitClient;
2628

2729
constructor() {
2830
super('http://localhost:9000', 'http://localhost:9001');
31+
this.rateLimitClient = new RateLimitClient();
32+
}
33+
34+
async clearRateLimits() {
35+
this.rateLimitClient.resetCounts();
2936
}
3037

3138
async createAccount(
3239
email: string,
3340
password: string,
3441
options = { lang: 'en', preVerified: 'true' as BoolString }
3542
) {
43+
// Quick and dirty way to see if this works...
44+
await this.rateLimitClient.resetCounts();
3645
const result = await this.authClient.signUp(email, password, options);
3746
await this.authClient.deviceRegister(
3847
result.sessionToken,

packages/functional-tests/lib/targets/remote.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
import { BaseTarget, Credentials } from './base';
66

77
export abstract class RemoteTarget extends BaseTarget {
8+
async clearRateLimits() {
9+
// no-op: We can't clear customs for smoke tests.
10+
}
11+
812
async createAccount(
913
email: string,
1014
password: string,

packages/functional-tests/tests/key-stretching-v2/changePassword.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ test.describe('severity-2 #smoke', () => {
4343
{ signupVersion: v2, changeVersion: v2 },
4444
];
4545

46+
test.beforeEach(async ({ target }) => {
47+
await target.clearRateLimits();
48+
});
49+
4650
for (const { signupVersion, changeVersion } of TestCases) {
4751
test(`signs up as v${signupVersion.version} changes password from settings as v${changeVersion.version} for backbone`, async ({
4852
page,

packages/functional-tests/tests/misc/authClientV2.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ test.describe('auth-client-tests', () => {
3232
return credentials;
3333
}
3434

35-
test.beforeEach(({}, { project }) => {
35+
test.beforeEach(async ({ target }, { project }) => {
3636
test.skip(project.name === 'production');
37+
await target.clearRateLimits();
3738
});
3839

3940
test('it creates with v1 and signs in', async ({

packages/functional-tests/tests/settings/changePassword.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ import { SigninPage } from '../../pages/signin';
1010

1111
test.describe('severity-1 #smoke', () => {
1212
test.describe('change password tests', () => {
13+
14+
test.beforeEach(async ({ target }) => {
15+
await target.clearRateLimits();
16+
});
17+
1318
test('change password with an incorrect old password', async ({
1419
target,
1520
pages: { page, changePassword, settings, signin },

packages/functional-tests/tests/settings/recoveryPhone.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ test.describe('severity-1 #smoke', () => {
7575
}
7676
});
7777

78+
test.beforeEach(async ({ target }) => {
79+
await target.clearRateLimits();
80+
});
81+
7882
test('setup fails with invalid number', async ({
7983
target,
8084
pages: { page, settings, signin, recoveryPhone, totp },

0 commit comments

Comments
 (0)