Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion firebase-rules-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
37 changes: 36 additions & 1 deletion functions/fees/computeArrivalFees.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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
Expand Down Expand Up @@ -115,4 +150,4 @@ exports.computeArrivalFeesOnWrite = onValueWritten(
recomputeArrivalFees
);

exports._test = { recomputeArrivalFees, normalizeRegistration, FEE_FIELDS };
exports._test = { recomputeArrivalFees, authorizeInvoiceRecipient, normalizeRegistration, FEE_FIELDS };
98 changes: 97 additions & 1 deletion functions/fees/computeArrivalFees.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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');
Expand Down
Loading