diff --git a/backend/__tests__/services/taxReportService.test.js b/backend/__tests__/services/taxReportService.test.js index 12fc4ed6..50eb9fdd 100644 --- a/backend/__tests__/services/taxReportService.test.js +++ b/backend/__tests__/services/taxReportService.test.js @@ -371,7 +371,9 @@ describe('getTaxReport', () => { expect(out.summary).toMatchObject({ incomeNetMinor: 10000, incomeVatMinor: 770, incomeGrossMinor: 10770, costNetMinor: 0, costVatMinor: 0, costGrossMinor: 0, - resultNetMinor: 10000, resultGrossMinor: 10770, vatPayableMinor: 770, + resultNetMinor: 10000, resultGrossMinor: 10770, + // VAT registration unconfigured in the test DB → refuse to compute payable. + vatRegistrationConfigured: false, vatPayableMinor: null, }); }); @@ -433,7 +435,8 @@ describe('getTaxReport', () => { expect(out.summary).toMatchObject({ incomeNetMinor: 100000, incomeVatMinor: 7700, incomeGrossMinor: 107700, costNetMinor: 25000, costVatMinor: 1540, costGrossMinor: 26540, - resultNetMinor: 75000, resultGrossMinor: 81160, vatPayableMinor: 6160, + resultNetMinor: 75000, resultGrossMinor: 81160, + vatRegistrationConfigured: false, vatPayableMinor: null, }); }); diff --git a/backend/src/middleware/requireFeatureFlag.js b/backend/src/middleware/requireFeatureFlag.js index 339fd9b5..c2637dae 100644 --- a/backend/src/middleware/requireFeatureFlag.js +++ b/backend/src/middleware/requireFeatureFlag.js @@ -6,27 +6,50 @@ * hit must still be refused so a disabled feature is never actable. Mirrors the * truthy logic feature_flags uses everywhere (true | 1 | '1'). * + * Cached: the accounting area alone is 10+ gated endpoints and the dashboard + * polls several, so a per-request DB read is wasteful. Flags change rarely and + * only via `PUT /admin/feature-flags`, which calls invalidateFeatureFlagCache() + * — so a short TTL is belt-and-braces against any other mutation path. + * * Several route files (adminLedger, adminExpenses) predate this and define an * identical local `requireFlag`; new gates should import this instead. */ const { db } = require('../database/db'); +const TTL_MS = 10_000; +const cache = new Map(); // key -> { enabled, expires } + +function flagEnabledFromRow(row) { + return !!(row && (row.value === true || row.value === 1 || row.value === '1')); +} + +async function isFeatureEnabled(key) { + const now = Date.now(); + const hit = cache.get(key); + if (hit && hit.expires > now) return hit.enabled; + const row = await db('feature_flags').where({ key }).first(); + const enabled = flagEnabledFromRow(row); + cache.set(key, { enabled, expires: now + TTL_MS }); + return enabled; +} + +/** Clear the flag cache — call after any write to feature_flags. */ +function invalidateFeatureFlagCache() { + cache.clear(); +} + function requireFeatureFlag(key, code) { return async (req, res, next) => { try { - const row = await db('feature_flags').where({ key }).first(); - const enabled = row && (row.value === true || row.value === 1 || row.value === '1'); - if (!enabled) { - return res.status(403).json({ - error: `${key} feature is disabled`, - code: code || `${key.replace(/([a-z])([A-Z])/g, '$1_$2').toUpperCase()}_DISABLED`, - }); - } - return next(); + if (await isFeatureEnabled(key)) return next(); + return res.status(403).json({ + error: `${key} feature is disabled`, + code: code || `${key.replace(/([a-z])([A-Z])/g, '$1_$2').toUpperCase()}_DISABLED`, + }); } catch (err) { return next(err); } }; } -module.exports = { requireFeatureFlag }; +module.exports = { requireFeatureFlag, isFeatureEnabled, invalidateFeatureFlagCache }; diff --git a/backend/src/routes/adminFeatureFlags.js b/backend/src/routes/adminFeatureFlags.js index 6eda04b9..8c447f24 100644 --- a/backend/src/routes/adminFeatureFlags.js +++ b/backend/src/routes/adminFeatureFlags.js @@ -18,6 +18,7 @@ const router = express.Router(); const { db, logActivity } = require('../database/db'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); +const { invalidateFeatureFlagCache } = require('../middleware/requireFeatureFlag'); const logger = require('../utils/logger'); // Canonical flag list. Keep in sync with frontend @@ -226,6 +227,10 @@ router.put('/', adminAuth, requirePermission('settings.edit'), async (req, res) } }); + // Drop the requireFeatureFlag middleware's short-TTL cache so a toggle takes + // effect immediately instead of after ≤10s. + invalidateFeatureFlagCache(); + await logActivity( 'feature_flags_updated', { changed, actor: adminUsername }, diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index 350d5dc7..87040945 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -19,6 +19,7 @@ const { getRawPublicSiteSettings, } = require('../services/publicSiteService'); const { sanitizeCss } = require('../utils/cssSanitizer'); +const { upsertAppSetting } = require('../utils/appSettings'); const { clearShareLinkSettingsCache } = require('../services/shareLinkService'); const { resetSecurityConfigCache } = require('../utils/authSecurity'); const router = express.Router(); @@ -211,19 +212,7 @@ router.put('/customer-surface', adminAuth, requirePermission('settings.edit'), a } for (const u of updates) { - const existing = await db('app_settings').where('setting_key', u.setting_key).first(); - if (existing) { - await db('app_settings').where('setting_key', u.setting_key).update({ - setting_value: u.setting_value, - setting_type: u.setting_type, - updated_at: new Date(), - }); - } else { - // app_settings has no created_at column (see src/database/db.js: only - // setting_key/value/type + updated_at). Inserting created_at errors — - // which broke first-time keys like the VAT-registration toggle. - await db('app_settings').insert({ ...u, updated_at: new Date() }); - } + await upsertAppSetting(u.setting_key, u.setting_value, u.setting_type); } // Clear the public-site cache so any consumer relying on it @@ -282,17 +271,7 @@ router.put('/accounting', adminAuth, requirePermission('settings.edit'), async ( }); } for (const u of updates) { - const existing = await db('app_settings').where('setting_key', u.setting_key).first(); - if (existing) { - await db('app_settings').where('setting_key', u.setting_key).update({ - setting_value: u.setting_value, setting_type: u.setting_type, updated_at: new Date(), - }); - } else { - // app_settings has no created_at column (see src/database/db.js: only - // setting_key/value/type + updated_at). Inserting created_at errors — - // which broke first-time keys like the VAT-registration toggle. - await db('app_settings').insert({ ...u, updated_at: new Date() }); - } + await upsertAppSetting(u.setting_key, u.setting_value, u.setting_type); } res.json({ message: 'Accounting settings updated', updated: updates.map((u) => u.setting_key) }); } catch (error) { diff --git a/backend/src/routes/customer.js b/backend/src/routes/customer.js index f9ac8796..d2b827d9 100644 --- a/backend/src/routes/customer.js +++ b/backend/src/routes/customer.js @@ -24,6 +24,19 @@ const { customerAuth } = require('../middleware/customerAuth'); const { setGalleryAuthCookies } = require('../utils/tokenUtils'); const customerAccountsService = require('../services/customerAccountsService'); +// Gate a customer-facing route on BOTH the global master flag AND the +// per-customer override — getEffectiveFeaturesForCustomer combines them, so an +// admin disabling e.g. Bills globally is honoured even when feature_bills=true +// on the row. Sends the 403 and returns false on denial; true if allowed. +async function customerFeatureAllowed(req, res, featureKey, label) { + const eff = await customerAccountsService.getEffectiveFeaturesForCustomer(req.customer.id); + if (!eff || !eff[featureKey]) { + res.status(403).json({ error: `${label} are disabled for this account`, code: 'CUSTOMER_FEATURE_DISABLED' }); + return false; + } + return true; +} + /** * Customer-side password policy mirrors the one in customerAuth.js — kept * deliberately simple (8 chars, one uppercase, one digit) since a customer @@ -380,11 +393,8 @@ router.post('/profile/password', [ router.get('/quotes', customerAuth, async (req, res) => { try { const { db: dbi } = require('../database/db'); - // Customer-feature gate. is_active is enforced by customerAuth. - const customer = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!customer || customer.feature_quotes === false || customer.feature_quotes === 0) { - return res.status(403).json({ error: 'Quotes are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Customer-feature gate — master flag AND per-customer override. + if (!(await customerFeatureAllowed(req, res, 'quotes', 'Quotes'))) return; const rows = await dbi('quotes') .where({ customer_account_id: req.customer.id }) // Hide drafts — they're admin scratch work; nothing has been @@ -459,10 +469,8 @@ router.get('/quotes', customerAuth, async (req, res) => { router.get('/invoices', customerAuth, async (req, res) => { try { const { db: dbi } = require('../database/db'); - const customer = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!customer || customer.feature_bills === false || customer.feature_bills === 0) { - return res.status(403).json({ error: 'Invoices are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Customer-feature gate — master flag AND per-customer override. + if (!(await customerFeatureAllowed(req, res, 'bills', 'Invoices'))) return; // Visibility rules for the customer-facing list: // - Hide `scheduled` always (drafts the admin is still tweaking). // - Show `sent`, `overdue`, `paid` always (the customer's @@ -551,14 +559,8 @@ router.get('/invoices', customerAuth, async (req, res) => { router.get('/quotes/:id/pdf', customerAuth, async (req, res) => { try { const { db: dbi } = require('../database/db'); - // Feature-gate identically to /quotes (list endpoint). req.customer does - // NOT carry feature_* columns (customerAuth only selects identity), so we - // read the row here — the previous req.customer.feature_quotes check was a - // silent no-op (always undefined). - const account = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!account || account.feature_quotes === false || account.feature_quotes === 0) { - return res.status(403).json({ error: 'Quotes are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Feature-gate — master flag AND per-customer override. + if (!(await customerFeatureAllowed(req, res, 'quotes', 'Quotes'))) return; const quote = await dbi('quotes') .where({ id: parseInt(req.params.id, 10), customer_account_id: req.customer.id }) .first(); @@ -588,12 +590,8 @@ router.get('/quotes/:id/pdf', customerAuth, async (req, res) => { router.get('/invoices/:id/pdf', customerAuth, async (req, res) => { try { const { db: dbi } = require('../database/db'); - // Feature-gate identically to /invoices (list endpoint) — a direct hit must - // not download an invoice PDF when Bills is disabled for the account. - const account = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!account || account.feature_bills === false || account.feature_bills === 0) { - return res.status(403).json({ error: 'Invoices are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Feature-gate — master flag AND per-customer override. + if (!(await customerFeatureAllowed(req, res, 'bills', 'Invoices'))) return; const invoice = await dbi('invoices') .where({ id: parseInt(req.params.id, 10), customer_account_id: req.customer.id }) .first(); @@ -633,12 +631,8 @@ router.get('/contracts', customerAuth, async (req, res) => { // Feature not migrated on this install yet. return res.json({ contracts: [] }); } - // Per-customer contracts gate (migration 131) — mirrors /quotes + /invoices - // so a direct hit is refused when Contracts is off for the account. - const account = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!account || account.feature_contracts === false || account.feature_contracts === 0) { - return res.status(403).json({ error: 'Contracts are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Contracts gate — master flag AND per-customer override (migration 131). + if (!(await customerFeatureAllowed(req, res, 'contracts', 'Contracts'))) return; const rows = await dbi('contracts') .where({ customer_account_id: req.customer.id }) .whereNotIn('status', ['draft']) @@ -696,10 +690,8 @@ router.get('/contracts/:id/pdf', customerAuth, async (req, res) => { if (!(await dbi.schema.hasTable('contracts'))) { return res.status(404).json({ error: 'Contract not found' }); } - const account = await dbi('customer_accounts').where({ id: req.customer.id }).first(); - if (!account || account.feature_contracts === false || account.feature_contracts === 0) { - return res.status(403).json({ error: 'Contracts are disabled for this account', code: 'CUSTOMER_FEATURE_DISABLED' }); - } + // Contracts gate — master flag AND per-customer override. + if (!(await customerFeatureAllowed(req, res, 'contracts', 'Contracts'))) return; const contract = await dbi('contracts') .where({ id: parseInt(req.params.id, 10), customer_account_id: req.customer.id }) .first(); diff --git a/backend/src/services/expenseService.js b/backend/src/services/expenseService.js index fdff8ab6..d15b7cc8 100644 --- a/backend/src/services/expenseService.js +++ b/backend/src/services/expenseService.js @@ -153,7 +153,10 @@ async function recordInboundDocument({ source, filePath, originalFilename, mimeT status: duplicateOfId ? 'duplicate' : 'unsorted', parse_status: 'pending', parse_method: 'none', - page_count: pageCount, + // Cap stored page_count to the renderable max (rasterizeService + // MAX_RENDERABLE_PAGES) so a hostile high-page PDF can't drive an + // unbounded inbox pager (PR #622 concern 6). + page_count: pageCount != null ? Math.min(pageCount, 200) : null, duplicate_of_id: duplicateOfId, created_by_admin_id: adminId || null, created_at: now, diff --git a/backend/src/services/rasterizeService.js b/backend/src/services/rasterizeService.js index 77a84e8d..a7c8f3cb 100644 --- a/backend/src/services/rasterizeService.js +++ b/backend/src/services/rasterizeService.js @@ -22,6 +22,10 @@ const logger = require('../utils/logger'); const RENDER_TIMEOUT_MS = 25000; const RENDER_DPI = 150; +// Per-file resource bound (PR #622 concern 6): pages render one-per-request, so +// a 1000-page hostile PDF could otherwise be walked page-by-page. Refuse to +// render beyond this — the inbox pager is capped to match. +const MAX_RENDERABLE_PAGES = 200; function renderedDir(docId) { return path.join(getStoragePath(), 'business-docs', 'inbound', 'rendered', String(docId)); @@ -41,6 +45,9 @@ function execFileAsync(cmd, args, opts) { * @throws AppError 503 when pdftoppm is unavailable, 500 on render failure. */ async function getRenderedPagePath(docId, pdfPath, pageNum) { + if (!Number.isInteger(pageNum) || pageNum < 1 || pageNum > MAX_RENDERABLE_PAGES) { + throw new AppError(`Page out of range (1–${MAX_RENDERABLE_PAGES})`, 400, 'PAGE_OUT_OF_RANGE'); + } const dir = renderedDir(docId); const outPng = path.join(dir, `page-${pageNum}.png`); if (fs.existsSync(outPng)) return outPng; diff --git a/backend/src/services/taxReportService.js b/backend/src/services/taxReportService.js index c08c2ce6..31ebda84 100644 --- a/backend/src/services/taxReportService.js +++ b/backend/src/services/taxReportService.js @@ -502,10 +502,14 @@ async function getTaxReport({ from, to, currency, includeCosts = true } = {}) { // business doesn't file VAT (payable = 0); when registered it's output VAT // minus the RECLAIMABLE input VAT only (foreign non-reclaimable cost VAT is // not deducted). Guideline figure — verify with your Treuhänder. - let vatRegistered = await getVatRegisteredSetting(); - // Unset → preserve prior behaviour: if the business charged output VAT this - // period it's effectively registered; otherwise treat as small-business. - if (vatRegistered === null) vatRegistered = grandTotalVat > 0; + // PR #622 concern 4: when VAT registration is UNSET we must NOT guess from + // `grandTotalVat > 0` — a quarter with all-exempt cross-border sales has zero + // output VAT and would silently flip to "not registered", hiding the reclaim. + // Treat null as "not configured": refuse to compute a payable, surface a + // warning in the UI instead. + const vatRegisteredSetting = await getVatRegisteredSetting(); + const vatRegistrationConfigured = vatRegisteredSetting !== null; + const vatRegistered = vatRegisteredSetting === true; const reclaimableInputVat = costs.reclaimableVat != null ? costs.reclaimableVat : costs.totalVat; // Summary: income vs cost vs result. Result = a simplified @@ -520,7 +524,12 @@ async function getTaxReport({ from, to, currency, includeCosts = true } = {}) { resultNetMinor: grandTotalNet - costs.totalNet, resultGrossMinor: grandTotal - costs.totalGross, vatRegistered, - vatPayableMinor: vatRegistered ? (grandTotalVat - reclaimableInputVat) : 0, + vatRegistrationConfigured, + // null (not 0) when registration is unconfigured — the UI renders "—" + a + // "configure VAT registration" warning rather than a misleading number. + vatPayableMinor: !vatRegistrationConfigured + ? null + : (vatRegistered ? (grandTotalVat - reclaimableInputVat) : 0), }; // Unified ledger (#5 — one typed, signed, sortable list). Outgoing diff --git a/backend/src/utils/appSettings.js b/backend/src/utils/appSettings.js index 354eaa05..29a989d6 100644 --- a/backend/src/utils/appSettings.js +++ b/backend/src/utils/appSettings.js @@ -35,4 +35,22 @@ async function getAppSetting(key, defaultValue = null) { } } -module.exports = { getAppSetting }; +/** + * Schema-correct upsert into app_settings. The table has NO `created_at` + * column (only setting_key/setting_value/setting_type + updated_at — see + * src/database/db.js), so inserting created_at throws and silently breaks the + * FIRST save of any new key. Centralised here so route authors can't + * re-introduce that bug (PR #622 concern 5). `setting_value` is expected to be + * already JSON-stringified, matching getAppSetting's JSON.parse on read. + */ +async function upsertAppSetting(setting_key, setting_value, setting_type, conn = db) { + const existing = await conn('app_settings').where({ setting_key }).first(); + if (existing) { + await conn('app_settings').where({ setting_key }) + .update({ setting_value, setting_type, updated_at: new Date() }); + } else { + await conn('app_settings').insert({ setting_key, setting_value, setting_type, updated_at: new Date() }); + } +} + +module.exports = { getAppSetting, upsertAppSetting }; diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 6670a57a..f3f90e36 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -3974,7 +3974,8 @@ "income": "Einnahmen", "costs": "Ausgaben", "result": "Ergebnis", - "vatPayable": "MWST-Zahllast (Umsatz- − Vorsteuer)" + "vatPayable": "MWST-Zahllast (Umsatz- − Vorsteuer)", + "vatUnconfigured": "Die MWST-Registrierung ist nicht konfiguriert, daher kann die MWST-Zahllast nicht berechnet werden. Lege sie unter Einstellungen → Buchhaltung fest." }, "cost": { "source": "Art", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 5b4f48e2..86281af7 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -3974,7 +3974,8 @@ "income": "Income", "costs": "Costs", "result": "Result", - "vatPayable": "VAT payable (output − input)" + "vatPayable": "VAT payable (output − input)", + "vatUnconfigured": "VAT registration isn’t configured, so VAT payable can’t be computed. Set it under Settings → Accounting." }, "cost": { "source": "Type", diff --git a/frontend/src/pages/admin/clients/TaxReportPage.tsx b/frontend/src/pages/admin/clients/TaxReportPage.tsx index 51f02487..d74aca29 100644 --- a/frontend/src/pages/admin/clients/TaxReportPage.tsx +++ b/frontend/src/pages/admin/clients/TaxReportPage.tsx @@ -442,9 +442,17 @@ export const TaxReportPage: React.FC = () => {
{t('taxReport.summary.vatPayable', 'VAT payable (output − input)')} - {formatMinor(report.summary.vatPayableMinor, report.currency, intlLocale)} + {report.summary.vatRegistrationConfigured === false || report.summary.vatPayableMinor == null + ? '—' + : formatMinor(report.summary.vatPayableMinor, report.currency, intlLocale)}
+ {report.summary.vatRegistrationConfigured === false && ( +

+ + {t('taxReport.summary.vatUnconfigured', 'VAT registration isn’t configured, so VAT payable can’t be computed. Set it under Settings → Accounting.')} +

+ )} )} diff --git a/frontend/src/services/taxReport.service.ts b/frontend/src/services/taxReport.service.ts index eeffd28f..94eac99b 100644 --- a/frontend/src/services/taxReport.service.ts +++ b/frontend/src/services/taxReport.service.ts @@ -84,7 +84,12 @@ export interface TaxReportSummary { costGrossMinor: number; resultNetMinor: number; resultGrossMinor: number; - vatPayableMinor: number; + /** Whether `accounting_vat_registered` is configured. When false, the + * report refuses to guess and `vatPayableMinor` is null. */ + vatRegistrationConfigured?: boolean; + /** output VAT − reclaimable input VAT; `null` when VAT registration is + * unconfigured (the UI shows "—" + a warning instead of a guess). */ + vatPayableMinor: number | null; } /** A single row of the unified ledger (#5). Outgoing invoices carry