diff --git a/firebase-rules-template.json b/firebase-rules-template.json index 5e6e3d28..986de95f 100644 --- a/firebase-rules-template.json +++ b/firebase-rules-template.json @@ -218,7 +218,7 @@ "paymentMethod": { ".validate": "newData.hasChildren(['method'])", "method": { - ".validate": "newData.isString() && newData.val().length <= 50 && newData.val().length > 0" + ".validate": "newData.val() === 'card' || newData.val() === 'checkout' || newData.val() === 'cash' || newData.val() === 'card_external' || newData.val() === 'twint_external' || newData.val() === 'invoice'" }, "invoiceRecipientName": { ".validate": "newData.isString() && newData.val().length <= 200" diff --git a/functions/fees/computeArrivalFees.js b/functions/fees/computeArrivalFees.js index 5d514076..25f8bead 100644 --- a/functions/fees/computeArrivalFees.js +++ b/functions/fees/computeArrivalFees.js @@ -34,6 +34,37 @@ const normalizeRegistration = (immatriculation) => * the lspl pilot). Only strategies ported into functions/fees are honoured; an * unrecognised strategy fails closed (logs, writes nothing). */ +/** + * A pilot may only bill an invoice recipient they are authorized for — one + * whose configured `emails` include the arrival author's authenticated e-mail. + * The client only offers authorized recipients, but a direct database writer + * could name any recipient and misattribute the bill (SEC-03). On the trusted + * write path, clear an invoice recipient the author is not authorized for. + * + * `settings/invoiceRecipients` is an array of `{ name, emails }`, which the + * security rules cannot search — hence this server-side check. + */ +async function authorizeInvoiceRecipient(after, arrival, db) { + const pm = arrival.paymentMethod; + if (!pm || pm.method !== 'invoice' || !pm.invoiceRecipientName) { + return; + } + + const authEmail = arrival.createdBy; + const raw = (await db.ref('/settings/invoiceRecipients').once('value')).val(); + const recipients = Array.isArray(raw) ? raw : Object.values(raw || {}); + const authorized = authEmail + ? recipients + .filter(r => r && Array.isArray(r.emails) && r.emails.includes(authEmail)) + .map(r => r.name) + : []; + + if (!authorized.includes(pm.invoiceRecipientName)) { + await after.ref.child('paymentMethod/invoiceRecipientName').remove(); + logger.warn(`Cleared unauthorized invoiceRecipientName on arrival ${after.ref.key}`); + } +} + async function recomputeArrivalFees(event) { const after = event.data.after; if (!after.exists()) { @@ -51,6 +82,10 @@ async function recomputeArrivalFees(event) { return; // project not on server-owned fees } + // Authorize the declared invoice recipient (independent of the fee recompute, + // so the fee change-guard below can never skip it). + await authorizeInvoiceRecipient(after, arrival, db); + // MTOW / category: authoritative from the registry when the registration is // known; otherwise the pilot-submitted values, flagged for admin review. The // record's own mtow/aircraftCategory are left untouched (never silently @@ -115,4 +150,4 @@ exports.computeArrivalFeesOnWrite = onValueWritten( recomputeArrivalFees ); -exports._test = { recomputeArrivalFees, normalizeRegistration, FEE_FIELDS }; +exports._test = { recomputeArrivalFees, authorizeInvoiceRecipient, normalizeRegistration, FEE_FIELDS }; diff --git a/functions/fees/computeArrivalFees.spec.js b/functions/fees/computeArrivalFees.spec.js index b84694be..4b5d6e55 100644 --- a/functions/fees/computeArrivalFees.spec.js +++ b/functions/fees/computeArrivalFees.spec.js @@ -43,18 +43,41 @@ const makeDb = (data) => { // Event whose `after` snapshot carries the arrival and captures write-backs. const makeEvent = (arrival, key = 'arr1') => { const update = jest.fn().mockResolvedValue(); + const removed = []; return { _update: update, + _removed: removed, data: { after: { exists: () => arrival !== null, val: () => arrival, - ref: { key, update } + ref: { + key, + update, + child: (path) => ({ remove: () => { removed.push(path); return Promise.resolve(); } }) + } } } }; }; +// Minimal `after` for exercising authorizeInvoiceRecipient directly. +const makeAfter = (key = 'arr1') => { + const removed = []; + return { + _removed: removed, + ref: { + key, + child: (path) => ({ remove: () => { removed.push(path); return Promise.resolve(); } }) + } + }; +}; + +const RECIPIENTS = [ + { name: 'Club Alpha', emails: ['alpha@example.com'] }, + { name: 'Club Bravo', emails: ['bravo@example.com', 'shared@example.com'] }, +]; + describe('functions/fees/computeArrivalFees', () => { beforeEach(() => { jest.clearAllMocks(); @@ -176,6 +199,79 @@ describe('functions/fees/computeArrivalFees', () => { expect(mockLogger.error).toHaveBeenCalled(); }); + it('clears an unauthorized invoice recipient during recompute (integration)', async () => { + mockAdmin.database.mockReturnValue(makeDb({ + '/settings/landingFeesStrategy': 'lspl', + '/settings/invoiceRecipients': RECIPIENTS, + })); + const event = makeEvent({ + immatriculation: 'HBUNK', mtow: 1001, flightType: 'private', + aircraftCategory: 'Flugzeug', landingCount: 1, + createdBy: 'alpha@example.com', + paymentMethod: { method: 'invoice', invoiceRecipientName: 'Club Bravo' }, // not theirs + }); + await _test.recomputeArrivalFees(event); + expect(event._removed).toContain('paymentMethod/invoiceRecipientName'); + }); + + describe('authorizeInvoiceRecipient', () => { + const run = (arrival, data = { '/settings/invoiceRecipients': RECIPIENTS }) => { + const db = makeDb(data); + const after = makeAfter(); + return _test.authorizeInvoiceRecipient(after, arrival, db).then(() => after); + }; + + it('clears a recipient the author is not authorized for', async () => { + const after = await run({ + createdBy: 'alpha@example.com', + paymentMethod: { method: 'invoice', invoiceRecipientName: 'Club Bravo' }, + }); + expect(after._removed).toContain('paymentMethod/invoiceRecipientName'); + }); + + it('keeps a recipient the author is authorized for', async () => { + const after = await run({ + createdBy: 'alpha@example.com', + paymentMethod: { method: 'invoice', invoiceRecipientName: 'Club Alpha' }, + }); + expect(after._removed).toHaveLength(0); + }); + + it('clears any recipient when the arrival has no authenticated author', async () => { + const after = await run({ + paymentMethod: { method: 'invoice', invoiceRecipientName: 'Club Alpha' }, + }); + expect(after._removed).toContain('paymentMethod/invoiceRecipientName'); + }); + + it('ignores non-invoice payment methods', async () => { + const after = await run({ + createdBy: 'alpha@example.com', + paymentMethod: { method: 'cash' }, + }); + expect(after._removed).toHaveLength(0); + }); + + it('is a no-op when no invoice recipient is set', async () => { + const after = await run({ + createdBy: 'alpha@example.com', + paymentMethod: { method: 'invoice' }, + }); + expect(after._removed).toHaveLength(0); + }); + + it('handles recipients stored as an index-keyed object', async () => { + const after = await run( + { + createdBy: 'shared@example.com', + paymentMethod: { method: 'invoice', invoiceRecipientName: 'Club Bravo' }, + }, + { '/settings/invoiceRecipients': { 0: RECIPIENTS[0], 1: RECIPIENTS[1] } } + ); + expect(after._removed).toHaveLength(0); // shared@ is authorized for Bravo + }); + }); + describe('normalizeRegistration', () => { it('strips dashes/spaces and upper-cases', () => { expect(_test.normalizeRegistration('hb-abc')).toBe('HBABC');