mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-29 09:23:16 -06:00
fix(#143): enforceable device block + fix the null-token auth short-circuit
Highest-priority #143 item (operator finding from Bold): nulling a device's token
did NOT lock it out — device 75c2a08a immediately reconnected and saturated the
loop. Two distinct defects:
1. Auth short-circuit (the cause). device:register used
if (device.device_token && !validateDeviceToken(...)) { reject }
so a NULL/empty STORED token made the guard falsy -> validation SKIPPED, and the
next block even MINTED a fresh token and persisted it. Nulling a token thus
RE-PROVISIONED the device instead of locking it out. Fix: drop the
`device.device_token &&` guard -> `if (!validateDeviceToken(device_id, device_token))`
(validateDeviceToken already returns false for null-stored/missing/mismatch), and
remove the legacy "mint a token for a null-token device" path (the re-provision
vector). An already-provisioned device (every row, incl. 'provisioning', is created
WITH a token) presenting null/empty/invalid is now REJECTED + disconnected.
The first-pairing seam is unaffected: a brand-new device has NO device_id and goes
through the pairing_code branch (which mints id+token) — a different code path.
2. No server-side kill switch. Added a `blocked` column (devices.blocked INTEGER
NOT NULL DEFAULT 0; schema.sql + a database.js migration). The block is the FIRST
gate at the top of device:register — before the fingerprint block, the reconnect
throttle, any DB writes, or playlist build — so a blocked device's socket is
refused immediately (auth-error 'Device blocked' + disconnect, zero further work).
It does NOT rely on null-token (the thing that failed). The row is re-read every
register, so a DIRECT SQLite edit takes effect on the device's NEXT reconnect with
NO server restart. Operator statements (dashboard-down, hand-edit):
block: UPDATE devices SET blocked = 1 WHERE id = '<device_id>';
unblock: UPDATE devices SET blocked = 0 WHERE id = '<device_id>';
Tests (port 3987): nulled-token provisioned device is REJECTED (75c2a08a repro);
blocked=1 refused at the first gate (no register/playlist); unblock reconnects;
first-pairing still works; normal valid-token device unaffected. Full suite green
serial AND parallel (213); reconnect-throttle.js + the dbac699 content-ack limiter
untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dbac699854
commit
404c3301dd
|
|
@ -234,6 +234,10 @@ const migrations = [
|
||||||
// schema.sql creates these on fresh installs; this covers existing DBs.
|
// schema.sql creates these on fresh installs; this covers existing DBs.
|
||||||
"CREATE TABLE IF NOT EXISTS event_loop_lag (id INTEGER PRIMARY KEY AUTOINCREMENT, sampled_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), mean_ms REAL NOT NULL, p50_ms REAL NOT NULL, p99_ms REAL NOT NULL, max_ms REAL NOT NULL, band TEXT NOT NULL DEFAULT 'normal')",
|
"CREATE TABLE IF NOT EXISTS event_loop_lag (id INTEGER PRIMARY KEY AUTOINCREMENT, sampled_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), mean_ms REAL NOT NULL, p50_ms REAL NOT NULL, p99_ms REAL NOT NULL, max_ms REAL NOT NULL, band TEXT NOT NULL DEFAULT 'normal')",
|
||||||
"CREATE INDEX IF NOT EXISTS idx_event_loop_lag_sampled ON event_loop_lag(sampled_at)",
|
"CREATE INDEX IF NOT EXISTS idx_event_loop_lag_sampled ON event_loop_lag(sampled_at)",
|
||||||
|
// #143: operator device kill switch. blocked=1 refuses the device at the first
|
||||||
|
// register gate on its next reconnect (no restart). Hand-settable by direct SQLite:
|
||||||
|
// UPDATE devices SET blocked = 1 WHERE id = '<device_id>'; (0 to unblock)
|
||||||
|
"ALTER TABLE devices ADD COLUMN blocked INTEGER NOT NULL DEFAULT 0",
|
||||||
];
|
];
|
||||||
// Apply each ALTER idempotently. A "duplicate column name" / "already exists"
|
// Apply each ALTER idempotently. A "duplicate column name" / "already exists"
|
||||||
// error means the column is already present (expected on a migrated DB) - benign.
|
// error means the column is already present (expected on a migrated DB) - benign.
|
||||||
|
|
|
||||||
|
|
@ -64,6 +64,7 @@ CREATE TABLE IF NOT EXISTS devices (
|
||||||
name TEXT NOT NULL DEFAULT 'Unnamed Display',
|
name TEXT NOT NULL DEFAULT 'Unnamed Display',
|
||||||
pairing_code TEXT UNIQUE,
|
pairing_code TEXT UNIQUE,
|
||||||
status TEXT NOT NULL DEFAULT 'offline',
|
status TEXT NOT NULL DEFAULT 'offline',
|
||||||
|
blocked INTEGER NOT NULL DEFAULT 0,
|
||||||
last_heartbeat INTEGER,
|
last_heartbeat INTEGER,
|
||||||
ip_address TEXT,
|
ip_address TEXT,
|
||||||
android_version TEXT,
|
android_version TEXT,
|
||||||
|
|
|
||||||
109
server/test/device-block-and-auth.test.js
Normal file
109
server/test/device-block-and-auth.test.js
Normal file
|
|
@ -0,0 +1,109 @@
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
// #143 (highest-priority) — auth short-circuit fix + operator kill switch.
|
||||||
|
// - A provisioned device whose token is NULLed is now REJECTED (Bold 75c2a08a:
|
||||||
|
// nulling the token used to RE-PROVISION the device instead of locking it out).
|
||||||
|
// - A `blocked=1` device is refused at the first register gate (the enforceable
|
||||||
|
// kill switch), settable by DIRECT SQLite edit while the server runs (no restart).
|
||||||
|
// - First pairing (token-less, no device_id) and normal auth still work.
|
||||||
|
// Direct DB edits below mimic the operator hand-editing SQLite during an outage.
|
||||||
|
// Unique PORT 3987 (not 3982-3986).
|
||||||
|
|
||||||
|
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 ioClient = require('socket.io-client');
|
||||||
|
const Database = require('better-sqlite3');
|
||||||
|
|
||||||
|
const PORT = 3987;
|
||||||
|
const BASE = `http://127.0.0.1:${PORT}`;
|
||||||
|
const DATA_DIR = path.join(os.tmpdir(), 'st-block-' + crypto.randomBytes(4).toString('hex'));
|
||||||
|
const LOG = path.join(os.tmpdir(), 'st-block-' + crypto.randomBytes(4).toString('hex') + '.log');
|
||||||
|
const DB_PATH = path.join(DATA_DIR, 'db', 'remote_display.db');
|
||||||
|
let proc;
|
||||||
|
let tdb; // ONE long-lived operator-style connection (mirrors how the server holds one);
|
||||||
|
// avoids the cross-process WAL checkpoint churn of many short-lived openers.
|
||||||
|
const sleep = (ms) => new Promise(r => setTimeout(r, ms));
|
||||||
|
|
||||||
|
before(async () => {
|
||||||
|
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 { /* */ } await sleep(250); }
|
||||||
|
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
|
||||||
|
tdb = new Database(DB_PATH);
|
||||||
|
tdb.pragma('busy_timeout = 3000');
|
||||||
|
});
|
||||||
|
after(() => { try { tdb && tdb.close(); } catch { /* */ } try { proc.kill('SIGKILL'); } catch { /* */ } });
|
||||||
|
|
||||||
|
// Operator's direct hand-edit of SQLite while the server is running (no restart) —
|
||||||
|
// a single persistent connection, as a `sqlite3` session would be.
|
||||||
|
function dbEdit(sql, ...params) { return tdb.prepare(sql).run(...params).changes; }
|
||||||
|
|
||||||
|
function register(payload) {
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
const sock = ioClient(`${BASE}/device`, { transports: ['websocket'], reconnection: false, forceNew: true });
|
||||||
|
const got = { registered: false, authError: false, errorMsg: null, playlist: false, newId: null, newToken: null };
|
||||||
|
const finish = () => { try { sock.close(); } catch { /* */ } resolve(got); };
|
||||||
|
sock.on('connect', () => sock.emit('device:register', payload));
|
||||||
|
sock.on('device:registered', (d) => { got.registered = true; got.newId = d.device_id; got.newToken = d.device_token; setTimeout(finish, 250); });
|
||||||
|
sock.on('device:playlist-update', () => { got.playlist = true; });
|
||||||
|
sock.on('device:auth-error', (e) => { got.authError = true; got.errorMsg = e && e.error; finish(); });
|
||||||
|
setTimeout(finish, 4000);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
async function provision() {
|
||||||
|
const g = await register({ pairing_code: String(crypto.randomInt(100000, 1000000)) });
|
||||||
|
return g.registered ? { id: g.newId, token: g.newToken } : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
test('#143 repro: a provisioned device whose token is NULLed is REJECTED (was: re-provisioned)', async () => {
|
||||||
|
const dev = await provision();
|
||||||
|
assert.ok(dev && dev.token, 'provisioned with a token');
|
||||||
|
assert.equal(dbEdit('UPDATE devices SET device_token = NULL WHERE id = ?', dev.id), 1, 'operator nulled the token');
|
||||||
|
const got = await register({ device_id: dev.id, device_token: dev.token, device_info: { app_version: 'test' } });
|
||||||
|
assert.ok(got.authError, 'null-token device is rejected (auth-error)');
|
||||||
|
assert.ok(!got.registered, 'and must NOT register / re-provision');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('kill switch: blocked=1 refuses at the first gate (no register, no playlist)', async () => {
|
||||||
|
const dev = await provision();
|
||||||
|
assert.equal(dbEdit('UPDATE devices SET blocked = 1 WHERE id = ?', dev.id), 1, 'operator blocked the device');
|
||||||
|
// no server restart — the block takes effect on the very next reconnect
|
||||||
|
const got = await register({ device_id: dev.id, device_token: dev.token, device_info: { app_version: 'test' } });
|
||||||
|
assert.ok(got.authError && got.errorMsg === 'Device blocked', 'refused with Device blocked');
|
||||||
|
assert.ok(!got.registered, 'no register');
|
||||||
|
assert.ok(!got.playlist, 'no playlist build (refused at the first gate)');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('unblocking (blocked=0) lets the same device connect again', async () => {
|
||||||
|
const dev = await provision();
|
||||||
|
dbEdit('UPDATE devices SET blocked = 1 WHERE id = ?', dev.id);
|
||||||
|
let got = await register({ device_id: dev.id, device_token: dev.token, device_info: {} });
|
||||||
|
assert.ok(!got.registered, 'blocked first');
|
||||||
|
dbEdit('UPDATE devices SET blocked = 0 WHERE id = ?', dev.id);
|
||||||
|
got = await register({ device_id: dev.id, device_token: dev.token, device_info: {} });
|
||||||
|
assert.ok(got.registered, 'unblocked -> connects normally again');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the pairing/provisioning seam still works: a NEW token-less device first-pairs', async () => {
|
||||||
|
const got = await register({ pairing_code: String(crypto.randomInt(100000, 1000000)) });
|
||||||
|
assert.ok(got.registered, 'first pairing (no device_id) still succeeds');
|
||||||
|
assert.ok(got.newId && got.newToken, 'a fresh device_id + token are issued');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('no regression: a normal device with a valid token registers + gets its playlist', async () => {
|
||||||
|
const dev = await provision();
|
||||||
|
const got = await register({ device_id: dev.id, device_token: dev.token, device_info: { app_version: 'test' } });
|
||||||
|
assert.ok(got.registered, 'valid token registers');
|
||||||
|
assert.ok(got.playlist, 'and receives its playlist');
|
||||||
|
assert.ok(!got.authError, 'no auth error');
|
||||||
|
});
|
||||||
|
|
@ -263,6 +263,24 @@ module.exports = function setupDeviceSocket(io) {
|
||||||
socket.on('device:register', (data) => {
|
socket.on('device:register', (data) => {
|
||||||
const { pairing_code, device_id, device_token, device_info, fingerprint } = data;
|
const { pairing_code, device_id, device_token, device_info, fingerprint } = data;
|
||||||
|
|
||||||
|
// #143 operator KILL SWITCH — the FIRST gate, before the fingerprint block,
|
||||||
|
// the reconnect throttle, any DB writes, or playlist build. A device flagged
|
||||||
|
// `blocked` is refused immediately. Settable by DIRECT SQLite during an outage
|
||||||
|
// (dashboard down): UPDATE devices SET blocked = 1 WHERE id = '<device_id>';
|
||||||
|
// The row is re-read on every register, so a hand-edited UPDATE takes effect on
|
||||||
|
// the device's NEXT reconnect with NO server restart. Unblock: blocked = 0.
|
||||||
|
// Unlike nulling the token (#143: that re-provisioned instead of locking out),
|
||||||
|
// this is an explicit, enforceable block.
|
||||||
|
if (device_id) {
|
||||||
|
const blk = db.prepare('SELECT blocked FROM devices WHERE id = ?').get(device_id);
|
||||||
|
if (blk && blk.blocked) {
|
||||||
|
console.warn(`[blocked] refused device ${device_id} (operator block) from ${getClientIp(socket)}`);
|
||||||
|
socket.emit('device:auth-error', { error: 'Device blocked' });
|
||||||
|
process.nextTick(() => { try { socket.disconnect(true); } catch (_) { /* */ } });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Track device fingerprint to prevent reinstall abuse
|
// Track device fingerprint to prevent reinstall abuse
|
||||||
if (fingerprint) {
|
if (fingerprint) {
|
||||||
try {
|
try {
|
||||||
|
|
@ -353,9 +371,16 @@ module.exports = function setupDeviceSocket(io) {
|
||||||
// reconnected" and reading as connection instability (#134 — there were 1415
|
// reconnected" and reading as connection instability (#134 — there were 1415
|
||||||
// "reconnected" logs against only ~30 real socket connects and 0 heartbeat timeouts).
|
// "reconnected" logs against only ~30 real socket connects and 0 heartbeat timeouts).
|
||||||
const isPlaylistRefresh = currentDeviceId === device_id;
|
const isPlaylistRefresh = currentDeviceId === device_id;
|
||||||
// Validate device token (skip for legacy devices that don't have a token yet)
|
// #143 AUTH FIX: an already-provisioned device (it has a row — every row,
|
||||||
if (device.device_token && !validateDeviceToken(device_id, device_token)) {
|
// even `provisioning`, is created WITH a token) presenting a null/empty/
|
||||||
console.warn(`Invalid device token for ${device_id} from ${getClientIp(socket)} — received_len=${(device_token || '').length}, stored_len=${device.device_token.length}, received_prefix=${(device_token || '').substring(0, 8)}, stored_prefix=${device.device_token.substring(0, 8)}`);
|
// invalid token is NOT authenticated — reject and disconnect. The old guard
|
||||||
|
// `device.device_token && !validate(...)` short-circuited on a NULL stored
|
||||||
|
// token, so nulling a device's token RE-PROVISIONED it (auth skipped + a
|
||||||
|
// fresh token minted) instead of locking it out (Bold #143 / 75c2a08a).
|
||||||
|
// validateDeviceToken already returns false for null-stored/missing/mismatch.
|
||||||
|
// First pairing is the pairing_code path below (no device_id) — unaffected.
|
||||||
|
if (!validateDeviceToken(device_id, device_token)) {
|
||||||
|
console.warn(`Invalid/missing device token for ${device_id} from ${getClientIp(socket)} — received_len=${(device_token || '').length}, has_stored_token=${!!device.device_token}`);
|
||||||
socket.emit('device:auth-error', { error: 'Invalid device token' });
|
socket.emit('device:auth-error', { error: 'Invalid device token' });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -388,12 +413,10 @@ module.exports = function setupDeviceSocket(io) {
|
||||||
db.prepare("UPDATE devices SET status = 'online', last_heartbeat = strftime('%s','now'), ip_address = ?, updated_at = strftime('%s','now') WHERE id = ?")
|
db.prepare("UPDATE devices SET status = 'online', last_heartbeat = strftime('%s','now'), ip_address = ?, updated_at = strftime('%s','now') WHERE id = ?")
|
||||||
.run(getClientIp(socket), device_id);
|
.run(getClientIp(socket), device_id);
|
||||||
|
|
||||||
// Generate token for legacy devices that don't have one yet
|
// #143: past the validateDeviceToken gate above the stored token is
|
||||||
let tokenToSend = device.device_token;
|
// guaranteed non-null, so we just echo it back. The old "mint a token for a
|
||||||
if (!tokenToSend) {
|
// null-token device" path is removed — that was the re-provisioning vector.
|
||||||
tokenToSend = generateDeviceToken();
|
const tokenToSend = device.device_token;
|
||||||
db.prepare('UPDATE devices SET device_token = ? WHERE id = ?').run(tokenToSend, device_id);
|
|
||||||
}
|
|
||||||
|
|
||||||
if (device_info) {
|
if (device_info) {
|
||||||
db.prepare(`UPDATE devices SET android_version = ?, app_version = ?, screen_width = ?, screen_height = ?, render_width = ?, render_height = ?,
|
db.prepare(`UPDATE devices SET android_version = ?, app_version = ?, screen_width = ?, screen_height = ?, render_width = ?, render_height = ?,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue