fix(#688): route expired auth sessions to sign-in (#689)

* fix(#688): route expired auth sessions to sign-in

Adopt global 401 latch, WS disconnect, and Back/profile guards.

* chore: clean up formatting noise

* fix(ktlint): reorder imports to place java and kotlinx at end

* fix(ktlint): place java.io.IOException at end of imports

* test: fix NavBackStack type parameter in AuthenticationRoutingTest

* test: add AuthSessionState tests to AuthLoginViewModelTest

* fix(ktlint): fix import order in AuthLoginViewModelTest
This commit is contained in:
M57 authored and GitHub committed 2026-07-23 00:44:17 +03:00
1 parent def122ab58
commit a5baf41545
10 files changed
+217 -4

No files matched your search

@@ -28,6 +28,7 @@ import androidx.compose.material3.Text
import androidx.compose.material3.rememberDrawerState import androidx.compose.material3.rememberDrawerState
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
@@ -43,6 +44,7 @@ import androidx.navigation3.runtime.NavKey
import androidx.navigation3.runtime.entryProvider import androidx.navigation3.runtime.entryProvider
import androidx.navigation3.ui.NavDisplay import androidx.navigation3.ui.NavDisplay
import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.AuthManager
import com.m57.hermescontrol.data.local.AuthSessionState
import com.m57.hermescontrol.data.ws.ConnectionStatus import com.m57.hermescontrol.data.ws.ConnectionStatus
import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.HermesWsClient
import com.m57.hermescontrol.theme.LocalHermesStatusColors import com.m57.hermescontrol.theme.LocalHermesStatusColors
@@ -132,6 +134,14 @@ private fun appEntryProvider(
@Composable @Composable
fun MainNavigation(sessionId: String? = null) { fun MainNavigation(sessionId: String? = null) {
val signInRequired by AuthSessionState.signInRequired.collectAsState()
LaunchedEffect(signInRequired) {
if (signInRequired) {
HermesWsClient.disconnect()
NavigationController.resetTo(AuthLoginScreen)
}
}
val token by AuthManager.tokenFlow.collectAsState() val token by AuthManager.tokenFlow.collectAsState()
val hasToken = !token.isNullOrBlank() val hasToken = !token.isNullOrBlank()
val startScreen: NavKey = if (hasToken) ChatScreen else LandingScreen val startScreen: NavKey = if (hasToken) ChatScreen else LandingScreen
@@ -2,6 +2,7 @@ package com.m57.hermescontrol
import androidx.navigation3.runtime.NavBackStack import androidx.navigation3.runtime.NavBackStack
import androidx.navigation3.runtime.NavKey import androidx.navigation3.runtime.NavKey
import com.m57.hermescontrol.data.local.AuthSessionState
/** /**
* Central navigation controller with deduplication guard. * Central navigation controller with deduplication guard.
@@ -58,11 +59,24 @@ object NavigationController {
*/ */
fun goBack(fallback: NavKey = ChatScreen) { fun goBack(fallback: NavKey = ChatScreen) {
val stack = backStack ?: return val stack = backStack ?: return
val effectiveFallback =
if (AuthSessionState.signInRequired.value && (fallback == ChatScreen || isPrimaryScreen(fallback))) {
LandingScreen
} else {
fallback
}
if (stack.size > 1) { if (stack.size > 1) {
stack.removeLastOrNull() stack.removeLastOrNull()
if (AuthSessionState.signInRequired.value &&
stack.lastOrNull() != AuthLoginScreen &&
stack.lastOrNull() != LandingScreen
) {
stack.clear()
stack.add(LandingScreen)
}
} else if (stack.size == 1) { } else if (stack.size == 1) {
stack.clear() stack.clear()
stack.add(fallback) stack.add(effectiveFallback)
} }
} }
} }
@@ -0,0 +1,31 @@
package com.m57.hermescontrol.data.local
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
/**
* Single source of truth for app-wide auth session expiry latch.
* When a REST call returns 401 (AuthExpired), [requireSignIn] is invoked,
* triggering central navigation reset to [AuthLoginScreen] and WS disconnect.
*/
object AuthSessionState {
private val _signInRequired = MutableStateFlow(false)
val signInRequired: StateFlow<Boolean> = _signInRequired.asStateFlow()
fun requireSignIn() {
_signInRequired.value = true
}
fun markAuthenticated() {
_signInRequired.value = false
}
fun clearSignInRequired() {
_signInRequired.value = false
}
fun resetForTest() {
_signInRequired.value = false
}
}
@@ -1,5 +1,6 @@
package com.m57.hermescontrol.data.remote package com.m57.hermescontrol.data.remote
import com.m57.hermescontrol.data.local.AuthSessionState
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonObject
import kotlinx.serialization.json.jsonPrimitive import kotlinx.serialization.json.jsonPrimitive
@@ -90,6 +91,7 @@ fun jitteredBackoff(baseMs: Long): Long {
suspend inline fun <reified T> safeApiCall( suspend inline fun <reified T> safeApiCall(
retries: Int = 2, retries: Int = 2,
reportAuthExpiry: Boolean = true,
crossinline call: suspend () -> Response<T>, crossinline call: suspend () -> Response<T>,
): NetworkResult<T> { ): NetworkResult<T> {
var lastException: IOException? = null var lastException: IOException? = null
@@ -118,7 +120,11 @@ suspend inline fun <reified T> safeApiCall(
delay(backoff) delay(backoff)
continue continue
} }
return NetworkResult.Failure(mapHttpError(code, errorBody)) val failure = NetworkResult.Failure(mapHttpError(code, errorBody))
if (reportAuthExpiry && failure.error is NetworkError.AuthExpired) {
AuthSessionState.requireSignIn()
}
return failure
} }
} catch (e: IOException) { } catch (e: IOException) {
lastException = e lastException = e
@@ -8,6 +8,7 @@ import androidx.lifecycle.viewModelScope
import com.m57.hermescontrol.R import com.m57.hermescontrol.R
import com.m57.hermescontrol.data.config.ConnectionProfile import com.m57.hermescontrol.data.config.ConnectionProfile
import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.AuthManager
import com.m57.hermescontrol.data.local.AuthSessionState
import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.ApiClient
import com.m57.hermescontrol.data.remote.AuthPayloads import com.m57.hermescontrol.data.remote.AuthPayloads
import com.m57.hermescontrol.data.remote.CleartextPolicy import com.m57.hermescontrol.data.remote.CleartextPolicy
@@ -80,9 +81,20 @@ class AuthLoginViewModel(
init { init {
loadLoggedInProfiles() loadLoggedInProfiles()
viewModelScope.launch {
AuthSessionState.signInRequired.collect { required ->
if (required) {
_uiState.update { it.copy(loggedInProfiles = emptyList()) }
}
}
}
} }
fun loadLoggedInProfiles() { fun loadLoggedInProfiles() {
if (AuthSessionState.signInRequired.value) {
_uiState.update { it.copy(loggedInProfiles = emptyList()) }
return
}
val allProfiles = AuthManager.getConnectionProfiles() val allProfiles = AuthManager.getConnectionProfiles()
val loggedIn = val loggedIn =
allProfiles.filter { profile -> allProfiles.filter { profile ->
@@ -95,6 +107,7 @@ class AuthLoginViewModel(
fun useExistingProfile(profileId: String) { fun useExistingProfile(profileId: String) {
AuthManager.setSelectedProfileId(profileId) AuthManager.setSelectedProfileId(profileId)
ApiClient.rebuild() ApiClient.rebuild()
AuthSessionState.markAuthenticated()
HermesWsClient.connect() HermesWsClient.connect()
_uiState.update { it.copy(connectionSuccess = true) } _uiState.update { it.copy(connectionSuccess = true) }
} }
@@ -345,6 +358,7 @@ class AuthLoginViewModel(
AuthManager.setWsAuthParam("ticket") AuthManager.setWsAuthParam("ticket")
} }
ApiClient.rebuild() ApiClient.rebuild()
AuthSessionState.markAuthenticated()
HermesWsClient.connect() HermesWsClient.connect()
_uiState.update { it.copy(isLoading = false, connectionSuccess = true) } _uiState.update { it.copy(isLoading = false, connectionSuccess = true) }
} }
@@ -366,7 +380,7 @@ class AuthLoginViewModel(
} }
val tempApi = ApiClient.createTempService(endpoint.baseUrl.toString(), token) val tempApi = ApiClient.createTempService(endpoint.baseUrl.toString(), token)
val result = safeApiCall { tempApi.getSessions() } val result = safeApiCall(reportAuthExpiry = false) { tempApi.getSessions() }
return when (result) { return when (result) {
is com.m57.hermescontrol.data.remote.NetworkResult.Success -> { is com.m57.hermescontrol.data.remote.NetworkResult.Success -> {
@@ -8,6 +8,7 @@ import android.util.Log
import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope import androidx.lifecycle.viewModelScope
import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.AuthManager
import com.m57.hermescontrol.data.local.AuthSessionState
import com.m57.hermescontrol.data.local.HermesDatabase import com.m57.hermescontrol.data.local.HermesDatabase
import com.m57.hermescontrol.data.model.Attachment import com.m57.hermescontrol.data.model.Attachment
import com.m57.hermescontrol.data.model.ModelProvider import com.m57.hermescontrol.data.model.ModelProvider
@@ -1854,6 +1855,7 @@ class ChatViewModel(
} }
fun reconnect() { fun reconnect() {
AuthSessionState.markAuthenticated()
_uiState.update { _uiState.update {
it.copy( it.copy(
isLoading = true, isLoading = true,
@@ -1951,6 +1953,7 @@ class ChatViewModel(
AuthManager.setWsAuthParam("ticket") AuthManager.setWsAuthParam("ticket")
AuthManager.setToken(ticket) AuthManager.setToken(ticket)
AuthSessionState.markAuthenticated()
withContext(Dispatchers.Main) { withContext(Dispatchers.Main) {
onResult(true, null) onResult(true, null)
@@ -8,6 +8,7 @@ import com.m57.hermescontrol.R
import com.m57.hermescontrol.data.config.ConnectionProfile import com.m57.hermescontrol.data.config.ConnectionProfile
import com.m57.hermescontrol.data.config.resolveBaseUrl import com.m57.hermescontrol.data.config.resolveBaseUrl
import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.AuthManager
import com.m57.hermescontrol.data.local.AuthSessionState
import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.ApiClient
import com.m57.hermescontrol.data.remote.CleartextPolicy import com.m57.hermescontrol.data.remote.CleartextPolicy
import com.m57.hermescontrol.data.remote.NetworkError import com.m57.hermescontrol.data.remote.NetworkError
@@ -115,7 +116,7 @@ class ConnectViewModel(
val result = val result =
withContext(Dispatchers.IO) { withContext(Dispatchers.IO) {
val tempApi = ApiClient.createTempService(endpoint.baseUrl.toString(), state.token) val tempApi = ApiClient.createTempService(endpoint.baseUrl.toString(), state.token)
safeApiCall { tempApi.getStatus() } safeApiCall(reportAuthExpiry = false) { tempApi.getStatus() }
} }
when (result) { when (result) {
is NetworkResult.Success -> { is NetworkResult.Success -> {
@@ -123,6 +124,7 @@ class ConnectViewModel(
AuthManager.setToken(state.token) AuthManager.setToken(state.token)
AuthManager.setBaseUrl(state.baseUrl) AuthManager.setBaseUrl(state.baseUrl)
ApiClient.rebuild() ApiClient.rebuild()
AuthSessionState.markAuthenticated()
if (state.saveProfile) { if (state.saveProfile) {
val currentProfiles = AuthManager.getConnectionProfiles() val currentProfiles = AuthManager.getConnectionProfiles()
@@ -0,0 +1,66 @@
package com.m57.hermescontrol
import androidx.navigation3.runtime.NavBackStack
import androidx.navigation3.runtime.NavKey
import com.m57.hermescontrol.data.local.AuthSessionState
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
class AuthenticationRoutingTest {
@Before
fun setup() {
AuthSessionState.resetForTest()
}
@After
fun teardown() {
AuthSessionState.resetForTest()
NavigationController.backStack = null
}
@Test
fun testExpiryPropagationState() {
assertFalse(AuthSessionState.signInRequired.value)
AuthSessionState.requireSignIn()
assertTrue(AuthSessionState.signInRequired.value)
AuthSessionState.markAuthenticated()
assertFalse(AuthSessionState.signInRequired.value)
}
@Test
fun testGoBackFallbackGuardsChatWhenSignInRequired() {
val stack = NavBackStack<NavKey>(AuthLoginScreen)
NavigationController.backStack = stack
AuthSessionState.requireSignIn()
assertTrue(AuthSessionState.signInRequired.value)
// Attempting to go back from AuthLoginScreen when single element stack
NavigationController.goBack(fallback = ChatScreen)
// Must fall back to LandingScreen, not ChatScreen
assertEquals(LandingScreen, stack.lastOrNull())
}
@Test
fun testGoBackClearsToLandingWhenSignInRequiredAndExposingProtectedScreen() {
val stack = NavBackStack<NavKey>(ChatScreen)
stack.add(AuthLoginScreen)
NavigationController.backStack = stack
AuthSessionState.requireSignIn()
assertTrue(AuthSessionState.signInRequired.value)
// Attempting to pop AuthLoginScreen when ChatScreen is underneath
NavigationController.goBack()
// Must not expose ChatScreen
assertEquals(LandingScreen, stack.lastOrNull())
}
}
@@ -1,9 +1,11 @@
package com.m57.hermescontrol.data.remote package com.m57.hermescontrol.data.remote
import com.m57.hermescontrol.data.local.AuthSessionState
import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runBlocking
import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.MediaType.Companion.toMediaTypeOrNull
import okhttp3.ResponseBody.Companion.toResponseBody import okhttp3.ResponseBody.Companion.toResponseBody
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
import retrofit2.Response import retrofit2.Response
@@ -196,4 +198,36 @@ class NetworkResultTest {
assertTrue(unknownError.message.contains("Unexpected error")) assertTrue(unknownError.message.contains("Unexpected error"))
assertEquals(1, callCount) // Should not retry for unknown exceptions assertEquals(1, callCount) // Should not retry for unknown exceptions
} }
@Test
fun testSafeApiCall_authExpired_triggersLatchWhenReportTrue() =
runBlocking {
AuthSessionState.resetForTest()
assertFalse(AuthSessionState.signInRequired.value)
val result =
safeApiCall<String>(reportAuthExpiry = true) {
Response.error(401, "Unauthorized".toResponseBody("text/plain".toMediaTypeOrNull()))
}
assertTrue(result is NetworkResult.Failure)
assertTrue((result as NetworkResult.Failure).error is NetworkError.AuthExpired)
assertTrue(AuthSessionState.signInRequired.value)
}
@Test
fun testSafeApiCall_authExpired_doesNotTriggerLatchWhenReportFalse() =
runBlocking {
AuthSessionState.resetForTest()
assertFalse(AuthSessionState.signInRequired.value)
val result =
safeApiCall<String>(reportAuthExpiry = false) {
Response.error(401, "Unauthorized".toResponseBody("text/plain".toMediaTypeOrNull()))
}
assertTrue(result is NetworkResult.Failure)
assertTrue((result as NetworkResult.Failure).error is NetworkError.AuthExpired)
assertFalse(AuthSessionState.signInRequired.value)
}
} }
@@ -3,9 +3,12 @@ package com.m57.hermescontrol.ui.authlogin
import android.app.Application import android.app.Application
import android.util.Log import android.util.Log
import com.m57.hermescontrol.R import com.m57.hermescontrol.R
import com.m57.hermescontrol.data.config.ConnectionProfile
import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.AuthManager
import com.m57.hermescontrol.data.local.AuthSessionState
import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.data.remote.OkHttpProvider
import com.m57.hermescontrol.data.remote.ServerEndpoint import com.m57.hermescontrol.data.remote.ServerEndpoint
import com.m57.hermescontrol.data.ws.HermesWsClient
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import io.mockk.mockkObject import io.mockk.mockkObject
@@ -19,6 +22,7 @@ import org.junit.After
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import java.io.IOException import java.io.IOException
@@ -152,4 +156,33 @@ class AuthLoginViewModelTest {
viewModel.deriveAuthMode(authRequired = true, providers = listOf("weird")), viewModel.deriveAuthMode(authRequired = true, providers = listOf("weird")),
) )
} }
@Test
fun `loadLoggedInProfiles when signInRequired is true clears profile shortcuts`() {
AuthSessionState.requireSignIn()
every { AuthManager.getConnectionProfiles() } returns
listOf(
ConnectionProfile(id = "p1", name = "P1", baseUrl = "http://127.0.0.1"),
)
every { AuthManager.getProfileToken("p1") } returns "token1"
viewModel.loadLoggedInProfiles()
assertTrue(viewModel.uiState.value.loggedInProfiles.isEmpty())
AuthSessionState.resetForTest()
}
@Test
fun `useExistingProfile marks session authenticated`() {
AuthSessionState.requireSignIn()
mockkObject(HermesWsClient)
every { AuthManager.setSelectedProfileId("p1") } returns Unit
every { HermesWsClient.connect() } returns Unit
viewModel.useExistingProfile("p1")
assertFalse(AuthSessionState.signInRequired.value)
assertTrue(viewModel.uiState.value.connectionSuccess)
AuthSessionState.resetForTest()
}
} }