fix(backend): shorten payment-check token TTL and notify admin on use
The unauthenticated payment-check magic link (intentional, matches
publicQuotes.js) had a 30-day token lifetime and wrote to the invoice
ledger silently. Shortened the TTL and added a best-effort admin
notification on every write via this route, so the no-login
convenience stays but an admin always sees the action happen.
(cherry picked from commit 262c3a4705)
This commit is contained in:
Paul Nothaft
@@ -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: '[email protected]', username: 'admin' };
|
||||
pickChainFor('business_profile')._firstValue = null;
|
||||
pickChainFor('customer_accounts')._firstValue = { id: 7, email: '[email protected]', 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('[email protected]');
|
||||
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: '[email protected]', username: 'admin' };
|
||||
pickChainFor('business_profile')._firstValue = null;
|
||||
pickChainFor('customer_accounts')._firstValue = { id: 7, email: '[email protected]', 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: '[email protected]', username: 'admin' };
|
||||
pickChainFor('business_profile')._firstValue = null;
|
||||
pickChainFor('customer_accounts')._firstValue = { id: 5, email: '[email protected]', 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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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: `<h2>Payment-check link used</h2>
|
||||
<p>Someone used the unauthenticated payment-check link for invoice <strong>{{invoice_number}}</strong>{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} and recorded: <strong>{{action}}</strong>{{#if has_amount}} ({{amount}}){{/if}}.</p>
|
||||
<table role="presentation" cellpadding="6" cellspacing="0" border="0" style="border-collapse: collapse; margin: 16px 0;">
|
||||
<tr><td style="color: #666;">Action</td><td><strong>{{action}}</strong></td></tr>
|
||||
{{#if has_amount}}<tr><td style="color: #666;">Amount</td><td>{{amount}}</td></tr>{{/if}}
|
||||
<tr><td style="color: #666;">IP address</td><td>{{ip}}</td></tr>
|
||||
<tr><td style="color: #666;">Recorded at</td><td>{{recorded_at}}</td></tr>
|
||||
</table>
|
||||
<p style="font-size: 13px; color: #666;">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.</p>`,
|
||||
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: `<h2>Zahlungsprüfungs-Link verwendet</h2>
|
||||
<p>Der nicht-authentifizierte Zahlungsprüfungs-Link für Rechnung <strong>{{invoice_number}}</strong>{{#if customer_name}} ({{customer_name}}){{/if}}{{#if event_name}}, {{event_name}}{{/if}} wurde verwendet und hat erfasst: <strong>{{action}}</strong>{{#if has_amount}} ({{amount}}){{/if}}.</p>
|
||||
<table role="presentation" cellpadding="6" cellspacing="0" border="0" style="border-collapse: collapse; margin: 16px 0;">
|
||||
<tr><td style="color: #666;">Aktion</td><td><strong>{{action}}</strong></td></tr>
|
||||
{{#if has_amount}}<tr><td style="color: #666;">Betrag</td><td>{{amount}}</td></tr>{{/if}}
|
||||
<tr><td style="color: #666;">IP-Adresse</td><td>{{ip}}</td></tr>
|
||||
<tr><td style="color: #666;">Erfasst am</td><td>{{recorded_at}}</td></tr>
|
||||
</table>
|
||||
<p style="font-size: 13px; color: #666;">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.</p>`,
|
||||
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'],
|
||||
|
||||
@@ -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, {
|
||||
|
||||
Reference in New Issue
Block a user