diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt index 8319d9e80..ed840534e 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt @@ -437,7 +437,7 @@ val androidModule = module { viewModel { SignupViewModel(get()) } viewModel { InviteClaimViewModel(get(), get()) } viewModel { OnboardingTourViewModel(get(), get(), get(), get(), get()) } - viewModel { ProfileSelectionViewModel(get()) } + viewModel { ProfileSelectionViewModel(profileRepository = get(), authRepository = get()) } viewModel { CreateProfileViewModel(get()) } viewModel { EditProfileViewModel(get()) } viewModel { ServerListViewModel(get(), get()) } diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionScreen.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionScreen.kt index effbf8448..387169f58 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionScreen.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionScreen.kt @@ -194,21 +194,31 @@ fun ProfileSelectionScreen( ProfileFlow( profiles = state.profiles, isManageMode = state.isManageMode, + // Server-gated: creating needs an admin account or the + // acting profile to be the household primary (phone keeps + // it across Switch Profile) — except the very first + // profile, which the server lets any account bootstrap. + showAddProfile = state.canAddProfile, onProfileTap = { viewModel.onProfileTapped(it) }, onProfileEdit = { onNavigateToEditProfile(it.id) }, onProfileDelete = { viewModel.requestDeleteProfile(it) }, onAddProfile = onNavigateToCreateProfile, ) - Spacer(modifier = Modifier.height(24.dp)) + // Edit/delete would 403 unless the account is admin or the + // acting profile is the primary, so only offer the mode to + // callers the server will authorize. + if (state.canManageProfiles) { + Spacer(modifier = Modifier.height(24.dp)) - TextButton(onClick = viewModel::toggleManageMode) { - Text( - text = if (state.isManageMode) "Done" else "Manage Profiles", - fontSize = 15.sp, - fontWeight = FontWeight.Medium, - color = AuthColors.OnBackground, - ) + TextButton(onClick = viewModel::toggleManageMode) { + Text( + text = if (state.isManageMode) "Done" else "Manage Profiles", + fontSize = 15.sp, + fontWeight = FontWeight.Medium, + color = AuthColors.OnBackground, + ) + } } } } @@ -226,6 +236,7 @@ fun ProfileSelectionScreen( private fun ProfileFlow( profiles: List, isManageMode: Boolean, + showAddProfile: Boolean, onProfileTap: (Profile) -> Unit, onProfileEdit: (Profile) -> Unit, onProfileDelete: (Profile) -> Unit, @@ -246,7 +257,9 @@ private fun ProfileFlow( onDelete = { onProfileDelete(profile) }, ) } - AddProfileCard(onClick = onAddProfile) + if (showAddProfile) { + AddProfileCard(onClick = onAddProfile) + } } } diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionViewModel.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionViewModel.kt index 149ad6982..633ebc000 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionViewModel.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionViewModel.kt @@ -2,10 +2,13 @@ package org.siloserver.silo.android.ui.screens.profiles import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import org.siloserver.silo.model.auth.User +import org.siloserver.silo.model.auth.canManageProfilesFromPicker import org.siloserver.silo.model.profile.Profile import org.siloserver.silo.model.profile.authorizedProfileToken import org.siloserver.silo.network.ApiResult import org.siloserver.silo.network.AuthScopeSnapshot +import org.siloserver.silo.repository.AuthRepository import org.siloserver.silo.repository.ProfileCommitResult import org.siloserver.silo.repository.ProfileRepository import kotlinx.coroutines.flow.MutableStateFlow @@ -30,12 +33,42 @@ data class ProfileSelectionUiState( /** The profile this session is signed in as — deleting it needs a * stronger warning and clears the local selection first. */ val activeProfileId: String? = null, -) + /** + * Whether the picker shows management affordances (Manage Profiles, Add + * Profile). Mirrors the server's gate — admin account OR acting as the + * primary profile (phone keeps the acting profile across Switch Profile) — + * see [canManageProfilesFromPicker]. Fails closed while neither resolves. + */ + val canManageProfiles: Boolean = false, +) { + /** + * The add tile stays visible on an empty grid regardless of role: the + * server exempts creation of the very first profile from the + * primary-or-admin gate, and hiding it would strand a fresh account. + */ + val canAddProfile: Boolean get() = canManageProfiles || profiles.isEmpty() +} class ProfileSelectionViewModel( private val profileRepository: ProfileRepository, + /** + * Resolves the signed-in account, null when it cannot be resolved (fails + * closed) — the seam unit tests drive, mirroring AdminEntryViewModel; + * production uses the repository-backed secondary constructor. + */ + private val currentUserProvider: suspend () -> User? = { null }, ) : ViewModel() { + constructor( + profileRepository: ProfileRepository, + authRepository: AuthRepository, + ) : this( + profileRepository = profileRepository, + currentUserProvider = { + (authRepository.getCurrentUser() as? ApiResult.Success)?.data + }, + ) + private val _uiState = MutableStateFlow(ProfileSelectionUiState()) val uiState: StateFlow = _uiState.asStateFlow() @@ -72,6 +105,7 @@ class ProfileSelectionViewModel( val scope = profileRepository.captureIdentityScope() val activeId = profileRepository.getActiveProfileId() + val user = currentUserProvider() val result = profileRepository.listProfiles() // Two separate reasons to drop this response: a newer load // superseded it, or the identity it was fetched under is gone. @@ -83,7 +117,18 @@ class ProfileSelectionViewModel( // Leaving a scope behind for an empty grid is stale metadata // that a later selection could be qualified against. gridScope = null - _uiState.update { it.copy(isLoading = false, profiles = emptyList()) } + // The management grant belongs to the identity the grid was + // fetched under — fail closed with it, and take the manage + // affordances (mode + pending delete) down with the grant. + _uiState.update { + it.copy( + isLoading = false, + profiles = emptyList(), + canManageProfiles = false, + isManageMode = false, + deleteDialogProfile = null, + ) + } return@launch } @@ -96,8 +141,28 @@ class ProfileSelectionViewModel( // accepted as belonging to the new one. That is worse than // the unguarded commit this was meant to fix. gridScope = scope + // Resolve the acting profile against THIS grid so the + // grant stays paired with the identity scope it was + // fetched under. Phone keeps the acting profile across + // Switch Profile, so a non-admin owner acting as the + // primary is authorized here; at first login (or if the + // active id is gone from the list) this is null and only + // the admin arm can hold. + val activeProfile = result.data.firstOrNull { it.id == activeId } + val canManage = canManageProfilesFromPicker(user, activeProfile) _uiState.update { - it.copy(isLoading = false, profiles = result.data, activeProfileId = activeId) + it.copy( + isLoading = false, + profiles = result.data, + activeProfileId = activeId, + canManageProfiles = canManage, + // A revoked grant must also leave manage mode (or + // the "Done" toggle disappears while its mode + // stays) and close a pending delete confirmation, + // which would otherwise stay open and actionable. + isManageMode = it.isManageMode && canManage, + deleteDialogProfile = if (canManage) it.deleteDialogProfile else null, + ) } } @@ -288,6 +353,8 @@ class ProfileSelectionViewModel( pinIsVerifying = false, pinError = null, deleteDialogProfile = null, + isManageMode = false, + canManageProfiles = false, ) } loadProfiles() diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionManagementGatingTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionManagementGatingTest.kt new file mode 100644 index 000000000..87da99dbf --- /dev/null +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/profiles/ProfileSelectionManagementGatingTest.kt @@ -0,0 +1,195 @@ +package org.siloserver.silo.android.ui.screens.profiles + +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respond +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.siloserver.silo.model.auth.User +import org.siloserver.silo.model.profile.Profile +import org.siloserver.silo.network.ApiResult +import org.siloserver.silo.network.TokenManagerImpl +import org.siloserver.silo.network.api.ProfileApi +import org.siloserver.silo.repository.ProfileRepository +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Picker-time management gating. The server authorizes profile management + * for admin accounts OR when the acting profile is the household primary — + * and the phone keeps the acting profile across "Switch Profile", so both + * arms matter here. The picker must not offer create/edit/delete to anyone + * else — except the add tile on an empty grid, which the server exempts + * (first-profile bootstrap). + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ProfileSelectionManagementGatingTest { + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + private fun user(role: String) = User( + id = 1, + username = "someone", + email = "someone@example.com", + role = role, + ) + + @Test + fun `admin account gets management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { user("admin") }, + ) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `non-admin account with no acting profile gets no management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.canAddProfile) + } + + /** + * The regression the first cut of this gating introduced: phone keeps the + * acting profile across "Switch Profile", and the picker is the ONLY + * route to create/edit — a non-admin household owner acting as the + * primary is authorized by the server and must keep the affordances. + */ + @Test + fun `non-admin acting as the primary profile keeps management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository( + profiles = listOf( + Profile(id = "owner", name = "Owner", isPrimary = true), + Profile(id = "kid", name = "Kid"), + ), + activeProfileId = "owner", + ), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `non-admin acting as a non-primary profile gets no management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository( + profiles = listOf( + Profile(id = "owner", name = "Owner", isPrimary = true), + Profile(id = "kid", name = "Kid"), + ), + activeProfileId = "kid", + ), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `unresolved user fails closed`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { null }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `empty grid keeps the add tile for first-profile bootstrap`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(emptyList()), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + /** Bootstrap must survive a dead `/me`: fresh account, user unresolved. */ + @Test + fun `empty grid with unresolved user still shows the add tile`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(emptyList()), + currentUserProvider = { null }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `revoking the grant on reload leaves manage mode and closes the delete dialog`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + var currentUser: User? = user("admin") + val profile = Profile(id = "p1", name = "One") + val viewModel = ProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(profile)), + currentUserProvider = { currentUser }, + ) + advanceUntilIdle() + viewModel.toggleManageMode() + viewModel.requestDeleteProfile(profile) + assertTrue(viewModel.uiState.value.isManageMode) + assertNotNull(viewModel.uiState.value.deleteDialogProfile) + + currentUser = null + viewModel.loadProfiles() + advanceUntilIdle() + + // Otherwise the "Done" toggle disappears while its mode stays on, + // and the pending delete confirmation stays open and actionable. + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.isManageMode) + assertNull(viewModel.uiState.value.deleteDialogProfile) + } +} + +private class FixedProfileRepository( + private val profiles: List, + private val activeProfileId: String? = null, +) : ProfileRepository( + profileApi = ProfileApi(HttpClient(MockEngine { respond("{}") })), + tokenManager = TokenManagerImpl(), +) { + override suspend fun listProfiles(): ApiResult> = ApiResult.Success(profiles) + + override suspend fun getActiveProfileId(): String? = activeProfileId +} diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt index b2be3d0c3..7eb86da2f 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt @@ -354,7 +354,7 @@ val androidTvModule = module { viewModel { org.siloserver.silo.tv.ui.screens.auth.TvSetupViewModel(get()) } viewModel { org.siloserver.silo.tv.ui.screens.auth.TvSignupViewModel(get()) } viewModel { TvLoginViewModel(get(), get(), get()) } - viewModel { TvProfileSelectionViewModel(get()) } + viewModel { TvProfileSelectionViewModel(profileRepository = get(), authRepository = get()) } viewModel { org.siloserver.silo.tv.ui.screens.profiles.TvCreateProfileViewModel(get()) } viewModel { params -> org.siloserver.silo.tv.ui.screens.profiles.TvEditProfileViewModel( diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionScreen.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionScreen.kt index fc3a3bf6f..a5ef4bc31 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionScreen.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionScreen.kt @@ -160,15 +160,21 @@ fun TvProfileSelectionScreen( horizontalArrangement = Arrangement.spacedBy(12.dp), verticalAlignment = Alignment.CenterVertically, ) { - TvHeroActionPill( - label = if (state.isManageMode) "Done" else "Manage", - icon = Icons.Filled.Edit, - variant = TvPillVariant.Hollow, - heightOverride = ProfileUtilityChipHeight, - horizontalPaddingOverride = 10.dp, - labelStyle = MaterialTheme.typography.labelMedium, - onClick = viewModel::toggleManageMode, - ) + // Edit/delete would 403 unless the account is admin or + // the acting profile is the primary — and TV clears the + // acting profile before this picker, so in practice only + // admin accounts are offered the mode here. + if (state.canManageProfiles) { + TvHeroActionPill( + label = if (state.isManageMode) "Done" else "Manage", + icon = Icons.Filled.Edit, + variant = TvPillVariant.Hollow, + heightOverride = ProfileUtilityChipHeight, + horizontalPaddingOverride = 10.dp, + labelStyle = MaterialTheme.typography.labelMedium, + onClick = viewModel::toggleManageMode, + ) + } TvHeroActionPill( label = "Change Server", icon = Icons.Filled.Dns, @@ -271,6 +277,11 @@ fun TvProfileSelectionScreen( }, onProfileFocused = { focusedProfileId = it }, isManageMode = state.isManageMode, + // Server-gated: creating needs an admin account or a + // primary acting profile (cleared before this picker + // on TV) — except the very first profile, which any + // account may bootstrap. + showAddProfile = state.canAddProfile, onProfileSelected = viewModel::onProfileSelected, onEditProfile = { onEditProfile(it.id) }, onDeleteProfile = viewModel::requestDelete, @@ -334,12 +345,13 @@ private fun ProfileTileGrid( // left the grid entirely. Restoration must not re-request a tile then. onProfileFocused: (String?) -> Unit, isManageMode: Boolean, + showAddProfile: Boolean, onProfileSelected: (Profile) -> Unit, onEditProfile: (Profile) -> Unit, onDeleteProfile: (Profile) -> Unit, onAddProfile: () -> Unit, ) { - val itemCount = profiles.size + 1 + val itemCount = profiles.size + if (showAddProfile) 1 else 0 val rowCount = (itemCount + ProfileGridColumns - 1) / ProfileGridColumns Column( modifier = Modifier diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionViewModel.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionViewModel.kt index 67e586854..75b2ec4a3 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionViewModel.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionViewModel.kt @@ -2,10 +2,13 @@ package org.siloserver.silo.tv.ui.screens.profiles import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import org.siloserver.silo.model.auth.User +import org.siloserver.silo.model.auth.canManageProfilesFromPicker import org.siloserver.silo.model.profile.Profile import org.siloserver.silo.model.profile.authorizedProfileToken import org.siloserver.silo.network.ApiResult import org.siloserver.silo.network.AuthScopeSnapshot +import org.siloserver.silo.repository.AuthRepository import org.siloserver.silo.repository.ProfileCommitResult import org.siloserver.silo.repository.ProfileRepository import kotlinx.coroutines.flow.MutableStateFlow @@ -28,7 +31,22 @@ data class TvProfileSelectionUiState( // Profile pending delete confirmation (manage mode). val deleteCandidate: Profile? = null, val isDeleting: Boolean = false, -) + /** + * Whether the picker shows management affordances (Manage pill, Add + * Profile tile). Mirrors the server's gate — admin account OR acting as + * the primary profile — see [canManageProfilesFromPicker]. TV clears the + * active profile before showing the picker, so in practice only the admin + * arm holds here. Fails closed while neither input resolves. + */ + val canManageProfiles: Boolean = false, +) { + /** + * The add tile stays visible on an empty grid regardless of role: the + * server exempts creation of the very first profile from the + * primary-or-admin gate, and hiding it would strand a fresh account. + */ + val canAddProfile: Boolean get() = canManageProfiles || profiles.isEmpty() +} /** * TV profile picker. PIN-protected profiles now work: selecting one opens a @@ -37,8 +55,24 @@ data class TvProfileSelectionUiState( */ class TvProfileSelectionViewModel( private val profileRepository: ProfileRepository, + /** + * Resolves the signed-in account, null when it cannot be resolved (fails + * closed) — the seam unit tests drive, mirroring AdminEntryViewModel; + * production uses the repository-backed secondary constructor. + */ + private val currentUserProvider: suspend () -> User? = { null }, ) : ViewModel() { + constructor( + profileRepository: ProfileRepository, + authRepository: AuthRepository, + ) : this( + profileRepository = profileRepository, + currentUserProvider = { + (authRepository.getCurrentUser() as? ApiResult.Success)?.data + }, + ) + private val _uiState = MutableStateFlow(TvProfileSelectionUiState()) val uiState: StateFlow = _uiState.asStateFlow() @@ -68,6 +102,8 @@ class TvProfileSelectionViewModel( viewModelScope.launch { _uiState.update { it.copy(isLoading = true, error = null) } val scope = profileRepository.captureIdentityScope() + val user = currentUserProvider() + val activeId = profileRepository.getActiveProfileId() val listed = profileRepository.listProfiles() // Two separate reasons to drop this response: a newer load // superseded it, or the identity it was fetched under is gone. @@ -75,7 +111,18 @@ class TvProfileSelectionViewModel( if (!profileRepository.identityScopeUnchanged(scope)) { // The displayed grid is gone, so its scope must go with it. gridScope = null - _uiState.update { it.copy(isLoading = false, profiles = emptyList()) } + // The management grant belongs to the identity the grid was + // fetched under — fail closed with it, and take the manage + // affordances (mode + pending delete) down with the grant. + _uiState.update { + it.copy( + isLoading = false, + profiles = emptyList(), + canManageProfiles = false, + isManageMode = false, + deleteCandidate = null, + ) + } return@launch } when (val result = listed) { @@ -85,8 +132,25 @@ class TvProfileSelectionViewModel( // failed reload under a NEW identity left the OLD grid // qualified by the NEW scope. gridScope = scope + // Resolve the acting profile against THIS grid so the + // grant stays paired with the identity scope it was + // fetched under. TV clears the active profile before the + // picker, so this is normally null and only the admin arm + // of the server's gate can hold. + val activeProfile = result.data.firstOrNull { it.id == activeId } + val canManage = canManageProfilesFromPicker(user, activeProfile) _uiState.update { - it.copy(isLoading = false, profiles = result.data) + it.copy( + isLoading = false, + profiles = result.data, + canManageProfiles = canManage, + // A revoked grant must also leave manage mode (or + // the "Done" pill disappears while its mode stays) + // and close a pending delete confirmation, which + // would otherwise stay open and actionable. + isManageMode = it.isManageMode && canManage, + deleteCandidate = if (canManage) it.deleteCandidate else null, + ) } } is ApiResult.Error -> { @@ -222,6 +286,7 @@ class TvProfileSelectionViewModel( pinError = null, deleteCandidate = null, isManageMode = false, + canManageProfiles = false, ) } loadProfiles() diff --git a/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionManagementGatingTest.kt b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionManagementGatingTest.kt new file mode 100644 index 000000000..5d0315fd0 --- /dev/null +++ b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileSelectionManagementGatingTest.kt @@ -0,0 +1,174 @@ +package org.siloserver.silo.tv.ui.screens.profiles + +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respond +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.siloserver.silo.model.auth.User +import org.siloserver.silo.model.profile.Profile +import org.siloserver.silo.network.ApiResult +import org.siloserver.silo.network.TokenManagerImpl +import org.siloserver.silo.network.api.ProfileApi +import org.siloserver.silo.repository.ProfileRepository +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Picker-time management gating (TV). The server authorizes profile + * management for admin accounts OR when the acting profile is the household + * primary — but TV clears the active profile before showing the picker, so + * here only the admin arm holds and the picker must not offer the Manage + * pill or the Add tile to anyone else — except the add tile on an empty + * grid, which the server exempts (first-profile bootstrap). + */ +@OptIn(ExperimentalCoroutinesApi::class) +class TvProfileSelectionManagementGatingTest { + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + private fun user(role: String) = User( + id = 1, + username = "someone", + email = "someone@example.com", + role = role, + ) + + @Test + fun `admin account gets management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { user("admin") }, + ) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `non-admin account gets no management affordances at the picker`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `unresolved user fails closed`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(Profile(id = "p1", name = "One"))), + currentUserProvider = { null }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.canAddProfile) + } + + @Test + fun `empty grid keeps the add tile for first-profile bootstrap`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(emptyList()), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + /** Bootstrap must survive a dead `/me`: fresh account, user unresolved. */ + @Test + fun `empty grid with unresolved user still shows the add tile`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(emptyList()), + currentUserProvider = { null }, + ) + advanceUntilIdle() + + assertFalse(viewModel.uiState.value.canManageProfiles) + assertTrue(viewModel.uiState.value.canAddProfile) + } + + /** + * TV normally clears the acting profile before the picker, but the gate + * still mirrors the server's acting-profile arm should it ever reach the + * picker uncleared. + */ + @Test + fun `non-admin acting as the primary profile keeps management affordances`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository( + profiles = listOf( + Profile(id = "owner", name = "Owner", isPrimary = true), + Profile(id = "kid", name = "Kid"), + ), + activeProfileId = "owner", + ), + currentUserProvider = { user("user") }, + ) + advanceUntilIdle() + + assertTrue(viewModel.uiState.value.canManageProfiles) + } + + @Test + fun `revoking the grant on reload leaves manage mode and closes the delete dialog`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + var currentUser: User? = user("admin") + val profile = Profile(id = "p1", name = "One") + val viewModel = TvProfileSelectionViewModel( + profileRepository = FixedProfileRepository(listOf(profile)), + currentUserProvider = { currentUser }, + ) + advanceUntilIdle() + viewModel.toggleManageMode() + viewModel.requestDelete(profile) + assertTrue(viewModel.uiState.value.isManageMode) + assertNotNull(viewModel.uiState.value.deleteCandidate) + + currentUser = null + viewModel.loadProfiles() + advanceUntilIdle() + + // Otherwise the "Done" pill disappears while its mode stays on, + // and the pending delete confirmation stays open and actionable. + assertFalse(viewModel.uiState.value.canManageProfiles) + assertFalse(viewModel.uiState.value.isManageMode) + assertNull(viewModel.uiState.value.deleteCandidate) + } +} + +private class FixedProfileRepository( + private val profiles: List, + private val activeProfileId: String? = null, +) : ProfileRepository( + profileApi = ProfileApi(HttpClient(MockEngine { respond("{}") })), + tokenManager = TokenManagerImpl(), +) { + override suspend fun listProfiles(): ApiResult> = ApiResult.Success(profiles) + + override suspend fun getActiveProfileId(): String? = activeProfileId +} 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 3629bfef1..7f65d0ec2 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 @@ -29,3 +29,32 @@ const val ADMIN_ROLE = "admin" */ fun isActingAdmin(user: User?, profile: Profile?): Boolean = user?.role == ADMIN_ROLE && profile?.isPrimary == true + +/** + * Client mirror of the server's profile-management gate as seen from the + * profile picker ("Who's watching?"). + * + * The server authorizes `POST /profiles`, `PUT /profiles/{id}` and + * `DELETE /profiles/{id}` when the caller's ACTIVE profile is the household + * primary, OR the account role is admin — both arms are mirrored here. + * Which arm can hold at the picker depends on how the app got there: + * + * - Phone keeps the acting profile across "Switch Profile", so a non-admin + * household owner acting as the primary profile is fully authorized at + * the picker ([activeProfile] is the primary). + * - TV clears the active profile before showing the picker (and at first + * login nothing is selected on either platform), so [activeProfile] is + * null there and only the admin arm can hold. A management call from any + * other account with no acting profile can only end in a 403 ("Profile + * management requires the primary profile or admin access"). + * + * Creation has one exemption this predicate deliberately does not cover: + * bootstrap of the very FIRST profile (the server allows `POST /profiles` + * when the account has none). Call sites keep the add affordance visible for + * an empty profile list for that reason. + * + * Fails CLOSED when neither input resolves, like [isActingAdmin] — call + * sites must re-evaluate once they do (the pickers reload on every resume). + */ +fun canManageProfilesFromPicker(user: User?, activeProfile: Profile?): Boolean = + user?.role == ADMIN_ROLE || activeProfile?.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 c28e1697b..d08734e3b 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 @@ -59,4 +59,40 @@ class AdminPermissionsTest { val p = Profile(id = "p", name = "Kid") assertFalse(p.isPrimary) } + + @Test + fun `admin account can manage profiles from the picker without an acting profile`() { + assertTrue(canManageProfilesFromPicker(user("admin"), activeProfile = null)) + } + + /** + * The phone keeps the acting profile across "Switch Profile", so a + * non-admin household owner acting as the primary is authorized by the + * server's acting-profile arm — hiding management from them would remove + * their only route to profile management. + */ + @Test + fun `non-admin acting as the primary profile can manage profiles from the picker`() { + assertTrue(canManageProfilesFromPicker(user("user"), profile(isPrimary = true))) + } + + @Test + fun `non-admin acting as a non-primary profile cannot manage profiles from the picker`() { + assertFalse(canManageProfilesFromPicker(user("user"), profile(isPrimary = false))) + } + + /** + * The reported bug: the picker offered create/edit/delete to every + * account, but with no acting profile the server only authorizes an + * admin — everyone else got a 403 after filling in the form. + */ + @Test + fun `non-admin with no acting profile cannot manage profiles from the picker`() { + assertFalse(canManageProfilesFromPicker(user("user"), activeProfile = null)) + } + + @Test + fun `nothing resolved cannot manage profiles from the picker`() { + assertFalse(canManageProfilesFromPicker(null, null)) + } }