feat(permissions): granular permission gating + role editor & presets (#747, phase 1 of #743) (#1045)

* feat(permissions): granular permission gating + role editor & presets

Make every admin feature permission-gateable so multi-user studios can
split capability across roles (#747, and phase 1 of #743).

- Split the catch-all settings.edit into dedicated dangerous-config perms
  (banking / domains / security / integrations / features): a team member
  can no longer change IBAN, domains, SSO, webhooks, API tokens or feature
  flags. Reads keep an OR with settings.view so existing roles keep
  visibility. The site-URL write inside /general is change-gated on
  settings.domains.
- Add dedicated perms for admin surfaces miscategorised under settings.*
  (whatsapp, event_types, image_security, notifications, system) plus
  roles.manage and vat_codes.view; gate the previously-ungated VAT read.
- Boot self-heal (_permissionsBoot.js): super_admin always holds every
  permission (tracks-all) so new perms never need a compensation
  migration; all other roles stay frozen (no silent escalation on upgrade).
- Seed two presets: Solo Photographer (full operator) and Team
  Photographer (contributor — view events + manage photos + read-only CRM;
  no settings/users/billing edits, no events.edit).
- Role editor: adminRoles CRUD (create/edit/clone/delete + permission
  matrix; system roles protected, super_admin immutable) and a Roles tab
  with a category-grouped matrix and preset cloning.
- Settings page tabs are permission-gated with snap-back; i18n en/de.

Migration 174. Backward-compatible: admin/editor/viewer unchanged.

* feat(permissions): hide in-page action buttons a role can't use

Wrap mutating controls on the surfaces restricted roles actually reach
(Events list, Archives, gallery photo grid, event detail) in
PermissionGate so they are HIDDEN when the user lacks the permission,
rather than shown-then-403:

- Events list: create / bulk archive / bulk delete / row archive /
  row delete / download-archive.
- Archives: restore / download / delete.
- Photo grid: single + bulk delete (photos.delete), per-photo download
  (photos.download), bulk move/hide/show (photos.edit).
- Event detail: edit / rename / publish (events.edit), duplicate
  (events.create), archive (events.archive), create-invoice
  (bills.manage); the Actions card is hidden entirely for view-only roles.
- Photos tab: upload / external import (photos.upload), export menu
  (photos.download).

Backend already enforces these with 403; this is the matching UX so a
Team Photographer never sees delete/settings controls.

* fix(permissions): close settings-split bypass via generic settings writers

Security review found the settings.edit split was bypassable: the generic
settings writers (/general, /analytics, /seo, /security) upsert arbitrary
setting_keys, so a role holding only settings.edit (or settings.security)
could write keys owned by a narrower permission — repointing the public
site URL (settings.domains), security policy (settings.security) or
VAT/accounting config (settings.banking) via the wrong endpoint.

Add stripUnauthorizedProtectedKeys(): before every generic upsert, drop
any protected key the caller isn't permitted to write (general_site_url →
settings.domains, security_* → settings.security, accounting_* →
settings.banking). Dedicated routes still work because their caller holds
the matching perm. Replaces the narrower in-handler site-URL guard.

Also fix two tests affected by the RBAC changes:
- authzPermissionGaps: API-token management moved to settings.integrations,
  so grant that (not settings.edit) to exercise the ownership 404.
- AdminPhotoGrid.viewToggle: stub PermissionGate (its buttons are now gated
  and the test renders without a PermissionsProvider).

* fix(permissions): address upstream review (#1045)

- Renumber migration 174 -> 175 (174 now taken by 174_sqlite_nullable_event_dates
  from #1035; the collision made picpeakImportService's forward-only restore
  guard treat both as order 174 and accept a newer .picpeak onto an older schema).
- Contain the roles.manage blast radius (delegation, not root escalation): a
  non-super_admin can no longer edit their own role, nor grant any permission
  their own role doesn't already hold (createRole + updateRole).
- Protected-key denial now 403s (naming the keys + required perms) instead of
  silently stripping and reporting "saved" (adminSettings generic writers).
- Reserve team_photographer so a custom role can't squat the preset name.
- Boot self-heal: per-step try/catch so a role_permissions insert race on one
  replica doesn't skip preset seeding.
- Forward-project the feature .manage perms that also replaced settings.edit
  gates (whatsapp/event_types/image_security/notifications/system), matching the
  settings.* split projection so the pattern is symmetric for phase-2.
- Guard exports.down's roles/admin_users queries with hasTable.

* fix(permissions): change-detection on protected-key 403 + commit guard tests (#1045)

Round-2 review:
- The protected-key 403 fired on key PRESENCE. The General tab re-posts
  general_site_url on every save, so a settings.edit-only role (the office
  manager this PR enables) got 403'd on every General save even when the URL
  was unchanged. Restore change-detection: compare the incoming value against
  the stored one and 403 only on an actual change; unchanged protected keys are
  dropped so the rest of the save proceeds. Only /general is affected.
- Commit the self-amplification guard test (was run locally, never staged):
  adminRolesGuards.test.js — non-super can't grant perms it lacks, can't edit
  its own role, can't escalate another role; super_admin bypasses;
  team_photographer name reserved.
- Add adminSettingsProtectedKeys.test.js pinning the change-detection: an
  unchanged general_site_url saves, an actual change 403s, super_admin changes it.
This commit is contained in:
Luca
2026-08-16 14:59:52 +02:00
committed by GitHub
parent 9b976386f9
commit b118695474
35 changed files with 2215 additions and 331 deletions
+3 -3
View File
@@ -18,7 +18,7 @@ const router = express.Router();
// List tokens for the current admin (or all, if super_admin) — without
// the plaintext, never recoverable after creation.
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/', adminAuth, requirePermission(['settings.view', 'settings.integrations']), async (req, res) => {
try {
// Scope to the caller's own tokens unless super_admin — the previous
// query returned every admin's token metadata (name/preview/scopes/
@@ -60,7 +60,7 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res)
router.post(
'/',
adminAuth,
requirePermission('settings.edit'),
requirePermission('settings.integrations'),
[
body('name').isString().trim().isLength({ min: 1, max: 100 }),
body('scopes').isArray({ min: 1 }).custom((arr) => {
@@ -112,7 +112,7 @@ router.post(
);
// Revoke a token (soft-delete; lookups still find it but reject).
router.delete('/:id', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.delete('/:id', adminAuth, requirePermission('settings.integrations'), async (req, res) => {
try {
const { id } = req.params;
const row = await db('api_tokens').where({ id }).first();
+9 -9
View File
@@ -216,7 +216,7 @@ router.use(adminAuth);
// ---- GET / ------------------------------------------------------------
router.get(
'/',
requirePermission('settings.view'),
requirePermission(['settings.view', 'settings.banking']),
handleAsync(async (req, res) => {
const { profile, bankAccounts } = await businessProfileService.getProfile();
return successResponse(res, {
@@ -235,7 +235,7 @@ router.get(
// an existing file. Read-only — never modifies anything.
router.get(
'/logo-diagnostic',
requirePermission('settings.view'),
requirePermission(['settings.view', 'settings.banking']),
handleAsync(async (req, res) => {
const fs = require('fs');
const path = require('path');
@@ -345,7 +345,7 @@ router.get(
// branding_logo_path still applies when this is unset.
router.post(
'/logo',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
pdfLogoUpload.single('logo'),
handleAsync(async (req, res) => {
if (!req.file) {
@@ -380,7 +380,7 @@ router.post(
router.delete(
'/logo',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
handleAsync(async (req, res) => {
const existing = await db('business_profile').where({ id: 1 }).first();
const prev = existing?.logo_path;
@@ -402,7 +402,7 @@ router.delete(
// ---- PUT / ------------------------------------------------------------
router.put(
'/',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
[
// All fields optional — partial update is fine. We only run shallow
// shape validation on the types that absolutely must be sane;
@@ -525,7 +525,7 @@ router.put(
// ---- bank accounts ----------------------------------------------------
router.get(
'/bank-accounts',
requirePermission('settings.view'),
requirePermission(['settings.view', 'settings.banking']),
handleAsync(async (req, res) => {
const { bankAccounts } = await businessProfileService.getProfile();
return successResponse(res, { bankAccounts: bankAccounts.map(transformBank) });
@@ -534,7 +534,7 @@ router.get(
router.post(
'/bank-accounts',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
[
body('iban').isString().isLength({ min: 5, max: 64 }).withMessage('IBAN is required')
.bail().custom(ibanValidator({ required: true })),
@@ -562,7 +562,7 @@ router.post(
router.put(
'/bank-accounts/:id',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
[
param('id').isInt({ min: 1 }),
body('iban').optional({ values: 'falsy' }).isString().isLength({ min: 5, max: 64 })
@@ -599,7 +599,7 @@ router.put(
router.delete(
'/bank-accounts/:id',
requirePermission('settings.edit'),
requirePermission('settings.banking'),
[param('id').isInt({ min: 1 })],
handleAsync(async (req, res) => {
validateRequest(req);
+6 -6
View File
@@ -19,7 +19,7 @@ const router = express.Router();
* GET /admin/event-types
* Get all event types (for admin management)
*/
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/', adminAuth, requirePermission(['settings.view', 'event_types.view', 'events.create', 'events.edit']), async (req, res) => {
try {
const includeInactive = req.query.includeInactive === 'true';
const eventTypes = await eventTypeService.getAllEventTypes({
@@ -51,7 +51,7 @@ router.get('/active', adminAuth, async (req, res) => {
* GET /admin/event-types/:id
* Get a single event type by ID
*/
router.get('/:id', adminAuth, requirePermission('settings.view'), [
router.get('/:id', adminAuth, requirePermission(['settings.view', 'event_types.view', 'events.create', 'events.edit']), [
param('id').isInt().withMessage('Invalid event type ID')
], async (req, res) => {
try {
@@ -78,7 +78,7 @@ router.get('/:id', adminAuth, requirePermission('settings.view'), [
* POST /admin/event-types
* Create a new event type
*/
router.post('/', adminAuth, requirePermission('settings.edit'), [
router.post('/', adminAuth, requirePermission('event_types.manage'), [
body('name').notEmpty().trim().withMessage('Name is required'),
body('slug_prefix')
.notEmpty()
@@ -138,7 +138,7 @@ router.post('/', adminAuth, requirePermission('settings.edit'), [
* PUT /admin/event-types/:id
* Update an event type
*/
router.put('/:id', adminAuth, requirePermission('settings.edit'), [
router.put('/:id', adminAuth, requirePermission('event_types.manage'), [
param('id').isInt().withMessage('Invalid event type ID'),
body('name').optional().notEmpty().trim().withMessage('Name cannot be empty'),
body('slug_prefix')
@@ -190,7 +190,7 @@ router.put('/:id', adminAuth, requirePermission('settings.edit'), [
* DELETE /admin/event-types/:id
* Delete an event type (only non-system types with no events)
*/
router.delete('/:id', adminAuth, requirePermission('settings.edit'), [
router.delete('/:id', adminAuth, requirePermission('event_types.manage'), [
param('id').isInt().withMessage('Invalid event type ID')
], async (req, res) => {
try {
@@ -228,7 +228,7 @@ router.delete('/:id', adminAuth, requirePermission('settings.edit'), [
* POST /admin/event-types/reorder
* Reorder event types by providing an array of IDs in the desired order
*/
router.post('/reorder', adminAuth, requirePermission('settings.edit'), [
router.post('/reorder', adminAuth, requirePermission('event_types.manage'), [
body('orderedIds').isArray().withMessage('orderedIds must be an array'),
body('orderedIds.*').isInt().withMessage('Each ID must be an integer')
], async (req, res) => {
+2 -2
View File
@@ -189,7 +189,7 @@ function applyDependencyRules(flags) {
return out;
}
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/', adminAuth, requirePermission(['settings.view', 'settings.features']), async (req, res) => {
try {
const flags = await readAllFlags();
// Always run the rules so derived flags (e.g. `clients`) and
@@ -202,7 +202,7 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res)
}
});
router.put('/', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/', adminAuth, requirePermission('settings.features'), async (req, res) => {
try {
const body = req.body || {};
if (typeof body !== 'object' || Array.isArray(body)) {
+8 -8
View File
@@ -10,7 +10,7 @@ const router = express.Router();
/**
* Get image security settings
*/
router.get('/settings', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/settings', adminAuth, requirePermission(['settings.view', 'image_security.view']), async (req, res) => {
try {
const settings = await db('app_settings')
.whereIn('setting_key', [
@@ -47,7 +47,7 @@ router.get('/settings', adminAuth, requirePermission('settings.view'), async (re
/**
* Update image security settings
*/
router.put('/settings', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/settings', adminAuth, requirePermission('image_security.manage'), async (req, res) => {
try {
const updates = req.body;
@@ -98,7 +98,7 @@ router.put('/settings', adminAuth, requirePermission('settings.edit'), async (re
/**
* Get security monitoring dashboard data
*/
router.get('/dashboard', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/dashboard', adminAuth, requirePermission(['settings.view', 'image_security.view']), async (req, res) => {
try {
const { timeframe = '24h' } = req.query;
@@ -203,7 +203,7 @@ router.get('/dashboard', adminAuth, requirePermission('settings.view'), async (r
/**
* Get detailed security logs
*/
router.get('/logs', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/logs', adminAuth, requirePermission(['settings.view', 'image_security.view']), async (req, res) => {
try {
const {
page = 1,
@@ -272,7 +272,7 @@ router.get('/logs', adminAuth, requirePermission('settings.view'), async (req, r
/**
* Get image access logs for a specific event
*/
router.get('/events/:eventId/access-logs', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/events/:eventId/access-logs', adminAuth, requirePermission(['settings.view', 'image_security.view']), async (req, res) => {
try {
const { eventId } = req.params;
const { page = 1, limit = 50 } = req.query;
@@ -322,7 +322,7 @@ router.get('/events/:eventId/access-logs', adminAuth, requirePermission('setting
/**
* Block/unblock suspicious IPs
*/
router.post('/block-ip', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.post('/block-ip', adminAuth, requirePermission('image_security.manage'), async (req, res) => {
try {
const { ip, action = 'block' } = req.body;
@@ -368,7 +368,7 @@ router.post('/block-ip', adminAuth, requirePermission('settings.edit'), async (r
/**
* Clear security logs older than specified time
*/
router.delete('/logs/cleanup', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.delete('/logs/cleanup', adminAuth, requirePermission('image_security.manage'), async (req, res) => {
try {
const { olderThan = '30d' } = req.body;
@@ -425,7 +425,7 @@ router.delete('/logs/cleanup', adminAuth, requirePermission('settings.edit'), as
/**
* Export security data for analysis
*/
router.get('/export', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/export', adminAuth, requirePermission(['settings.view', 'image_security.view']), async (req, res) => {
try {
const { format = 'json', timeframe = '7d' } = req.query;
+4 -4
View File
@@ -6,7 +6,7 @@ const logger = require('../utils/logger');
const router = express.Router();
// Get notifications (unread activity logs)
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/', adminAuth, requirePermission(['settings.view', 'notifications.view']), async (req, res) => {
try {
const { limit = 20, includeRead = false } = req.query;
@@ -66,7 +66,7 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res)
});
// Mark notification as read
router.put('/:id/read', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/:id/read', adminAuth, requirePermission('notifications.manage'), async (req, res) => {
try {
const { id } = req.params;
@@ -84,7 +84,7 @@ router.put('/:id/read', adminAuth, requirePermission('settings.edit'), async (re
});
// Mark all notifications as read
router.put('/read-all', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/read-all', adminAuth, requirePermission('notifications.manage'), async (req, res) => {
try {
await db('activity_logs')
.whereNull('read_at')
@@ -108,7 +108,7 @@ router.put('/read-all', adminAuth, requirePermission('settings.edit'), async (re
// nothing matched the date filter, so it was effectively a confusingly
// named Clear All anyway. Drop the rename and the branching, return
// the simple deletedCount the existing test (and frontend toast) expect.
router.delete('/clear-all', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.delete('/clear-all', adminAuth, requirePermission('notifications.manage'), async (req, res) => {
try {
const deletedCount = await db('activity_logs').delete();
res.json({ message: 'All notifications cleared', deletedCount });
+125
View File
@@ -0,0 +1,125 @@
/**
* Role editor API — create/edit/delete roles and their permission sets, and
* read the permission catalog for the matrix UI.
*
* Mounted at /api/admin/roles. Every mutation is gated by the highly-privileged
* `roles.manage` permission (held by super_admin + the solo_photographer preset,
* and grantable to a custom role by the owner). Reads allow `users.view` too so
* the User Management page can show role details without role-editing rights.
*
* System roles (super_admin/admin/editor/viewer/solo_photographer/
* team_photographer) are protected in the service layer: they can't be deleted
* or renamed, and super_admin's permission set is immutable (the boot self-heal
* keeps it complete regardless).
*
* See project_permission_gating.
*/
const express = require('express');
const { body, param } = require('express-validator');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { handleAsync, validateRequest, successResponse } = require('../utils/routeHelpers');
const userManagementService = require('../services/userManagementService');
const router = express.Router();
const READ_ROLES = ['users.view', 'roles.manage'];
function transformRole(role) {
return {
id: role.id,
name: role.name,
displayName: role.display_name,
description: role.description,
isSystem: role.is_system === true || role.is_system === 1,
priority: role.priority,
userCount: role.user_count,
permissions: role.permissions || [],
};
}
// GET / — list roles with their permissions + user counts
router.get('/', adminAuth, requirePermission(READ_ROLES), handleAsync(async (req, res) => {
const roles = await userManagementService.getRolesWithPermissions();
return successResponse(res, { roles: roles.map(transformRole) });
}));
// GET /permissions — the full permission catalog (for the matrix)
router.get('/permissions', adminAuth, requirePermission(READ_ROLES), handleAsync(async (req, res) => {
const permissions = await userManagementService.getPermissionCatalog();
return successResponse(res, { permissions });
}));
// POST / — create a custom role
router.post('/', [
adminAuth,
requirePermission('roles.manage'),
body('name').isString().trim().isLength({ min: 2, max: 49 }),
body('displayName').optional().isString().trim().isLength({ min: 1, max: 150 }),
body('description').optional({ nullable: true }).isString().isLength({ max: 500 }),
body('priority').optional().isInt({ min: 0, max: 99 }),
body('permissions').optional().isArray(),
body('permissions.*').optional().isString(),
], handleAsync(async (req, res) => {
validateRequest(req);
const role = await userManagementService.createRole({
name: req.body.name,
displayName: req.body.displayName,
description: req.body.description,
priority: req.body.priority,
permissions: req.body.permissions || [],
}, req.admin.id);
return successResponse(res, { role: transformRole(role) }, 201);
}));
// POST /:id/clone — clone a role (e.g. start from a preset)
router.post('/:id/clone', [
adminAuth,
requirePermission('roles.manage'),
param('id').isInt(),
body('name').isString().trim().isLength({ min: 2, max: 49 }),
body('displayName').optional().isString().trim().isLength({ min: 1, max: 150 }),
body('description').optional({ nullable: true }).isString().isLength({ max: 500 }),
], handleAsync(async (req, res) => {
validateRequest(req);
const role = await userManagementService.cloneRole(Number(req.params.id), {
name: req.body.name,
displayName: req.body.displayName,
description: req.body.description,
}, req.admin.id);
return successResponse(res, { role: transformRole(role) }, 201);
}));
// PUT /:id — update display/description/priority and/or permission set
router.put('/:id', [
adminAuth,
requirePermission('roles.manage'),
param('id').isInt(),
body('displayName').optional().isString().trim().isLength({ min: 1, max: 150 }),
body('description').optional({ nullable: true }).isString().isLength({ max: 500 }),
body('priority').optional().isInt({ min: 0, max: 99 }),
body('permissions').optional().isArray(),
body('permissions.*').optional().isString(),
], handleAsync(async (req, res) => {
validateRequest(req);
const role = await userManagementService.updateRole(Number(req.params.id), {
displayName: req.body.displayName,
description: req.body.description,
priority: req.body.priority,
permissions: req.body.permissions,
}, req.admin.id);
return successResponse(res, { role: transformRole(role) });
}));
// DELETE /:id — delete a custom role
router.delete('/:id', [
adminAuth,
requirePermission('roles.manage'),
param('id').isInt(),
], handleAsync(async (req, res) => {
validateRequest(req);
await userManagementService.deleteRole(Number(req.params.id), req.admin.id);
return successResponse(res, { success: true });
}));
module.exports = router;
+76 -7
View File
@@ -6,7 +6,7 @@ const { body, validationResult } = require('express-validator');
const { db, logActivity } = require('../database/db');
const { formatBoolean } = require('../utils/dbCompat');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { requirePermission, userHasAnyPermission } = require('../middleware/permissions');
const { clearMaintenanceCache } = require('../middleware/maintenance');
const { clearSettingsCache } = require('../services/rateLimitService');
const {
@@ -65,6 +65,63 @@ const stripReservedSettingKeys = (settings) => {
return settings;
};
// Migration 174 hardening — per-key permission boundary for the GENERIC settings
// writers. /general, /analytics, /seo and /security all upsert arbitrary
// setting_keys, so without this a role holding only the broad `settings.edit`
// (or `settings.security`) could set keys owned by a NARROWER permission —
// repointing the public site URL, security policy, or VAT/accounting config —
// via the wrong endpoint, defeating the settings.edit split. Any protected key
// the caller isn't permitted to write is stripped before the upsert. The
// dedicated routes still work because their caller holds the matching perm
// (e.g. PUT /accounting is gated by settings.banking, so accounting_* survives).
const PROTECTED_SETTING_KEY_PERMS = [
{ match: (k) => k === 'general_site_url', perm: 'settings.domains' },
{ match: (k) => k.startsWith('security_'), perm: 'settings.security' },
{ match: (k) => k.startsWith('accounting_'), perm: 'settings.banking' },
];
// Returns the list of {key, perm} the caller tried to CHANGE without the owning
// permission. Callers 403 when it's non-empty rather than silently no-op'ing a
// permission boundary. Change-detection matters: the General tab re-posts
// general_site_url on every save, so a no-op round-trip of the stored value must
// not 403 an otherwise-safe settings.edit save (the office-manager role this PR
// exists to enable) — only an actual change is rejected. A denied key the caller
// couldn't change is left in `settings` (the request 403s before the upsert); an
// unauthorized no-op is dropped so the rest of the save proceeds.
const collectUnauthorizedProtectedKeys = async (settings, adminId) => {
const denied = [];
for (const key of Object.keys(settings)) {
const rule = PROTECTED_SETTING_KEY_PERMS.find((r) => r.match(key));
if (!rule) continue;
if (await userHasAnyPermission(adminId, [rule.perm])) continue;
const row = await db('app_settings').where({ setting_key: key }).first();
let stored = null;
if (row) {
try { stored = JSON.parse(row.setting_value); } catch (_) { stored = row.setting_value; }
}
if (String(stored ?? '') === String(settings[key] ?? '')) {
delete settings[key]; // unchanged — let the rest of the save through
continue;
}
denied.push({ key, perm: rule.perm });
}
return denied;
};
// Express helper: 403 (naming the keys + required perms) when the caller tried
// to write a protected key they don't hold; returns true if the request was
// rejected so the route can stop.
const rejectUnauthorizedProtectedKeys = async (settings, req, res) => {
const denied = await collectUnauthorizedProtectedKeys(settings, req.admin.id);
if (denied.length > 0) {
res.status(403).json({
error: `You don't have permission to change: ${denied.map((d) => d.key).join(', ')}`,
code: 'FORBIDDEN',
keys: denied,
});
return true;
}
return false;
};
// Configure multer for logo uploads
const storage = multer.diskStorage({
destination: async (req, file, cb) => {
@@ -288,7 +345,8 @@ router.put('/customer-surface', adminAuth, requirePermission('settings.edit'), a
// Accounting settings (km rate, per-diem rate, require-proof). Read via the
// generic GET /:type ('accounting'); this is the typed write. Rates are
// integer minor units; verify legal/tax guidance with a Treuhaender.
router.put('/accounting', adminAuth, requirePermission('settings.edit'), async (req, res) => {
// Migration 174: VAT/accounting config is money-adjacent → settings.banking.
router.put('/accounting', adminAuth, requirePermission('settings.banking'), async (req, res) => {
try {
const updates = [];
const setInt = (key) => {
@@ -577,7 +635,8 @@ router.put('/downloads', adminAuth, requirePermission('settings.edit'), async (r
// Read the SSO config. The secret is redacted to a set/unset flag; the
// computed redirect URI is included for copy-paste into the IdP client.
router.get('/sso', adminAuth, requirePermission('settings.view'), async (req, res) => {
// Migration 174: SSO/OIDC + security config → settings.security.
router.get('/sso', adminAuth, requirePermission(['settings.view', 'settings.security']), async (req, res) => {
try {
const oidcService = require('../services/oidcService');
const cfg = await oidcService.getOidcConfig();
@@ -610,7 +669,7 @@ router.get('/sso', adminAuth, requirePermission('settings.view'), async (req, re
}
});
router.put('/sso', adminAuth, requirePermission('settings.edit'), [
router.put('/sso', adminAuth, requirePermission('settings.security'), [
body('oidc_enabled').optional().isBoolean(),
body('oidc_issuer_url').optional({ checkFalsy: true }).isURL({ protocols: ['http', 'https'], require_tld: false }),
body('oidc_client_id').optional().isString().trim(),
@@ -717,7 +776,7 @@ router.put('/sso', adminAuth, requirePermission('settings.edit'), [
// Server-side discovery probe: confirms the issuer is reachable and speaks
// OIDC before the admin flips the enable toggle. Uses the SAVED config.
router.post('/sso/test', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.post('/sso/test', adminAuth, requirePermission('settings.security'), async (req, res) => {
try {
const oidcService = require('../services/oidcService');
const cfg = await oidcService.getOidcConfig();
@@ -1300,6 +1359,12 @@ router.put('/general', adminAuth, requirePermission('settings.edit'), async (req
const settings = stripReservedSettingKeys({ ...req.body });
let uploadLimitTouched = false;
// Migration 174: drop any protected key (site URL / security / accounting)
// the caller isn't permitted to write, so the settings.edit bucket can't be
// used to repoint the install via this generic writer. See
// rejectUnauthorizedProtectedKeys (403s when a protected key is denied).
if (await rejectUnauthorizedProtectedKeys(settings, req, res)) return;
const publicSiteKeysTouched = Object.keys(settings).some((key) => key.startsWith('general_public_site_'));
if (Object.prototype.hasOwnProperty.call(settings, 'general_max_files_per_upload')) {
@@ -1425,9 +1490,11 @@ router.put('/general', adminAuth, requirePermission('settings.edit'), async (req
});
// Update security settings
router.put('/security', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/security', adminAuth, requirePermission('settings.security'), async (req, res) => {
try {
const settings = stripReservedSettingKeys({ ...req.body });
// A settings.security holder still can't write domain/accounting keys here.
if (await rejectUnauthorizedProtectedKeys(settings, req, res)) return;
// Update or insert each setting
for (const [key, value] of Object.entries(settings)) {
@@ -1466,6 +1533,7 @@ router.put('/security', adminAuth, requirePermission('settings.edit'), async (re
router.put('/analytics', adminAuth, requirePermission('settings.edit'), async (req, res) => {
try {
const settings = stripReservedSettingKeys({ ...req.body });
if (await rejectUnauthorizedProtectedKeys(settings, req, res)) return;
// Validate the provider switch (#663 Phase 1). Reject unknown values
// so the dashboard route's factory doesn't have to defensively guard.
@@ -1521,6 +1589,7 @@ router.put('/analytics', adminAuth, requirePermission('settings.edit'), async (r
router.put('/seo', adminAuth, requirePermission('settings.edit'), async (req, res) => {
try {
const settings = stripReservedSettingKeys({ ...req.body });
if (await rejectUnauthorizedProtectedKeys(settings, req, res)) return;
// Validate seo_blocked_ai_agents is an array of strings
if (settings.seo_blocked_ai_agents !== undefined) {
@@ -1845,7 +1914,7 @@ router.post('/favicon', adminAuth, requirePermission('settings.edit'), faviconUp
});
// Update rate limit settings
router.put('/security/rate-limit', adminAuth, requirePermission('settings.edit'), [
router.put('/security/rate-limit', adminAuth, requirePermission('settings.security'), [
body('rate_limit_enabled').isBoolean().withMessage('Enabled must be a boolean'),
body('rate_limit_window_minutes').isInt({ min: 1, max: 60 }).withMessage('Window must be between 1 and 60 minutes'),
body('rate_limit_max_requests').isInt({ min: 10, max: 10000 }).withMessage('Max requests must be between 10 and 10000'),
+12 -12
View File
@@ -20,7 +20,7 @@ const {
const router = express.Router();
// Get system version
router.get('/version', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/version', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
// Read backend version from package.json
let backendVersion = '1.0.0';
@@ -49,7 +49,7 @@ router.get('/version', adminAuth, requirePermission('settings.view'), async (req
});
// Check for updates
router.get('/updates', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/updates', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
// Check if update checking is enabled
const updateCheckEnabled = process.env.UPDATE_CHECK_ENABLED !== 'false';
@@ -92,7 +92,7 @@ router.get('/updates', adminAuth, requirePermission('settings.view'), async (req
// brand-new install initialises the marker silently so it never pops
// "what's new" with nothing to compare against. Best-effort: any failure
// (GitHub unreachable, etc.) returns hasNews:false, never errors.
router.get('/updates/whatsnew', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/updates/whatsnew', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
if (process.env.UPDATE_CHECK_ENABLED === 'false') {
return res.json({ enabled: false, hasNews: false });
@@ -137,7 +137,7 @@ router.get('/updates/whatsnew', adminAuth, requirePermission('settings.view'), a
// Acknowledge the What's New — advance the per-instance marker to the
// running version so it stops showing for every admin.
router.post('/updates/whatsnew/seen', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.post('/updates/whatsnew/seen', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
const running = await getCurrentVersion();
await upsertAppSetting('whatsnew_last_seen_version', JSON.stringify(running), 'system');
@@ -153,7 +153,7 @@ router.post('/updates/whatsnew/seen', adminAuth, requirePermission('settings.vie
// admin can read release notes for ALL versions they're behind on, not
// just the latest. Body is raw GitHub-flavoured markdown; rendering is
// the client's job (frontend uses `marked`).
router.get('/updates/changelog', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/updates/changelog', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
const updateCheckEnabled = process.env.UPDATE_CHECK_ENABLED !== 'false';
if (!updateCheckEnabled) {
@@ -176,7 +176,7 @@ router.get('/updates/changelog', adminAuth, requirePermission('settings.view'),
});
// Get update instructions for current environment
router.get('/updates/instructions', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/updates/instructions', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
// Check if update checking is enabled
const updateCheckEnabled = process.env.UPDATE_CHECK_ENABLED !== 'false';
@@ -217,7 +217,7 @@ router.get('/updates/instructions', adminAuth, requirePermission('settings.view'
});
// Get comprehensive system status
router.get('/status', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/status', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
// Database size. Read the LIVE connection rather than re-deriving any of
// this from the environment (#1038): DATABASE_CLIENT is not the only thing
@@ -351,7 +351,7 @@ router.get('/status', adminAuth, requirePermission('settings.view'), async (req,
});
// Get database statistics
router.get('/database', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/database', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
// Get table info
const tables = [
@@ -413,7 +413,7 @@ router.get('/database', adminAuth, requirePermission('settings.view'), async (re
});
// Get update notification settings
router.get('/updates/notifications', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/updates/notifications', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
const settings = await getUpdateNotificationSettings();
res.json(settings);
@@ -424,7 +424,7 @@ router.get('/updates/notifications', adminAuth, requirePermission('settings.view
});
// Update notification settings
router.put('/updates/notifications', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/updates/notifications', adminAuth, requirePermission('system.manage'), async (req, res) => {
try {
const { enabled, recipients } = req.body;
@@ -460,7 +460,7 @@ router.put('/updates/notifications', adminAuth, requirePermission('settings.edit
// `version_update_available`, so admins on the latest version can still
// verify their SMTP + recipient config — the previous handler bailed
// with "No updates available" when nothing was pending (#418).
router.post('/updates/notifications/send', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.post('/updates/notifications/send', adminAuth, requirePermission('system.manage'), async (req, res) => {
try {
const result = await sendTestUpdateNotification();
res.json(result);
@@ -471,7 +471,7 @@ router.post('/updates/notifications/send', adminAuth, requirePermission('setting
});
// Check and send update notifications (called on admin login or periodically)
router.post('/updates/notifications/check', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.post('/updates/notifications/check', adminAuth, requirePermission(['settings.view', 'system.view']), async (req, res) => {
try {
const result = await checkAndNotifyUpdates();
res.json(result);
+5 -5
View File
@@ -33,7 +33,7 @@ const VALID_SCOPES = ['quote', 'contract', 'contract-signature', 'invoice'];
router.get(
'/backup-integrity',
requirePermission('settings.view'),
requirePermission(['settings.view', 'system.view']),
[
// CSV string like `?scope=contract,invoice`. Each member must be
// one of the four known scopes. Empty / omitted means full scan.
@@ -78,7 +78,7 @@ router.get(
*/
router.get(
'/backup-coverage',
requirePermission('settings.view'),
requirePermission(['settings.view', 'system.view']),
handleAsync(async (req, res) => {
const report = await getCoverageReport();
return successResponse(res, { report });
@@ -97,7 +97,7 @@ router.get(
*/
router.get(
'/failures',
requirePermission('settings.view'),
requirePermission(['settings.view', 'system.view']),
handleAsync(async (req, res) => {
const stuckEmails = await db('email_queue')
.where(function () {
@@ -132,7 +132,7 @@ router.get(
*/
router.post(
'/failures/email/:id/retry',
requirePermission('settings.edit'),
requirePermission('system.manage'),
handleAsync(async (req, res) => {
const id = parseInt(req.params.id, 10);
if (!Number.isFinite(id) || id <= 0) return res.status(400).json({ error: 'Invalid id' });
@@ -153,7 +153,7 @@ router.post(
*/
router.delete(
'/failures/email/:id',
requirePermission('settings.edit'),
requirePermission('system.manage'),
handleAsync(async (req, res) => {
const id = parseInt(req.params.id, 10);
if (!Number.isFinite(id) || id <= 0) return res.status(400).json({ error: 'Invalid id' });
+14 -1
View File
@@ -11,12 +11,25 @@
*/
const express = require('express');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
const { handleAsync, successResponse } = require('../utils/routeHelpers');
const ledgerService = require('../services/ledgerService');
const router = express.Router();
router.get('/', adminAuth, handleAsync(async (req, res) => {
// Migration 174: the VAT-code list is reference data the invoice/quote editors
// read, so it must stay readable by anyone who can touch those documents. We
// gate it with a broad OR (its own vat_codes.view plus every CRM-editor perm)
// rather than a single narrow perm — completeness without breaking any editor.
const VAT_CODE_READERS = [
'vat_codes.view',
'bills.view', 'bills.manage',
'quotes.view', 'quotes.manage',
'contracts.view', 'contracts.manage',
'accounting.view', 'accounting.manage',
];
router.get('/', adminAuth, requirePermission(VAT_CODE_READERS), handleAsync(async (req, res) => {
const items = await ledgerService.listVatCodes();
const active = items.filter((v) => v.active !== false);
const { direction } = req.query;
+13 -9
View File
@@ -20,6 +20,10 @@ const { body, query, validationResult } = require('express-validator');
const { db, logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
// Migration 174: webhooks are an integration surface. Mutations require the
// dedicated `settings.integrations` perm (split out of the old catch-all
// `settings.edit`); reads allow either the general `settings.view` or the
// integration perm so a role scoped to integrations can still read them.
const { validateExternalUrlAsync } = require('../utils/networkValidation');
const webhookService = require('../services/webhookService');
const logger = require('../utils/logger');
@@ -52,7 +56,7 @@ function safeJson(s, fallback) {
}
// ─── List ────────────────────────────────────────────────────────────────
router.get('/', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/', adminAuth, requirePermission(['settings.view', 'settings.integrations']), async (req, res) => {
try {
const rows = await db('webhooks')
.leftJoin('admin_users', 'admin_users.id', 'webhooks.created_by')
@@ -75,7 +79,7 @@ router.get('/', adminAuth, requirePermission('settings.view'), async (req, res)
router.post(
'/',
adminAuth,
requirePermission('settings.edit'),
requirePermission('settings.integrations'),
[
body('name').isString().trim().isLength({ min: 1, max: 100 }),
body('url').isString().isLength({ max: 2048 }).custom(async (url) => {
@@ -142,7 +146,7 @@ router.post(
);
// ─── Detail ──────────────────────────────────────────────────────────────
router.get('/:id', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/:id', adminAuth, requirePermission(['settings.view', 'settings.integrations']), async (req, res) => {
try {
const row = await db('webhooks').where({ id: req.params.id }).first();
if (!row) return res.status(404).json({ error: 'Webhook not found' });
@@ -157,7 +161,7 @@ router.get('/:id', adminAuth, requirePermission('settings.view'), async (req, re
router.put(
'/:id',
adminAuth,
requirePermission('settings.edit'),
requirePermission('settings.integrations'),
[
body('name').optional().isString().trim().isLength({ min: 1, max: 100 }),
body('url').optional().isString().isLength({ max: 2048 }).custom(async (url) => {
@@ -217,7 +221,7 @@ router.put(
);
// ─── Delete ──────────────────────────────────────────────────────────────
router.delete('/:id', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.delete('/:id', adminAuth, requirePermission('settings.integrations'), async (req, res) => {
try {
const row = await db('webhooks').where({ id: req.params.id }).first();
if (!row) return res.status(404).json({ error: 'Webhook not found' });
@@ -236,7 +240,7 @@ router.delete('/:id', adminAuth, requirePermission('settings.edit'), async (req,
router.post(
'/:id/test',
adminAuth,
requirePermission('settings.edit'),
requirePermission('settings.integrations'),
[body('event_type').optional().isIn(webhookService.EVENT_TYPES)],
async (req, res) => {
try {
@@ -284,7 +288,7 @@ router.post(
router.get(
'/:id/deliveries',
adminAuth,
requirePermission('settings.view'),
requirePermission(['settings.view', 'settings.integrations']),
[
query('status').optional().isIn(['pending', 'success', 'failed']),
query('page').optional().isInt({ min: 1 }),
@@ -330,7 +334,7 @@ router.get(
router.get(
'/:id/deliveries/:deliveryId',
adminAuth,
requirePermission('settings.view'),
requirePermission(['settings.view', 'settings.integrations']),
async (req, res) => {
try {
const row = await db('webhook_deliveries')
@@ -352,7 +356,7 @@ router.get(
router.post(
'/:id/deliveries/:deliveryId/replay',
adminAuth,
requirePermission('settings.edit'),
requirePermission('settings.integrations'),
async (req, res) => {
try {
const row = await db('webhook_deliveries')
+3 -3
View File
@@ -33,7 +33,7 @@ const logger = require('../utils/logger');
// pattern). The Settings UI hides the tab as well; this is defence in depth.
router.use(requireFeatureFlag('whatsapp'));
router.get('/config', adminAuth, requirePermission('settings.view'), async (req, res) => {
router.get('/config', adminAuth, requirePermission(['settings.view', 'whatsapp.view']), async (req, res) => {
try {
const config = await db('whatsapp_configs').first();
if (!config) {
@@ -62,7 +62,7 @@ router.get('/config', adminAuth, requirePermission('settings.view'), async (req,
}
});
router.put('/config', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.put('/config', adminAuth, requirePermission('whatsapp.manage'), async (req, res) => {
try {
const { phone_number_id, waba_id, access_token, template_name, template_language, template_params, enabled } = req.body;
@@ -142,7 +142,7 @@ router.put('/config', adminAuth, requirePermission('settings.edit'), async (req,
}
});
router.post('/test', adminAuth, requirePermission('settings.edit'), async (req, res) => {
router.post('/test', adminAuth, requirePermission('whatsapp.manage'), async (req, res) => {
try {
const { phone } = req.body;
if (!phone) {