clean up MFA tests per review feedback

utkrishtsahu · utkrishtsahu · commit dc3c8fb83525 · 2026-02-04T10:34:12.000+05:30
diff --git a/auth0/src/test/java/com/auth0/android/authentication/MfaApiClientTest.kt b/auth0/src/test/java/com/auth0/android/authentication/MfaApiClientTest.kt
@@ -5,14 +5,15 @@ import com.auth0.android.authentication.mfa.MfaApiClient
 import com.auth0.android.authentication.mfa.MfaEnrollmentType
 import com.auth0.android.authentication.mfa.MfaVerificationType
 import com.auth0.android.authentication.mfa.MfaException.*
-import com.auth0.android.callback.Callback
 import com.auth0.android.request.internal.ThreadSwitcherShadow
 import com.auth0.android.result.Authenticator
 import com.auth0.android.result.Challenge
 import com.auth0.android.result.Credentials
 import com.auth0.android.result.EnrollmentChallenge
 import com.auth0.android.result.MfaEnrollmentChallenge
 import com.auth0.android.result.TotpEnrollmentChallenge
+import com.auth0.android.util.CallbackMatcher
+import com.auth0.android.util.MockCallback
 import com.auth0.android.util.SSLTestUtils
 import com.google.gson.Gson
 import com.google.gson.GsonBuilder
@@ -32,8 +33,6 @@ import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 import org.robolectric.shadows.ShadowLooper
-import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
 
 @RunWith(RobolectricTestRunner::class)
 @Config(shadows = [ThreadSwitcherShadow::class])
