refactor: proper OkHttp CookieJar integration (issue #470) (#483)

Replace the manual session-cookie interceptor in ApiClient and the
manual cookie branch in TokenRefreshAuthenticator with a shared
PersistentCookieJar backed by an encrypted, per-server EncryptedCookieStore.

- Add CookieStore iface + EncryptedCookieStore (EncryptedSharedPreferences,
  per-server scope, legacy single-cookie migration)
- Add CookieSerialization (OkHttp Cookie <-> holder) + PersistentCookieJar
  (in-mem cache, async persist, atomic useStore scope switching, prune,
  session-cookie accessor)
- Add CookieManager coordinator (init/useStore/prune/clear) and wire its
  cookieJar into OkHttpProvider base/websocket/probe (lazy)
- AuthManager.get/setSessionCookie now delegate to CookieManager; add
  pruneServerCache(); clear path drops the scope's session cookie
- AuthLoginViewModel + HermesWsClient rely on the jar for Set-Cookie capture
  and session-cookie re-attachment (no manual header injection)
- Add fakes (FakeEncryptedCookieStore/buildFakePersistentCookieJar) + unit
  tests (PersistentCookieJarTest, CookieManagerTest); inject fake jar into
  ApiClientTest/HermesWsClientTest setup

WS token-out-of-URL (step 5) deferred: backend hermes-agent authenticates
WS only via ws.query_params, so moving it requires a coordinated backend
change (noted for follow-up).

Verified: testDebugUnitTest (334 passing) + assembleDebug green via local SDK.
This commit is contained in:
M57 authored and GitHub committed 2026-07-07 21:51:04 +03:00
1 parent ce7048c07e
commit d178d7f69e
15 files changed
+953 -88

No files matched your search

@@ -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"
@@ -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())
}
}
@@ -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<String, MutableList<CookieHolder>>()
override suspend fun save(
serverId: String,
cookies: List<Cookie>,
) {
data[serverId] = cookies.map { it.serialize() }.toMutableList()
}
override suspend fun load(serverId: String): List<Cookie> =
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<CookieHolder> = 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,
),
)
@@ -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())
}
}
@@ -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