diff --git a/app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt b/app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt index 8e7f9ece70c..649fb7c6e6b 100644 --- a/app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt @@ -113,6 +113,11 @@ class BrowserLoginActivity : BaseActivity() { logger.e(TAG, "Post login step failed") Snackbar.make(binding.root, R.string.nc_common_error_sorry, Snackbar.LENGTH_SHORT).show() } + BrowserLoginActivityViewModel.PostLoginViewState.PostLoginTooManyLoginAttempts -> { + logger.e(TAG, "Login refused because of too many failed logins") + Snackbar.make(binding.root, R.string.nc_login_too_many_attempts, Snackbar.LENGTH_LONG) + .show() + } BrowserLoginActivityViewModel.PostLoginViewState.PostLoginRestartApp -> { restartApp() } diff --git a/app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt b/app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt index c9a3bf05d66..a51604352aa 100644 --- a/app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt @@ -31,6 +31,9 @@ class LoginRepository(val network: NetworkLoginDataSource, val local: LocalLogin val TAG: String = LoginRepository::class.java.simpleName private const val INTERVAL = 250L private const val HTTP_OK = 200 + + /** The status of a [LoginCompletion] for a login the server refused because of too many failed logins. */ + const val HTTP_TOO_MANY_REQUESTS = 429 private const val USER_KEY = "user:" private const val SERVER_KEY = "server:" private const val PASS_KEY = "password:" @@ -171,7 +174,12 @@ class LoginRepository(val network: NetworkLoginDataSource, val local: LocalLogin // Need to use the qr code token to create temporary credentials to get access to the actual app password val credentials = Credentials.basic(loginName, appPassword) - val oneTimePassword = network.oneTimePasswordRequest(server, credentials) + val oneTimePassword = try { + network.oneTimePasswordRequest(server, credentials) + } catch (e: NetworkLoginDataSource.TooManyLoginAttemptsException) { + Log.w(TAG, "Server refused the one-time login because of too many failed logins", e) + return@withContext LoginCompletion(HTTP_TOO_MANY_REQUESTS, server, loginName, "") + } return@withContext if (server.isNotEmpty() && loginName.isNotEmpty() && oneTimePassword != null) { LoginCompletion(HTTP_OK, server, loginName, oneTimePassword) diff --git a/app/src/main/java/com/nextcloud/talk/account/data/network/NetworkLoginDataSource.kt b/app/src/main/java/com/nextcloud/talk/account/data/network/NetworkLoginDataSource.kt index f8315913006..2cdfc674d54 100644 --- a/app/src/main/java/com/nextcloud/talk/account/data/network/NetworkLoginDataSource.kt +++ b/app/src/main/java/com/nextcloud/talk/account/data/network/NetworkLoginDataSource.kt @@ -26,8 +26,19 @@ class NetworkLoginDataSource(val okHttpClient: OkHttpClient) { companion object { val TAG: String = NetworkLoginDataSource::class.java.simpleName + private const val HTTP_TOO_MANY_REQUESTS = 429 } + /** + * The server refused a login because of too many failed logins from this IP, without checking the credentials. + */ + class TooManyLoginAttemptsException(message: String) : IOException(message) + + /** + * Exchanges the one-time password of [oneTimeCredentials] for an app password, or returns null if that failed. + * + * @throws TooManyLoginAttemptsException if the server refused the login, the one-time password is still unused then + */ fun oneTimePasswordRequest(baseUrl: String, oneTimeCredentials: String): String? { val url = "$baseUrl/ocs/v2.php/core/getapppassword-onetime" var result: String? = null @@ -42,6 +53,8 @@ class NetworkLoginDataSource(val okHttpClient: OkHttpClient) { result = appPassword }.getOrElse { e -> when (e) { + is TooManyLoginAttemptsException -> throw e + is SSLHandshakeException, is NullPointerException, is IOException -> { @@ -66,6 +79,9 @@ class NetworkLoginDataSource(val okHttpClient: OkHttpClient) { val newOkHttpClient = OkHttpClient() newOkHttpClient.newCall(request).execute().use { response -> + if (response.code == HTTP_TOO_MANY_REQUESTS) { + throw TooManyLoginAttemptsException("Too many failed logins from this IP: $response") + } if (!response.isSuccessful) { throw IOException("Unexpected code $response") } diff --git a/app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt b/app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt index 481719fe076..9ab868f8c02 100644 --- a/app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt @@ -11,6 +11,7 @@ import android.os.Bundle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.nextcloud.talk.account.data.LoginRepository +import com.nextcloud.talk.account.data.model.LoginCompletion import com.nextcloud.talk.account.data.model.LoginResponse import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -37,6 +38,7 @@ class BrowserLoginActivityViewModel @Inject constructor(val repository: LoginRep data object None : PostLoginViewState() data object PostLoginRestartApp : PostLoginViewState() data object PostLoginError : PostLoginViewState() + data object PostLoginTooManyLoginAttempts : PostLoginViewState() data class PostLoginContinue(val data: Bundle) : PostLoginViewState() data object PostLoginDifferentAccount : PostLoginViewState() } @@ -83,14 +85,7 @@ class BrowserLoginActivityViewModel @Inject constructor(val repository: LoginRep fun handleWebBrowserLogin() { savedResponse?.let { response -> viewModelScope.launch { - val loginCompletionResponse = repository.pollLogin(response) - - if (loginCompletionResponse == null) { - _postLoginState.value = PostLoginViewState.PostLoginError - return@launch - } - - _postLoginState.value = postLoginStateFor(repository.parseAndLogin(loginCompletionResponse)) + _postLoginState.value = postLoginStateFor(repository.pollLogin(response)) } } } @@ -98,29 +93,29 @@ class BrowserLoginActivityViewModel @Inject constructor(val repository: LoginRep fun loginWithQR(dataString: String, reAuth: Boolean = false, accountToReauthorize: Long? = null) { if (!startLoginOnce()) return viewModelScope.launch { - val loginCompletionResponse = repository.startLoginFlowFromQR(dataString, reAuth, accountToReauthorize) - if (loginCompletionResponse == null) { - _postLoginState.value = PostLoginViewState.PostLoginError - return@launch - } - - _postLoginState.value = postLoginStateFor(repository.parseAndLogin(loginCompletionResponse)) + _postLoginState.value = + postLoginStateFor(repository.startLoginFlowFromQR(dataString, reAuth, accountToReauthorize)) } } fun loginWithOTPQR(dataString: String, reAuth: Boolean = false, accountToReauthorize: Long? = null) { if (!startLoginOnce()) return viewModelScope.launch { - val loginCompletionResponse = repository.startOTPLoginFlow(dataString, reAuth, accountToReauthorize) - if (loginCompletionResponse == null) { - _postLoginState.value = PostLoginViewState.PostLoginError - return@launch - } - - _postLoginState.value = postLoginStateFor(repository.parseAndLogin(loginCompletionResponse)) + _postLoginState.value = + postLoginStateFor(repository.startOTPLoginFlow(dataString, reAuth, accountToReauthorize)) } } + private suspend fun postLoginStateFor(loginCompletion: LoginCompletion?): PostLoginViewState = + when { + loginCompletion == null -> PostLoginViewState.PostLoginError + + loginCompletion.status == LoginRepository.HTTP_TOO_MANY_REQUESTS -> + PostLoginViewState.PostLoginTooManyLoginAttempts + + else -> postLoginStateFor(repository.parseAndLogin(loginCompletion)) + } + private fun postLoginStateFor(result: LoginRepository.LoginResult): PostLoginViewState = when (result) { is LoginRepository.LoginResult.NewAccount -> PostLoginViewState.PostLoginContinue(result.bundle) diff --git a/app/src/main/java/com/nextcloud/talk/activities/BaseActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/BaseActivity.kt index d6a988aab69..40df531f44c 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/BaseActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/BaseActivity.kt @@ -23,7 +23,6 @@ import android.view.WindowManager import android.view.inputmethod.EditorInfo import android.webkit.SslErrorHandler import android.widget.EditText -import android.widget.Toast import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatActivity import androidx.core.content.res.ResourcesCompat @@ -41,7 +40,6 @@ import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.chat.ChatActivity import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.events.CertificateEvent -import com.nextcloud.talk.events.RemoteWipeEvent import com.nextcloud.talk.lock.LockedActivity import com.nextcloud.talk.users.DefaultAccountProvider import com.nextcloud.talk.users.UserManager @@ -402,14 +400,6 @@ open class BaseActivity : AppCompatActivity() { showCertificateDialog(event.x509Certificate, event.trustManager, event.sslErrorHandler) } - @Subscribe(threadMode = ThreadMode.MAIN) - fun onRemoteWipeEvent(event: RemoteWipeEvent) { - Toast.makeText(context, R.string.nc_remote_wipe_logged_out, Toast.LENGTH_LONG).show() - val intent = Intent(this, MainActivity::class.java) - intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) - startActivity(intent) - } - override fun startActivity(intent: Intent) { // Links to the own server are opened for the account of this screen. Screens without an account (boundUser // never loaded) use the default account, without resolving one from their intent. diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 751af01da8c..5b871c2ce3e 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -30,7 +30,6 @@ import com.nextcloud.talk.models.json.chat.ChatOverallSingleMessage import com.nextcloud.talk.models.json.converters.EnumActorTypeConverter import com.nextcloud.talk.models.json.generic.GenericOverall import com.nextcloud.talk.models.json.participants.ParticipantDto -import com.nextcloud.talk.utils.bundle.BundleKeys import com.nextcloud.talk.utils.message.SendMessageUtils import com.nextcloud.talk.utils.revertOnCancellation import com.nextcloud.talk.utils.withRetry @@ -42,6 +41,7 @@ import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first @@ -52,9 +52,11 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.take import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull import retrofit2.HttpException import java.io.IOException import javax.inject.Inject +import kotlin.math.min @Suppress("LargeClass", "TooManyFunctions") class OfflineFirstChatRepository @Inject constructor( @@ -303,18 +305,25 @@ class OfflineFirstChatRepository @Inject constructor( lastKnown = initialMessageId.toInt() ) - val networkParams = Bundle() + var failureDelayMillis = LONG_POLLING_FAILURE_INITIAL_DELAY while (true) { if (!networkMonitor.isOnline.value || itIsPaused) { + failureDelayMillis = LONG_POLLING_FAILURE_INITIAL_DELAY delay(HALF_SECOND) } else { // sync database with server // (This is a long blocking call because long polling (lookIntoFuture and timeout) is set) - networkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - Log.d(TAG, "Starting online request for long polling") - getAndPersistMessages(networkParams) + val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) + + // A failed request returns right away, e.g. with rejected credentials, which the server counts as a + // failed login. Without waiting, the next requests would follow at once until the server throttles. + failureDelayMillis = if (outcome.syncFailed) { + waitAfterFailedRequest(failureDelayMillis) + } else { + LONG_POLLING_FAILURE_INITIAL_DELAY + } val newestMessage = chatBlocksDao.getNewestMessageIdFromChatBlocks( internalConversationId, @@ -332,6 +341,24 @@ class OfflineFirstChatRepository @Inject constructor( } } + /** + * Waits [delayMillis] after a failed long polling request, and returns how long to wait if the next one fails too. + * Requests that failed while the device went offline should not delay the next one, so the wait ends early once + * it is online again. + */ + private suspend fun waitAfterFailedRequest(delayMillis: Long): Long { + Log.d(TAG, "Long polling failed, next request in $delayMillis ms") + val gotOnlineAgain = withTimeoutOrNull(delayMillis) { + networkMonitor.isOnline.drop(1).first { isOnline -> isOnline } + } != null + + return if (gotOnlineAgain) { + LONG_POLLING_FAILURE_INITIAL_DELAY + } else { + min(delayMillis * 2, LONG_POLLING_FAILURE_MAX_DELAY) + } + } + suspend fun initInsuranceRequests() { Log.d(TAG, "---- initInsuranceRequests ------------") @@ -563,16 +590,6 @@ class OfflineFirstChatRepository @Inject constructor( } } - // Callers must put a KEY_FIELD_MAP (see getFieldMap/syncer.buildFieldMap) into bundle before - // calling this. - private suspend fun getAndPersistMessages(bundle: Bundle): Boolean { - val fieldMap = requireNotNull(bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as? HashMap) { - "getAndPersistMessages requires bundle to carry KEY_FIELD_MAP" - } - val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) - return outcome.persistedNewMessages - } - private fun isUntranslatedSystemMessage(messagesJson: List): Boolean = syncer.isUntranslatedSystemMessage(messagesJson) @@ -1273,6 +1290,8 @@ class OfflineFirstChatRepository @Inject constructor( companion object { val TAG: String = OfflineFirstChatRepository::class.java.simpleName private const val HALF_SECOND = 500L + private const val LONG_POLLING_FAILURE_INITIAL_DELAY = 1_000L + private const val LONG_POLLING_FAILURE_MAX_DELAY = 60_000L private const val DEFAULT_MESSAGES_LIMIT = 100 private const val MILLIES = 1000L private const val INSURANCE_REQUEST_DELAY = 2 * 60 * MILLIES diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt index 368c1c7a2c4..80645a8124e 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt @@ -14,6 +14,7 @@ import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.pm.PackageManager +import android.database.SQLException import android.net.Uri import android.os.Build import android.os.Bundle @@ -70,6 +71,7 @@ import com.nextcloud.talk.jobs.AccountRemovalWorker import com.nextcloud.talk.jobs.ContactAddressBookWorker.Companion.run import com.nextcloud.talk.jobs.DeleteConversationWorker import com.nextcloud.talk.jobs.LeaveConversationWorker +import com.nextcloud.talk.jobs.RemoteWipeSuccessWorker import com.nextcloud.talk.jobs.UploadAndShareFilesWorker import com.nextcloud.talk.models.domain.ConversationModel import com.nextcloud.talk.models.domain.SearchMessageEntry @@ -93,6 +95,7 @@ import com.nextcloud.talk.utils.FileUtils import com.nextcloud.talk.utils.Mimetype import com.nextcloud.talk.utils.NotificationUtils import com.nextcloud.talk.utils.ParticipantPermissions +import com.nextcloud.talk.utils.RemoteWipeHandler import com.nextcloud.talk.utils.ShareUtils import com.nextcloud.talk.utils.ShortcutManagerHelper import com.nextcloud.talk.utils.SpreedFeatures @@ -119,6 +122,7 @@ import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder import java.util.concurrent.TimeUnit import javax.inject.Inject import kotlinx.coroutines.delay +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach @@ -155,6 +159,9 @@ class ConversationsListActivity : BaseActivity() { @Inject lateinit var contactsViewModelFactory: ContactsViewModel.Factory + @Inject + lateinit var remoteWipeHandler: RemoteWipeHandler + val contactsViewModel: ContactsViewModel by assistedViewModels { contactsViewModelFactory.build(currentUser) } val conversationsListViewModel: ConversationsListViewModel by assistedViewModels { @@ -193,6 +200,16 @@ class ConversationsListActivity : BaseActivity() { private var selectedMessageId: String? = null private var pendingDirectShareToken: String? = null private var isDirectShareTarget = false + private var unauthorizedDialog: AlertDialog? = null + private var unauthorizedHandling: Job? = null + + // The list syncs again on every resume, e.g. after the unauthorized dialog was closed, while the server answers + // only 10 wipe checks per 5 minutes from an IP and further ones with 429. So the server is asked once per list. + private var isWipeChecked = false + + // From starting to remove the account of this list until that ended. Meanwhile the account is not to be + // reauthorized or removed again. + private var isRemovingAccount = false lateinit var ecosystemManager: EcosystemManager @@ -522,7 +539,7 @@ class ConversationsListActivity : BaseActivity() { // Update Direct Share targets lifecycleScope.launch { - DirectShareHelper.publishShareTargetShortcuts(context, currentUser, list) + DirectShareHelper.publishShareTargetShortcuts(this@ConversationsListActivity, currentUser, list) } // check for Direct Share @@ -749,7 +766,7 @@ class ConversationsListActivity : BaseActivity() { if (throwable is HttpException) { when (throwable.code()) { - HTTP_UNAUTHORIZED -> showUnauthorizedDialog() + HTTP_UNAUTHORIZED -> handleUnauthorized() HTTP_CLIENT_UPGRADE_REQUIRED -> showOutdatedClientDialog() HTTP_SERVICE_UNAVAILABLE -> showServiceUnavailableDialog(throwable) else -> { @@ -1401,6 +1418,26 @@ class ConversationsListActivity : BaseActivity() { ) } + /** + * The server rejected the credentials of the account, which it also does once it requested a wipe of it. So the + * account is wiped if so, and otherwise the user can reauthorize or remove it. + */ + private fun handleUnauthorized() { + if (isRemovingAccount || unauthorizedHandling?.isActive == true || unauthorizedDialog?.isShowing == true) { + return + } + + unauthorizedHandling = lifecycleScope.launch { + val isWipeRequested = !isWipeChecked && remoteWipeHandler.isWipeRequested(currentUser) + isWipeChecked = true + if (isWipeRequested) { + removeAccountAndRestartApp(wipeToken = currentUser.token) + } else { + showUnauthorizedDialog() + } + } + } + private fun showUnauthorizedDialog() { val dialogBuilder = MaterialAlertDialogBuilder(this) .setIcon( @@ -1413,7 +1450,7 @@ class ConversationsListActivity : BaseActivity() { .setMessage(R.string.nc_dialog_reauth_or_delete) .setCancelable(false) .setPositiveButton(R.string.nc_settings_remove_account) { _, _ -> - deleteUserAndRestartApp() + removeAccountAndRestartApp() } .setNegativeButton(R.string.nc_settings_reauthorize) { _, _ -> val intent = Intent(context, BrowserLoginActivity::class.java) @@ -1427,38 +1464,58 @@ class ConversationsListActivity : BaseActivity() { viewThemeUtils.dialog.colorMaterialAlertDialogBackground(this, dialogBuilder) val dialog = dialogBuilder.show() + unauthorizedDialog = dialog viewThemeUtils.platform.colorTextButtons( dialog.getButton(AlertDialog.BUTTON_POSITIVE), dialog.getButton(AlertDialog.BUTTON_NEGATIVE) ) } - private fun deleteUserAndRestartApp() { + /** + * Removes the account of this list and restarts the app once it is removed. With [wipeToken], the account is + * removed because its server requested a wipe, which is reported to the server with that token afterwards. + */ + private fun removeAccountAndRestartApp(wipeToken: String? = null) { + isRemovingAccount = true lifecycleScope.launch { - userManager.scheduleUserForDeletionWithId(currentUser.id!!) val accountRemovalWork = OneTimeWorkRequest.Builder(AccountRemovalWorker::class.java) .setExpeditedIfSupported() .build() - WorkManager.getInstance(applicationContext).enqueue(accountRemovalWork) + try { + userManager.scheduleUserForDeletionWithId(currentUser.id!!) + val removal = WorkManager.getInstance(applicationContext).beginWith(accountRemovalWork) + if (wipeToken == null) { + removal.enqueue() + } else { + removal.then(RemoteWipeSuccessWorker.workRequest(currentUser.baseUrl!!, wipeToken)).enqueue() + } + } catch (e: SQLException) { + showAccountRemovalFailed(e) + return@launch + } catch (e: IllegalStateException) { + showAccountRemovalFailed(e) + return@launch + } WorkManager.getInstance(context).getWorkInfoByIdLiveData(accountRemovalWork.id) .observeForever { workInfo: WorkInfo? -> when (workInfo?.state) { WorkInfo.State.SUCCEEDED -> { - val text = String.format( - context.resources.getString(R.string.nc_deleted_user), - currentUser.displayName - ) - Toast.makeText( - context, - text, - Toast.LENGTH_LONG - ).show() + val text = if (wipeToken == null) { + String.format( + context.resources.getString(R.string.nc_deleted_user), + currentUser.displayName + ) + } else { + context.resources.getString(R.string.nc_remote_wipe_logged_out) + } + Toast.makeText(context, text, Toast.LENGTH_LONG).show() restartApp() } WorkInfo.State.FAILED, WorkInfo.State.CANCELLED -> { + isRemovingAccount = false logger.e(TAG, "something went wrong when deleting user with id " + currentUser.userId) Toast.makeText( @@ -1475,6 +1532,12 @@ class ConversationsListActivity : BaseActivity() { } } + private fun showAccountRemovalFailed(e: Exception) { + isRemovingAccount = false + logger.e(TAG, "Failed to remove user with id " + currentUser.userId, e) + Toast.makeText(context, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() + } + private fun restartApp() { val intent = Intent(context, MainActivity::class.java) intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) @@ -1543,7 +1606,7 @@ class ConversationsListActivity : BaseActivity() { .setMessage(R.string.nc_settings_server_eol) .setCancelable(false) .setPositiveButton(R.string.nc_settings_remove_account) { _, _ -> - deleteUserAndRestartApp() + removeAccountAndRestartApp() } if (resources!!.getBoolean(R.bool.multiaccount_support) && runBlocking { userManager.getUsers() }.size > 1) { diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/DirectShareHelper.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/DirectShareHelper.kt index 088de5cf23b..59d4700236e 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/DirectShareHelper.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/DirectShareHelper.kt @@ -10,6 +10,7 @@ package com.nextcloud.talk.conversationlist import android.content.Context import android.content.Intent +import android.content.res.Configuration import android.graphics.drawable.BitmapDrawable import android.util.Log import androidx.core.app.Person @@ -20,11 +21,16 @@ import coil.imageLoader import coil.request.ImageRequest import coil.transform.CircleCropTransformation import com.nextcloud.talk.R +import com.nextcloud.talk.conversationlist.ui.AvatarContent +import com.nextcloud.talk.conversationlist.ui.buildAvatarContent import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.domain.ConversationModel import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.AvatarImageLoader import com.nextcloud.talk.utils.ConversationUtils import com.nextcloud.talk.utils.bundle.BundleKeys +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock object DirectShareHelper { @@ -34,9 +40,22 @@ object DirectShareHelper { private const val SHORTCUT_ID_MIN_PARTS = 4 private const val AVATAR_SIZE_PX = 256 + private val publishMutex = Mutex() + private enum class MessageDirection { NONE, SEND, RECEIVE } + /** + * Publishes the most recent [conversations] as direct share shortcuts, with the avatars the conversation list + * shows. [context] must be the activity of the list: the application context does not follow the theme setting + * of the app, so it would look for the avatars of the other theme. + */ suspend fun publishShareTargetShortcuts(context: Context, user: User, conversations: List) { + // One publication at a time, as each loads its avatars one after another, see loadListAvatarIcon. The list + // publishes them with every change, so another publication can start while one is still loading. + publishMutex.withLock { publish(context, user, conversations) } + } + + private suspend fun publish(context: Context, user: User, conversations: List) { val maxShortcuts = ShortcutManagerCompat.getMaxShortcutCountPerActivity(context) // Preserve shortcuts not managed by DirectShareHelper (e.g. Note to Self). @@ -55,9 +74,10 @@ object DirectShareHelper { val credentials = ApiUtils.getCredentials(user.username, user.token) // Build shortcuts most-recent-first; setDynamicShortcuts uses index order (0 = most important). + // The avatars are loaded one after another, see loadListAvatarIcon. val shareShortcuts = candidates.map { conversation -> val displayName = conversation.displayName - val icon = loadAvatarIcon(context, user, conversation.token, displayName, credentials) + val icon = loadListAvatarIcon(context, user, conversation, credentials) prepShortcutBuilder(context, user, conversation.token, displayName, icon).build() } @@ -155,6 +175,45 @@ object DirectShareHelper { private fun shortcutId(user: User, token: String): String = "$SHORTCUT_ID_PREFIX${user.id}_$token" + /** + * The avatar of [conversation] as the conversation list shows it. It is loaded with the same URL and cache key as + * in the list, so the avatars the list loaded come from the cache, and the ones loaded here are cached for the + * list. + * + * The shortcuts are published again with every change of the conversation list, so they must load their avatars + * one after another: while the server rejects the credentials, every request with them counts as a failed login. + * After the first one got its 401, RejectedCredentialsInterceptor does not send the following ones. + */ + private suspend fun loadListAvatarIcon( + context: Context, + user: User, + conversation: ConversationModel, + credentials: String? + ): IconCompat { + val isDark = context.resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK == + Configuration.UI_MODE_NIGHT_YES + + return when (val avatar = buildAvatarContent(conversation, user, isDark)) { + is AvatarContent.Url -> { + val imageLoader = if (avatar.versioned) AvatarImageLoader.get(context) else context.imageLoader + val request = ImageRequest.Builder(context) + .data(avatar.url) + .diskCacheKey(avatar.diskCacheKey) + .apply { credentials?.let { addHeader("Authorization", it) } } + .size(AVATAR_SIZE_PX) + .allowHardware(false) + .transformations(CircleCropTransformation()) + .build() + val bitmap = (imageLoader.execute(request).drawable as? BitmapDrawable)?.bitmap + bitmap?.let { IconCompat.createWithBitmap(it) } ?: defaultIcon(context) + } + + is AvatarContent.Res -> IconCompat.createWithResource(context, avatar.resId) + + else -> defaultIcon(context) + } + } + private suspend fun loadAvatarIcon( context: Context, user: User, diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt index 4f7f4af0003..ca61696efc0 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt @@ -29,9 +29,16 @@ interface OfflineConversationsRepository { * slow network) while there are no locally cached conversations to fall back on for that * account, so the UI can tell the user why the list is empty instead of failing silently. * A failed sync while conversations are already cached does not emit here, since - * [roomListFlow] already has data to show and the sync is a best-effort background refresh. + * [roomListFlow] already has data to show and the sync is a best-effort background refresh, + * unless the server rejected the credentials, which the user has to act on. + * + * The repository is shared by all accounts and a sync can outlive the screen that started it, + * so each error carries the account it belongs to. */ - val syncErrorFlow: Flow + val syncErrorFlow: Flow + + /** A failed sync of the conversations of the account with [accountId]. */ + data class SyncError(val accountId: Long, val throwable: Throwable) /** * Stream of a single conversation, for use in each conversations settings. diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 6fec011d58f..dd477782797 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -50,6 +50,7 @@ import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import java.util.concurrent.ConcurrentHashMap +import retrofit2.HttpException import javax.inject.Inject import kotlin.collections.map import kotlin.time.Duration.Companion.milliseconds @@ -81,9 +82,9 @@ class OfflineFirstConversationsRepository @Inject constructor( get() = _conversationFlow private val _conversationFlow: MutableSharedFlow = MutableSharedFlow() - override val syncErrorFlow: Flow + override val syncErrorFlow: Flow get() = _syncErrorFlow - private val _syncErrorFlow: MutableSharedFlow = MutableSharedFlow() + private val _syncErrorFlow: MutableSharedFlow = MutableSharedFlow() private val scope = CoroutineScope(Dispatchers.IO) @@ -186,9 +187,9 @@ class OfflineFirstConversationsRepository @Inject constructor( * Syncs [user]'s room list, returning the rooms whose messages should be caught up, or null * when the sync failed. * - * [reportSyncError] lets a failure reach [syncErrorFlow]. That flow does not say which account - * failed and the conversation list shows whatever arrives there, so only a sync of the account - * the list shows may report to it. + * [reportSyncError] lets a failure reach [syncErrorFlow]. The conversation list shows the errors + * of its account that arrive there, so only the sync the list starts may report to it, not the + * background sync of the same account. */ @Suppress("Detekt.TooGenericExceptionCaught") private suspend fun getRoomsFromServer( @@ -215,8 +216,10 @@ class OfflineFirstConversationsRepository @Inject constructor( } catch (e: Exception) { Log.e(TAG, "Something went wrong when fetching conversations", e) storeTimestamp(accountId, KEY_MODIFIED_SINCE, null) - if (reportSyncError && dao.getConversationsForUser(accountId).first().isEmpty()) { - _syncErrorFlow.emit(e) + val hasCachedConversations = dao.getConversationsForUser(accountId).first().isNotEmpty() + // Cached conversations do not help when the credentials were rejected, the user has to act on that. + if (reportSyncError && (!hasCachedConversations || e.isUnauthorized())) { + _syncErrorFlow.emit(OfflineConversationsRepository.SyncError(accountId, e)) } null } @@ -249,7 +252,8 @@ class OfflineFirstConversationsRepository @Inject constructor( val roomList = withRetry( retries = NETWORK_FETCH_RETRIES, initialDelayMillis = NETWORK_FETCH_RETRY_INITIAL_DELAY_MS, - maxDelayMillis = NETWORK_FETCH_RETRY_MAX_DELAY_MS + maxDelayMillis = NETWORK_FETCH_RETRY_MAX_DELAY_MS, + retryOn = { !it.isUnauthorized() } ) { network.getRooms(user, user.baseUrl!!, includeStatus, modifiedSince) .subscribeOn(Schedulers.io()) @@ -348,6 +352,8 @@ class OfflineFirstConversationsRepository @Inject constructor( arbitraryStorageManager.storeStorageSetting(accountId, key, value?.toString(), "") } + private fun Exception.isUnauthorized(): Boolean = this is HttpException && code() == HTTP_UNAUTHORIZED + /** * Determines the rooms whose messages should be caught up in the background: rooms with * activity newer than the last synced state (matching the iOS behavior) plus unread rooms that @@ -498,6 +504,7 @@ class OfflineFirstConversationsRepository @Inject constructor( private const val NETWORK_FETCH_RETRIES = 3 private const val NETWORK_FETCH_RETRY_INITIAL_DELAY_MS = 1000L private const val NETWORK_FETCH_RETRY_MAX_DELAY_MS = 8000L + private const val HTTP_UNAUTHORIZED = 401 private const val FULL_SYNC_INTERVAL_MILLIS = 5 * 60 * 1000L private const val KEY_MODIFIED_SINCE = "conversation_list_modified_since" private const val KEY_LAST_FULL_SYNC_AT = "conversation_list_last_full_sync_at" diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt index 6fbe53bafc9..7b275a57a56 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/AvatarContent.kt @@ -21,7 +21,17 @@ internal sealed class AvatarContent { * conversation avatars are versioned; a one-to-one room's avatar is the peer's user avatar, * which is outside the version scheme and must revalidate via the response cache headers. */ - data class Url(val url: String, val versioned: Boolean) : AvatarContent() + data class Url(val url: String, val versioned: Boolean) : AvatarContent() { + /** + * The key of the avatar in the disk cache, so other places can use the avatar the list loaded. + * + * The suffix only names the cache entry, the request still goes to [url]. It keeps these entries apart from + * avatars cached under the plain URL elsewhere, and changing it makes all avatars cached with it unused, so + * they are loaded again. + */ + val diskCacheKey: String + get() = "$url#v2" + } data class Res(@param:DrawableRes val resId: Int) : AvatarContent() object System : AvatarContent() object NoteToSelf : AvatarContent() diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt index 437f3f161e1..2e347f4009c 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ui/ConversationListItem.kt @@ -262,7 +262,7 @@ private fun ConversationAvatarImage(model: ConversationModel, currentUser: User, val request = remember(avatarContent.url, credentials) { ImageRequest.Builder(context) .data(avatarContent.url) - .diskCacheKey("${avatarContent.url}#v2") + .diskCacheKey(avatarContent.diskCacheKey) .addHeader("Authorization", credentials) .crossfade(true) .listener( diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt index f8d4303821b..29082a46a5a 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt @@ -60,6 +60,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn @@ -154,7 +155,9 @@ class ConversationsListViewModel @AssistedInject constructor( init { repository.syncErrorFlow - .onEach { throwable -> _getRoomsViewState.value = GetRoomsErrorState(throwable) } + // e.g. the sync of the account shown before switching to this one + .filter { syncError -> syncError.accountId == currentUser.id } + .onEach { syncError -> _getRoomsViewState.value = GetRoomsErrorState(syncError.throwable) } .launchIn(viewModelScope) } diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java index c84f7df537a..c70bafc1bd6 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RestModule.java @@ -18,7 +18,7 @@ import com.nextcloud.talk.users.UserManager; import com.nextcloud.talk.utils.AccountCookieInterceptor; import com.nextcloud.talk.utils.ApiUtils; -import com.nextcloud.talk.utils.RemoteWipeInterceptor; +import com.nextcloud.talk.utils.RejectedCredentialsInterceptor; import com.nextcloud.talk.utils.preferences.AppPreferences; import com.nextcloud.talk.utils.ssl.KeyManager; import com.nextcloud.talk.utils.ssl.SSLSocketFactoryCompat; @@ -183,7 +183,6 @@ OkHttpClient provideHttpClient(Proxy proxy, AppPreferences appPreferences, SSLSocketFactoryCompat sslSocketFactoryCompat, Cache cache, CookieManager cookieManager, Dispatcher dispatcher, - UserManager userManager, LoggingHttpInterceptor loggingHttpInterceptor, AccountCookieInterceptor accountCookieInterceptor) { OkHttpClient.Builder httpClient = new OkHttpClient.Builder(); @@ -218,7 +217,7 @@ OkHttpClient provideHttpClient(Proxy proxy, AppPreferences appPreferences, } httpClient.addInterceptor(new HeadersInterceptor()); - httpClient.addInterceptor(new RemoteWipeInterceptor(userManager, context, sslSocketFactoryCompat, trustManager)); + httpClient.addInterceptor(new RejectedCredentialsInterceptor()); httpClient.addInterceptor(loggingHttpInterceptor); // Each account keeps its own cookies, so its server session is kept but never used by another account. httpClient.addNetworkInterceptor(accountCookieInterceptor); diff --git a/app/src/main/java/com/nextcloud/talk/events/RemoteWipeEvent.kt b/app/src/main/java/com/nextcloud/talk/events/RemoteWipeEvent.kt deleted file mode 100644 index e3bae5ce095..00000000000 --- a/app/src/main/java/com/nextcloud/talk/events/RemoteWipeEvent.kt +++ /dev/null @@ -1,10 +0,0 @@ -/* - * Nextcloud Talk - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: GPL-3.0-or-later - */ - -package com.nextcloud.talk.events - -class RemoteWipeEvent diff --git a/app/src/main/java/com/nextcloud/talk/jobs/RemoteWipeSuccessWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/RemoteWipeSuccessWorker.kt index b39653a631c..a264ffddb7b 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/RemoteWipeSuccessWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/RemoteWipeSuccessWorker.kt @@ -9,6 +9,12 @@ package com.nextcloud.talk.jobs import android.content.Context import android.util.Log +import androidx.work.BackoffPolicy +import androidx.work.Constraints +import androidx.work.Data +import androidx.work.NetworkType +import androidx.work.OneTimeWorkRequest +import androidx.work.WorkRequest import androidx.work.Worker import androidx.work.WorkerParameters import autodagger.AutoInjector @@ -16,6 +22,8 @@ import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.utils.ApiUtils import kotlinx.coroutines.runBlocking +import java.io.IOException +import java.util.concurrent.TimeUnit import javax.inject.Inject @AutoInjector(NextcloudTalkApplication::class) @@ -27,24 +35,58 @@ class RemoteWipeSuccessWorker(context: Context, workerParams: WorkerParameters) override fun doWork(): Result { NextcloudTalkApplication.sharedApplication!!.componentApplication.inject(this) - val baseUrl = inputData.getString(KEY_BASE_URL) ?: return Result.failure() - val token = inputData.getString(KEY_TOKEN) ?: return Result.failure() + val baseUrl = inputData.getString(KEY_BASE_URL) + val token = inputData.getString(KEY_TOKEN) + return if (baseUrl == null || token == null) Result.failure() else reportWipeSuccess(baseUrl, token) + } + + private fun reportWipeSuccess(baseUrl: String, token: String): Result = + try { + val url = ApiUtils.getUrlForRemoteWipeSuccess(baseUrl) + val response = runBlocking { ncApiCoroutines.reportRemoteWipeSuccess(url, token) } + when { + response.isSuccessful -> Result.success() + + response.code() == HTTP_TOO_MANY_REQUESTS || response.code() >= HTTP_INTERNAL_SERVER_ERROR -> { + Log.w(TAG, "Server could not take the remote wipe success now: ${response.code()}") + retryOrFail() + } - return try { - runBlocking { - val url = ApiUtils.getUrlForRemoteWipeSuccess(baseUrl) - ncApiCoroutines.reportRemoteWipeSuccess(url, token) + else -> { + // e.g. 404 when the server does not know the wipe of the token any more, so trying again won't help + Log.e(TAG, "Server refused the remote wipe success: ${response.code()}") + Result.failure() + } } - Result.success() - } catch (e: Exception) { - Log.e(TAG, "Failed to report remote wipe success to server", e) - Result.failure() + } catch (e: IOException) { + Log.w(TAG, "Failed to report remote wipe success to server", e) + retryOrFail() } - } + + private fun retryOrFail(): Result = if (runAttemptCount < MAX_ATTEMPTS - 1) Result.retry() else Result.failure() companion object { const val TAG = "RemoteWipeSuccessWorker" const val KEY_BASE_URL = "baseUrl" const val KEY_TOKEN = "token" + private const val MAX_ATTEMPTS = 10 + private const val HTTP_TOO_MANY_REQUESTS = 429 + private const val HTTP_INTERNAL_SERVER_ERROR = 500 + + /** + * Reports to the server at [baseUrl] that the wipe it requested for the app password [token] is done. It is + * sent once the device is online, and tried again later if it failed for a reason that may pass. + */ + fun workRequest(baseUrl: String, token: String): OneTimeWorkRequest = + OneTimeWorkRequest.Builder(RemoteWipeSuccessWorker::class.java) + .setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, WorkRequest.MIN_BACKOFF_MILLIS, TimeUnit.MILLISECONDS) + .setInputData( + Data.Builder() + .putString(KEY_BASE_URL, baseUrl) + .putString(KEY_TOKEN, token) + .build() + ) + .build() } } diff --git a/app/src/main/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptor.kt b/app/src/main/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptor.kt new file mode 100644 index 00000000000..7f9333ea286 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptor.kt @@ -0,0 +1,124 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.utils + +import android.os.SystemClock +import android.util.Log +import okhttp3.Interceptor +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import java.io.IOException +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.TimeUnit + +/** + * Stops sending credentials that the server rejected. + * + * The server counts every request with rejected credentials as a failed login, and after some of them it throttles + * and finally refuses all requests from the same IP with 429, also those of other accounts and devices. So once a + * request got a 401 for its credentials, further requests with them are answered with a 401 here, without sending + * them. Other credentials, e.g. after the account was reauthorized, are sent as usual. + * + * The rejection might be temporary, so one request with the credentials is sent again every + * [RETRY_INTERVAL_MILLIS]. Once it is not rejected, the credentials are sent as usual again. + */ +class RejectedCredentialsInterceptor @JvmOverloads constructor( + private val elapsedRealtime: () -> Long = SystemClock::elapsedRealtime +) : Interceptor { + + // Only in memory, keyed by the credentials of the requests, which are kept in memory anyway. + private val lastRejectionTimes = ConcurrentHashMap() + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val credentials = request.header(AUTHORIZATION) + val now = elapsedRealtime() + + return when { + credentials == null -> chain.proceed(request) + + !mayBeSent(credentials, now) -> { + Log.d(TAG, "Not sending request with rejected credentials: ${request.url}") + rejectedResponse(request) + } + + else -> proceedAndRememberRejection(chain, request, credentials, sentAt = now) + } + } + + private fun proceedAndRememberRejection( + chain: Interceptor.Chain, + request: Request, + credentials: String, + sentAt: Long + ): Response { + val response = try { + chain.proceed(request) + } catch (e: IOException) { + // A retry that got no answer says nothing about the credentials, so the next request may try again. + lastRejectionTimes.computeIfPresent(credentials) { _, lastRejection -> + if (lastRejection == sentAt) lastRejection - RETRY_INTERVAL_MILLIS else lastRejection + } + throw e + } + // After a redirect to another host, OkHttp drops the credentials, so that 401 says nothing about them. + val credentialsWereRejected = response.code == HTTP_UNAUTHORIZED && + response.request.header(AUTHORIZATION) == credentials + + // A 429 or a server error, e.g. in maintenance mode, is answered without checking the credentials. + val credentialsWereChecked = response.code != HTTP_TOO_MANY_REQUESTS && + response.code < HTTP_INTERNAL_SERVER_ERROR + + if (credentialsWereRejected) { + lastRejectionTimes[credentials] = elapsedRealtime() + } else if (credentialsWereChecked) { + // Only a request sent after the rejection shows that the credentials are accepted again, e.g. not a long + // polling request that was already waiting for its answer. + lastRejectionTimes.computeIfPresent(credentials) { _, lastRejection -> + lastRejection.takeIf { sentAt < it } + } + } + return response + } + + /** + * Whether a request with [credentials] may be sent [now]: they were not rejected, or long enough ago for one + * request to try them again. Only one request claims that retry, the following ones wait for the next interval. + */ + private fun mayBeSent(credentials: String, now: Long): Boolean { + var maySend = true + lastRejectionTimes.computeIfPresent(credentials) { _, lastRejection -> + if (now - lastRejection >= RETRY_INTERVAL_MILLIS) { + now + } else { + maySend = false + lastRejection + } + } + return maySend + } + + private fun rejectedResponse(request: Request): Response = + Response.Builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .code(HTTP_UNAUTHORIZED) + .message("Credentials were rejected before, request not sent") + .body("".toResponseBody()) + .build() + + companion object { + private const val TAG = "RejectedCredentials" + private const val AUTHORIZATION = "Authorization" + private const val HTTP_UNAUTHORIZED = 401 + private const val HTTP_TOO_MANY_REQUESTS = 429 + private const val HTTP_INTERNAL_SERVER_ERROR = 500 + private val RETRY_INTERVAL_MILLIS = TimeUnit.MINUTES.toMillis(5) + } +} diff --git a/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeHandler.kt b/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeHandler.kt new file mode 100644 index 00000000000..ea73c5892a9 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeHandler.kt @@ -0,0 +1,53 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.utils + +import android.util.Log +import com.nextcloud.talk.api.NcApiCoroutines +import com.nextcloud.talk.data.user.model.User +import java.io.IOException +import javax.inject.Inject + +/** + * Asks the server whether it requested a remote wipe of an account. + * + * A wipe makes the server reject the credentials of the account, but rejected credentials do not mean that a wipe + * was requested, e.g. a password change in an external user backend causes that too. So the server is asked. + */ +class RemoteWipeHandler @Inject constructor(private val ncApiCoroutines: NcApiCoroutines) { + + /** + * Whether the server requested a wipe of [user]. Removing the account and reporting the wipe is up to the caller. + */ + suspend fun isWipeRequested(user: User): Boolean { + val baseUrl = user.baseUrl + val token = user.token + return baseUrl != null && token != null && askServerAboutWipe(baseUrl, token) + } + + private suspend fun askServerAboutWipe(baseUrl: String, token: String): Boolean { + val wipeRequested = try { + val response = ncApiCoroutines.checkRemoteWipe(ApiUtils.getUrlForRemoteWipeCheck(baseUrl), token) + Log.d(TAG, "Wipe check at $baseUrl answered with ${response.code()}") + response.isSuccessful && response.body()?.wipe == true + } catch (e: IOException) { + Log.e(TAG, "Failed to check remote wipe status", e) + false + } catch (e: IllegalArgumentException) { + Log.e(TAG, "Failed to check remote wipe status", e) + false + } + + Log.d(TAG, "Wipe requested by $baseUrl: $wipeRequested") + return wipeRequested + } + + companion object { + private const val TAG = "RemoteWipeHandler" + } +} diff --git a/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt b/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt deleted file mode 100644 index ef6df7cd403..00000000000 --- a/app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt +++ /dev/null @@ -1,173 +0,0 @@ -/* - * Nextcloud Talk - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: GPL-3.0-or-later - */ - -package com.nextcloud.talk.utils - -import android.content.Context -import android.util.Log -import androidx.work.Data -import androidx.work.OneTimeWorkRequest -import androidx.work.WorkManager -import com.bluelinelabs.logansquare.LoganSquare -import com.nextcloud.talk.data.user.model.User -import com.nextcloud.talk.events.RemoteWipeEvent -import com.nextcloud.talk.jobs.AccountRemovalWorker -import com.nextcloud.talk.jobs.RemoteWipeSuccessWorker -import com.nextcloud.talk.models.json.wipe.WipeCheckResponseDto -import com.nextcloud.talk.users.UserManager -import com.nextcloud.talk.utils.ssl.SSLSocketFactoryCompat -import com.nextcloud.talk.utils.ssl.TrustManager -import kotlinx.coroutines.runBlocking -import okhttp3.FormBody -import okhttp3.Interceptor -import okhttp3.OkHttpClient -import okhttp3.Request -import okhttp3.Response -import okhttp3.internal.tls.OkHostnameVerifier -import org.greenrobot.eventbus.EventBus -import java.io.IOException -import java.util.Collections -import java.util.concurrent.ConcurrentHashMap - -class RemoteWipeInterceptor( - private val userManager: UserManager, - private val context: Context, - private val sslSocketFactory: SSLSocketFactoryCompat, - private val trustManager: TrustManager -) : Interceptor { - - private val handledUserIds = Collections.newSetFromMap(ConcurrentHashMap()) - - private val wipeCheckClient: OkHttpClient by lazy { - OkHttpClient.Builder() - .sslSocketFactory(sslSocketFactory, trustManager) - .hostnameVerifier(trustManager.getHostnameVerifier(OkHostnameVerifier)) - .build() - } - - override fun intercept(chain: Interceptor.Chain): Response { - val response = chain.proceed(chain.request()) - if (response.code != HTTP_UNAUTHORIZED) return response - - // The request that got the 401. After a redirect, it differs from chain.request(): e.g. OkHttp drops the - // credentials when redirecting to another host, so that 401 says nothing about the original credentials. - val rejectedRequest = response.request - Log.d(TAG, "Received 401 for ${rejectedRequest.url}") - - if (WIPE_PATH in rejectedRequest.url.encodedPath) { - Log.d(TAG, "401 was from the wipe endpoint itself, ignoring to avoid recursion") - return response - } - - val candidate = resolveWipeCandidate(rejectedRequest) ?: return response - if (!handledUserIds.add(candidate.userId)) { - Log.d(TAG, "User ${candidate.userId} was already handled, ignoring") - return response - } - - val wipeRequestedByServer = isWipeRequestedByServer(candidate) - performWipe(candidate, wipeRequestedByServer) - - return response - } - - /** - * The account whose credentials the server rejected: the one with exactly the credentials of [request], on the - * server of its URL. A 401 for a request without credentials, or with credentials of no stored account, says - * nothing about an account, e.g. a request that relied on a session cookie, and is ignored. Several accounts can - * be on the same server, so the server alone does not identify the account. - */ - private fun resolveWipeCandidate(request: Request): WipeCandidate? { - val user = findUserWithCredentialsOf(request) - val token = user?.token - val userId = user?.id - - return if (user != null && token != null && userId != null) { - WipeCandidate(user, token, userId) - } else { - Log.d(TAG, "401 for a request without the credentials of a known account, ignoring: ${request.url}") - null - } - } - - /** - * The stored account on the server of [request] whose credentials the request carries, if any. - */ - private fun findUserWithCredentialsOf(request: Request): User? { - val credentials = request.header(AUTHORIZATION) ?: return null - val requestUrl = request.url.toString() - - return runBlocking { userManager.getUsers() }.firstOrNull { user -> - val isOnServerOfRequest = user.baseUrl?.let { UriUtils.isOnServer(requestUrl, it) } == true - isOnServerOfRequest && ApiUtils.getCredentials(user.username, user.token) == credentials - } - } - - private fun isWipeRequestedByServer(candidate: WipeCandidate): Boolean { - Log.d( - TAG, - "Asking server whether it requested a wipe for user ${candidate.userId} at ${candidate.user.baseUrl}" - ) - - val wipeRequestedByServer = try { - val wipeCheckRequest = Request.Builder() - .url(ApiUtils.getUrlForRemoteWipeCheck(candidate.user.baseUrl!!)) - .post(FormBody.Builder().add("token", candidate.token).build()) - .build() - wipeCheckClient.newCall(wipeCheckRequest).execute().use { wipeResponse -> - val body = wipeResponse.body?.string() - wipeResponse.isSuccessful && - body != null && - LoganSquare.parse(body, WipeCheckResponseDto::class.java)?.wipe == true - } - } catch (e: IOException) { - Log.e(TAG, "Failed to check remote wipe status", e) - false - } catch (e: IllegalArgumentException) { - Log.e(TAG, "Failed to check remote wipe status", e) - false - } - - Log.d(TAG, "Server response for user ${candidate.userId}: wipe requested = $wipeRequestedByServer") - return wipeRequestedByServer - } - - private fun performWipe(candidate: WipeCandidate, wipeRequestedByServer: Boolean) { - Log.d(TAG, "Scheduling user ${candidate.userId} for deletion") - runBlocking { userManager.scheduleUserForDeletionWithId(candidate.userId) } - - val accountRemovalWork = OneTimeWorkRequest.Builder(AccountRemovalWorker::class.java).build() - var workContinuation = WorkManager.getInstance(context).beginWith(accountRemovalWork) - - if (wipeRequestedByServer) { - Log.d(TAG, "Server had requested this wipe, will report success after account removal") - val remoteWipeSuccessWork = OneTimeWorkRequest.Builder(RemoteWipeSuccessWorker::class.java) - .setInputData( - Data.Builder() - .putString(RemoteWipeSuccessWorker.KEY_BASE_URL, candidate.user.baseUrl) - .putString(RemoteWipeSuccessWorker.KEY_TOKEN, candidate.token) - .build() - ) - .build() - workContinuation = workContinuation.then(remoteWipeSuccessWork) - } else { - Log.d(TAG, "Server did not request this wipe, account is still removed but no success report is sent") - } - workContinuation.enqueue() - - EventBus.getDefault().post(RemoteWipeEvent()) - } - - private data class WipeCandidate(val user: User, val token: String, val userId: Long) - - companion object { - private const val TAG = "RemoteWipeInterceptor" - private const val HTTP_UNAUTHORIZED = 401 - private const val AUTHORIZATION = "Authorization" - private const val WIPE_PATH = "wipe" - } -} diff --git a/app/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.kt b/app/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.kt index 08981e9b889..4041f5218a0 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.kt @@ -82,7 +82,6 @@ object BundleKeys { const val KEY_CHAT_API_VERSION = "KEY_CHAT_API_VERSION" const val KEY_CALL_FLAG = "KEY_CALL_FLAG" const val KEY_CREDENTIALS: String = "KEY_CREDENTIALS" - const val KEY_FIELD_MAP: String = "KEY_FIELD_MAP" const val KEY_CHAT_URL: String = "KEY_CHAT_URL" const val KEY_SCROLL_TO_NOTIFICATION_CATEGORY: String = "KEY_SCROLL_TO_NOTIFICATION_CATEGORY" const val KEY_FOCUS_INPUT: String = "KEY_FOCUS_INPUT" diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8ab569cb88a..b3ee36ff94e 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -32,6 +32,7 @@ How to translate with transifex: Set Dismiss Sorry, something went wrong! + Too many failed login attempts from this network. Try again later. You logged in with a different account. Log in with the account you want to reauthorize. Create Unknown diff --git a/app/src/test/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModelTest.kt b/app/src/test/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModelTest.kt index 0d089501bd0..6401bfb13a6 100644 --- a/app/src/test/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModelTest.kt +++ b/app/src/test/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModelTest.kt @@ -7,6 +7,7 @@ package com.nextcloud.talk.account.viewmodels import com.nextcloud.talk.account.data.LoginRepository +import com.nextcloud.talk.account.data.model.LoginCompletion import com.nextcloud.talk.account.data.model.LoginResponse import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -20,6 +21,7 @@ import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.wheneverBlocking @@ -67,6 +69,20 @@ class BrowserLoginActivityViewModelTest { verifyBlocking(repository, times(1)) { startOTPLoginFlow(any(), any(), anyOrNull()) } } + @Test + fun `a login refused because of too many failed logins is reported as such`() { + wheneverBlocking { repository.startOTPLoginFlow(any(), any(), anyOrNull()) } + .thenReturn(LoginCompletion(LoginRepository.HTTP_TOO_MANY_REQUESTS, BASE_URL, "user", "")) + + viewModel.loginWithOTPQR("qr") + + assertEquals( + BrowserLoginActivityViewModel.PostLoginViewState.PostLoginTooManyLoginAttempts, + viewModel.postLoginState.value + ) + verifyBlocking(repository, never()) { parseAndLogin(any()) } + } + companion object { private const val BASE_URL = "https://example.com" private const val URL = "https://example.com/login" diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt index 12e3fa3780c..debd0ce17e1 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt @@ -33,14 +33,18 @@ import com.nextcloud.talk.models.json.generic.GenericMetaDto import com.nextcloud.talk.models.json.generic.GenericOverall import com.nextcloud.talk.models.json.conversations.ConversationDto import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeout import okhttp3.MediaType.Companion.toMediaType import okhttp3.ResponseBody.Companion.toResponseBody import org.junit.Assert.assertEquals @@ -55,6 +59,8 @@ import org.mockito.kotlin.eq import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.timeout +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever @@ -107,6 +113,44 @@ class OfflineFirstChatRepositoryTest { repository.initData(user(), CREDENTIALS, CHAT_URL, ROOM_TOKEN, null) } + @Test + fun `long polling waits before the next request after a rejected one`() = + runBlocking { + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.error(HTTP_UNAUTHORIZED, "".toResponseBody("text/plain".toMediaType()))) + + val polling = launch(Dispatchers.Default) { repository.initLongPolling() } + // shorter than the wait after a failed request, so only the first request may have been sent + delay(LONG_POLLING_OBSERVATION_MILLIS) + polling.cancelAndJoin() + + verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } + } + + @Test + fun `long polling does not wait longer after a failed request once the device is online again`() = + runBlocking { + val isOnline = MutableStateFlow(true) + whenever(networkMonitor.isOnline).thenReturn(isOnline) + val firstRequestSent = CompletableDeferred() + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } doSuspendableAnswer { + firstRequestSent.complete(Unit) + Response.error(HTTP_UNAUTHORIZED, "".toResponseBody("text/plain".toMediaType())) + } + + val polling = launch(Dispatchers.Default) { repository.initLongPolling() } + // only go offline once the first request failed, so its wait is what the reconnect has to end + withTimeout(AWAIT_TIMEOUT_MILLIS) { firstRequestSent.await() } + isOnline.value = false + delay(LONG_POLLING_OBSERVATION_MILLIS) + isOnline.value = true + + // still shorter than the wait after a failed request + verify(network, timeout(LONG_POLLING_OBSERVATION_MILLIS * 2).times(2)) + .pullChatMessages(any(), any(), any()) + polling.cancelAndJoin() + } + @Test fun `markPendingReadMarker registers the marker synchronously, without any suspension`() { // Leaving the chat can race a room list sync triggered by the conversation list resuming @@ -781,6 +825,9 @@ class OfflineFirstChatRepositoryTest { private const val ROOM_TOKEN = "room1" private const val INTERNAL_CONVERSATION_ID = "$ACCOUNT_ID@$ROOM_TOKEN" private const val CREDENTIALS = "credentials" + private const val HTTP_UNAUTHORIZED = 401 + private const val LONG_POLLING_OBSERVATION_MILLIS = 200L + private const val AWAIT_TIMEOUT_MILLIS = 2000L private const val CHAT_URL = "https://server.example.com/ocs/v2.php/apps/spreed/api/v1/chat/$ROOM_TOKEN" private const val MESSAGE_ID = 42L private const val SYSTEM_MESSAGE_ID = 43L diff --git a/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt index f61f2522884..d45d2e39fdd 100644 --- a/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt +++ b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt @@ -13,6 +13,7 @@ import android.os.PowerManager import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource +import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository import com.nextcloud.talk.data.database.dao.ConversationsDao import com.nextcloud.talk.data.database.mappers.asEntity import com.nextcloud.talk.data.database.model.ConversationEntity @@ -38,6 +39,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest +import okhttp3.ResponseBody.Companion.toResponseBody import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -59,6 +61,8 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.wheneverBlocking import org.mockito.kotlin.whenever +import retrofit2.HttpException +import retrofit2.Response /** * Covers [OfflineFirstConversationsRepository] at the unit level, with all collaborators mocked. @@ -195,7 +199,7 @@ class OfflineFirstConversationsRepositoryTest { whenever(network.getRooms(any(), any(), any(), anyOrNull())).thenReturn(roomList(listOf(room))) wheneverBlocking { dao.syncConversationsForUser(any(), any(), any()) } .thenThrow(IllegalStateException("database is gone")) - val errors = mutableListOf() + val errors = mutableListOf() val collector = launch(Dispatchers.Unconfined) { repository.syncErrorFlow.collect { errors += it } } repository.getRooms(user()).join() @@ -211,13 +215,13 @@ class OfflineFirstConversationsRepositoryTest { whenever(network.getRooms(any(), any(), any(), anyOrNull())).thenReturn(roomList(listOf(room))) wheneverBlocking { dao.syncConversationsForUser(any(), any(), any()) } .thenThrow(IllegalStateException("database is gone")) - val errors = mutableListOf() + val errors = mutableListOf() val collector = launch(Dispatchers.Unconfined) { repository.syncErrorFlow.collect { errors += it } } repository.syncRooms(user()) collector.cancel() - assertEquals(emptyList(), errors) + assertEquals(emptyList(), errors) } @Test @@ -587,6 +591,29 @@ class OfflineFirstConversationsRepositoryTest { collector.cancel() } + @Test + fun `getRooms reports rejected credentials without retrying even when conversations are cached`() = + runBlocking { + val cached = conversation(token = ROOM_TOKEN, lastActivity = 1, unreadMessages = 0).asEntity(ACCOUNT_ID) + whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(listOf(cached))) + val unauthorized = HttpException(Response.error(HTTP_UNAUTHORIZED, "".toResponseBody())) + whenever(network.getRooms(any(), any(), any(), anyOrNull())).thenReturn(Observable.error(unauthorized)) + + val errors = mutableListOf() + val collector = launch(Dispatchers.IO) { + repository.syncErrorFlow.collect { errors.add(it) } + } + // syncErrorFlow has no replay, so the collector must already be subscribed before the error is emitted + delay(COLLECTOR_STARTUP_MILLIS) + + repository.getRooms(user()).join() + + awaitUntil { errors.isNotEmpty() } + assertEquals(OfflineConversationsRepository.SyncError(ACCOUNT_ID, unauthorized), errors.first()) + verify(network, times(1)).getRooms(any(), any(), any(), anyOrNull()) + collector.cancel() + } + private fun stubCatchUpRoom() { wheneverBlocking { chatMessageSyncer.catchUpRoom(any(), any(), anyOrNull(), any()) } .thenReturn(ChatMessageSyncer.SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null)) @@ -643,6 +670,7 @@ class OfflineFirstConversationsRepositoryTest { private const val POLL_INTERVAL_MILLIS = 50L private const val COLLECTOR_STARTUP_MILLIS = 100L private const val ROOM_LIST_TIMEOUT_MILLIS = 1000L + private const val HTTP_UNAUTHORIZED = 401 } } diff --git a/app/src/test/java/com/nextcloud/talk/login/data/network/NetworkLoginDataSourceTest.kt b/app/src/test/java/com/nextcloud/talk/login/data/network/NetworkLoginDataSourceTest.kt index e83156f044c..42f9392d18b 100644 --- a/app/src/test/java/com/nextcloud/talk/login/data/network/NetworkLoginDataSourceTest.kt +++ b/app/src/test/java/com/nextcloud/talk/login/data/network/NetworkLoginDataSourceTest.kt @@ -11,6 +11,7 @@ import com.nextcloud.talk.account.data.model.LoginResponse import com.nextcloud.talk.account.data.network.NetworkLoginDataSource import junit.framework.TestCase.assertNotNull import junit.framework.TestCase.assertNull +import okhttp3.Credentials import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -164,4 +165,24 @@ class NetworkLoginDataSourceTest { val loginCompletion = network.performLoginFlowV2(loginResponse) assertNull(loginCompletion) } + + @Test(expected = NetworkLoginDataSource.TooManyLoginAttemptsException::class) + fun `testing oneTimePasswordRequest refused because of too many failed logins`() { + val server = MockWebServer() + server.enqueue(MockResponse().setResponseCode(429)) + server.start() + + network.oneTimePasswordRequest(server.url("").toString(), Credentials.basic("username", "oneTimePassword")) + } + + @Test + fun `testing oneTimePasswordRequest error path`() { + val server = MockWebServer() + server.enqueue(MockResponse().setResponseCode(401)) + server.start() + + val appPassword = + network.oneTimePasswordRequest(server.url("").toString(), Credentials.basic("username", "oneTimePassword")) + assertNull(appPassword) + } } diff --git a/app/src/test/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptorTest.kt b/app/src/test/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptorTest.kt new file mode 100644 index 00000000000..49b9a9c10c4 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/RejectedCredentialsInterceptorTest.kt @@ -0,0 +1,183 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.utils + +import okhttp3.Credentials +import okhttp3.Interceptor +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertThrows +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import java.io.IOException +import java.util.concurrent.TimeUnit + +class RejectedCredentialsInterceptorTest { + + private var now = 0L + private val interceptor = RejectedCredentialsInterceptor { now } + + private val credentialsA = Credentials.basic("userA", "tokenA") + private val credentialsB = Credentials.basic("userB", "tokenB") + + @Test + fun `rejected credentials are not sent again`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + + interceptor.intercept(server.chainFor(request(credentialsA))) + val response = interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(1, server.requestCount) + assertEquals(HTTP_UNAUTHORIZED, response.code) + } + + @Test + fun `other credentials are still sent`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + interceptor.intercept(server.chainFor(request(credentialsA))) + + server.code = HTTP_OK + val response = interceptor.intercept(server.chainFor(request(credentialsB))) + + assertEquals(2, server.requestCount) + assertEquals(HTTP_OK, response.code) + } + + @Test + fun `rejected credentials are tried again once per interval`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + interceptor.intercept(server.chainFor(request(credentialsA))) + + now += TimeUnit.MINUTES.toMillis(5) + interceptor.intercept(server.chainFor(request(credentialsA))) + interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(2, server.requestCount) + } + + @Test + fun `credentials accepted on a retry are sent as usual again`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + interceptor.intercept(server.chainFor(request(credentialsA))) + + now += TimeUnit.MINUTES.toMillis(5) + server.code = HTTP_OK + interceptor.intercept(server.chainFor(request(credentialsA))) + interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(3, server.requestCount) + } + + @Test + fun `a 401 after a redirect that dropped the credentials does not reject them`() { + val server = FakeServer(HTTP_UNAUTHORIZED, answeredRequest = Request.Builder().url(OTHER_HOST_URL).build()) + + interceptor.intercept(server.chainFor(request(credentialsA))) + interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(2, server.requestCount) + } + + @Test + fun `an answer to a request sent before the rejection does not accept the credentials again`() { + val rejectingServer = FakeServer(HTTP_UNAUTHORIZED) + // While the long polling request waits for its answer, another request with the credentials is rejected. + val longPollingServer = FakeServer(HTTP_OK) { + now += 1 + interceptor.intercept(rejectingServer.chainFor(request(credentialsA))) + } + interceptor.intercept(longPollingServer.chainFor(request(credentialsA))) + + interceptor.intercept(rejectingServer.chainFor(request(credentialsA))) + + assertEquals(1, rejectingServer.requestCount) + } + + @Test + fun `a server error on a retry does not accept the credentials again`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + interceptor.intercept(server.chainFor(request(credentialsA))) + + now += TimeUnit.MINUTES.toMillis(5) + server.code = HTTP_SERVICE_UNAVAILABLE + interceptor.intercept(server.chainFor(request(credentialsA))) + interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(2, server.requestCount) + } + + @Test + fun `a retry that got no answer lets the next request try again`() { + interceptor.intercept(FakeServer(HTTP_UNAUTHORIZED).chainFor(request(credentialsA))) + + now += TimeUnit.MINUTES.toMillis(5) + val offlineServer = FakeServer(HTTP_OK) { throw IOException("offline") } + assertThrows(IOException::class.java) { + interceptor.intercept(offlineServer.chainFor(request(credentialsA))) + } + val server = FakeServer(HTTP_OK) + interceptor.intercept(server.chainFor(request(credentialsA))) + + assertEquals(1, server.requestCount) + } + + @Test + fun `requests without credentials are always sent`() { + val server = FakeServer(HTTP_UNAUTHORIZED) + val request = Request.Builder().url(URL).build() + + interceptor.intercept(server.chainFor(request)) + interceptor.intercept(server.chainFor(request)) + + assertEquals(2, server.requestCount) + } + + private fun request(credentials: String): Request = + Request.Builder().url(URL).header("Authorization", credentials).build() + + /** + * Answers every request with [code]. [answeredRequest] stands in for the request OkHttp sent last, e.g. after a + * redirect. [whileAnswering] runs before the answer, e.g. for what happens while a request waits for it. + */ + private class FakeServer( + var code: Int, + private val answeredRequest: Request? = null, + private val whileAnswering: () -> Unit = {} + ) { + var requestCount = 0 + + fun chainFor(request: Request): Interceptor.Chain = + mock { + on { request() } doReturn request + on { proceed(any()) } doAnswer { invocation -> + requestCount++ + whileAnswering() + Response.Builder() + .request(answeredRequest ?: invocation.getArgument(0)) + .protocol(Protocol.HTTP_1_1) + .code(code) + .message("") + .body("".toResponseBody()) + .build() + } + } + } + + companion object { + private const val URL = "https://cloud.example.com/ocs/v2.php/apps/spreed/api/v4/room" + private const val OTHER_HOST_URL = "https://other.example.org/room" + private const val HTTP_OK = 200 + private const val HTTP_UNAUTHORIZED = 401 + private const val HTTP_SERVICE_UNAVAILABLE = 503 + } +} diff --git a/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeHandlerTest.kt b/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeHandlerTest.kt new file mode 100644 index 00000000000..ffecbe8a1f3 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeHandlerTest.kt @@ -0,0 +1,90 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.utils + +import com.nextcloud.talk.api.NcApiCoroutines +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.json.wipe.WipeCheckResponseDto +import kotlinx.coroutines.runBlocking +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verifyBlocking +import org.mockito.kotlin.wheneverBlocking +import retrofit2.Response +import java.io.IOException + +/** + * An account must only be removed when its server confirms that it requested a wipe, so all other answers mean no wipe. + */ +class RemoteWipeHandlerTest { + + private val ncApiCoroutines: NcApiCoroutines = mock() + private val handler = RemoteWipeHandler(ncApiCoroutines) + + private val user = User(id = USER_ID, username = "userA", token = "tokenA", baseUrl = BASE_URL) + + @Test + fun `a wipe is requested when the server answers so`() { + wheneverBlocking { ncApiCoroutines.checkRemoteWipe(any(), any()) } doReturn + Response.success(WipeCheckResponseDto(wipe = true)) + + assertTrue(runBlocking { handler.isWipeRequested(user) }) + } + + @Test + fun `no wipe is requested when the server did not request one`() { + wheneverBlocking { ncApiCoroutines.checkRemoteWipe(any(), any()) } doReturn + Response.error(HTTP_NOT_FOUND, "".toResponseBody()) + + assertFalse(runBlocking { handler.isWipeRequested(user) }) + } + + @Test + fun `no wipe is requested when the server answers so`() { + wheneverBlocking { ncApiCoroutines.checkRemoteWipe(any(), any()) } doReturn + Response.success(WipeCheckResponseDto(wipe = false)) + + assertFalse(runBlocking { handler.isWipeRequested(user) }) + } + + @Test + fun `no wipe is requested when the wipe check is rate limited`() { + wheneverBlocking { ncApiCoroutines.checkRemoteWipe(any(), any()) } doReturn + Response.error(HTTP_TOO_MANY_REQUESTS, "".toResponseBody()) + + assertFalse(runBlocking { handler.isWipeRequested(user) }) + } + + @Test + fun `no wipe is requested when the server cannot be asked`() { + wheneverBlocking { ncApiCoroutines.checkRemoteWipe(any(), any()) } doSuspendableAnswer { + throw IOException("unreachable") + } + + assertFalse(runBlocking { handler.isWipeRequested(user) }) + } + + @Test + fun `the server is not asked for an account without a token`() { + assertFalse(runBlocking { handler.isWipeRequested(user.copy(token = null)) }) + verifyBlocking(ncApiCoroutines, never()) { checkRemoteWipe(any(), any()) } + } + + companion object { + private const val USER_ID = 1L + private const val BASE_URL = "https://cloud.example.com" + private const val HTTP_NOT_FOUND = 404 + private const val HTTP_TOO_MANY_REQUESTS = 429 + } +} diff --git a/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeInterceptorTest.kt b/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeInterceptorTest.kt deleted file mode 100644 index 390338d21cf..00000000000 --- a/app/src/test/java/com/nextcloud/talk/utils/RemoteWipeInterceptorTest.kt +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Nextcloud Talk - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: GPL-3.0-or-later - */ -package com.nextcloud.talk.utils - -import android.content.Context -import com.nextcloud.talk.data.user.model.User -import com.nextcloud.talk.users.UserManager -import com.nextcloud.talk.utils.ssl.SSLSocketFactoryCompat -import com.nextcloud.talk.utils.ssl.TrustManager -import okhttp3.Credentials -import okhttp3.Interceptor -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import org.junit.Assert.assertEquals -import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.doReturn -import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verifyBlocking -import org.mockito.kotlin.wheneverBlocking - -/** - * A 401 must only remove the account whose credentials the request carried. These cases must never remove one. - */ -class RemoteWipeInterceptorTest { - - private val userManager: UserManager = mock() - private val interceptor = RemoteWipeInterceptor( - userManager, - mock(), - mock(), - mock() - ) - - private val userA = User(id = 1, username = "userA", token = "tokenA", baseUrl = BASE_URL) - private val userB = User(id = 2, username = "userB", token = "tokenB", baseUrl = BASE_URL) - - @Test - fun `a 401 for a request without credentials removes no account`() { - wheneverBlocking { userManager.getUsers() } doReturn listOf(userA, userB) - - val response = interceptor.intercept(chainAnswering401(Request.Builder().url(PREVIEW_URL).build())) - - assertEquals(HTTP_UNAUTHORIZED, response.code) - verifyBlocking(userManager, never()) { scheduleUserForDeletionWithId(any()) } - } - - @Test - fun `a 401 for credentials of no stored account removes no account`() { - wheneverBlocking { userManager.getUsers() } doReturn listOf(userA, userB) - val request = Request.Builder() - .url(PREVIEW_URL) - .header("Authorization", Credentials.basic("userB", "outdatedToken")) - .build() - - interceptor.intercept(chainAnswering401(request)) - - verifyBlocking(userManager, never()) { scheduleUserForDeletionWithId(any()) } - } - - @Test - fun `a 401 after a redirect to another host removes no account`() { - wheneverBlocking { userManager.getUsers() } doReturn listOf(userA, userB) - val original = Request.Builder() - .url(PREVIEW_URL) - .header("Authorization", Credentials.basic("userA", "tokenA")) - .build() - // OkHttp drops the credentials when following a redirect to another host. - val redirected = Request.Builder().url("https://other.example.org/image").build() - - interceptor.intercept(chainAnswering401(original, answeredRequest = redirected)) - - verifyBlocking(userManager, never()) { scheduleUserForDeletionWithId(any()) } - } - - private fun chainAnswering401(request: Request, answeredRequest: Request = request): Interceptor.Chain { - val response = Response.Builder() - .request(answeredRequest) - .protocol(Protocol.HTTP_1_1) - .code(HTTP_UNAUTHORIZED) - .message("Unauthorized") - .build() - return mock { - on { request() } doReturn request - on { proceed(any()) } doReturn response - } - } - - companion object { - private const val BASE_URL = "https://cloud.example.com" - private const val PREVIEW_URL = "$BASE_URL/index.php/core/preview?fileId=1" - private const val HTTP_UNAUTHORIZED = 401 - } -}