diff --git a/src/common/request-language.store.ts b/src/common/request-language.store.ts new file mode 100644 index 00000000..7dcb44bb --- /dev/null +++ b/src/common/request-language.store.ts @@ -0,0 +1,29 @@ +/** + * Request-scoped language context for the ValidationPipe exceptionFactory. + * + * ValidationPipe's exceptionFactory runs outside the normal filter/interceptor + * chain and has no access to the Express Request. Middleware stores the + * Accept-Language header (and optional user preference) in AsyncLocalStorage + * so the factory can pass them into I18nService.translate. + * + * Issue #1234 / #964. + */ +import { AsyncLocalStorage } from 'async_hooks'; + +export interface RequestLanguageContext { + acceptLanguageHeader?: string | null; + userPreference?: string | null; +} + +export const requestLanguageStore = new AsyncLocalStorage(); + +export function getRequestLanguageContext(): RequestLanguageContext { + return requestLanguageStore.getStore() ?? {}; +} + +export function runWithRequestLanguage( + ctx: RequestLanguageContext, + fn: () => T, +): T { + return requestLanguageStore.run(ctx, fn); +} diff --git a/src/email/email.service.spec.ts b/src/email/email.service.spec.ts index 1e5ab7b7..48be0202 100644 --- a/src/email/email.service.spec.ts +++ b/src/email/email.service.spec.ts @@ -1,10 +1,24 @@ -import { EmailService } from './email.service'; +import { EmailService, HARD_BOUNCE_SUPPRESSION_THRESHOLD } from './email.service'; import { ConfigService } from '@nestjs/config'; import { PrismaService } from '../database/prisma.service'; import { TrackingService } from '../tracking/tracking.service'; import { I18nService } from '../i18n/i18n.service'; import { Queue } from 'bullmq'; +function createService( + prisma: Partial, + configGet: (key: string, def?: string) => string | undefined = () => 'http://localhost:3000', + queueAdd: jest.Mock = jest.fn().mockResolvedValue(undefined), +) { + return new EmailService( + { get: jest.fn().mockImplementation(configGet) } as unknown as ConfigService, + prisma as unknown as PrismaService, + { createEmailEngagement: jest.fn() } as unknown as TrackingService, + { translate: jest.fn((key) => key) } as unknown as I18nService, + { add: queueAdd } as unknown as Queue, + ); +} + describe('EmailService.handleBounce', () => { it('disables email notifications on hard bounce', async () => { const prisma = { @@ -20,14 +34,7 @@ describe('EmailService.handleBounce', () => { }, }; - const service = new EmailService( - { get: jest.fn().mockReturnValue('http://localhost:3000/api') } as unknown as ConfigService, - prisma as unknown as PrismaService, - { createEmailEngagement: jest.fn() } as unknown as TrackingService, - { translate: jest.fn((key) => key) } as unknown as I18nService, - { add: jest.fn() } as unknown as Queue, - ); - + const service = createService(prisma as any); await service.handleBounce('test@example.com', 'HARD', 'Mailbox disabled', { id: 'evt-1', }); @@ -57,46 +64,90 @@ describe('EmailService.handleBounce', () => { }); }); - -describe('EmailService localization (issue #1231)', () => { - function buildService(i18nTranslate: jest.Mock) { +describe('EmailService bounce suppression (issue #1233)', () => { + it('shouldSuppressAddress is true when hard-bounce count meets threshold', async () => { const prisma = { - user: { findUnique: jest.fn().mockResolvedValue(null) }, + emailBounce: { + count: jest.fn().mockResolvedValue(HARD_BOUNCE_SUPPRESSION_THRESHOLD), + }, }; - const queue = { add: jest.fn().mockResolvedValue({ id: 'q1' }) }; - const service = new EmailService( - { get: jest.fn().mockReturnValue('http://localhost:3000/api') } as any, - prisma as any, - { createEmailEngagement: jest.fn() } as any, - { - translate: i18nTranslate, - tFor: i18nTranslate, - resolveLanguage: jest.fn().mockReturnValue('es'), - } as any, - queue as any, + const service = createService(prisma as any); + await expect(service.shouldSuppressAddress('bounced@example.com')).resolves.toBe(true); + expect(prisma.emailBounce.count).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + email: 'bounced@example.com', + bounceType: 'HARD', + }), + }), ); - return { service, queue }; - } + }); - it('injects context.t with Spanish strings when language=es', async () => { - const i18nTranslate = jest.fn((key: string) => { - if (key === 'email.password_reset_title') return 'Solicitud de restablecimiento de contraseña'; - if (key === 'email.password_reset_subject') return 'Restablecimiento de contraseña - PropChain'; - return key; + it('sendEmail skips queue when address is hard-bounced', async () => { + const queueAdd = jest.fn().mockResolvedValue(undefined); + const prisma = { + emailBounce: { + count: jest + .fn() + // first call: hard bounces >= threshold + .mockResolvedValueOnce(HARD_BOUNCE_SUPPRESSION_THRESHOLD) + .mockResolvedValue(0), + }, + user: { findUnique: jest.fn() }, + }; + const service = createService(prisma as any, () => 'https://app.example.com', queueAdd); + + await service.sendEmail({ + to: 'bounced@example.com', + subject: 'Hello', + text: 'body', }); - const { service, queue } = buildService(i18nTranslate); + + expect(queueAdd).not.toHaveBeenCalled(); + }); + + it('sendEmail queues when no recent hard bounces', async () => { + const queueAdd = jest.fn().mockResolvedValue(undefined); + const prisma = { + emailBounce: { + count: jest.fn().mockResolvedValue(0), + }, + user: { findUnique: jest.fn().mockResolvedValue(null) }, + }; + const service = createService(prisma as any, () => 'https://app.example.com', queueAdd); + await service.sendEmail({ - to: 'user@example.com', - subject: 'Password Reset - PropChain', - template: 'password-reset', - context: { resetUrl: 'https://example.com/reset' }, - language: 'es', + to: 'ok@example.com', + subject: 'Hello', + text: 'body', }); - expect(queue.add).toHaveBeenCalled(); - const payload = queue.add.mock.calls[0][1]; - expect(payload.context.t).toBeDefined(); - expect(payload.context.language).toBe('es'); - expect(payload.context.t.password_reset_title).toBe('Solicitud de restablecimiento de contraseña'); + + expect(queueAdd).toHaveBeenCalled(); }); }); +describe('EmailService unsubscribe URL (issue #1232)', () => { + it('buildListUnsubscribeHeader uses ConfigService FRONTEND_URL at call time', () => { + const prisma = { emailBounce: { count: jest.fn() } }; + const service = createService(prisma as any, (key: string) => + key === 'FRONTEND_URL' ? 'https://tenant-a.example.com' : undefined, + ); + const header = service.buildListUnsubscribeHeader('user-1', 'u@example.com'); + expect(header).toContain('https://tenant-a.example.com/unsubscribe?token='); + }); + + it('reflects a different FRONTEND_URL when config changes', () => { + const prisma = { emailBounce: { count: jest.fn() } }; + let frontend = 'https://first.example.com'; + const service = createService(prisma as any, (key: string) => + key === 'FRONTEND_URL' ? frontend : undefined, + ); + expect(service.buildListUnsubscribeHeader('u', 'a@b.com')).toContain( + 'https://first.example.com/unsubscribe', + ); + frontend = 'https://second.example.com'; + expect(service.buildListUnsubscribeHeader('u', 'a@b.com')).toContain( + 'https://second.example.com/unsubscribe', + ); + }); +}); diff --git a/src/email/email.service.ts b/src/email/email.service.ts index 4eda31cb..6f5e58d5 100644 --- a/src/email/email.service.ts +++ b/src/email/email.service.ts @@ -7,8 +7,12 @@ import { v4 as uuidv4 } from 'uuid'; import { InjectQueue } from '@nestjs/bullmq'; import { Queue } from 'bullmq'; import { redactEmail } from '../auth/security.utils'; +import { buildUnsubscribeUrl } from './unsubscribe-url.helper'; -const UNSUBSCRIBE_URL = process.env.FRONTEND_URL || 'http://localhost:3000'; +/** Number of hard bounces within the lookback window that triggers suppression. */ +export const HARD_BOUNCE_SUPPRESSION_THRESHOLD = 1; +/** Lookback window for hard-bounce suppression (ms). Default 90 days. */ +export const HARD_BOUNCE_LOOKBACK_MS = 90 * 24 * 60 * 60 * 1000; export interface EmailOptions { to: string; @@ -251,7 +255,42 @@ export class EmailService { buildListUnsubscribeHeader(userId?: string, email?: string): string | null { if (!userId || !email) return null; const token = Buffer.from(`${userId}:${email}`).toString('base64'); - return `<${UNSUBSCRIBE_URL}/unsubscribe?token=${token}>`; + // Read FRONTEND_URL at call time via ConfigService so tests and multi-tenant + // overrides are not stuck with a module-load snapshot (issue #1232). + const frontendUrl = this.configService.get('FRONTEND_URL'); + try { + const url = buildUnsubscribeUrl(token, frontendUrl); + return `<${url}>`; + } catch { + this.logger.warn('FRONTEND_URL not set; omitting List-Unsubscribe header'); + return null; + } + } + + /** + * Returns true when the address has recent hard bounces / complaints at or + * above HARD_BOUNCE_SUPPRESSION_THRESHOLD (issue #1233). + */ + async shouldSuppressAddress(email: string): Promise { + const since = new Date(Date.now() - HARD_BOUNCE_LOOKBACK_MS); + const hardCount = await this.prisma.emailBounce.count({ + where: { + email, + bounceType: 'HARD', + createdAt: { gte: since }, + }, + }); + if (hardCount >= HARD_BOUNCE_SUPPRESSION_THRESHOLD) { + return true; + } + const complaints = await this.prisma.emailBounce.count({ + where: { + email, + spamAction: 'COMPLAINED', + createdAt: { gte: since }, + }, + }); + return complaints >= HARD_BOUNCE_SUPPRESSION_THRESHOLD; } async sendEmail(options: EmailOptions): Promise { @@ -284,11 +323,19 @@ export class EmailService { } } - // 1. Check if user is blocked or has invalid email + // 0. Bounce / complaint suppression (issue #1233) + if (await this.shouldSuppressAddress(options.to)) { + this.logger.warn( + `🚫 Skipping email to ${redactEmail(options.to)} (hard-bounce or complaint suppression)`, + ); + return; + } + + // 1. Check if user is blocked or has invalid / bounced email if (options.userId) { const user = await this.prisma.user.findUnique({ where: { id: options.userId } }); - if (user && (user.isBlocked || user.emailStatus === 'INVALID')) { - this.logger.warn(`🚫 Skipping email to ${redactEmail(options.to)} (User blocked or email invalid)`); + if (user && (user.isBlocked || user.emailStatus === 'INVALID' || user.emailStatus === 'BOUNCED')) { + this.logger.warn(`🚫 Skipping email to ${redactEmail(options.to)} (User blocked or email invalid/bounced)`); return; } } diff --git a/src/i18n/i18n.service.spec.ts b/src/i18n/i18n.service.spec.ts index efe5701a..1ffca0a4 100644 --- a/src/i18n/i18n.service.spec.ts +++ b/src/i18n/i18n.service.spec.ts @@ -3,8 +3,9 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -function writeCatalogue(dir: string, lang: string, payload: Record): void { - fs.writeFileSync(path.join(dir, `${lang}.json`), JSON.stringify(payload)); +function writeCatalogue(dir: string, lang: string, payload: Record | string): void { + const body = typeof payload === 'string' ? payload : JSON.stringify(payload); + fs.writeFileSync(path.join(dir, `${lang}.json`), body); } describe('I18nService', () => { @@ -17,6 +18,7 @@ describe('I18nService', () => { common: { not_found: 'Resource not found' }, welcome: 'Hello {name}', fallback: 'English fallback', + nested: { a: { b: 'deep' }, list: ['x', 'y'] }, }); writeCatalogue(tmpDir, 'es', { common: { not_found: 'Recurso no encontrado' }, @@ -65,6 +67,11 @@ describe('I18nService', () => { expect(service.tFor('welcome', 'es', { name: 'Ada' })).toBe('Hola Ada'); }); + it('leaves placeholder when interpolation param is missing', () => { + expect(service.tFor('welcome', 'en', {})).toBe('Hello {name}'); + expect(service.tFor('welcome', 'en')).toBe('Hello {name}'); + }); + it('falls back to default language when key is missing', () => { expect(service.tFor('fallback', 'es')).toBe('English fallback'); }); @@ -73,6 +80,20 @@ describe('I18nService', () => { expect(service.tFor('not.a.key', 'en')).toBe('not.a.key'); }); + describe('nested lookup', () => { + it('resolves deep object paths', () => { + expect(service.tFor('nested.a.b', 'en')).toBe('deep'); + }); + + it('returns key when intermediate segment is an array (non-object)', () => { + expect(service.tFor('nested.list.0', 'en')).toBe('nested.list.0'); + }); + + it('returns key when intermediate is missing', () => { + expect(service.tFor('nested.missing.child', 'en')).toBe('nested.missing.child'); + }); + }); + describe('parseAcceptLanguage', () => { it('honours the highest-quality supported tag', () => { expect(service.parseAcceptLanguage('fr;q=0.9, es;q=1.0, en;q=0.5')).toBe('es'); @@ -84,6 +105,8 @@ describe('I18nService', () => { it('returns null for empty / all-unsupported headers', () => { expect(service.parseAcceptLanguage('')).toBeNull(); + expect(service.parseAcceptLanguage(null)).toBeNull(); + expect(service.parseAcceptLanguage(undefined)).toBeNull(); expect(service.parseAcceptLanguage('fr;q=0.8, de;q=0.6')).toBeNull(); }); @@ -91,18 +114,62 @@ describe('I18nService', () => { expect(service.parseAcceptLanguage('en-US')).toBe('en'); expect(service.parseAcceptLanguage('es-MX, en;q=0.8')).toBe('es'); }); + + it('ignores tags with q=0', () => { + expect(service.parseAcceptLanguage('es;q=0, en;q=0.5')).toBe('en'); + expect(service.parseAcceptLanguage('es;q=0')).toBeNull(); + }); + + it('handles wildcard * by falling through to next supported or null', () => { + expect(service.parseAcceptLanguage('*')).toBeNull(); + expect(service.parseAcceptLanguage('es, *;q=0.1')).toBe('es'); + }); + + it('sorts by q then by original order for equal q', () => { + expect(service.parseAcceptLanguage('en;q=0.5, es;q=0.5')).toBe('en'); + }); }); - describe('catalogue loading', () => { + describe('catalogue loading failure / fallback', () => { it('keeps functioning when a translation file is missing', () => { const missingDir = fs.mkdtempSync(path.join(os.tmpdir(), 'i18n-missing-')); writeCatalogue(missingDir, 'en', { common: { not_found: 'Missing EN' } }); const isolated = new I18nService(missingDir); isolated.onModuleInit(); expect(isolated.translate('common.not_found', {})).toBe('Missing EN'); - expect(isolated.tFor('common.not_found', 'es')).toBe('Missing EN'); // en fallback + expect(isolated.tFor('common.not_found', 'es')).toBe('Missing EN'); fs.rmSync(missingDir, { recursive: true, force: true }); }); + + it('installs empty catalogue and logs warning on malformed JSON', () => { + const badDir = fs.mkdtempSync(path.join(os.tmpdir(), 'i18n-bad-')); + writeCatalogue(badDir, 'en', { common: { not_found: 'OK EN' } }); + writeCatalogue(badDir, 'es', '{ this is not valid json'); + const isolated = new I18nService(badDir); + const warnSpy = jest.spyOn((isolated as any).logger, 'warn').mockImplementation(() => undefined); + isolated.onModuleInit(); + + expect(warnSpy).toHaveBeenCalled(); + const warnMsg = String(warnSpy.mock.calls[0]?.[0] ?? ''); + expect(warnMsg).toMatch(/Failed to load translations for "es"/); + + expect(isolated.tFor('common.not_found', 'es')).toBe('OK EN'); + expect(isolated.tFor('only.es.key', 'es')).toBe('only.es.key'); + expect(isolated.hasLanguage('es')).toBe(true); + + warnSpy.mockRestore(); + fs.rmSync(badDir, { recursive: true, force: true }); + }); + + it('degrades to key return when both catalogues fail to load', () => { + const emptyDir = fs.mkdtempSync(path.join(os.tmpdir(), 'i18n-empty-')); + const isolated = new I18nService(emptyDir); + jest.spyOn((isolated as any).logger, 'warn').mockImplementation(() => undefined); + isolated.onModuleInit(); + expect(isolated.tFor('any.key', 'en')).toBe('any.key'); + expect(isolated.tFor('any.key', 'es')).toBe('any.key'); + fs.rmSync(emptyDir, { recursive: true, force: true }); + }); }); }); diff --git a/src/i18n/i18n.service.ts b/src/i18n/i18n.service.ts index 1c70aeb0..74b96929 100644 --- a/src/i18n/i18n.service.ts +++ b/src/i18n/i18n.service.ts @@ -266,7 +266,8 @@ export class I18nService implements OnModuleInit { } } } - if (normalisedTag) { + // RFC 7231: q=0 means "not acceptable" — skip those tags. + if (normalisedTag && q > 0) { entries.push({ tag: normalisedTag, q, index }); } index += 1; @@ -274,6 +275,10 @@ export class I18nService implements OnModuleInit { entries.sort((a, b) => b.q - a.q || a.index - b.index); for (const entry of entries) { + // Wildcard * does not map to a concrete catalogue. + if (entry.tag === '*') { + continue; + } const direct = this.normalise(entry.tag); if (direct) { return direct; diff --git a/src/main.ts b/src/main.ts index 3bf20d81..3ea6256b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -87,9 +87,31 @@ async function bootstrap() { next(); }); - // Issue #964 – Localize validation error messages via the I18nService. + // Issue #964 / #1234 – Localize validation messages using the request's + // Accept-Language (and optional user preference) captured by middleware + // into AsyncLocalStorage. exceptionFactory has no Request; the store bridges it. const { I18nService } = await import('./i18n/i18n.service'); + const { getRequestLanguageContext, runWithRequestLanguage } = await import( + './common/request-language.store' + ); const i18n = app.get(I18nService); + + app.use((req: { headers: Record; user?: { languagePreference?: string | null } }, _res: unknown, next: () => void) => { + const accept = + typeof req.headers['accept-language'] === 'string' + ? req.headers['accept-language'] + : undefined; + const xLang = + typeof req.headers['x-language'] === 'string' ? req.headers['x-language'] : undefined; + runWithRequestLanguage( + { + acceptLanguageHeader: accept ?? null, + userPreference: req.user?.languagePreference ?? xLang ?? null, + }, + () => next(), + ); + }); + app.useGlobalPipes( new ValidationPipe({ whitelist: true, @@ -99,8 +121,12 @@ async function bootstrap() { const messages = (errors ?? []).flatMap((err) => Object.values((err as { constraints?: Record }).constraints ?? {}), ); + const langCtx = getRequestLanguageContext(); const translated = messages.map((message) => - i18n.translate(message, { acceptLanguageHeader: undefined }), + i18n.translate(message, { + acceptLanguageHeader: langCtx.acceptLanguageHeader, + userPreference: langCtx.userPreference, + }), ); return new BadRequestException( Array.isArray(translated) && translated.length > 0 ? translated : messages,