diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ec085e6ec08..fc1053a4bfe 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -36,7 +36,7 @@ pbandk = "0.14.2" turbine = "1.0.0" avs = "9.2.22" jna = "5.6.0" -core-crypto = "1.0.0-pre.6+v1-schemafix-003" +core-crypto = "1.0.0-pre.6+v1-schemafix-007" core-crypto-multiplatform = "0.6.0-rc.3-multiplatform-pre1" completeKotlin = "1.1.0" desugar-jdk = "1.1.5" diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index c3989502541..ea063e8808c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -24,6 +24,7 @@ import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.functional.Either import com.wire.kalium.network.exceptions.KaliumException import com.wire.kalium.network.exceptions.isFederationDenied +import com.wire.kalium.network.exceptions.isFederationNotEnabled import com.wire.kalium.network.utils.NetworkResponse import io.ktor.utils.io.errors.IOException import kotlinx.coroutines.flow.Flow @@ -147,6 +148,7 @@ sealed class NetworkFailure : CoreFailure { data class General(val label: String) : FederatedBackendFailure() data class FederationDenied(val label: String) : FederatedBackendFailure() + data class FederationNotEnabled(val label: String) : FederatedBackendFailure() data class ConflictingBackends(override val domains: List) : FederatedBackendFailure(), RetryableFailure @@ -200,6 +202,8 @@ internal inline fun wrapApiRequest(networkCall: () -> NetworkResponse< exception is KaliumException.FederationError -> { if (exception.isFederationDenied()) { Either.Left(NetworkFailure.FederatedBackendFailure.FederationDenied(exception.errorResponse.label)) + } else if (exception.isFederationNotEnabled()) { + Either.Left(NetworkFailure.FederatedBackendFailure.FederationNotEnabled(exception.errorResponse.label)) } else { Either.Left(NetworkFailure.FederatedBackendFailure.General(exception.errorResponse.label)) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt index c1ed4da2348..25d2e8815f0 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt @@ -18,7 +18,9 @@ package com.wire.kalium.logic.data.user +import com.wire.kalium.logger.obfuscateDomain import com.wire.kalium.logic.CoreFailure +import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.StorageFailure import com.wire.kalium.logic.data.conversation.MemberMapper import com.wire.kalium.logic.data.conversation.Recipient @@ -37,12 +39,12 @@ import com.wire.kalium.logic.data.team.Team import com.wire.kalium.logic.data.team.TeamMapper import com.wire.kalium.logic.data.user.type.DomainUserTypeMapper import com.wire.kalium.logic.data.user.type.UserEntityTypeMapper -import com.wire.kalium.logic.data.user.type.isFederated import com.wire.kalium.logic.di.MapperProvider import com.wire.kalium.logic.failure.SelfUserDeleted import com.wire.kalium.logic.feature.SelfTeamIdProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.flatMap +import com.wire.kalium.logic.functional.flatMapLeft import com.wire.kalium.logic.functional.fold import com.wire.kalium.logic.functional.foldToEitherWhileRight import com.wire.kalium.logic.functional.getOrNull @@ -117,6 +119,7 @@ internal interface UserRepository { * when backends stops federating. */ suspend fun defederateUser(userId: UserId): Either + // TODO: move to migration repo suspend fun insertUsersIfUnknown(users: List): Either suspend fun fetchUserInfo(userId: UserId): Either @@ -155,12 +158,12 @@ internal class UserDataSource internal constructor( ) : UserRepository { /** - * In case of federated users, we need to refresh their info every time. - * Since the current backend implementation at wire does not emit user events across backends. + * Stores the last time a user's details were fetched from remote. * - * This is an in-memory cache, to help avoid unnecessary requests in a time window. + * @see Event.User.Update + * @see USER_DETAILS_MAX_AGE */ - private val federatedUsersExpirationCache = ConcurrentMap() + private val userDetailsRefreshInstantCache = ConcurrentMap() override suspend fun fetchSelfUser(): Either = wrapApiRequest { selfApi.getSelfInfo() } .flatMap { userDTO -> @@ -186,20 +189,25 @@ internal class UserDataSource internal constructor( .map { userEntity -> userEntity?.let { publicUserMapper.fromUserEntityToOtherUser(userEntity) } }.onEach { otherUser -> - processFederatedUserRefresh(userId, otherUser) + if (otherUser != null) { + refreshUserDetailsIfNeeded(userId) + } } /** - * Only in case of federated users and if it's expired or not cached, we fetch and refresh the user info. + * Only refresh user profiles if it wasn't fetched recently. + * + * @see userDetailsRefreshInstantCache + * @see USER_DETAILS_MAX_AGE */ - private suspend fun processFederatedUserRefresh(userId: UserId, otherUser: OtherUser?) { - if (otherUser != null && otherUser.userType.isFederated() - && federatedUsersExpirationCache[userId]?.let { DateTimeUtil.currentInstant() > it } != false - ) { + private suspend fun refreshUserDetailsIfNeeded(userId: UserId) { + val now = DateTimeUtil.currentInstant() + val wasFetchedRecently = userDetailsRefreshInstantCache[userId]?.let { now < it + USER_DETAILS_MAX_AGE } ?: false + if (!wasFetchedRecently) { fetchUserInfo(userId).also { - kaliumLogger.d("Federated user, refreshing user info from API after $FEDERATED_USER_TTL") + kaliumLogger.d("Federated user, refreshing user info from API after $USER_DETAILS_MAX_AGE") } - federatedUsersExpirationCache[userId] = DateTimeUtil.currentInstant().plus(FEDERATED_USER_TTL) + userDetailsRefreshInstantCache[userId] = now } } @@ -237,6 +245,24 @@ internal class UserDataSource internal constructor( ) } } + .flatMapLeft { error -> + if (error is NetworkFailure.FederatedBackendFailure.FederationNotEnabled) { + val domains = qualifiedUserIdList + .filterNot { it.domain == selfUserId.domain } + .map { it.domain.obfuscateDomain() } + .toSet() + val domainNames = domains.joinToString(separator = ", ") + kaliumLogger.e("User ids contains different domains when federation is not enabled by backend: $domainNames") + wrapApiRequest { + userDetailsApi.getMultipleUsers( + ListUserRequest.qualifiedIds(qualifiedUserIdList.filter { it.domain == selfUserId.domain } + .map { userId -> userId.toApi() }) + ) + } + } else { + Either.Left(error) + } + } .flatMap { listUserProfileDTO -> if (listUserProfileDTO.usersFailed.isNotEmpty()) { kaliumLogger.d("Handling ${listUserProfileDTO.usersFailed.size} failed users") @@ -503,7 +529,16 @@ internal class UserDataSource internal constructor( companion object { internal const val SELF_USER_ID_KEY = "selfUserID" - internal val FEDERATED_USER_TTL = 5.minutes + + /** + * Maximum age for user details. + * + * The USER_DETAILS_MAX_AGE constant represents the maximum age in minutes that user details can be considered valid. After + * this duration, the user details should be refreshed. + * + * This is needed because some users don't get `user.update` events, so we need to refresh their details every so often. + */ + internal val USER_DETAILS_MAX_AGE = 5.minutes internal const val BATCH_SIZE = 500 } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt index 5c3fc08aa7e..9285ffd7f8d 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/UserRepositoryTest.kt @@ -36,6 +36,7 @@ import com.wire.kalium.logic.framework.TestUser.LIST_USERS_DTO import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.getOrNull import com.wire.kalium.logic.sync.receiver.UserEventReceiverTest +import com.wire.kalium.logic.test_util.TestNetworkException.federationNotEnabled import com.wire.kalium.logic.util.shouldFail import com.wire.kalium.logic.util.shouldSucceed import com.wire.kalium.network.api.base.authenticated.TeamsApi @@ -201,6 +202,26 @@ class UserRepositoryTest { .wasNotInvoked() } + @Test + fun givenAnUserIdListWithDifferentDomain_whenApiReturnsFederationDisabledError_thenShouldTryToFetchOnlyUsersWithSelfDomain() = runTest { + // given + val requestedUserIds = setOf(TestUser.OTHER_USER_ID, TestUser.OTHER_FEDERATED_USER_ID) + val (arrangement, userRepository) = Arrangement() + .withGetMultipleUsersApiRequestFederationNotEnabledError() + .arrange() + // when + userRepository.fetchUsersByIds(requestedUserIds).shouldFail() + // then + verify(arrangement.userDetailsApi) + .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) + .with(eq(QualifiedUserIdListRequest(requestedUserIds.map { it.toApi() }.toList()))) + .wasInvoked(exactly = once) + verify(arrangement.userDetailsApi) + .suspendFunction(arrangement.userDetailsApi::getMultipleUsers) + .with(eq(QualifiedUserIdListRequest(listOf(TestUser.OTHER_USER_ID.toApi())))) + .wasInvoked(exactly = once) + } + @Test fun givenAnEmptyUserIdListFromSameDomainAsSelf_whenFetchingUsers_thenShouldNotFetchMultipleUsersAndSucceed() = runTest { // given @@ -356,9 +377,9 @@ class UserRepositoryTest { } @Test - fun givenAKnownNOTFederatedUser_whenGettingFromDb_thenShouldNotRefreshItsDataFromAPI() = runTest { + fun givenAKnownUser_whenGettingFromDb_thenShouldRefreshItsDataFromAPI() = runTest { val (arrangement, userRepository) = Arrangement() - .withUserDaoReturning(TestUser.ENTITY.copy(userType = UserTypeEntity.STANDARD)) + .withUserDaoReturning(TestUser.ENTITY) .withSuccessfulGetUsersInfo() .arrange() @@ -368,22 +389,22 @@ class UserRepositoryTest { verify(arrangement.userDetailsApi) .suspendFunction(arrangement.userDetailsApi::getUserInfo) .with(any()) - .wasNotInvoked() + .wasInvoked(exactly = once) verify(arrangement.userDAO) .suspendFunction(arrangement.userDAO::upsertTeamMembers) .with(any()) - .wasNotInvoked() + .wasInvoked(exactly = once) verify(arrangement.userDAO) .suspendFunction(arrangement.userDAO::upsertUsers) .with(any()) - .wasNotInvoked() + .wasInvoked(exactly = once) } } @Test - fun givenAKnownFederatedUser_whenGettingFromDbAndCacheValid_thenShouldNOTRefreshItsDataFromAPI() = runTest { + fun givenAKnownUser_whenGettingFromDbAndCacheValid_thenShouldNOTRefreshItsDataFromAPI() = runTest { val (arrangement, userRepository) = Arrangement() - .withUserDaoReturning(TestUser.ENTITY.copy(userType = UserTypeEntity.FEDERATED)) + .withUserDaoReturning(TestUser.ENTITY) .withSuccessfulGetUsersInfo() .arrange() @@ -743,6 +764,13 @@ class UserRepositoryTest { .thenReturn(NetworkResponse.Success(result, mapOf(), HttpStatusCode.OK.value)) } + fun withGetMultipleUsersApiRequestFederationNotEnabledError() = apply { + given(userDetailsApi) + .suspendFunction(userDetailsApi::getMultipleUsers) + .whenInvokedWith(any()) + .thenReturn(NetworkResponse.Error(federationNotEnabled)) + } + fun withUpdateDisplayNameApiRequestResponse(response: NetworkResponse) = apply { given(selfApi) .suspendFunction(selfApi::updateSelf) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/test_util/TestNetworkException.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/test_util/TestNetworkException.kt index 850cafe01ec..23da8afe25f 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/test_util/TestNetworkException.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/test_util/TestNetworkException.kt @@ -130,6 +130,10 @@ object TestNetworkException { val guestLinkDisables = KaliumException.InvalidRequestError( ErrorResponse(409, "Guest links are disabled", "guest-links-disabled") ) + + val federationNotEnabled = KaliumException.FederationError( + ErrorResponse(400, "no federator configured", "federation-not-enabled") + ) } object TestNetworkResponseError { diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/model/ErrorResponse.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/model/ErrorResponse.kt index d8167d436de..3bfe1af82c7 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/model/ErrorResponse.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/model/ErrorResponse.kt @@ -28,7 +28,7 @@ data class ErrorResponse( @SerialName("label") val label: String, @SerialName("data") val cause: Cause? = null, ) { - fun isFederationError() = cause?.type == "federation" + fun isFederationError() = cause?.type == "federation" || label.contains("federation") } @Serializable diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt index 87575b423ae..df48be0e151 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt @@ -32,6 +32,7 @@ import com.wire.kalium.network.exceptions.NetworkErrorLabel.BLACKLISTED_EMAIL import com.wire.kalium.network.exceptions.NetworkErrorLabel.DOMAIN_BLOCKED_FOR_REGISTRATION import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_DENIED import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_FAILURE +import com.wire.kalium.network.exceptions.NetworkErrorLabel.FEDERATION_NOT_ENABLED import com.wire.kalium.network.exceptions.NetworkErrorLabel.GUEST_LINKS_DISABLED import com.wire.kalium.network.exceptions.NetworkErrorLabel.HANDLE_EXISTS import com.wire.kalium.network.exceptions.NetworkErrorLabel.INVALID_CODE @@ -224,3 +225,4 @@ val KaliumException.InvalidRequestError.authenticationCodeFailure: Authenticatio } fun KaliumException.FederationError.isFederationDenied() = errorResponse.label == FEDERATION_DENIED +fun KaliumException.FederationError.isFederationNotEnabled() = errorResponse.label == FEDERATION_NOT_ENABLED diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt index bba49a103d6..8159370cbef 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt @@ -42,15 +42,18 @@ internal object NetworkErrorLabel { const val MLS_KEY_PACKAGE_REF_NOT_FOUND = "mls-key-package-ref-not-found" const val MLS_MISSING_GROUP_INFO = "mls-missing-group-info" const val UNKNOWN_CLIENT = "unknown-client" - const val FEDERATION_FAILURE = "federation-remote-error" const val NOT_TEAM_MEMBER = "no-team-member" const val NO_CONVERSATION = "no-conversation" const val NO_CONVERSATION_CODE = "no-conversation-code" const val GUEST_LINKS_DISABLED = "guest-links-disabled" const val ACCESS_DENIED = "access-denied" const val WRONG_CONVERSATION_PASSWORD = "invalid-conversation-password" - const val FEDERATION_UNREACHABLE_DOMAINS = "federation-unreachable-domains-error" + + // Federation + const val FEDERATION_FAILURE = "federation-remote-error" const val FEDERATION_DENIED = "federation-denied" + const val FEDERATION_NOT_ENABLED = "federation-not-enabled" + const val FEDERATION_UNREACHABLE_DOMAINS = "federation-unreachable-domains-error" object KaliumCustom { const val MISSING_REFRESH_TOKEN = "missing-refresh_token"