Fix crash for uncaught exceptions when refresh token request fails (#…

…2772) * Fix crash for uncaught exceptions on refreshing token * Update android/quest/src/main/java/org/smartregister/fhircore/quest/ui/login/AccountAuthenticator.kt --------- Co-authored-by: Peter Lubell-Doughtie <[email protected]>
opensrp · Sep 27, 2023 · b4b5429 · b4b5429
1 parent 0ea392b
commit b4b5429
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 46 deletions.
diff --git a/.../src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt b/.../src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt
@@ -67,40 +67,43 @@ constructor(
   private var isLoginPageRendered = false
 
   fun getAccessToken(): String {
-    val account = findAccount()
-    return if (account != null) {
-      val accessToken = accountManager.peekAuthToken(account, AUTH_TOKEN_TYPE) ?: ""
-      if (!isTokenActive(accessToken)) {
-        accountManager.run {
-          invalidateAuthToken(account.type, accessToken)
-          try {
-            getAuthToken(
-              account,
-              AUTH_TOKEN_TYPE,
-              bundleOf(),
-              true,
-              handleAccountManagerFutureCallback(account),
-              Handler(Looper.getMainLooper()) { message: Message ->
-                Timber.e(message.toString())
-                true
-              },
-            )
-          } catch (operationCanceledException: OperationCanceledException) {
-            Timber.e(operationCanceledException)
-          } catch (ioException: IOException) {
-            Timber.e(ioException)
-          } catch (authenticatorException: AuthenticatorException) {
-            Timber.e(authenticatorException)
-            // TODO: Should we cancel the sync job to avoid retries when offline?
-          }
-        }
-      } else {
-        isLoginPageRendered = false
+    val account = findAccount() ?: return ""
+    val accessToken = accountManager.peekAuthToken(account, AUTH_TOKEN_TYPE) ?: ""
+    if (isTokenActive(accessToken)) {
+      isLoginPageRendered = false
+      return accessToken
+    }
+
+    accountManager.invalidateAuthToken(account.type, accessToken)
+    val authTokenBundleFuture =
+      with(accountManager) {
+        getAuthToken(
+          account,
+          AUTH_TOKEN_TYPE,
+          bundleOf(),
+          true,
+          handleAccountManagerFutureCallback(account),
+          Handler(Looper.getMainLooper()) { message: Message ->
+            Timber.e(message.toString())
+            true
+          },
+        )
       }
-      accessToken
-    } else {
-      ""
+
+    try {
+      val authTokenBundle = authTokenBundleFuture.result
+      if (authTokenBundle.containsKey(AccountManager.KEY_AUTHTOKEN)) {
+        return authTokenBundle.getString(AccountManager.KEY_AUTHTOKEN)!! // refreshed token
+      }
+    } catch (operationCanceledException: OperationCanceledException) {
+      Timber.e(operationCanceledException)
+    } catch (ioException: IOException) {
+      Timber.e(ioException)
+    } catch (authenticatorException: AuthenticatorException) {
+      Timber.e(authenticatorException)
+      // TODO: Should we cancel the sync job to avoid retries when offline?
     }
+    return accessToken
   }
 
   private fun AccountManager.handleAccountManagerFutureCallback(account: Account?) =

diff --git a/...oid/engine/src/test/java/org/smartregister/fhircore/engine/auth/TokenAuthenticatorTest.kt b/...oid/engine/src/test/java/org/smartregister/fhircore/engine/auth/TokenAuthenticatorTest.kt
@@ -18,9 +18,11 @@ package org.smartregister.fhircore.engine.auth
 
 import android.accounts.Account
 import android.accounts.AccountManager
+import android.accounts.AccountManagerFuture
 import android.accounts.AuthenticatorException
 import android.accounts.OperationCanceledException
 import android.os.Bundle
+import androidx.core.os.bundleOf
 import androidx.test.core.app.ApplicationProvider
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
@@ -121,13 +123,15 @@ class TokenAuthenticatorTest : RobolectricTest() {
   fun testGetAccessTokenShouldInvalidateExpiredToken() {
     val account = Account(sampleUsername, PROVIDER)
     val accessToken = "gibberishaccesstoken"
+    val accountManagerFuture = mockk<AccountManagerFuture<Bundle>>()
     every { tokenAuthenticator.findAccount() } returns account
     every { tokenAuthenticator.isTokenActive(any()) } returns false
     every { accountManager.peekAuthToken(account, AUTH_TOKEN_TYPE) } returns accessToken
     every { accountManager.invalidateAuthToken(account.type, accessToken) } just runs
+    every { accountManagerFuture.result } returns bundleOf()
     every {
       accountManager.getAuthToken(account, AUTH_TOKEN_TYPE, any(), true, any(), any())
-    } returns mockk()
+    } returns accountManagerFuture
 
     tokenAuthenticator.getAccessToken()
 
@@ -146,22 +150,26 @@ class TokenAuthenticatorTest : RobolectricTest() {
   @Test
   fun testGetAccessTokenShouldCatchOperationCanceledAndIOAndAuthenticatorExceptions() {
     val account = Account(sampleUsername, PROVIDER)
+    val accountManagerFutureBundle = mockk<AccountManagerFuture<Bundle>>()
     every { tokenAuthenticator.findAccount() } returns account
     every { tokenAuthenticator.isTokenActive(any()) } returns false
     val accessToken = "gibberishaccesstoken"
     every { accountManager.peekAuthToken(account, AUTH_TOKEN_TYPE) } returns accessToken
     every { accountManager.invalidateAuthToken(account.type, accessToken) } just runs
+    every { accountManagerFutureBundle.result } throws OperationCanceledException()
     every {
       accountManager.getAuthToken(account, AUTH_TOKEN_TYPE, any<Bundle>(), true, any(), any())
-    } throws OperationCanceledException()
+    } returns accountManagerFutureBundle
     Assert.assertEquals(accessToken, tokenAuthenticator.getAccessToken())
+    every { accountManagerFutureBundle.result } throws IOException()
     every {
       accountManager.getAuthToken(account, AUTH_TOKEN_TYPE, any<Bundle>(), true, any(), any())
-    } throws IOException()
+    } returns accountManagerFutureBundle
     Assert.assertEquals(accessToken, tokenAuthenticator.getAccessToken())
+    every { accountManagerFutureBundle.result } throws AuthenticatorException()
     every {
       accountManager.getAuthToken(account, AUTH_TOKEN_TYPE, any<Bundle>(), true, any(), any())
-    } throws AuthenticatorException()
+    } returns accountManagerFutureBundle
     Assert.assertEquals(accessToken, tokenAuthenticator.getAccessToken())
   }
 

diff --git a/...oid/quest/src/main/java/org/smartregister/fhircore/quest/ui/login/AccountAuthenticator.kt b/...oid/quest/src/main/java/org/smartregister/fhircore/quest/ui/login/AccountAuthenticator.kt
@@ -25,12 +25,10 @@ import android.content.Intent
 import android.os.Bundle
 import androidx.core.os.bundleOf
 import dagger.hilt.android.qualifiers.ApplicationContext
-import java.net.UnknownHostException
 import java.util.Locale
 import javax.inject.Inject
 import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
 import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator.Companion.AUTH_TOKEN_TYPE
-import retrofit2.HttpException
 import timber.log.Timber
 
 class AccountAuthenticator
@@ -82,11 +80,8 @@ constructor(
             tokenAuthenticator.refreshToken(refreshToken)
           } catch (ex: Exception) {
             Timber.e(ex)
-            when (ex) {
-              is HttpException,
-              is UnknownHostException, -> ""
-              else -> throw ex
-            }
+            // any form of exception will unset token to empty, thereby forcing re-login
+            ""
           }
       }
     }

