diff --git a/backend/__tests__/services/invoiceService.locks.test.js b/backend/__tests__/services/invoiceService.locks.test.js index 5ab507a8..414b30ba 100644 --- a/backend/__tests__/services/invoiceService.locks.test.js +++ b/backend/__tests__/services/invoiceService.locks.test.js @@ -102,6 +102,7 @@ jest.mock('../../src/utils/logger', () => ({ })); const invoiceService = require('../../src/services/invoiceService'); +const emailProcessor = require('../../src/services/emailProcessor'); function resetChains() { for (const k of Object.keys(tableChains)) delete tableChains[k]; @@ -261,7 +262,10 @@ describe('invoiceService.releaseForDelivery', () => { }); describe('invoiceService.recordPaymentCheckAction', () => { - beforeEach(() => resetChains()); + beforeEach(() => { + resetChains(); + emailProcessor.queueEmail.mockClear(); + }); it('rejects invalid actions', async () => { await expect(invoiceService.recordPaymentCheckAction({ @@ -321,6 +325,71 @@ describe('invoiceService.recordPaymentCheckAction', () => { token: 'a'.repeat(64), action: 'partial', amountMinor: 9999, })).rejects.toMatchObject({ statusCode: 400 }); }); + + // GHSA-wg94-f86h-vq68 hardening: every write via this unauthenticated + // route notifies the admin. Uses 'paid_full' as the exercised action — + // it stays inside markPaid (no workflow-engine / PDF-rendering + // dependencies to stub) while still going through the full + // recordPaymentCheckAction write path. + it('queues an admin notification email after a successful action', async () => { + pickChainFor('invoice_payment_check_tokens')._firstValue = { + id: 1, used_at: null, + expires_at: new Date(Date.now() + 86400000), + }; + pickChainFor('invoices')._firstValue = { + id: 5, invoice_number: 'INV-0005', status: 'overdue', + total_amount_minor: 10000, paid_amount_minor: 0, late_fee_amount_minor: 0, + customer_account_id: 7, created_by_admin_id: 42, + currency: 'CHF', language: 'de', event_id: null, + }; + pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' }; + pickChainFor('business_profile')._firstValue = null; + pickChainFor('customer_accounts')._firstValue = { id: 7, email: 'c@example.com', display_name: 'Test Customer' }; + + const result = await invoiceService.recordPaymentCheckAction({ + token: 'a'.repeat(64), action: 'paid_full', ip: '203.0.113.7', + }); + expect(result).toEqual({ applied: 'paid_full' }); + + expect(emailProcessor.queueEmail).toHaveBeenCalledTimes(1); + const [, recipientEmail, templateKey, data] = emailProcessor.queueEmail.mock.calls[0]; + expect(recipientEmail).toBe('admin@example.com'); + expect(templateKey).toBe('invoice_payment_check_action_recorded'); + expect(data.invoice_number).toBe('INV-0005'); + expect(data.action).toBe('paid_full'); + expect(data.ip).toBe('203.0.113.7'); + }); + + it('does not fail (or roll back) the ledger write when the admin notification fails to send', async () => { + pickChainFor('invoice_payment_check_tokens')._firstValue = { + id: 1, used_at: null, + expires_at: new Date(Date.now() + 86400000), + }; + pickChainFor('invoices')._firstValue = { + id: 5, invoice_number: 'INV-0005', status: 'overdue', + total_amount_minor: 10000, paid_amount_minor: 0, late_fee_amount_minor: 0, + customer_account_id: 7, created_by_admin_id: 42, + currency: 'CHF', language: 'de', event_id: null, + }; + pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' }; + pickChainFor('business_profile')._firstValue = null; + pickChainFor('customer_accounts')._firstValue = { id: 7, email: 'c@example.com', display_name: 'Test Customer' }; + emailProcessor.queueEmail.mockRejectedValueOnce(new Error('smtp down')); + + // The write itself (token consumption + markPaid) must still + // succeed — the notification is best-effort only. + const result = await invoiceService.recordPaymentCheckAction({ + token: 'a'.repeat(64), action: 'paid_full', ip: '203.0.113.7', + }); + expect(result).toEqual({ applied: 'paid_full' }); + + // Token was actually consumed (the real assertion that the write + // committed): the mock chain's .update() ran with used_at set. + const tokenChain = pickChainFor('invoice_payment_check_tokens'); + expect(tokenChain.update).toHaveBeenCalledWith( + expect.objectContaining({ used_at: expect.any(Date), used_action: 'paid_full' }), + ); + }); }); describe('invoiceService.queuePaymentCheckEmail', () => { @@ -371,4 +440,35 @@ describe('invoiceService.queuePaymentCheckEmail', () => { expect(res.sent).toBe(true); expect(res.token).toMatch(/^[a-f0-9]{64}$/); }); + + // GHSA-wg94-f86h-vq68 hardening: token TTL shortened from 30 days to 72h. + it('mints a token with a ~72h TTL, not the old 30-day window', async () => { + pickChainFor('invoices')._firstValue = { + id: 1, status: 'overdue', + customer_account_id: 5, + created_by_admin_id: 42, + total_amount_minor: 10000, + currency: 'CHF', + language: 'de', + reminder_level: 0, + due_date: '2026-05-01', + last_payment_check_at: null, + event_id: null, + }; + pickChainFor('admin_users')._firstValue = { id: 42, email: 'admin@example.com', username: 'admin' }; + pickChainFor('business_profile')._firstValue = null; + pickChainFor('customer_accounts')._firstValue = { id: 5, email: 'c@example.com', display_name: 'Test' }; + + const before = Date.now(); + const res = await invoiceService.queuePaymentCheckEmail(1); + expect(res.sent).toBe(true); + + const tokenChain = pickChainFor('invoice_payment_check_tokens'); + const insertedRow = tokenChain.insert.mock.calls[0][0]; + const ttlMs = new Date(insertedRow.expires_at).getTime() - before; + expect(ttlMs).toBeGreaterThan(71 * 60 * 60 * 1000); + expect(ttlMs).toBeLessThanOrEqual(72 * 60 * 60 * 1000 + 5000); + // Well under the old 30-day TTL — the actual regression guard. + expect(ttlMs).toBeLessThan(24 * 60 * 60 * 1000 * 30); + }); }); diff --git a/backend/src/services/crmEmailTemplates.js b/backend/src/services/crmEmailTemplates.js index 84572e24..bffaa01b 100644 --- a/backend/src/services/crmEmailTemplates.js +++ b/backend/src/services/crmEmailTemplates.js @@ -360,6 +360,56 @@ Rechnung {{invoice_number}} für {{customer_name}}{{#if event_name}} ({{event_na Automatische Benachrichtigung — keine Aktion erforderlich.`, }, }, + invoice_payment_check_action_recorded: { + // GHSA-wg94-f86h-vq68 hardening: the payment-check link at + // /payment-check/:token is unauthenticated by design (see + // publicPaymentCheck.js) — token possession is the only gate. + // This notifies the admin every time that link is used to write + // to the invoice ledger, so the no-login convenience stays but an + // admin always sees the action happen. + category: 'billing', feature_flag: 'bills', + variables: ['invoice_number', 'customer_name', 'event_name', 'action', 'has_amount', 'amount', 'ip', 'recorded_at'], + en: { + subject: 'Payment-check action recorded: invoice {{invoice_number}}', + body_html: `

Payment-check link used

+

Someone used the unauthenticated payment-check link for invoice {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} and recorded: {{action}}{{#if has_amount}} ({{amount}}){{/if}}.

+ + + {{#if has_amount}}{{/if}} + + +
Action{{action}}
Amount{{amount}}
IP address{{ip}}
Recorded at{{recorded_at}}
+

This link requires no login — only the token in the URL. If you don't recognise this action, review the invoice in the admin panel.

`, + body_text: `Payment-check link used + +Invoice {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} — recorded: {{action}}{{#if has_amount}} ({{amount}}){{/if}}. + + IP address: {{ip}} + Recorded at: {{recorded_at}} + +This link requires no login — only the token in the URL. If you don't recognise this action, review the invoice in the admin panel.`, + }, + de: { + subject: 'Zahlungsprüfung ausgelöst: Rechnung {{invoice_number}}', + body_html: `

Zahlungsprüfungs-Link verwendet

+

Der nicht-authentifizierte Zahlungsprüfungs-Link für Rechnung {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} wurde verwendet und hat erfasst: {{action}}{{#if has_amount}} ({{amount}}){{/if}}.

+ + + {{#if has_amount}}{{/if}} + + +
Aktion{{action}}
Betrag{{amount}}
IP-Adresse{{ip}}
Erfasst am{{recorded_at}}
+

Dieser Link erfordert kein Login — nur den Token in der URL. Falls Ihnen diese Aktion unbekannt vorkommt, prüfen Sie die Rechnung im Admin-Bereich.

`, + body_text: `Zahlungsprüfungs-Link verwendet + +Rechnung {{invoice_number}}{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} — erfasst: {{action}}{{#if has_amount}} ({{amount}}){{/if}}. + + IP-Adresse: {{ip}} + Erfasst am: {{recorded_at}} + +Dieser Link erfordert kein Login — nur den Token in der URL. Falls Ihnen diese Aktion unbekannt vorkommt, prüfen Sie die Rechnung im Admin-Bereich.`, + }, + }, invoice_collections_handoff: { category: 'billing', feature_flag: 'bills', variables: ['invoice_number', 'customer_name', 'customer_email', 'customer_address', 'event_name', 'original_amount', 'late_fee_amount', 'paid_amount', 'outstanding_amount', 'due_date', 'reminder_level'], diff --git a/backend/src/services/invoice/payments.js b/backend/src/services/invoice/payments.js index 05b6fdba..279507eb 100644 --- a/backend/src/services/invoice/payments.js +++ b/backend/src/services/invoice/payments.js @@ -12,6 +12,15 @@ const { ensureInt } = require('../../utils/numericHelpers'); const { formatMajor } = require('./helpers'); const { applyReminder, resolveAdminEmailForInvoice, resolvePerReminderFeeMinor, resolveSkontoPercentForInvoice } = require('./reminders'); +// Payment-check token lifetime (GHSA-wg94-f86h-vq68 hardening). This +// unauthenticated magic link is the only gate on a write to the +// invoice ledger, so it's kept short rather than the prior 30 days. +// The scheduler re-queues a fresh token daily (throttled by +// last_payment_check_at, see queuePaymentCheckEmail below) for as +// long as the invoice stays past its reminder cutoff, so a short TTL +// doesn't strand an admin who hasn't acted yet — they just get a new +// link on the next tick. +const PAYMENT_CHECK_TOKEN_TTL_MS = 72 * 60 * 60 * 1000; // 72h /** * Record a payment against an invoice. Supports partial payments @@ -226,7 +235,7 @@ async function queuePaymentCheckEmail(invoiceId, { skipThrottle = false } = {}) } const token = crypto.randomBytes(32).toString('hex'); - const expiresAt = new Date(now.getTime() + 30 * 24 * 60 * 60 * 1000); + const expiresAt = new Date(now.getTime() + PAYMENT_CHECK_TOKEN_TTL_MS); await db('invoice_payment_check_tokens').insert({ invoice_id: invoiceId, token, @@ -378,6 +387,45 @@ async function getPaymentCheckByToken(token) { }; } +/** + * Best-effort admin notification for every write via the public, + * unauthenticated payment-check route (GHSA-wg94-f86h-vq68 + * hardening). Token possession is the only gate on that route, so + * this fires on every successful action — 'paid_full', 'partial', + * 'unpaid', 'paid_with_skonto' — regardless of what the ledger + * effect ends up being, so an admin always sees the action happen. + * Callers MUST wrap this in try/catch: a failed send must never + * fail (or roll back) the ledger write it's reporting on. + */ +async function notifyAdminOfPaymentCheckAction({ invoice, action, amountMinor, ip }) { + const adminContact = await resolveAdminEmailForInvoice(invoice); + if (!adminContact?.email) { + logger.warn('Payment-check action notification skipped — no admin email resolved', + { invoiceId: invoice.id, action }); + return; + } + + const profile = await db('business_profile').where({ id: 1 }).first(); + const locale = invoice.language || profile?.default_locale || 'de'; + const customer = await db('customer_accounts').where({ id: invoice.customer_account_id }).first(); + + await emailProcessor.queueEmail(invoice.event_id || null, adminContact.email, + 'invoice_payment_check_action_recorded', { + invoice_number: invoice.invoice_number, + customer_name: customer?.company_name + || customer?.display_name + || [customer?.first_name, customer?.last_name].filter(Boolean).join(' ') + || customer?.email || '', + event_name: invoice.event_name || '', + __language: locale, + action, + amount: amountMinor ? formatMajor(ensureInt(amountMinor), invoice.currency, locale) : '', + has_amount: !!amountMinor, + ip: ip || 'unknown', + recorded_at: formatShortDate(new Date()), + }); +} + /** * Record the admin's payment-check action and fire the downstream * consequences: @@ -446,6 +494,19 @@ async function recordPaymentCheckAction({ token, action, amountMinor, ip, adminI adminId ? `admin:${adminId}` : 'public:payment-check'); } catch (_) {} + // Notify the admin this write happened. Best-effort / non-blocking + // — the ledger write above already committed, and a failed + // notification send must not undo or fail it. + if (!adminId) { + try { + await notifyAdminOfPaymentCheckAction({ invoice, action, amountMinor, ip }); + } catch (err) { + logger.warn('Payment-check action admin notification failed', { + invoiceId: invoice.id, action, err: err.message, + }); + } + } + // --- Apply the action ----------------------------------------- if (action === 'paid_full') { await markPaid(invoice.id, {