From f3dc0233c9175c56cef45f2479b76fab7fde23c4 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 16 Oct 2026 16:13:56 +0200 Subject: [PATCH 1/2] fix(ssl): cache the accounts for choosing client certificates KeyManager queried the database on every TLS handshake, and again for every certificate chain and private key lookup. It also got the default account via getDefaultUser(), which can change the active account as a side effect. The accounts are now kept in memory by ClientCertificateUsers and updated from a Room flow. The active account for resolving conflicting certificates on one server is taken from the cached users' current flag. KeyManager now extends X509ExtendedKeyManager so SSLEngine based connections choose the certificate by their peer as well, and the socket host fallback no longer does a reverse DNS lookup. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marcel Hibbe --- .../talk/dagger/modules/RestModule.java | 3 +- .../com/nextcloud/talk/data/user/UsersDao.kt | 3 + .../talk/data/user/UsersRepository.kt | 1 + .../talk/data/user/UsersRepositoryImpl.kt | 2 + .../com/nextcloud/talk/users/UserManager.kt | 5 + .../utils/preview/ComposePreviewUtilsDaos.kt | 2 + .../utils/ssl/ClientCertificateSelector.kt | 21 ++-- .../talk/utils/ssl/ClientCertificateUsers.kt | 39 ++++++++ .../nextcloud/talk/utils/ssl/KeyManager.java | 95 ++++++++----------- .../ssl/ClientCertificateSelectorTest.kt | 59 ++++++++---- .../utils/ssl/ClientCertificateUsersTest.kt | 66 +++++++++++++ 11 files changed, 219 insertions(+), 77 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsers.kt create mode 100644 app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsersTest.kt 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 c70bafc1bd6..00a59da2f80 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 @@ -20,6 +20,7 @@ import com.nextcloud.talk.utils.ApiUtils; import com.nextcloud.talk.utils.RejectedCredentialsInterceptor; import com.nextcloud.talk.utils.preferences.AppPreferences; +import com.nextcloud.talk.utils.ssl.ClientCertificateUsers; import com.nextcloud.talk.utils.ssl.KeyManager; import com.nextcloud.talk.utils.ssl.SSLSocketFactoryCompat; import com.nextcloud.talk.utils.ssl.TrustManager; @@ -130,7 +131,7 @@ KeyManager provideKeyManager(AppPreferences appPreferences, UserManager userMana KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); kmf.init(keyStore, null); X509KeyManager origKm = (X509KeyManager) kmf.getKeyManagers()[0]; - return new KeyManager(origKm, userManager, appPreferences); + return new KeyManager(origKm, new ClientCertificateUsers(userManager), appPreferences); } catch (KeyStoreException e) { Log.e(TAG, "KeyStoreException " + e.getLocalizedMessage()); } catch (CertificateException e) { diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt index cc5c5e52045..f0341dd9bfd 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt @@ -54,6 +54,9 @@ interface UsersDao { @Query("SELECT * FROM User where scheduledForDeletion != 1") suspend fun getUsers(): List + @Query("SELECT * FROM User where scheduledForDeletion != 1") + fun getUsersFlow(): Flow> + @Query("SELECT * FROM User where id = :id") suspend fun getUserWithId(id: Long): UserEntity? diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt index bf79cf82a96..509b5adf266 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt @@ -19,6 +19,7 @@ interface UsersRepository { suspend fun getActiveUser(): User? fun getActiveUserFlow(): Flow suspend fun getUsers(): List + fun getUsersFlow(): Flow> suspend fun getUserWithId(id: Long): User? fun getUserWithIdFlow(id: Long): Flow suspend fun repairMultipleActiveUsers(): Int diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt index e995d3726de..f3bfccd0bc0 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt @@ -33,6 +33,8 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { override suspend fun getUsers(): List = UserMapper.toModel(usersDao.getUsers()) + override fun getUsersFlow(): Flow> = usersDao.getUsersFlow().map { UserMapper.toModel(it) } + override suspend fun getUserWithId(id: Long): User? = UserMapper.toModel(usersDao.getUserWithId(id)) override fun getUserWithIdFlow(id: Long): Flow = diff --git a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt index 1497c6d4d23..8a22e57a06d 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -32,6 +32,11 @@ class UserManager internal constructor(private val userRepository: UsersReposito suspend fun getUsers(): List = userRepository.getUsers() + /** + * Emits the users not scheduled for deletion whenever the user table changes. + */ + fun usersFlow(): Flow> = userRepository.getUsersFlow() + suspend fun getUsersScheduledForDeletion(): List = userRepository.getUsersScheduledForDeletion() suspend fun getUsersNotScheduledForDeletion(): List = userRepository.getUsersNotScheduledForDeletion() diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt index 2ce7abc5034..f238a61120e 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt @@ -226,6 +226,8 @@ class DummyUserDaoImpl : UsersDao { override suspend fun getUsers(): List = dummyUsers.filter { !it.scheduledForDeletion } + override fun getUsersFlow(): Flow> = flowOf(dummyUsers.filter { !it.scheduledForDeletion }) + override suspend fun getUserWithId(id: Long): UserEntity? = dummyUsers.find { it.id == id } override fun getUserWithIdFlow(id: Long): Flow = flowOf(dummyUsers.find { it.id == id }) diff --git a/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelector.kt b/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelector.kt index fc5aac1f01c..5f6d19cdde8 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelector.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelector.kt @@ -15,26 +15,35 @@ import java.net.URISyntaxException * * The TLS handshake only knows the server (host and port), not the account of the request. So the certificate is * chosen by the server: the one of the accounts on it. Only if accounts on the same server use different certificates, - * the default account decides, as the connection may then be reused by all of them. + * the active account decides, as the connection may then be reused by all of them. */ object ClientCertificateSelector { /** * Returns the alias of the client certificate for a connection to [host] and [port], or null if no account on - * that server uses one. Without a [host], the certificate of [defaultUser] is used. A negative [port] is unknown, - * then the host alone identifies the server. + * that server uses one. Without a [host], the certificate of the active account is used. A negative [port] is + * unknown, then the host alone identifies the server. */ @JvmStatic - fun selectAlias(host: String?, port: Int, users: List, defaultUser: User?): String? { + fun selectAlias(host: String?, port: Int, users: List): String? { val usersOnServer = users.filter { host != null && isOnServer(it, host, port) } val aliases = usersOnServer.mapNotNull { aliasOf(it) }.toSet() return when { - host == null -> aliasOf(defaultUser) + host == null -> aliasOf(activeUserOf(users)) aliases.size <= 1 -> aliases.firstOrNull() - else -> defaultUser?.takeIf { default -> usersOnServer.any { it.id == default.id } }?.let { aliasOf(it) } + else -> aliasOf(activeUserOf(usersOnServer)) } } + /** + * Returns the aliases of the client certificates of all [users]. + */ + @JvmStatic + fun aliasesOf(users: List): Set = users.mapNotNull { aliasOf(it) }.toSet() + + // Matches UsersDao.getActiveUser(), which prefers the highest id if several users are marked as active. + private fun activeUserOf(users: List): User? = users.filter { it.current }.maxByOrNull { it.id ?: -1L } + private fun isOnServer(user: User, host: String, port: Int): Boolean { val uri = uriOf(user) ?: return false return uri.host.equals(host, ignoreCase = true) && (port < 0 || effectivePortOf(uri) == port) diff --git a/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsers.kt b/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsers.kt new file mode 100644 index 00000000000..dfefe940a1d --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsers.kt @@ -0,0 +1,39 @@ +/* + * 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.ssl + +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.users.UserManager +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking + +/** + * The accounts [KeyManager] chooses client certificates from. They are kept up to date from the database, so TLS + * handshakes don't have to query it. + */ +class ClientCertificateUsers @JvmOverloads constructor( + private val userManager: UserManager, + scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) +) { + @Volatile + private var users: List? = null + + init { + scope.launch { + userManager.usersFlow().collect { users = it } + } + } + + /** + * The users not scheduled for deletion. Until the database emitted them for the first time, they are queried + * directly. + */ + fun getUsers(): List = users ?: runBlocking { userManager.getUsers() } +} diff --git a/app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java b/app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java index 338e613dff7..4ca9439329a 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java +++ b/app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java @@ -13,39 +13,38 @@ import android.util.Log; import com.nextcloud.talk.application.NextcloudTalkApplication; -import com.nextcloud.talk.data.user.model.User; -import com.nextcloud.talk.users.UserManager; import com.nextcloud.talk.utils.preferences.AppPreferences; +import java.net.InetSocketAddress; import java.net.Socket; +import java.net.SocketAddress; import java.security.Principal; import java.security.PrivateKey; import java.security.cert.X509Certificate; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; -import java.util.List; import java.util.Set; +import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; +import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509KeyManager; -import kotlin.coroutines.EmptyCoroutineContext; -import kotlinx.coroutines.BuildersKt; import androidx.annotation.Nullable; -public class KeyManager implements X509KeyManager { +public class KeyManager extends X509ExtendedKeyManager { private static final String TAG = "KeyManager"; private final X509KeyManager keyManager; - private UserManager userManager; + private final ClientCertificateUsers clientCertificateUsers; private AppPreferences appPreferences; private Context context; - public KeyManager(X509KeyManager keyManager, UserManager userManager, AppPreferences appPreferences) { + public KeyManager(X509KeyManager keyManager, ClientCertificateUsers clientCertificateUsers, + AppPreferences appPreferences) { this.keyManager = keyManager; - this.userManager = userManager; + this.clientCertificateUsers = clientCertificateUsers; this.appPreferences = appPreferences; context = NextcloudTalkApplication.Companion.getSharedApplication().getApplicationContext(); @@ -58,31 +57,25 @@ public KeyManager(X509KeyManager keyManager, UserManager userManager, AppPrefere */ @Override public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) { - String alias; - List users = new ArrayList<>(); - User defaultUser = null; - try { - users = BuildersKt.runBlocking( - EmptyCoroutineContext.INSTANCE, - (scope, continuation) -> userManager.getUsers(continuation)); - defaultUser = BuildersKt.runBlocking( - EmptyCoroutineContext.INSTANCE, - (scope, continuation) -> userManager.getDefaultUser(continuation)); - } catch (InterruptedException e) { - Log.e(TAG, "Interrupted while getting the users: " + e.getLocalizedMessage()); - Thread.currentThread().interrupt(); - } - if (!TextUtils.isEmpty(alias = ClientCertificateSelector.selectAlias( - getPeerHost(socket), - getPeerPort(socket), - users, - defaultUser)) || - !TextUtils.isEmpty(alias = appPreferences.getTemporaryClientCertAlias()) - && new ArrayList<>(Arrays.asList(getClientAliases())).contains(alias)) { + return chooseAlias(getPeerHost(socket), getPeerPort(socket)); + } + + @Override + public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { + return engine == null ? chooseAlias(null, -1) : chooseAlias(engine.getPeerHost(), engine.getPeerPort()); + } + + /** + * Falls back to the certificate chosen during login, as the account it is for doesn't exist yet. + */ + @Nullable + private String chooseAlias(@Nullable String host, int port) { + String alias = ClientCertificateSelector.selectAlias(host, port, clientCertificateUsers.getUsers()); + if (!TextUtils.isEmpty(alias)) { return alias; } - - return null; + alias = appPreferences.getTemporaryClientCertAlias(); + return TextUtils.isEmpty(alias) ? null : alias; } private static int getPeerPort(@Nullable Socket socket) { @@ -98,6 +91,10 @@ private static int getPeerPort(@Nullable Socket socket) { return -1; } + /** + * The host name the connection was opened for. Unlike InetAddress.getHostName(), the fallback never does a reverse + * DNS lookup. + */ @Nullable private static String getPeerHost(@Nullable Socket socket) { if (socket instanceof SSLSocket) { @@ -106,8 +103,11 @@ private static String getPeerHost(@Nullable Socket socket) { return session.getPeerHost(); } } - if (socket != null && socket.getInetAddress() != null) { - return socket.getInetAddress().getHostName(); + if (socket != null) { + SocketAddress address = socket.getRemoteSocketAddress(); + if (address instanceof InetSocketAddress) { + return ((InetSocketAddress) address).getHostString(); + } } return null; } @@ -152,7 +152,7 @@ private PrivateKey getPrivateKeyForAlias(@Nullable String alias) { @Override public X509Certificate[] getCertificateChain(String s) { - if (new ArrayList<>(Arrays.asList(getClientAliases())).contains(s)) { + if (isClientAlias(s)) { return getCertificatesForAlias(s); } @@ -160,30 +160,19 @@ public X509Certificate[] getCertificateChain(String s) { } private String[] getClientAliases() { - Set aliases = new HashSet<>(); + Set aliases = new HashSet<>(ClientCertificateSelector.aliasesOf(clientCertificateUsers.getUsers())); String alias; if (!TextUtils.isEmpty(alias = appPreferences.getTemporaryClientCertAlias())) { aliases.add(alias); } - List userEntities = new ArrayList<>(); - try { - userEntities = BuildersKt.runBlocking( - EmptyCoroutineContext.INSTANCE, - (scope, continuation) -> userManager.getUsers(continuation)); - } catch (InterruptedException e) { - Log.e(TAG, "Interrupted while getting users: " + e.getLocalizedMessage()); - Thread.currentThread().interrupt(); - } - for (int i = 0; i < userEntities.size(); i++) { - if (!TextUtils.isEmpty(alias = userEntities.get(i).getClientCertificate())) { - aliases.add(alias); - } - } - return aliases.toArray(new String[aliases.size()]); } + private boolean isClientAlias(String alias) { + return Arrays.asList(getClientAliases()).contains(alias); + } + @Override public String[] getClientAliases(String s, Principal[] principals) { return getClientAliases(); @@ -196,7 +185,7 @@ public String[] getServerAliases(String s, Principal[] principals) { @Override public PrivateKey getPrivateKey(String s) { - if (new ArrayList<>(Arrays.asList(getClientAliases())).contains(s)) { + if (isClientAlias(s)) { return getPrivateKeyForAlias(s); } diff --git a/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelectorTest.kt b/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelectorTest.kt index b6bad64fbfb..13f00274884 100644 --- a/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelectorTest.kt +++ b/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelectorTest.kt @@ -19,49 +19,74 @@ class ClientCertificateSelectorTest { private val otherCertB = User(id = 4, baseUrl = "https://b.example.com", clientCertificate = "otherB") @Test - fun `the certificate of the server's account is used, not the default account's`() { - assertEquals("certA", select(HOST_A, HTTPS_PORT, listOf(withCertA, withoutCertB), withoutCertB)) + fun `the certificate of the server's account is used, not the active account's`() { + assertEquals("certA", select(HOST_A, HTTPS_PORT, listOf(withCertA, active(withoutCertB)))) } @Test - fun `no certificate for a server whose accounts use none, even if the default account has one`() { - assertNull(select(HOST_B, HTTPS_PORT, listOf(withCertA, withoutCertB), withCertA)) + fun `no certificate for a server whose accounts use none, even if the active account has one`() { + assertNull(select(HOST_B, HTTPS_PORT, listOf(active(withCertA), withoutCertB))) + } + + @Test + fun `no certificate for a server without accounts`() { + assertNull(select("c.example.com", HTTPS_PORT, listOf(active(withCertA), withCertB))) } @Test fun `accounts on the same server with one certificate share it`() { - assertEquals("certB", select(HOST_B, HTTPS_PORT, listOf(withoutCertB, withCertB), withoutCertB)) + assertEquals("certB", select(HOST_B, HTTPS_PORT, listOf(active(withoutCertB), withCertB))) } @Test - fun `different certificates on the same server are decided by the default account`() { - val users = listOf(withCertB, otherCertB, withCertA) + fun `different certificates on the same server are decided by the active account`() { + assertEquals("otherB", select(HOST_B, HTTPS_PORT, listOf(withCertB, active(otherCertB), withCertA))) + assertNull(select(HOST_B, HTTPS_PORT, listOf(withCertB, otherCertB, active(withCertA)))) + } - assertEquals("otherB", select(HOST_B, HTTPS_PORT, users, otherCertB)) - assertNull(select(HOST_B, HTTPS_PORT, users, withCertA)) + @Test + fun `the host is matched case-insensitively`() { + assertEquals("certA", select("A.Example.COM", HTTPS_PORT, listOf(withCertA))) } @Test fun `accounts on another port of the same host are another server`() { val onOtherPort = User(id = 5, baseUrl = "https://b.example.com:8443", clientCertificate = "portB") - val users = listOf(withCertB, onOtherPort) - assertEquals("certB", select(HOST_B, HTTPS_PORT, users, onOtherPort)) - assertEquals("portB", select(HOST_B, OTHER_PORT, users, withCertB)) + assertEquals("certB", select(HOST_B, HTTPS_PORT, listOf(withCertB, active(onOtherPort)))) + assertEquals("portB", select(HOST_B, OTHER_PORT, listOf(active(withCertB), onOtherPort))) } @Test fun `an unknown port matches the host only`() { - assertEquals("certA", select(HOST_A, UNKNOWN_PORT, listOf(withCertA), null)) + assertEquals("certA", select(HOST_A, UNKNOWN_PORT, listOf(withCertA))) } @Test - fun `without a host the default account's certificate is used`() { - assertEquals("certA", select(null, UNKNOWN_PORT, listOf(withCertA, withCertB), withCertA)) + fun `accounts with an invalid base url match no server`() { + val invalid = User(id = 6, baseUrl = "https://a.example.com/ invalid", clientCertificate = "invalid") + + assertEquals("certA", select(HOST_A, HTTPS_PORT, listOf(invalid, withCertA))) } - private fun select(host: String?, port: Int, users: List, defaultUser: User?) = - ClientCertificateSelector.selectAlias(host, port, users, defaultUser) + @Test + fun `without a host the active account's certificate is used`() { + assertEquals("certA", select(null, UNKNOWN_PORT, listOf(active(withCertA), withCertB))) + assertNull(select(null, UNKNOWN_PORT, listOf(withCertA, withCertB))) + } + + @Test + fun `the aliases of all accounts are distinct and skip accounts without certificate`() { + assertEquals( + setOf("certA", "certB"), + ClientCertificateSelector.aliasesOf(listOf(withCertA, withoutCertB, withCertB, withCertB.copy(id = 7))) + ) + } + + private fun active(user: User) = user.copy(current = true) + + private fun select(host: String?, port: Int, users: List) = + ClientCertificateSelector.selectAlias(host, port, users) companion object { private const val HOST_A = "a.example.com" diff --git a/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsersTest.kt b/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsersTest.kt new file mode 100644 index 00000000000..0746c8e993a --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateUsersTest.kt @@ -0,0 +1,66 @@ +/* + * 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.ssl + +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.users.UserManager +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verifyBlocking +import org.mockito.kotlin.whenever +import org.mockito.kotlin.wheneverBlocking + +class ClientCertificateUsersTest { + + private val usersFlow = MutableSharedFlow>(replay = 1) + private val userManager: UserManager = mock() + private val scope = CoroutineScope(UnconfinedTestDispatcher()) + + @After + fun tearDown() { + scope.cancel() + } + + @Test + fun `the users are queried until the database emitted them`() { + val stored = listOf(User(id = 1, clientCertificate = "certA")) + whenever(userManager.usersFlow()).thenReturn(usersFlow) + wheneverBlocking { userManager.getUsers() }.thenReturn(stored) + + assertEquals(stored, ClientCertificateUsers(userManager, scope).getUsers()) + } + + @Test + fun `the emitted users are used without querying the database`() { + val emitted = listOf(User(id = 2, clientCertificate = "certB")) + whenever(userManager.usersFlow()).thenReturn(usersFlow) + val clientCertificateUsers = ClientCertificateUsers(userManager, scope) + + usersFlow.tryEmit(emitted) + + assertEquals(emitted, clientCertificateUsers.getUsers()) + verifyBlocking(userManager, never()) { getUsers() } + } + + @Test + fun `changes of the users replace the cached ones`() { + whenever(userManager.usersFlow()).thenReturn(usersFlow) + val clientCertificateUsers = ClientCertificateUsers(userManager, scope) + + usersFlow.tryEmit(listOf(User(id = 1, clientCertificate = "certA"))) + usersFlow.tryEmit(emptyList()) + + assertEquals(emptyList(), clientCertificateUsers.getUsers()) + } +} From 972530770d5c2cdde97ff2959ec766aa4f798984 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 16 Oct 2026 16:25:15 +0200 Subject: [PATCH 2/2] fix(login): use the shared client for the one-time password request The request for the app password with the one-time credentials used a new OkHttpClient without the app's KeyManager and TrustManager. On servers that require a client certificate or use a certificate trusted only by the user, it failed, so the login could not be completed. It now uses the client of the data source, which is based on the shared one. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marcel Hibbe --- .../talk/account/data/network/NetworkLoginDataSource.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 aa1b5aa104a..6eaf38cec85 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 @@ -80,8 +80,7 @@ class NetworkLoginDataSource(val okHttpClient: OkHttpClient) { .header("User-Agent", ApiUtils.loginUserAgent) .build() - val newOkHttpClient = OkHttpClient() - newOkHttpClient.newCall(request).execute().use { response -> + client.newCall(request).execute().use { response -> if (response.code == HTTP_TOO_MANY_REQUESTS) { throw TooManyLoginAttemptsException("Too many failed logins from this IP: $response") }