From 90fe6e0f9a0067be5a4098da5ab6e9976e5465ac Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Mon, 11 May 2026 22:12:13 -0500 Subject: [PATCH] Phase 2.2j: assignments.js scoped to workspace_id via playlist.workspace_id; fixes 3 pre-existing cross-tenant leaks (content add, widget add with NO existing check, copy-to cross-workspace); ensureDevicePlaylist loop-closer; status.js playlist INSERT bundle --- server/routes/assignments.js | 110 ++++++++++++++++++++++++----------- server/routes/status.js | 6 +- 2 files changed, 79 insertions(+), 37 deletions(-) diff --git a/server/routes/assignments.js b/server/routes/assignments.js index 00005d7..dc9b96e 100644 --- a/server/routes/assignments.js +++ b/server/routes/assignments.js @@ -2,32 +2,42 @@ const express = require('express'); const router = express.Router(); const { v4: uuidv4 } = require('uuid'); const { db } = require('../db/database'); -const { ELEVATED_ROLES } = require('../middleware/auth'); +const { PLATFORM_ROLES, ELEVATED_ROLES } = require('../middleware/auth'); +// Phase 2.2j: workspace-aware access. Underlying tables (devices, playlists) +// already carry workspace_id from Phase 1; this route can use them even +// though playlists.js itself isn't yet workspace-filtered. +const { accessContext } = require('../lib/tenancy'); // Mark playlist as draft (called after any item mutation) function markDraft(playlistId) { db.prepare("UPDATE playlists SET status = 'draft', updated_at = strftime('%s','now') WHERE id = ?").run(playlistId); } -// Check device ownership for device-scoped routes -function checkDeviceAccess(req, res, paramName = 'deviceId') { - const device = db.prepare('SELECT user_id FROM devices WHERE id = ?').get(req.params[paramName]); - if (!device) { res.status(404).json({ error: 'Device not found' }); return false; } - if (!ELEVATED_ROLES.includes(req.user.role) && device.user_id && device.user_id !== req.user.id) { - res.status(403).json({ error: 'Access denied' }); return false; +// Phase 2.2j: workspace-aware device access check. Returns access context +// (with workspaceRole/actingAs) or null. Caller decides if read or write. +function checkDeviceAccess(req, res, paramName = 'deviceId', requireWrite = true) { + const device = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(req.params[paramName]); + if (!device) { res.status(404).json({ error: 'Device not found' }); return null; } + if (!device.workspace_id) { res.status(403).json({ error: 'Device not assigned to a workspace' }); return null; } + const ws = db.prepare('SELECT * FROM workspaces WHERE id = ?').get(device.workspace_id); + const ctx = ws && accessContext(req.user.id, req.user.role, ws); + if (!ctx) { res.status(403).json({ error: 'Access denied' }); return null; } + if (requireWrite && !ctx.actingAs && ctx.workspaceRole === 'workspace_viewer') { + res.status(403).json({ error: 'Read-only access' }); return null; } - return true; + return { device, ctx }; } -// Ensure device has a playlist; auto-create one if missing +// Ensure device has a playlist; auto-create one if missing. +// Phase 2.2j: stamps workspace_id on the auto-created playlist so it remains +// visible once playlists.js migrates. Mirrors the 2.2i fix in device-groups.js. function ensureDevicePlaylist(deviceId, userId) { - const device = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(deviceId); + const device = db.prepare('SELECT playlist_id, workspace_id, name FROM devices WHERE id = ?').get(deviceId); if (device?.playlist_id) return device.playlist_id; - const deviceRow = db.prepare('SELECT name FROM devices WHERE id = ?').get(deviceId); const playlistId = uuidv4(); - db.prepare('INSERT INTO playlists (id, user_id, name, is_auto_generated) VALUES (?, ?, ?, 1)') - .run(playlistId, userId, `${deviceRow?.name || 'Display'} playlist`); + db.prepare('INSERT INTO playlists (id, user_id, workspace_id, name, is_auto_generated) VALUES (?, ?, ?, ?, 1)') + .run(playlistId, userId, device?.workspace_id || null, `${device?.name || 'Display'} playlist`); db.prepare('UPDATE devices SET playlist_id = ? WHERE id = ?').run(playlistId, deviceId); return playlistId; } @@ -47,7 +57,7 @@ const ITEM_SELECT = ` // Get assignments (playlist items) for a device router.get('/device/:deviceId', (req, res) => { - if (!checkDeviceAccess(req, res)) return; + if (!checkDeviceAccess(req, res, 'deviceId', false)) return; const device = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(req.params.deviceId); if (!device?.playlist_id) return res.json([]); @@ -56,23 +66,35 @@ router.get('/device/:deviceId', (req, res) => { res.json(items); }); -// Add content or widget to device playlist +// Add content or widget to device playlist. +// Phase 2.2j: closes 2 pre-existing cross-tenant leaks: +// 1. Content gate: today checks content.user_id == caller. A workspace_admin +// who happens to own content in another workspace could push it into a +// device in this workspace. Now: content must be in device's workspace +// (or be a platform-template, workspace_id IS NULL). +// 2. Widget gate: today checks ONLY existence - any user could attach any +// widget UUID to their own device's playlist. Now: widget must be in +// device's workspace (or be a platform-template). router.post('/device/:deviceId', (req, res) => { - if (!checkDeviceAccess(req, res)) return; + const access = checkDeviceAccess(req, res, 'deviceId', true); + if (!access) return; const { content_id, widget_id, zone_id, duration_sec = 10, sort_order } = req.body; if (!content_id && !widget_id) return res.status(400).json({ error: 'content_id or widget_id required' }); if (content_id) { - const content = db.prepare('SELECT id, user_id FROM content WHERE id = ?').get(content_id); + const content = db.prepare('SELECT id, workspace_id FROM content WHERE id = ?').get(content_id); if (!content) return res.status(404).json({ error: 'Content not found' }); - if (!ELEVATED_ROLES.includes(req.user.role) && content.user_id && content.user_id !== req.user.id) { - return res.status(403).json({ error: 'Content not owned by you' }); + if (content.workspace_id && content.workspace_id !== access.device.workspace_id) { + return res.status(403).json({ error: 'Content is not in this device\'s workspace' }); } } if (widget_id) { - const widget = db.prepare('SELECT id FROM widgets WHERE id = ?').get(widget_id); + const widget = db.prepare('SELECT id, workspace_id FROM widgets WHERE id = ?').get(widget_id); if (!widget) return res.status(404).json({ error: 'Widget not found' }); + if (widget.workspace_id && widget.workspace_id !== access.device.workspace_id) { + return res.status(403).json({ error: 'Widget is not in this device\'s workspace' }); + } } const playlistId = ensureDevicePlaylist(req.params.deviceId, req.user.id); @@ -102,13 +124,25 @@ router.post('/device/:deviceId', (req, res) => { } }); +// Helper: load a playlist item and check write access via the parent +// playlist's workspace. Returns the item row or null after sending 403/404. +function checkItemWrite(req, res) { + const item = db.prepare('SELECT pi.*, p.workspace_id AS pl_workspace_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id); + if (!item) { res.status(404).json({ error: 'Item not found' }); return null; } + if (!item.pl_workspace_id) { res.status(403).json({ error: 'Playlist not assigned to a workspace' }); return null; } + const ws = db.prepare('SELECT * FROM workspaces WHERE id = ?').get(item.pl_workspace_id); + const ctx = ws && accessContext(req.user.id, req.user.role, ws); + if (!ctx) { res.status(403).json({ error: 'Access denied' }); return null; } + if (!ctx.actingAs && ctx.workspaceRole === 'workspace_viewer') { + res.status(403).json({ error: 'Read-only access' }); return null; + } + return item; +} + // Update playlist item router.put('/:id', (req, res) => { - const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id); - if (!item) return res.status(404).json({ error: 'Item not found' }); - if (!ELEVATED_ROLES.includes(req.user.role) && item.user_id !== req.user.id) { - return res.status(403).json({ error: 'Access denied' }); - } + const item = checkItemWrite(req, res); + if (!item) return; const { sort_order, duration_sec, zone_id } = req.body; const updates = []; @@ -130,11 +164,8 @@ router.put('/:id', (req, res) => { // Delete playlist item router.delete('/:id', (req, res) => { - const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id); - if (!item) return res.status(404).json({ error: 'Item not found' }); - if (!ELEVATED_ROLES.includes(req.user.role) && item.user_id !== req.user.id) { - return res.status(403).json({ error: 'Access denied' }); - } + const item = checkItemWrite(req, res); + if (!item) return; db.prepare('DELETE FROM playlist_items WHERE id = ?').run(req.params.id); markDraft(item.playlist_id); @@ -144,7 +175,7 @@ router.delete('/:id', (req, res) => { // Reorder items for a device's playlist router.post('/device/:deviceId/reorder', (req, res) => { - if (!checkDeviceAccess(req, res)) return; + if (!checkDeviceAccess(req, res, 'deviceId', true)) return; const { order } = req.body; if (!Array.isArray(order)) return res.status(400).json({ error: 'order must be an array of item IDs' }); @@ -166,10 +197,21 @@ router.post('/device/:deviceId/reorder', (req, res) => { res.json(items); }); -// Copy playlist from one device to another +// Copy playlist from one device to another. +// Phase 2.2j: closes a pre-existing cross-tenant leak. Today both deviceIds +// only got the user_id ownership check; a caller with reach into a foreign +// workspace could copy that workspace's playlist into a device in their own +// workspace (or vice versa). Now: both devices must be in the same workspace, +// and the caller must have write access there. router.post('/device/:deviceId/copy-to/:targetDeviceId', (req, res) => { - if (!checkDeviceAccess(req, res, 'deviceId')) return; - if (!checkDeviceAccess(req, res, 'targetDeviceId')) return; + const sourceAccess = checkDeviceAccess(req, res, 'deviceId', true); + if (!sourceAccess) return; + const targetAccess = checkDeviceAccess(req, res, 'targetDeviceId', true); + if (!targetAccess) return; + if (sourceAccess.device.workspace_id !== targetAccess.device.workspace_id) { + return res.status(403).json({ error: 'Source and target devices must be in the same workspace' }); + } + const sourceDevice = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(req.params.deviceId); if (!sourceDevice?.playlist_id) return res.status(404).json({ error: 'Source device has no playlist' }); diff --git a/server/routes/status.js b/server/routes/status.js index 505d0e4..6c9db72 100644 --- a/server/routes/status.js +++ b/server/routes/status.js @@ -360,7 +360,7 @@ router.post('/import', importUpload.single('file'), async (req, res) => { for (const p of (data.playlists || [])) { const newId = uuid.v4(); idMap.playlists[p.id] = newId; - db.prepare('INSERT INTO playlists (id, user_id, name, description, is_auto_generated, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?)').run(newId, userId, p.name, p.description || '', p.is_auto_generated || 0, p.created_at || Math.floor(Date.now() / 1000), p.updated_at || Math.floor(Date.now() / 1000)); + db.prepare('INSERT INTO playlists (id, user_id, workspace_id, name, description, is_auto_generated, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)').run(newId, userId, workspaceId, p.name, p.description || '', p.is_auto_generated || 0, p.created_at || Math.floor(Date.now() / 1000), p.updated_at || Math.floor(Date.now() / 1000)); stats.playlists++; } for (const pi of (data.playlist_items || [])) { @@ -502,8 +502,8 @@ router.post('/import', importUpload.single('file'), async (req, res) => { items.push({ contentId, widgetId, sort_order: a.sort_order || 0, duration }); } - db.prepare('INSERT INTO playlists (id, user_id, name, description, is_auto_generated) VALUES (?, ?, ?, ?, 1)') - .run(playlistId, userId, `${devName} (imported)`, 'Converted from v1 assignments'); + db.prepare('INSERT INTO playlists (id, user_id, workspace_id, name, description, is_auto_generated) VALUES (?, ?, ?, ?, ?, 1)') + .run(playlistId, userId, workspaceId, `${devName} (imported)`, 'Converted from v1 assignments'); for (const item of items) { db.prepare('INSERT INTO playlist_items (playlist_id, content_id, widget_id, sort_order, duration_sec) VALUES (?, ?, ?, ?, ?)') .run(playlistId, item.contentId, item.widgetId, item.sort_order, item.duration);