From 97337690e80fbb00e9be3a1d4a3e3f293032b217 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Tue, 17 Dec 2024 10:25:35 -0700 Subject: [PATCH 1/2] handle unexpected exceptions thrown during Google Authentication --- .../provider/google/GoogleAccountProvider.kt | 29 ++++++++++++------- .../google/GoogleAccountProviderTest.kt | 27 +++++++++++++++-- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt index 9e29e1c195..9aa6562fbf 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt +++ b/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt @@ -13,8 +13,10 @@ import com.google.android.gms.auth.api.signin.GoogleSignInAccount import com.google.android.gms.auth.api.signin.GoogleSignInClient import com.google.android.gms.common.api.ApiException import dagger.hilt.android.qualifiers.ApplicationContext +import java.io.IOException import javax.inject.Inject import javax.inject.Singleton +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf @@ -96,18 +98,25 @@ internal class GoogleAccountProvider @Inject constructor( // endregion Login/Logout override suspend fun authenticateWithMobileContentApi(createUser: Boolean): Result { - var account = GoogleSignIn.getLastSignedInAccount(context) - ?: return Result.failure(AuthenticationException.MissingCredentials) - var resp = account.authenticateWithMobileContentApi(createUser) - - if (account.idToken == null || resp?.isSuccessful != true) { - account = refreshSignIn() ?: return Result.failure(AuthenticationException.UnableToRefreshCredentials) - resp = account.authenticateWithMobileContentApi(createUser) + try { + var account = GoogleSignIn.getLastSignedInAccount(context) ?: return Result.failure(AuthenticationException.MissingCredentials) - } + var resp = account.authenticateWithMobileContentApi(createUser) - return resp.extractAuthToken() - .onSuccess { prefs.edit { putString(account.PREF_USER_ID, it.userId) } } + if (account.idToken == null || resp?.isSuccessful != true) { + account = refreshSignIn() ?: return Result.failure(AuthenticationException.UnableToRefreshCredentials) + resp = account.authenticateWithMobileContentApi(createUser) + ?: return Result.failure(AuthenticationException.MissingCredentials) + } + + return resp.extractAuthToken() + .onSuccess { prefs.edit { putString(account.PREF_USER_ID, it.userId) } } + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + if (e !is IOException) Timber.tag(TAG).e(e, "Unexpected error authenticating with Google") + return Result.failure(e) + } } private suspend fun GoogleSignInAccount.authenticateWithMobileContentApi(createUser: Boolean) = diff --git a/library/account/src/test/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProviderTest.kt b/library/account/src/test/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProviderTest.kt index ce69275316..188de9feb1 100644 --- a/library/account/src/test/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProviderTest.kt +++ b/library/account/src/test/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProviderTest.kt @@ -19,6 +19,7 @@ import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.unmockkObject import io.mockk.unmockkStatic +import java.net.UnknownHostException import java.util.UUID import kotlin.random.Random import kotlin.test.AfterTest @@ -28,6 +29,8 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.uuid.ExperimentalUuidApi +import kotlin.uuid.Uuid import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runCurrent @@ -42,11 +45,12 @@ import org.cru.godtools.api.model.AuthToken import org.junit.runner.RunWith import retrofit2.Response -private const val ID_TOKEN_INVALID = "invalid" private const val ID_TOKEN_VALID = "valid" +private const val ID_TOKEN_INVALID = "invalid" +private const val ID_TOKEN_EXCEPTION = "exception" @RunWith(AndroidJUnit4::class) -@OptIn(ExperimentalCoroutinesApi::class) +@OptIn(ExperimentalCoroutinesApi::class, ExperimentalUuidApi::class) class GoogleAccountProviderTest { private val lastSignedInAccount = MutableStateFlow(null) private val userId = UUID.randomUUID().toString() @@ -191,6 +195,25 @@ class GoogleAccountProviderTest { ) } + @Test + fun `authenticateWithMobileContentApi() - Api Exception - UnknownHostException()`() = runTest { + val exception = UnknownHostException() + val token = Uuid.random().toString() + lastSignedInAccount.value = mockk { every { idToken } returns token } + + coEvery { authApi.authenticate(AuthToken.Request(googleIdToken = token, createUser = createUser)) } + .throws(exception) + + assertEquals( + Result.failure(exception), + provider.authenticateWithMobileContentApi(createUser) + ) + coVerifyAll { + authApi.authenticate(AuthToken.Request(googleIdToken = token, createUser = createUser)) + googleSignInClient wasNot Called + } + } + @Test fun `authenticateWithMobileContentApi() - Not authenticated`() = runTest { lastSignedInAccount.value = null From 06f6333c29d4c2a1a52aebb84f05c5a7e732651d Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Tue, 17 Dec 2024 13:56:04 -0700 Subject: [PATCH 2/2] handle unexpected exceptions thrown during facebook auth --- .../facebook/FacebookAccountProvider.kt | 39 ++++++++++++------- .../facebook/FacebookAccountProviderTest.kt | 15 +++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt index d0a4f0551c..d8aef840ab 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt +++ b/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt @@ -12,8 +12,10 @@ import com.facebook.AccessTokenManager import com.facebook.FacebookException import com.facebook.login.LoginManager import dagger.hilt.android.qualifiers.ApplicationContext +import java.io.IOException import javax.inject.Inject import javax.inject.Singleton +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf @@ -29,9 +31,11 @@ import org.cru.godtools.account.provider.AuthenticationException import org.cru.godtools.account.provider.extractAuthToken import org.cru.godtools.api.AuthApi import org.cru.godtools.api.model.AuthToken +import timber.log.Timber private val FACEBOOK_SCOPE = setOf("email", "public_profile") +private const val TAG = "FacebookAccountProvider" private const val PREFS_FACEBOOK_ACCOUNT_PROVIDER = "org.godtools.account.facebook" private const val PREF_USER_ID_PREFIX = "user_id_" @@ -88,22 +92,29 @@ internal class FacebookAccountProvider @Inject constructor( // endregion Login/Logout override suspend fun authenticateWithMobileContentApi(createUser: Boolean): Result { - var accessToken = accessTokenManager.currentAccessToken - ?: return Result.failure(AuthenticationException.MissingCredentials) - var resp = accessToken.authenticateWithMobileContentApi(createUser) + try { + var accessToken = accessTokenManager.currentAccessToken + ?: return Result.failure(AuthenticationException.MissingCredentials) + var resp = accessToken.authenticateWithMobileContentApi(createUser) - // try refreshing the access token if the API rejected it - if (!resp.isSuccessful) { - accessToken = try { - accessTokenManager.refreshCurrentAccessToken() - } catch (e: FacebookException) { - null - } ?: return Result.failure(AuthenticationException.UnableToRefreshCredentials) - resp = accessToken.authenticateWithMobileContentApi(createUser) - } + // try refreshing the access token if the API rejected it + if (!resp.isSuccessful) { + accessToken = try { + accessTokenManager.refreshCurrentAccessToken() + } catch (e: FacebookException) { + null + } ?: return Result.failure(AuthenticationException.UnableToRefreshCredentials) + resp = accessToken.authenticateWithMobileContentApi(createUser) + } - return resp.extractAuthToken() - .onSuccess { prefs.edit { putString(accessToken.PREF_USER_ID, it.userId) } } + return resp.extractAuthToken() + .onSuccess { prefs.edit { putString(accessToken.PREF_USER_ID, it.userId) } } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (e !is IOException) Timber.tag(TAG).e(e, "Unexpected error authenticating with Facebook") + return Result.failure(e) + } } private suspend fun AccessToken.authenticateWithMobileContentApi(createUser: Boolean) = diff --git a/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt b/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt index cedddf4552..e598cb6fa6 100644 --- a/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt +++ b/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt @@ -15,6 +15,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkStatic +import java.net.UnknownHostException import java.util.Date import java.util.UUID import kotlin.random.Random @@ -234,6 +235,20 @@ class FacebookAccountProviderTest { } } + @Test + fun `authenticateWithMobileContentApi() - Error - api throws UnknownHostException`() = runTest { + val accessToken = accessToken() + val createUser = Random.nextBoolean() + val exception = UnknownHostException() + currentAccessTokenFlow.value = accessToken + coEvery { api.authenticate(any()) } throws exception + + assertEquals(Result.failure(exception), provider.authenticateWithMobileContentApi(createUser)) + coVerifyAll { + api.authenticate(AuthToken.Request(fbAccessToken = accessToken.token, createUser = createUser)) + } + } + @Test fun `authenticateWithMobileContentApi() - Error - jsonapi errors`() = runTest { val accessToken = accessToken()