Update refresh token to avoid reuse of stale refresh token (#2959)

Stale refresh token on expiry leads to requests raising 401 and sync failing
opensrp · Jan 11, 2024 · 2df2764 · 2df2764
1 parent ba88193
commit 2df2764
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 8 deletions.
diff --git a/android/engine/src/main/java/org/smartregister/fhircore/engine/auth/AccountAuthenticator.kt b/android/engine/src/main/java/org/smartregister/fhircore/engine/auth/AccountAuthenticator.kt
@@ -83,7 +83,7 @@ constructor(
       if (!refreshToken.isNullOrEmpty()) {
         authToken =
           try {
-            tokenAuthenticator.refreshToken(refreshToken)
+            tokenAuthenticator.refreshToken(account, refreshToken)
           } catch (ex: Exception) {
             Timber.e(ex)
             "" // Set to EMPTY, so as to redirect to log in screen, and try again

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
@@ -223,13 +223,16 @@ constructor(
    * [HttpException] or [UnknownHostException] exceptions
    */
   @Throws(HttpException::class, UnknownHostException::class)
-  fun refreshToken(currentRefreshToken: String): String {
+  fun refreshToken(account: Account, currentRefreshToken: String): String {
     return runBlocking {
       val oAuthResponse =
         oAuthService.fetchToken(
           buildOAuthPayload(REFRESH_TOKEN).apply { put(REFRESH_TOKEN, currentRefreshToken) }
         )
 
+      // Updates with new refresh-token
+      accountManager.setPassword(account, oAuthResponse.refreshToken!!)
+
       // Returns valid token or throws exception, NullPointerException not expected
       oAuthResponse.accessToken!!
     }

diff --git a/...d/engine/src/test/java/org/smartregister/fhircore/engine/auth/AccountAuthenticatorTest.kt b/...d/engine/src/test/java/org/smartregister/fhircore/engine/auth/AccountAuthenticatorTest.kt
@@ -253,8 +253,9 @@ class AccountAuthenticatorTest : RobolectricTest() {
     every { accountManager.peekAuthToken(account, authTokenType) } returns ""
     val refreshToken = "refreshToken"
     every { accountManager.getPassword(account) } returns refreshToken
+    every { accountManager.setPassword(account, any()) } just runs
 
-    every { tokenAuthenticator.refreshToken(refreshToken) } returns ""
+    every { tokenAuthenticator.refreshToken(account, refreshToken) } returns ""
 
     val authToken = accountAuthenticator.getAuthToken(mockk(), account, authTokenType, bundleOf())
     val parcelable = authToken.get(AccountManager.KEY_INTENT) as Intent
@@ -273,7 +274,8 @@ class AccountAuthenticatorTest : RobolectricTest() {
 
     val refreshToken = "refreshToken"
     every { accountManager.getPassword(account) } returns refreshToken
-    every { tokenAuthenticator.refreshToken(refreshToken) } returns "newAccessToken"
+    every { accountManager.setPassword(account, any()) } just runs
+    every { tokenAuthenticator.refreshToken(account, refreshToken) } returns "newAccessToken"
 
     val authTokenBundle: Bundle =
       accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())
@@ -291,7 +293,7 @@ class AccountAuthenticatorTest : RobolectricTest() {
 
     val refreshToken = "refreshToken"
     every { accountManager.getPassword(account) } returns refreshToken
-    every { tokenAuthenticator.refreshToken(refreshToken) } throws
+    every { tokenAuthenticator.refreshToken(account, refreshToken) } throws
       HttpException(
         mockk {
           every { code() } returns 0
@@ -314,7 +316,7 @@ class AccountAuthenticatorTest : RobolectricTest() {
 
     val refreshToken = "refreshToken"
     every { accountManager.getPassword(account) } returns refreshToken
-    every { tokenAuthenticator.refreshToken(refreshToken) } throws UnknownHostException()
+    every { tokenAuthenticator.refreshToken(account, refreshToken) } throws UnknownHostException()
 
     val authTokenBundle: Bundle =
       accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())
@@ -330,7 +332,7 @@ class AccountAuthenticatorTest : RobolectricTest() {
 
     val refreshToken = "refreshToken"
     every { accountManager.getPassword(account) } returns refreshToken
-    every { tokenAuthenticator.refreshToken(refreshToken) } throws RuntimeException()
+    every { tokenAuthenticator.refreshToken(account, refreshToken) } throws RuntimeException()
 
     val authTokenBundle =
       accountAuthenticator.getAuthToken(null, account, authTokenType, bundleOf())

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
@@ -34,6 +34,7 @@ import io.mockk.mockk
 import io.mockk.runs
 import io.mockk.slot
 import io.mockk.spyk
+import io.mockk.verify
 import io.mockk.verifyOrder
 import java.io.IOException
 import java.net.UnknownHostException
@@ -388,6 +389,7 @@ class TokenAuthenticatorTest : RobolectricTest() {
 
   @Test
   fun testRefreshTokenShouldReturnToken() {
+    val account = Account(sampleUsername, PROVIDER)
     val accessToken = "soRefreshingNewToken"
     val oAuthResponse =
       OAuthResponse(
@@ -398,11 +400,13 @@ class TokenAuthenticatorTest : RobolectricTest() {
         scope = SCOPE
       )
     coEvery { oAuthService.fetchToken(any()) } returns oAuthResponse
+    every { accountManager.setPassword(account, any()) } just runs
 
     val currentRefreshToken = "oldRefreshToken"
-    val newAccessToken = tokenAuthenticator.refreshToken(currentRefreshToken)
+    val newAccessToken = tokenAuthenticator.refreshToken(account, currentRefreshToken)
     Assert.assertNotNull(newAccessToken)
     Assert.assertEquals(accessToken, newAccessToken)
+    verify { accountManager.setPassword(eq(account), oAuthResponse.refreshToken) }
   }
 
   @OptIn(ExperimentalCoroutinesApi::class)