mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 13:53:12 -06:00
fix(pairing): reinstalled panel reclaims its device row instead of being blocked [Bold] (#180)
Bold Media Group's fleet broke on the 1.9.3->1.9.6 upgrade. Their MDM does an
uninstall/reinstall (app data wiped), so the player registers with
{ pairing_code, fingerprint } and NO device_id and shows a pairing code — but the
dashboard reported "code does not exist". Deleting the device_fingerprints row fixed
it, which pinpointed the fingerprint-reclaim guard in server/ws/deviceSocket.js.
Root cause: the reclaim guard was
`stillAlive = !!liveConn || secondsSince < reclaimSettleSeconds; if (stillAlive) reject`.
On an in-place reinstall the old row heartbeat seconds ago, so `secondsSince < 300` is
ALWAYS true -> it emitted device:auth-error and returned BEFORE the pairing_code INSERT,
so the code the player displayed never existed server-side.
The settle window's real purpose was to REMATCH an existing fingerprint back to its
device row on reinstall — not to force a fresh re-pair. So the fix keys off claim status,
not the timer (server-only; no APK change — reviewed and confirmed unnecessary):
- Reject ONLY when the old row has a genuinely LIVE socket (liveConn) — the real anti-
hijack boundary. Unchanged.
- CLAIMED old row (user_id set) -> RECLAIM it regardless of the settle window: reuse the
row, rotate the token, emit device:registered{online} + device:paired. The panel returns
straight to paired (no operator re-pair, no orphaned duplicate row), preserving name /
claim / playlist / content. device:paired drives the app off the pairing screen, so the
fresh code it showed is irrelevant.
- UNCLAIMED old row -> fall through to the pairing_code path and PROVISION FRESH with the
shown code (reclaiming would leave a stale/null code -> "code does not exist"). #150
relinks the fingerprint to the new row.
`reclaimSettleSeconds` is now vestigial for this path. Trade-off: a fingerprint-only reclaim
of a CLAIMED-but-offline device is no longer delayed ~300s — not a new attack class (the old
code already granted it once the window elapsed); liveConn remains the hard boundary. Truly
closing that window without a re-pair needs client keystore attestation (a future APK).
Also fixes a latent crash this newly exercises: middleware/subscription.js getUserPlan()
dereferenced an undefined user in its else branch ("Cannot set properties of undefined
(setting 'trial_active')") when the user/plan JOIN missed. Under the claimed-reclaim path
that ran checkDeviceAccess->getUserPlan, the throw was swallowed by the reclaim try/catch and
silently dropped the device to provision-fresh. Guard: `if (!user) return null`.
Tests (server/test/fingerprint-reclaim.test.js):
- NEW: a CLAIMED reinstall reclaims the SAME row, emits device:paired, creates no duplicate,
keeps the fingerprint linked — regardless of the settle window (the Bold repro, fixed right).
- NEW: recent heartbeat + no live socket, UNCLAIMED -> provisions fresh with the shown code.
- NEW: a LIVE old socket still rejects and creates no new row (security preserved).
- Updated the #143 gone-device test to expect provision-fresh for an unclaimed row, and the
log-noise assertion to the "reclaim rejected" message.
465/465 server tests pass. Server-only: NOT deployed, no version bump, Android untouched.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
screentinker
GitHub
parent
cf4c71d7d0
commit
51b0b006b1
|
|
@ -13,8 +13,15 @@ function getUserPlan(userId) {
|
|||
WHERE u.id = ?
|
||||
`).get(userId);
|
||||
|
||||
// No user row (or no joinable plan) — return null so callers treat it as unrestricted
|
||||
// (checkDeviceAccess: `if (!plan) return { allowed: true }`). Previously the else branch
|
||||
// below dereferenced an undefined `user` ("Cannot set properties of undefined"), which — once
|
||||
// a claimed device's reclaim runs checkDeviceAccess — was swallowed by the caller's try/catch
|
||||
// and silently dropped the device to the provision-fresh path instead of reclaiming it.
|
||||
if (!user) return null;
|
||||
|
||||
// Check if trial has expired
|
||||
if (user && user.trial_started) {
|
||||
if (user.trial_started) {
|
||||
const trialEnd = user.trial_started + (TRIAL_DAYS * 86400);
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
user.trial_active = now < trialEnd;
|
||||
|
|
|
|||
|
|
@ -41,9 +41,9 @@ before(async () => {
|
|||
after(() => { try { tdb && tdb.close(); } catch { /* */ } try { proc.kill('SIGKILL'); } catch { /* */ } });
|
||||
|
||||
// Seed a device + its fingerprint link directly (no socket -> no lingering liveConn).
|
||||
function seedDevice(fp, { token, heartbeatAgo }) {
|
||||
function seedDevice(fp, { token, heartbeatAgo, userId = null }) {
|
||||
const id = crypto.randomUUID();
|
||||
tdb.prepare("INSERT INTO devices (id, status, last_heartbeat, device_token) VALUES (?, 'offline', strftime('%s','now') - ?, ?)").run(id, heartbeatAgo, token);
|
||||
tdb.prepare("INSERT INTO devices (id, status, last_heartbeat, device_token, user_id) VALUES (?, 'offline', strftime('%s','now') - ?, ?, ?)").run(id, heartbeatAgo, token, userId);
|
||||
tdb.prepare('INSERT INTO device_fingerprints (fingerprint, device_id) VALUES (?, ?)').run(fp, id);
|
||||
return { id, token };
|
||||
}
|
||||
|
|
@ -52,10 +52,12 @@ function staleHeartbeat(id, ago) { tdb.prepare("UPDATE devices SET last_heartbea
|
|||
function attempt(payload) { // one-shot register; resolves and closes
|
||||
return new Promise((resolve) => {
|
||||
const sock = ioClient(`${BASE}/device`, { transports: ['websocket'], reconnection: false, forceNew: true });
|
||||
const got = { registered: false, newId: null, authError: false, errorMsg: null };
|
||||
const got = { registered: false, newId: null, authError: false, errorMsg: null, paired: false, pairedId: 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; setTimeout(finish, 150); });
|
||||
// device:paired arrives right after device:registered on a claimed reclaim — wait 200ms to catch it.
|
||||
sock.on('device:registered', (d) => { got.registered = true; got.newId = d.device_id; setTimeout(finish, 200); });
|
||||
sock.on('device:paired', (d) => { got.paired = true; got.pairedId = d && d.device_id; });
|
||||
sock.on('device:auth-error', (e) => { got.authError = true; got.errorMsg = e && e.error; finish(); });
|
||||
setTimeout(finish, 4000);
|
||||
});
|
||||
|
|
@ -71,13 +73,15 @@ function connectLive(payload) { // keeps the socket open (live connection); call
|
|||
}
|
||||
const rnd = () => String(crypto.randomInt(100000, 1000000));
|
||||
|
||||
test('#143 repro: a gone device (no live conn + stale heartbeat) is reclaimable', async () => {
|
||||
test('#143 no reclaim-loop: an UNCLAIMED gone device provisions fresh with the shown code', async () => {
|
||||
const fp = 'fp-gone-' + crypto.randomBytes(4).toString('hex');
|
||||
const dev = seedDevice(fp, { token: 'tok', heartbeatAgo: 99999 }); // ~27h stale, never connected
|
||||
const r = await attempt({ pairing_code: rnd(), fingerprint: fp }); // no device_id -> reclaim path
|
||||
assert.ok(r.registered, 'reclaim SUCCEEDS for a gone device');
|
||||
assert.equal(r.newId, dev.id, 'it reclaims the SAME device identity');
|
||||
assert.ok(!r.authError, 'no rejection');
|
||||
const dev = seedDevice(fp, { token: 'tok', heartbeatAgo: 99999 }); // unclaimed, ~27h stale, never connected
|
||||
const code = rnd();
|
||||
const r = await attempt({ pairing_code: code, fingerprint: fp }); // no device_id
|
||||
assert.ok(r.registered && !r.authError, 'registers cleanly — no stuck reclaim/retry loop (#143)');
|
||||
assert.notEqual(r.newId, dev.id, 'an UNCLAIMED old row is NOT reclaimed (its stale code would break pairing) — a fresh row is provisioned');
|
||||
const row = tdb.prepare('SELECT status FROM devices WHERE pairing_code = ?').get(code);
|
||||
assert.ok(row && row.status === 'provisioning', 'the on-screen code is inserted as a fresh provisioning row');
|
||||
});
|
||||
|
||||
test('no regression: a genuinely live device REJECTS a fingerprint reclaim', async () => {
|
||||
|
|
@ -90,6 +94,60 @@ test('no regression: a genuinely live device REJECTS a fingerprint reclaim', asy
|
|||
try { live.sock.close(); } catch { /* */ }
|
||||
});
|
||||
|
||||
// Bold 1.9.3->1.9.6 upgrade regression: a reinstall registers { pairing_code, fingerprint,
|
||||
// no device_id } while the OLD device row heartbeat only seconds ago but has NO live socket
|
||||
// (the app was uninstalled). The old guard treated the recent heartbeat as "still alive" and
|
||||
// returned before the pairing_code INSERT, so the code the player displayed was never created
|
||||
// and the dashboard said "code does not exist". It must now PROVISION FRESH with that code.
|
||||
test('Bold upgrade: recent heartbeat + NO live socket provisions fresh with the shown code', async () => {
|
||||
const fp = 'fp-upgrade-' + crypto.randomBytes(4).toString('hex');
|
||||
const dev = seedDevice(fp, { token: 'tokU', heartbeatAgo: 10 }); // heartbeat 10s ago, never socket-connected
|
||||
const code = rnd();
|
||||
const r = await attempt({ pairing_code: code, fingerprint: fp }); // no device_id
|
||||
assert.ok(r.registered, 'the reinstalled device provisions (device:registered), not blocked');
|
||||
assert.ok(!r.authError, 'not rejected by the reclaim-settle guard');
|
||||
const row = tdb.prepare('SELECT * FROM devices WHERE pairing_code = ?').get(code);
|
||||
assert.ok(row, 'a devices row exists carrying the pairing_code the player is showing');
|
||||
assert.equal(row.status, 'provisioning', 'provisioned as a new, unclaimed device');
|
||||
assert.notEqual(row.id, dev.id, 'a fresh row — not a silent reclaim of the old identity');
|
||||
assert.equal(r.newId, row.id, 'the client is told its new device_id');
|
||||
const fpRow = tdb.prepare('SELECT device_id FROM device_fingerprints WHERE fingerprint = ?').get(fp);
|
||||
assert.equal(fpRow.device_id, row.id, '#150: fingerprint relinked to the new device row');
|
||||
});
|
||||
|
||||
// The security boundary must survive the fix: if the OLD device still has a genuinely LIVE
|
||||
// socket, a fingerprint clash must be rejected and NOT provision a new row.
|
||||
test('security preserved: a LIVE old socket still rejects provisioning (no new row created)', async () => {
|
||||
const fp = 'fp-live2-' + crypto.randomBytes(4).toString('hex');
|
||||
const dev = seedDevice(fp, { token: 'tokL', heartbeatAgo: 10 });
|
||||
const live = await connectLive({ device_id: dev.id, device_token: 'tokL', device_info: {} });
|
||||
assert.ok(live.registered, 'old device has a LIVE socket');
|
||||
const code = rnd();
|
||||
const r = await attempt({ pairing_code: code, fingerprint: fp });
|
||||
assert.ok(r.authError && !r.registered, 'rejected while the old device is genuinely live');
|
||||
const row = tdb.prepare('SELECT id FROM devices WHERE pairing_code = ?').get(code);
|
||||
assert.ok(!row, 'no new device row is provisioned for a clash against a live device');
|
||||
try { live.sock.close(); } catch { /* */ }
|
||||
});
|
||||
|
||||
// The claim-status fix: a CLAIMED panel that reinstalls (fingerprint only, no live socket) must
|
||||
// REMATCH its existing row — preserve the claim/name/content, no operator re-pair, no orphaned
|
||||
// duplicate — regardless of the settle window. This is what keeps a fleet upgrade seamless.
|
||||
test('claimed reinstall RECLAIMS its row (no re-pair, no duplicate) regardless of the settle window', async () => {
|
||||
const fp = 'fp-claimed-' + crypto.randomBytes(4).toString('hex');
|
||||
// CLAIMED (user_id set), heartbeat only 10s ago (well inside the old settle window), no live socket.
|
||||
const dev = seedDevice(fp, { token: 'tokC', heartbeatAgo: 10, userId: 'user-' + crypto.randomBytes(3).toString('hex') });
|
||||
const before = tdb.prepare('SELECT COUNT(*) c FROM devices').get().c;
|
||||
const code = rnd();
|
||||
const r = await attempt({ pairing_code: code, fingerprint: fp }); // reinstall: fingerprint only
|
||||
assert.ok(r.registered && !r.authError, 'registers');
|
||||
assert.equal(r.newId, dev.id, 'rematches the SAME claimed device row — identity/claim preserved');
|
||||
assert.ok(r.paired, 'a claimed row emits device:paired, so the panel returns straight to paired (no code screen)');
|
||||
assert.equal(tdb.prepare('SELECT COUNT(*) c FROM devices').get().c, before, 'no new device row created (no fleet duplication)');
|
||||
assert.ok(!tdb.prepare('SELECT id FROM devices WHERE pairing_code = ?').get(code), 'the on-screen code is NOT provisioned as a separate row');
|
||||
assert.equal(tdb.prepare('SELECT device_id FROM device_fingerprints WHERE fingerprint = ?').get(fp).device_id, dev.id, 'the fingerprint stays linked to the reclaimed row');
|
||||
});
|
||||
|
||||
test('clear-on-leave: after disconnect, liveConn is cleared so a (stale) device reclaims', async () => {
|
||||
const fp = 'fp-leave-' + crypto.randomBytes(4).toString('hex');
|
||||
const dev = seedDevice(fp, { token: 'tok3', heartbeatAgo: 99999 });
|
||||
|
|
@ -113,6 +171,6 @@ test('log noise: a retried reclaim logs at most once per device per window', asy
|
|||
for (let i = 0; i < 4; i++) { const r = await attempt({ pairing_code: rnd(), fingerprint: fp }); assert.ok(r.authError, 'each retry is deferred'); }
|
||||
try { live.sock.close(); } catch { /* */ }
|
||||
await sleep(200);
|
||||
const lines = fs.readFileSync(LOG, 'utf8').split('\n').filter(l => l.includes('reclaim deferred for ' + dev.id)).length;
|
||||
assert.ok(lines <= 1, `at most one deferral log per window (got ${lines}); no double-log / per-2s flood`);
|
||||
const lines = fs.readFileSync(LOG, 'utf8').split('\n').filter(l => l.includes('reclaim rejected for ' + dev.id)).length;
|
||||
assert.ok(lines <= 1, `at most one rejection log per window (got ${lines}); no double-log / per-2s flood`);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -470,20 +470,41 @@ module.exports = function setupDeviceSocket(io) {
|
|||
const liveConn = heartbeat.getConnection(existing.device_id);
|
||||
const lastBeat = oldDevice.last_heartbeat || 0;
|
||||
const secondsSince = Math.floor(Date.now() / 1000) - lastBeat;
|
||||
const stillAlive = !!liveConn || secondsSince < config.reclaimSettleSeconds;
|
||||
if (stillAlive) {
|
||||
// Bold 1.9.3->1.9.6 upgrade fix: a reinstalled panel registers with
|
||||
// { pairing_code, fingerprint } and NO device_id — it is asking to be
|
||||
// PROVISIONED, not to reclaim. The ONLY grounds to reject is a genuinely
|
||||
// LIVE socket on the old device row (stops a duplicated fingerprint from
|
||||
// hijacking an actively-connected display). The previous guard also rejected
|
||||
// on a merely-recent heartbeat (secondsSince < reclaimSettleSeconds), which
|
||||
// on an in-place-reinstall upgrade is ALWAYS true — so it returned before the
|
||||
// pairing_code INSERT and the code the player was displaying was never created,
|
||||
// leaving the dashboard with "code does not exist" until device_fingerprints
|
||||
// was cleared by hand. The real security boundary is the operator claiming the
|
||||
// code in the dashboard; a cloned fingerprint that falls through only gets an
|
||||
// unclaimed, tokenless, content-less row — harmless.
|
||||
if (liveConn) {
|
||||
// Log at most once per device per window so a retrying/stuck device can't flood stdout.
|
||||
const nowMs = Date.now();
|
||||
if (nowMs - (lastReclaimRejectLogAt.get(existing.device_id) || 0) >= config.reclaimRejectLogWindowMs) {
|
||||
lastReclaimRejectLogAt.set(existing.device_id, nowMs);
|
||||
console.warn(`Fingerprint reclaim deferred for ${existing.device_id}: still settling (status=${oldDevice.status}, ${secondsSince}s since heartbeat, liveConn=${!!liveConn}); reclaimable after ${config.reclaimSettleSeconds}s offline`);
|
||||
console.warn(`Fingerprint reclaim rejected for ${existing.device_id}: old device has a LIVE socket (status=${oldDevice.status}, ${secondsSince}s since heartbeat)`);
|
||||
}
|
||||
socket.emit('device:auth-error', {
|
||||
error: `This display was recently active. If you reinstalled the app, retry after it has been offline for ${config.reclaimSettleSeconds} seconds.`
|
||||
error: 'This display is currently active on another connection.'
|
||||
});
|
||||
return;
|
||||
}
|
||||
lastReclaimRejectLogAt.delete(existing.device_id); // reclaim proceeding — clear any deferral log state
|
||||
// No live socket from here on — the old connection is gone.
|
||||
lastReclaimRejectLogAt.delete(existing.device_id);
|
||||
if (oldDevice.user_id) {
|
||||
// The old row is CLAIMED. A reinstalled panel (MDM wiped its data, so it presents
|
||||
// only its stable fingerprint — no device_id, no token) must REMATCH to this row —
|
||||
// preserving the claim, name, playlist, and content — instead of provisioning a
|
||||
// stranger the operator has to re-pair across the whole fleet. device:paired below
|
||||
// drives the app off the pairing screen, so the fresh code it was displaying is
|
||||
// irrelevant. We reclaim regardless of the settle window: liveConn (checked above)
|
||||
// is the real anti-hijack boundary; the settle delay only postponed a
|
||||
// fingerprint-only reclaim the server already grants once the window elapses.
|
||||
|
||||
// Fingerprint matched — this is a reinstalled app reconnecting to its old device.
|
||||
// Issue a fresh token so the app can authenticate going forward.
|
||||
|
|
@ -526,6 +547,12 @@ module.exports = function setupDeviceSocket(io) {
|
|||
}
|
||||
return;
|
||||
}
|
||||
// The old row is UNCLAIMED (never paired). Reclaiming it would leave it carrying a
|
||||
// stale/null pairing_code while the player shows a fresh one -> "code does not exist".
|
||||
// Fall through to the pairing_code path below, which provisions a fresh row with the
|
||||
// on-screen code and (#150) relinks the fingerprint to it. reclaimSettleSeconds no
|
||||
// longer gates this path — a genuinely live socket (above) is the only rejection.
|
||||
}
|
||||
}
|
||||
} else if (device_id || pairing_code) {
|
||||
// device_id can be stale (e.g. a reconnect after the device row was
|
||||
|
|
|
|||
Loading…
Reference in a new issue