diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index 264daf4..7a01f6d 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -1230,12 +1230,18 @@ Turning this off re-enables allow-same-origin. Widget HTML will then run with the same privileges as ScreenTinker itself. Any script in any widget in this organization will be able to: - - Read the session token of every logged-in user who views a display or - preview + - Read the device token of every display that shows the widget, and act as + that display against the ScreenTinker API + - Read the session token of any logged-in user who opens a display in their + own browser - Call the ScreenTinker API as that user, including admin actions - Read and modify content on every other display in this organization - Silently exfiltrate all of the above to any server it likes +The widget editor's Preview is NOT affected: it renders inside the dashboard, +where your session lives, so it stays isolated whatever this setting says. A +widget may therefore behave differently in Preview than on a display. + Because allow-scripts is also required for widgets to function, a widget can remove its own sandbox entirely once same-origin is granted. There is no partial protection left after this point. diff --git a/frontend/js/views/widgets.js b/frontend/js/views/widgets.js index e24de5c..88ffd0c 100644 --- a/frontend/js/views/widgets.js +++ b/frontend/js/views/widgets.js @@ -292,7 +292,7 @@ function openContentPicker({ multiple = false, title } = {}) { }); } -function showPreviewModal(sessionId, widgetType, widgetSandboxIsolationDisabled = false) { +function showPreviewModal(sessionId, widgetType) { const overlay = document.createElement('div'); overlay.style.cssText = 'position:fixed;inset:0;background:rgba(0,0,0,0.85);display:flex;align-items:center;justify-content:center;z-index:10000;padding:16px'; // #104: webpage widgets pointing at frame-denying sites (X-Frame-Options) can't be @@ -307,7 +307,16 @@ function showPreviewModal(sessionId, widgetType, widgetSandboxIsolationDisabled ${t('widget.preview_title')} - + + ${webpageNote} `; document.body.appendChild(overlay); @@ -1004,9 +1013,7 @@ export async function render(container) { }); if (!res.ok) throw new Error(t('widget.toast.preview_failed')); const { id } = await res.json(); - let user = null; - try { user = JSON.parse(localStorage.getItem('user') || 'null'); } catch (_) { user = null; } - showPreviewModal(id, type, !!user?.current_organization?.widget_sandbox_isolation_disabled); + showPreviewModal(id, type); } catch (err) { showToast(err.message, 'error'); } }; diff --git a/server/routes/widgets.js b/server/routes/widgets.js index 9cd2060..0e09157 100644 --- a/server/routes/widgets.js +++ b/server/routes/widgets.js @@ -210,6 +210,17 @@ function renderWidgetHtml(type, config, opts = {}) { } } +// The widget editor's Preview is framed by the DASHBOARD, from the dashboard's own +// origin, and the dashboard keeps its session JWT in localStorage. So preview HTML is +// pinned to the isolating sandbox and never consults the org setting: otherwise anyone +// who can author a widget (workspace_editor and up) could run script in the dashboard +// origin and lift the session of whichever admin clicked Preview. +// +// The org setting exists so PLAYERS can embed origin-strict third-party sites. A player +// runs on a kiosk with a device token, which is the risk the confirmation modal +// describes; an admin's dashboard session is not. +const PREVIEW_IFRAME_SANDBOX = 'allow-scripts'; + function widgetIframeSandboxForWorkspace(workspaceId) { if (!workspaceId) return 'allow-scripts'; try { @@ -328,8 +339,9 @@ router.post('/preview', (req, res) => { const { widget_type, config } = req.body || {}; if (!widget_type || typeof widget_type !== 'string') return res.status(400).json({ error: 'widget_type required' }); if (!KNOWN_WIDGET_TYPES.has(widget_type)) return res.status(400).json({ error: 'Unknown widget_type' }); - const iframeSandbox = widgetIframeSandboxForWorkspace(req.workspaceId); - let html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox }); + // Preview renders inside the DASHBOARD origin, so it never opts into same-origin — + // see PREVIEW_IFRAME_SANDBOX. + let html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox: PREVIEW_IFRAME_SANDBOX }); if (req.workspaceId) html = inlineUserContent(html, req.workspaceId); res.setHeader('Content-Type', 'text/html'); res.send(html); @@ -351,8 +363,8 @@ router.post('/preview-session', (req, res) => { if (!widget_type || typeof widget_type !== 'string') return res.status(400).json({ error: 'widget_type required' }); if (!KNOWN_WIDGET_TYPES.has(widget_type)) return res.status(400).json({ error: 'Unknown widget_type' }); const id = uuidv4(); - const iframeSandbox = widgetIframeSandboxForWorkspace(req.workspaceId); - const html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox }); + // Same reasoning as /preview — dashboard origin, never same-origin. + const html = renderWidgetHtml(widget_type, config || {}, { iframeSandbox: PREVIEW_IFRAME_SANDBOX }); previewStore.set(id, { html, widget_type, created: Date.now() }); res.json({ id, url: `/api/widgets/preview-session/${id}` }); }); diff --git a/server/test/widget-preview-stays-isolated.test.js b/server/test/widget-preview-stays-isolated.test.js new file mode 100644 index 0000000..2ad75cc --- /dev/null +++ b/server/test/widget-preview-stays-isolated.test.js @@ -0,0 +1,73 @@ +'use strict'; + +// Guards the boundary added on top of #254 (org-level widget sandbox toggle). +// +// #254 let an org opt out of widget iframe isolation so PLAYERS can embed +// origin-strict third-party sites. As merged it applied the same opt-out to the +// widget editor's Preview — which is framed by the DASHBOARD, from the dashboard's +// own origin, where the admin's session JWT lives in localStorage. That turned +// "my kiosks are less isolated" into "anyone who can author a widget can lift the +// session of whichever admin clicks Preview" (workspace_editor and up; viewers are +// refused at the create route). +// +// These tests fail if the preview path ever consults the org setting again. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const WIDGETS_ROUTE = fs.readFileSync(path.join(__dirname, '..', 'routes', 'widgets.js'), 'utf8'); +const WIDGETS_VIEW = fs.readFileSync( + path.join(__dirname, '..', '..', 'frontend', 'js', 'views', 'widgets.js'), + 'utf8' +); + +// Body of a router handler, from its `router.(''` to the next `router.` +function handlerBody(source, verb, route) { + const start = source.indexOf(`router.${verb}('${route}'`); + assert.notEqual(start, -1, `could not find router.${verb}('${route}') — test needs updating`); + const rest = source.slice(start + 1); + const end = rest.indexOf('\nrouter.'); + return end === -1 ? rest : rest.slice(0, end); +} + +test('preview sandbox constant is the isolating one', () => { + assert.match( + WIDGETS_ROUTE, + /const PREVIEW_IFRAME_SANDBOX = 'allow-scripts';/, + 'PREVIEW_IFRAME_SANDBOX must be exactly allow-scripts (no allow-same-origin)' + ); +}); + +for (const [verb, route] of [['post', '/preview'], ['post', '/preview-session']]) { + test(`${verb.toUpperCase()} ${route} pins the isolating sandbox and ignores the org setting`, () => { + const body = handlerBody(WIDGETS_ROUTE, verb, route); + assert.match(body, /PREVIEW_IFRAME_SANDBOX/, `${route} must render with PREVIEW_IFRAME_SANDBOX`); + assert.doesNotMatch( + body, + /widgetIframeSandboxForWorkspace/, + `${route} must NOT consult the org widget-sandbox setting — it renders in the dashboard origin` + ); + }); +} + +test('player render path still honours the org setting (the feature itself)', () => { + const body = handlerBody(WIDGETS_ROUTE, 'get', '/:id/render'); + assert.match( + body, + /widgetIframeSandboxForWorkspace\(widget\.workspace_id\)/, + 'the /render path is what #254 is for — it must keep consulting the org setting' + ); +}); + +test('dashboard preview iframe is hard-coded to allow-scripts', () => { + const iframeTag = WIDGETS_VIEW.match(/