Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e3a86fff authored by Abhishek Aggarwal's avatar Abhishek Aggarwal
Browse files

fix(auth): keep microG token refresh from being lost as stale write

parent 9641eb67
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -139,10 +139,13 @@ class MicrogLoginManager @Inject constructor(

    private suspend fun fetchRefreshedToken(oldToken: String): String {
        val accountName = playStoreAuthStore.awaitEmail()
        invalidateAuthToken(accountName, oldToken)
        // Fetch the replacement token before invalidating the old one. If the fetch
        // fails (e.g. RequiresUserAction), we keep the old token intact so the user
        // can retry without first being booted into a re-auth flow.
        val result = fetchAccount(accountName)
        return when (result) {
            is MicrogAccountFetchResult.Success -> {
                invalidateAuthToken(accountName, oldToken)
                playStoreAuthStore.saveGoogleLogin(
                    result.email,
                    result.oauthToken,
+7 −6
Original line number Diff line number Diff line
@@ -66,9 +66,6 @@ class PlayStoreAuthPersistenceGuard @Inject constructor(
            loginIntent = sessionRepository.awaitLoginIntent(),
            sourceSelection = sourceSelectionRepository.currentSourceSelection(),
            authDataFingerprint = playStoreAuthStore.awaitAuthData().fingerprint(),
            email = playStoreAuthStore.awaitEmail(),
            oauthToken = playStoreAuthStore.awaitOauthToken(),
            aasToken = playStoreAuthStore.awaitAasToken(),
            authSource = playStoreAuthStore.awaitPlayStoreAuthSource(),
        )
    }
@@ -86,13 +83,17 @@ class PlayStoreAuthPersistenceGuard @Inject constructor(
        ).joinToString("|")
    }

    /**
     * Identity snapshot taken before a Play login flow. Compared on persist to detect
     * concurrent identity changes (logout, source/login-mode switch, competing login that
     * already wrote AuthData). Excludes email/oauthToken/aasToken: those are credential
     * fields the login itself rewrites mid-flight (e.g. microG token refresh), and including
     * them caused legitimate refreshes to be misclassified as stale and discarded.
     */
    data class Context internal constructor(
        val loginIntent: PersistedLoginIntent,
        val sourceSelection: SourceSelection,
        val authDataFingerprint: String?,
        val email: String,
        val oauthToken: String,
        val aasToken: String,
        val authSource: PlayStoreAuthSource?,
    )
}
+88 −0
Original line number Diff line number Diff line
@@ -242,6 +242,94 @@ class MicrogLoginManagerTest {
        org.mockito.kotlin.verify(accountManager).invalidateAuthToken(account.type, "ya29.old")
        org.mockito.kotlin.verify(playStoreAuthStore).saveGoogleLogin(account.name, "token123")
        org.mockito.kotlin.verify(playStoreAuthStore).saveAasToken("")

        // The replacement token must be fetched before the old one is invalidated, so a
        // failed refresh leaves the user with their existing credentials intact.
        val order = org.mockito.kotlin.inOrder(accountManager)
        order.verify(accountManager).getAuthToken(
            eq(account),
            eq(MicrogCertUtil.PLAY_AUTH_SCOPE),
            any<Bundle>(),
            eq(false),
            isNull(),
            isNull(),
        )
        order.verify(accountManager).invalidateAuthToken(account.type, "ya29.old")
    }

    @Test
    fun `login does not invalidate old token when refresh requires user action`() = runBlocking {
        val account = Account("user@gmail.com", MicrogCertUtil.GOOGLE_ACCOUNT_TYPE)
        val accountManager = mock<AccountManager>()
        whenever(accountManager.getAccountsByType(eq(MicrogCertUtil.GOOGLE_ACCOUNT_TYPE)))
            .thenReturn(arrayOf(account))
        val intent = Intent("foundation.e.apps.ACTION_LOGIN")
        val resultBundle = Bundle().apply {
            putParcelable(AccountManager.KEY_INTENT, intent)
            classLoader = Intent::class.java.classLoader
        }
        whenever(
            accountManager.getAuthToken(
                eq(account),
                eq(MicrogCertUtil.PLAY_AUTH_SCOPE),
                any<Bundle>(),
                eq(false),
                isNull(),
                isNull()
            )
        ).thenReturn(ImmediateAccountManagerFuture(resultBundle))
        val playStoreAuthStore = mock<PlayStoreAuthStore>()
        whenever(playStoreAuthStore.awaitOauthToken()).thenReturn("ya29.old")
        whenever(playStoreAuthStore.awaitEmail()).thenReturn(account.name)

        assertThrows(IllegalStateException::class.java) {
            runBlocking {
                buildMicrogLoginManager(accountManager, playStoreAuthStore = playStoreAuthStore)
                    .login()
            }
        }

        org.mockito.kotlin.verify(accountManager, org.mockito.kotlin.never())
            .invalidateAuthToken(any(), any())
        org.mockito.kotlin.verify(playStoreAuthStore, org.mockito.kotlin.never())
            .saveGoogleLogin(any(), any())
        org.mockito.kotlin.verify(playStoreAuthStore, org.mockito.kotlin.never())
            .saveAasToken(any())
    }

    @Test
    fun `login does not invalidate old token when token fetch errors`() = runBlocking {
        val account = Account("user@gmail.com", MicrogCertUtil.GOOGLE_ACCOUNT_TYPE)
        val accountManager = mock<AccountManager>()
        whenever(accountManager.getAccountsByType(eq(MicrogCertUtil.GOOGLE_ACCOUNT_TYPE)))
            .thenReturn(arrayOf(account))
        whenever(
            accountManager.getAuthToken(
                eq(account),
                eq(MicrogCertUtil.PLAY_AUTH_SCOPE),
                any<Bundle>(),
                eq(false),
                isNull(),
                isNull()
            )
        ).thenReturn(ThrowingAccountManagerFuture(IllegalStateException("boom")))
        val playStoreAuthStore = mock<PlayStoreAuthStore>()
        whenever(playStoreAuthStore.awaitOauthToken()).thenReturn("ya29.old")
        whenever(playStoreAuthStore.awaitEmail()).thenReturn(account.name)

        assertThrows(IllegalStateException::class.java) {
            runBlocking {
                buildMicrogLoginManager(accountManager, playStoreAuthStore = playStoreAuthStore)
                    .login()
            }
        }

        org.mockito.kotlin.verify(accountManager, org.mockito.kotlin.never())
            .invalidateAuthToken(any(), any())
        org.mockito.kotlin.verify(playStoreAuthStore, org.mockito.kotlin.never())
            .saveGoogleLogin(any(), any())
        org.mockito.kotlin.verify(playStoreAuthStore, org.mockito.kotlin.never())
            .saveAasToken(any())
    }

    @Test
+5 −3
Original line number Diff line number Diff line
@@ -254,9 +254,9 @@ class PlayStoreTokenRefreshHandlerTest {
    @Test
    fun `refreshPlayStoreToken skips persistence when account changes during refresh`() = runTest {
        val oldAuth = AuthData(email = "old@example.com", authToken = "stale-token")
        val competingAuth = AuthData(email = "competing@example.com", authToken = "competing-token")
        val freshAuth = AuthData(email = "old@example.com", authToken = "fresh-token")
        val authStore = InMemoryPlayStoreAuthStore(oldAuth)
        authStore.saveGoogleLogin("old@example.com", "old-oauth-token")
        val authDataCache = AuthDataCache(authStore, json)
        val playStoreAuthenticator = mockk<PlayStoreAuthenticator>()
        val playStoreStoredAuthPolicy = alwaysValidStoredAuthPolicy(authStore)
@@ -268,14 +268,16 @@ class PlayStoreTokenRefreshHandlerTest {
        )

        coEvery { playStoreAuthenticator.refreshLogin() } coAnswers {
            authStore.saveGoogleLogin("new@example.com", "new-oauth-token")
            // Simulate a competing flow that already saved fresh AuthData before
            // this refresh reached the persistence guard.
            authStore.saveAuthData(competingAuth)
            successfulRefresh(freshAuth)
        }

        val result = handler.refreshPlayStoreToken()

        assertThat(result).isInstanceOf(AuthResult.Failure::class.java)
        assertThat(authStore.awaitAuthData()).isEqualTo(oldAuth)
        assertThat(authStore.awaitAuthData()).isEqualTo(competingAuth)
        coVerify(exactly = 1) { playStoreAuthenticator.refreshLogin() }
    }

+5 −3
Original line number Diff line number Diff line
@@ -191,6 +191,7 @@ class AuthenticatorRepositoryTest {
    @Test
    fun fetchAuthResults_skipsPersistingAuthDataWhenAccountChangesBeforePersistence() = runTest {
        val authData = AuthData(email = "old@example.com")
        val competingAuthData = AuthData(email = "competing@example.com", authToken = "competing")
        val storeResult = StoreAuthResult(
            storeType = AuthStore.PLAY_STORE,
            result = ResultSupreme.Success(
@@ -203,13 +204,14 @@ class AuthenticatorRepositoryTest {
            currentLoginIntent = PersistedLoginIntent.PLAY_GOOGLE,
            persistedLoginIntent = PersistedLoginIntent.PLAY_GOOGLE,
        )
        inMemoryStore.saveGoogleLogin("old@example.com", "old-oauth-token")
        val playAuthenticator = mockk<StoreAuthenticator>()

        every { playAuthenticator.storeType } returns AuthStore.PLAY_STORE
        coEvery { playAuthenticator.isStoreActive() } returns true
        coEvery { playAuthenticator.login() } coAnswers {
            inMemoryStore.saveGoogleLogin("new@example.com", "new-oauth-token")
            // Simulate a competing login that wrote fresh credentials before this
            // result reached the persistence guard.
            inMemoryStore.saveAuthData(competingAuthData)
            storeResult
        }
        val repository = authenticatorRepository(
@@ -222,7 +224,7 @@ class AuthenticatorRepositoryTest {
        val result = repository.fetchAuthResults()

        assertThat(result.single().result).isInstanceOf(ResultSupreme.Error::class.java)
        assertThat(inMemoryStore.awaitAuthData()).isNull()
        assertThat(inMemoryStore.awaitAuthData()).isEqualTo(competingAuthData)
    }

    @Test