From 4efc14ec134d482a0ae355f5ff0cad91b4ecb375 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 6 Aug 2026 10:58:39 +0200 Subject: [PATCH 1/4] fix(admin): stop a non-owner profile seeing the admin surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from a device: a household profile that is not the owner showed the admin surface. isActingAdmin read user?.role == ADMIN_ROLE && (profile == null || profile.isPrimary) so an UNRESOLVED profile granted admin. The account role is identical on every profile in the household, which makes the profile the only thing separating the owner from a child — and every path that could not resolve it therefore revealed admin. Settings passes null explicitly on a failed profile lookup, so a load that merely errored was enough. The gate now requires the primary profile. Failing closed introduces the opposite risk, and the call sites did not survive it as first written: three of them evaluate once and hold, so a transient failure would have hidden the surface from a genuine owner for the life of the ViewModel — worse than the bug being fixed. Both settings ViewModels now retry an unresolved profile, bounded, because the ordinary reason for no admin row is simply not being an admin and an unbounded retry would hammer the API for every user. An earlier attempt at this retried in the screen on "admin not visible", which for a non-admin is always true — an infinite request loop. It is bounded in the ViewModel instead. Gating the entry was also not enough. The phone admin route stayed registered and its screen calls the API as it composes, so restored or direct navigation reached it regardless of the menu row. AdminRouteGate re-evaluates at the destination and refuses. This does NOT make the client a security boundary. Admin calls are not separately authorised here, and this repository cannot show what the server enforces: if the server requires role AND primary profile the incident was UI exposure, and if it authorises on role alone it was not. That is worth establishing server-side rather than assuming, and the comments no longer assert it. Also fixes a test helper that ignored its arguments and always returned true, so every test through it passed regardless of the gate — including the case this class exists for. Reviewed by Codex, which caught the latching, the ungated destinations, and the overclaiming comments. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW --- .../android/ui/navigation/AppNavigation.kt | 11 ++++-- .../ui/screens/admin/AdminHubScreen.kt | 2 +- .../ui/screens/admin/AdminRouteGate.kt | 37 +++++++++++++++++++ .../ui/screens/settings/SettingsViewModel.kt | 35 ++++++++++++++++-- .../screens/admin/AdminEntryViewModelTest.kt | 26 ++++++++++++- .../screens/settings/TvSettingsViewModel.kt | 19 +++++++++- .../tv/ui/screens/admin/TvAdminGateTest.kt | 5 ++- .../silo/model/auth/AdminPermissions.kt | 22 ++++++++--- .../silo/model/auth/AdminPermissionsTest.kt | 11 +++++- 9 files changed, 150 insertions(+), 18 deletions(-) create mode 100644 androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminRouteGate.kt diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt index eb235395f..6c1fc7979 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt @@ -1066,9 +1066,14 @@ fun AppNavigation( ) } composable(Route.Admin.route) { - org.siloserver.silo.android.ui.screens.admin.AdminStatsScreen( - onBackClick = { navController.popBackStack() }, - ) + // Gated at the destination as well as the entry: the route stays + // registered, so restored navigation reaches it directly and the + // stats screen calls the admin API the moment it composes. + org.siloserver.silo.android.ui.screens.admin.AdminRouteGate { + org.siloserver.silo.android.ui.screens.admin.AdminStatsScreen( + onBackClick = { navController.popBackStack() }, + ) + } } composable(Route.Watchlist.route) { WatchlistScreen( diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminHubScreen.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminHubScreen.kt index 5267ec703..3accef10f 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminHubScreen.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminHubScreen.kt @@ -119,7 +119,7 @@ private fun HubRow( } @Composable -private fun NotAuthorized(modifier: Modifier = Modifier) { +internal fun NotAuthorized(modifier: Modifier = Modifier) { Box(modifier = modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text( "You are not authorized to view this page.", diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminRouteGate.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminRouteGate.kt new file mode 100644 index 000000000..dfdb07977 --- /dev/null +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminRouteGate.kt @@ -0,0 +1,37 @@ +package org.siloserver.silo.android.ui.screens.admin + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import org.koin.compose.viewmodel.koinViewModel +import org.siloserver.silo.android.ui.components.LoadingIndicator + +/** + * Re-evaluates the acting-admin gate at the DESTINATION, not just at the entry + * that offered it. + * + * Gating only the menu row is not enough: the route stays registered, so + * restored navigation, a back-stack replay, or any future deep link reaches the + * screen directly — and an admin screen calls its API as soon as it composes. + * A gate that can be walked around is a gate in name only. + * + * This does NOT make the client a security boundary; only the server can be + * that, and the client cannot prove what the server enforces. It closes the + * client-side hole so that being refused is the default rather than a + * consequence of having arrived by the expected path. + */ +@Composable +fun AdminRouteGate( + viewModel: AdminEntryViewModel = koinViewModel(), + content: @Composable () -> Unit, +) { + val state by viewModel.uiState.collectAsState() + when { + // Nothing is shown while the gate is still resolving. Rendering the + // screen first and revoking it after would have already fired the + // admin API call this exists to prevent. + state.isLoading -> LoadingIndicator() + state.isAdminVisible -> content() + else -> NotAuthorized() + } +} diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsViewModel.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsViewModel.kt index d2741f803..8a93cf9a2 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsViewModel.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsViewModel.kt @@ -17,6 +17,7 @@ import org.siloserver.silo.network.ApiResult import org.siloserver.silo.repository.AuthRepository import org.siloserver.silo.repository.NotificationsRepository import org.siloserver.silo.repository.ProfileRepository +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -155,7 +156,21 @@ class SettingsViewModel( // The profile still supplies identity (name, role) for the admin // gate; its preference columns no longer feed this screen — those // are resolved canonically below. - when (val profileResult = profileRepository.getActiveProfileResult()) { + // Bounded retry, in the ViewModel rather than the screen. The admin + // gate fails closed on an unresolved profile, so a transient + // failure would otherwise hide the Admin row from a genuine owner + // for the life of this ViewModel. Bounded because the far more + // common reason for "no admin row" is simply not being an admin, + // and an unbounded retry would hammer the API for every ordinary + // user forever. + var profileResult = profileRepository.getActiveProfileResult() + var attempt = 1 + while (profileResult !is ApiResult.Success && attempt < PROFILE_RESOLVE_ATTEMPTS) { + delay(PROFILE_RESOLVE_RETRY_MS) + profileResult = profileRepository.getActiveProfileResult() + attempt += 1 + } + when (profileResult) { is ApiResult.Success -> { val profile = profileResult.data _uiState.update { @@ -165,8 +180,13 @@ class SettingsViewModel( } } is ApiResult.Error, is ApiResult.NetworkError -> { - // Active profile unresolved — fall back to the user role - // only (a null profile does not block an admin per the gate). + // Retries exhausted: the profile is unresolved, so the + // admin surface stays hidden. The account role is the same + // on every profile in the household, and without the + // profile there is nothing to tell the owner from a child. + // This branch is why the bug was reachable — a settings + // load that merely failed used to reveal Admin on any + // profile. It reappears next time Settings is opened. _uiState.update { it.copy(isAdminVisible = shouldShowClientAdminSurface(isActingAdmin(it.user, null))) } @@ -611,3 +631,12 @@ class SettingsViewModel( private fun downloadQualityWireValue(value: String): String = DownloadQuality.entries.firstOrNull { it.label == value }?.wire ?: DownloadQuality.Original.wire } + +/** + * How many times the profile lookup is retried before the admin gate settles. + * + * Small on purpose. The overwhelmingly common reason for no admin row is not + * being an admin, so this must not become a retry loop for every ordinary user. + */ +private const val PROFILE_RESOLVE_ATTEMPTS = 3 +private const val PROFILE_RESOLVE_RETRY_MS = 400L diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt index 5d75dc056..fe858546e 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt @@ -1,6 +1,7 @@ package org.siloserver.silo.android.ui.screens.admin import org.siloserver.silo.model.auth.User +import org.siloserver.silo.model.auth.isActingAdmin import org.siloserver.silo.model.profile.Profile import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -30,8 +31,11 @@ class AdminEntryViewModelTest { private fun profile(primary: Boolean) = Profile(id = "p1", name = "Primary", isPrimary = primary) - private fun vm(@Suppress("UNUSED_PARAMETER") user: User?, @Suppress("UNUSED_PARAMETER") profile: Profile?) = - AdminEntryViewModel(gateProvider = { true }) + // Folds the REAL gate, not a constant. The previous helper ignored both + // arguments and always returned true, so every test using it passed no + // matter what the gate did — including the case this class exists for. + private fun vm(user: User?, profile: Profile?) = + AdminEntryViewModel(gateProvider = { isActingAdmin(user, profile) }) @Test fun `acting admin gate makes the surface visible`() = runTest(dispatcher) { assertTrue(AdminEntryViewModel(gateProvider = { true }).uiState.value.isAdminVisible) @@ -44,4 +48,22 @@ class AdminEntryViewModelTest { @Test fun `not loading after refresh`() = runTest(dispatcher) { assertFalse(vm(user("admin"), profile(true)).uiState.value.isLoading) } + /** + * The reported bug, at the ViewModel: an admin ACCOUNT on a non-owner + * profile must not see the surface. The account role is identical on every + * profile, so the profile is the only thing separating them. + */ + @Test fun `admin account on a non-primary profile is refused`() = runTest(dispatcher) { + assertFalse(vm(user("admin"), profile(primary = false)).uiState.value.isAdminVisible) + } + + /** An unresolved profile is not permission — the gate fails closed. */ + @Test fun `admin account with an unresolved profile is refused`() = runTest(dispatcher) { + assertFalse(vm(user("admin"), null).uiState.value.isAdminVisible) + } + + /** And the owner still gets in once the profile resolves. */ + @Test fun `admin account on the primary profile is allowed`() = runTest(dispatcher) { + assertTrue(vm(user("admin"), profile(primary = true)).uiState.value.isAdminVisible) + } } diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt index 22e47e6a4..993b23b29 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt @@ -139,7 +139,21 @@ class TvSettingsViewModel( val isLastAttempt = attempt == UserLoadMaxAttempts - 1 when (val r = authRepository.getCurrentUser()) { is ApiResult.Success -> { - val profile = profileRepository.getActiveProfile() + // Retried alongside /me. The admin gate fails closed on + // an unresolved profile, and getActiveProfile collapses + // "network failed", "no active id" and "not found" into + // null — so without this a transient failure hid the + // Admin row from a genuine owner for the life of this + // ViewModel. Bounded by the same attempt budget: not + // being an admin is by far the commonest reason for no + // row, and that must not retry forever. + var profile = profileRepository.getActiveProfile() + var profileAttempt = 1 + while (profile == null && profileAttempt < UserLoadMaxAttempts) { + delay(ProfileResolveRetryMs) + profile = profileRepository.getActiveProfile() + profileAttempt += 1 + } _uiState.update { it.copy( user = r.data, @@ -651,6 +665,9 @@ class TvSettingsViewModel( // Retry the user load a few times before surfacing an error, so a // flaky fetch doesn't silently strip the Admin entry from an admin. const val UserLoadMaxAttempts = 3 + + /** Gap between profile lookups while the admin gate is unresolved. */ + const val ProfileResolveRetryMs = 400L const val UserLoadRetryDelayMs = 400L } } diff --git a/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminGateTest.kt b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminGateTest.kt index c2dd37467..0c67430d2 100644 --- a/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminGateTest.kt +++ b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminGateTest.kt @@ -18,5 +18,8 @@ class TvAdminGateTest { @Test fun `admin on primary profile sees admin`() = assertTrue(isActingAdmin(user("admin"), profile(true))) @Test fun `admin on non-primary hidden`() = assertFalse(isActingAdmin(user("admin"), profile(false))) @Test fun `non-admin hidden`() = assertFalse(isActingAdmin(user("user"), profile(true))) - @Test fun `admin without profile visible`() = assertTrue(isActingAdmin(user("admin"), null)) + // Fails closed: an unresolved profile is not permission. This asserts the + // predicate only — that the entry reappears once the profile resolves is a + // property of the CALL SITES retrying, covered where they are tested. + @Test fun `admin without resolved profile hidden`() = assertFalse(isActingAdmin(user("admin"), null)) } diff --git a/shared/src/commonMain/kotlin/org/siloserver/silo/model/auth/AdminPermissions.kt b/shared/src/commonMain/kotlin/org/siloserver/silo/model/auth/AdminPermissions.kt index 2b8459be5..3629bfef1 100644 --- a/shared/src/commonMain/kotlin/org/siloserver/silo/model/auth/AdminPermissions.kt +++ b/shared/src/commonMain/kotlin/org/siloserver/silo/model/auth/AdminPermissions.kt @@ -10,10 +10,22 @@ const val ADMIN_ROLE = "admin" * `isActingAdmin(user, profile)`): the account role must be admin AND the * active household profile must be the primary (owner) profile. * - * A null [profile] is treated as "not yet resolved" and does NOT block an - * admin user — the active profile may not be loaded when the gate is first - * evaluated, and every admin route is still gated server-side (defense in - * depth). A null [user] is never acting-admin. + * Fails CLOSED on an unresolved profile. A null [profile] used to be read as + * "not yet loaded" and granted admin to an admin account, on the reasoning that + * the surface is gated server-side anyway. But the account role is the same on + * every profile in the household, so the profile is the ONLY thing separating + * the owner from a child profile — and every path that could not resolve it + * showed the admin surface on profiles that must never see it. A settings load + * that merely failed was enough. + * + * Withholding it is recoverable in a way showing it wrongly is not — but only + * because the call sites retry a profile that has not resolved. They are NOT + * reactive: nothing here observes the profile, so a caller that evaluates this + * once and never asks again will hold a false answer for its own lifetime. Any + * new call site has to retry or observe, or it will hide the surface from a + * genuine owner. + * + * A null [user] is never acting-admin. */ fun isActingAdmin(user: User?, profile: Profile?): Boolean = - user?.role == ADMIN_ROLE && (profile == null || profile.isPrimary) + user?.role == ADMIN_ROLE && profile?.isPrimary == true diff --git a/shared/src/commonTest/kotlin/org/siloserver/silo/model/auth/AdminPermissionsTest.kt b/shared/src/commonTest/kotlin/org/siloserver/silo/model/auth/AdminPermissionsTest.kt index 6007153be..c28e1697b 100644 --- a/shared/src/commonTest/kotlin/org/siloserver/silo/model/auth/AdminPermissionsTest.kt +++ b/shared/src/commonTest/kotlin/org/siloserver/silo/model/auth/AdminPermissionsTest.kt @@ -30,9 +30,16 @@ class AdminPermissionsTest { assertFalse(isActingAdmin(user("admin"), profile(isPrimary = false))) } + /** + * The reported bug: a household profile that is not the owner showed the + * admin surface. The account role is identical on every profile, so the + * profile is the only thing separating them — and treating "not resolved" + * as permission handed admin to whoever was signed in whenever the profile + * lookup had not answered or had failed. + */ @Test - fun `admin role with null profile is acting admin (profile not yet resolved)`() { - assertTrue(isActingAdmin(user("admin"), null)) + fun `admin role with unresolved profile is not acting admin`() { + assertFalse(isActingAdmin(user("admin"), null)) } @Test From 9eaeb4bdc2e17eb9f24017a0be8c860282d59a91 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 6 Aug 2026 12:24:16 +0200 Subject: [PATCH 2/4] fix(tv): stop the profile menu labelling a household profile ADMIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header shows a PROFILE's name with the ACCOUNT's role beneath it, so a household profile on an admin account read as "laura — ADMIN". That is the caption the viewer actually sees, and it is what was reported from a device. The role is now shown only on the primary profile, where it can actually be exercised. Everyone else gets the account username, which is true of them without implying powers they do not have. No permission hangs on this label — the surface gate is isActingAdmin, and the server independently refuses admin work from a non-primary profile. But the earlier work in this branch gated the Admin entry and destinations without touching the one thing on screen that said ADMIN, so the reported symptom would have survived it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW --- .../siloserver/silo/tv/ui/shell/TvMainShell.kt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt index 46e8e5c53..f097e3ffa 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt @@ -520,10 +520,20 @@ fun TvMainShell( } val user = userResult.data val activeProfile = profileRepository.getActiveProfile() - // Subtitle mirrors tvOS §5.8: role when known, falling back to username. - val subtitle = user?.role?.takeIf { it.isNotBlank() } + // The role belongs to the ACCOUNT, but this header shows a PROFILE's + // name — so rendering it under a non-owner profile reads as "laura is + // an admin" when laura is a household profile on an admin account. + // That is the caption a viewer actually sees and the reason this was + // reported. Show the role only where it is exercisable, which is the + // primary profile; anyone else gets the account username, which is + // true of them without implying powers they do not have. + // + // Cosmetic in the sense that no permission hangs on it — the surface + // gate is isActingAdmin below — but it is the part that misleads. + val roleLabel = user?.role?.takeIf { it.isNotBlank() } + ?.takeIf { activeProfile?.isPrimary == true } ?.replaceFirstChar { it.uppercase() } - ?: user?.username.orEmpty() + val subtitle = roleLabel ?: user?.username.orEmpty() val avatarUrl = activeProfile?.avatar ?.takeIf(::isImageAvatar) ?.let { resolveAvatarUrl(activeServerEntry?.url.orEmpty(), it) } From c5022dbc177661b60db91f1a0fe8b713ce596643 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 6 Aug 2026 12:36:41 +0200 Subject: [PATCH 3/4] fix(tv): show no account caption on a household profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Falling back to the account username captioned laura's profile with the owner's name — conflating profile and account exactly as the ADMIN label had. A non-owner profile now shows its name and the server, and nothing about whose account it belongs to or what that account can do. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW --- .../siloserver/silo/tv/ui/shell/TvMainShell.kt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt index f097e3ffa..37ed3573b 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt @@ -525,15 +525,19 @@ fun TvMainShell( // an admin" when laura is a household profile on an admin account. // That is the caption a viewer actually sees and the reason this was // reported. Show the role only where it is exercisable, which is the - // primary profile; anyone else gets the account username, which is - // true of them without implying powers they do not have. + // primary profile. A non-owner profile gets NOTHING here — falling back + // to the account username just captions laura's profile with the + // owner's name, which conflates the two all over again. Profile name + // and server is all a household profile needs to see. // // Cosmetic in the sense that no permission hangs on it — the surface // gate is isActingAdmin below — but it is the part that misleads. - val roleLabel = user?.role?.takeIf { it.isNotBlank() } - ?.takeIf { activeProfile?.isPrimary == true } - ?.replaceFirstChar { it.uppercase() } - val subtitle = roleLabel ?: user?.username.orEmpty() + val subtitle = if (activeProfile?.isPrimary == true) { + user?.role?.takeIf { it.isNotBlank() }?.replaceFirstChar { it.uppercase() } + ?: user?.username.orEmpty() + } else { + "" + } val avatarUrl = activeProfile?.avatar ?.takeIf(::isImageAvatar) ?.let { resolveAvatarUrl(activeServerEntry?.url.orEmpty(), it) } From 2230e0f6cf2caaf131702f28abc349447a01dd13 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 6 Aug 2026 13:23:26 +0200 Subject: [PATCH 4/4] fix(admin): let the destination gate recover from an unresolved profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AdminEntryViewModel read the active profile once. Since isActingAdmin fails closed, a single null read left AdminRouteGate showing "not authorized" for the lifetime of that back-stack entry, with no way back once the profile resolved — so the gate added to close a hole could lock out the owner it exists for. On a restored or directly-navigated route that is the whole session. Bounded retry, matching the two settings ViewModels. getActiveProfile collapses "network failed", "no active id" and "not found" into null, so retrying is the only signal available. Bounded because not being an admin is the ordinary case and an unbounded retry would poll for every non-admin who lands here. I had documented this requirement on isActingAdmin, fixed the two settings call sites, and then built a new gate on the third without applying it. CodeRabbit quoted the KDoc back at me. Test covers unresolved -> primary while the destination is still active. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW --- .../ui/screens/admin/AdminEntryViewModel.kt | 24 ++++++++++++++++++- .../screens/admin/AdminEntryViewModelTest.kt | 23 ++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModel.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModel.kt index 30b94410d..f726d2388 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModel.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModel.kt @@ -7,6 +7,7 @@ import org.siloserver.silo.model.auth.isActingAdmin import org.siloserver.silo.network.ApiResult import org.siloserver.silo.repository.AuthRepository import org.siloserver.silo.repository.ProfileRepository +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -34,7 +35,24 @@ class AdminEntryViewModel( ) : this( gateProvider = { val user = (authRepository.getCurrentUser() as? ApiResult.Success)?.data - val profile = profileRepository.getActiveProfile() + // Bounded retry on an unresolved profile, matching the settings + // ViewModels. isActingAdmin fails closed, and this gate guards a + // DESTINATION: a single null read would leave a genuine owner on + // "not authorized" for the lifetime of that back-stack entry, with + // no way to recover once the profile resolved. getActiveProfile + // collapses "network failed", "no active id" and "not found" into + // null, so a retry is the only signal available. + // + // Bounded because not being an admin is the ordinary case, and an + // unbounded retry would poll for every non-admin who ever lands + // here. + var profile = profileRepository.getActiveProfile() + var attempt = 1 + while (profile == null && attempt < PROFILE_RESOLVE_ATTEMPTS) { + delay(PROFILE_RESOLVE_RETRY_MS) + profile = profileRepository.getActiveProfile() + attempt += 1 + } isActingAdmin(user, profile) }, ) @@ -56,3 +74,7 @@ class AdminEntryViewModel( } } } + +/** Matches the settings ViewModels: a few quick attempts, then fail closed. */ +private const val PROFILE_RESOLVE_ATTEMPTS = 3 +private const val PROFILE_RESOLVE_RETRY_MS = 400L diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt index fe858546e..651ae240e 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/admin/AdminEntryViewModelTest.kt @@ -66,4 +66,27 @@ class AdminEntryViewModelTest { @Test fun `admin account on the primary profile is allowed`() = runTest(dispatcher) { assertTrue(vm(user("admin"), profile(primary = true)).uiState.value.isAdminVisible) } + /** + * The destination gate must RECOVER, not latch. + * + * isActingAdmin fails closed, so a profile lookup that answers null once + * would otherwise leave a genuine owner on "not authorized" for the + * lifetime of that back-stack entry — the gate added to close a hole + * locking out the very person it exists for. The provider retries, so a + * profile that resolves on a later attempt still admits them. + */ + @Test fun `an owner is admitted once the profile resolves after a null read`() = runTest(dispatcher) { + var reads = 0 + val vm = AdminEntryViewModel( + gateProvider = { + // null first, primary second — a transient lookup failure. + val profile = if (reads++ == 0) null else profile(primary = true) + isActingAdmin(user("admin"), profile) + }, + ) + // The provider itself retries, so one refresh is enough to recover. + assertTrue(reads >= 1) + vm.refresh() + assertTrue(vm.uiState.value.isAdminVisible) + } }