From 1ae36f7f98cdcf44007f99481e7542cf9f94ac84 Mon Sep 17 00:00:00 2001 From: Ho Ngoc Hai Date: Thu, 30 Apr 2026 15:35:13 +0700 Subject: [PATCH] fix(auth+web): unblock test accounts + public catalog routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated production blockers came up while exercising the live deploy: 1. Auth rate limit too aggressive (5 req/h) The throttler hit `429 Too Many Requests` after just five login attempts — testers (and the post-login refresh churn the SPA does on cold start) were locking themselves out almost immediately. - `auth.controller.ts`: `AUTH_RATE_LIMIT` and the per-IP login burst limit are now read from env vars (`AUTH_RATE_LIMIT`, `AUTH_PER_IP_LIMIT`), default 5 in production but easy to raise for staging without redeploying. Cluster ConfigMap now sets 200 / 100 respectively. - `throttler-behind-proxy.guard.ts`: added `shouldSkip()` that bypasses throttling entirely when the request body or JWT identifies a seed / demo account (admin + 10 seeded buyer / seller / agent / developer / park-operator phones). Also reads `THROTTLER_BYPASS_PHONES` and `_EMAILS` env vars so the ops team can temporarily allow-list a tester's number without code change. 2. `/khu-cong-nghiep` (and 6 other public catalog pages) redirected anonymous users to `/login` The Next.js middleware allow-list only covered `/login`, `/register`, `/search`, `/listings`, `/auth/callback`. Visiting the industrial parks catalog without a session sent users straight to a login wall — broken UX since the catalog is supposed to be public. Added these prefixes to `publicPaths`: /khu-cong-nghiep (industrial parks) /du-an (real estate projects) /chuyen-nhuong (property transfers) /bang-gia (pricing) /forgot-password /reset-password /about /contact /privacy /terms Verified live (https://platform.goodgo.vn after rollout): - 50 logins in a row with seed-admin → 50× 201, 0× 429 - Anonymous access: /khu-cong-nghiep, /du-an, /chuyen-nhuong, /search, /listings, /khu-cong-nghiep/thang-long → all 200 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../controllers/auth.controller.ts | 32 +++-- .../guards/throttler-behind-proxy.guard.ts | 117 +++++++++++++++++- apps/web/middleware.ts | 20 ++- 3 files changed, 157 insertions(+), 12 deletions(-) diff --git a/apps/api/src/modules/auth/presentation/controllers/auth.controller.ts b/apps/api/src/modules/auth/presentation/controllers/auth.controller.ts index 4779bba..cfcd3ab 100644 --- a/apps/api/src/modules/auth/presentation/controllers/auth.controller.ts +++ b/apps/api/src/modules/auth/presentation/controllers/auth.controller.ts @@ -62,7 +62,23 @@ import { RolesGuard } from '../guards/roles.guard'; const IS_PRODUCTION = process.env['NODE_ENV'] === 'production'; const IS_TEST = process.env['NODE_ENV'] === 'test'; -const AUTH_RATE_LIMIT = IS_TEST ? 10_000 : 5; +/** + * Hourly rate limit for auth endpoints. Default 5 is the production + * safety threshold; raise via env in dev/staging when exercising flows + * (e.g. `AUTH_RATE_LIMIT=200` in the cluster ConfigMap so testers + * don't lock themselves out after a few attempts). + */ +const AUTH_RATE_LIMIT = (() => { + if (IS_TEST) return 10_000; + const fromEnv = Number(process.env['AUTH_RATE_LIMIT']); + return Number.isFinite(fromEnv) && fromEnv > 0 ? fromEnv : 5; +})(); +/** Per-IP burst limit for the login / register endpoints (per minute). */ +const AUTH_PER_IP_LIMIT = (() => { + if (IS_TEST) return 10_000; + const fromEnv = Number(process.env['AUTH_PER_IP_LIMIT']); + return Number.isFinite(fromEnv) && fromEnv > 0 ? fromEnv : 5; +})(); const ACCESS_TOKEN_MAX_AGE = 15 * 60 * 1000; // 15 minutes const REFRESH_TOKEN_MAX_AGE = 30 * 24 * 60 * 60 * 1000; // 30 days const AUTH_COOKIE_MAX_AGE = 30 * 24 * 60 * 60 * 1000; // 30 days @@ -109,7 +125,7 @@ export class AuthController { ) {} @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 3, windowSeconds: 60, keyStrategy: 'ip' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'ip' }) @UseGuards(EndpointRateLimitGuard) @Post('register') @ApiOperation({ summary: 'Register a new user' }) @@ -132,7 +148,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 5, windowSeconds: 60, keyStrategy: 'ip' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'ip' }) @UseGuards(EndpointRateLimitGuard, LocalAuthGuard) @Post('login') @ApiOperation({ summary: 'Login with phone and password' }) @@ -198,7 +214,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 3, windowSeconds: 60, keyStrategy: 'ip' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'ip' }) @UseGuards(EndpointRateLimitGuard) @Post('forgot-password') @ApiOperation({ @@ -215,7 +231,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 5, windowSeconds: 60, keyStrategy: 'ip' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'ip' }) @UseGuards(EndpointRateLimitGuard) @Post('reset-password') @ApiOperation({ summary: 'Reset password using OTP code' }) @@ -231,7 +247,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: 20 } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 5, windowSeconds: 60, keyStrategy: 'ip' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'ip' }) @UseGuards(EndpointRateLimitGuard) @Post('exchange-token') @ApiOperation({ summary: 'Exchange OAuth token pair for httpOnly cookies' }) @@ -286,7 +302,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 5, windowSeconds: 60, keyStrategy: 'user' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'user' }) @UseGuards(JwtAuthGuard, EndpointRateLimitGuard) @Post('profile/verify-phone') @ApiBearerAuth('JWT') @@ -307,7 +323,7 @@ export class AuthController { } @Throttle({ default: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT }, auth: { ttl: 3_600_000, limit: AUTH_RATE_LIMIT } }) - @EndpointRateLimit({ limit: IS_TEST ? 10_000 : 5, windowSeconds: 60, keyStrategy: 'user' }) + @EndpointRateLimit({ limit: IS_TEST ? 10_000 : AUTH_PER_IP_LIMIT, windowSeconds: 60, keyStrategy: 'user' }) @UseGuards(JwtAuthGuard, EndpointRateLimitGuard) @Post('profile/verify-email') @ApiBearerAuth('JWT') diff --git a/apps/api/src/modules/shared/infrastructure/guards/throttler-behind-proxy.guard.ts b/apps/api/src/modules/shared/infrastructure/guards/throttler-behind-proxy.guard.ts index f643ff4..c952448 100644 --- a/apps/api/src/modules/shared/infrastructure/guards/throttler-behind-proxy.guard.ts +++ b/apps/api/src/modules/shared/infrastructure/guards/throttler-behind-proxy.guard.ts @@ -1,10 +1,83 @@ -import { Injectable } from '@nestjs/common'; +import { type ExecutionContext, Injectable } from '@nestjs/common'; import { ThrottlerGuard } from '@nestjs/throttler'; import { type Request } from 'express'; /** - * Extends ThrottlerGuard to extract real client IP behind reverse proxies - * (e.g., nginx, CloudFlare, AWS ALB) using X-Forwarded-For header. + * Phone numbers we use for demo / QA / E2E walkthroughs. Requests where + * the body / headers identify one of these accounts skip rate limiting + * entirely so testers (and automated UI tests) don't get blocked while + * they exercise login + flows repeatedly. + * + * Source-of-truth: the seed accounts in `prisma/seed.ts` and + * `prisma/seed-b2b-accounts.ts`. Prefix `+8487...` is the platform admin; + * the `+8490...` and `+8491...` ranges are the seed buyers / sellers / + * agents / developers / park-operators. + */ +const TEST_ACCOUNT_PHONES: ReadonlySet = new Set([ + '+84876677771', // admin + '+84900000002', + '+84900000003', + '+84900000004', + '+84900000005', + '+84900000006', + '+84900000007', + '+84900000008', + '+84912000001', + '+84912000002', + '+84912000003', +]); + +const TEST_ACCOUNT_EMAILS: ReadonlySet = new Set([ + 'hongochai10@icloud.com', + 'agent.nguyen@goodgo.vn', + 'agent.tran@goodgo.vn', + 'agent.le.hong@goodgo.vn', + 'buyer.le@gmail.com', + 'buyer.hoang@gmail.com', + 'seller.pham@gmail.com', + 'seller.vo@gmail.com', + 'cdt-vingroup@goodgo.vn', + 'cdt-masterise@goodgo.vn', + 'kcn-vsip@goodgo.vn', +]); + +/** + * Optional override: a comma-separated list in `THROTTLER_BYPASS_PHONES` + * (or `..._EMAILS`) is added to the static set above. Useful for + * temporarily whitelisting a tester's number without redeploying. + */ +function envSet(name: string): Set { + const raw = process.env[name]; + if (!raw) return new Set(); + return new Set( + raw + .split(',') + .map((s) => s.trim()) + .filter(Boolean), + ); +} + +const ALL_BYPASS_PHONES = new Set([ + ...TEST_ACCOUNT_PHONES, + ...envSet('THROTTLER_BYPASS_PHONES'), +]); +const ALL_BYPASS_EMAILS = new Set([ + ...TEST_ACCOUNT_EMAILS, + ...envSet('THROTTLER_BYPASS_EMAILS'), +]); + +interface AuthBody { + phone?: unknown; + email?: unknown; + identifier?: unknown; +} + +/** + * Extends ThrottlerGuard to: + * 1. Extract real client IP behind reverse proxies via X-Forwarded-For. + * 2. Skip rate limiting entirely for demo / QA accounts (matched by the + * body's `phone` / `email` / `identifier`, or by the authenticated + * JWT subject when present), so testers don't lock themselves out. */ @Injectable() export class ThrottlerBehindProxyGuard extends ThrottlerGuard { @@ -14,4 +87,42 @@ export class ThrottlerBehindProxyGuard extends ThrottlerGuard { typeof forwarded === 'string' ? (forwarded.split(',')[0]?.trim() ?? '127.0.0.1') : req.ip; return Promise.resolve(ip ?? '127.0.0.1'); } + + protected override shouldSkip(context: ExecutionContext): Promise { + const req = context.switchToHttp().getRequest< + Request & { user?: { phone?: string; email?: string; sub?: string } } + >(); + + // 1. Authenticated request — JWT payload phone/email tested first. + if (req.user) { + const phone = typeof req.user.phone === 'string' ? req.user.phone : undefined; + const email = typeof req.user.email === 'string' ? req.user.email : undefined; + if (phone && ALL_BYPASS_PHONES.has(phone)) return Promise.resolve(true); + if (email && ALL_BYPASS_EMAILS.has(email.toLowerCase())) return Promise.resolve(true); + } + + // 2. Login / register / password-reset bodies — extract from body fields + // that auth flows commonly use (`phone`, `email`, `identifier`). + const body = (req.body ?? {}) as AuthBody; + const phoneFromBody = + typeof body.phone === 'string' ? body.phone : undefined; + const identifierFromBody = + typeof body.identifier === 'string' ? body.identifier : undefined; + const emailFromBody = + typeof body.email === 'string' ? body.email : undefined; + + if (phoneFromBody && ALL_BYPASS_PHONES.has(phoneFromBody)) { + return Promise.resolve(true); + } + if (identifierFromBody) { + if (ALL_BYPASS_PHONES.has(identifierFromBody)) return Promise.resolve(true); + if (ALL_BYPASS_EMAILS.has(identifierFromBody.toLowerCase())) + return Promise.resolve(true); + } + if (emailFromBody && ALL_BYPASS_EMAILS.has(emailFromBody.toLowerCase())) { + return Promise.resolve(true); + } + + return Promise.resolve(false); + } } diff --git a/apps/web/middleware.ts b/apps/web/middleware.ts index 400a6e3..bcce95b 100644 --- a/apps/web/middleware.ts +++ b/apps/web/middleware.ts @@ -4,7 +4,25 @@ import { routing } from '@/i18n/routing'; const intlMiddleware = createIntlMiddleware(routing); -const publicPaths = ['/login', '/register', '/search', '/listings', '/auth/callback']; +// Anonymous users may visit these paths without being redirected to /login. +// Use prefixes — every nested route under each entry is also public. +const publicPaths = [ + '/login', + '/register', + '/auth/callback', + '/forgot-password', + '/reset-password', + '/search', + '/listings', + '/khu-cong-nghiep', // industrial parks catalog + detail + '/du-an', // projects (real estate developments) + '/chuyen-nhuong', // property transfers + '/bang-gia', // pricing + '/about', + '/contact', + '/privacy', + '/terms', +]; const publicExactPaths = ['/']; const authOnlyPaths = ['/login', '/register'];