@@ -624,119 +623,67 @@ public class MfaApiClientTest {
         val json = """[{"id": "sms|dev_123", "authenticator_type": "oob", "active": true}]"""
         enqueueMockResponse(json)
 
-        val latch = CountDownLatch(1)
-        var callbackResult: List<Authenticator>? = null
-        var callbackError: MfaListAuthenticatorsException? = null
+        val callback = MockCallback<List<Authenticator>, MfaListAuthenticatorsException>()
 
         mfaClient.getAuthenticators(listOf("oob"))
-            .start(object : Callback<List<Authenticator>, MfaListAuthenticatorsException> {
-                override fun onSuccess(result: List<Authenticator>) {
-                    callbackResult = result
-                    latch.countDown()
-                }
-
-                override fun onFailure(error: MfaListAuthenticatorsException) {
-                    callbackError = error
-                    latch.countDown()
-                }
-            })
+            .start(callback)
 
         ShadowLooper.idleMainLooper()
-        latch.await(5, TimeUnit.SECONDS)
 
-        assertThat(callbackResult, `is`(notNullValue()))
-        assertThat(callbackResult, hasSize(1))
-        assertThat(callbackError, `is`(nullValue()))
+        assertThat(callback.getPayload(), `is`(notNullValue()))
+        assertThat(callback.getPayload(), hasSize(1))
+        assertThat(callback.getError(), `is`(nullValue()))
     }
 
     @Test
     public fun shouldEnrollPhoneWithCallback(): Unit {
         val json = """{"id": "sms|dev_123", "auth_session": "session_abc"}"""
         enqueueMockResponse(json)
 
-        val latch = CountDownLatch(1)
-        var callbackResult: EnrollmentChallenge? = null
-        var callbackError: MfaEnrollmentException? = null
+        val callback = MockCallback<EnrollmentChallenge, MfaEnrollmentException>()
 
         mfaClient.enroll(MfaEnrollmentType.Phone("+12025550135"))
-            .start(object : Callback<EnrollmentChallenge, MfaEnrollmentException> {
-                override fun onSuccess(result: EnrollmentChallenge) {
-                    callbackResult = result
-                    latch.countDown()
-                }
-
-                override fun onFailure(error: MfaEnrollmentException) {
-                    callbackError = error
-                    latch.countDown()
-                }
-            })
+            .start(callback)
 
         ShadowLooper.idleMainLooper()
-        latch.await(5, TimeUnit.SECONDS)
 
-        assertThat(callbackResult, `is`(notNullValue()))
-        assertThat(callbackResult!!.id, `is`("sms|dev_123"))
-        assertThat(callbackError, `is`(nullValue()))
+        assertThat(callback.getPayload(), `is`(notNullValue()))
+        assertThat(callback.getPayload().id, `is`("sms|dev_123"))
+        assertThat(callback.getError(), `is`(nullValue()))
     }
 
     @Test
     public fun shouldChallengeWithCallback(): Unit {
         val json = """{"challenge_type": "oob", "oob_code": "oob_123"}"""
         enqueueMockResponse(json)
 
-        val latch = CountDownLatch(1)
-        var callbackResult: Challenge? = null
-        var callbackError: MfaChallengeException? = null
+        val callback = MockCallback<Challenge, MfaChallengeException>()
 
         mfaClient.challenge("sms|dev_123")
-            .start(object : Callback<Challenge, MfaChallengeException> {
-                override fun onSuccess(result: Challenge) {
-                    callbackResult = result
-                    latch.countDown()
-                }
-
-                override fun onFailure(error: MfaChallengeException) {
-                    callbackError = error
-                    latch.countDown()
-                }
-            })
+            .start(callback)
 
         ShadowLooper.idleMainLooper()
-        latch.await(5, TimeUnit.SECONDS)
 
-        assertThat(callbackResult, `is`(notNullValue()))
-        assertThat(callbackResult!!.challengeType, `is`("oob"))
-        assertThat(callbackError, `is`(nullValue()))
+        assertThat(callback.getPayload(), `is`(notNullValue()))
+        assertThat(callback.getPayload().challengeType, `is`("oob"))
+        assertThat(callback.getError(), `is`(nullValue()))
     }
 
     @Test
     public fun shouldVerifyOtpWithCallback(): Unit {
         val json = """{"access_token": "$ACCESS_TOKEN", "id_token": "$ID_TOKEN", "token_type": "Bearer", "expires_in": 86400}"""
         enqueueMockResponse(json)
 
-        val latch = CountDownLatch(1)
-        var callbackResult: Credentials? = null
-        var callbackError: MfaVerifyException? = null
+        val callback = MockCallback<Credentials, MfaVerifyException>()
 
         mfaClient.verify(MfaVerificationType.Otp("123456"))
-            .start(object : Callback<Credentials, MfaVerifyException> {
-                override fun onSuccess(result: Credentials) {
-                    callbackResult = result
-                    latch.countDown()
-                }
-
-                override fun onFailure(error: MfaVerifyException) {
-                    callbackError = error
-                    latch.countDown()
-                }
-            })
+            .start(callback)
 
         ShadowLooper.idleMainLooper()
-        latch.await(5, TimeUnit.SECONDS)
 
-        assertThat(callbackResult, `is`(notNullValue()))
-        assertThat(callbackResult!!.accessToken, `is`(ACCESS_TOKEN))
-        assertThat(callbackError, `is`(nullValue()))
+        assertThat(callback.getPayload(), `is`(notNullValue()))
+        assertThat(callback.getPayload().accessToken, `is`(ACCESS_TOKEN))
+        assertThat(callback.getError(), `is`(nullValue()))
     }
 
 
diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.kt b/auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.kt
@@ -1953,7 +1953,6 @@ public class CredentialsManagerTest {
 
     @Test
     public fun shouldFailWithMfaRequiredWhenRenewingExpiredCredentials() {
-        // Arrange: Set up expired credentials that need renewal
         Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
         Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
         Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
@@ -1964,7 +1963,6 @@ public class CredentialsManagerTest {
         Mockito.`when`(storage.retrieveString("com.auth0.scope")).thenReturn("scope")
         Mockito.`when`(client.renewAuth("refreshToken")).thenReturn(request)
 
-        // Create an AuthenticationException that simulates MFA required response
         val mfaRequiredValues: MutableMap<String, Any> = mutableMapOf(
             "error" to "mfa_required",
             "error_description" to "Multifactor authentication required",
@@ -1978,24 +1976,19 @@ public class CredentialsManagerTest {
         )
         val mfaRequiredException = AuthenticationException(mfaRequiredValues, 403)
         
-        // Verify the exception is correctly configured
         MatcherAssert.assertThat(mfaRequiredException.isMultifactorRequired, Is.`is`(true))
         MatcherAssert.assertThat(mfaRequiredException.getCode(), Is.`is`("mfa_required"))
         
         Mockito.`when`(request.execute()).thenThrow(mfaRequiredException)
 
-        // Act: Try to get credentials, which triggers renewal
         manager.getCredentials(callback)
 
-        // Assert: Verify the callback receives MFA_REQUIRED exception with payload
         verify(callback).onFailure(exceptionCaptor.capture())
         val exception = exceptionCaptor.firstValue
         MatcherAssert.assertThat(exception, Is.`is`(Matchers.notNullValue()))
-        // Note: CredentialsManager uses error.message which is the DEFAULT_MESSAGE from AuthenticationException
         MatcherAssert.assertThat(exception.message, Matchers.containsString("authenticate"))
         MatcherAssert.assertThat(exception.cause, Is.`is`(mfaRequiredException))
         
-        // Verify MFA payload is properly passed through
         MatcherAssert.assertThat(exception.mfaRequiredErrorPayload, Is.`is`(Matchers.notNullValue()))
         MatcherAssert.assertThat(exception.mfaToken, Is.`is`("test-mfa-token-12345"))
         MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.challenge, Is.`is`(Matchers.notNullValue()))
@@ -2004,7 +1997,6 @@ public class CredentialsManagerTest {
 
     @Test
     public fun shouldFailWithMfaRequiredWithEnrollmentOptionsWhenRenewingCredentials() {
-        // Arrange: Set up expired credentials that need renewal
         Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
         Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
         Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
@@ -2015,7 +2007,6 @@ public class CredentialsManagerTest {
         Mockito.`when`(storage.retrieveString("com.auth0.scope")).thenReturn("scope")
         Mockito.`when`(client.renewAuth("refreshToken")).thenReturn(request)
 
-        // Create an AuthenticationException with enrollment options (user needs to enroll MFA)
         val mfaRequiredValues = mapOf(
             "error" to "mfa_required",
             "error_description" to "Multifactor authentication required",
@@ -2031,13 +2022,10 @@ public class CredentialsManagerTest {
         val mfaRequiredException = AuthenticationException(mfaRequiredValues, 403)
         Mockito.`when`(request.execute()).thenThrow(mfaRequiredException)
 
-        // Act: Try to get credentials
         manager.getCredentials(callback)
 
-        // Assert: Verify MFA required with enrollment options
         verify(callback).onFailure(exceptionCaptor.capture())
         val exception = exceptionCaptor.firstValue
-        // Note: CredentialsManager uses error.message which is the DEFAULT_MESSAGE from AuthenticationException
         MatcherAssert.assertThat(exception.message, Matchers.containsString("authenticate"))
         MatcherAssert.assertThat(exception.mfaToken, Is.`is`("enroll-mfa-token"))
         MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.enroll, Is.`is`(Matchers.notNullValue()))
@@ -2047,7 +2035,6 @@ public class CredentialsManagerTest {
 
     @Test
     public fun shouldNotStoreMfaPayloadWhenNonMfaApiErrorOccurs() {
-        // Arrange: Set up expired credentials that need renewal
         Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
         Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
         Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
@@ -2068,13 +2055,10 @@ public class CredentialsManagerTest {
         )
         Mockito.`when`(request.execute()).thenThrow(regularApiError)
 
-        // Act: Try to get credentials
         manager.getCredentials(callback)
 
-        // Assert: Verify no MFA payload is present for non-MFA errors
         verify(callback).onFailure(exceptionCaptor.capture())
         val exception = exceptionCaptor.firstValue
-        // For non-MFA API errors, message is "An error occurred while processing the request."
         MatcherAssert.assertThat(exception.message, Matchers.containsString("processing the request"))
         MatcherAssert.assertThat(exception.mfaRequiredErrorPayload, Is.`is`(Matchers.nullValue()))
         MatcherAssert.assertThat(exception.mfaToken, Is.`is`(Matchers.nullValue()))
@@ -2083,7 +2067,6 @@ public class CredentialsManagerTest {
     @Test
     @ExperimentalCoroutinesApi
     public fun shouldThrowMfaRequiredExceptionWhenAwaitingExpiredCredentials(): Unit = runTest {
-        // Arrange: Set up expired credentials that need renewal
         Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
         Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
         Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
@@ -2094,7 +2077,6 @@ public class CredentialsManagerTest {
         Mockito.`when`(storage.retrieveString("com.auth0.scope")).thenReturn("scope")
         Mockito.`when`(client.renewAuth("refreshToken")).thenReturn(request)
 
-        // Create an AuthenticationException that simulates MFA required response
         val mfaRequiredValues = mapOf(
             "error" to "mfa_required",
             "error_description" to "Multifactor authentication required",
@@ -2108,7 +2090,6 @@ public class CredentialsManagerTest {
         val mfaRequiredException = AuthenticationException(mfaRequiredValues, 403)
         Mockito.`when`(request.execute()).thenThrow(mfaRequiredException)
 
-        // Act & Assert: Verify awaitCredentials throws CredentialsManagerException with MFA payload
         val exception = assertThrows(CredentialsManagerException::class.java) {
             runBlocking { manager.awaitCredentials() }
         }
@@ -2118,58 +2099,8 @@ public class CredentialsManagerTest {
         MatcherAssert.assertThat(exception.mfaToken, Is.`is`("await-mfa-token-12345"))
     }
 
-    @Test
-    public fun shouldFailWithMfaRequiredContainingBothChallengeAndEnrollOptions() {
-        // Arrange: Set up expired credentials that need renewal
-        Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
-        Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
-        Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
-        Mockito.`when`(storage.retrieveString("com.auth0.token_type")).thenReturn("type")
-        val expirationTime = CredentialsMock.CURRENT_TIME_MS // Expired
-        Mockito.`when`(storage.retrieveLong("com.auth0.expires_at")).thenReturn(expirationTime)
-        Mockito.`when`(storage.retrieveLong("com.auth0.cache_expires_at")).thenReturn(expirationTime)
-        Mockito.`when`(storage.retrieveString("com.auth0.scope")).thenReturn("scope")
-        Mockito.`when`(client.renewAuth("refreshToken")).thenReturn(request)
-
-        // Create MFA required with BOTH challenge (existing factors) AND enroll (can add more factors)
-        val mfaRequiredValues = mapOf(
-            "error" to "mfa_required",
-            "error_description" to "Multifactor authentication required",
-            "mfa_token" to "combined-mfa-token",
-            "mfa_requirements" to mapOf(
-                "challenge" to listOf(
-                    mapOf("type" to "otp")
-                ),
-                "enroll" to listOf(
-                    mapOf("type" to "sms"),
-                    mapOf("type" to "push-notification")
-                )
-            )
-        )
-        val mfaRequiredException = AuthenticationException(mfaRequiredValues, 403)
-        Mockito.`when`(request.execute()).thenThrow(mfaRequiredException)
-
-        // Act: Try to get credentials
-        manager.getCredentials(callback)
-
-        // Assert: Verify both challenge and enroll are present in the payload
-        verify(callback).onFailure(exceptionCaptor.capture())
-        val exception = exceptionCaptor.firstValue
-        MatcherAssert.assertThat(exception.mfaRequiredErrorPayload, Is.`is`(Matchers.notNullValue()))
-        MatcherAssert.assertThat(exception.mfaToken, Is.`is`("combined-mfa-token"))
-        
-        // Verify challenge factors
-        MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.challenge, Is.`is`(Matchers.notNullValue()))
-        MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.challenge?.size, Is.`is`(1))
-        
-        // Verify enroll factors
-        MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.enroll, Is.`is`(Matchers.notNullValue()))
-        MatcherAssert.assertThat(exception.mfaRequiredErrorPayload?.mfaRequirements?.enroll?.size, Is.`is`(2))
-    }
-
     @Test
     public fun shouldPreserveOriginalAuthenticationExceptionAsCauseForMfaRequired() {
-        // Arrange: Set up expired credentials that need renewal
         Mockito.`when`(storage.retrieveString("com.auth0.id_token")).thenReturn("idToken")
         Mockito.`when`(storage.retrieveString("com.auth0.access_token")).thenReturn("accessToken")
         Mockito.`when`(storage.retrieveString("com.auth0.refresh_token")).thenReturn("refreshToken")
@@ -2188,14 +2119,11 @@ public class CredentialsManagerTest {
         val originalException = AuthenticationException(mfaRequiredValues, 403)
         Mockito.`when`(request.execute()).thenThrow(originalException)
 
-        // Act
         manager.getCredentials(callback)
 
-        // Assert: Verify the original exception is preserved as cause
         verify(callback).onFailure(exceptionCaptor.capture())
         val exception = exceptionCaptor.firstValue
         
-        // The cause should be the original AuthenticationException
         MatcherAssert.assertThat(exception.cause, Is.`is`(Matchers.notNullValue()))
         MatcherAssert.assertThat(exception.cause, IsInstanceOf.instanceOf(AuthenticationException::class.java))
         
diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt
