fix: Delete factor should delete it locally for deleted factors from …

…API [ACCSEC-18910](#141)
twilio · Sep 23, 2020 · 11bc7c7 · 11bc7c7
1 parent a0b3dc0
commit 11bc7c7
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 5 deletions.
diff --git a/verify/src/androidTest/java/com/twilio/verify/BaseServerTest.kt b/verify/src/androidTest/java/com/twilio/verify/BaseServerTest.kt
@@ -83,7 +83,7 @@ open class BaseServerTest {
 }
 
 fun CountingIdlingResource.waitForResource(
-  waitFor: Long = 100,
+  waitFor: Long = 200,
   times: Int = 20
 ) {
   for (i in 0..times) {

diff --git a/verify/src/androidTest/java/com/twilio/verify/DeleteFactorTests.kt b/verify/src/androidTest/java/com/twilio/verify/DeleteFactorTests.kt
@@ -4,6 +4,7 @@
 package com.twilio.verify
 
 import com.twilio.verify.TwilioVerifyException.ErrorCode.NetworkError
+import com.twilio.verify.api.dateHeaderKey
 import com.twilio.verify.networking.NetworkException
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
@@ -34,6 +35,28 @@ class DeleteFactorTests : BaseFactorTest() {
     idlingResource.waitForResource()
   }
 
+  @Test
+  fun testDeleteFactorWithNoExistingFactorShouldCallSuccess() {
+    assertTrue(keyStore.containsAlias(factor!!.keyPairAlias))
+    assertTrue(sharedPreferences.contains(factor!!.sid))
+    enqueueMockResponse(401, headers = mapOf(dateHeaderKey to listOf("Tue, 21 Jul 2020 17:07:32 GMT")))
+    enqueueMockResponse(401)
+    idlingResource.increment()
+    twilioVerify.deleteFactor(
+      factor!!.sid,
+      {
+        assertFalse(keyStore.containsAlias(factor!!.keyPairAlias))
+        assertFalse(sharedPreferences.contains(factor!!.sid))
+        idlingResource.decrement()
+      },
+      { e ->
+        fail(e.message)
+        idlingResource.decrement()
+      }
+    )
+    idlingResource.waitForResource()
+  }
+
   @Test
   fun testDeleteFactorWithInvalidAPIResponseShouldCallError() {
     assertTrue(keyStore.containsAlias(factor!!.keyPairAlias))

diff --git a/verify/src/main/java/com/twilio/verify/api/BaseAPIClient.kt b/verify/src/main/java/com/twilio/verify/api/BaseAPIClient.kt
@@ -10,6 +10,7 @@ import com.twilio.verify.networking.NetworkException
  */
 
 internal const val unauthorized = 401
+internal const val notFound = 404
 internal const val dateHeaderKey = "Date"
 internal const val retryTimes = 1
 

diff --git a/verify/src/main/java/com/twilio/verify/api/FactorAPIClient.kt b/verify/src/main/java/com/twilio/verify/api/FactorAPIClient.kt
@@ -182,7 +182,16 @@ internal class FactorAPIClient(
             success()
           },
           { exception ->
-            validateException(exception, ::deleteFactor, retries, error)
+            when (exception.failureResponse?.responseCode) {
+              notFound -> success()
+              unauthorized ->
+                if (retries == 0) {
+                  success()
+                } else {
+                  validateException(exception, ::deleteFactor, retries, error)
+                }
+              else -> validateException(exception, ::deleteFactor, retries, error)
+            }
           }
         )
       } catch (e: TwilioVerifyException) {

diff --git a/verify/src/test/java/com/twilio/verify/api/FactorAPIClientTest.kt b/verify/src/test/java/com/twilio/verify/api/FactorAPIClientTest.kt
@@ -727,7 +727,7 @@ class FactorAPIClientTest {
   }
 
   @Test
-  fun `Delete a factor with out of sync time should retry only another time`() {
+  fun `Delete a factor with unauthorized response code should retry only another time and call success because factor does not exist`() {
     val identity = "identity"
     val factorSid = "sid"
     val serviceSid = "serviceSid"
@@ -754,11 +754,10 @@ class FactorAPIClientTest {
     factorAPIClient.delete(
       factor,
       {
-        fail()
         idlingResource.operationFinished()
       },
       { exception ->
-        assertEquals(expectedException, exception.cause)
+        fail(exception.message)
         idlingResource.operationFinished()
       }
     )
@@ -767,6 +766,43 @@ class FactorAPIClientTest {
     verify(networkProvider, times(retryTimes + 1)).execute(any(), any(), any())
   }
 
+  @Test
+  fun `Delete a factor with notFound response code should call success because factor does not exist`() {
+    val identity = "identity"
+    val factorSid = "sid"
+    val serviceSid = "serviceSid"
+    val factor =
+      PushFactor(
+        factorSid, "friendlyName", "accountSid", serviceSid, identity, Verified, Date(),
+        config = Config("credentialSid")
+      )
+    val expectedException = NetworkException(
+      FailureResponse(
+        notFound,
+        null,
+        null
+      )
+    )
+    argumentCaptor<(NetworkException) -> Unit>().apply {
+      whenever(networkProvider.execute(any(), any(), capture())).then {
+        lastValue.invoke(expectedException)
+      }
+    }
+    whenever(authentication.generateJWT(factor)).thenReturn("authToken")
+    idlingResource.startOperation()
+    factorAPIClient.delete(
+      factor,
+      {
+        idlingResource.operationFinished()
+      },
+      {
+        fail()
+        idlingResource.operationFinished()
+      }
+    )
+    idlingResource.waitForIdle()
+  }
+
   @Test
   fun `Delete a factor with an error response should not call success`() {
     val identity = "identity"