Skip to content

Commit

Permalink
Merge pull request #3823 from CruGlobal/GT-2487-Handle-IOExceptions-D…
Browse files Browse the repository at this point in the history 
…uring-Login

GT-2487 Handle unexpected exceptions thrown during authentication
  • Loading branch information
@frett
frett authored Dec 17, 2024
2 parents 9a1eaea + 06f6333 commit 4094345
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 26 deletions.
39 changes: 25 additions & 14 deletions ...unt/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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_"

Expand Down Expand Up @@ -88,22 +92,29 @@ internal class FacebookAccountProvider @Inject constructor(
// endregion Login/Logout

override suspend fun authenticateWithMobileContentApi(createUser: Boolean): Result<AuthToken> {
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) =
Expand Down
29 changes: 19 additions & 10 deletions ...account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -96,18 +98,25 @@ internal class GoogleAccountProvider @Inject constructor(
// endregion Login/Logout

override suspend fun authenticateWithMobileContentApi(createUser: Boolean): Result<AuthToken> {
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) =
Expand Down
15 changes: 15 additions & 0 deletions ...src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 25 additions & 2 deletions ...unt/src/test/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProviderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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<GoogleSignInAccount?>(null)
private val userId = UUID.randomUUID().toString()
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4094345

Please sign in to comment.