From 1a9cbfdf9ea7897cc212aaf27bb638b59bbb6381 Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Fri, 13 Mar 2026 10:51:13 +0100 Subject: [PATCH] fix: fix false positive dialog in app details Remove InvalidAuthEvent. Re-auth should be rather lower level and for Play Store it is now done per-request in PlayStoreRepository with refreshPlayStoreAuthentication() --- .../foundation/e/apps/data/event/AppEvent.kt | 2 - .../data/login/core/AuthFailureFactory.kt | 41 --------- .../e/apps/data/login/core/AuthObject.kt | 17 +--- .../data/playstore/PlayStoreRepository.kt | 4 +- .../data/playstore/utils/GPlayHttpClient.kt | 7 -- .../foundation/e/apps/ui/LoginViewModel.kt | 26 ------ .../java/foundation/e/apps/ui/MainActivity.kt | 25 ------ .../ui/application/ApplicationViewModel.kt | 12 +-- .../data/playstore/PlayStoreRepositoryTest.kt | 30 ++++++- .../e/apps/gplay/GPlayHttpClientTest.kt | 8 +- .../e/apps/login/LoginViewModelTest.kt | 21 ----- .../application/ApplicationViewModelTest.kt | 86 +++++++++++++++++++ 12 files changed, 125 insertions(+), 154 deletions(-) delete mode 100644 app/src/main/java/foundation/e/apps/data/login/core/AuthFailureFactory.kt create mode 100644 app/src/test/java/foundation/e/apps/ui/application/ApplicationViewModelTest.kt diff --git a/app/src/main/java/foundation/e/apps/data/event/AppEvent.kt b/app/src/main/java/foundation/e/apps/data/event/AppEvent.kt index b12a12ea8..e7f90536c 100644 --- a/app/src/main/java/foundation/e/apps/data/event/AppEvent.kt +++ b/app/src/main/java/foundation/e/apps/data/event/AppEvent.kt @@ -29,8 +29,6 @@ import kotlinx.coroutines.CompletableDeferred sealed class AppEvent(val data: Any) { class SignatureMissMatchError(packageName: String) : AppEvent(packageName) class UpdateEvent(result: ResultSupreme.WorkError) : AppEvent(result) - - class InvalidAuthEvent(authName: String) : AppEvent(authName) class ErrorMessageEvent(stringResourceId: Int) : AppEvent(stringResourceId) class ErrorMessageDialogEvent(stringResourceId: Int) : AppEvent(stringResourceId) class AppPurchaseEvent(appInstall: AppInstall) : AppEvent(appInstall) diff --git a/app/src/main/java/foundation/e/apps/data/login/core/AuthFailureFactory.kt b/app/src/main/java/foundation/e/apps/data/login/core/AuthFailureFactory.kt deleted file mode 100644 index 70eea68d7..000000000 --- a/app/src/main/java/foundation/e/apps/data/login/core/AuthFailureFactory.kt +++ /dev/null @@ -1,41 +0,0 @@ -package foundation.e.apps.data.login.core - -import com.aurora.gplayapi.data.models.AuthData -import foundation.e.apps.data.ResultSupreme -import foundation.e.apps.data.login.exceptions.CleanApkException -import foundation.e.apps.data.login.exceptions.GPlayValidationException -import foundation.e.apps.domain.model.User -import java.net.HttpURLConnection - -object AuthFailureFactory { - fun createGplayValidationFailure(user: User, otherPayload: Any?): AuthObject { - val message = - "Validating AuthData failed.\nNetwork code: ${HttpURLConnection.HTTP_UNAUTHORIZED}" - return AuthObject.GPlayAuth( - ResultSupreme.Error( - message = message, - exception = GPlayValidationException( - message, - user, - HttpURLConnection.HTTP_UNAUTHORIZED, - ) - ).apply { - this.otherPayload = otherPayload - }, - user, - ) - } - - fun createCleanApkUnauthorizedFailure(user: User): AuthObject { - return AuthObject.CleanApk( - ResultSupreme.Error( - message = "Unauthorized", - exception = CleanApkException( - isTimeout = false, - message = "Unauthorized", - ) - ), - user, - ) - } -} diff --git a/app/src/main/java/foundation/e/apps/data/login/core/AuthObject.kt b/app/src/main/java/foundation/e/apps/data/login/core/AuthObject.kt index 3c89fa0d8..2e77aff60 100644 --- a/app/src/main/java/foundation/e/apps/data/login/core/AuthObject.kt +++ b/app/src/main/java/foundation/e/apps/data/login/core/AuthObject.kt @@ -36,21 +36,8 @@ sealed class AuthObject { abstract val result: ResultSupreme<*> abstract val user: User - abstract fun createInvalidAuthObject(): AuthObject + class GPlayAuth(override val result: ResultSupreme, override val user: User) : AuthObject() - class GPlayAuth(override val result: ResultSupreme, override val user: User) : AuthObject() { - override fun createInvalidAuthObject(): AuthObject { - return AuthFailureFactory.createGplayValidationFailure( - user = user, - otherPayload = result.otherPayload - ) - } - } - - class CleanApk(override val result: ResultSupreme, override val user: User) : AuthObject() { - override fun createInvalidAuthObject(): AuthObject { - return AuthFailureFactory.createCleanApkUnauthorizedFailure(user) - } - } + class CleanApk(override val result: ResultSupreme, override val user: User) : AuthObject() // Add more auth types here } diff --git a/app/src/main/java/foundation/e/apps/data/playstore/PlayStoreRepository.kt b/app/src/main/java/foundation/e/apps/data/playstore/PlayStoreRepository.kt index 7f2a6579f..933099af5 100644 --- a/app/src/main/java/foundation/e/apps/data/playstore/PlayStoreRepository.kt +++ b/app/src/main/java/foundation/e/apps/data/playstore/PlayStoreRepository.kt @@ -172,7 +172,7 @@ class PlayStoreRepository @Inject constructor( getAppDetailsHelper().getAppByPackageName(packageNames) } - if (!isEmulator() && appDetails.all { it.versionCode == 0L } && isAnonymousUser()) { + if (!isEmulator() && appDetails.all { it.versionCode == 0L }) { // Google Play returns limited result ( i.e. version code being 0) with a stale token, // so we need to refresh authentication to get a new token. Timber.i("Version code is 0.") @@ -205,7 +205,7 @@ class PlayStoreRepository @Inject constructor( throw exception } - if (!isEmulator() && appDetails.versionCode == 0L && isAnonymousUser()) { + if (!isEmulator() && appDetails.versionCode == 0L) { // Google Play returns limited result ( i.e. version code being 0) with a stale token, // so we need to refresh authentication to get a new token. Timber.i("Version code is 0 for ${appDetails.packageName}.") diff --git a/app/src/main/java/foundation/e/apps/data/playstore/utils/GPlayHttpClient.kt b/app/src/main/java/foundation/e/apps/data/playstore/utils/GPlayHttpClient.kt index 8cc187ea3..8f689017c 100644 --- a/app/src/main/java/foundation/e/apps/data/playstore/utils/GPlayHttpClient.kt +++ b/app/src/main/java/foundation/e/apps/data/playstore/utils/GPlayHttpClient.kt @@ -23,7 +23,6 @@ import com.aurora.gplayapi.data.models.PlayResponse import com.aurora.gplayapi.network.IHttpClient import foundation.e.apps.data.event.AppEvent import foundation.e.apps.data.event.EventBus -import foundation.e.apps.data.login.core.AuthObject import foundation.e.apps.data.system.SystemInfoProvider import kotlinx.coroutines.MainScope import kotlinx.coroutines.flow.MutableStateFlow @@ -202,12 +201,6 @@ class GPlayHttpClient @Inject constructor( val responseBytes = response.body?.bytes() ?: byteArrayOf() when (code) { - STATUS_CODE_UNAUTHORIZED -> MainScope().launch { - EventBus.invokeEvent( - AppEvent.InvalidAuthEvent(AuthObject.GPlayAuth::class.java.simpleName) - ) - } - STATUS_CODE_TOO_MANY_REQUESTS -> MainScope().launch { if (url.toString().contains(SEARCH_SUGGEST)) { return@launch diff --git a/app/src/main/java/foundation/e/apps/ui/LoginViewModel.kt b/app/src/main/java/foundation/e/apps/ui/LoginViewModel.kt index 8ed768eb5..b3b506c94 100644 --- a/app/src/main/java/foundation/e/apps/ui/LoginViewModel.kt +++ b/app/src/main/java/foundation/e/apps/ui/LoginViewModel.kt @@ -36,9 +36,7 @@ import foundation.e.apps.domain.login.InitialGoogleLoginUseCase import foundation.e.apps.domain.login.InitialMicrogLoginUseCase import foundation.e.apps.domain.login.InitialNoGoogleLoginUseCase import foundation.e.apps.domain.login.LogoutUseCase -import foundation.e.apps.ui.parentFragment.LoadingViewModel import kotlinx.coroutines.launch -import okhttp3.Cache import javax.inject.Inject /** @@ -56,7 +54,6 @@ class LoginViewModel @Inject constructor( private val initialGoogleLoginUseCase: InitialGoogleLoginUseCase, private val initialNoGoogleLoginUseCase: InitialNoGoogleLoginUseCase, private val logoutUseCase: LogoutUseCase, - private val cache: Cache, @ApplicationContext private val context: Context ) : ViewModel() { @@ -157,29 +154,6 @@ class LoginViewModel @Inject constructor( } } - /** - * Once an AuthObject is marked as invalid, it will be refreshed - * automatically by LoadingViewModel. - * If GPlay auth is invalid, [LoadingViewModel.onLoadData] has a retry block, - * this block will clear existing GPlay AuthData and freshly start the login flow. - */ - fun markInvalidAuthObject(authObjectName: String) { - val authObjectsLocal = authObjects.value?.toMutableList() - val invalidObject = authObjectsLocal?.find { it::class.java.simpleName == authObjectName } - - val replacedObject = invalidObject?.createInvalidAuthObject() - - authObjectsLocal?.apply { - if (invalidObject != null && replacedObject != null) { - remove(invalidObject) - add(replacedObject) - } - } - - authObjects.postValue(authObjectsLocal) - cache.evictAll() - } - /** * Clears all saved data and logs out the user to the sign in screen. */ diff --git a/app/src/main/java/foundation/e/apps/ui/MainActivity.kt b/app/src/main/java/foundation/e/apps/ui/MainActivity.kt index b34791151..c8dfe64be 100644 --- a/app/src/main/java/foundation/e/apps/ui/MainActivity.kt +++ b/app/src/main/java/foundation/e/apps/ui/MainActivity.kt @@ -23,7 +23,6 @@ import android.os.Build.VERSION import android.os.Build.VERSION_CODES import android.os.Bundle import android.view.View -import android.widget.Toast import android.window.OnBackInvokedDispatcher.PRIORITY_DEFAULT import androidx.activity.OnBackPressedCallback import androidx.activity.result.contract.ActivityResultContracts @@ -222,10 +221,6 @@ class MainActivity : AppCompatActivity() { private fun observeEvents() { lifecycleScope.launch { repeatOnLifecycle(Lifecycle.State.STARTED) { - launch { - observeInvalidAuth() - } - launch { observeAppUnavailable() } @@ -481,19 +476,6 @@ class MainActivity : AppCompatActivity() { } } - private suspend fun observeInvalidAuth() { - EventBus.events.filter { appEvent -> - appEvent is AppEvent.InvalidAuthEvent - }.distinctUntilChanged { old, new -> - ((old.data is String) && (new.data is String) && old.data == new.data) - }.collectLatest { - if (BuildConfig.DEBUG) { - Toast.makeText(this, "Refreshing token...", Toast.LENGTH_SHORT).show() - } - validatedAuthObject(it) - } - } - private suspend fun observeAppUnavailable() { EventBus.events.filterIsInstance() .collectLatest { event -> @@ -503,13 +485,6 @@ class MainActivity : AppCompatActivity() { } } - private fun validatedAuthObject(appEvent: AppEvent) { - val data = appEvent.data as String - if (data.isNotBlank()) { - loginViewModel.markInvalidAuthObject(data) - } - } - private suspend fun observeTooManyRequests() { EventBus.events.filter { appEvent -> appEvent is AppEvent.TooManyRequests diff --git a/app/src/main/java/foundation/e/apps/ui/application/ApplicationViewModel.kt b/app/src/main/java/foundation/e/apps/ui/application/ApplicationViewModel.kt index d656f87c2..9e0013e7a 100644 --- a/app/src/main/java/foundation/e/apps/ui/application/ApplicationViewModel.kt +++ b/app/src/main/java/foundation/e/apps/ui/application/ApplicationViewModel.kt @@ -146,14 +146,14 @@ class ApplicationViewModel @Inject constructor( app.isPurchased = isPurchased applicationLiveData.postValue(result) - updateShareVisibilityState(app.shareUri.toString()) - updateAppContentRatingState(packageName, app.contentRating) - if (status != ResultStatus.OK) { - EventBus.invokeEvent( - AppEvent.InvalidAuthEvent(AuthObject.GPlayAuth::class.java.simpleName) - ) + _errorMessageLiveData.postValue(R.string.data_load_error) + Timber.w("error loading app. status=$status") + return@launch } + + updateShareVisibilityState(app.shareUri.toString()) + updateAppContentRatingState(packageName, app.contentRating) } catch (e: InternalException.AppNotFound) { _errorMessageLiveData.postValue(R.string.app_not_found) scheduleAutoRedirect() diff --git a/app/src/test/java/foundation/e/apps/data/playstore/PlayStoreRepositoryTest.kt b/app/src/test/java/foundation/e/apps/data/playstore/PlayStoreRepositoryTest.kt index 9450b82dc..06b0e63c2 100644 --- a/app/src/test/java/foundation/e/apps/data/playstore/PlayStoreRepositoryTest.kt +++ b/app/src/test/java/foundation/e/apps/data/playstore/PlayStoreRepositoryTest.kt @@ -103,8 +103,8 @@ class PlayStoreRepositoryTest { } @Test - fun `getAppDetails refreshes auth when anonymous and version code is zero`() = runTest { - val authData = AuthData(email = "anon@example.com", isAnonymous = true) + fun `getAppDetails refreshes auth when version code is zero`() = runTest { + val authData = AuthData(email = "user@gmail.com", isAnonymous = false) val staleApp = App(packageName = "pkg.test", versionCode = 0) val refreshedApp = App(packageName = "pkg.test", versionCode = 4) val authenticatorRepository = mock() @@ -146,8 +146,8 @@ class PlayStoreRepositoryTest { } @Test - fun `getAppsDetails refreshes auth when anonymous and all versions zero`() = runTest { - val authData = AuthData(email = "anon@example.com", isAnonymous = true) + fun `getAppsDetails refreshes auth when all versions zero`() = runTest { + val authData = AuthData(email = "user@gmail.com", isAnonymous = false) val staleApps = listOf(App(packageName = "pkg.test", versionCode = 0)) val refreshedApps = listOf(App(packageName = "pkg.test", versionCode = 9)) val authenticatorRepository = mock() @@ -171,6 +171,28 @@ class PlayStoreRepositoryTest { } } + @Test + fun `getAppDetails rethrows unauthorized when auth refresh fails`() = runTest { + val authData = AuthData(email = "user@gmail.com") + val authenticatorRepository = mock() + + repository = createRepository(authenticatorRepository) + + whenever(authenticatorRepository.getGPlayAuthOrThrow()).thenReturn(authData) + whenever(authenticatorRepository.getValidatedAuthData()).thenReturn(ResultSupreme.Error("refresh failed")) + every { + anyConstructed().getAppByPackageName("pkg.test") + } throws GplayHttpRequestException(401, "unauthorized") + + val exception = kotlin.test.assertFailsWith { + repository.getAppDetails("pkg.test") + } + + assertThat(exception.status).isEqualTo(401) + mockitoVerify(authenticatorRepository).getValidatedAuthData() + verify(exactly = 1) { anyConstructed().getAppByPackageName("pkg.test") } + } + @Test fun `getAppsDetails filters invalid apps`() = runTest { val authData = AuthData(email = "user@gmail.com") diff --git a/app/src/test/java/foundation/e/apps/gplay/GPlayHttpClientTest.kt b/app/src/test/java/foundation/e/apps/gplay/GPlayHttpClientTest.kt index a846fe545..2a35976f1 100644 --- a/app/src/test/java/foundation/e/apps/gplay/GPlayHttpClientTest.kt +++ b/app/src/test/java/foundation/e/apps/gplay/GPlayHttpClientTest.kt @@ -19,7 +19,6 @@ package foundation.e.apps.gplay import com.aurora.gplayapi.data.models.PlayResponse -import foundation.e.apps.data.login.core.AuthObject import foundation.e.apps.data.playstore.utils.GPlayHttpClient import foundation.e.apps.data.playstore.utils.GplayHttpRequestException import foundation.e.apps.util.FakeCall @@ -32,6 +31,7 @@ import io.mockk.mockkObject import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeoutOrNull import okhttp3.OkHttpClient import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.logging.HttpLoggingInterceptor @@ -170,9 +170,7 @@ class GPlayHttpClientTest { private suspend fun assertResponse(response: PlayResponse, statusValue: Int = 401) { assertFalse(response.isSuccessful) assertTrue(response.code == statusValue) - val event = EventBus.events.first() - assertTrue(event is AppEvent.InvalidAuthEvent) - assertTrue(event.data is String) - assertTrue(event.data == AuthObject.GPlayAuth::class.java.simpleName) + val event = withTimeoutOrNull(100) { EventBus.events.first() } + assertTrue(event == null) } } diff --git a/app/src/test/java/foundation/e/apps/login/LoginViewModelTest.kt b/app/src/test/java/foundation/e/apps/login/LoginViewModelTest.kt index d5d338eca..82c0a7925 100644 --- a/app/src/test/java/foundation/e/apps/login/LoginViewModelTest.kt +++ b/app/src/test/java/foundation/e/apps/login/LoginViewModelTest.kt @@ -44,7 +44,6 @@ import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain -import okhttp3.Cache import org.junit.After import org.junit.Before import org.junit.Rule @@ -76,8 +75,6 @@ class LoginViewModelTest { @Mock private lateinit var logoutUseCase: LogoutUseCase @Mock - private lateinit var cache: Cache - @Mock private lateinit var context: Context private lateinit var loginViewModel: LoginViewModel @@ -99,7 +96,6 @@ class LoginViewModelTest { initialGoogleLoginUseCase, initialNoGoogleLoginUseCase, logoutUseCase, - cache, context ) whenever(context.getString(R.string.sign_in_microg_login_failed)).thenReturn("fallback") @@ -110,23 +106,6 @@ class LoginViewModelTest { Dispatchers.resetMain() } - @Test - fun testMarkInvalidAuthObject() { - val authObjectList = mutableListOf( - AuthObject.GPlayAuth( - ResultSupreme.Success(AuthData("aa@aa.com", "feri4234")), User.GOOGLE - ) - ) - loginViewModel.authObjects.value = authObjectList - - loginViewModel.markInvalidAuthObject(AuthObject.GPlayAuth::class.java.simpleName) - val currentAuthObjectList = loginViewModel.authObjects.value as List - val invalidGplayAuth = currentAuthObjectList.find { it is AuthObject.GPlayAuth } - - assert(invalidGplayAuth != null) - assert((invalidGplayAuth as AuthObject.GPlayAuth).result.isUnknownError()) - } - @Test fun `initialMicrogLogin saves user and starts flow on success`() = runTest { val account = Account("user@gmail.com", "com.google") diff --git a/app/src/test/java/foundation/e/apps/ui/application/ApplicationViewModelTest.kt b/app/src/test/java/foundation/e/apps/ui/application/ApplicationViewModelTest.kt new file mode 100644 index 000000000..d7f907d61 --- /dev/null +++ b/app/src/test/java/foundation/e/apps/ui/application/ApplicationViewModelTest.kt @@ -0,0 +1,86 @@ +package foundation.e.apps.ui.application + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import com.aurora.gplayapi.data.models.AuthData +import foundation.e.apps.data.ResultSupreme +import foundation.e.apps.data.application.ApplicationRepository +import foundation.e.apps.data.application.data.Application +import foundation.e.apps.data.enums.ResultStatus +import foundation.e.apps.data.enums.Source +import foundation.e.apps.data.event.EventBus +import foundation.e.apps.data.install.AppManagerWrapper +import foundation.e.apps.data.install.download.data.DownloadProgressLD +import foundation.e.apps.data.login.core.AuthObject +import foundation.e.apps.data.parentalcontrol.fdroid.FDroidAntiFeatureRepository +import foundation.e.apps.data.playstore.PlayStoreRepository +import foundation.e.apps.domain.model.User +import foundation.e.apps.util.MainCoroutineRule +import foundation.e.apps.util.getOrAwaitValue +import io.mockk.coEvery +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.Assert.assertNull +import org.junit.Rule +import org.junit.Test +import kotlin.test.assertEquals + +@OptIn(ExperimentalCoroutinesApi::class) +class ApplicationViewModelTest { + + @get:Rule + val instantTaskExecutorRule = InstantTaskExecutorRule() + + @get:Rule + val mainCoroutineRule = MainCoroutineRule() + + private val downloadProgressLD = mockk(relaxed = true) + private val applicationRepository = mockk() + private val playStoreRepository = mockk() + private val appManagerWrapper = mockk(relaxed = true) + private val fDroidAntiFeatureRepository = mockk(relaxed = true) + + private val viewModel = ApplicationViewModel( + downloadProgressLD, + applicationRepository, + playStoreRepository, + appManagerWrapper, + fDroidAntiFeatureRepository + ) + + @Test + fun `loadData does not emit invalid auth event when details load is non auth failure`() = runTest { + val application = Application() + val authObject = AuthObject.GPlayAuth( + ResultSupreme.Success(AuthData("user@gmail.com", "token")), + User.GOOGLE + ) + + coEvery { + applicationRepository.getApplicationDetails("app-id", "pkg.test", Source.PLAY_STORE) + } returns Pair(application, ResultStatus.UNKNOWN) + + val eventDeferred = async { + withTimeoutOrNull(100) { EventBus.events.first() } + } + + val result = viewModel.applicationLiveData.getOrAwaitValue(time = 1) { + viewModel.loadData( + ApplicationLoadingParams( + appId = "app-id", + packageName = "pkg.test", + source = Source.PLAY_STORE, + isFdroidDeepLink = false, + authObjectList = listOf(authObject), + isPurchased = false + ) + ) { false } + } + + assertEquals(ResultStatus.UNKNOWN, result.second) + assertNull(eventDeferred.await()) + } +} -- GitLab