diff --git a/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt b/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt index 0a7d511741d0..0d66fff3b402 100644 --- a/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt +++ b/app/src/androidTest/java/com/owncloud/android/EncryptedFoldersIT.kt @@ -9,11 +9,14 @@ package com.owncloud.android import com.nextcloud.client.account.UserAccountManager import com.nextcloud.client.account.UserAccountManagerImpl +import com.nextcloud.client.core.ClockImpl import com.nextcloud.client.database.entity.FileEntity import com.nextcloud.client.network.NetworkModule +import com.nextcloud.repository.ClientRepository import com.nextcloud.test.SinceServer import com.nextcloud.utils.e2ee.E2EEActionResolver import com.nextcloud.utils.e2ee.E2EEKeyInspector +import com.nextcloud.utils.e2ee.E2EEServerKeyCache import com.owncloud.android.datamodel.OCFile import com.owncloud.android.lib.resources.e2ee.ToggleEncryptionRemoteOperation import com.owncloud.android.lib.resources.status.GetCapabilitiesRemoteOperation @@ -33,6 +36,7 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import org.mockito.kotlin.mock @SinceServer(majorVersion = 30) open class EncryptedFoldersIT : AbstractOnServerIT() { @@ -61,12 +65,18 @@ open class EncryptedFoldersIT : AbstractOnServerIT() { init { val accountManager: UserAccountManager = UserAccountManagerImpl.fromContext(targetContext) + val clientRepository: ClientRepository = mock { + onBlocking { getOwncloudClient() }.thenReturn(client) + onBlocking { getNextcloudClient() }.thenReturn(nextcloudClient) + } val inspector = E2EEKeyInspector( targetContext, storageManager, CertificateValidator(), arbitraryDataProvider, - accountManager + accountManager, + clientRepository, + E2EEServerKeyCache(ClockImpl()) ) e2eeActionResolver = E2EEActionResolver( storageManager, diff --git a/app/src/main/java/com/nextcloud/client/di/AppModule.kt b/app/src/main/java/com/nextcloud/client/di/AppModule.kt index 8154074c608d..c3f5dd10586b 100644 --- a/app/src/main/java/com/nextcloud/client/di/AppModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/AppModule.kt @@ -46,6 +46,7 @@ import com.nextcloud.repository.ClientRepository import com.nextcloud.repository.RemoteClientRepository import com.nextcloud.utils.e2ee.E2EEActionResolver import com.nextcloud.utils.e2ee.E2EEKeyInspector +import com.nextcloud.utils.e2ee.E2EEServerKeyCache import com.nextcloud.utils.thumbnail.FolderThumbnailGenerator import com.owncloud.android.authentication.PassCodeManager import com.owncloud.android.datamodel.ArbitraryDataProvider @@ -239,13 +240,17 @@ internal class AppModule { fileDataStorageManager: FileDataStorageManager, certificateValidator: CertificateValidator, arbitraryDataProvider: ArbitraryDataProvider, - accountManager: UserAccountManager + accountManager: UserAccountManager, + clientRepository: ClientRepository, + serverKeyCache: E2EEServerKeyCache ): E2EEKeyInspector = E2EEKeyInspector( context, fileDataStorageManager, certificateValidator, arbitraryDataProvider, - accountManager + accountManager, + clientRepository, + serverKeyCache ) @Provides diff --git a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt index 18b4c98d4542..9adee7357be8 100644 --- a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt +++ b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEKeyInspector.kt @@ -10,12 +10,12 @@ package com.nextcloud.utils.e2ee import android.content.Context import com.google.gson.reflect.TypeToken import com.nextcloud.client.account.UserAccountManager +import com.nextcloud.repository.ClientRepository import com.owncloud.android.datamodel.ArbitraryDataProvider import com.owncloud.android.datamodel.FileDataStorageManager import com.owncloud.android.datamodel.OCFile import com.owncloud.android.datamodel.e2e.v1.encrypted.EncryptedFolderMetadataFileV1 import com.owncloud.android.lib.common.OwnCloudClient -import com.owncloud.android.lib.common.OwnCloudClientFactory import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.lib.resources.e2ee.GetMetadataRemoteOperation import com.owncloud.android.lib.resources.status.OCCapability @@ -36,7 +36,9 @@ class E2EEKeyInspector @Inject constructor( private val storageManager: FileDataStorageManager, private val certificateValidator: CertificateValidator, private val arbitraryDataProvider: ArbitraryDataProvider, - private val accountManager: UserAccountManager + private val accountManager: UserAccountManager, + private val clientRepository: ClientRepository, + private val serverKeyCache: E2EEServerKeyCache ) { companion object { private const val TAG = "E2EEKeyInspector" @@ -47,7 +49,8 @@ class E2EEKeyInspector @Inject constructor( arbitraryDataProvider.getValue(accountManager.user, EncryptionUtils.PUBLIC_KEY).isEmpty() suspend fun fetchCapabilities(): Boolean = withContext(Dispatchers.IO) { - val result = GetCapabilitiesOperation(storageManager).execute(context).isSuccess + val client = clientRepository.getOwncloudClient() ?: return@withContext false + val result = GetCapabilitiesOperation(storageManager).execute(client).isSuccess if (result) { Log_OC.i(TAG, "capabilities fetched") } else { @@ -59,9 +62,10 @@ class E2EEKeyInspector @Inject constructor( suspend fun compareWithServerKey(storedPublicKey: String): DownloadKeyResult = withContext(Dispatchers.IO) { Log_OC.d(TAG, "comparing local public key with remote public key") - val user = accountManager.user + val client = clientRepository.getNextcloudClient() + ?: return@withContext DownloadKeyResult.UnexpectedError() - val certificateResult = GetCertificateRemoteOperation().executeNextcloudClient(user, context) + val certificateResult = GetCertificateRemoteOperation().execute(client) if (!certificateResult.isSuccess) { return@withContext if (certificateResult.httpCode == HttpStatus.SC_NOT_FOUND) { DownloadKeyResult.NoServerKey @@ -70,7 +74,9 @@ class E2EEKeyInspector @Inject constructor( } } - val serverPublicKeyResult = GetServerPublicKeyRemoteOperation().executeNextcloudClient(user, context) + val serverPublicKeyResult = serverKeyCache.get(client.baseUri.toString(), certificateResult.resultData) { + GetServerPublicKeyRemoteOperation().execute(client) + } if (!serverPublicKeyResult.isSuccess) return@withContext DownloadKeyResult.ServerPublicKeyUnavailable() val isCertificateValid = certificateValidator.validate( @@ -86,7 +92,7 @@ class E2EEKeyInspector @Inject constructor( withContext(Dispatchers.IO) { Log_OC.d(TAG, "checking folder metadata key") - val client = OwnCloudClientFactory.createOwnCloudClient(accountManager.currentAccount, context) + val client = clientRepository.getOwncloudClient() ?: return@withContext false val metadataResult = GetMetadataRemoteOperation(folder.localId).execute(client) if (!metadataResult.isSuccess) { diff --git a/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt new file mode 100644 index 000000000000..35ffcf6532aa --- /dev/null +++ b/app/src/main/java/com/nextcloud/utils/e2ee/E2EEServerKeyCache.kt @@ -0,0 +1,43 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import androidx.annotation.WorkerThread +import com.nextcloud.client.core.Clock +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import java.util.concurrent.TimeUnit +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class E2EEServerKeyCache @Inject constructor(private val clock: Clock) { + private val entries = mutableMapOf, Pair>>() + + @WorkerThread + @Synchronized + fun get( + serverUrl: String, + certificate: String, + fetch: () -> RemoteOperationResult + ): RemoteOperationResult { + val now = clock.millisSinceBoot + entries.entries.removeAll { now - it.value.first !in 0 until LIFETIME_MILLIS } + val key = serverUrl to certificate + entries[key]?.let { return it.second } + + return fetch().also { result -> + if (result.isSuccess && !result.resultData.isNullOrEmpty()) { + entries[key] = clock.millisSinceBoot to result + } + } + } + + companion object { + private val LIFETIME_MILLIS = TimeUnit.MINUTES.toMillis(10) + } +} diff --git a/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt new file mode 100644 index 000000000000..48bdf7e2f663 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEKeyInspectorServerKeyTest.kt @@ -0,0 +1,68 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import android.net.Uri +import com.nextcloud.client.core.Clock +import com.nextcloud.common.NextcloudClient +import com.nextcloud.repository.ClientRepository +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import com.owncloud.android.lib.resources.users.GetCertificateRemoteOperation +import com.owncloud.android.lib.resources.users.GetServerPublicKeyRemoteOperation +import com.owncloud.android.ui.dialog.setupEncryption.CertificateValidator +import com.owncloud.android.ui.dialog.setupEncryption.model.DownloadKeyResult +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.mockito.Mockito.mockConstruction +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class E2EEKeyInspectorServerKeyTest { + @Test + fun repeatedEntryFetchesCertificateAndVerifiesItWhileSharingServerKeyAcrossInspectors() = runTest { + withContext(Dispatchers.IO) { + val uri: Uri = mock { on { toString() } doReturn "https://cloud.example" } + val client: NextcloudClient = mock { on { baseUri } doReturn uri } + val repository: ClientRepository = mock { onBlocking { getNextcloudClient() } doReturn client } + val validator: CertificateValidator = mock() + whenever(validator.validate(SERVER_KEY, CERTIFICATE)).thenReturn(true) + val cache = E2EEServerKeyCache(mock()) + val certificateResult = RemoteOperationResult(ResultCode.OK).apply { resultData = CERTIFICATE } + val serverKeyResult = RemoteOperationResult(ResultCode.OK).apply { resultData = SERVER_KEY } + + mockConstruction(GetCertificateRemoteOperation::class.java) { operation, _ -> + whenever(operation.execute(client)).thenReturn(certificateResult) + }.use { certificates -> + mockConstruction(GetServerPublicKeyRemoteOperation::class.java) { operation, _ -> + whenever(operation.execute(client)).thenReturn(serverKeyResult) + }.use { serverKeys -> + repeat(ENTRIES) { + val inspector = E2EEKeyInspector(mock(), mock(), validator, mock(), mock(), repository, cache) + assertEquals(DownloadKeyResult.CompareKeys(true), inspector.compareWithServerKey(CERTIFICATE)) + } + assertEquals(ENTRIES, certificates.constructed().size) + assertEquals(1, serverKeys.constructed().size) + } + } + verify(validator, times(ENTRIES)).validate(SERVER_KEY, CERTIFICATE) + } + } + + companion object { + private const val SERVER_KEY = "server-key" + private const val CERTIFICATE = "certificate" + private const val ENTRIES = 10 + } +} diff --git a/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt new file mode 100644 index 000000000000..2ab4766a4238 --- /dev/null +++ b/app/src/test/java/com/nextcloud/utils/e2ee/E2EEServerKeyCacheTest.kt @@ -0,0 +1,134 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.utils.e2ee + +import com.nextcloud.client.core.Clock +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode +import org.junit.Assert.assertEquals +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.util.concurrent.TimeUnit +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.atomic.AtomicInteger + +class E2EEServerKeyCacheTest { + private val clock: Clock = mock() + private val cache = E2EEServerKeyCache(clock) + private val success = RemoteOperationResult(ResultCode.OK).apply { resultData = "server-key" } + + @Test + fun repeatedFolderEntryReusesServerKey() { + var requests = 0 + repeat(REPEATED_ENTRIES) { + assertSame( + success, + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + ) + } + assertEquals(1, requests) + } + + @Test + fun expirationFetchesServerKeyAgain() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + whenever(clock.millisSinceBoot).thenReturn(TimeUnit.MINUTES.toMillis(10)) + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + assertEquals(2, requests) + } + + @Test + fun changedCertificateFetchesServerKeyAgain() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + cache.get(SERVER, "renewed-certificate") { + requests++ + success + } + assertEquals(2, requests) + } + + @Test + fun differentServersDoNotShareCachedKey() { + var requests = 0 + cache.get(SERVER, CERTIFICATE) { + requests++ + success + } + cache.get("https://cloud.example:8443", CERTIFICATE) { + requests++ + success + } + cache.get("https://other.example", CERTIFICATE) { + requests++ + success + } + assertEquals(3, requests) + } + + @Test + fun failuresAndEmptyResponsesAreNotCached() { + val failure = RemoteOperationResult(ResultCode.FORBIDDEN) + val empty = RemoteOperationResult(ResultCode.OK).apply { resultData = "" } + assertSame(failure, cache.get(SERVER, CERTIFICATE) { failure }) + assertSame(empty, cache.get(SERVER, CERTIFICATE) { empty }) + assertSame(success, cache.get(SERVER, CERTIFICATE) { success }) + } + + @Test + fun concurrentFolderEntriesShareOneRequest() { + val executor = Executors.newFixedThreadPool(REPEATED_ENTRIES) + val started = CountDownLatch(1) + val response = CountDownLatch(1) + val requests = AtomicInteger() + try { + val results = List(REPEATED_ENTRIES) { + executor.submit> { + cache.get(SERVER, CERTIFICATE) { + requests.incrementAndGet() + started.countDown() + check(response.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + success + } + } + } + assertTrue(started.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + assertEquals(1, requests.get()) + response.countDown() + results.forEach { assertSame(success, it.get(TIMEOUT_SECONDS, TimeUnit.SECONDS)) } + assertEquals(1, requests.get()) + } finally { + response.countDown() + executor.shutdownNow() + } + } + + companion object { + private const val SERVER = "https://cloud.example" + private const val CERTIFICATE = "certificate" + private const val REPEATED_ENTRIES = 10 + private const val TIMEOUT_SECONDS = 5L + } +}