From 2a0e6ba5a0ad43801ec6b78eded85ade4649ddb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joris=20Pelgr=C3=B6m?= Date: Fri, 25 Aug 2023 21:15:29 +0200 Subject: [PATCH 1/3] Add/update/delete Thread credentials by border agent ID --- .../android/thread/ThreadManagerImpl.kt | 58 +++++++++++++++---- .../companion/android/thread/ThreadManager.kt | 8 ++- .../data/integration/IntegrationRepository.kt | 6 ++ .../impl/IntegrationRepositoryImpl.kt | 9 +++ .../impl/entities/ThreadDatasetResponse.kt | 1 + 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt b/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt index cf8ead01d61..666eb6e4bf1 100644 --- a/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt +++ b/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt @@ -12,6 +12,7 @@ import com.google.android.gms.threadnetwork.ThreadBorderAgent import com.google.android.gms.threadnetwork.ThreadNetwork import com.google.android.gms.threadnetwork.ThreadNetworkCredentials import io.homeassistant.companion.android.common.data.HomeAssistantVersion +import io.homeassistant.companion.android.common.data.integration.IntegrationRepository import io.homeassistant.companion.android.common.data.servers.ServerManager import io.homeassistant.companion.android.common.data.websocket.impl.entities.ThreadDatasetResponse import kotlinx.coroutines.CoroutineScope @@ -23,12 +24,13 @@ import kotlin.coroutines.suspendCoroutine class ThreadManagerImpl @Inject constructor( private val serverManager: ServerManager, - private val packageManager: PackageManager + private val packageManager: PackageManager, + private val integrationRepository: IntegrationRepository ) : ThreadManager { companion object { private const val TAG = "ThreadManagerImpl" - // ID is a placeholder while we wait for Google to remove the requirement to provide one + // ID is a placeholder used in previous app versions / for older Home Assistant versions private const val BORDER_AGENT_ID = "0000000000000001" } @@ -62,8 +64,12 @@ class ThreadManagerImpl @Inject constructor( return if (deviceThreadIntent == null && coreThreadDataset != null) { try { - importDatasetFromServer(context, coreThreadDataset.datasetId, serverId) + importDatasetFromServer(context, coreThreadDataset.datasetId, coreThreadDataset.preferredBorderAgentId, serverId) + val localIds = integrationRepository.getThreadBorderAgentIds() + integrationRepository.setThreadBorderAgentIds(localIds + (coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID)) + Log.d(TAG, "Thread import to device completed") + ThreadManager.SyncResult.OnlyOnServer(imported = true) } catch (e: Exception) { Log.e(TAG, "Thread import to device failed", e) @@ -85,11 +91,31 @@ class ThreadManagerImpl @Inject constructor( if (appIsDevicePreferred) { // Update or remove the device preferred credential to match core state try { + val localIds = integrationRepository.getThreadBorderAgentIds().toMutableList() + if (localIds.isEmpty()) { // Prefers something from HA, must've been added before BA ID logic + localIds += BORDER_AGENT_ID + } + updated = if (coreThreadDataset.source != "Google") { // Credential from HA, update - importDatasetFromServer(context, coreThreadDataset.datasetId, serverId) + localIds.filter { it != coreThreadDataset.preferredBorderAgentId }.forEach { baId -> + try { + deleteThreadCredential(context, baId) + } catch (e: Exception) { + Log.e(TAG, "Unable to delete credential for border agent ID $baId", e) + } + } + importDatasetFromServer(context, coreThreadDataset.datasetId, coreThreadDataset.preferredBorderAgentId, serverId) + integrationRepository.setThreadBorderAgentIds(listOf(coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID)) true - } else { // Imported from another app, so this shouldn't be managed by HA - deleteThreadCredential(context) + } else { // Core prefers imported from other app, this shouldn't be managed by HA + localIds.forEach { baId -> + try { + deleteThreadCredential(context, baId) + } catch (e: Exception) { + Log.e(TAG, "Unable to delete credential for border agent ID $baId", e) + } + } + integrationRepository.setThreadBorderAgentIds(emptyList()) false } Log.d(TAG, "Thread update device completed") @@ -120,10 +146,21 @@ class ThreadManagerImpl @Inject constructor( override suspend fun getPreferredDatasetFromServer(serverId: Int): ThreadDatasetResponse? = getDatasetsFromServer(serverId)?.firstOrNull { it.preferred } - override suspend fun importDatasetFromServer(context: Context, datasetId: String, serverId: Int) { + override suspend fun importDatasetFromServer( + context: Context, + datasetId: String, + preferredBorderAgentId: String?, + serverId: Int + ) { val tlv = serverManager.webSocketRepository(serverId).getThreadDatasetTlv(datasetId)?.tlvAsByteArray if (tlv != null) { - val threadBorderAgent = ThreadBorderAgent.newBuilder(BORDER_AGENT_ID.toByteArray()).build() + val borderAgentId = ( + preferredBorderAgentId ?: run { + Log.w(TAG, "Adding dataset with placeholder border agent ID") + BORDER_AGENT_ID + } + ).toByteArray() + val threadBorderAgent = ThreadBorderAgent.newBuilder(borderAgentId).build() val threadNetworkCredentials = ThreadNetworkCredentials.fromActiveOperationalDataset(tlv) suspendCoroutine { cont -> ThreadNetwork.getClient(context).addCredentials(threadBorderAgent, threadNetworkCredentials) @@ -189,9 +226,8 @@ class ThreadManagerImpl @Inject constructor( return null } - private suspend fun deleteThreadCredential(context: Context) = suspendCoroutine { cont -> - // This only works because we currently always use the same border agent ID - val threadBorderAgent = ThreadBorderAgent.newBuilder(BORDER_AGENT_ID.toByteArray()).build() + private suspend fun deleteThreadCredential(context: Context, borderAgentId: String) = suspendCoroutine { cont -> + val threadBorderAgent = ThreadBorderAgent.newBuilder(borderAgentId.toByteArray()).build() ThreadNetwork.getClient(context) .removeCredentials(threadBorderAgent) .addOnSuccessListener { cont.resume(true) } diff --git a/app/src/main/java/io/homeassistant/companion/android/thread/ThreadManager.kt b/app/src/main/java/io/homeassistant/companion/android/thread/ThreadManager.kt index cabec682943..7976e890c45 100644 --- a/app/src/main/java/io/homeassistant/companion/android/thread/ThreadManager.kt +++ b/app/src/main/java/io/homeassistant/companion/android/thread/ThreadManager.kt @@ -48,10 +48,16 @@ interface ThreadManager { /** * Import a Thread dataset from the server to this device. * @param datasetId The dataset ID as provided by the server + * @param preferredBorderAgentId The ID for the border agent that provides the dataset * @throws Exception if a preferred dataset exists on the server, but it wasn't possible to * import it */ - suspend fun importDatasetFromServer(context: Context, datasetId: String, serverId: Int) + suspend fun importDatasetFromServer( + context: Context, + datasetId: String, + preferredBorderAgentId: String?, + serverId: Int + ) /** * Start a flow to get the preferred Thread dataset from this device to export to the server. diff --git a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt index 43100b1cf13..41a3141cbb6 100644 --- a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt +++ b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt @@ -68,6 +68,12 @@ interface IntegrationRepository { suspend fun getLastUsedPipelineSttSupport(): Boolean suspend fun setLastUsedPipeline(pipelineId: String, supportsStt: Boolean) + + /** @return List of border agent IDs added from the server to this device */ + suspend fun getThreadBorderAgentIds(): List + + /** Set the list of border agent IDs added from the server to this device */ + suspend fun setThreadBorderAgentIds(ids: List) } @AssistedFactory diff --git a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt index 99504db3c37..e08e3386bea 100644 --- a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt +++ b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt @@ -64,6 +64,7 @@ class IntegrationRepositoryImpl @AssistedInject constructor( private const val PREF_SEC_WARNING_NEXT = "sec_warning_last" private const val PREF_LAST_USED_PIPELINE_ID = "last_used_pipeline" private const val PREF_LAST_USED_PIPELINE_STT = "last_used_pipeline_stt" + private const val PREF_THREAD_BORDER_AGENT_IDS = "thread_border_agent_ids" private const val TAG = "IntegrationRepository" private const val RATE_LIMIT_URL = BuildConfig.RATE_LIMIT_URL @@ -169,6 +170,7 @@ class IntegrationRepositoryImpl @AssistedInject constructor( localStorage.remove("${serverId}_$PREF_SEC_WARNING_NEXT") localStorage.remove("${serverId}_$PREF_LAST_USED_PIPELINE_ID") localStorage.remove("${serverId}_$PREF_LAST_USED_PIPELINE_STT") + // TODO Thread // app version and push token is device-specific } @@ -565,6 +567,13 @@ class IntegrationRepositoryImpl @AssistedInject constructor( localStorage.putBoolean("${serverId}_$PREF_LAST_USED_PIPELINE_STT", supportsStt) } + override suspend fun getThreadBorderAgentIds(): List = + localStorage.getStringSet("${serverId}_$PREF_THREAD_BORDER_AGENT_IDS").orEmpty().toList() + + override suspend fun setThreadBorderAgentIds(ids: List) { + localStorage.putStringSet("${serverId}_$PREF_THREAD_BORDER_AGENT_IDS", ids.toSet()) + } + override suspend fun getEntities(): List>? { val response = webSocketRepository.getStates() diff --git a/common/src/main/java/io/homeassistant/companion/android/common/data/websocket/impl/entities/ThreadDatasetResponse.kt b/common/src/main/java/io/homeassistant/companion/android/common/data/websocket/impl/entities/ThreadDatasetResponse.kt index b2d45a42246..8abc5ea02d0 100644 --- a/common/src/main/java/io/homeassistant/companion/android/common/data/websocket/impl/entities/ThreadDatasetResponse.kt +++ b/common/src/main/java/io/homeassistant/companion/android/common/data/websocket/impl/entities/ThreadDatasetResponse.kt @@ -9,5 +9,6 @@ data class ThreadDatasetResponse( val networkName: String, val panId: String, val preferred: Boolean, + val preferredBorderAgentId: String?, // only on core >= 2023.9, may still be null val source: String ) From 41bb42b8b65fe32947c1993c25b42950c0a77058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joris=20Pelgr=C3=B6m?= Date: Fri, 25 Aug 2023 22:07:44 +0200 Subject: [PATCH 2/3] Multiserver: only keep one HA credential, handle credentials for deleted servers - Make sure the app only stores one HA credential that matches the preferred credential for the server executing the sync. - Create an 'orphaned Thread credentials' system to be able to delete credentials for servers that have been previously deleted. --- .../android/thread/ThreadManagerImpl.kt | 48 ++++++++++++++----- .../data/integration/IntegrationRepository.kt | 10 +++- .../impl/IntegrationRepositoryImpl.kt | 18 ++++++- 3 files changed, 62 insertions(+), 14 deletions(-) diff --git a/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt b/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt index 666eb6e4bf1..81e57c947da 100644 --- a/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt +++ b/app/src/full/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt @@ -12,7 +12,6 @@ import com.google.android.gms.threadnetwork.ThreadBorderAgent import com.google.android.gms.threadnetwork.ThreadNetwork import com.google.android.gms.threadnetwork.ThreadNetworkCredentials import io.homeassistant.companion.android.common.data.HomeAssistantVersion -import io.homeassistant.companion.android.common.data.integration.IntegrationRepository import io.homeassistant.companion.android.common.data.servers.ServerManager import io.homeassistant.companion.android.common.data.websocket.impl.entities.ThreadDatasetResponse import kotlinx.coroutines.CoroutineScope @@ -24,8 +23,7 @@ import kotlin.coroutines.suspendCoroutine class ThreadManagerImpl @Inject constructor( private val serverManager: ServerManager, - private val packageManager: PackageManager, - private val integrationRepository: IntegrationRepository + private val packageManager: PackageManager ) : ThreadManager { companion object { private const val TAG = "ThreadManagerImpl" @@ -56,6 +54,8 @@ class ThreadManagerImpl @Inject constructor( if (!appSupportsThread()) return ThreadManager.SyncResult.AppUnsupported if (!coreSupportsThread(serverId)) return ThreadManager.SyncResult.ServerUnsupported + deleteOrphanedThreadCredentials(context, serverId) + val getDeviceDataset = scope.async { getPreferredDatasetFromDevice(context) } val getCoreDatasets = scope.async { getDatasetsFromServer(serverId) } val deviceThreadIntent = getDeviceDataset.await() @@ -65,11 +65,8 @@ class ThreadManagerImpl @Inject constructor( return if (deviceThreadIntent == null && coreThreadDataset != null) { try { importDatasetFromServer(context, coreThreadDataset.datasetId, coreThreadDataset.preferredBorderAgentId, serverId) - val localIds = integrationRepository.getThreadBorderAgentIds() - integrationRepository.setThreadBorderAgentIds(localIds + (coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID)) - + serverManager.integrationRepository(serverId).setThreadBorderAgentIds(listOf((coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID))) Log.d(TAG, "Thread import to device completed") - ThreadManager.SyncResult.OnlyOnServer(imported = true) } catch (e: Exception) { Log.e(TAG, "Thread import to device failed", e) @@ -89,9 +86,14 @@ class ThreadManagerImpl @Inject constructor( var updated: Boolean? = null if (!coreIsDevicePreferred) { if (appIsDevicePreferred) { - // Update or remove the device preferred credential to match core state + // Update or remove the device preferred credential to match core state. + // The device credential store currently doesn't allow the user to choose + // which credential should be used. To prevent unexpected behavior, HA only + // contributes one credential at a time, which is for _this_ server. try { - val localIds = integrationRepository.getThreadBorderAgentIds().toMutableList() + val localIds = serverManager.defaultServers.flatMap { + serverManager.integrationRepository(it.id).getThreadBorderAgentIds() + }.toMutableList() if (localIds.isEmpty()) { // Prefers something from HA, must've been added before BA ID logic localIds += BORDER_AGENT_ID } @@ -105,7 +107,15 @@ class ThreadManagerImpl @Inject constructor( } } importDatasetFromServer(context, coreThreadDataset.datasetId, coreThreadDataset.preferredBorderAgentId, serverId) - integrationRepository.setThreadBorderAgentIds(listOf(coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID)) + serverManager.defaultServers.forEach { + serverManager.integrationRepository(it.id).setThreadBorderAgentIds( + if (it.id == serverId) { + listOf(coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID) + } else { + emptyList() + } + ) + } true } else { // Core prefers imported from other app, this shouldn't be managed by HA localIds.forEach { baId -> @@ -115,7 +125,9 @@ class ThreadManagerImpl @Inject constructor( Log.e(TAG, "Unable to delete credential for border agent ID $baId", e) } } - integrationRepository.setThreadBorderAgentIds(emptyList()) + serverManager.defaultServers.forEach { + serverManager.integrationRepository(it.id).setThreadBorderAgentIds(emptyList()) + } false } Log.d(TAG, "Thread update device completed") @@ -226,6 +238,20 @@ class ThreadManagerImpl @Inject constructor( return null } + private suspend fun deleteOrphanedThreadCredentials(context: Context, serverId: Int) { + val orphanedCredentials = serverManager.integrationRepository(serverId).getThreadBorderAgentIds() + if (orphanedCredentials.isEmpty()) return + + orphanedCredentials.forEach { + try { + deleteThreadCredential(context, it) + } catch (e: Exception) { + Log.w(TAG, "Unable to delete credential for border agent ID $it", e) + } + } + serverManager.integrationRepository(serverId).clearOrphanedThreadBorderAgentIds() + } + private suspend fun deleteThreadCredential(context: Context, borderAgentId: String) = suspendCoroutine { cont -> val threadBorderAgent = ThreadBorderAgent.newBuilder(borderAgentId.toByteArray()).build() ThreadNetwork.getClient(context) diff --git a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt index 41a3141cbb6..b15e34a52e6 100644 --- a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt +++ b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt @@ -69,11 +69,17 @@ interface IntegrationRepository { suspend fun setLastUsedPipeline(pipelineId: String, supportsStt: Boolean) - /** @return List of border agent IDs added from the server to this device */ + /** @return List of border agent IDs added to this device from the server */ suspend fun getThreadBorderAgentIds(): List - /** Set the list of border agent IDs added from the server to this device */ + /** Set the list of border agent IDs added to this device from the server */ suspend fun setThreadBorderAgentIds(ids: List) + + /** @return List of border agent IDs added to this device from a server that no longer exists */ + suspend fun getOrphanedThreadBorderAgentIds(): List + + /** Clear the list of orphaned border agent IDs, to use after removing them from storage */ + suspend fun clearOrphanedThreadBorderAgentIds() } @AssistedFactory diff --git a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt index e08e3386bea..fc622d9d15a 100644 --- a/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt +++ b/common/src/main/java/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt @@ -56,6 +56,7 @@ class IntegrationRepositoryImpl @AssistedInject constructor( private const val PREF_APP_VERSION = "app_version" // Note: _not_ server-specific private const val PREF_PUSH_TOKEN = "push_token" // Note: _not_ server-specific + private const val PREF_ORPHANED_THREAD_BORDER_AGENT_IDS = "orphaned_thread_border_agent_ids" // Note: _not_ server-specific private const val PREF_CHECK_SENSOR_REGISTRATION_NEXT = "sensor_reg_last" private const val PREF_SESSION_TIMEOUT = "session_timeout" @@ -170,7 +171,15 @@ class IntegrationRepositoryImpl @AssistedInject constructor( localStorage.remove("${serverId}_$PREF_SEC_WARNING_NEXT") localStorage.remove("${serverId}_$PREF_LAST_USED_PIPELINE_ID") localStorage.remove("${serverId}_$PREF_LAST_USED_PIPELINE_STT") - // TODO Thread + + // Thread credentials are managed in the app module and can't be deleted now, so store them + val threadBorderAgentIds = getThreadBorderAgentIds() + if (threadBorderAgentIds.any()) { + val orphanedBorderAgentIds = localStorage.getStringSet(PREF_ORPHANED_THREAD_BORDER_AGENT_IDS).orEmpty() + localStorage.putStringSet(PREF_ORPHANED_THREAD_BORDER_AGENT_IDS, orphanedBorderAgentIds + threadBorderAgentIds.toSet()) + } + localStorage.remove("${serverId}_$PREF_THREAD_BORDER_AGENT_IDS") + // app version and push token is device-specific } @@ -574,6 +583,13 @@ class IntegrationRepositoryImpl @AssistedInject constructor( localStorage.putStringSet("${serverId}_$PREF_THREAD_BORDER_AGENT_IDS", ids.toSet()) } + override suspend fun getOrphanedThreadBorderAgentIds(): List = + localStorage.getStringSet(PREF_ORPHANED_THREAD_BORDER_AGENT_IDS).orEmpty().toList() + + override suspend fun clearOrphanedThreadBorderAgentIds() { + localStorage.remove(PREF_ORPHANED_THREAD_BORDER_AGENT_IDS) + } + override suspend fun getEntities(): List>? { val response = webSocketRepository.getStates() From 1ba556e4554bb922da48219b289471ea0bfaab1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joris=20Pelgr=C3=B6m?= Date: Fri, 25 Aug 2023 22:45:42 +0200 Subject: [PATCH 3/3] Fix minimal --- .../companion/android/thread/ThreadManagerImpl.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/minimal/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt b/app/src/minimal/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt index 4b417b17f0d..875ea499bfe 100644 --- a/app/src/minimal/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt +++ b/app/src/minimal/java/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt @@ -24,7 +24,12 @@ class ThreadManagerImpl @Inject constructor() : ThreadManager { override suspend fun getPreferredDatasetFromServer(serverId: Int): ThreadDatasetResponse? = null - override suspend fun importDatasetFromServer(context: Context, datasetId: String, serverId: Int) { } + override suspend fun importDatasetFromServer( + context: Context, + datasetId: String, + preferredBorderAgentId: String?, + serverId: Int + ) { } override suspend fun getPreferredDatasetFromDevice(context: Context): IntentSender? { throw IllegalStateException("Thread is not supported with the minimal flavor")