Merge pull request #2742 from opensrp/fix_subsequent_crash_on_keycloa…

…k_error

[MWCore] - Fix crash occurring on authentication fails after initial log-in

android/engine/src/main/java/org/smartregister/fhircore/engine/auth/AccountAuthenticator.kt

@@ -28,14 +28,12 @@ import android.os.Handler
 import android.os.Looper
 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 org.smartregister.fhircore.engine.ui.login.LoginActivity
 import org.smartregister.fhircore.engine.util.SecureSharedPreference
-import retrofit2.HttpException
 import timber.log.Timber
 
 class AccountAuthenticator
@@ -88,10 +86,7 @@ constructor(
             tokenAuthenticator.refreshToken(refreshToken)
           } catch (ex: Exception) {
             Timber.e(ex)
-            when (ex) {
-              is HttpException, is UnknownHostException -> ""
-              else -> throw ex
-            }
+            "" // Set to EMPTY, so as to redirect to log in screen, and try again
           }
       }
     }

android/engine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt

@@ -66,38 +66,42 @@ constructor(
   private var isLoginPageRendered = false
 
   override 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)
+    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 authResultBundle =
+      try {
+        val authResultFuture =
+          accountManager.getAuthToken(
+            account,
+            AUTH_TOKEN_TYPE,
+            bundleOf(),
+            true,
+            accountManager.handleAccountManagerFutureCallback(account),
+            Handler(Looper.getMainLooper()) { message: Message ->
+              Timber.e(message.toString())
+              true
+            }
+          )
+        authResultFuture.result
+      } catch (exception: Exception) {
+        when (exception) {
+          is OperationCanceledException, is AuthenticatorException, is IOException -> {
             // TODO: Should we cancel the sync job to avoid retries when offline?
+            Timber.e(exception)
+            bundleOf(AccountManager.KEY_AUTHTOKEN to accessToken)
           }
+          else -> bundleOf()
         }
-      } else {
-        isLoginPageRendered = false
       }
-      accessToken
-    } else ""
+    return if (authResultBundle.containsKey(AccountManager.KEY_AUTHTOKEN))
+      authResultBundle.getString(AccountManager.KEY_AUTHTOKEN)!!
+    else ""
   }
 
   private fun AccountManager.handleAccountManagerFutureCallback(account: Account?) =
@@ -248,8 +252,7 @@ constructor(
     }
   }
 
