From 454d71d079306b38598c4f3394dd02e726b31a30 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 6 Nov 2026 10:22:59 -0500 Subject: [PATCH] fix(android): the app stops signing itself out at launch (#6911) At launch the stored session cookie loads from the Keystore off the main thread. A request sent before that finished went out without it, typically the library sync that fires as soon as the server answers. The server then answered 401, and the interceptor cleared the store on any 401, which also stopped the load from restoring the cookie and wrote null over the stored one. The user had to sign in on every launch. A session lost mid-use also left them on Home with every request failing, because the app only routes to sign-in when it starts. - The interceptor waits for the stored cookie to load before attaching it, bounded by the existing hydration deadline. - A 401 signs out only when the request carried the current cookie (compare-and-clear). A request sent without one, or with a cookie a newer sign-in has replaced, leaves the login alone. - When the server refuses the session mid-use, the app opens the sign-in screen. A sign-out from Settings does not trigger this. Co-Authored-By: Claude Opus 5.5 --- .../com/fabledsword/minstrel/MainActivity.kt | 21 +++++ .../minstrel/api/AuthCookieInterceptor.kt | 14 +-- .../fabledsword/minstrel/auth/AuthStore.kt | 44 +++++++++ .../minstrel/auth/ui/AuthGateViewModel.kt | 9 ++ .../minstrel/api/AuthCookieInterceptorTest.kt | 90 +++++++++++++++++++ 5 files changed, 173 insertions(+), 5 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt b/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt index cb90f2e4..65c7edf0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt @@ -24,6 +24,7 @@ import androidx.core.content.ContextCompat import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.lifecycleScope +import androidx.navigation.NavHostController import androidx.navigation.compose.rememberNavController import com.fabledsword.minstrel.auth.AuthStore import com.fabledsword.minstrel.auth.ui.AuthGateViewModel @@ -33,6 +34,7 @@ import com.fabledsword.minstrel.connectivity.ServerHealth import com.fabledsword.minstrel.connectivity.NetworkStatusController import com.fabledsword.minstrel.nav.DetailSeedCache import com.fabledsword.minstrel.nav.LocalDetailSeedCache +import com.fabledsword.minstrel.nav.Login import com.fabledsword.minstrel.nav.MinstrelNavGraph import com.fabledsword.minstrel.nav.Notifications import com.fabledsword.minstrel.nav.NowPlaying @@ -177,6 +179,7 @@ private fun App( navController.navigate(route) { launchSingleTop = true } onOpenedRoute() } + SignInWhenSessionRejected(gate, navController) MinstrelNavGraph( navController = navController, startDestination = resolved, @@ -187,6 +190,24 @@ private fun App( } } +/** + * When the server ends the session while the app is in use, opens the + * sign-in screen rather than leaving the user on screens whose every + * request fails. + */ +@Composable +private fun SignInWhenSessionRejected(gate: AuthGateViewModel, navController: NavHostController) { + val sessionRejected by gate.sessionRejected.collectAsStateWithLifecycle() + LaunchedEffect(sessionRejected, navController) { + if (!sessionRejected) return@LaunchedEffect + navController.navigate(Login) { + popUpTo(0) { inclusive = true } + launchSingleTop = true + } + gate.consumeSessionRejected() + } +} + @Composable private fun BootSplash() { Surface( diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/AuthCookieInterceptor.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/AuthCookieInterceptor.kt index b31d21ef..4c721863 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/AuthCookieInterceptor.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/AuthCookieInterceptor.kt @@ -8,9 +8,9 @@ import javax.inject.Singleton /** * Attaches the session cookie from [AuthStore] to Minstrel-server - * requests, captures Set-Cookie from successful responses (login - * flow), and clears the store on 401 so downstream code can react - * to logout. + * requests (waiting for the stored one to load at launch), captures + * Set-Cookie from successful responses (login flow), and signs out when + * the server refuses the cookie a request carried. * * Scoped to the [BaseUrlInterceptor.PLACEHOLDER_HOST] sentinel host * the same way BaseUrlInterceptor is. The shared OkHttpClient also @@ -32,7 +32,7 @@ class AuthCookieInterceptor @Inject constructor( // session. Pass through untouched. return chain.proceed(original) } - val cookie = authStore.sessionCookie.value + val cookie = authStore.sessionCookieForRequest() val request = original.newBuilder().apply { if (!cookie.isNullOrEmpty()) header("Cookie", cookie) }.build() @@ -40,7 +40,11 @@ class AuthCookieInterceptor @Inject constructor( val response = chain.proceed(request) if (response.code == HTTP_UNAUTHORIZED) { - authStore.setSessionCookie(null) + // Only a refusal of the cookie this request carried ends the + // session. A request sent without one says nothing about the + // stored login, and a refusal of an older cookie must not + // discard one a sign-in has stored since. + if (!cookie.isNullOrEmpty()) authStore.dropRejectedSessionCookie(cookie) } else if (response.isSuccessful) { // Capture the first Set-Cookie pair on login; subsequent // responses that include Set-Cookie keep the store fresh. diff --git a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt index e4fb4c4f..a525116b 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/auth/AuthStore.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withTimeoutOrNull @@ -148,6 +149,49 @@ class AuthStore @Inject constructor( scope.launch { cookieLock.withLock { storeCookie(value) } } } + /** + * The cookie to attach to an outgoing request. Blocks the calling + * (OkHttp) thread until the stored cookie has loaded, up to + * [HYDRATION_DEADLINE_MS], so a request made in the first moment after + * launch carries the login instead of going out without it. Once + * hydration has finished this is a plain read. + */ + fun sessionCookieForRequest(): String? { + if (!cookieHydration.isCompleted) { + runBlocking { withTimeoutOrNull(HYDRATION_DEADLINE_MS) { cookieHydration.await() } } + } + return sessionCookieState.value + } + + /** + * True once the server has refused the current session (see + * [dropRejectedSessionCookie]) and the UI has not yet sent the user to + * sign in. Held rather than fired, so a refusal that lands while the app + * is in the background is still acted on when it comes back. A sign-out + * the user asked for does not set it. + */ + private val sessionRejectedState = MutableStateFlow(false) + val sessionRejected: StateFlow = sessionRejectedState.asStateFlow() + + /** The UI has opened the sign-in screen for a refused session. */ + fun consumeSessionRejected() { + sessionRejectedState.value = false + } + + /** + * Signs out because the server answered 401 to a request that carried + * [rejected]. Does nothing when the store has since moved on to another + * cookie (a sign-in landed meanwhile), so a refusal of an old session + * never discards a new one. What is persisted is read inside the lock, + * so a sign-in racing this call is the value that ends up stored. + */ + fun dropRejectedSessionCookie(rejected: String) { + if (!sessionCookieState.compareAndSet(rejected, null)) return + cookieTouched = true + scope.launch { cookieLock.withLock { storeCookie(sessionCookieState.value) } } + sessionRejectedState.value = true + } + private suspend fun hydrateSessionCookie() = cookieLock.withLock { val legacy = runCatching { dao.get()?.sessionCookie }.getOrNull() if (cookieTouched) return@withLock diff --git a/android/app/src/main/java/com/fabledsword/minstrel/auth/ui/AuthGateViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/auth/ui/AuthGateViewModel.kt index 39d228bb..b8b86a1f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/auth/ui/AuthGateViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/auth/ui/AuthGateViewModel.kt @@ -38,6 +38,15 @@ class AuthGateViewModel @Inject constructor( private val internal = MutableStateFlow(null) val startDestination: StateFlow = internal.asStateFlow() + /** + * True when the server has ended the session while the app was in use; + * the root NavHost answers by opening the sign-in screen, then calls + * [consumeSessionRejected]. + */ + val sessionRejected: StateFlow = authStore.sessionRejected + + fun consumeSessionRejected() = authStore.consumeSessionRejected() + init { viewModelScope.launch { authStore.awaitSessionHydrated() diff --git a/android/app/src/test/java/com/fabledsword/minstrel/api/AuthCookieInterceptorTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/api/AuthCookieInterceptorTest.kt index fb685099..f93a9ac0 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/api/AuthCookieInterceptorTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/api/AuthCookieInterceptorTest.kt @@ -2,23 +2,31 @@ package com.fabledsword.minstrel.api import com.fabledsword.minstrel.auth.AuthStore import com.fabledsword.minstrel.auth.FakeSessionVault +import com.fabledsword.minstrel.auth.SessionVault import com.fabledsword.minstrel.cache.db.dao.AuthSessionDao import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNull +import kotlin.test.assertTrue @OptIn(ExperimentalCoroutinesApi::class) class AuthCookieInterceptorTest { @@ -92,6 +100,84 @@ class AuthCookieInterceptorTest { assertNull(authStore.sessionCookie.value) } + @Test + fun `a refused session asks the UI to open sign-in`() { + authStore.setSessionCookie("session=expired") + server.enqueue(MockResponse().setResponseCode(401)) + + client.newCall(Request.Builder().url(placeholderUrl("/api/test")).build()).execute() + + assertTrue(authStore.sessionRejected.value) + } + + // A refusal of a request that carried no cookie says nothing about the + // stored login, and must not raise the sign-in screen either. + @Test + fun `a 401 to a request sent without a cookie signs nobody out`() { + server.enqueue(MockResponse().setResponseCode(401)) + + client.newCall(Request.Builder().url(placeholderUrl("/api/test")).build()).execute() + + assertFalse(authStore.sessionRejected.value) + } + + // The interceptor reads the cookie when the request starts. A sign-in + // that lands while an old request is in flight must survive that old + // request's 401. + @Test + fun `a 401 for an older cookie keeps the one stored since`() { + authStore.setSessionCookie("session=old") + server.dispatcher = + object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse { + authStore.setSessionCookie("session=new") + return MockResponse().setResponseCode(401) + } + } + + client.newCall(Request.Builder().url(placeholderUrl("/api/test")).build()).execute() + + assertEquals("session=new", authStore.sessionCookie.value) + assertFalse(authStore.sessionRejected.value) + } + + // At launch the stored cookie loads from the Keystore off the main + // thread. A request made before that finishes waits for it rather than + // going out without the login. + @Test + fun `a request made while the stored cookie is still loading carries it`() { + val slowVault = + object : SessionVault { + override fun read(): String? { + Thread.sleep(SLOW_VAULT_MS) + return "session=stored" + } + + override fun write(value: String?): Boolean = true + } + val dao = + mockk { + every { observe() } returns flowOf(null) + coEvery { get() } returns null + coEvery { upsert(any()) } returns Unit + coEvery { setSessionCookie(any()) } returns Unit + coEvery { setBaseUrl(any()) } returns Unit + } + val loading = AuthStore(dao, slowVault, CoroutineScope(SupervisorJob() + Dispatchers.Default)) + loading.setBaseUrl(server.url("/").toString()) + val launchClient = + OkHttpClient.Builder() + .addInterceptor(AuthCookieInterceptor(loading)) + .addInterceptor(BaseUrlInterceptor(loading)) + .build() + server.enqueue(MockResponse().setResponseCode(200)) + + launchClient.newCall(Request.Builder().url(placeholderUrl("/api/library/sync")).build()).execute() + + assertEquals("session=stored", server.takeRequest().getHeader("Cookie")) + assertEquals("session=stored", loading.sessionCookie.value) + } + @Test fun `captures Set-Cookie on successful response`() { server.enqueue( @@ -145,4 +231,8 @@ class AuthCookieInterceptorTest { assertEquals("session=keep", authStore.sessionCookie.value) } + + private companion object { + const val SLOW_VAULT_MS = 300L + } }