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 + } }