From 6d4fa0654570ac19f4ec634c267e58f067a3ba4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joris=20Pelgr=C3=B6m?= Date: Sat, 26 Aug 2023 22:54:40 +0200 Subject: [PATCH] Thread updates: add border agent ID, multiserver improvements (#3820) * Add/update/delete Thread credentials by border agent ID * 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. * Fix minimal --- .../android/thread/ThreadManagerImpl.kt | 84 ++++++++++++++++--- .../companion/android/thread/ThreadManager.kt | 8 +- .../android/thread/ThreadManagerImpl.kt | 7 +- .../data/integration/IntegrationRepository.kt | 12 +++ .../impl/IntegrationRepositoryImpl.kt | 25 ++++++ .../impl/entities/ThreadDatasetResponse.kt | 1 + 6 files changed, 124 insertions(+), 13 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..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 @@ -28,7 +28,7 @@ class ThreadManagerImpl @Inject constructor( 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" } @@ -54,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() @@ -62,7 +64,8 @@ class ThreadManagerImpl @Inject constructor( return if (deviceThreadIntent == null && coreThreadDataset != null) { try { - importDatasetFromServer(context, coreThreadDataset.datasetId, serverId) + importDatasetFromServer(context, coreThreadDataset.datasetId, coreThreadDataset.preferredBorderAgentId, serverId) + 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) { @@ -83,13 +86,48 @@ 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 = 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 + } + 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) + serverManager.defaultServers.forEach { + serverManager.integrationRepository(it.id).setThreadBorderAgentIds( + if (it.id == serverId) { + listOf(coreThreadDataset.preferredBorderAgentId ?: BORDER_AGENT_ID) + } else { + emptyList() + } + ) + } 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) + } + } + serverManager.defaultServers.forEach { + serverManager.integrationRepository(it.id).setThreadBorderAgentIds(emptyList()) + } false } Log.d(TAG, "Thread update device completed") @@ -120,10 +158,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 +238,22 @@ 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 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) .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/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") 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..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 @@ -68,6 +68,18 @@ interface IntegrationRepository { suspend fun getLastUsedPipelineSttSupport(): Boolean suspend fun setLastUsedPipeline(pipelineId: String, supportsStt: Boolean) + + /** @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 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 99504db3c37..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" @@ -64,6 +65,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 +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") + + // 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 } @@ -565,6 +576,20 @@ 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 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() 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 )