diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt b/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt index 4201cfc4..dd6bfa33 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt @@ -11,6 +11,7 @@ import com.m57.hermescontrol.data.config.ServerStoreSerializer import com.m57.hermescontrol.data.config.resolvedHost import com.m57.hermescontrol.data.config.resolvedPort import com.m57.hermescontrol.data.model.PinnedModel +import com.m57.hermescontrol.data.remote.CookieManager import com.m57.hermescontrol.theme.BottomNavDisplayMode import com.m57.hermescontrol.theme.ThemePreference import com.m57.hermescontrol.theme.ThemePreset @@ -96,16 +97,6 @@ object AuthManager { val store = ServerStore(dataStore, scope) _serverStore = store - scope.launch { - store.stateFlow.collect { state -> - _bottomNavItemsFlow.value = state.bottomNavItems - _themePreferenceFlow.value = state.themePreference - _useDynamicColorsFlow.value = state.useDynamicColors - _themePresetFlow.value = state.themePreset - _bottomNavDisplayModeFlow.value = state.bottomNavDisplayMode - } - } - if (prefsDeferred == null) { prefsDeferred = CoroutineScope(Dispatchers.IO).async { @@ -123,6 +114,22 @@ object AuthManager { p } } + + // Initialize the encrypted cookie store (issue #470). The legacy + // session-cookie prefs are passed as a Deferred so existing gated + // sessions can be migrated on first load WITHOUT blocking startup. + // The Deferred is created above, before this call. + CookieManager.initialize(context, prefsDeferred) + + scope.launch { + store.stateFlow.collect { state -> + _bottomNavItemsFlow.value = state.bottomNavItems + _themePreferenceFlow.value = state.themePreference + _useDynamicColorsFlow.value = state.useDynamicColors + _themePresetFlow.value = state.themePreset + _bottomNavDisplayModeFlow.value = state.bottomNavDisplayMode + } + } } } @@ -155,13 +162,25 @@ object AuthManager { /** * In gated mode (basic auth), the dashboard authenticates REST API * requests via the `hermes_session_at` cookie, not via - * `Authorization: *** We store it here so [ApiClient]'s - * authInterceptor can add it as a `Cookie` header. + * `Authorization: Bearer`. The cookie is now owned by the shared + * [CookieManager]/[PersistentCookieJar] (issue #470) which attaches it + * automatically on every REST call, follows redirects, and persists it + * encrypted. This accessor is a thin read-through to that store. */ - fun getSessionCookie(): String? = requirePrefs().getString(KEY_SESSION_COOKIE, null) + fun getSessionCookie(): String? = CookieManager.getSessionCookie() fun setSessionCookie(cookie: String?) { - requirePrefs().edit().putString(KEY_SESSION_COOKIE, cookie).apply() + // The session cookie is host-scoped to the current dashboard host so + // the CookieJar only attaches it to matching requests. + CookieManager.setSessionCookie(cookie, AuthManager.getHost()) + } + + /** + * Evict expired (non-session) cookies for the active server scope to + * bound cookie growth (issue #470 step 7). + */ + fun pruneServerCache() { + CookieManager.pruneServerCache() } // ── Database Master Password ───────────────────────────────────────── diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt index bcb5d40f..6ae07356 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt @@ -91,31 +91,22 @@ object ApiClient { } } + // Loopback mode: authenticate via Bearer token (the session cookie used + // in gated mode is now handled automatically by the shared CookieJar — + // see issue #470). val authInterceptor = Interceptor { chain -> val request = chain.request() - val sessionCookie = AuthManager.getSessionCookie() - if (!sessionCookie.isNullOrBlank()) { - // Gated mode: authenticate via session cookie + val token = AuthManager.getToken() + if (!token.isNullOrBlank()) { chain.proceed( request .newBuilder() - .addHeader("Cookie", "hermes_session_at=$sessionCookie") + .addHeader("Authorization", "Bearer $token") .build(), ) } else { - // Loopback mode: authenticate via Bearer token - val token = AuthManager.getToken() - if (!token.isNullOrBlank()) { - chain.proceed( - request - .newBuilder() - .addHeader("Authorization", "Bearer $token") - .build(), - ) - } else { - chain.proceed(request) - } + chain.proceed(request) } } diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/CookieManager.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/CookieManager.kt new file mode 100644 index 00000000..27e66ae1 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/CookieManager.kt @@ -0,0 +1,119 @@ +package com.m57.hermescontrol.data.remote + +import android.content.Context +import android.content.SharedPreferences +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import okhttp3.Cookie +import okhttp3.HttpUrl.Companion.toHttpUrl + +/** + * Coordinator for the issue #470 cookie stack. + * + * Owns the single [PersistentCookieJar] instance and exposes: + * - [cookieJar] — inject into every OkHttp client (REST + WS + probe). + * - [initialize] — build the encrypted store + jar (idempotent). + * - [useStore] — atomically switch the active server scope. + * - [pruneServerCache] / [clearAll] — lifecycle/maintenance. + * - [getSessionCookie] / [setSessionCookie] — backward-compatible accessors + * used by [com.m57.hermescontrol.data.local.AuthManager] and the login flow. + * + * The session cookie is still exposed as a single string because the WS client + * and a few REST paths reference it directly; internally it is just the + * `hermes_session_at` cookie from the active scope. + */ +object CookieManager { + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + + @Volatile private var jar: PersistentCookieJar? = null + + val cookieJar: PersistentCookieJar + get() = jar ?: error("CookieManager.initialize(context) must be called before use") + + fun isInitialized(): Boolean = jar != null + + fun initialize( + context: Context, + legacyPrefsDeferred: Deferred? = null, + ) { + if (jar != null) return + synchronized(this) { + if (jar != null) return + val store = EncryptedCookieStore(context.applicationContext, legacyPrefsDeferred) + jar = PersistentCookieJar(store, scope) + // Eagerly load the default scope off the caller thread so the + // first REST call is instant without blocking app startup. + scope.launch { jar!!.useStore(PersistentCookieJar.DEFAULT_SERVER_ID) } + } + } + + /** Switch the active server scope and load its persisted cookies. */ + fun useStore(serverId: String) { + val j = jar ?: return + runBlocking(scope.coroutineContext) { j.useStore(serverId) } + } + + /** Read the current `hermes_session_at` cookie value (or null). */ + fun getSessionCookie(): String? = jar?.getSessionCookieValue() + + /** + * Set the `hermes_session_at` session cookie for the active scope. + * [host] should be the dashboard host so the cookie matches REST requests. + */ + fun setSessionCookie( + rawValue: String?, + host: String, + ) { + val j = jar ?: return + if (rawValue.isNullOrBlank()) { + // Clearing: drop the session cookie (and any other cookies) for the + // active scope — equivalent to the old AuthManager nulling the + // session-cookie pref. + j.clearActive() + return + } + val cookie = + Cookie + .Builder() + .name(SESSION_COOKIE_NAME) + .value(rawValue) + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain(host) + .path("/") + .httpOnly() + .build() + val url = "http://$host/".toHttpUrl() + j.saveFromResponse(url, listOf(cookie)) + } + + /** Evict expired (non-session) cookies for the active scope. */ + fun pruneServerCache() { + jar?.pruneServerCache(allScopes = false) + } + + /** Wipe all cookies across all server scopes (logout / full reset). */ + fun clearAll() { + jar?.clearAll() + } + + // ── Test seam ──────────────────────────────────────────────────────── + + /** + * Replace the backing jar with [jar] (used by unit tests to inject a + * [FakePersistentCookieJar]). Not for production use. + */ + @Suppress("unused") + internal fun setJarForTest(jar: PersistentCookieJar?) { + this.jar = jar + } + + /** Reset to uninitialized state (tests only). */ + @Suppress("unused") + internal fun resetForTest() { + jar = null + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/CookieSerialization.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/CookieSerialization.kt new file mode 100644 index 00000000..9a9bb018 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/CookieSerialization.kt @@ -0,0 +1,96 @@ +package com.m57.hermescontrol.data.remote + +import okhttp3.Cookie +import okhttp3.HttpUrl + +/** + * Wire format for a persisted [Cookie]. + * + * OkHttp's [Cookie] is not itself serializable, so we project its immutable + * fields into this flat holder. `sameSite` is nullable in newer OkHttp + * versions — default to null on decode so older payloads still load. + */ +@kotlinx.serialization.Serializable +data class CookieHolder( + val name: String, + val value: String, + val expiresAt: Long, + val domain: String, + val path: String, + val secure: Boolean, + val httpOnly: Boolean, + val hostOnly: Boolean, + val sameSite: String? = null, +) + +/** Project a [Cookie] into its serializable form. */ +fun Cookie.serialize(): CookieHolder = + CookieHolder( + name = name, + value = value, + expiresAt = expiresAt, + domain = domain, + path = path, + secure = secure, + httpOnly = httpOnly, + hostOnly = hostOnly, + sameSite = sameSite, + ) + +/** Rebuild a [Cookie] from a [CookieHolder]. */ +fun CookieHolder.toCookie(): Cookie? = + runCatching { + Cookie + .Builder() + .name(name) + .value(value) + .expiresAt(expiresAt) + .apply { if (hostOnly) hostOnlyDomain(domain) else domain(domain) } + .path(path) + .also { if (secure) it.secure() } + .also { if (httpOnly) it.httpOnly() } + .also { if (sameSite != null) it.sameSite(sameSite) } + .build() + }.getOrNull() + +/** + * Wrap a raw `hermes_session_at` value (the pre-#470 single-string session + * cookie) as a permissive session cookie. Host/path are left empty so it + * matches any request the jar sends — matching the previous behaviour where + * the value was injected as a bare `Cookie: hermes_session_at=` header + * on every REST call. + */ +fun wrapSessionCookie(rawValue: String): Cookie? { + val value = rawValue.trim() + if (value.isEmpty()) return null + return runCatching { + Cookie + .Builder() + .name(SESSION_COOKIE_NAME) + .value(value) + // Far-future expiry (10 years) — session lifetime is owned by the + // dashboard, not by client-side cookie expiry. + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .path("/") + .httpOnly() + .build() + }.getOrNull() +} + +/** + * Build a permissive [Cookie] from a Set-Cookie `hermes_session_at=...` + * header captured during login. Used by [PersistentCookieJar] when the server + * does not echo an explicit domain (common for same-host dashboard logins). + */ +fun sessionCookieFromValue(value: String): Cookie? = wrapSessionCookie(value) + +const val SESSION_COOKIE_NAME = "hermes_session_at" + +/** True when [cookie] is the dashboard session cookie. */ +fun isSessionCookie(cookie: Cookie): Boolean = cookie.name == SESSION_COOKIE_NAME + +/** Parse a raw `Set-Cookie` header value (sans the `hermes_session_at=` key). */ +fun Cookie.Companion.parseRaw( + url: HttpUrl, + header: String, +): Cookie? = Cookie.parse(url, header) diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/EncryptedCookieStore.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/EncryptedCookieStore.kt new file mode 100644 index 00000000..20d7ec2a --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/EncryptedCookieStore.kt @@ -0,0 +1,134 @@ +package com.m57.hermescontrol.data.remote + +import android.content.Context +import android.content.SharedPreferences +import androidx.security.crypto.EncryptedSharedPreferences +import androidx.security.crypto.MasterKeys +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.encodeToString +import kotlinx.serialization.json.Json +import okhttp3.Cookie + +/** + * Persistence contract for server-scoped cookies. + * + * Implementations store cookies per logical server id (not the raw host) so + * that switching [PersistentCookieJar] scopes never lets cookies from one + * gateway leak into another (issue #470 — multi-server isolation). + * + * Splitting this into an interface lets unit tests inject an in-memory fake + * ([com.m57.hermescontrol.data.remote.FakeEncryptedCookieStore]) instead of + * spinning up [EncryptedSharedPreferences], which requires Android Keystore. + */ +interface CookieStore { + /** Persist [cookies] for [serverId]. Replaces the previous set atomically. */ + suspend fun save( + serverId: String, + cookies: List, + ) + + /** Load the persisted cookie set for [serverId] (may be empty). */ + suspend fun load(serverId: String): List + + /** Drop all persisted cookies for [serverId] (e.g. on logout). */ + suspend fun clear(serverId: String) + + /** Drop everything across all servers. */ + suspend fun clearAll() +} + +/** + * Encrypted, per-server cookie persistence backed by + * [EncryptedSharedPreferences] (AES256-GCM). Cookies are serialized with + * kotlinx.serialization and written on [Dispatchers.IO]. + * + * Legacy migration: the pre-#470 code stored a single raw + * `hermes_session_at` value in `AuthManager`'s prefs under + * [LEGACY_SESSION_COOKIE_KEY]. [load] transparently folds any such legacy + * value into the requested [serverId] scope on first read so existing gated + * sessions keep working without a re-login. + */ +class EncryptedCookieStore( + private val context: Context, + private val legacyPrefsDeferred: Deferred? = null, +) : CookieStore { + private val json = + Json { + ignoreUnknownKeys = true + encodeDefaults = true + } + + private val masterKeyAlias by lazy { MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC) } + + private val prefs by lazy { + EncryptedSharedPreferences.create( + PREFS_FILE, + masterKeyAlias, + context.applicationContext, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM, + ) + } + + override suspend fun save( + serverId: String, + cookies: List, + ) = withContext(Dispatchers.IO) { + val serialized = cookies.map { it.serialize() } + prefs.edit().putString(keyFor(serverId), json.encodeToString(serialized)).apply() + } + + override suspend fun load(serverId: String): List = + withContext(Dispatchers.IO) { + // Fold in legacy single-cookie storage once per server scope. + // This awaits the deferred prefs on the IO dispatcher — never + // blocks the caller (e.g. main thread) during app startup. + val migrated = maybeMigrateLegacy(serverId) + val raw = prefs.getString(keyFor(serverId), null) ?: return@withContext migrated + runCatching { + json + .decodeFromString>(raw) + .mapNotNull { it.toCookie() } + .plus(migrated) + .distinctBy { Triple(it.name, it.domain, it.path) } + }.getOrDefault(migrated) + } + + override suspend fun clear(serverId: String) = + withContext(Dispatchers.IO) { + prefs.edit().remove(keyFor(serverId)).apply() + } + + override suspend fun clearAll() = + withContext(Dispatchers.IO) { + prefs.edit().clear().apply() + } + + /** + * If a legacy `hermes_session_at` value exists and we have not yet folded + * it into [serverId], wrap it as a host-less session cookie and clear the + * legacy key so the migration is one-shot. Awaits the deferred legacy + * prefs on the IO dispatcher so [load] (already on IO) never blocks the + * caller thread. + */ + private suspend fun maybeMigrateLegacy(serverId: String): List { + val prefs = + legacyPrefsDeferred?.await() + ?: return emptyList() + val legacy = + prefs.getString(LEGACY_SESSION_COOKIE_KEY, null) + ?: return emptyList() + if (legacy.isBlank()) return emptyList() + prefs.edit().remove(LEGACY_SESSION_COOKIE_KEY).apply() + return wrapSessionCookie(legacy)?.let { listOf(it) } ?: emptyList() + } + + private fun keyFor(serverId: String) = "cookies::$serverId" + + companion object { + const val PREFS_FILE = "hermes_secure_cookies" + const val LEGACY_SESSION_COOKIE_KEY = "session_cookie" + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/OkHttpProvider.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/OkHttpProvider.kt index 8f2b8550..bda2d5aa 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/OkHttpProvider.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/OkHttpProvider.kt @@ -12,36 +12,55 @@ object OkHttpProvider { // 5 idle connections, 30s keep-alive — tuned for single-server LAN private val connectionPool = ConnectionPool(5, 30, TimeUnit.SECONDS) - // Base client: connection pool + sensible defaults - // Note: retryOnConnectionFailure(true) is enabled to allow OkHttp to recover from - // low-level connection/route failures (e.g. route timeouts, IPv4/IPv6 fallback). This is - // separate from safeApiCall's app-level retries (which add backoff delays and handle 5xx/429/timeouts). - val base: OkHttpClient = + /** + * Shared [okhttp3.CookieJar] used by every client (REST, WS, probe) so a + * Set-Cookie from one request is reusable by the others (issue #470). + * + * Resolved lazily (per client build) so it is only touched after + * [CookieManager.initialize] has run at app startup — NOT at + * [OkHttpProvider] object-init time, which would otherwise throw for unit + * tests / early access before the app context exists. + */ + private fun resolveCookieJar(): okhttp3.CookieJar = CookieManager.cookieJar + + // Base client: connection pool + sensible defaults. + // Lazily built so the shared CookieJar is only resolved at first use + // (after CookieManager.initialize), not during object construction. + // Note: retryOnConnectionFailure(true) lets OkHttp recover from low-level + // connection/route failures (route timeouts, IPv4/IPv6 fallback), separate + // from safeApiCall's app-level retries (backoff + 5xx/429/timeouts). + val base: OkHttpClient by lazy { OkHttpClient .Builder() + .cookieJar(resolveCookieJar()) .connectionPool(connectionPool) .connectTimeout(15, TimeUnit.SECONDS) .readTimeout(30, TimeUnit.SECONDS) .writeTimeout(15, TimeUnit.SECONDS) .retryOnConnectionFailure(true) .build() + } // WebSocket-optimized variant (infinite read timeout, ping interval) - val websocket: OkHttpClient = + val websocket: OkHttpClient by lazy { base .newBuilder() + .cookieJar(resolveCookieJar()) .readTimeout(0, TimeUnit.MILLISECONDS) .pingInterval(30, TimeUnit.SECONDS) .build() + } // Short-timeout variant for probes and ticket minting - val probe: OkHttpClient = + val probe: OkHttpClient by lazy { base .newBuilder() + .cookieJar(resolveCookieJar()) .connectTimeout(5, TimeUnit.SECONDS) .readTimeout(5, TimeUnit.SECONDS) .followRedirects(false) .build() + } @OptIn(ExperimentalSerializationApi::class) val json: Json = diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/PersistentCookieJar.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/PersistentCookieJar.kt new file mode 100644 index 00000000..2423fbf2 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/PersistentCookieJar.kt @@ -0,0 +1,177 @@ +package com.m57.hermescontrol.data.remote + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import okhttp3.Cookie +import okhttp3.CookieJar +import okhttp3.HttpUrl +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicReference + +/** + * OkHttp [CookieJar] backed by an in-memory [ConcurrentHashMap] cache plus + * encrypted [CookieStore] persistence. + * + * Design notes (issue #470): + * - **Memory cache** keyed by host for O(1) reads on every request. + * - **Async persistence** — writes are dispatched to [storeScope] and never + * block the calling OkHttp thread. + * - **Atomic server scoping** via [useStore] — callers (login, profile switch) + * flip the active server id; subsequent load/save target that scope. Guarded + * by [scopeMutex] so a swap can't race a mid-flight load. + * - **Lazy load** — the first request for a server scope triggers a one-shot + * [CookieStore.load] (double-checked through [loadedScopes]). + * - **Pruning** — [pruneServerCache] evicts expired cookies to bound growth. + * + * The session cookie (`hermes_session_at`) is explicitly retained across + * pruning because its lifetime is owned server-side, not by client expiry. + */ +class PersistentCookieJar( + private val store: CookieStore, + private val storeScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO), +) : CookieJar { + // serverId -> (host -> cookies) + private val cache = ConcurrentHashMap>>() + private val loadedScopes = ConcurrentHashMap.newKeySet() + + @Volatile private var currentServerId = AtomicReference(DEFAULT_SERVER_ID) + + private val scopeMutex = Mutex() + + /** Atomically switch the active server scope and ensure its cookies are loaded. */ + suspend fun useStore(serverId: String) { + scopeMutex.withLock { + currentServerId.set(serverId) + ensureLoaded(serverId) + } + } + + /** Current active server scope id. */ + fun currentServer(): String = currentServerId.get() + + /** + * Return the live [Cookie] with [name] from the active server scope, or + * null if absent. Scans every host bucket; the session cookie is typically + * host-scoped to the dashboard host. + */ + fun getCookie(name: String): Cookie? { + val serverId = currentServerId.get() + val hosts = cache[serverId] ?: return null + for (bucket in hosts.values) { + synchronized(bucket) { + val match = bucket.firstOrNull { it.name == name } + if (match != null) return match + } + } + return null + } + + /** Convenience: value of the `hermes_session_at` session cookie. */ + fun getSessionCookieValue(): String? = getCookie(SESSION_COOKIE_NAME)?.value + + override fun saveFromResponse( + url: HttpUrl, + cookies: List, + ) { + if (cookies.isEmpty()) return + val serverId = currentServerId.get() + val hostKey = url.host + val bucket = + cache + .getOrPut(serverId) { ConcurrentHashMap() } + .getOrPut(hostKey) { mutableListOf() } + synchronized(bucket) { + for (cookie in cookies) { + val idx = bucket.indexOfFirst { it.name == cookie.name && it.path == cookie.path } + if (idx >= 0) bucket[idx] = cookie else bucket.add(cookie) + } + } + // Persist the whole server scope asynchronously. + persist(serverId) + } + + override fun loadForRequest(url: HttpUrl): List { + val serverId = currentServerId.get() + // Best-effort synchronous load on first touch (tests/first call). + if (!loadedScopes.contains(serverId)) { + kotlinx.coroutines.runBlocking { ensureLoaded(serverId) } + } + val hostKey = url.host + val hosts = cache[serverId] ?: return emptyList() + val now = System.currentTimeMillis() + val buckets = listOfNotNull(hosts[hostKey], hosts[WILDCARD_HOST]) + synchronized(buckets) { + return buckets.flatMap { bucket -> + synchronized(bucket) { + bucket.filter { it.matches(url) && (it.expiresAt > now || isSessionCookie(it)) } + } + } + } + } + + private suspend fun ensureLoaded(serverId: String) { + if (loadedScopes.contains(serverId)) return + val persisted = store.load(serverId) + val byHost = ConcurrentHashMap>() + for (cookie in persisted) { + // Blank domain => host-only (e.g. legacy migrated session cookie). + // Bucket it under a wildcard "*" so it is returned for every host. + val host = cookie.domain.removePrefix(".").ifBlank { WILDCARD_HOST } + byHost.getOrPut(host) { mutableListOf() }.add(cookie) + } + cache[serverId] = byHost + loadedScopes.add(serverId) + } + + private fun persist(serverId: String) { + val snapshot = + cache[serverId]?.values?.flatten()?.toList() ?: return + storeScope.launch { store.save(serverId, snapshot) } + } + + /** + * Evict expired cookies for the active (or all) server scopes to prevent + * unbounded growth. Session cookies are preserved. + */ + fun pruneServerCache(allScopes: Boolean = false) { + val now = System.currentTimeMillis() + val scopeKeys = + if (allScopes) cache.keys.toList() else listOf(currentServerId.get()) + for (scope in scopeKeys) { + val hosts = cache[scope] ?: continue + for ((host, bucket) in hosts) { + synchronized(bucket) { + bucket.removeAll { it.expiresAt <= now && !isSessionCookie(it) } + } + if (bucket.isEmpty()) hosts.remove(host) + } + persist(scope) + } + } + + /** Clear the active server scope's in-memory + persisted cookies. */ + fun clearActive() { + val scope = currentServerId.get() + cache.remove(scope) + loadedScopes.remove(scope) + storeScope.launch { store.clear(scope) } + } + + /** Clear every server scope (logout / full reset). */ + fun clearAll() { + cache.clear() + loadedScopes.clear() + storeScope.launch { store.clearAll() } + } + + companion object { + const val DEFAULT_SERVER_ID = "default" + + /** Bucket key for host-only (blank-domain) cookies, returned for any host. */ + const val WILDCARD_HOST = "*" + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/TokenRefreshAuthenticator.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/TokenRefreshAuthenticator.kt index a7076c97..b0456b3c 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/TokenRefreshAuthenticator.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/TokenRefreshAuthenticator.kt @@ -14,26 +14,20 @@ object TokenRefreshAuthenticator : Authenticator { if (response.priorResponse != null) { return null // Only retry once } - val sessionCookie = AuthManager.getSessionCookie() - if (!sessionCookie.isNullOrBlank()) { - val requestCookie = response.request.header("Cookie") - val currentCookieHeader = "hermes_session_at=$sessionCookie" - if (requestCookie == null || !requestCookie.contains(currentCookieHeader)) { - return response.request - .newBuilder() - .header("Cookie", currentCookieHeader) - .build() - } - } else { - val token = AuthManager.getToken() - val requestAuth = response.request.header("Authorization") - val currentAuthHeader = "Bearer $token" - if (!token.isNullOrBlank() && (requestAuth == null || !requestAuth.contains(currentAuthHeader))) { - return response.request - .newBuilder() - .header("Authorization", currentAuthHeader) - .build() - } + // Gated mode: the session cookie is now managed automatically by the + // shared CookieJar (issue #470), so OkHttp re-attaches it on retry + // without any manual header manipulation here. + + // Loopback mode: re-assert the Bearer token if the request lacked it + // (or it drifted from the current stored token). + val token = AuthManager.getToken() + val requestAuth = response.request.header("Authorization") + val currentAuthHeader = "Bearer $token" + if (!token.isNullOrBlank() && (requestAuth == null || !requestAuth.contains(currentAuthHeader))) { + return response.request + .newBuilder() + .header("Authorization", currentAuthHeader) + .build() } return null } diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt index 65a4650f..8c3eb446 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt @@ -176,6 +176,9 @@ object HermesWsClient { * If a session cookie is present (gated mode), mint a fresh WS ticket * from the dashboard. The ticket is single-use and has a 30-second TTL, * so we must mint a new one on every connect (first launch and reconnect). + * + * The session cookie is attached automatically by the shared CookieJar on + * OkHttpProvider.probe (issue #470), so we no longer inject it manually. */ private fun refreshWsTicketIfNeeded() { val sessionCookie = AuthManager.getSessionCookie() @@ -188,13 +191,12 @@ object HermesWsClient { Request .Builder() .url("http://${AuthManager.getHost()}:${AuthManager.getPort()}/api/auth/ws-ticket") - .header("Cookie", "hermes_session_at=$sessionCookie") .post("{}".toRequestBody()) .build() val response = client.newCall(request).execute() if (response.isSuccessful) { val body = response.body?.string() ?: "" - val ticketMatch = Regex(""""ticket":"([^"]+)"""").find(body) + val ticketMatch = Regex("""\"ticket\":\"([^\"]+)\""").find(body) val ticket = ticketMatch?.groupValues?.getOrNull(1) if (!ticket.isNullOrBlank()) { AuthManager.setToken(ticket) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/authlogin/AuthLoginViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/authlogin/AuthLoginViewModel.kt index 8cdac4a8..bf7c5b5f 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/authlogin/AuthLoginViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/authlogin/AuthLoginViewModel.kt @@ -258,8 +258,6 @@ class AuthLoginViewModel( private data class ConnectResult( /** WS credential: session token (loopback) or WS ticket (gated). */ val wsCredential: String, - /** Session cookie for REST API auth in gated mode (null in loopback). */ - val sessionCookie: String? = null, ) /** @@ -298,12 +296,17 @@ class AuthLoginViewModel( AuthManager.setHost(state.host) AuthManager.setPort(port) AuthManager.setToken(result.wsCredential) - if (result.sessionCookie != null) { - AuthManager.setSessionCookie(result.sessionCookie) - AuthManager.setWsAuthParam("ticket") - } else { + if (state.authMode == DashboardAuthMode.TOKEN_ONLY) { + // Loopback mode — no session cookie; ensure any stale one + // is cleared so the jar only sends the Bearer token. AuthManager.setSessionCookie(null) AuthManager.setWsAuthParam("token") + } else { + // Gated (BASIC_AUTH / ALL): the session cookie was captured + // automatically by the shared CookieJar during the login + // call (issue #470), so we keep it and switch the WS auth + // param to the ticket minted above. + AuthManager.setWsAuthParam("ticket") } ApiClient.rebuild() HermesWsClient.connect() @@ -419,27 +422,10 @@ class AuthLoginViewModel( return null } - // Step 2: Extract hermes_session_at cookie from Set-Cookie headers - val setCookieHeaders = loginResp.headers("Set-Cookie") - var sessionAccessToken: String? = null - for (header in setCookieHeaders) { - if (header.startsWith("hermes_session_at=")) { - sessionAccessToken = header.split(";")[0].removePrefix("hermes_session_at=").trim() - break - } - } - - if (sessionAccessToken.isNullOrBlank()) { - _uiState.update { - it.copy( - isLoading = false, - errorMessage = "Login succeeded but no session cookie received", - ) - } - return null - } - - val cookieHeader = "hermes_session_at=$sessionAccessToken" + // The session cookie is captured automatically by the shared + // CookieJar (issue #470) attached to OkHttpProvider.probe — no + // manual Set-Cookie parsing needed. We still mint a WS ticket below + // using whatever cookie the jar carries into that request. // Step 3: Mint a WebSocket ticket using the session cookie val ticketClient = @@ -453,7 +439,6 @@ class AuthLoginViewModel( Request .Builder() .url("$baseUrl/api/auth/ws-ticket") - .header("Cookie", cookieHeader) .post("{}".toRequestBody()) .build() val ticketResp = ticketClient.newCall(ticketReq).execute() @@ -482,7 +467,7 @@ class AuthLoginViewModel( return null } - return ConnectResult(wsCredential = ticket, sessionCookie = sessionAccessToken) + return ConnectResult(wsCredential = ticket) } catch (e: Exception) { _uiState.update { it.copy( diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt index c1bfa41d..d21aeecc 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt @@ -34,6 +34,11 @@ class ApiClientTest { mockkObject(AuthManager) + // Issue #470: ApiClient builds through OkHttpProvider, which resolves + // the shared CookieManager.cookieJar. Inject a fake jar so the test + // can build clients without app context. + CookieManager.setJarForTest(buildFakePersistentCookieJar()) + // Point AuthManager at our MockWebServer every { AuthManager.baseUrl() } returns mockWebServer.url("/").toString() every { AuthManager.getHost() } returns "127.0.0.1" diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/CookieManagerTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/CookieManagerTest.kt new file mode 100644 index 00000000..814969c8 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/CookieManagerTest.kt @@ -0,0 +1,85 @@ +package com.m57.hermescontrol.data.remote + +import kotlinx.coroutines.test.runTest +import okhttp3.HttpUrl.Companion.toHttpUrl +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Unit tests for [CookieManager] — the issue #470 coordinator. + * + * Injects a [FakePersistentCookieJar] via the test seam so no Android deps + * are needed. Covers: + * - session cookie set/get round-trip (host-scoped) + * - clearing the session cookie + * - prune/clear pass-through to the jar + */ +class CookieManagerTest { + private fun fakeJar(): PersistentCookieJar { + val jar = buildFakePersistentCookieJar() + runTest { jar.useStore(PersistentCookieJar.DEFAULT_SERVER_ID) } + CookieManager.setJarForTest(jar) + return jar + } + + @After + fun tearDown() { + CookieManager.resetForTest() + } + + @Test + fun setThenGetSessionCookie_roundTrips() { + fakeJar() + CookieManager.setSessionCookie("sess-xyz", "dashboard.local") + + assertEquals("sess-xyz", CookieManager.getSessionCookie()) + } + + @Test + fun setSessionCookie_nullClearsValue() { + fakeJar() + CookieManager.setSessionCookie("sess-xyz", "dashboard.local") + assertEquals("sess-xyz", CookieManager.getSessionCookie()) + + CookieManager.setSessionCookie(null, "dashboard.local") + assertNull(CookieManager.getSessionCookie()) + } + + @Test + fun setSessionCookie_hostScoped_onlyMatchesThatHost() = + runTest { + val jar = fakeJar() + CookieManager.setSessionCookie("sess-xyz", "dashboard.local") + + val onHost = + jar.loadForRequest( + "http://dashboard.local/api/status".toHttpUrl(), + ) + assertEquals(1, onHost.size) + assertEquals("sess-xyz", onHost[0].value) + + val offHost = jar.loadForRequest("http://other.local/".toHttpUrl()) + assertEquals(0, offHost.size) + } + + @Test + fun pruneServerCache_delegatesToJar() = + runTest { + val jar = fakeJar() + CookieManager.setSessionCookie("keep-me", "dashboard.local") + CookieManager.pruneServerCache() + // Session cookie is preserved by prune. + assertEquals("keep-me", CookieManager.getSessionCookie()) + } + + @Test + fun clearAll_wipesSession() = + runTest { + val jar = fakeJar() + CookieManager.setSessionCookie("bye", "dashboard.local") + CookieManager.clearAll() + assertNull(CookieManager.getSessionCookie()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/FakeCookieJar.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/FakeCookieJar.kt new file mode 100644 index 00000000..a942190b --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/FakeCookieJar.kt @@ -0,0 +1,50 @@ +package com.m57.hermescontrol.data.remote + +import okhttp3.Cookie + +/** + * In-memory [CookieStore] for unit tests — no Android Keystore / + * [EncryptedSharedPreferences] required. Mirrors the production + * [EncryptedCookieStore] contract so the same [PersistentCookieJar] can be + * exercised end-to-end in JVM tests (issue #470). + */ +class FakeEncryptedCookieStore : CookieStore { + private val data = mutableMapOf>() + + override suspend fun save( + serverId: String, + cookies: List, + ) { + data[serverId] = cookies.map { it.serialize() }.toMutableList() + } + + override suspend fun load(serverId: String): List = + data[serverId] + ?.mapNotNull { it.toCookie() } + .orEmpty() + + override suspend fun clear(serverId: String) { + data.remove(serverId) + } + + override suspend fun clearAll() { + data.clear() + } + + /** Test helper: inspect raw holders for a server scope. */ + fun storedHolders(serverId: String): List = data[serverId].orEmpty() +} + +/** + * Build a [PersistentCookieJar] pre-wired with a [FakeEncryptedCookieStore] + * plus a deterministic single-threaded [kotlinx.coroutines.CoroutineScope] + * so async persistence completes predictably inside tests. + */ +fun buildFakePersistentCookieJar(): PersistentCookieJar = + PersistentCookieJar( + store = FakeEncryptedCookieStore(), + storeScope = + kotlinx.coroutines.CoroutineScope( + kotlinx.coroutines.Dispatchers.Unconfined, + ), + ) diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/PersistentCookieJarTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/PersistentCookieJarTest.kt new file mode 100644 index 00000000..585033ec --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/PersistentCookieJarTest.kt @@ -0,0 +1,182 @@ +package com.m57.hermescontrol.data.remote + +import kotlinx.coroutines.test.runTest +import okhttp3.Cookie +import okhttp3.HttpUrl.Companion.toHttpUrl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for [PersistentCookieJar] — the issue #470 cookie stack. + * + * Uses [FakeEncryptedCookieStore] (no Android deps) and exercises: + * - Set-Cookie capture + re-attachment on matching requests + * - host/path/session-cookie matching rules + * - atomic server-scope switching (multi-server isolation) + * - async persistence round-trip through the fake store + * - pruning of expired (but not session) cookies + */ +class PersistentCookieJarTest { + private fun makeJar(): PersistentCookieJar = + PersistentCookieJar( + store = FakeEncryptedCookieStore(), + storeScope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.Dispatchers.Unconfined), + ) + + private fun sessionCookie( + host: String, + value: String, + ): Cookie = + Cookie + .Builder() + .name(SESSION_COOKIE_NAME) + .value(value) + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain(host) + .path("/") + .httpOnly() + .build() + + @Test + fun saveFromResponse_thenLoadForRequest_attachesCookie() = + runTest { + val jar = makeJar() + jar.useStore("server-a") + val url = "http://dashboard.local/".toHttpUrl() + jar.saveFromResponse(url, listOf(sessionCookie("dashboard.local", "abc123"))) + + val loaded = jar.loadForRequest("http://dashboard.local/api/status".toHttpUrl()) + assertEquals(1, loaded.size) + assertEquals("abc123", loaded[0].value) + assertEquals(SESSION_COOKIE_NAME, loaded[0].name) + } + + @Test + fun cookie_doesNotLeakToOtherHost() = + runTest { + val jar = makeJar() + jar.useStore("server-a") + jar.saveFromResponse( + "http://dashboard.local/".toHttpUrl(), + listOf(sessionCookie("dashboard.local", "abc123")), + ) + + val loaded = jar.loadForRequest("http://evil.local/".toHttpUrl()) + assertTrue(loaded.isEmpty()) + } + + @Test + fun useStore_isolatesServers() = + runTest { + val jar = makeJar() + jar.useStore("server-a") + jar.saveFromResponse( + "http://a.local/".toHttpUrl(), + listOf(sessionCookie("a.local", "cookie-a")), + ) + jar.useStore("server-b") + jar.saveFromResponse( + "http://b.local/".toHttpUrl(), + listOf(sessionCookie("b.local", "cookie-b")), + ) + + // Active scope is server-b now — only b's cookie should load. + val fromB = jar.loadForRequest("http://b.local/x".toHttpUrl()) + assertEquals("cookie-b", fromB.firstOrNull()?.value) + + // Switch back to server-a. + jar.useStore("server-a") + val fromA = jar.loadForRequest("http://a.local/x".toHttpUrl()) + assertEquals("cookie-a", fromA.firstOrNull()?.value) + } + + @Test + fun persistence_roundTripThroughStore() = + runTest { + val store = FakeEncryptedCookieStore() + val jar = + PersistentCookieJar( + store = store, + storeScope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.Dispatchers.Unconfined), + ) + jar.useStore("persist-scope") + jar.saveFromResponse( + "http://p.local/".toHttpUrl(), + listOf(sessionCookie("p.local", "persisted-value")), + ) + + // The fake store should now hold the serialized cookie. + assertTrue(store.storedHolders("persist-scope").any { it.value == "persisted-value" }) + + // A fresh jar sharing the same store must reload it. + val jar2 = + PersistentCookieJar( + store = store, + storeScope = kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.Dispatchers.Unconfined), + ) + jar2.useStore("persist-scope") + val reloaded = jar2.loadForRequest("http://p.local/".toHttpUrl()) + assertEquals("persisted-value", reloaded.firstOrNull()?.value) + } + + @Test + fun pruneServerCache_removesExpiredKeepsSession() = + runTest { + val jar = makeJar() + jar.useStore("prune-scope") + val url = "http://p.local/".toHttpUrl() + val expired = + Cookie + .Builder() + .name("expired_cookie") + .value("x") + .expiresAt(System.currentTimeMillis() - 1000) + .hostOnlyDomain("p.local") + .path("/") + .build() + jar.saveFromResponse(url, listOf(expired, sessionCookie("p.local", "live"))) + + jar.pruneServerCache() + + val remaining = jar.loadForRequest(url) + assertEquals(1, remaining.size) + assertEquals(SESSION_COOKIE_NAME, remaining[0].name) + } + + @Test + fun saveReplacesCookieWithSameNameAndPath() = + runTest { + val jar = makeJar() + jar.useStore("dup-scope") + val url = "http://d.local/".toHttpUrl() + jar.saveFromResponse(url, listOf(sessionCookie("d.local", "v1"))) + jar.saveFromResponse(url, listOf(sessionCookie("d.local", "v2"))) + + val loaded = jar.loadForRequest(url) + assertEquals(1, loaded.size) + assertEquals("v2", loaded[0].value) + } + + @Test + fun clearActive_dropsScopeCookies() = + runTest { + val jar = makeJar() + jar.useStore("clear-scope") + jar.saveFromResponse( + "http://c.local/".toHttpUrl(), + listOf(sessionCookie("c.local", "bye")), + ) + jar.clearActive() + + assertTrue(jar.loadForRequest("http://c.local/".toHttpUrl()).isEmpty()) + } + + @Test + fun loadForRequest_returnsEmptyWhenNoCookies() { + val jar = makeJar() + assertTrue(jar.loadForRequest("http://empty.local/".toHttpUrl()).isEmpty()) + assertNull(jar.getSessionCookieValue()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index 15da3383..7a73746e 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -2,6 +2,8 @@ package com.m57.hermescontrol.data.ws import android.util.Log import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.remote.CookieManager +import com.m57.hermescontrol.data.remote.buildFakePersistentCookieJar import io.mockk.every import io.mockk.mockkObject import io.mockk.mockkStatic @@ -44,6 +46,11 @@ class HermesWsClientTest { every { AuthManager.isAutoReconnect() } returns false every { AuthManager.getSessionCookie() } returns null + // Issue #470: clients are built through OkHttpProvider, which now + // resolves the shared CookieManager.cookieJar. Inject a fake jar so + // the WS stack can build its OkHttp clients without app context. + CookieManager.setJarForTest(buildFakePersistentCookieJar()) + // Reset state val connectedField = HermesWsClient::class.java.getDeclaredField("connected") connectedField.isAccessible = true