Merge pull request 'fix/login-lockout-test-mock' (#1) from fix/login-lockout-test-mock into main
Reviewed-on: #1
This commit was merged in pull request #1.
This commit is contained in:
@@ -23,8 +23,7 @@ LLM_MAX_RETRIES=1
|
||||
LLM_TEMPERATURE=0.7
|
||||
LLM_MAX_TOKENS=2048
|
||||
|
||||
# Rate limits
|
||||
RATE_LIMIT_LOGIN=5
|
||||
# Rate limits (login uses progressive lockout: 5/10/20 failed attempts -> 15m/1h/24h block)
|
||||
RATE_LIMIT_REGISTER=3
|
||||
RATE_LIMIT_FORGOT_PASSWORD=3
|
||||
RATE_LIMIT_VERIFY_EMAIL=5
|
||||
|
||||
@@ -65,10 +65,25 @@ Do G1–G4, commit after each.
|
||||
**Agent H (Testing):**
|
||||
|
||||
```text
|
||||
Implement Agent H tasks from AGENT_TASKS.md. Work in branch feat/testing.
|
||||
Implement Agent H tasks from AGENT_TASKS.md. Work in branch feat/testing.
|
||||
Do H1–H7, commit after each. Target ≥70% coverage on services.
|
||||
```
|
||||
|
||||
**Agent A2 (Progressive Login Lockout):**
|
||||
|
||||
```text
|
||||
Implement Agent A2 task from AGENT_TASKS.md. Work in branch feat/progressive-login-lockout.
|
||||
Branch from dev. Do task A2-1. Commit with the message from the table.
|
||||
```
|
||||
|
||||
**Agent A2-2 (Fix Login Lockout Tests):**
|
||||
|
||||
```text
|
||||
Implement Agent A2-2 task from AGENT_TASKS.md. Work in branch fix/login-lockout-test-mock.
|
||||
Branch from dev. Do task A2-2. Commit with the message from the table.
|
||||
Ensure tests/integration/auth.routes.test.ts passes.
|
||||
```
|
||||
|
||||
## Текущее состояние репозитория
|
||||
|
||||
Часть работы уже выполнена одним агентом:
|
||||
@@ -138,6 +153,69 @@ Do H1–H7, commit after each. Target ≥70% coverage on services.
|
||||
|
||||
---
|
||||
|
||||
## Agent A2: Progressive Login Lockout
|
||||
|
||||
**Зависимости:** Agent A (redis, rateLimit), Agent C (auth routes). Уже есть Auth и rateLimit.
|
||||
|
||||
**Ветка:** `feat/progressive-login-lockout`
|
||||
|
||||
**Контекст:** Сейчас `POST /auth/login` использует фиксированный лимит через `@fastify/rate-limit` (N попыток на 15 мин). По [security.md](samreshu_docs/principles/security.md) нужен **прогрессивный lockout** — считаются только **неудачные** попытки входа, с нарастающим временем блокировки:
|
||||
|
||||
| Неудачных попыток | Блокировка |
|
||||
|-------------------|------------|
|
||||
| 5 за 15 мин | 15 минут |
|
||||
| 10 за 1 час | 1 час |
|
||||
| 20 за 24 часа | 24 часа |
|
||||
|
||||
Счётчики в Redis по ключу `lockout:<ip>`. При успешном логине — сброс счётчиков.
|
||||
|
||||
**Задача A2-1:**
|
||||
|
||||
1. **Создать `src/utils/loginLockout.ts`** — утилита для работы с Redis:
|
||||
- `checkBlocked(redis, ip: string)` → `{ blocked: boolean, retryAfter?: number }` — проверяет `lockout:blocked:<ip>`; если ключ есть и TTL > 0, возвращает `{ blocked: true, retryAfter }`.
|
||||
- `recordFailedAttempt(redis, ip: string)` — INCR по ключам `lockout:15m:<ip>`, `lockout:1h:<ip>`, `lockout:24h:<ip>` с TTL 15m/1h/24h; при достижении порогов (5/10/20) устанавливает `lockout:blocked:<ip>` с соответствующим TTL.
|
||||
- `clearOnSuccess(redis, ip: string)` — DEL всех ключей `lockout:*:<ip>` и `lockout:blocked:<ip>`.
|
||||
|
||||
2. **Обновить `src/plugins/rateLimit.ts`** — убрать `login` из `rateLimitOptions` (больше не используется для login). Остальные endpoints без изменений.
|
||||
|
||||
3. **Обновить `src/routes/auth.ts`** — для `POST /login`:
|
||||
- Убрать `config: { rateLimit: rateLimitOptions.login }`.
|
||||
- Добавить `preValidation`: вызвать `checkBlocked(app.redis, req.ip)`; если `blocked` — `reply.status(429).send({ error: { code: 'RATE_LIMIT_EXCEEDED', message: '...', retryAfter } })`.
|
||||
- В handler: обернуть `authService.login(...)` в try/catch; при успехе — `clearOnSuccess(app.redis, ip)`; при throw (например `unauthorized`) — `recordFailedAttempt(app.redis, ip)`, затем rethrow.
|
||||
|
||||
4. **Опционально** — вынести пороги (5, 10, 20) и окна в `env.ts` или оставить константами в `loginLockout.ts` (документация security.md указывает фиксированные значения).
|
||||
|
||||
**Коммит:** `feat: replace fixed login rate limit with progressive lockout`
|
||||
|
||||
**Итого:** 1 коммит. После — PR в `dev`.
|
||||
|
||||
---
|
||||
|
||||
## Agent A2-2: Fix Login Lockout Tests (после A2)
|
||||
|
||||
**Зависимости:** Agent A2 выполнен.
|
||||
|
||||
**Ветка:** `fix/login-lockout-test-mock`
|
||||
|
||||
**Проблема:** Auth routes используют `app.redis` для `checkBlocked`, `recordFailedAttempt`, `clearOnSuccess`. В `buildAuthTestApp` Redis не подключён — тесты `POST /auth/login` падают с 500.
|
||||
|
||||
**Задача A2-2:**
|
||||
|
||||
1. **Добавить mock Redis** в `tests/helpers/build-test-app.ts`:
|
||||
- Создать объект, реализующий минимальный интерфейс ioredis для loginLockout: `ttl`, `setex`, `del`, `eval`.
|
||||
- `ttl(key)` → -2 (ключ не существует) или -1/положительное значение для тестов.
|
||||
- `setex`, `del` → no-op или простая in-memory имитация.
|
||||
- `eval(script, keysCount, ...keys, ...args)` → вернуть `[0, 0, 0]` (счётчики не достигли порога).
|
||||
- `app.decorate('redis', mockRedis)` перед регистрацией auth routes.
|
||||
|
||||
2. **Обновить `rateLimitOptions`** в buildAuthTestApp — убрать `login` (его больше нет в типе из rateLimit.ts).
|
||||
|
||||
**Коммит:** `test: add mock Redis for auth integration tests with login lockout`
|
||||
|
||||
**Итого:** 1 коммит. Проверить: `npm run test -- tests/integration/auth.routes.test.ts` проходит.
|
||||
|
||||
---
|
||||
|
||||
## Agent B: Data Model & Drizzle Schema
|
||||
|
||||
**Зависимости:** Agent A (нужен database plugin). Может стартовать после A4.
|
||||
|
||||
@@ -22,7 +22,6 @@ const envSchema = z.object({
|
||||
LLM_TEMPERATURE: z.coerce.number().min(0).max(2).default(0.7),
|
||||
LLM_MAX_TOKENS: z.coerce.number().default(2048),
|
||||
|
||||
RATE_LIMIT_LOGIN: z.coerce.number().default(5),
|
||||
RATE_LIMIT_REGISTER: z.coerce.number().default(3),
|
||||
RATE_LIMIT_FORGOT_PASSWORD: z.coerce.number().default(3),
|
||||
RATE_LIMIT_VERIFY_EMAIL: z.coerce.number().default(5),
|
||||
|
||||
@@ -6,7 +6,6 @@ import { env } from '../config/env.js';
|
||||
declare module 'fastify' {
|
||||
interface FastifyInstance {
|
||||
rateLimitOptions: {
|
||||
login: { max: number; timeWindow: string };
|
||||
register: { max: number; timeWindow: string };
|
||||
forgotPassword: { max: number; timeWindow: string };
|
||||
verifyEmail: { max: number; timeWindow: string };
|
||||
@@ -18,7 +17,6 @@ declare module 'fastify' {
|
||||
|
||||
const rateLimitPlugin: FastifyPluginAsync = async (app: FastifyInstance) => {
|
||||
const options = {
|
||||
login: { max: env.RATE_LIMIT_LOGIN, timeWindow: '15 minutes' },
|
||||
register: { max: env.RATE_LIMIT_REGISTER, timeWindow: '1 hour' },
|
||||
forgotPassword: { max: env.RATE_LIMIT_FORGOT_PASSWORD, timeWindow: '1 hour' },
|
||||
verifyEmail: { max: env.RATE_LIMIT_VERIFY_EMAIL, timeWindow: '15 minutes' },
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import { AuthService } from '../services/auth/auth.service.js';
|
||||
import { checkBlocked, clearOnSuccess, recordFailedAttempt } from '../utils/loginLockout.js';
|
||||
|
||||
const registerSchema = {
|
||||
body: {
|
||||
@@ -89,20 +90,43 @@ export async function authRoutes(app: FastifyInstance) {
|
||||
|
||||
app.post(
|
||||
'/login',
|
||||
{ schema: loginSchema, config: { rateLimit: rateLimitOptions.login } },
|
||||
{
|
||||
schema: loginSchema,
|
||||
preValidation: async (req, reply) => {
|
||||
const ip = req.ip ?? 'unknown';
|
||||
const { blocked, retryAfter } = await checkBlocked(app.redis, ip);
|
||||
if (blocked) {
|
||||
if (retryAfter !== undefined) {
|
||||
reply.header('Retry-After', String(retryAfter));
|
||||
}
|
||||
return reply.status(429).send({
|
||||
error: {
|
||||
code: 'RATE_LIMIT_EXCEEDED',
|
||||
message: 'Too many failed login attempts. Please try again later.',
|
||||
retryAfter,
|
||||
},
|
||||
});
|
||||
}
|
||||
},
|
||||
},
|
||||
async (req, reply) => {
|
||||
const body = req.body as { email: string; password: string };
|
||||
const userAgent = req.headers['user-agent'];
|
||||
const ipAddress = req.ip;
|
||||
const ip = req.ip ?? 'unknown';
|
||||
|
||||
const result = await authService.login({
|
||||
email: body.email,
|
||||
password: body.password,
|
||||
userAgent,
|
||||
ipAddress,
|
||||
});
|
||||
|
||||
return reply.send(result);
|
||||
try {
|
||||
const result = await authService.login({
|
||||
email: body.email,
|
||||
password: body.password,
|
||||
userAgent,
|
||||
ipAddress: ip,
|
||||
});
|
||||
await clearOnSuccess(app.redis, ip);
|
||||
return reply.send(result);
|
||||
} catch (err) {
|
||||
await recordFailedAttempt(app.redis, ip);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
106
src/utils/loginLockout.ts
Normal file
106
src/utils/loginLockout.ts
Normal file
@@ -0,0 +1,106 @@
|
||||
import type { Redis } from 'ioredis';
|
||||
|
||||
const WINDOW_15M_SEC = 15 * 60; // 900
|
||||
const WINDOW_1H_SEC = 60 * 60; // 3600
|
||||
const WINDOW_24H_SEC = 24 * 60 * 60; // 86400
|
||||
|
||||
const THRESHOLD_15M = 5;
|
||||
const THRESHOLD_1H = 10;
|
||||
const THRESHOLD_24H = 20;
|
||||
|
||||
const BLOCK_15M_SEC = WINDOW_15M_SEC;
|
||||
const BLOCK_1H_SEC = WINDOW_1H_SEC;
|
||||
const BLOCK_24H_SEC = WINDOW_24H_SEC;
|
||||
|
||||
const KEY_PREFIX = 'lockout';
|
||||
|
||||
function key15m(ip: string): string {
|
||||
return `${KEY_PREFIX}:15m:${ip}`;
|
||||
}
|
||||
function key1h(ip: string): string {
|
||||
return `${KEY_PREFIX}:1h:${ip}`;
|
||||
}
|
||||
function key24h(ip: string): string {
|
||||
return `${KEY_PREFIX}:24h:${ip}`;
|
||||
}
|
||||
function keyBlocked(ip: string): string {
|
||||
return `${KEY_PREFIX}:blocked:${ip}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the IP is currently blocked due to progressive login lockout.
|
||||
* @returns { blocked: true, retryAfter } if blocked, { blocked: false } otherwise
|
||||
*/
|
||||
export async function checkBlocked(
|
||||
redis: Redis,
|
||||
ip: string
|
||||
): Promise<{ blocked: boolean; retryAfter?: number }> {
|
||||
const blockedKey = keyBlocked(ip);
|
||||
const ttl = await redis.ttl(blockedKey);
|
||||
if (ttl > 0) {
|
||||
return { blocked: true, retryAfter: ttl };
|
||||
}
|
||||
return { blocked: false };
|
||||
}
|
||||
|
||||
const RECORD_SCRIPT = `
|
||||
local c15 = redis.call('INCR', KEYS[1])
|
||||
if redis.call('TTL', KEYS[1]) == -1 then redis.call('EXPIRE', KEYS[1], ARGV[1]) end
|
||||
local c1h = redis.call('INCR', KEYS[2])
|
||||
if redis.call('TTL', KEYS[2]) == -1 then redis.call('EXPIRE', KEYS[2], ARGV[2]) end
|
||||
local c24 = redis.call('INCR', KEYS[3])
|
||||
if redis.call('TTL', KEYS[3]) == -1 then redis.call('EXPIRE', KEYS[3], ARGV[3]) end
|
||||
return {c15, c1h, c24}
|
||||
`;
|
||||
|
||||
/**
|
||||
* Record a failed login attempt. Increments counters and sets blocked key when thresholds are reached.
|
||||
* Thresholds: 5 in 15m -> 15m block; 10 in 1h -> 1h block; 20 in 24h -> 24h block.
|
||||
*/
|
||||
export async function recordFailedAttempt(redis: Redis, ip: string): Promise<void> {
|
||||
const k15 = key15m(ip);
|
||||
const k1h = key1h(ip);
|
||||
const k24 = key24h(ip);
|
||||
const kBlocked = keyBlocked(ip);
|
||||
|
||||
const counts = (await redis.eval(
|
||||
RECORD_SCRIPT,
|
||||
3,
|
||||
k15,
|
||||
k1h,
|
||||
k24,
|
||||
String(WINDOW_15M_SEC),
|
||||
String(WINDOW_1H_SEC),
|
||||
String(WINDOW_24H_SEC)
|
||||
)) as number[];
|
||||
|
||||
const count15m = counts[0] ?? 0;
|
||||
const count1h = counts[1] ?? 0;
|
||||
const count24h = counts[2] ?? 0;
|
||||
|
||||
let blockTtl = 0;
|
||||
if (count24h >= THRESHOLD_24H) {
|
||||
blockTtl = BLOCK_24H_SEC;
|
||||
} else if (count1h >= THRESHOLD_1H) {
|
||||
blockTtl = BLOCK_1H_SEC;
|
||||
} else if (count15m >= THRESHOLD_15M) {
|
||||
blockTtl = BLOCK_15M_SEC;
|
||||
}
|
||||
|
||||
if (blockTtl > 0) {
|
||||
await redis.setex(kBlocked, blockTtl, '1');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear all lockout counters and blocked state on successful login.
|
||||
*/
|
||||
export async function clearOnSuccess(redis: Redis, ip: string): Promise<void> {
|
||||
const keys = [
|
||||
key15m(ip),
|
||||
key1h(ip),
|
||||
key24h(ip),
|
||||
keyBlocked(ip),
|
||||
];
|
||||
await redis.del(...keys);
|
||||
}
|
||||
@@ -4,9 +4,29 @@ import { authRoutes } from '../../src/routes/auth.js';
|
||||
import type { MockDb } from '../test-utils.js';
|
||||
import { createMockDb } from '../test-utils.js';
|
||||
|
||||
/** Mock Redis for login lockout in auth tests. Implements ttl, setex, del, eval. */
|
||||
const mockRedis = {
|
||||
async ttl(_key: string): Promise<number> {
|
||||
return -2; // key does not exist -> not blocked
|
||||
},
|
||||
async setex(_key: string, _ttl: number, _value: string): Promise<'OK'> {
|
||||
return 'OK';
|
||||
},
|
||||
async del(..._keys: string[]): Promise<number> {
|
||||
return 0;
|
||||
},
|
||||
async eval(
|
||||
_script: string,
|
||||
_keysCount: number,
|
||||
..._keysAndArgs: string[]
|
||||
): Promise<number[]> {
|
||||
return [0, 0, 0]; // counters below threshold
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Build a minimal Fastify app for auth route integration tests.
|
||||
* Uses mock db and rate limit options (no actual rate limiting).
|
||||
* Uses mock db, mock Redis for login lockout, and rate limit options (no actual rate limiting).
|
||||
*/
|
||||
export async function buildAuthTestApp(mockDb?: MockDb): Promise<FastifyInstance> {
|
||||
const db = mockDb ?? createMockDb();
|
||||
@@ -31,8 +51,8 @@ export async function buildAuthTestApp(mockDb?: MockDb): Promise<FastifyInstance
|
||||
});
|
||||
|
||||
app.decorate('db', db);
|
||||
app.decorate('redis', mockRedis);
|
||||
app.decorate('rateLimitOptions', {
|
||||
login: { max: 100, timeWindow: '1 minute' },
|
||||
register: { max: 100, timeWindow: '1 hour' },
|
||||
forgotPassword: { max: 100, timeWindow: '1 hour' },
|
||||
verifyEmail: { max: 100, timeWindow: '15 minutes' },
|
||||
|
||||
Reference in New Issue
Block a user