fix(devices): authorize the screenshot route on the device's workspace

GET /api/devices/:id/screenshot returns a live picture of what a screen is showing, but
it was still authorized pre-tenancy: `device.user_id !== user.id`, with a role bypass
listing 'admin'/'superadmin'. Three consequences, all now covered by tests:

- `device.user_id &&` SHORT-CIRCUITED. A device with no user_id — never paired, or its
  owner deleted — skipped the ownership test entirely, so any authenticated account on the
  instance could read it. An unpaired panel displays its pairing code on screen, so that
  image is also a route to claiming the device (AUTH-10, out of scope here but connected).
- 'platform_admin' was absent from the bypass list. #14 renamed 'superadmin' to
  'platform_admin', so an actual platform admin fell through to the ownership test and was
  denied unless they happened to own the row.
- Workspace members other than the owner were denied a device they administer through
  every other endpoint.

Now uses accessContext() against the device's workspace — the same helper routes/devices.js
uses — which covers direct membership, org-level access and platform staff in one call. A
device with no workspace is denied outright rather than defaulting open.

Deliberately unchanged: the ?token= query-parameter mechanism on this route, which is a
separate finding with its own blast radius.

No response shape change: still 200 / 401 / 403 / 404 with the same bodies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ScreenTinker 2026-07-26 14:17:54 -05:00
parent 8a55798eaf
commit dda6f5b41e
2 changed files with 138 additions and 3 deletions

View file

