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 index 9a9bb018..e86ea814 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/CookieSerialization.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/CookieSerialization.kt @@ -86,8 +86,25 @@ fun sessionCookieFromValue(value: String): Cookie? = wrapSessionCookie(value) const val SESSION_COOKIE_NAME = "hermes_session_at" +/** + * All names the dashboard may use for its session cookie, strictest variant + * first. Over HTTPS the server emits a [`__Host-`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#cookie_prefixes) + * (root-facing HTTPS) or `__Secure-` (HTTPS reverse-proxy with a path prefix) + * prefixed cookie; the bare name is the legacy HTTP form. Ordering matches the + * server's own fallback precedence so the most-constrained variant wins. + */ +val SESSION_COOKIE_NAMES: List = + listOf( + "__Host-$SESSION_COOKIE_NAME", + "__Secure-$SESSION_COOKIE_NAME", + SESSION_COOKIE_NAME, + ) + +/** True when [name] is any recognized dashboard session-cookie name. */ +fun isSessionCookieName(name: String): Boolean = name in SESSION_COOKIE_NAMES + /** True when [cookie] is the dashboard session cookie. */ -fun isSessionCookie(cookie: Cookie): Boolean = cookie.name == SESSION_COOKIE_NAME +fun isSessionCookie(cookie: Cookie): Boolean = isSessionCookieName(cookie.name) /** Parse a raw `Set-Cookie` header value (sans the `hermes_session_at=` key). */ fun Cookie.Companion.parseRaw( 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 index be18059e..a67c4682 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/PersistentCookieJar.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/PersistentCookieJar.kt @@ -74,8 +74,14 @@ class PersistentCookieJar( return null } - /** Convenience: value of the `hermes_session_at` session cookie. */ - fun getSessionCookieValue(): String? = getCookie(SESSION_COOKIE_NAME)?.value + /** + * Convenience: value of the dashboard session cookie. + * + * Iterates [SESSION_COOKIE_NAMES] (strictest `__Host-` / `__Secure-` + * prefix first) and returns the first match. Over HTTPS the server may + * store the cookie under a prefixed name the bare-name lookup would miss. + */ + fun getSessionCookieValue(): String? = SESSION_COOKIE_NAMES.firstNotNullOfOrNull { name -> getCookie(name)?.value } override fun saveFromResponse( url: HttpUrl, diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt index 75ec2592..3d0199bf 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt @@ -62,7 +62,15 @@ object ProfileScopeInterceptor : Interceptor { return chain.proceed(request) // explicit param wins } - if (!isProfileScopedPath(url.encodedPath)) { + // Derive the path relative to the endpoint's proxy prefix so profile + // scoping works when the dashboard is served behind a reverse proxy + // (e.g. `/hermes/api/status` → relative `/api/status`). A transient + // DataStore read failure mid-intercept must not crash every request. + val relativePath = + runCatching { AuthManager.endpoint().relativeRequestPath(url) } + .getOrDefault(url.encodedPath) + + if (!isProfileScopedPath(relativePath)) { return chain.proceed(request) } 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 c6d28865..c2a4ff0e 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 @@ -253,13 +253,6 @@ object HermesWsClient { return true } - val sessionCookie = AuthManager.getSessionCookie() - if (sessionCookie.isNullOrBlank()) { - Log.w(TAG, "WS ticket refresh aborted: no session cookie found in gated mode") - _connectionStatus.value = ConnectionStatus.AUTH_EXPIRED - return false - } - try { val client = OkHttpProvider.probe val request = @@ -294,7 +287,7 @@ object HermesWsClient { return false } } catch (e: Exception) { - Log.w(TAG, "WS ticket refresh failed: ${e.message}") + Log.w(TAG, "WS ticket refresh failed: ${e.javaClass.simpleName}") _connectionStatus.value = ConnectionStatus.AUTH_EXPIRED return false } @@ -539,7 +532,8 @@ object HermesWsClient { code: Int, reason: String, ) { - Log.d(TAG, "WebSocket closing: $code $reason") + // Do NOT log [reason] — it may carry server-side context. + Log.d(TAG, "WebSocket closing: $code") webSocket.close(code, reason) } @@ -548,7 +542,9 @@ object HermesWsClient { code: Int, reason: String, ) { - Log.i(TAG, "WebSocket closed: $code $reason") + // Do NOT log [reason] — it may carry server-side context. The + // reason is still inspected internally to detect auth failures. + Log.i(TAG, "WebSocket closed: $code") connected.set(false) stopHealthTracking() if (code == 4001 || code == 4401 || @@ -567,7 +563,10 @@ object HermesWsClient { t: Throwable, response: Response?, ) { - Log.e(TAG, "WebSocket failure: ${t.message}", t) + // Log the exception class only — [Throwable.message] can leak URLs + // or headers. The message is still inspected internally for auth + // detection. + Log.e(TAG, "WebSocket failure: ${t.javaClass.simpleName}", t) connected.set(false) stopHealthTracking() val code = response?.code ?: 0 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 index a3334c25..02487eee 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/remote/CookieManagerTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/CookieManagerTest.kt @@ -1,10 +1,12 @@ package com.m57.hermescontrol.data.remote import kotlinx.coroutines.test.runTest +import okhttp3.Cookie import okhttp3.HttpUrl.Companion.toHttpUrl import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -28,11 +30,62 @@ class CookieManagerTest { return ServerEndpoint.fromLegacy(host, 9119) } + private fun httpsEp( + host: String, + path: String = "/", + ): ServerEndpoint = + ServerEndpoint.parse( + "https://$host:9119$path", + CleartextPolicy.ALLOW_WITH_WARNING, + ) + @After fun tearDown() { CookieManager.resetForTest() } + @Test + fun getSessionCookie_recognizesHostPrefixedCookie() = + runTest { + val jar = buildFakePersistentCookieJar() + jar.useStore(PersistentCookieJar.DEFAULT_SERVER_ID) + CookieManager.setJarForTest(jar) + // Server stored a __Host- prefixed cookie (HTTPS deployment). + jar.saveFromResponse( + "https://dash.local/".toHttpUrl(), + listOf( + Cookie + .Builder() + .name("__Host-hermes_session_at") + .value("prefixed-value") + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain("dash.local") + .path("/") + .secure() + .httpOnly() + .build(), + ), + ) + + assertEquals("prefixed-value", CookieManager.getSessionCookie()) + } + + @Test + fun setSessionCookie_httpsMarksSecureAndHonorsPath() = + runTest { + val jar = fakeJar() + // Proxy-prefixed HTTPS endpoint. + CookieManager.setSessionCookie("sess-secure", httpsEp("dash.local", "/hermes/")) + + val cookies = + jar.loadForRequest( + "https://dash.local/hermes/api/status".toHttpUrl(), + ) + assertEquals(1, cookies.size) + assertTrue("HTTPS session cookie must be Secure", cookies[0].secure) + assertEquals("/hermes/", cookies[0].path) + } + @Test fun setThenGetSessionCookie_roundTrips() { fakeJar() 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 index 585033ec..6344db27 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/remote/PersistentCookieJarTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/PersistentCookieJarTest.kt @@ -179,4 +179,60 @@ class PersistentCookieJarTest { assertTrue(jar.loadForRequest("http://empty.local/".toHttpUrl()).isEmpty()) assertNull(jar.getSessionCookieValue()) } + + @Test + fun getSessionCookieValue_findsPrefixedHostCookie() = + runTest { + val jar = makeJar() + jar.useStore("prefixed-scope") + val url = "https://dash.local/".toHttpUrl() + val prefixed = + Cookie + .Builder() + .name("__Host-hermes_session_at") + .value("host-prefixed-value") + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain("dash.local") + .path("/") + .secure() + .httpOnly() + .build() + jar.saveFromResponse(url, listOf(prefixed)) + + assertEquals("host-prefixed-value", jar.getSessionCookieValue()) + } + + @Test + fun getSessionCookieValue_prefersStrictestVariant() = + runTest { + val jar = makeJar() + jar.useStore("multi-scope") + val url = "https://dash.local/".toHttpUrl() + // Both a bare and a __Secure- prefixed cookie present; the strictest + // (__Host- / __Secure- first in SESSION_COOKIE_NAMES) must win. + val bare = + Cookie + .Builder() + .name(SESSION_COOKIE_NAME) + .value("bare-value") + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain("dash.local") + .path("/") + .httpOnly() + .build() + val secure = + Cookie + .Builder() + .name("__Secure-hermes_session_at") + .value("secure-value") + .expiresAt(System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000) + .hostOnlyDomain("dash.local") + .path("/") + .secure() + .httpOnly() + .build() + jar.saveFromResponse(url, listOf(bare, secure)) + + assertEquals("secure-value", 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 7a73746e..508a30bd 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,9 +2,12 @@ package com.m57.hermescontrol.data.ws import android.util.Log import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.remote.CleartextPolicy import com.m57.hermescontrol.data.remote.CookieManager +import com.m57.hermescontrol.data.remote.ServerEndpoint import com.m57.hermescontrol.data.remote.buildFakePersistentCookieJar import io.mockk.every +import io.mockk.mockk import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.unmockkAll @@ -45,6 +48,13 @@ class HermesWsClientTest { every { AuthManager.wsUrl() } returns mockWebServer.url("/").toString().replace("http://", "ws://") every { AuthManager.isAutoReconnect() } returns false every { AuthManager.getSessionCookie() } returns null + // Non-gated by default (token mode) so the gated ticket path is exercised + // only by the explicit gated-mode test below. + every { AuthManager.serverStore } returns + mockk().also { + every { it.getLatestState() } returns + com.m57.hermescontrol.data.config.ServerStoreState() + } // Issue #470: clients are built through OkHttpProvider, which now // resolves the shared CookieManager.cookieJar. Inject a fake jar so @@ -475,4 +485,65 @@ class HermesWsClientTest { assertEquals(ConnectionStatus.DISCONNECTED, HermesWsClient.connectionStatus.value) assertFalse(HermesWsClient.isConnected) } + + // ── Issue #635: gated-mode WS ticket fetch must not be blocked by a + // missing bare-name session cookie (HTTPS deployments prefix it with + // __Host- / __Secure-). ──────────────────────────────────────────────── + + @Test + fun testGatedMode_attemptsTicketFetchWithoutBareCookie() { + // Force gated mode (ws auth via ticket, not loopback token). + every { AuthManager.serverStore } returns + mockk().also { + every { it.getLatestState() } returns + com.m57.hermescontrol.data.config.ServerStoreState(wsAuthParam = "ticket") + } + // No bare-name session cookie present (the prefixed one is server-side). + every { AuthManager.getSessionCookie() } returns null + // setToken is exercised by the ticket refresh; stub it (AuthManager is + // a mocked object, so unstubbed calls throw). + every { AuthManager.setToken(any()) } returns Unit + + // Separate server for the ticket endpoint so its queue can't interleave + // with the WebSocket upgrade on the main mockWebServer. + val ticketServer = MockWebServer() + ticketServer.start() + every { AuthManager.endpointForBuild() } returns + ServerEndpoint.parse( + ticketServer.url("/").toString(), + CleartextPolicy.ALLOW_WITH_WARNING, + ) + ticketServer.enqueue( + MockResponse() + .setResponseCode(200) + .setBody("""{"ticket":"refreshed-ticket"}"""), + ) + + val connectLatch = CountDownLatch(1) + mockWebServer.enqueue( + MockResponse().withWebSocketUpgrade( + object : WebSocketListener() { + override fun onOpen( + ws: WebSocket, + response: okhttp3.Response, + ) { + connectLatch.countDown() + } + }, + ), + ) + + HermesWsClient.connect() + + // Before the fix, a null bare cookie short-circuited to AUTH_EXPIRED and + // the ticket endpoint was NEVER called. After the fix it is attempted, + // so the connection reaches CONNECTED. + assertTrue( + "Gated WS ticket fetch should be attempted even without a bare cookie", + connectLatch.await(5, TimeUnit.SECONDS), + ) + assertEquals(ConnectionStatus.CONNECTED, HermesWsClient.connectionStatus.value) + + ticketServer.shutdown() + } }