From 35ce0401a8bcb72c4d0ed236c3404c8ae7cd8ad1 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 01:14:07 +0200 Subject: [PATCH] feat(client/android): hosts get a stable identity, and own their clipboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host store was keyed by `"address:port"`, which had two consequences. Editing a host's address had to re-key its record — delete the old key, write the new one — and nothing could hold a durable reference to a host at all, because the key moved whenever the host did. Profile bindings, pinned cards and `punktfunk://` shortcuts all need exactly such a reference, so the store is now keyed by a minted stable id: a lowercase UUID, the same shape Apple's `StoredHost.id` and the Rust `KnownHost.id` already carry, so a host reference is one grammar on every platform. The re-key-on-edit dance is gone; an edit is a plain save. `clipboardSync` moves from a global onto the record. It was always a decision about a HOST — whether text on this device may cross to that machine — and one global for the work box and the couch box was the wrong shape; the desktop clients have had it per-host since it shipped. It is edited from the host's Edit sheet, which is also where the profile binding will live. Both, plus the minted ids, ride ONE migration pass. The store is being rewritten anyway and every extra pass is another chance to strand somebody's hosts. It runs once against real user data, so its pure half is tested against a verbatim pre-migration blob — every host survives with its pin, paired flag and MACs, IPv6 addresses included; each lands on its own id; whatever the global clipboard setting said lands on every host, on or off; and a second pass over its own output changes nothing. Re-trusting a host now goes through `KnownHostStore.trust`, which preserves the existing record's identity and everything the user set on it. Three call sites used to construct a fresh `KnownHost` — harmless while the key was the address, but with an id it would have forked the record on every re-pair. While the wiring was open: `StreamScreen` takes an `ActiveSession` — the settings the connect actually resolved, plus that host's clipboard answer — instead of re-reading `SettingsStore` behind its own connect's back. That is also the seam profiles need. --- .../src/main/kotlin/io/unom/punktfunk/App.kt | 20 +- .../io/unom/punktfunk/ConnectDialogs.kt | 28 ++- .../kotlin/io/unom/punktfunk/ConnectScreen.kt | 29 ++- .../kotlin/io/unom/punktfunk/LibraryScreen.kt | 9 +- .../main/kotlin/io/unom/punktfunk/Settings.kt | 14 +- .../io/unom/punktfunk/SettingsScreen.kt | 9 +- .../kotlin/io/unom/punktfunk/StreamScreen.kt | 15 +- .../io/unom/punktfunk/models/UiModels.kt | 15 ++ clients/android/kit/build.gradle.kts | 6 +- .../punktfunk/kit/security/KnownHostStore.kt | 225 +++++++++++++++--- .../kit/security/KnownHostStoreTest.kt | 108 +++++++++ 11 files changed, 392 insertions(+), 86 deletions(-) diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/App.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/App.kt index d77232b1..8e87cec4 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/App.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/App.kt @@ -32,7 +32,6 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableLongStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -42,6 +41,7 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.dp +import io.unom.punktfunk.models.ActiveSession import io.unom.punktfunk.models.Tab @Composable @@ -49,7 +49,9 @@ fun App(forceGamepadUi: Boolean = false) { val context = LocalContext.current val settingsStore = remember { SettingsStore(context) } var settings by remember { mutableStateOf(settingsStore.load()) } - var streamHandle by remember { mutableLongStateOf(0L) } // 0 = not streaming + // The active session (null = not streaming). It carries the settings the connect resolved, + // so the stream screen never re-reads the store behind its own connect's back. + var session by remember { mutableStateOf(null) } var tab by remember { mutableStateOf(Tab.Connect) } // Console (gamepad) mode mirrors the Apple client: the setting AND (a pad is attached OR this is @@ -59,20 +61,20 @@ fun App(forceGamepadUi: Boolean = false) { val gamepadUi = gamepadUiActive(settings.gamepadUiEnabled, controllerConnected, tv, forceGamepadUi) AnimatedContent( - targetState = streamHandle != 0L, + targetState = session, transitionSpec = { fadeIn() togetherWith fadeOut() }, label = "StreamTransition" - ) { isStreaming -> - if (isStreaming) { + ) { active -> + if (active != null) { // Immersive: the stream takes the whole screen, no bottom bar. - StreamScreen(streamHandle, micEnabled = settings.micEnabled, onDisconnect = { streamHandle = 0L }) + StreamScreen(active, onDisconnect = { session = null }) } else if (gamepadUi) { GamepadShell( settings = settings, onSettingsChange = { settings = it; settingsStore.save(it) }, - onConnected = { streamHandle = it }, + onConnected = { session = it }, ) } else { // Adaptive nav: a bottom bar on phones; on tablets / large windows a side NavigationRail @@ -103,7 +105,7 @@ fun App(forceGamepadUi: Boolean = false) { label = "TabTransition" ) { targetTab -> when (targetTab) { - Tab.Connect -> ConnectScreen(settings = settings, onConnected = { streamHandle = it }) + Tab.Connect -> ConnectScreen(settings = settings, onConnected = { session = it }) Tab.Settings -> SettingsScreen( initial = settings, onChange = { settings = it; settingsStore.save(it) }, @@ -167,7 +169,7 @@ private enum class GamepadScreen { Home, Settings, Library } fun GamepadShell( settings: Settings, onSettingsChange: (Settings) -> Unit, - onConnected: (Long) -> Unit, + onConnected: (ActiveSession) -> Unit, ) { val context = LocalContext.current var screen by remember { mutableStateOf(GamepadScreen.Home) } diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectDialogs.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectDialogs.kt index 377dd14f..0ad4a9cb 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectDialogs.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectDialogs.kt @@ -19,6 +19,7 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.rememberModalBottomSheetState @@ -353,11 +354,12 @@ internal fun AwaitingApprovalDialog(hostLabel: String, onCancel: () -> Unit) { } /** - * Edit a saved host: name, address, port, and the Wake-on-LAN MAC. The MAC is auto-learned from the - * host's mDNS advert while it's online, but this is where you can enter or correct it (e.g. to wake a - * host you've only ever reached by address). [suggestedMacs] prefills the field from the live advert - * when nothing's been learned yet. Keyed by the host so reopening resets the fields. Mirrors the - * Apple client's edit form. + * Edit a saved host: name, address, port, the Wake-on-LAN MAC, and the per-host settings the record + * owns — shared clipboard (a trust decision about THIS machine, so it was never really a global). + * The MAC is auto-learned from the host's mDNS advert while it's online, but this is where you can + * enter or correct it (e.g. to wake a host you've only ever reached by address). [suggestedMacs] + * prefills the field from the live advert when nothing's been learned yet. Keyed by the host so + * reopening resets the fields. Mirrors the Apple client's edit form. */ @Composable internal fun EditHostDialog( @@ -372,6 +374,7 @@ internal fun EditHostDialog( var mac by remember(target) { mutableStateOf(target.mac.ifEmpty { suggestedMacs }.joinToString(", ")) } + var clipboard by remember(target) { mutableStateOf(target.clipboardSync) } AlertDialog( onDismissRequest = onDismiss, title = { Text("Edit host") }, @@ -407,6 +410,20 @@ internal fun EditHostDialog( placeholder = { Text("auto-filled when the host is seen") }, singleLine = true, ) + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + ) { + Column(Modifier.weight(1f)) { + Text("Shared clipboard", style = MaterialTheme.typography.bodyLarge) + Text( + "Text copied here pastes on this host and vice versa", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + Switch(checked = clipboard, onCheckedChange = { clipboard = it }) + } } }, confirmButton = { @@ -419,6 +436,7 @@ internal fun EditHostDialog( address = address.trim(), port = port.toIntOrNull() ?: target.port, mac = KnownHostStore.parseMacs(mac), + clipboardSync = clipboard, ), ) }, diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectScreen.kt index e9068a9e..b0456f42 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ConnectScreen.kt @@ -63,6 +63,7 @@ import io.unom.punktfunk.kit.security.IdentityStore import io.unom.punktfunk.kit.security.KnownHost import io.unom.punktfunk.kit.security.KnownHostStore import io.unom.punktfunk.kit.security.obtainIdentity +import io.unom.punktfunk.models.ActiveSession import io.unom.punktfunk.models.HostStatus import io.unom.punktfunk.models.PendingTrust import java.util.concurrent.atomic.AtomicBoolean @@ -101,7 +102,7 @@ private class ConnectAttempt(val hostName: String) { @Composable fun ConnectScreen( settings: Settings, - onConnected: (Long) -> Unit, + onConnected: (ActiveSession) -> Unit, // Console (gamepad) mode: render the host carousel instead of the touch grid, sharing all of this // screen's connect/trust/discovery logic. [onOpenSettings]/[onOpenLibrary] are the X/Y actions the // gamepad shell owns (the touch UI reaches Settings via the bottom bar and has no library button). @@ -270,6 +271,12 @@ fun ConnectScreen( suspend fun connectNative(id: ClientIdentity, targetHost: String, targetPort: Int, pinHex: String, timeoutMs: Int): Long = connectToHost(context, settings, id, targetHost, targetPort, pinHex, launch = null, timeoutMs = timeoutMs) + // What the stream screen is handed: the settings this connect actually used, plus the HOST's + // clipboard decision (a property of the record, not a global). A host we never saved — a + // connect that failed to pin — falls back to the on default the setting always had. + fun session(handle: Long, record: KnownHost?) = + ActiveSession(handle, settings, clipboardSync = record?.clipboardSync ?: true) + // The actual dial (identity already ready). On a TOFU connect (pinHex null), pin the fingerprint // the host presented (as an unpaired known host) so the next connect goes straight through and it // appears in the saved-hosts list. [onFailure], when set, takes over a failed dial (the wake-wait @@ -296,13 +303,14 @@ fun ConnectScreen( attempt = null connecting = false if (handle != 0L) { + var record = knownHostStore.get(targetHost, targetPort) if (pinHex == null) { // TOFU: pin what we observed (unpaired) val fp = NativeBridge.nativeHostFingerprint(handle) if (fp.isNotEmpty()) { - knownHostStore.save(KnownHost(targetHost, targetPort, name, fp, paired = false)) + record = knownHostStore.trust(targetHost, targetPort, name, fp, paired = false) } } - onConnected(handle) + onConnected(session(handle, record)) } else { discovery.start() val token = NativeBridge.nativeTakeLastError() @@ -368,7 +376,7 @@ fun ConnectScreen( // connects) point at the live one, then dial there (no fallback on this // redial — a second failure surfaces as the plain error). if (live != null && kh != null && (live.host != kh.address || live.port != kh.port)) { - knownHostStore.update(kh.address, kh.port, kh.copy(address = live.host, port = live.port)) + knownHostStore.save(kh.copy(address = live.host, port = live.port)) savedHosts = knownHostStore.all() } doConnectDirect(live?.host ?: targetHost, live?.port ?: targetPort, name, pinHex) @@ -414,11 +422,12 @@ fun ConnectScreen( // Approved — save the host as PAIRED, pinning the fingerprint it presented, so // future connects are silent (exactly like after a PIN ceremony). val fp = NativeBridge.nativeHostFingerprint(handle) + var record = knownHostStore.get(target.host, target.port) if (fp.isNotEmpty()) { - knownHostStore.save(KnownHost(target.host, target.port, target.name, fp, paired = true)) + record = knownHostStore.trust(target.host, target.port, target.name, fp, paired = true) savedHosts = knownHostStore.all() } - onConnected(handle) + onConnected(session(handle, record)) } else { // Cause-specific: an operator denial, an approval timeout, and a request that // never reached the host are different problems with different fixes. @@ -621,7 +630,7 @@ fun ConnectScreen( enabled = !connecting, onConnect = { connect(kh.address, kh.port) }, onForget = { - knownHostStore.remove(kh.address, kh.port) + knownHostStore.remove(kh) savedHosts = knownHostStore.all() }, onEdit = { editTarget = kh }, @@ -741,7 +750,7 @@ fun ConnectScreen( // Same trust/pairing logic, console-styled + controller-navigable in gamepad mode. val onPair = { pendingTrust = pt.copy(kind = PendingTrust.Kind.PAIR) } val onSavePaired = { fp: String -> - knownHostStore.save(KnownHost(pt.host, pt.port, pt.name, fp, paired = true)) + knownHostStore.trust(pt.host, pt.port, pt.name, fp, paired = true) savedHosts = knownHostStore.all() pendingTrust = null doConnect(pt.host, pt.port, pt.name, fp) @@ -801,7 +810,7 @@ fun ConnectScreen( }, onEdit = { optionsTarget = null; editTarget = kh }, onForget = { - knownHostStore.remove(kh.address, kh.port) + knownHostStore.remove(kh) savedHosts = knownHostStore.all() optionsTarget = null }, @@ -814,7 +823,7 @@ fun ConnectScreen( // `discovery.hosts.first { host.matches($0) }?.macAddresses`. val suggested = discovered.firstOrNull { kh.matches(it) }?.mac ?: emptyList() val onSaveHost: (KnownHost) -> Unit = { updated -> - knownHostStore.update(kh.address, kh.port, updated) + knownHostStore.save(updated) savedHosts = knownHostStore.all() editTarget = null } diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/LibraryScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/LibraryScreen.kt index 1d2a8bdf..93939b23 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/LibraryScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/LibraryScreen.kt @@ -63,6 +63,7 @@ import io.unom.punktfunk.kit.security.ClientIdentity import io.unom.punktfunk.kit.security.IdentityStore import io.unom.punktfunk.kit.security.KnownHost import io.unom.punktfunk.kit.security.obtainIdentity +import io.unom.punktfunk.models.ActiveSession import kotlin.math.PI import kotlin.math.absoluteValue import kotlin.math.cos @@ -85,7 +86,7 @@ private sealed class LibState { fun LibraryScreen( host: KnownHost, settings: Settings, - onLaunched: (Long) -> Unit, + onLaunched: (ActiveSession) -> Unit, onBack: () -> Unit, navActive: Boolean = true, ) { @@ -142,7 +143,11 @@ fun LibraryScreen( host.address, host.port, host.fpHex, launch = game.id, ) launching = false - if (handle != 0L) onLaunched(handle) + if (handle != 0L) { + onLaunched( + ActiveSession(handle, settings, host.clipboardSync), + ) + } else Toast.makeText( context, "Launch failed — check the host and try again.", diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt index 5d0d3873..fb0d163d 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/Settings.kt @@ -123,13 +123,10 @@ data class Settings( * the Apple/GTK clients' "Invert scroll direction". */ val invertScroll: Boolean = false, - - /** - * Sync text copied on this device to the host and vice versa while streaming (the desktop - * clients' shared clipboard, text-only here). Only effective when the host advertises the - * clipboard capability; the protocol is opt-in per session either way. - */ - val clipboardSync: Boolean = true, + // NOTE: clipboard sync is NOT here. It is a decision about a HOST, not about this device or + // this stream (design/client-settings-profiles.md §3, tier H), so it lives on the host record + // — see `KnownHost.clipboardSync`. It used to be a global here; `KnownHostStore.migrate` + // copied that value onto every saved host and retired the key. ) /** [Settings.touchMode] values; persisted by name. */ @@ -214,7 +211,6 @@ class SettingsStore(context: Context) { // lands where it already was. ?: if (prefs.getBoolean(K_POINTER_CAPTURE, false)) MouseMode.CAPTURE else MouseMode.DESKTOP, invertScroll = prefs.getBoolean(K_INVERT_SCROLL, false), - clipboardSync = prefs.getBoolean(K_CLIPBOARD_SYNC, true), ) fun save(s: Settings) { @@ -240,7 +236,6 @@ class SettingsStore(context: Context) { .putBoolean(K_SC2_CAPTURE, s.sc2Capture) .putString(K_MOUSE_MODE, s.mouseMode.storedName) .putBoolean(K_INVERT_SCROLL, s.invertScroll) - .putBoolean(K_CLIPBOARD_SYNC, s.clipboardSync) .apply() } @@ -284,7 +279,6 @@ class SettingsStore(context: Context) { /** Legacy Boolean the [K_MOUSE_MODE] enum replaced — read once for migration, never written. */ const val K_POINTER_CAPTURE = "pointer_capture" const val K_INVERT_SCROLL = "invert_scroll" - const val K_CLIPBOARD_SYNC = "clipboard_sync" /** Legacy Boolean the enum replaced — read once as the migration default, never written. */ const val K_TRACKPAD = "trackpad_mode" diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt index 1e0900a6..999c05a5 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/SettingsScreen.kt @@ -500,13 +500,8 @@ private fun InputSettings(s: Settings, update: (Settings) -> Unit) { checked = s.invertScroll, onCheckedChange = { on -> update(s.copy(invertScroll = on)) }, ) - ToggleRow( - title = "Shared clipboard", - subtitle = "Text copied here pastes on the host and vice versa (hosts with " + - "clipboard sharing enabled)", - checked = s.clipboardSync, - onCheckedChange = { on -> update(s.copy(clipboardSync = on)) }, - ) + // "Shared clipboard" is NOT here any more: it is a trust decision about one host, so it + // lives on the host record and is edited from that host's Edit sheet. } } diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt index 75d82552..da763ca7 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/StreamScreen.kt @@ -59,11 +59,21 @@ import io.unom.punktfunk.kit.deviceBodyVibrator import io.unom.punktfunk.kit.NativeBridge import io.unom.punktfunk.kit.Sc2Capture import io.unom.punktfunk.kit.VideoDecoders +import io.unom.punktfunk.models.ActiveSession import java.util.concurrent.atomic.AtomicBoolean import kotlinx.coroutines.delay +/** + * The immersive stream. Everything it reads about the session comes from [session] — the settings + * the connect actually resolved (globals, or a profile's overrides on top of them) and the HOST's + * clipboard decision — rather than from a fresh `SettingsStore` load, which could disagree with + * the connect that produced this handle. + */ @Composable -fun StreamScreen(handle: Long, micEnabled: Boolean, onDisconnect: () -> Unit) { +fun StreamScreen(session: ActiveSession, onDisconnect: () -> Unit) { + val handle = session.handle + val initialSettings = session.settings + val micEnabled = initialSettings.micEnabled val context = LocalContext.current val activity = context as? MainActivity val window = activity?.window @@ -85,7 +95,6 @@ fun StreamScreen(handle: Long, micEnabled: Boolean, onDisconnect: () -> Unit) { // Settings. The tier only changes how many lines `StatsOverlay` draws — switching between the // visible tiers keeps sampling running (the effect keys on `statsOn`, not the tier) so it never // blanks the numbers for a poll interval. - val initialSettings = remember { SettingsStore(context).load() } var stats by remember { mutableStateOf(null) } var decoderLabel by remember { mutableStateOf("") } var codecLabel by remember { mutableStateOf("") } @@ -276,7 +285,7 @@ fun StreamScreen(handle: Long, micEnabled: Boolean, onDisconnect: () -> Unit) { activity?.remotePointer = remote // Shared clipboard (text v1): only when the user setting is on AND the host has a // working clipboard service. Protocol-level opt-in + the poll thread live in the sync. - val clip = if (initialSettings.clipboardSync && NativeBridge.nativeClipSupported(handle)) { + val clip = if (session.clipboardSync && NativeBridge.nativeClipSupported(handle)) { ClipboardSync(context, handle).also { it.start() } } else { null diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/models/UiModels.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/models/UiModels.kt index c456a8d4..b3066cae 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/models/UiModels.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/models/UiModels.kt @@ -29,6 +29,21 @@ data class PendingTrust( enum class Kind { TRUST_NEW, FP_CHANGED, PAIR, REQUEST_ACCESS } } +/** + * A stream session that just opened, and the state the stream screen needs about it. + * + * [settings] is the settings the connect ACTUALLY used, resolved once at connect time — not + * "whatever the settings store says now". Every post-connect read (the stats tier, the touch and + * mouse models, the low-latency pipeline, rumble, SC2 capture) takes it, so the stream can never + * disagree with the connect that produced it. [clipboardSync] comes from the host record, because + * clipboard sync is a decision about that host rather than about this device. + */ +data class ActiveSession( + val handle: Long, + val settings: io.unom.punktfunk.Settings, + val clipboardSync: Boolean, +) + /** Trust state of a host, shown as a colored pill on its card. */ enum class HostStatus(val label: String) { PAIRED("Paired"), diff --git a/clients/android/kit/build.gradle.kts b/clients/android/kit/build.gradle.kts index 35eaa0cb..b73f22c3 100644 --- a/clients/android/kit/build.gradle.kts +++ b/clients/android/kit/build.gradle.kts @@ -33,7 +33,11 @@ dependencies { // mTLS HTTPS client for the host's management API (the game-library fetch + cover-art loads). // OkHttp lets us present the paired client cert and pin the host's self-signed cert by SHA-256. implementation("com.squareup.okhttp3:okhttp:4.12.0") - testImplementation("junit:junit:4.13.2") // JVM unit test for the pure TXT parser + testImplementation("junit:junit:4.13.2") // JVM unit tests for the pure parsers/migrations + // A REAL org.json on the unit-test classpath. android.jar's org.json is stubs that throw + // "Stub!", so the host-store migration test — which asserts over the very JSON blobs the store + // reads and writes — cannot run without it. Explicit test deps precede the mockable android.jar. + testImplementation("org.json:json:20250107") } // ------------------------------------------------------------------------------------------------ diff --git a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/security/KnownHostStore.kt b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/security/KnownHostStore.kt index 5ab3f683..4f6f4271 100644 --- a/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/security/KnownHostStore.kt +++ b/clients/android/kit/src/main/kotlin/io/unom/punktfunk/kit/security/KnownHostStore.kt @@ -1,11 +1,19 @@ package io.unom.punktfunk.kit.security import android.content.Context +import java.util.UUID +import org.json.JSONArray import org.json.JSONObject /** * A host the user has trusted (pinned). [fpHex] is the pinned host-cert SHA-256 (64-hex); [paired] * is true when trust was established via the SPAKE2 PIN ceremony (vs trust-on-first-use). + * + * [id] is the record's **stable identity** — minted once, never changed, and the key this record is + * stored under. Everything that needs to point AT a host (a settings-profile binding, a pinned + * card, a `punktfunk://` link) points at the id, so renaming a host or moving it to a new address + * doesn't strand those references. Mirrors the Apple client's `StoredHost.id` and the Rust + * `KnownHost.id`; the shape is a lowercase UUID v4, one grammar on every platform. */ data class KnownHost( val address: String, @@ -18,37 +26,79 @@ data class KnownHost( * online, so the client can wake it once it sleeps. Empty until first learned. */ val mac: List = emptyList(), + /** Stable record identity — see the class doc. Minted here for a genuinely new record. */ + val id: String = newRecordId(), + /** + * Sync text copied on this device to this host and back while streaming. **A property of the + * host, not of the stream** (design/client-settings-profiles.md §3, tier H): it is a trust + * decision about that machine, so it is never in a settings profile and never global — the + * work box and the couch box get their own answers. Only effective when the host advertises + * the clipboard capability; the protocol is opt-in per session either way. + */ + val clipboardSync: Boolean = true, + /** + * The settings profile a plain tap on this host connects with — `null` (or an id whose profile + * was deleted) means the global defaults, i.e. today's behaviour. A dangling id is never an + * error and never blocks a connect. + */ + val profileId: String? = null, + /** + * Profiles pinned as their own cards for this host (design §5.2a). Presentation only: order is + * card order, and this is NOT the default binding ([profileId] is). Duplicates and profiles + * that no longer exist are dropped when the cards are rendered. + */ + val pinnedProfileIds: List = emptyList(), ) /** * Persists trusted hosts — the pinned-fingerprint store *and* the saved-hosts list — keyed by - * `address:port`. Replaces the old fp-only PinStore so a discovered and a manually-typed connection - * to the same host share one trust record (and so saved hosts can be listed + reconnected). Plain - * `SharedPreferences` in app-private storage: pinned fingerprints are public host identities, not - * secrets; the property we need is integrity, which app sandboxing provides. + * [KnownHost.id]. Plain `SharedPreferences` in app-private storage: pinned fingerprints are public + * host identities, not secrets; the property we need is integrity, which app sandboxing provides. + * + * Records used to be keyed by `"address:port"`, which meant editing a host's address had to + * re-key its record (delete + write) or leave a ghost behind, and meant nothing could hold a + * durable reference to a host. Keying by the minted stable id retires both. [migrate] moves an + * existing store over in one pass — see its doc for what else rides along. */ class KnownHostStore(context: Context) { private val prefs = - context.applicationContext.getSharedPreferences("punktfunk_hosts", Context.MODE_PRIVATE) + context.applicationContext.getSharedPreferences(PREFS_HOSTS, Context.MODE_PRIVATE) - // The pref key is just a unique id; address/port are also stored in the value so an IPv6 - // address (which contains colons) round-trips without parsing the key. - private fun key(address: String, port: Int) = "$address:$port" + init { + migrateIfNeeded(context) + } /** The trusted record for [address]:[port], or `null` if this host has never been trusted. */ fun get(address: String, port: Int): KnownHost? = - prefs.getString(key(address, port), null)?.let(::parse) + all().firstOrNull { it.address == address && it.port == port } - /** Pin (or update) a trusted host — upsert by `address:port`. */ + /** The trusted record with this stable [id], or `null` — the lookup a binding or link uses. */ + fun byId(id: String): KnownHost? = prefs.getString(id, null)?.let(::parse) + + /** + * Pin (or update) a trusted host — upsert by [KnownHost.id]. An edit that moves the address or + * port is a plain save now: the key is the identity, not the address. + */ fun save(host: KnownHost) { - val json = JSONObject() - .put("addr", host.address) - .put("port", host.port) - .put("name", host.name) - .put("fp", host.fpHex.lowercase()) - .put("paired", host.paired) - .put("mac", host.mac.joinToString(",")) - prefs.edit().putString(key(host.address, host.port), json.toString()).apply() + prefs.edit().putString(host.id, encode(host)).apply() + } + + /** + * Trust (or re-trust) the host at [address]:[port] with the fingerprint it presented. + * + * When a record already exists there — a re-pair after the host's identity changed, an + * approval that upgrades a TOFU record to paired — it keeps its identity and everything the + * user set on it: the stable [KnownHost.id] (so profile bindings, pinned cards and any + * `punktfunk://` shortcut still point at it), the per-host clipboard decision, the binding, + * the pins and the learned MACs. Only the name, pin and paired flag are refreshed. Returns the + * stored record. + */ + fun trust(address: String, port: Int, name: String, fpHex: String, paired: Boolean): KnownHost { + val existing = get(address, port) + val host = existing?.copy(name = name, fpHex = fpHex, paired = paired) + ?: KnownHost(address, port, name, fpHex, paired) + save(host) + return host } /** @@ -63,30 +113,45 @@ class KnownHostStore(context: Context) { save(h.copy(mac = mac)) } - /** Forget [address]:[port] (the next connect re-pairs / re-TOFUs). */ - fun remove(address: String, port: Int) { - prefs.edit().remove(key(address, port)).apply() - } - - /** Set a saved host's display name, keeping its pin + paired flag. No-op if not saved. */ - fun rename(address: String, port: Int, newName: String) { - val h = get(address, port) ?: return - save(h.copy(name = newName)) - } - - /** - * Edit a saved host, RE-KEYING if the address or port changed (the pref key IS `address:port`, so - * a plain [save] would otherwise leave a stale record under the old key). The caller passes an - * [updated] copy that preserves `fpHex`/`paired` (and sets `mac` from the edit form). - */ - fun update(oldAddress: String, oldPort: Int, updated: KnownHost) { - if (oldAddress != updated.address || oldPort != updated.port) remove(oldAddress, oldPort) - save(updated) + /** Forget [host] (the next connect re-pairs / re-TOFUs). */ + fun remove(host: KnownHost) { + prefs.edit().remove(host.id).apply() } /** All trusted hosts, name-sorted — backs the saved-hosts list. */ - fun all(): List = - prefs.all.values.mapNotNull { (it as? String)?.let(::parse) }.sortedBy { it.name.lowercase() } + fun all(): List = prefs.all + .filterKeys { it != K_SCHEMA } + .values + .mapNotNull { (it as? String)?.let(::parse) } + .sortedBy { it.name.lowercase() } + + /** + * One-time move from the `"address:port"`-keyed schema to id-keyed records, run on first + * construction after the upgrade and never again ([K_SCHEMA] records that it happened). + * + * It is deliberately ONE pass, not three: the store is being rewritten anyway, and every extra + * migration pass is another chance to strand somebody's hosts. So the same pass mints the + * stable id, re-keys the record onto it, and copies the retiring GLOBAL clipboard-sync setting + * onto every host — behaviour-preserving, since every host was following that one value. + */ + private fun migrateIfNeeded(context: Context) { + if (prefs.getInt(K_SCHEMA, 0) >= SCHEMA_VERSION) return + val settings = + context.applicationContext.getSharedPreferences(PREFS_SETTINGS, Context.MODE_PRIVATE) + val result = migrate(prefs.all, settings.getBoolean(K_GLOBAL_CLIPBOARD_SYNC, true)) + // `commit`, not `apply`: the re-keyed records and the schema flag are one atomic write to + // disk, and the global below is only retired once that write has landed. With `apply` a + // process death in between could drop the old global while the hosts that were supposed to + // inherit it were still only in memory. Once, on one small file, on an upgrade. + val written = prefs.edit().apply { + result.removals.forEach(::remove) + result.writes.forEach { (k, v) -> putString(k, v) } + putInt(K_SCHEMA, SCHEMA_VERSION) + }.commit() + if (written && settings.contains(K_GLOBAL_CLIPBOARD_SYNC)) { + settings.edit().remove(K_GLOBAL_CLIPBOARD_SYNC).apply() + } + } private fun parse(s: String): KnownHost? = runCatching { val j = JSONObject(s) @@ -97,10 +162,66 @@ class KnownHostStore(context: Context) { fpHex = j.getString("fp"), paired = j.optBoolean("paired", false), mac = j.optString("mac", "").split(",").map { it.trim() }.filter { it.isNotEmpty() }, + // A record without an id can only be one this build wrote before the migration ran, or + // a hand-edited file; minting here keeps the parse total rather than dropping a host. + id = j.optString("id", "").ifEmpty { newRecordId() }, + clipboardSync = j.optBoolean("clip", true), + profileId = j.optString("profile", "").ifEmpty { null }, + pinnedProfileIds = stringList(j.optJSONArray("pins")), ) }.getOrNull() companion object { + /** The prefs file holding the host records. */ + private const val PREFS_HOSTS = "punktfunk_hosts" + + /** The app's settings file — read once by [migrate] for the retiring global. */ + private const val PREFS_SETTINGS = "punktfunk_settings" + + /** + * The global clipboard-sync key this migration retires. Clipboard sync is a decision about + * a HOST (design §3, tier H), so it lives on the record now; the global is read once, to + * seed every host, and then deleted. + */ + private const val K_GLOBAL_CLIPBOARD_SYNC = "clipboard_sync" + + /** Schema marker inside the hosts file. Reserved — never a host record. */ + private const val K_SCHEMA = "__schema" + + /** 1 = id-keyed records with per-host clipboard sync, profile binding and pins. */ + private const val SCHEMA_VERSION = 1 + + /** What [migrate] decided: entries to write, and (old) keys to drop. */ + data class Migration(val writes: Map, val removals: Set) + + /** + * The pure half of the store migration, over a raw prefs snapshot ([entries] as returned by + * `SharedPreferences.all`) — so it can be tested against a real pre-migration blob without + * an Android runtime. + * + * Every host record survives with its address, port, name, pin, paired flag and MACs + * intact, gains a minted [KnownHost.id], moves to that key, and takes + * [globalClipboardSync] as its own [KnownHost.clipboardSync]. Entries that aren't parsable + * host records are left alone: they were already invisible (`all()` skipped them), and + * deleting things we don't understand is not this pass's job. + */ + fun migrate(entries: Map, globalClipboardSync: Boolean): Migration { + val writes = mutableMapOf() + val removals = mutableSetOf() + for ((key, raw) in entries) { + if (key == K_SCHEMA) continue + val json = (raw as? String)?.let { runCatching { JSONObject(it) }.getOrNull() } + ?: continue + if (!json.has("addr") || !json.has("port")) continue + val id = json.optString("id", "").ifEmpty { newRecordId() } + json.put("id", id) + json.put("clip", json.optBoolean("clip", globalClipboardSync)) + writes[id] = json.toString() + if (key != id) removals += key + } + return Migration(writes, removals) + } + /** * Parse a free-typed Wake-on-LAN field into normalized `aa:bb:cc:dd:ee:ff` entries (comma / * space / newline separated). Anything that isn't six colon-separated hex octets is dropped; @@ -116,5 +237,31 @@ class KnownHostStore(context: Context) { o.size == 6 && o.all { it.length == 2 && it.all { c -> c in '0'..'9' || c in 'a'..'f' } } } } + + /** The stored JSON for one record — also the shape [migrate] upgrades into. */ + internal fun encode(host: KnownHost): String = JSONObject() + .put("id", host.id) + .put("addr", host.address) + .put("port", host.port) + .put("name", host.name) + .put("fp", host.fpHex.lowercase()) + .put("paired", host.paired) + .put("mac", host.mac.joinToString(",")) + .put("clip", host.clipboardSync) + .put("profile", host.profileId ?: "") + .put("pins", JSONArray(host.pinnedProfileIds)) + .toString() + + private fun stringList(a: JSONArray?): List { + if (a == null) return emptyList() + return (0 until a.length()).mapNotNull { a.optString(it, "").ifEmpty { null } } + } } } + +/** + * A fresh stable record identity: a lowercase UUID v4, the shape the Apple client's `StoredHost.id` + * and the Rust `KnownHost.id` already use, so a `punktfunk://` host reference is one grammar + * everywhere. + */ +fun newRecordId(): String = UUID.randomUUID().toString() diff --git a/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/security/KnownHostStoreTest.kt b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/security/KnownHostStoreTest.kt index 1b19e58b..826cf1e0 100644 --- a/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/security/KnownHostStoreTest.kt +++ b/clients/android/kit/src/test/kotlin/io/unom/punktfunk/kit/security/KnownHostStoreTest.kt @@ -1,6 +1,10 @@ package io.unom.punktfunk.kit.security +import org.json.JSONObject import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue import org.junit.Test /** Unit tests for the pure MAC-parsing helper backing the host edit form. */ @@ -31,3 +35,107 @@ class KnownHostStoreTest { assertEquals(listOf("aa:bb:cc:dd:ee:ff"), KnownHostStore.parseMacs("junk, aa:bb:cc:dd:ee:ff")) } } + +/** + * The store migration, run against a REAL pre-migration prefs blob — records exactly as the + * `"address:port"`-keyed store wrote them, IPv6 and all. It runs once against live user data on + * every upgraded install, so the property under test is blunt: **every host survives**, with its + * trust intact and the retiring global clipboard setting carried onto it. + */ +class KnownHostMigrationTest { + /** Verbatim shape of the old writer: no `id`, no `clip`, MACs as a comma-joined string. */ + private fun legacy(addr: String, port: Int, name: String, fp: String, paired: Boolean, mac: String) = + JSONObject() + .put("addr", addr) + .put("port", port) + .put("name", name) + .put("fp", fp) + .put("paired", paired) + .put("mac", mac) + .toString() + + private val preMigration: Map = mapOf( + "192.168.1.42:9777" to legacy("192.168.1.42", 9777, "Living Room PC", "a".repeat(64), true, "aa:bb:cc:dd:ee:ff"), + "192.168.1.50:9777" to legacy("192.168.1.50", 9777, "Office", "b".repeat(64), false, ""), + // An IPv6 host: the old key contains colons of its own, which is exactly why the record + // always carried its address in the VALUE rather than parsing it back out of the key. + "fd00::1:9777" to legacy("fd00::1", 9777, "Basement", "c".repeat(64), true, ""), + ) + + private fun migrated(globalClipboard: Boolean): List = + KnownHostStore.migrate(preMigration, globalClipboard).writes.values.map { JSONObject(it) } + + @Test + fun everyHostSurvivesWithItsTrustIntact() { + val result = KnownHostStore.migrate(preMigration, globalClipboardSync = true) + assertEquals(3, result.writes.size) + // Every old key is dropped — none of them is a valid new key (they aren't ids). + assertEquals(preMigration.keys, result.removals) + + val byName = result.writes.values.map { JSONObject(it) }.associateBy { it.getString("name") } + assertEquals(setOf("Living Room PC", "Office", "Basement"), byName.keys) + + val living = byName.getValue("Living Room PC") + assertEquals("192.168.1.42", living.getString("addr")) + assertEquals(9777, living.getInt("port")) + assertEquals("a".repeat(64), living.getString("fp")) + assertTrue(living.getBoolean("paired")) + assertEquals("aa:bb:cc:dd:ee:ff", living.getString("mac")) + + // The IPv6 address round-trips out of the value, untouched by the key rewrite. + assertEquals("fd00::1", byName.getValue("Basement").getString("addr")) + assertFalse(byName.getValue("Office").getBoolean("paired")) + } + + @Test + fun eachRecordIsRekeyedOntoItsOwnMintedId() { + val result = KnownHostStore.migrate(preMigration, globalClipboardSync = true) + // The key IS the record's id, and the ids are distinct — two hosts must never collide onto + // one record (which would silently lose one of them). + result.writes.forEach { (key, json) -> assertEquals(key, JSONObject(json).getString("id")) } + assertEquals(3, result.writes.keys.size) + // The minted shape is the cross-platform one: a lowercase UUID. + result.writes.keys.forEach { id -> + assertEquals(36, id.length) + assertEquals(id.lowercase(), id) + assertEquals(listOf(8, 4, 4, 4, 12), id.split("-").map { it.length }) + } + assertNotEquals(newRecordId(), newRecordId()) + } + + /** + * The behaviour-preserving half: clipboard sync was one global that every host followed, so + * after the migration every host must still be following the value that global held — on or + * off. This is the assertion that would catch a migration silently defaulting everyone to on. + */ + @Test + fun theRetiringGlobalClipboardSettingLandsOnEveryHost() { + migrated(globalClipboard = true).forEach { assertTrue(it.getBoolean("clip")) } + migrated(globalClipboard = false).forEach { assertFalse(it.getBoolean("clip")) } + } + + @Test + fun migrationIsIdempotentOverItsOwnOutput() { + val once = KnownHostStore.migrate(preMigration, globalClipboardSync = false) + val twice = KnownHostStore.migrate(once.writes, globalClipboardSync = true) + // Already-keyed records keep their ids and their per-host value: a second pass (a downgrade + // then re-upgrade, a schema flag lost) must not re-mint ids that bindings point at, and must + // not resurrect the global over a per-host answer the user has since changed. + assertEquals(once.writes, twice.writes) + assertTrue(twice.removals.isEmpty()) + } + + @Test + fun entriesThatArentHostRecordsAreLeftAlone() { + val junk = mapOf( + "some_unrelated_flag" to true, + "half_a_record" to JSONObject().put("name", "no address").toString(), + "not_even_json" to "{{{", + ) + val result = KnownHostStore.migrate(junk, globalClipboardSync = true) + // They were already invisible to `all()`; deleting what we don't understand isn't this + // pass's job, and writing them as hosts would invent records out of nothing. + assertTrue(result.writes.isEmpty()) + assertTrue(result.removals.isEmpty()) + } +}