refactor(auth): harden session write ordering and startup state ownership (209efb9e) · Commits · e / os / App Lounge

app/src/main/java/foundation/e/apps/feature/auth/login/LoginWorkflowCoordinator.kt

+3 −1

Original line number	Diff line number	Diff line
		@@ -299,12 +299,14 @@ class LoginWorkflowCoordinator @Inject constructor(
		}

		private suspend fun rollbackFailedLogin(rollbackState: LoginRollbackState) {
		restoreStoreSelection(rollbackState)
		// Restore credential-bearing auth state before source selection so a partial rollback
		// cannot publish the old catalog mix ahead of the old session state.
		playCredentialStateRepository.restoreCredentialState(rollbackState.playCredentialState)
		when (rollbackState.session) {
		AuthSession.Unauthenticated -> authSessionRepository.clearSession()
		else -> authSessionRepository.setSession(rollbackState.session)
		}
		restoreStoreSelection(rollbackState)
		sessionStateController.clearLoadedSessions()
		sessionStateController.refreshSessions()
		}

app/src/main/java/foundation/e/apps/feature/main/MainActivityStartupStateMachine.kt

+28 −0

Original line number	Diff line number	Diff line
		@@ -18,6 +18,7 @@

		package foundation.e.apps.feature.main

		import androidx.annotation.MainThread
		import foundation.e.apps.R
		import foundation.e.apps.domain.auth.AuthRefreshState
		import foundation.e.apps.domain.auth.AuthSession
		@@ -27,6 +28,7 @@ import foundation.e.apps.domain.startup.StartupDestination
		internal class MainActivityStartupStateMachine(
		private val resolveStartupDestinationUseCase: ResolveStartupDestinationUseCase,
		) {
		private var ownerThreadId: Long? = null
		private var latestTocAcceptance: Boolean? = null
		private var latestAuthRefreshState: AuthRefreshState = AuthRefreshState.Pending
		private var latestDestinationId: Int? = null
		@@ -35,6 +37,7 @@ internal class MainActivityStartupStateMachine(

		var gPlayLoginRequested: Boolean = false
		set(value) {
		verifySingleThreadAccess()
		field = value
		if (!value) {
		closeAfterLogin = false
		@@ -42,13 +45,21 @@ internal class MainActivityStartupStateMachine(
		}

		var closeAfterLogin: Boolean = false
		set(value) {
		verifySingleThreadAccess()
		field = value
		}

		@MainThread
		fun onTocAcceptanceChanged(isTocAccepted: Boolean): MainActivityStartupState {
		verifySingleThreadAccess()
		latestTocAcceptance = isTocAccepted
		return currentState()
		}

		@MainThread
		fun onAuthRefreshStateChanged(authRefreshState: AuthRefreshState): MainActivityStartupState {
		verifySingleThreadAccess()
		latestAuthRefreshState = authRefreshState
		val authRefreshSnapshot = (authRefreshState as? AuthRefreshState.Completed)?.snapshot

		@@ -82,12 +93,16 @@ internal class MainActivityStartupStateMachine(
		)
		}

		@MainThread
		fun onDestinationChanged(destinationId: Int?): MainActivityStartupState {
		verifySingleThreadAccess()
		latestDestinationId = destinationId
		return currentState()
		}

		@MainThread
		fun onLoginSubmissionChanged(isSubmitting: Boolean): MainActivityStartupState {
		verifySingleThreadAccess()
		isLoginSubmitting = isSubmitting
		return currentState()
		}
		@@ -123,6 +138,19 @@ internal class MainActivityStartupStateMachine(
		startupRefreshRequestedForCurrentPendingState = true
		return true
		}

		private fun verifySingleThreadAccess() {
		val currentThreadId = Thread.currentThread().id
		val expectedThreadId = ownerThreadId
		if (expectedThreadId == null) {
		ownerThreadId = currentThreadId
		return
		}

		check(expectedThreadId == currentThreadId) {
		"MainActivityStartupStateMachine must be mutated from a single owner thread"
		}
		}
		}

		internal data class MainActivityStartupState(

app/src/test/java/foundation/e/apps/feature/auth/login/LoginWorkflowCoordinatorTest.kt

+9 −0

Original line number	Diff line number	Diff line
		@@ -54,6 +54,7 @@ import org.junit.Test
		import org.mockito.Mock
		import org.mockito.MockitoAnnotations
		import org.mockito.kotlin.any
		import org.mockito.kotlin.inOrder
		import org.mockito.kotlin.never
		import org.mockito.kotlin.verify
		import org.mockito.kotlin.whenever
		@@ -318,6 +319,14 @@ class LoginWorkflowCoordinatorTest {
		coordinator.submitGoogleLogin("user@gmail.com", "oauth-token")
		advanceUntilIdle()

		val rollbackOrder = inOrder(
		playCredentialStateRepository,
		authSessionRepository,
		sourceSelectionRepository,
		)
		rollbackOrder.verify(playCredentialStateRepository).restoreCredentialState(playCredentialState)
		rollbackOrder.verify(authSessionRepository).setSession(AuthSession.OpenSourceSession)
		rollbackOrder.verify(sourceSelectionRepository).saveSourceSelection(sourceSelection)
		verify(sourceSelectionRepository).saveSourceSelection(sourceSelection)
		}

app/src/test/java/foundation/e/apps/feature/main/MainActivityStartupStateMachineTest.kt

+21 −0

Original line number	Diff line number	Diff line
		@@ -31,6 +31,7 @@ import foundation.e.apps.domain.auth.PlayStoreLoginMode
		import foundation.e.apps.domain.startup.ResolveStartupDestinationUseCase
		import foundation.e.apps.domain.startup.StartupDestination
		import org.junit.Test
		import java.util.concurrent.atomic.AtomicReference

		class MainActivityStartupStateMachineTest {

		@@ -171,4 +172,24 @@ class MainActivityStartupStateMachineTest {

		assertThat(state.shouldRefreshStartupSession).isTrue()
		}

		@Test
		fun `state machine rejects mutation from a different thread`() {
		stateMachine.onTocAcceptanceChanged(isTocAccepted = true)
		val failure = AtomicReference<IllegalStateException?>()
		val secondThread = Thread {
		try {
		stateMachine.onDestinationChanged(R.id.startupFragment)
		} catch (exception: IllegalStateException) {
		failure.set(exception)
		}
		}

		secondThread.start()
		secondThread.join()

		assertThat(failure.get()).isNotNull()
		assertThat(failure.get()?.message)
		.isEqualTo("MainActivityStartupStateMachine must be mutated from a single owner thread")
		}
		}

data/src/main/java/foundation/e/apps/data/login/repository/AuthSessionRepositoryImpl.kt

+31 −12

Original line number	Diff line number	Diff line
		@@ -66,24 +66,30 @@ class AuthSessionRepositoryImpl @Inject constructor(
		when (session) {
		is AuthSession.PlayStoreSession -> {
		when (session.loginMode) {
		PlayStoreLoginMode.ANONYMOUS -> {
		sessionRepository.saveLoginIntent(PersistedLoginIntent.PLAY_ANONYMOUS)
		playStoreAuthStore.savePlayStoreAuthSource(null)
		}
		PlayStoreLoginMode.ANONYMOUS ->
		persistPlayStoreSession(
		loginIntent = PersistedLoginIntent.PLAY_ANONYMOUS,
		authSource = null,
		)

		PlayStoreLoginMode.GOOGLE -> {
		sessionRepository.saveLoginIntent(PersistedLoginIntent.PLAY_GOOGLE)
		playStoreAuthStore.savePlayStoreAuthSource(PlayStoreAuthSource.GOOGLE)
		}
		PlayStoreLoginMode.GOOGLE ->
		persistPlayStoreSession(
		loginIntent = PersistedLoginIntent.PLAY_GOOGLE,
		authSource = PlayStoreAuthSource.GOOGLE,
		)

		PlayStoreLoginMode.MICROG -> {
		sessionRepository.saveLoginIntent(PersistedLoginIntent.PLAY_MICROG)
		playStoreAuthStore.savePlayStoreAuthSource(PlayStoreAuthSource.MICROG)
		}
		PlayStoreLoginMode.MICROG ->
		persistPlayStoreSession(
		loginIntent = PersistedLoginIntent.PLAY_MICROG,
		authSource = PlayStoreAuthSource.MICROG,
		)
		}
		}

		AuthSession.OpenSourceSession -> {
		// Crash-window note: credential destruction and loginIntent persistence still span
		// separate DataStore edits. We keep credential eviction first here to avoid leaving
		// a usable Play credential set behind once open-source mode is committed.
		playStoreAuthStore.destroyCredentials()
		sessionRepository.saveLoginIntent(PersistedLoginIntent.OPEN_SOURCE)
		}
		@@ -93,6 +99,8 @@ class AuthSessionRepositoryImpl @Inject constructor(
		}

		override suspend fun clearSession() {
		// Crash-window note: logout intentionally clears credentials before publishing NONE so a
		// completed write sequence never leaves stale Play credentials behind for later reuse.
		playStoreAuthStore.destroyCredentials()
		sessionRepository.saveLoginIntent(PersistedLoginIntent.NONE)
		}
		@@ -143,4 +151,15 @@ class AuthSessionRepositoryImpl @Inject constructor(
		}
		}
		}

		private suspend fun persistPlayStoreSession(
		loginIntent: PersistedLoginIntent,
		authSource: PlayStoreAuthSource?,
		) {
		// loginIntent is the authoritative published session signal. Persist the supporting Play
		// source first so a crash window cannot expose a new Play login intent without matching
		// source metadata.
		playStoreAuthStore.savePlayStoreAuthSource(authSource)
		sessionRepository.saveLoginIntent(loginIntent)
		}
		}