@ -431,9 +431,19 @@ app.get('/api/devices/:id/screenshot', (req, res) => {
return res.status(401).json({ error: 'Invalid or expired token' });
}
const { db: sdb } = require('./db/database');
const device = sdb.prepare('SELECT user_id FROM devices WHERE id = ?').get(req.params.id);
const device = sdb.prepare('SELECT user_id, workspace_id FROM devices WHERE id = ?').get(req.params.id);
if (!device) return res.status(404).json({ error: 'Device not found' });
if (!['admin','superadmin'].includes(user.role) && device.user_id && device.user_id !== user.id) return res.status(403).json({ error: 'Access denied' });
// Authorize on the DEVICE'S WORKSPACE, the same way routes/devices.js does. The previous
// test was pre-tenancy (`device.user_id !== user.id`) with a role bypass listing
// 'admin'/'superadmin', which had three problems: it short-circuited on `device.user_id &&`
// so a device with NO owner was readable by any authenticated account (an unpaired panel
// displays its pairing code, so that image is also a claim vector); it omitted
// 'platform_admin', the name #14 migrated 'superadmin' to, so real platform admins were
// denied; and it denied workspace members who administer the device everywhere else.
// accessContext covers direct membership, org-level access and platform staff in one call.
if (!device.workspace_id) return res.status(403).json({ error: 'Access denied' });
const ws = sdb.prepare('SELECT * FROM workspaces WHERE id = ?').get(device.workspace_id);
if (!ws || !accessContext(user.id, user.role, ws)) return res.status(403).json({ error: 'Access denied' });
// Serve from memory if available (device online), otherwise from disk (offline snapshot)
const deviceSocket = require('./ws/deviceSocket');
const memScreenshot = deviceSocket.lastScreenshots?.[req.params.id];
@ -555,7 +565,7 @@ app.get('/api/content/:id/thumbnail', (req, res) => {
// req.isPlatformAdmin, req.actingAs. Route handlers in 2.1 don't read these
// yet (they still filter by user_id); 2.2 will migrate them one route at a time.
const { requireAuth } = require('./middleware/auth');
const { resolveTenancy } = require('./lib/tenancy');
const { resolveTenancy, accessContext } = require('./lib/tenancy');
// Public API token front door (Phase 1). Attached ONLY to the public routers below.
const { bearerAuth, tokenScopeGate, agencyGate } = require('./middleware/apiToken');

View file

@ -0,0 +1,125 @@
'use strict';
// GET /api/devices/:id/screenshot returns a live picture of what a screen is showing, so
// it must be authorised the same way every other device route is: by the caller's access
// to the DEVICE'S WORKSPACE.
//
// It instead used a pre-tenancy ownership test — `device.user_id !== user.id`, with an
// elevated-role bypass listing 'admin'/'superadmin'. Three consequences this pins:
//
// 1. `device.user_id &&` SHORT-CIRCUITS. A device with no user_id (never paired, or its
// owner was deleted) skips the ownership test entirely, so ANY authenticated user on
// the instance can read it. An unpaired panel displays its pairing code on screen,
// so that image is also a route to claiming the device (see AUTH-10).
// 2. platform_admin is NOT in the bypass list. The #14 migration renamed superadmin ->
// platform_admin, so a real platform admin falls through to the ownership test and is
// denied unless they happen to own the row.
// 3. Workspace members other than the owner are denied a device they can otherwise fully
// administer through every other endpoint.
const { test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const { spawn } = require('node:child_process');
const path = require('node:path');
const os = require('node:os');
const fs = require('node:fs');
const crypto = require('node:crypto');
const Database = require('better-sqlite3');
const { freePort } = require('./helpers/free-port');
let PORT, BASE, proc, db;
const DATA_DIR = path.join(os.tmpdir(), 'st-shot-test-' + crypto.randomBytes(4).toString('hex'));
const LOG = path.join(os.tmpdir(), 'st-shot-' + crypto.randomBytes(4).toString('hex') + '.log');
const PW = 'Passw0rd123';
const S = {};
const jfetch = async (p, opts = {}) => {
const res = await fetch(BASE + p, opts);
let body = null; try { body = await res.json(); } catch { /* binary/none */ }
return { status: res.status, body };
};
const post = (obj) => ({ method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(obj) });
const shot = (deviceId, token) => fetch(`${BASE}/api/devices/${deviceId}/screenshot`, { headers: { Authorization: 'Bearer ' + token } });
async function register() {
const email = 'u' + crypto.randomBytes(5).toString('hex') + '@x.local';
const r = await jfetch('/api/auth/register', post({ email, password: PW }));
return { email, token: r.body.token, id: r.body.user.id, role: r.body.user.role };
}
// Give a device a screenshot on disk so a permitted caller gets a real 200.
function seedScreenshot(deviceId) {
const file = `${deviceId}_latest.jpg`;
fs.mkdirSync(path.join(DATA_DIR, 'uploads', 'screenshots'), { recursive: true });
fs.writeFileSync(path.join(DATA_DIR, 'uploads', 'screenshots', file), Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00]));
// screenshots.id is an INTEGER rowid alias — let SQLite assign it.
db.prepare('INSERT INTO screenshots (device_id, filepath, captured_at) VALUES (?,?,?)')
.run(deviceId, file, Math.floor(Date.now() / 1000));
}
before(async () => {
PORT = await freePort();
BASE = `http://127.0.0.1:${PORT}`;
const logFd = fs.openSync(LOG, 'w');
proc = spawn('node', ['server.js'], {
cwd: path.join(__dirname, '..'),
env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' },
stdio: ['ignore', logFd, logFd],
});
let up = false;
for (let i = 0; i < 80; i++) {
try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* not yet */ }
await new Promise(r => setTimeout(r, 250));
}
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'));
S.admin = await register(); // first user => platform_admin
assert.equal(S.admin.role, 'platform_admin');
S.owner = await register(); // owns a device, in its own workspace
S.outsider = await register(); // no relationship to that device
S.ownerWs = db.prepare('SELECT workspace_id FROM workspace_members WHERE user_id = ?').get(S.owner.id).workspace_id;
// A normally-paired device: owned, and in the owner's workspace.
S.owned = crypto.randomUUID();
db.prepare("INSERT INTO devices (id, user_id, workspace_id, name, status) VALUES (?,?,?,'Owned','online')")
.run(S.owned, S.owner.id, S.ownerWs);
seedScreenshot(S.owned);
// An UNPAIRED device: no user_id, no workspace — the state a panel sits in while it is
// displaying its pairing code.
S.unpaired = crypto.randomUUID();
db.prepare("INSERT INTO devices (id, user_id, workspace_id, name, status, pairing_code) VALUES (?,NULL,NULL,'Unpaired','provisioning','424242')")
.run(S.unpaired);
seedScreenshot(S.unpaired);
});
after(() => { try { db && db.close(); } catch { /* */ } try { proc.kill('SIGKILL'); } catch { /* */ } });
test('the device owner can read its screenshot', async () => {
const res = await shot(S.owned, S.owner.token);
assert.equal(res.status, 200, 'the owner must keep working');
});
test('an unrelated user cannot read a device in a workspace they are not in', async () => {
const res = await shot(S.owned, S.outsider.token);
assert.equal(res.status, 403, 'no access path to that workspace');
});
test('a platform admin can read any device screenshot', async () => {
const res = await shot(S.owned, S.admin.token);
assert.equal(res.status, 200, 'platform_admin must not be denied by a stale role list');
});
test('an UNPAIRED device is not readable by an arbitrary authenticated user', async () => {
// The short-circuit made this readable by anyone with an account. An unpaired panel is
// showing its pairing code, so the image is also a claim vector.
const res = await shot(S.unpaired, S.outsider.token);
assert.ok(res.status === 403 || res.status === 404,
`an unassigned device must not be readable by any account (got ${res.status})`);
});
test('an unknown device id is still a 404, and no token is still a 401', async () => {
assert.equal((await shot(crypto.randomUUID(), S.owner.token)).status, 404);
assert.equal((await fetch(`${BASE}/api/devices/${S.owned}/screenshot`)).status, 401);
});