diff --git a/...quest/src/test/java/org/smartregister/fhircore/quest/ui/login/AccountAuthenticatorTest.kt b/...quest/src/test/java/org/smartregister/fhircore/quest/ui/login/AccountAuthenticatorTest.kt
@@ -179,8 +179,8 @@ class AccountAuthenticatorTest : RobolectricTest() {
     Assert.assertFalse(authTokenBundle.containsKey(AccountManager.KEY_AUTHTOKEN))
   }
 
-  @Test(expected = RuntimeException::class)
-  fun testGetBundleWithoutAuthInfoWhenCaughtUnknownHost() {
+  @Test
+  fun testGetBundleWithoutAuthInfoWhenRefreshTokenFailsWithRuntimeException() {
     every { tokenAuthenticator.isTokenActive(any()) } returns false
     val account = spyk(Account("newAccName", "newAccType"))
     every { accountManager.peekAuthToken(account, authTokenType) } returns ""
@@ -189,7 +189,10 @@ class AccountAuthenticatorTest : RobolectricTest() {
     every { accountManager.getPassword(account) } returns refreshToken
     every { tokenAuthenticator.refreshToken(refreshToken) } throws RuntimeException()
 
-    accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())
+    accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf()).also {
+      Assert.assertNotNull(it)
+      Assert.assertFalse(it.containsKey(AccountManager.KEY_AUTHTOKEN))
+    }
   }
 
   @Test