-  fun sessionActive(): Boolean =
-    findAccount()?.let { isTokenActive(accountManager.peekAuthToken(it, AUTH_TOKEN_TYPE)) } ?: false
+  fun sessionActive(): Boolean = isTokenActive(getAccessToken())
 
   fun invalidateSession(onSessionInvalidated: () -> Unit) {
     findAccount()?.let { account ->

android/engine/src/main/java/org/smartregister/fhircore/engine/di/CoreModule.kt

@@ -34,6 +34,7 @@ import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry
 import org.smartregister.fhircore.engine.configuration.app.ConfigService
 import org.smartregister.fhircore.engine.data.local.DefaultRepository
 import org.smartregister.fhircore.engine.data.local.register.dao.HivRegisterDao
+import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
 import org.smartregister.fhircore.engine.domain.repository.PatientDao
 import org.smartregister.fhircore.engine.sync.SyncBroadcaster
 import org.smartregister.fhircore.engine.trace.PerformanceReporter
@@ -51,6 +52,7 @@ class CoreModule {
     configService: ConfigService,
     fhirEngine: FhirEngine,
     tracer: PerformanceReporter,
+    tokenAuthenticator: TokenAuthenticator,
     sharedPreferencesHelper: SharedPreferencesHelper
   ) =
     SyncBroadcaster(
@@ -59,6 +61,7 @@ class CoreModule {
       fhirEngine = fhirEngine,
       appContext = context,
       tracer = tracer,
+      tokenAuthenticator = tokenAuthenticator,
       sharedPreferencesHelper = sharedPreferencesHelper
     )
 

android/engine/src/main/java/org/smartregister/fhircore/engine/sync/SyncBroadcaster.kt

@@ -42,6 +42,7 @@ import org.hl7.fhir.r4.model.ResourceType
 import org.smartregister.fhircore.engine.R
 import org.smartregister.fhircore.engine.configuration.ConfigurationRegistry
 import org.smartregister.fhircore.engine.configuration.app.ConfigService
+import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
 import org.smartregister.fhircore.engine.trace.PerformanceReporter
 import org.smartregister.fhircore.engine.util.DefaultDispatcherProvider
 import org.smartregister.fhircore.engine.util.DispatcherProvider
@@ -64,6 +65,7 @@ constructor(
   val dispatcherProvider: DispatcherProvider = DefaultDispatcherProvider(),
   val tracer: PerformanceReporter,
   val sharedPreferencesHelper: SharedPreferencesHelper,
+  val tokenAuthenticator: TokenAuthenticator,
   @ApplicationContext val appContext: Context
 ) {
   /**
@@ -112,17 +114,26 @@ constructor(
   fun runSync(networkState: (Context) -> Boolean = { NetworkState(it).invoke() }) {
     Timber.i("Running one-time sync...")
     CoroutineScope(dispatcherProvider.io()).launch {
-      networkState(appContext).apply {
-        if (this) {
-          Sync.oneTimeSync<AppSyncWorker>(appContext)
-          getWorkerInfo<AppSyncWorker>().collect { sharedSyncStatus.emit(it) }
-        } else {
-          val message = appContext.getString(R.string.unable_to_sync)
-          val resourceSyncException =
-            listOf(ResourceSyncException(ResourceType.Flag, java.lang.Exception(message)))
-          sharedSyncStatus.emit(SyncJobStatus.Failed(resourceSyncException))
-        }
+      val isConnected = networkState(appContext)
+      if (!isConnected) {
+        val message = appContext.getString(R.string.unable_to_sync)
+        val resourceSyncException =
+          listOf(ResourceSyncException(ResourceType.Flag, java.lang.Exception(message)))
+        sharedSyncStatus.emit(SyncJobStatus.Failed(resourceSyncException))
+        return@launch
       }
+      val isAuthenticated = tokenAuthenticator.sessionActive()
+      if (!isAuthenticated) {
+        val authFailResourceSyncException =
+          ResourceSyncException(
+            ResourceType.Flag,
+            Exception(appContext.getString(R.string.sync_authentication_error))
+          )
+        sharedSyncStatus.emit(SyncJobStatus.Failed(listOf(authFailResourceSyncException)))
+        return@launch
+      }
+      Sync.oneTimeSync<AppSyncWorker>(appContext)
+      getWorkerInfo<AppSyncWorker>().collect { sharedSyncStatus.emit(it) }
     }
   }
 

android/engine/src/test/java/org/smartregister/fhircore/engine/auth/AccountAuthenticatorTest.kt

@@ -323,7 +323,6 @@ class AccountAuthenticatorTest : RobolectricTest() {
     Assert.assertFalse(authTokenBundle.containsKey(KEY_AUTHTOKEN))
   }
 
-  @Test(expected = RuntimeException::class)
   fun testGetBundleWithoutAuthInfoWhenCaughtUnknownHost() {
     every { tokenAuthenticator.isTokenActive(any()) } returns false
     val account = spyk(Account("newAccName", "newAccType"))
@@ -333,7 +332,11 @@ class AccountAuthenticatorTest : RobolectricTest() {
     every { accountManager.getPassword(account) } returns refreshToken
     every { tokenAuthenticator.refreshToken(refreshToken) } throws RuntimeException()
 
-    accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())
+    val authTokenBundle =
+      accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())
+    Assert.assertNotNull(authTokenBundle)
+    Assert.assertFalse(authTokenBundle.containsKey(KEY_AUTHTOKEN))
+    Assert.assertTrue(authTokenBundle.containsKey(KEY_INTENT)) // contains intent to re-login
   }
 
   @Test

android/engine/src/test/java/org/smartregister/fhircore/engine/auth/TokenAuthenticatorTest.kt

@@ -462,6 +462,7 @@ class TokenAuthenticatorTest : RobolectricTest() {
     every { accountManager.peekAuthToken(account, TokenAuthenticator.AUTH_TOKEN_TYPE) } returns
       token
     every { tokenAuthenticator.isTokenActive(any()) } returns false
+    every { accountManager.invalidateAuthToken(any(), any()) } just runs
 
     Assert.assertFalse(tokenAuthenticator.sessionActive())
   }

android/engine/src/test/java/org/smartregister/fhircore/engine/sync/SyncBroadcasterTest.kt

@@ -51,8 +51,10 @@ import org.junit.After
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
+import org.smartregister.fhircore.engine.R
 import org.smartregister.fhircore.engine.app.AppConfigService
 import org.smartregister.fhircore.engine.app.fakes.Faker
+import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
 import org.smartregister.fhircore.engine.robolectric.RobolectricTest
 import org.smartregister.fhircore.engine.rule.CoroutineTestRule
 import org.smartregister.fhircore.engine.trace.FakePerformanceReporter
@@ -65,6 +67,7 @@ class SyncBroadcasterTest : RobolectricTest() {
 
   private lateinit var syncBroadcaster: SyncBroadcaster
   private lateinit var workManager: WorkManager
+  private lateinit var tokenAuthenticator: TokenAuthenticator
   private val sharedSyncStatus: MutableSharedFlow<SyncJobStatus> = MutableSharedFlow()
   private val sharedPreferencesHelper: SharedPreferencesHelper = mockk()
   private lateinit var tracer: PerformanceReporter
@@ -75,6 +78,9 @@ class SyncBroadcasterTest : RobolectricTest() {
     mockkStatic(WorkManager::class)
     workManager = mockk()
     tracer = mockk()
+    tokenAuthenticator = mockk()
+
+    every { tokenAuthenticator.sessionActive() } returns true
 
     val mutableLiveData = MutableLiveData(listOf<WorkInfo>())
     every { WorkManager.getInstance(appContext) } returns workManager
@@ -103,6 +109,7 @@ class SyncBroadcasterTest : RobolectricTest() {
         sharedSyncStatus = sharedSyncStatus,
         dispatcherProvider = CoroutineTestRule().testDispatcherProvider,
         tracer = tracer,
+        tokenAuthenticator = tokenAuthenticator,
         sharedPreferencesHelper = sharedPreferencesHelper,
         appContext = mockk(relaxed = true)
       )
@@ -201,6 +208,7 @@ class SyncBroadcasterTest : RobolectricTest() {
         dispatcherProvider = CoroutineTestRule().testDispatcherProvider,
         appContext = context,
         tracer = FakePerformanceReporter(),
+        tokenAuthenticator = tokenAuthenticator,
         sharedPreferencesHelper = sharedPreferencesHelper
       )
     val collectedSyncStatusList = mutableListOf<SyncJobStatus>()
@@ -209,7 +217,49 @@ class SyncBroadcasterTest : RobolectricTest() {
         sharedSyncStatus.toList(collectedSyncStatusList)
       }
     syncBroadcaster.runSync { false }
-    Assert.assertTrue(collectedSyncStatusList.first() is SyncJobStatus.Failed)
+    val syncStatus = collectedSyncStatusList.first()
+    Assert.assertTrue(syncStatus is SyncJobStatus.Failed)
+    syncStatus as SyncJobStatus.Failed
+    Assert.assertEquals(
+      context.getString(R.string.unable_to_sync),
+      syncStatus.exceptions.first().exception.message
+    )
+    job.cancel()
+  }
+
+  @Test
+  fun shouldEmitSyncFailedWhenNetworkAndTokenSessionIsNotActive() = runTest {
+    val configurationRegistry = Faker.buildTestConfigurationRegistry()
+    val context = ApplicationProvider.getApplicationContext<HiltTestApplication>()
+    val configService = AppConfigService(context = context)
+    val sharedSyncStatus: MutableSharedFlow<SyncJobStatus> = MutableSharedFlow()
+    val tokenAuthenticatorAlt = mockk<TokenAuthenticator>()
+    every { tokenAuthenticatorAlt.sessionActive() } returns false
+    syncBroadcaster =
+      SyncBroadcaster(
+        configurationRegistry,
+        configService,
+        fhirEngine = mockk(),
+        sharedSyncStatus,
+        dispatcherProvider = CoroutineTestRule().testDispatcherProvider,
+        appContext = context,
+        tracer = FakePerformanceReporter(),
+        tokenAuthenticator = tokenAuthenticatorAlt,
+        sharedPreferencesHelper = sharedPreferencesHelper
+      )
+    val collectedSyncStatusList = mutableListOf<SyncJobStatus>()
+    val job =
+      launch(UnconfinedTestDispatcher(testScheduler)) {
+        sharedSyncStatus.toList(collectedSyncStatusList)
+      }
+    syncBroadcaster.runSync { true }
+    val syncStatus = collectedSyncStatusList.first()
+    Assert.assertTrue(syncStatus is SyncJobStatus.Failed)
+    syncStatus as SyncJobStatus.Failed
+    Assert.assertEquals(
+      context.getString(R.string.sync_authentication_error),
+      syncStatus.exceptions.first().exception.message
+    )
     job.cancel()
   }
 }

android/engine/src/test/java/org/smartregister/fhircore/engine/ui/userprofile/UserProfileViewModelTest.kt

@@ -43,6 +43,7 @@ import org.smartregister.fhircore.engine.auth.AccountAuthenticator
 import org.smartregister.fhircore.engine.configuration.app.ConfigService
 import org.smartregister.fhircore.engine.data.remote.fhir.resource.FhirResourceDataSource
 import org.smartregister.fhircore.engine.data.remote.fhir.resource.FhirResourceService
+import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
 import org.smartregister.fhircore.engine.domain.model.Language
 import org.smartregister.fhircore.engine.robolectric.RobolectricTest
 import org.smartregister.fhircore.engine.rule.CoroutineTestRule
@@ -63,6 +64,7 @@ class UserProfileViewModelTest : RobolectricTest() {
   lateinit var sharedPreferencesHelper: SharedPreferencesHelper
 
   @BindValue var configurationRegistry = Faker.buildTestConfigurationRegistry()
+  var tokenAuthenticator: TokenAuthenticator = mockk()
 
   private lateinit var configService: ConfigService
 
@@ -81,6 +83,8 @@ class UserProfileViewModelTest : RobolectricTest() {
     configService = AppConfigService(context = context)
     fhirResourceDataSource = spyk(FhirResourceDataSource(resourceService))
 
+    every { tokenAuthenticator.sessionActive() } returns true
+
     syncBroadcaster =
       SyncBroadcaster(
         configurationRegistry,
@@ -90,6 +94,7 @@ class UserProfileViewModelTest : RobolectricTest() {
         dispatcherProvider = CoroutineTestRule().testDispatcherProvider,
         appContext = context,
         tracer = mockk(),
+        tokenAuthenticator = tokenAuthenticator,
         sharedPreferencesHelper = sharedPreferencesHelper
       )
     userProfileViewModel =