mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 13:53:12 -06:00
Merge branch 'fix/unblock-clears-saved-block'
This commit is contained in:
commit
10f1dccdc8
|
|
@ -495,6 +495,22 @@ class MainActivity : AppCompatActivity() {
|
|||
// #170: on a fresh network connection, clear stuck download backoff so content that failed
|
||||
// to download while the link was settling retries on the next sweep (the service also
|
||||
// requests a playlist refresh). Keeps single-flight; only touches failure/backoff state.
|
||||
// #234: the server rejecting us (operator block, cleared credentials, reclaim settle) has
|
||||
// to be VISIBLE. Only ProvisioningActivity ever assigned onUnpaired, and it is gone by the
|
||||
// time playback is running — so a rejection left the screen sitting on "Connecting to
|
||||
// server", and the player then blamed the URL. The server always says why; show that.
|
||||
wsService?.onUnpaired = {
|
||||
runOnUiThread {
|
||||
val why = wsService?.lastRejectionReason ?: ""
|
||||
val blocked = why.contains("block", ignoreCase = true)
|
||||
Log.w("MainActivity", "server rejected this device ($why) — surfacing re-pair state")
|
||||
showStatus(
|
||||
if (blocked) getString(R.string.device_blocked_status)
|
||||
else getString(R.string.device_unpaired_status)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
wsService?.onNetworkAvailable = {
|
||||
if (::downloadCoordinator.isInitialized) downloadCoordinator.resetAllBackoff()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -729,6 +729,15 @@ class WebSocketService : Service() {
|
|||
|
||||
/** True from the first server rejection until the device is (re)paired — UI stays on re-pair. */
|
||||
fun isAwaitingRepair(): Boolean = awaitingRepair
|
||||
|
||||
/**
|
||||
* Why the server last refused us, verbatim from device:auth-error (e.g. "Device blocked").
|
||||
* The server always says why; the player used to throw it away and fall back to a generic
|
||||
* connection failure, so an operator block read as "couldn't reach the server, check the url"
|
||||
* and sent people off debugging their network. #234.
|
||||
*/
|
||||
@Volatile var lastRejectionReason: String? = null
|
||||
private set
|
||||
/** Milliseconds left in the reclaim-settle hold (0 once elapsed) — drives the UI countdown. */
|
||||
fun repairHoldRemainingMs(): Long = maxOf(0L, repairHoldUntilMs - SystemClock.elapsedRealtime())
|
||||
/** True only when the shown pairing code is server-accepted (pairable) — not a rejected/stale one. */
|
||||
|
|
@ -748,6 +757,7 @@ class WebSocketService : Service() {
|
|||
* scheduled retry, so the screen is stable — no register/reject/register churn.
|
||||
*/
|
||||
private fun handleServerRejection(reason: String) {
|
||||
lastRejectionReason = reason
|
||||
val settleSec = parseSettleSeconds(reason)
|
||||
Log.w("WebSocketService", "Server rejected device ($reason) — settle=${settleSec}s")
|
||||
pairingCodeLive = false // this registration was rejected — the local code is NOT pairable
|
||||
|
|
@ -778,6 +788,7 @@ class WebSocketService : Service() {
|
|||
|
||||
/** Re-pair complete (device:paired, or a normal authenticated reconnect) — clear all repair state. */
|
||||
private fun resetRepairBackoff() {
|
||||
lastRejectionReason = null
|
||||
repairRetryPending = false
|
||||
repairBackoffMs = 0L
|
||||
awaitingRepair = false
|
||||
|
|
|
|||
|
|
@ -4,4 +4,6 @@
|
|||
<string name="accessibility_description">RemoteDisplay nutzt die Bedienungshilfen, um Fernsteuerung der Stromzufuhr und Systemnavigation zu ermöglichen.</string>
|
||||
<string name="nothing_scheduled">Derzeit ist nichts geplant</string>
|
||||
<string name="waiting_for_content">Warte auf Inhalte…</string>
|
||||
<string name="device_blocked_status">Dieser Bildschirm wurde im Dashboard gesperrt</string>
|
||||
<string name="device_unpaired_status">Dieser Bildschirm wurde entkoppelt — warte auf erneute Kopplung</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -4,4 +4,6 @@
|
|||
<string name="accessibility_description">RemoteDisplay usa accesibilidad para habilitar el control remoto de encendido y la navegación del sistema.</string>
|
||||
<string name="nothing_scheduled">No hay nada programado en este momento</string>
|
||||
<string name="waiting_for_content">Esperando contenido…</string>
|
||||
<string name="device_blocked_status">Esta pantalla ha sido bloqueada en el panel</string>
|
||||
<string name="device_unpaired_status">Esta pantalla se desvinculó — esperando volver a vincularse</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -4,4 +4,6 @@
|
|||
<string name="accessibility_description">RemoteDisplay utilise l\'accessibilité pour activer les contrôles d\'alimentation à distance et la navigation système.</string>
|
||||
<string name="nothing_scheduled">Rien de programmé pour le moment</string>
|
||||
<string name="waiting_for_content">En attente de contenu…</string>
|
||||
<string name="device_blocked_status">Cet écran a été bloqué dans le tableau de bord</string>
|
||||
<string name="device_unpaired_status">Cet écran a été dissocié — en attente d\'un nouvel appairage</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -4,4 +4,6 @@
|
|||
<resources>
|
||||
<string name="app_name">RemoteDisplay</string>
|
||||
<string name="accessibility_description">RemoteDisplay uses accessibility to enable remote power controls and system navigation.</string>
|
||||
<string name="device_blocked_status">यह स्क्रीन डैशबोर्ड में अवरोधित कर दी गई है</string>
|
||||
<string name="device_unpaired_status">यह स्क्रीन अनयुग्मित हो गई — फिर से युग्मित होने की प्रतीक्षा</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -4,4 +4,6 @@
|
|||
<string name="accessibility_description">RemoteDisplay usa acessibilidade para habilitar controles remotos de energia e navegação do sistema.</string>
|
||||
<string name="nothing_scheduled">Nada programado no momento</string>
|
||||
<string name="waiting_for_content">Aguardando conteúdo…</string>
|
||||
<string name="device_blocked_status">Este ecrã foi bloqueado no painel</string>
|
||||
<string name="device_unpaired_status">Este ecrã foi desemparelhado — a aguardar novo emparelhamento</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -39,4 +39,6 @@
|
|||
<string name="settings_perm_notifications">Notifications</string>
|
||||
<string name="settings_perm_hint">Tap \"Open\" to manage permissions in system settings</string>
|
||||
<string name="settings_perm_open">Open settings</string>
|
||||
<string name="device_blocked_status">This screen has been blocked in the dashboard</string>
|
||||
<string name="device_unpaired_status">This screen was unpaired — waiting to be re-paired</string>
|
||||
</resources>
|
||||
|
|
|
|||
|
|
@ -121,4 +121,23 @@ function purgeWorkspaces(dbConn, workspaceIds) {
|
|||
return (dbConn || db).prepare(`DELETE FROM device_settings WHERE workspace_id IN (${ph})`).run(...ids).changes;
|
||||
}
|
||||
|
||||
module.exports = { snapshot, applyToDevice, listRemoved, getByFingerprint, purgeWorkspaces, validOrientation, ORIENTATIONS };
|
||||
/**
|
||||
* Mirror a device's blocked flag onto its SAVED settings.
|
||||
*
|
||||
* applyToDevice deliberately restores `blocked` across a re-pair, so a block cannot be shrugged off
|
||||
* by deleting the device. That is right — but it also means the saved copy is the real authority for
|
||||
* anything that outlives the device row, and unblocking used to touch only `devices`. The saved copy
|
||||
* stayed 1, so the very next delete-and-re-pair restored the block: from the operator's side, unblock
|
||||
* simply did not take, and there was no way out of it from the dashboard at all.
|
||||
*
|
||||
* No-ops when the device has no fingerprint yet (nothing to key the saved row on).
|
||||
*/
|
||||
function setBlockedByDevice(deviceId, blocked) {
|
||||
const fp = _fpForDevice.get(deviceId)?.fingerprint;
|
||||
if (!fp) return false;
|
||||
const r = db.prepare("UPDATE device_settings SET blocked = ?, last_seen = strftime('%s','now') WHERE fingerprint = ?")
|
||||
.run(blocked ? 1 : 0, fp);
|
||||
return r.changes > 0;
|
||||
}
|
||||
|
||||
module.exports = { snapshot, applyToDevice, listRemoved, getByFingerprint, purgeWorkspaces, setBlockedByDevice, validOrientation, ORIENTATIONS };
|
||||
|
|
|
|||
|
|
@ -282,6 +282,9 @@ router.post('/:id/block', (req, res) => {
|
|||
const device = checkDeviceOwnership(req, res);
|
||||
if (!device) return;
|
||||
db.prepare("UPDATE devices SET blocked = 1, updated_at = strftime('%s','now') WHERE id = ?").run(req.params.id);
|
||||
// Mirror onto the saved settings so the block survives a delete + re-pair on purpose rather than
|
||||
// by accident of whatever the saved copy happened to hold.
|
||||
try { deviceSettings.setBlockedByDevice(req.params.id, true); } catch (e) { console.warn(`[blocked] save mirror failed: ${e.message}`); }
|
||||
console.warn(`[blocked] device ${req.params.id} blocked via dashboard (user ${req.user.id})`);
|
||||
res.json({ success: true, id: req.params.id, blocked: true });
|
||||
});
|
||||
|
|
@ -289,6 +292,10 @@ router.post('/:id/unblock', (req, res) => {
|
|||
const device = checkDeviceOwnership(req, res);
|
||||
if (!device) return;
|
||||
db.prepare("UPDATE devices SET blocked = 0, updated_at = strftime('%s','now') WHERE id = ?").run(req.params.id);
|
||||
// MUST clear the saved copy too. applyToDevice() restores `blocked` on re-pair, so leaving the
|
||||
// saved 1 in place made unblock temporary: the next delete + re-pair silently re-blocked the
|
||||
// device, with nothing in the dashboard to explain it and no way for the operator to escape.
|
||||
try { deviceSettings.setBlockedByDevice(req.params.id, false); } catch (e) { console.warn(`[blocked] save mirror failed: ${e.message}`); }
|
||||
console.log(`[blocked] device ${req.params.id} unblocked via dashboard (user ${req.user.id})`);
|
||||
res.json({ success: true, id: req.params.id, blocked: false });
|
||||
});
|
||||
|
|
|
|||
94
server/test/unblock-clears-saved-block.test.js
Normal file
94
server/test/unblock-clears-saved-block.test.js
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
'use strict';
|
||||
|
||||
// A blocked device stays blocked across a delete + re-pair on purpose (device-settings applyToDevice
|
||||
// restores `blocked`, so a block cannot be shrugged off by deleting the device). That is the right
|
||||
// call — but it makes the SAVED copy the real authority, and unblocking only ever wrote `devices`.
|
||||
//
|
||||
// So unblock did not stick. The device row said 0, the saved row still said 1, and the next re-pair
|
||||
// restored the block. From the dashboard there was no way out: a customer unblocked, re-paired, was
|
||||
// refused again, and had nothing to tell them why. Found on #234 with a real device that had been
|
||||
// blocked once to see what the button did.
|
||||
//
|
||||
// The invariant: after unblock, NOTHING anywhere still claims the device is blocked.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-unblock-'));
|
||||
process.env.DATA_DIR = tmp;
|
||||
|
||||
const { db } = require('../db/database');
|
||||
const deviceSettings = require('../lib/device-settings');
|
||||
|
||||
// devices -> workspaces -> organizations -> users, all FK-enforced, so seed the whole chain.
|
||||
function seedWorkspace(suffix) {
|
||||
const u = 'u-' + suffix, o = 'o-' + suffix, ws = 'ws-' + suffix;
|
||||
db.prepare("INSERT OR IGNORE INTO users (id, email, password_hash) VALUES (?, ?, 'x')").run(u, suffix + '@test.local');
|
||||
db.prepare('INSERT OR IGNORE INTO organizations (id, name, owner_user_id) VALUES (?, ?, ?)').run(o, 'org ' + suffix, u);
|
||||
db.prepare('INSERT OR IGNORE INTO workspaces (id, organization_id, name) VALUES (?, ?, ?)').run(ws, o, 'ws ' + suffix);
|
||||
return ws;
|
||||
}
|
||||
|
||||
function seedBlockedDevice(id, fp) {
|
||||
const ws = seedWorkspace(id);
|
||||
db.prepare("INSERT INTO devices (id, name, workspace_id, blocked, created_at, updated_at) VALUES (?, ?, ?, 1, strftime('%s','now'), strftime('%s','now'))")
|
||||
.run(id, 'blocked-device', ws);
|
||||
db.prepare("INSERT INTO device_fingerprints (fingerprint, device_id, last_seen) VALUES (?, ?, strftime('%s','now'))")
|
||||
.run(fp, id);
|
||||
// The saved snapshot the re-pair path reads back.
|
||||
db.prepare("INSERT INTO device_settings (fingerprint, workspace_id, device_name, blocked, last_seen) VALUES (?, ?, ?, 1, strftime('%s','now'))")
|
||||
.run(fp, ws, 'blocked-device');
|
||||
return { id, fp, ws };
|
||||
}
|
||||
|
||||
const savedBlocked = (fp) => db.prepare('SELECT blocked FROM device_settings WHERE fingerprint = ?').get(fp)?.blocked;
|
||||
const liveBlocked = (id) => db.prepare('SELECT blocked FROM devices WHERE id = ?').get(id)?.blocked;
|
||||
|
||||
test('THE BUG: clearing devices.blocked alone leaves the saved copy blocked', () => {
|
||||
const { id, fp } = seedBlockedDevice('dev-stale', 'fp-stale');
|
||||
// What unblock used to do, and only this.
|
||||
db.prepare('UPDATE devices SET blocked = 0 WHERE id = ?').run(id);
|
||||
assert.equal(liveBlocked(id), 0);
|
||||
assert.equal(savedBlocked(fp), 1, 'the saved copy is what re-pair restores from');
|
||||
});
|
||||
|
||||
test('a re-pair after that half-unblock puts the block straight back', () => {
|
||||
const { id, fp } = seedBlockedDevice('dev-repair', 'fp-repair');
|
||||
db.prepare('UPDATE devices SET blocked = 0 WHERE id = ?').run(id);
|
||||
// This is exactly what the register path runs on a re-paired device.
|
||||
deviceSettings.applyToDevice(id, fp);
|
||||
assert.equal(liveBlocked(id), 1, 'unblock silently reverted — the customer cannot escape this');
|
||||
});
|
||||
|
||||
test('THE FIX: unblocking clears the saved copy, so a re-pair stays unblocked', () => {
|
||||
const { id, fp } = seedBlockedDevice('dev-fixed', 'fp-fixed');
|
||||
db.prepare('UPDATE devices SET blocked = 0 WHERE id = ?').run(id);
|
||||
assert.equal(deviceSettings.setBlockedByDevice(id, false), true);
|
||||
assert.equal(savedBlocked(fp), 0);
|
||||
deviceSettings.applyToDevice(id, fp);
|
||||
assert.equal(liveBlocked(id), 0, 're-pair must not resurrect the block');
|
||||
});
|
||||
|
||||
test('blocking still survives a re-pair — that property is deliberate and must not regress', () => {
|
||||
const { id, fp } = seedBlockedDevice('dev-stays', 'fp-stays');
|
||||
db.prepare('UPDATE devices SET blocked = 0 WHERE id = ?').run(id);
|
||||
deviceSettings.setBlockedByDevice(id, false);
|
||||
// Now block it for real, the way the route does.
|
||||
db.prepare('UPDATE devices SET blocked = 1 WHERE id = ?').run(id);
|
||||
deviceSettings.setBlockedByDevice(id, true);
|
||||
// Simulate delete + re-pair: the device row is recreated unblocked, then settings are restored.
|
||||
db.prepare('UPDATE devices SET blocked = 0 WHERE id = ?').run(id);
|
||||
deviceSettings.applyToDevice(id, fp);
|
||||
assert.equal(liveBlocked(id), 1, 'a genuine block must not be escapable by deleting the device');
|
||||
});
|
||||
|
||||
test('a device with no fingerprint yet is a no-op, not a throw', () => {
|
||||
const ws = seedWorkspace('nofp');
|
||||
db.prepare("INSERT INTO devices (id, name, workspace_id, blocked, created_at, updated_at) VALUES ('dev-nofp','x',?,1,strftime('%s','now'),strftime('%s','now'))").run(ws);
|
||||
assert.equal(deviceSettings.setBlockedByDevice('dev-nofp', false), false);
|
||||
});
|
||||
|
||||
test.after(() => { try { fs.rmSync(tmp, { recursive: true, force: true }); } catch (_) {} });
|
||||
Loading…
Reference in a new issue