Skip to content

Commit

Permalink
Implement Accounts creation rate limit (#1800)
Browse files Browse the repository at this point in the history 
Adds a rate limitation controlled by a cache key before proceeding with account creation
  • Loading branch information
@hectorgomezv
hectorgomezv authored Aug 7, 2024
1 parent e042b7b commit 234a559
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 23 deletions.
4 changes: 4 additions & 0 deletions src/config/entities/__tests__/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ export default (): ReturnType<typeof configuration> => ({
version: faker.system.semver(),
buildNumber: faker.string.numeric(),
},
accounts: {
creationRateLimitPeriodSeconds: faker.number.int(),
creationRateLimitCalls: faker.number.int(),
},
amqp: {
url: faker.internet.url({ appendSlash: false }),
exchange: { name: faker.string.sample(), mode: faker.string.sample() },
Expand Down
8 changes: 8 additions & 0 deletions src/config/entities/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ export default () => ({
version: process.env.APPLICATION_VERSION,
buildNumber: process.env.APPLICATION_BUILD_NUMBER,
},
accounts: {
creationRateLimitPeriodSeconds: parseInt(
process.env.ACCOUNT_CREATION_RATE_LIMIT_PERIOD_SECONDS ?? `${60}`,
),
creationRateLimitCalls: parseInt(
process.env.ACCOUNT_CREATION_RATE_LIMIT_CALLS_BY_PERIOD ?? `${1}`,
),
},
amqp: {
url: process.env.AMQP_URL || 'amqp://localhost:5672',
exchange: {
Expand Down
159 changes: 142 additions & 17 deletions src/datasources/accounts/accounts.datasource.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ describe('AccountsDatasource tests', () => {
it('creates an account successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());

const result = await target.createAccount(address);
const result = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});

expect(result).toStrictEqual({
id: expect.any(Number),
Expand All @@ -88,20 +91,124 @@ describe('AccountsDatasource tests', () => {
);
});

it('throws when an account with the same address already exists', async () => {
it('creates an account successfully if the clientIp is not a valid IP', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);

await expect(target.createAccount(address)).rejects.toThrow(
'Error creating account.',
const result = await target.createAccount({
address,
clientIp: faker.string.sample(),
});

expect(result).toStrictEqual({
id: expect.any(Number),
group_id: null,
address,
created_at: expect.any(Date),
updated_at: expect.any(Date),
});

// check the account is stored in the cache
const cacheDir = new CacheDir(`account_${address}`, '');
const cacheContent = await fakeCacheService.get(cacheDir);
expect(JSON.parse(cacheContent as string)).toStrictEqual(
expect.arrayContaining([
expect.objectContaining({
id: expect.any(Number),
group_id: null,
address,
}),
]),
);
});

it('should fail if the IP hits the rate limit', async () => {
const clientIp = faker.internet.ipv4();
const accountCreationRateLimitCalls = faker.number.int({
min: 2,
max: 5,
});
mockConfigurationService.getOrThrow.mockImplementation((key) => {
if (key === 'expirationTimeInSeconds.default')
return faker.number.int();
if (key === 'accounts.creationRateLimitCalls')
return accountCreationRateLimitCalls;
if (key === 'accounts.creationRateLimitPeriodSeconds')
return faker.number.int({ min: 10 });
});
target = new AccountsDatasource(
fakeCacheService,
sql,
new CachedQueryResolver(mockLoggingService, fakeCacheService),
mockLoggingService,
mockConfigurationService,
);

for (let i = 0; i < accountCreationRateLimitCalls; i++) {
await target.createAccount({
address: getAddress(faker.finance.ethereumAddress()),
clientIp,
});
}

await expect(
target.createAccount({
address: getAddress(faker.finance.ethereumAddress()),
clientIp,
}),
).rejects.toThrow('Rate limit reached');

const { count } = await sql`SELECT id FROM accounts`;
expect(count).toBe(accountCreationRateLimitCalls);
});

it('should create accounts while the IP does not hit the rate limit', async () => {
const clientIp = faker.internet.ipv4();
const accountsToCreate = faker.number.int({ min: 1, max: 5 });
const accountCreationRateLimitCalls = faker.number.int({
min: 5,
max: 10,
});
mockConfigurationService.getOrThrow.mockImplementation((key) => {
if (key === 'expirationTimeInSeconds.default')
return faker.number.int();
if (key === 'accounts.creationRateLimitCalls')
return accountCreationRateLimitCalls;
if (key === 'accounts.creationRateLimitPeriodSeconds')
return faker.number.int({ min: 10 });
});
target = new AccountsDatasource(
fakeCacheService,
sql,
new CachedQueryResolver(mockLoggingService, fakeCacheService),
mockLoggingService,
mockConfigurationService,
);

for (let i = 0; i < accountsToCreate; i++) {
await target.createAccount({
address: getAddress(faker.finance.ethereumAddress()),
clientIp,
});
}

const { count } = await sql`SELECT id FROM accounts`;
expect(count).toBe(accountsToCreate);
});

it('throws when an account with the same address already exists', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount({ address, clientIp: faker.internet.ipv4() });

await expect(
target.createAccount({ address, clientIp: faker.internet.ipv4() }),
).rejects.toThrow('Error creating account.');
});
});

describe('getAccount', () => {
it('returns an account successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });

const result = await target.getAccount(address);

Expand All @@ -116,7 +223,7 @@ describe('AccountsDatasource tests', () => {

it('returns an account from cache', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });

const result = await target.getAccount(address);

Expand Down Expand Up @@ -174,7 +281,7 @@ describe('AccountsDatasource tests', () => {
describe('deleteAccount', () => {
it('deletes an account successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });

await expect(target.deleteAccount(address)).resolves.not.toThrow();

Expand All @@ -191,7 +298,7 @@ describe('AccountsDatasource tests', () => {

it('should clear the cache on account deletion', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });

// get the account from the cache
const beforeDeletion = await target.getAccount(address);
Expand Down Expand Up @@ -335,7 +442,10 @@ describe('AccountsDatasource tests', () => {
describe('getAccountDataSettings', () => {
it('should get the account data settings successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -366,7 +476,10 @@ describe('AccountsDatasource tests', () => {

it('should get the account data settings from cache', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -426,7 +539,10 @@ describe('AccountsDatasource tests', () => {

it('should omit account data settings which data type is not active', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -467,7 +583,10 @@ describe('AccountsDatasource tests', () => {
describe('upsertAccountDataSettings', () => {
it('adds account data settings successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -500,7 +619,10 @@ describe('AccountsDatasource tests', () => {

it('should write the associated cache on upsert', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -538,7 +660,10 @@ describe('AccountsDatasource tests', () => {

it('updates existing account data settings successfully', async () => {
const address = getAddress(faker.finance.ethereumAddress());
const account = await target.createAccount(address);
const account = await target.createAccount({
address,
clientIp: faker.internet.ipv4(),
});
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -623,7 +748,7 @@ describe('AccountsDatasource tests', () => {

it('throws an error if a non-existent data type is provided', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });
const accountDataTypes = Array.from(
{ length: faker.number.int({ min: 1, max: 4 }) },
() => accountDataTypeBuilder().with('is_active', true).build(),
Expand Down Expand Up @@ -652,7 +777,7 @@ describe('AccountsDatasource tests', () => {

it('throws an error if an inactive data type is provided', async () => {
const address = getAddress(faker.finance.ethereumAddress());
await target.createAccount(address);
await target.createAccount({ address, clientIp: faker.internet.ipv4() });
const accountDataTypes = [
accountDataTypeBuilder().with('is_active', false).build(),
accountDataTypeBuilder().build(),
Expand Down
58 changes: 55 additions & 3 deletions src/datasources/accounts/accounts.datasource.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import {
import { MAX_TTL } from '@/datasources/cache/constants';
import { CachedQueryResolver } from '@/datasources/db/cached-query-resolver';
import { ICachedQueryResolver } from '@/datasources/db/cached-query-resolver.interface';
import { LimitReachedError } from '@/datasources/network/entities/errors/limit-reached.error';
import { AccountDataSetting } from '@/domain/accounts/entities/account-data-setting.entity';
import { AccountDataType } from '@/domain/accounts/entities/account-data-type.entity';
import { Account } from '@/domain/accounts/entities/account.entity';
import { UpsertAccountDataSettingsDto } from '@/domain/accounts/entities/upsert-account-data-settings.dto.entity';
import { IAccountsDatasource } from '@/domain/interfaces/accounts.datasource.interface';
import { ILoggingService, LoggingService } from '@/logging/logging.interface';
import { asError } from '@/logging/utils';
import { IpSchema } from '@/validation/entities/schemas/ip.schema';
import {
Inject,
Injectable,
Expand All @@ -25,7 +27,12 @@ import postgres from 'postgres';

@Injectable()
export class AccountsDatasource implements IAccountsDatasource, OnModuleInit {
private static readonly ACCOUNT_CREATION_CACHE_PREFIX = 'account_creation';
private readonly defaultExpirationTimeInSeconds: number;
// Number of seconds for each rate-limit cycle
private readonly accountCreationRateLimitPeriodSeconds: number;
// Number of allowed calls on each rate-limit cycle
private readonly accountCreationRateLimitCalls: number;

constructor(
@Inject(CacheService) private readonly cacheService: ICacheService,
Expand All @@ -40,6 +47,13 @@ export class AccountsDatasource implements IAccountsDatasource, OnModuleInit {
this.configurationService.getOrThrow<number>(
'expirationTimeInSeconds.default',
);
this.accountCreationRateLimitPeriodSeconds =
configurationService.getOrThrow(
'accounts.creationRateLimitPeriodSeconds',
);
this.accountCreationRateLimitCalls = configurationService.getOrThrow(
'accounts.creationRateLimitCalls',
);
}

/**
Expand All @@ -52,17 +66,21 @@ export class AccountsDatasource implements IAccountsDatasource, OnModuleInit {
);
}

async createAccount(address: `0x${string}`): Promise<Account> {
async createAccount(args: {
address: `0x${string}`;
clientIp: string;
}): Promise<Account> {
await this.checkCreationRateLimit(args.clientIp);
const [account] = await this.sql<[Account]>`
INSERT INTO accounts (address) VALUES (${address}) RETURNING *`.catch(
INSERT INTO accounts (address) VALUES (${args.address}) RETURNING *`.catch(
(e) => {
this.loggingService.warn(
`Error creating account: ${asError(e).message}`,
);
throw new UnprocessableEntityException('Error creating account.');
},
);
const cacheDir = CacheRouter.getAccountCacheDir(address);
const cacheDir = CacheRouter.getAccountCacheDir(args.address);
await this.cacheService.set(
cacheDir,
JSON.stringify([account]),
Expand Down Expand Up @@ -192,4 +210,38 @@ export class AccountsDatasource implements IAccountsDatasource, OnModuleInit {
);
}
}

/**
* Checks if the client IP address has reached the account creation rate limit.
*
* NOTE: the rate limit is implemented in the datasource layer for this use case
* because we need to restrict the actual creation of accounts, not merely
* the attempts to create them.
*
* If the client IP address is invalid, a warning is logged.
* If the client IP address is valid and rate limit is reached, a {@link LimitReachedError} is thrown.
*
* @param clientIp - client IP address.
*/
private async checkCreationRateLimit(clientIp: string): Promise<void> {
const { success: isValidIp } = IpSchema.safeParse(clientIp);
if (!clientIp || !isValidIp) {
this.loggingService.warn(
`Invalid client IP while creating account: ${clientIp}`,
);
} else {
const current = await this.cacheService.increment(
CacheRouter.getRateLimitCacheKey(
`${AccountsDatasource.ACCOUNT_CREATION_CACHE_PREFIX}_${clientIp}`,
),
this.accountCreationRateLimitPeriodSeconds,
);
if (current > this.accountCreationRateLimitCalls) {
this.loggingService.warn(
`Limit of ${this.accountCreationRateLimitCalls} reached for IP ${clientIp}`,
);
throw new LimitReachedError();
}
}
}
}
1 change: 1 addition & 0 deletions src/domain/accounts/accounts.repository.interface.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface IAccountsRepository {
createAccount(args: {
authPayload: AuthPayload;
address: `0x${string}`;
clientIp: string;
}): Promise<Account>;

getAccount(args: {
Expand Down
Loading

0 comments on commit 234a559

Please sign in to comment.