From a6fdb90838c84d3a31f1965babd6aba17706307e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 30 Oct 2024 09:39:44 +0100 Subject: [PATCH] Add quick fix in notification troubleshot test to perform a Firebase token rotation --- .../impl/troubleshoot/PushLoopbackTest.kt | 9 ++- .../impl/troubleshoot/PushLoopbackTestTest.kt | 37 +++++++++++++ .../pushproviders/api/PushProvider.kt | 6 ++ .../firebase/FirebasePushProvider.kt | 7 +++ .../firebase/FirebaseTokenDeleter.kt | 54 ++++++++++++++++++ .../firebase/FirebaseTokenGetter.kt | 55 +++++++++++++++++++ .../firebase/FirebaseTokenRotator.kt | 32 +++++++++++ .../firebase/FirebaseTroubleshooter.kt | 34 +----------- .../firebase/FakeFirebaseTokenRotator.kt | 18 ++++++ .../firebase/FirebasePushProviderTest.kt | 12 ++++ .../pushproviders/test/FakePushProvider.kt | 10 ++++ .../unifiedpush/UnifiedPushProvider.kt | 2 + 12 files changed, 243 insertions(+), 33 deletions(-) create mode 100644 libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenDeleter.kt create mode 100644 libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenGetter.kt create mode 100644 libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenRotator.kt create mode 100644 libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FakeFirebaseTokenRotator.kt diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt index e91f4351f7..57c255a406 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt @@ -52,9 +52,10 @@ class PushLoopbackTest @Inject constructor( val testPushResult = try { pushService.testPush() } catch (pusherRejected: PushGatewayFailure.PusherRejected) { + val hasQuickFix = pushService.getCurrentPushProvider()?.canRotateToken() == true delegate.updateState( description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_failure_1), - status = NotificationTroubleshootTestState.Status.Failure(false) + status = NotificationTroubleshootTestState.Status.Failure(hasQuickFix) ) job.cancel() return @@ -96,5 +97,11 @@ class PushLoopbackTest @Inject constructor( ) } + override suspend fun quickFix(coroutineScope: CoroutineScope) { + delegate.start() + pushService.getCurrentPushProvider()?.rotateToken() + run(coroutineScope) + } + override suspend fun reset() = delegate.reset() } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt index b12e0cf80b..57ba07a5f6 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt @@ -13,9 +13,11 @@ import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_FAILURE_REASON import io.element.android.libraries.push.api.gateway.PushGatewayFailure import io.element.android.libraries.push.test.FakePushService +import io.element.android.libraries.pushproviders.test.FakePushProvider import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootTestState import io.element.android.services.toolbox.test.strings.FakeStringProvider import io.element.android.services.toolbox.test.systemclock.FakeSystemClock +import io.element.android.tests.testutils.lambda.lambdaRecorder import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import org.junit.Test @@ -67,6 +69,41 @@ class PushLoopbackTestTest { } } + @Test + fun `test PushLoopbackTest PusherRejected error with quick fix`() = runTest { + val diagnosticPushHandler = DiagnosticPushHandler() + val rotateTokenLambda = lambdaRecorder> { Result.success(Unit) } + val sut = PushLoopbackTest( + pushService = FakePushService( + testPushBlock = { + throw PushGatewayFailure.PusherRejected() + }, + currentPushProvider = { + FakePushProvider( + canRotateTokenResult = { true }, + rotateTokenLambda = rotateTokenLambda, + ) + } + ), + diagnosticPushHandler = diagnosticPushHandler, + clock = FakeSystemClock(), + stringProvider = FakeStringProvider(), + ) + launch { + sut.run(this) + } + sut.state.test { + assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true)) + assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.InProgress) + val lastItem = awaitItem() + assertThat(lastItem.status).isEqualTo(NotificationTroubleshootTestState.Status.Failure(true)) + sut.quickFix(this) + assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.InProgress) + assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Failure(true)) + rotateTokenLambda.assertions().isCalledOnce() + } + } + @Test fun `test PushLoopbackTest setup error`() = runTest { val diagnosticPushHandler = DiagnosticPushHandler() diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt index 0f3cde8033..90b4cb0465 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt @@ -44,4 +44,10 @@ interface PushProvider { suspend fun unregister(matrixClient: MatrixClient): Result suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? + + fun canRotateToken(): Boolean + + suspend fun rotateToken(): Result { + error("rotateToken() not implemented, you need to override this method in your implementation") + } } diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt index 6bbdb1efe8..b1e31a2303 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt @@ -25,6 +25,7 @@ class FirebasePushProvider @Inject constructor( private val firebaseStore: FirebaseStore, private val pusherSubscriber: PusherSubscriber, private val isPlayServiceAvailable: IsPlayServiceAvailable, + private val firebaseTokenRotator: FirebaseTokenRotator, ) : PushProvider { override val index = FirebaseConfig.INDEX override val name = FirebaseConfig.NAME @@ -71,6 +72,12 @@ class FirebasePushProvider @Inject constructor( } } + override fun canRotateToken(): Boolean = true + + override suspend fun rotateToken(): Result { + return firebaseTokenRotator.rotate() + } + companion object { private val firebaseDistributor = Distributor("Firebase", "Firebase") } diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenDeleter.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenDeleter.kt new file mode 100644 index 0000000000..db7ac0bb7c --- /dev/null +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenDeleter.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2023, 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.firebase + +import com.google.firebase.messaging.FirebaseMessaging +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import timber.log.Timber +import javax.inject.Inject +import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException +import kotlin.coroutines.suspendCoroutine + +interface FirebaseTokenDeleter { + suspend fun delete() +} + +/** + * This class deletes the current Firebase token. + */ +@ContributesBinding(AppScope::class) +class DefaultFirebaseTokenDeleter @Inject constructor( + private val isPlayServiceAvailable: IsPlayServiceAvailable, +) : FirebaseTokenDeleter { + override suspend fun delete() { + suspendCoroutine { continuation -> + // 'app should always check the device for a compatible Google Play services APK before accessing Google Play services features' + if (isPlayServiceAvailable.isAvailable()) { + try { + FirebaseMessaging.getInstance().deleteToken() + .addOnSuccessListener { + continuation.resume(Unit) + } + .addOnFailureListener { e -> + Timber.e(e, "## deleteFirebaseToken() : failed") + continuation.resumeWithException(e) + } + } catch (e: Throwable) { + Timber.e(e, "## deleteFirebaseToken() : failed") + continuation.resumeWithException(e) + } + } else { + val e = Exception("No valid Google Play Services found. Cannot use FCM.") + Timber.e(e) + throw e + } + } + } +} diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenGetter.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenGetter.kt new file mode 100644 index 0000000000..b120687f9a --- /dev/null +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenGetter.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2023, 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.firebase + +import com.google.firebase.messaging.FirebaseMessaging +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import timber.log.Timber +import javax.inject.Inject +import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException +import kotlin.coroutines.suspendCoroutine + +interface FirebaseTokenGetter { + suspend fun get(): String +} + +/** + * This class read the current Firebase token. + * If the token does not exist, it will be generated. + */ +@ContributesBinding(AppScope::class) +class DefaultFirebaseTokenGetter @Inject constructor( + private val isPlayServiceAvailable: IsPlayServiceAvailable, +) : FirebaseTokenGetter { + override suspend fun get(): String { + return suspendCoroutine { continuation -> + // 'app should always check the device for a compatible Google Play services APK before accessing Google Play services features' + if (isPlayServiceAvailable.isAvailable()) { + try { + FirebaseMessaging.getInstance().token + .addOnSuccessListener { token -> + continuation.resume(token) + } + .addOnFailureListener { e -> + Timber.e(e, "## retrievedFirebaseToken() : failed") + continuation.resumeWithException(e) + } + } catch (e: Throwable) { + Timber.e(e, "## retrievedFirebaseToken() : failed") + continuation.resumeWithException(e) + } + } else { + val e = Exception("No valid Google Play Services found. Cannot use FCM.") + Timber.e(e) + continuation.resumeWithException(e) + } + } + } +} diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenRotator.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenRotator.kt new file mode 100644 index 0000000000..fc55d11a77 --- /dev/null +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTokenRotator.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.firebase + +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import javax.inject.Inject + +interface FirebaseTokenRotator { + suspend fun rotate(): Result +} + +/** + * This class delete the Firebase token and generate a new one. + */ +@ContributesBinding(AppScope::class) +class DefaultFirebaseTokenRotator @Inject constructor( + private val firebaseTokenDeleter: FirebaseTokenDeleter, + private val firebaseTokenGetter: FirebaseTokenGetter, +) : FirebaseTokenRotator { + override suspend fun rotate(): Result { + return runCatching { + firebaseTokenDeleter.delete() + firebaseTokenGetter.get() + } + } +} diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTroubleshooter.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTroubleshooter.kt index 6ef6931ea1..6991da5f25 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTroubleshooter.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseTroubleshooter.kt @@ -7,14 +7,9 @@ package io.element.android.libraries.pushproviders.firebase -import com.google.firebase.messaging.FirebaseMessaging import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope -import timber.log.Timber import javax.inject.Inject -import kotlin.coroutines.resume -import kotlin.coroutines.resumeWithException -import kotlin.coroutines.suspendCoroutine interface FirebaseTroubleshooter { suspend fun troubleshoot(): Result @@ -26,37 +21,12 @@ interface FirebaseTroubleshooter { @ContributesBinding(AppScope::class) class DefaultFirebaseTroubleshooter @Inject constructor( private val newTokenHandler: FirebaseNewTokenHandler, - private val isPlayServiceAvailable: IsPlayServiceAvailable, + private val firebaseTokenGetter: FirebaseTokenGetter, ) : FirebaseTroubleshooter { override suspend fun troubleshoot(): Result { return runCatching { - val token = retrievedFirebaseToken() + val token = firebaseTokenGetter.get() newTokenHandler.handle(token) } } - - private suspend fun retrievedFirebaseToken(): String { - return suspendCoroutine { continuation -> - // 'app should always check the device for a compatible Google Play services APK before accessing Google Play services features' - if (isPlayServiceAvailable.isAvailable()) { - try { - FirebaseMessaging.getInstance().token - .addOnSuccessListener { token -> - continuation.resume(token) - } - .addOnFailureListener { e -> - Timber.e(e, "## retrievedFirebaseToken() : failed") - continuation.resumeWithException(e) - } - } catch (e: Throwable) { - Timber.e(e, "## retrievedFirebaseToken() : failed") - continuation.resumeWithException(e) - } - } else { - val e = Exception("No valid Google Play Services found. Cannot use FCM.") - Timber.e(e) - continuation.resumeWithException(e) - } - } - } } diff --git a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FakeFirebaseTokenRotator.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FakeFirebaseTokenRotator.kt new file mode 100644 index 0000000000..c96c67c3fa --- /dev/null +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FakeFirebaseTokenRotator.kt @@ -0,0 +1,18 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.firebase + +import io.element.android.tests.testutils.lambda.lambdaError + +class FakeFirebaseTokenRotator( + private val rotateWithResult: () -> Result = { lambdaError() } +) : FirebaseTokenRotator { + override suspend fun rotate(): Result { + return rotateWithResult() + } +} diff --git a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt index f94f4ebac8..6a3a9474e4 100644 --- a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt @@ -166,15 +166,27 @@ class FirebasePushProviderTest { assertThat(result).isEqualTo(CurrentUserPushConfig(FirebaseConfig.PUSHER_HTTP_URL, "aToken")) } + @Test + fun `rotateToken invokes the FirebaseTokenRotator`() = runTest { + val lambda = lambdaRecorder> { Result.success(Unit) } + val firebasePushProvider = createFirebasePushProvider( + firebaseTokenRotator = FakeFirebaseTokenRotator(lambda), + ) + firebasePushProvider.rotateToken() + lambda.assertions().isCalledOnce() + } + private fun createFirebasePushProvider( firebaseStore: FirebaseStore = InMemoryFirebaseStore(), pusherSubscriber: PusherSubscriber = FakePusherSubscriber(), isPlayServiceAvailable: IsPlayServiceAvailable = FakeIsPlayServiceAvailable(false), + firebaseTokenRotator: FirebaseTokenRotator = FakeFirebaseTokenRotator(), ): FirebasePushProvider { return FirebasePushProvider( firebaseStore = firebaseStore, pusherSubscriber = pusherSubscriber, isPlayServiceAvailable = isPlayServiceAvailable, + firebaseTokenRotator = firebaseTokenRotator, ) } } diff --git a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt index 9fe222e1cb..6f5bdda5b7 100644 --- a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt +++ b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt @@ -21,6 +21,8 @@ class FakePushProvider( private val currentUserPushConfig: CurrentUserPushConfig? = null, private val registerWithResult: (MatrixClient, Distributor) -> Result = { _, _ -> lambdaError() }, private val unregisterWithResult: (MatrixClient) -> Result = { lambdaError() }, + private val canRotateTokenResult: () -> Boolean = { lambdaError() }, + private val rotateTokenLambda: () -> Result = { lambdaError() }, ) : PushProvider { override fun getDistributors(): List = distributors @@ -39,4 +41,12 @@ class FakePushProvider( override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return currentUserPushConfig } + + override fun canRotateToken(): Boolean { + return canRotateTokenResult() + } + + override suspend fun rotateToken(): Result { + return rotateTokenLambda() + } } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt index 9a2d5d5c10..1ce929a75b 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt @@ -53,4 +53,6 @@ class UnifiedPushProvider @Inject constructor( override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return unifiedPushCurrentUserPushConfigProvider.provide() } + + override fun canRotateToken(): Boolean = false }