Security & UX Release v7
Security: - H1: Stored-XSS-Fix — Upload-Pfad-Whitelist (Server + Frontend-Guard safeFileUrl) - H2: Transaktionen repariert — txDb-Contract in db.js (PG + SQLite), Rollback funktioniert - H3-Vorbereitung: SESSION_SECRET wird in Compose durchgereicht (Fix M3) - registerLimiter exportiert (Crash-Bug: Route.post ohne Callback) - LDAP-Sync: PG-Transaktionsabbruch bei UNIQUE-Verstoß behoben (Precheck-Selects) - LDAP-Filter: nur echte Benutzerkonten (keine Computer/Service-Accounts, Bit 512) - Rollen app-seitig: Sync ändert nie role/status, neue User immer user+inaktiv - DB-Cleanup: 82 Computer-/Service-Accounts aus lokaler User-Tabelle entfernt UX: - Dashboard: Vorlagen als Table-Liste + Column-Chart (Top 5 in %), 2 gleich große Spalten - Table-Listen (Dashboard/Vorlageneditor/Aufgaben) scrollbar bis Seitenende - Pagination 10/Seite im Dashboard, Sidebar-Label Dashboard
This commit is contained in:
Kühn
@@ -10,12 +10,60 @@ const db = require('../db');
|
||||
const { auditLog } = require('../auditLog');
|
||||
const { authMiddleware, adminMiddleware } = require('../middleware/auth');
|
||||
const { taskCreateLimiter } = require('../middleware/rateLimit');
|
||||
const { validate, validateQuery, createTaskSchema, updateTaskStatusSchema, updateTaskValuesSchema, addTaskFieldSchema, paginationSchema } = require('../middleware/validation');
|
||||
const { validate, validateQuery, createTaskSchema, updateTaskStatusSchema, updateTaskValuesSchema, addTaskFieldSchema, paginationSchema, isSafeUploadPath } = require('../middleware/validation');
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
router.use(authMiddleware);
|
||||
|
||||
// H1: Defense-in-Depth — file_upload-Step-Werte werden als Download-Link
|
||||
// gerendert. Nur Pfade akzeptieren, die exakt vom Upload-Endpoint stammen,
|
||||
// damit kein javascript:/data:-XSS über den Wert eingeschleust werden kann.
|
||||
// (Der Step-Typ ist erst serverseitig bekannt, daher die Prüfung hier.)
|
||||
// Variante für Task-Create: Werte tragen step_id, Step-Typ wird nachgeschlagen.
|
||||
async function validateFileUploadValues(values) {
|
||||
const stepIds = values.map(v => v.step_id).filter(id => Number.isInteger(id));
|
||||
if (stepIds.length === 0) return;
|
||||
const placeholders = stepIds.map(() => '?').join(',');
|
||||
const steps = await db.prepare(`SELECT id, type FROM template_steps WHERE id IN (${placeholders})`).all(...stepIds);
|
||||
const typeMap = {};
|
||||
steps.forEach(s => { typeMap[s.id] = s.type; });
|
||||
for (const v of values) {
|
||||
if (v.step_id && typeMap[v.step_id] === 'file_upload' && v.value && !isSafeUploadPath(v.value)) {
|
||||
const err = new Error('Ungueltiger Dateipfad in Werten.');
|
||||
err.status = 400;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// H1: Variante für Task-Values-Update (PUT /:id/values): Das Schema enthält
|
||||
// kein step_id, daher wird der Step-Typ über die bestehenden task_values
|
||||
// ermittelt. Werte, die unverändert zum DB-Stand sind, werden akzeptiert
|
||||
// (Legacy-Daten blockieren keine legitimen Edits); nur NEU gesetzte unsichere
|
||||
// Werte werden abgelehnt.
|
||||
async function validateFileUploadValueUpdates(taskId, values) {
|
||||
const ids = values.map(v => v.id).filter(id => Number.isInteger(id));
|
||||
if (ids.length === 0) return;
|
||||
const placeholders = ids.map(() => '?').join(',');
|
||||
const rows = await db.prepare(
|
||||
`SELECT tv.id, tv.value as old_value, ts.type as step_type
|
||||
FROM task_values tv LEFT JOIN template_steps ts ON tv.step_id = ts.id
|
||||
WHERE tv.task_id = ? AND tv.id IN (${placeholders})`
|
||||
).all(taskId, ...ids);
|
||||
const rowMap = {};
|
||||
rows.forEach(r => { rowMap[r.id] = r; });
|
||||
for (const v of values) {
|
||||
const row = rowMap[v.id];
|
||||
if (!row) continue; // Fremde/unbekannte IDs scheitern später am UPDATE selbst
|
||||
if (row.step_type === 'file_upload' && v.value && v.value !== row.old_value && !isSafeUploadPath(v.value)) {
|
||||
const err = new Error('Ungueltiger Dateipfad in Werten.');
|
||||
err.status = 400;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Create task - Punkt 8: Transaction
|
||||
router.post('/', taskCreateLimiter, validate(createTaskSchema), async (req, res) => {
|
||||
const { template_id, title, values, file_path, user_id } = req.validatedBody;
|
||||
@@ -27,22 +75,33 @@ router.post('/', taskCreateLimiter, validate(createTaskSchema), async (req, res)
|
||||
return res.status(400).json({ error: 'template_id und title sind erforderlich.' });
|
||||
}
|
||||
|
||||
const insertTask = db.prepare('INSERT INTO tasks (template_id, user_id, title, status, file_path) VALUES (?, ?, ?, \'offen\', ?)');
|
||||
const insertValue = db.prepare('INSERT INTO task_values (task_id, step_id, value, is_checked, file_path, snap_label, snap_type, snap_page_num, snap_ad_field, snap_ad_prefix, snap_dropdown_options, snap_email_source_fields, snap_hidden) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)');
|
||||
// H1: file_upload-Step-Werte gegen Upload-Whitelist prüfen (XSS-Schutz)
|
||||
try {
|
||||
await validateFileUploadValues(values);
|
||||
} catch (err) {
|
||||
return res.status(err.status || 400).json({ error: err.message });
|
||||
}
|
||||
|
||||
// H2: Alle Statements innerhalb der Transaktion MÜSSEN über txDb.prepare()
|
||||
// erzeugt werden (fn erhält txDb als this) — sonst laufen sie außerhalb der
|
||||
// Transaktion und ein Rollback ist unmöglich.
|
||||
const createTask = db.transaction(async function () {
|
||||
const txDb = this;
|
||||
const insertTask = txDb.prepare('INSERT INTO tasks (template_id, user_id, title, status, file_path) VALUES (?, ?, ?, \'offen\', ?)');
|
||||
const insertValue = txDb.prepare('INSERT INTO task_values (task_id, step_id, value, is_checked, file_path, snap_label, snap_type, snap_page_num, snap_ad_field, snap_ad_prefix, snap_dropdown_options, snap_email_source_fields, snap_hidden) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)');
|
||||
|
||||
const createTask = db.transaction(async () => {
|
||||
const info = await insertTask.run(template_id, targetUserId, title, file_path);
|
||||
const taskId = info.lastInsertRowid;
|
||||
|
||||
if (values.length > 0) {
|
||||
// Fetch step metadata for snapshot
|
||||
// Fetch step metadata for snapshot (auch über txDb — konsistente Connection)
|
||||
const stepIds = values.map(v => v.step_id).filter(Boolean);
|
||||
const stepMetaMap = {};
|
||||
if (stepIds.length > 0) {
|
||||
const validStepIds = stepIds.filter(id => Number.isInteger(id));
|
||||
if (validStepIds.length > 0) {
|
||||
const placeholders = validStepIds.map(() => '?').join(',');
|
||||
const steps = await db.prepare(`SELECT id, label, type, page_num, ad_field, ad_prefix, dropdown_options, email_source_fields, hidden FROM template_steps WHERE id IN (${placeholders})`).all(...validStepIds);
|
||||
const steps = await txDb.prepare(`SELECT id, label, type, page_num, ad_field, ad_prefix, dropdown_options, email_source_fields, hidden FROM template_steps WHERE id IN (${placeholders})`).all(...validStepIds);
|
||||
steps.forEach(s => { stepMetaMap[s.id] = s; });
|
||||
}
|
||||
}
|
||||
@@ -67,7 +126,7 @@ router.post('/', taskCreateLimiter, validate(createTaskSchema), async (req, res)
|
||||
|
||||
try {
|
||||
const taskId = await createTask();
|
||||
auditLog(req.user?.id, 'create_task', 'task', taskId, `Task created: ${title}`);
|
||||
auditLog(req.user?.id, 'create_task', 'task', taskId, `Task created: ${title}`, req);
|
||||
res.status(201).json({ id: taskId, template_id, user_id: targetUserId, title, status: 'offen', file_path, values });
|
||||
} catch (err) {
|
||||
console.error('[ERROR] POST /tasks -', err.message);
|
||||
@@ -87,7 +146,7 @@ router.patch('/:id/status', validate(updateTaskStatusSchema), async (req, res) =
|
||||
}
|
||||
const info = await db.prepare('UPDATE tasks SET status = ? WHERE id = ?').run(status, taskId);
|
||||
if (info.changes === 0) return res.status(404).json({ error: 'Aufgabe nicht gefunden.' });
|
||||
auditLog(req.user?.id, 'update_task', 'task', taskId, `Status changed to: ${status}`);
|
||||
auditLog(req.user?.id, 'update_task', 'task', taskId, `Status changed to: ${status}`, req);
|
||||
res.json({ id: taskId, status });
|
||||
});
|
||||
|
||||
@@ -96,8 +155,17 @@ router.put('/:id/values', adminMiddleware, validate(updateTaskValuesSchema), asy
|
||||
const taskId = parseInt(req.params.id);
|
||||
const { values } = req.validatedBody;
|
||||
|
||||
const updateValue = db.prepare('UPDATE task_values SET value = ?, is_checked = ? WHERE id = ? AND task_id = ?');
|
||||
const updateTransaction = db.transaction(async (vals) => {
|
||||
// H1: Auch beim Admin-Update nur sichere Upload-Pfade für file_upload-Steps akzeptieren
|
||||
try {
|
||||
await validateFileUploadValueUpdates(taskId, values);
|
||||
} catch (err) {
|
||||
return res.status(err.status || 400).json({ error: err.message });
|
||||
}
|
||||
|
||||
// H2: Statements innerhalb der Transaktion über txDb erzeugen
|
||||
const updateTransaction = db.transaction(async function (vals) {
|
||||
const txDb = this;
|
||||
const updateValue = txDb.prepare('UPDATE task_values SET value = ?, is_checked = ? WHERE id = ? AND task_id = ?');
|
||||
let updated = 0;
|
||||
for (const v of vals) {
|
||||
const info = await updateValue.run(v.value || '', v.is_checked ? 1 : 0, v.id, taskId);
|
||||
@@ -108,7 +176,7 @@ router.put('/:id/values', adminMiddleware, validate(updateTaskValuesSchema), asy
|
||||
|
||||
try {
|
||||
const updated = await updateTransaction(values);
|
||||
auditLog(req.user?.id, 'update_task', 'task', taskId, `Updated ${updated} task values`);
|
||||
auditLog(req.user?.id, 'update_task', 'task', taskId, `Updated ${updated} task values`, req);
|
||||
res.json({ updated, taskId });
|
||||
} catch (err) {
|
||||
res.status(500).json({ error: 'Interner Serverfehler.' });
|
||||
@@ -131,7 +199,7 @@ router.post('/:id/add-field', adminMiddleware, validate(addTaskFieldSchema), asy
|
||||
'INSERT INTO task_values (task_id, step_id, value, is_checked, custom_label, custom_type, custom_dropdown_options, custom_ad_field, custom_hidden, custom_email_source_fields) VALUES (?, NULL, ?, ?, ?, ?, ?, ?, ?, ?)'
|
||||
).run(taskId, fieldValue, fieldType === 'checkbox' ? 0 : 0, label.trim(), fieldType, customDropdownOptions, customAdField, customHidden, customEmailSourceFields);
|
||||
|
||||
auditLog(req.user?.id, 'task.add-field', 'task', taskId, `Added field: ${label.trim()}`);
|
||||
auditLog(req.user?.id, 'task.add-field', 'task', taskId, `Added field: ${label.trim()}`, req);
|
||||
res.status(201).json({
|
||||
id: info.lastInsertRowid, task_id: taskId, custom_label: label.trim(), custom_type: fieldType,
|
||||
value: fieldValue, page_num: page_num || 1,
|
||||
@@ -150,7 +218,7 @@ router.delete('/:id/fields/:fieldId', adminMiddleware, async (req, res) => {
|
||||
const fieldId = parseInt(req.params.fieldId);
|
||||
const info = await db.prepare('DELETE FROM task_values WHERE id = ? AND task_id = ? AND custom_label IS NOT NULL').run(fieldId, taskId);
|
||||
if (info.changes === 0) return res.status(404).json({ error: 'Feld nicht gefunden oder kein benutzerdefiniertes Feld.' });
|
||||
auditLog(req.user?.id, 'task.delete-field', 'task', taskId, `Deleted field: ${fieldId}`);
|
||||
auditLog(req.user?.id, 'task.delete-field', 'task', taskId, `Deleted field: ${fieldId}`, req);
|
||||
res.json({ message: 'Feld gelöscht.' });
|
||||
});
|
||||
|
||||
@@ -159,7 +227,7 @@ router.delete('/:id', adminMiddleware, async (req, res) => {
|
||||
const taskId = parseInt(req.params.id);
|
||||
const info = await db.prepare('DELETE FROM tasks WHERE id = ?').run(taskId);
|
||||
if (info.changes === 0) return res.status(404).json({ error: 'Aufgabe nicht gefunden.' });
|
||||
auditLog(req.user?.id, 'delete_task', 'task', taskId, null);
|
||||
auditLog(req.user?.id, 'delete_task', 'task', taskId, null, req);
|
||||
res.json({ message: 'Aufgabe gelöscht.' });
|
||||
});
|
||||
|
||||
@@ -169,11 +237,16 @@ router.get('/:id', async (req, res) => {
|
||||
const task = await db.prepare('SELECT t.*, u.name as user_name, u.email as user_email, tpl.name as template_name, tpl.ad_create FROM tasks t LEFT JOIN users u ON t.user_id = u.id LEFT JOIN templates tpl ON t.template_id = tpl.id WHERE t.id = ?').get(taskId);
|
||||
if (!task) return res.status(404).json({ error: 'Aufgabe nicht gefunden.' });
|
||||
|
||||
// K2: BOLA protection — non-admins may only read their own tasks
|
||||
if (task.user_id !== req.user.id && req.user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Keine Berechtigung, diese Aufgabe anzuzeigen.' });
|
||||
}
|
||||
|
||||
const values = await db.prepare(
|
||||
`SELECT tv.*, COALESCE(ts.label, tv.snap_label) as step_label, COALESCE(ts.type, tv.snap_type) as step_type, COALESCE(ts.page_num, tv.snap_page_num) as page_num, COALESCE(ts.ad_field, tv.snap_ad_field) as ad_field, COALESCE(ts.ad_prefix, tv.snap_ad_prefix) as ad_prefix, COALESCE(ts.dropdown_options, tv.snap_dropdown_options) as dropdown_options, COALESCE(ts.email_source_fields, tv.snap_email_source_fields) as email_source_fields, COALESCE(ts.hidden, tv.snap_hidden) as hidden, tv.custom_label, tv.custom_type, tv.custom_dropdown_options, tv.custom_ad_field, tv.custom_hidden, tv.custom_email_source_fields FROM task_values tv LEFT JOIN template_steps ts ON tv.step_id = ts.id WHERE tv.task_id = ? ORDER BY ts.step_order ASC, tv.id ASC`
|
||||
).all(taskId);
|
||||
|
||||
auditLog(req.user?.id, 'view_task', 'task', taskId, null);
|
||||
auditLog(req.user?.id, 'view_task', 'task', taskId, null, req);
|
||||
res.json({ ...task, values: values || [] });
|
||||
});
|
||||
|
||||
@@ -183,10 +256,14 @@ router.get('/', validateQuery(paginationSchema), async (req, res) => {
|
||||
const offset = (page - 1) * limit;
|
||||
const status = req.query.status;
|
||||
|
||||
let whereClause = '';
|
||||
// K2: BOLA protection — non-admins only see their own tasks
|
||||
const isAdmin = req.user.role === 'admin';
|
||||
let whereClause = isAdmin ? '' : ' WHERE t.user_id = ?';
|
||||
const params = [];
|
||||
if (!isAdmin) params.push(req.user.id);
|
||||
|
||||
if (status && ['offen', 'erledigt'].includes(status)) {
|
||||
whereClause = ' WHERE t.status = ?';
|
||||
whereClause = isAdmin ? ' WHERE t.status = ?' : ' AND t.status = ?';
|
||||
params.push(status);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user