diff --git a/app/src/main/java/com/m57/hermescontrol/data/config/ConnectionProfile.kt b/app/src/main/java/com/m57/hermescontrol/data/config/ConnectionProfile.kt index 331b5208..c312463a 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/config/ConnectionProfile.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/config/ConnectionProfile.kt @@ -14,16 +14,44 @@ data class ConnectionProfile( val port: Int = 9119, val baseUrl: String? = null, ) { + /** + * Resolves this profile's own base URL, falling back to the legacy loopback + * default when [baseUrl] was never stamped. Prefer [resolveBaseUrl] at the + * store/UI level so a real gateway URL used to log in takes precedence over + * the hardcoded loopback (issue #647). + */ val resolvedBaseUrl: String - get() = - baseUrl - ?.let { - ServerEndpoint - .parse( - it, - CleartextPolicy.ALLOW_WITH_WARNING, - ).baseUrl - .toString() - } - ?: ServerEndpoint.fromLegacy(host, port).baseUrl.toString() + get() = resolveBaseUrl(null) +} + +/** + * Resolves the base URL actually used to talk to this profile's server. + * + * Prefers the profile's own [ConnectionProfile.baseUrl]; if that was never + * stamped (legacy or a freshly-seeded Default profile) it falls back to + * [topLevelBaseUrl] — the URL the user last authenticated against — and only + * then to the legacy loopback default. This prevents the connection-profile + * list from showing the hardcoded `127.0.0.1:9119` when a real gateway URL was + * used to log in (issue #647). + */ +fun ConnectionProfile.resolveBaseUrl(topLevelBaseUrl: String?): String { + baseUrl + ?.let { + return ServerEndpoint + .parse( + it, + CleartextPolicy.ALLOW_WITH_WARNING, + ).baseUrl + .toString() + } + return topLevelBaseUrl + ?.let { + ServerEndpoint + .parse( + it, + CleartextPolicy.ALLOW_WITH_WARNING, + ).baseUrl + .toString() + } + ?: ServerEndpoint.fromLegacy(host, port).baseUrl.toString() } diff --git a/app/src/main/java/com/m57/hermescontrol/data/config/ServerCollectionOps.kt b/app/src/main/java/com/m57/hermescontrol/data/config/ServerCollectionOps.kt index 4761c2e6..856cd124 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/config/ServerCollectionOps.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/config/ServerCollectionOps.kt @@ -37,7 +37,13 @@ fun ServerStoreState.selfHealed(): ServerStoreState { val ServerStoreState.resolvedBaseUrl: String get() { val selected = connectionProfiles.firstOrNull { it.id == selectedProfileId } - if (selected != null) return selected.resolvedBaseUrl + if (selected != null) { + // Prefer the selected profile's own URL, but fall back to the + // top-level login URL (and only then to legacy loopback) so a profile + // whose baseUrl was never stamped still shows the gateway used to + // authenticate, not the hardcoded 127.0.0.1:9119 (issue #647). + return selected.resolveBaseUrl(baseUrl) + } return baseUrl ?.let { ServerEndpoint diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt b/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt index 55679a2e..3b2dbd1f 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/AuthManager.kt @@ -441,7 +441,16 @@ object AuthManager { profile } } - state.copy(connectionProfiles = profiles) + // Also cache the login URL at the top level so that getBaseUrl() and + // the store-level resolvedBaseUrl (used by the profile list display) + // reflect the URL actually authenticated against, never the hardcoded + // loopback default. Fixes issue #647: a Default profile whose own + // baseUrl was never stamped must still resolve to this URL, not + // 127.0.0.1:9119. + state.copy( + baseUrl = normalized, + connectionProfiles = profiles, + ) } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/connect/ConnectViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/connect/ConnectViewModel.kt index 6e618651..c1da25f5 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/connect/ConnectViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/connect/ConnectViewModel.kt @@ -6,6 +6,7 @@ import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope import com.m57.hermescontrol.R import com.m57.hermescontrol.data.config.ConnectionProfile +import com.m57.hermescontrol.data.config.resolveBaseUrl import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.CleartextPolicy @@ -53,7 +54,7 @@ class ConnectViewModel( _uiState.update { it.copy( token = savedToken, - baseUrl = selectedProfile?.resolvedBaseUrl ?: savedBaseUrl, + baseUrl = selectedProfile?.resolveBaseUrl(savedBaseUrl) ?: savedBaseUrl, profiles = profiles, selectedProfile = selectedProfile, profileName = selectedProfile?.name ?: "", @@ -76,7 +77,7 @@ class ConnectViewModel( it.copy( selectedProfile = profile, profileName = profile.name, - baseUrl = profile.resolvedBaseUrl, + baseUrl = profile.resolveBaseUrl(AuthManager.getBaseUrl()), token = token, ) } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt index 022ee105..700f60a5 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.m57.hermescontrol.ScreenRegistry import com.m57.hermescontrol.data.config.ConnectionProfile +import com.m57.hermescontrol.data.config.resolveBaseUrl import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.CleartextPolicy @@ -170,7 +171,7 @@ class SettingsViewModel( showProfileDialog = true, editingProfileId = profileId, dialogProfileName = profile.name, - dialogProfileBaseUrl = profile.resolvedBaseUrl, + dialogProfileBaseUrl = profile.resolveBaseUrl(AuthManager.getBaseUrl()), dialogProfileToken = token, dialogProfileError = null, ) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt index a99f5b30..2df75182 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/components/ConnectionSection.kt @@ -35,6 +35,7 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import androidx.lifecycle.viewmodel.compose.viewModel import com.m57.hermescontrol.R +import com.m57.hermescontrol.data.config.resolveBaseUrl import com.m57.hermescontrol.ui.settings.SectionCard import com.m57.hermescontrol.ui.settings.SettingsUiState import com.m57.hermescontrol.ui.settings.SettingsViewModel @@ -85,7 +86,7 @@ internal fun ConnectionSection( style = MaterialTheme.typography.bodyMedium.copy(fontWeight = FontWeight.Medium), ) Text( - text = profile.resolvedBaseUrl, + text = profile.resolveBaseUrl(state.baseUrl), style = MaterialTheme.typography.bodySmall.copy( color = MaterialTheme.colorScheme.onSurfaceVariant, diff --git a/app/src/test/java/com/m57/hermescontrol/data/local/Issue647ProfileUrlTest.kt b/app/src/test/java/com/m57/hermescontrol/data/local/Issue647ProfileUrlTest.kt new file mode 100644 index 00000000..751c1d6b --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/local/Issue647ProfileUrlTest.kt @@ -0,0 +1,238 @@ +package com.m57.hermescontrol.data.local + +import android.content.Context +import android.content.SharedPreferences +import androidx.security.crypto.EncryptedSharedPreferences +import androidx.security.crypto.MasterKeys +import com.m57.hermescontrol.data.config.ConnectionProfile +import com.m57.hermescontrol.data.config.ServerStoreState +import com.m57.hermescontrol.data.config.resolveBaseUrl +import com.m57.hermescontrol.data.config.resolvedBaseUrl +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Before +import org.junit.Test + +class ReproContext( + private val tempDir: java.io.File, + baseContext: Context, +) : android.content.ContextWrapper(baseContext) { + override fun getFilesDir(): java.io.File = tempDir + + override fun getApplicationContext(): Context = this +} + +/** + * Regression tests for issue #647: the Default connection profile must show the + * URL actually used to log in, never the hardcoded `127.0.0.1:9119` loopback. + * + * Two layers are covered: + * 1. The login path stamps the selected profile's `baseUrl` with the login URL + * (so the profile carries the real URL on its own). + * 2. Defensive layer: even when a profile's own `baseUrl` is null, resolution + * prefers the top-level login URL over the legacy loopback fallback. + */ +class Issue647ProfileUrlTest { + private lateinit var mockPrefs: SharedPreferences + private lateinit var mockEditor: SharedPreferences.Editor + private lateinit var mockContext: Context + private lateinit var testContext: Context + + @Before + fun setUp() { + mockPrefs = mockk(relaxed = true) + mockEditor = mockk(relaxed = true) + mockContext = mockk(relaxed = true) + + every { mockPrefs.edit() } returns mockEditor + every { mockEditor.putString(any(), any()) } returns mockEditor + every { mockEditor.putInt(any(), any()) } returns mockEditor + every { mockEditor.putBoolean(any(), any()) } returns mockEditor + every { mockPrefs.getBoolean("migrated_to_datastore", any()) } returns true + + mockkStatic(android.util.Log::class) + every { android.util.Log.d(any(), any()) } returns 0 + every { android.util.Log.e(any(), any()) } returns 0 + every { android.util.Log.e(any(), any(), any()) } returns 0 + every { android.util.Log.w(any(), any()) } returns 0 + every { android.util.Log.i(any(), any()) } returns 0 + + val tempDir = java.io.File(System.getProperty("java.io.tmpdir") ?: "/tmp") + testContext = ReproContext(tempDir, mockContext) + + val tempFile = java.io.File(tempDir, "server_store.json") + if (tempFile.exists()) tempFile.delete() + + mockkStatic(EncryptedSharedPreferences::class) + every { + EncryptedSharedPreferences.create( + any(), + any(), + any(), + any(), + any(), + ) + } returns mockPrefs + + mockkStatic(MasterKeys::class) + every { MasterKeys.getOrCreate(any()) } returns "mockMasterKey" + + val field = AuthManager::class.java.getDeclaredField("prefsDeferred") + field.isAccessible = true + field.set(AuthManager, null) + + val storeField = AuthManager::class.java.getDeclaredField("_serverStore") + storeField.isAccessible = true + storeField.set(AuthManager, null) + + AuthManager.resetAuthStateForTest() + AuthManager.init(testContext) + + kotlinx.coroutines.runBlocking { + val deferred = field.get(AuthManager) as? kotlinx.coroutines.Deferred<*> + deferred?.await() + } + AuthManager.serverStore.getLatestState() + } + + @After + fun tearDown() { + val tempDir = java.io.File(System.getProperty("java.io.tmpdir") ?: "/tmp") + val tempFile = java.io.File(tempDir, "server_store.json") + if (tempFile.exists()) tempFile.delete() + unmockkAll() + } + + @Test + fun defaultProfile_afterLogin_reflectsLoginUrl() { + // Mirror ConnectViewModel.connect() success path (no saveProfile branch). + val customUrl = "http://192.1.1.190:9119/" + AuthManager.ensureDefaultProfile() + AuthManager.setSelectedProfileId(AuthManager.DEFAULT_PROFILE_ID) + AuthManager.setBaseUrl(customUrl) + AuthManager.setProfileToken(AuthManager.DEFAULT_PROFILE_ID, "tok") + + val default = + AuthManager.getConnectionProfiles().first { + it.id == AuthManager.DEFAULT_PROFILE_ID + } + assertEquals(customUrl, default.resolveBaseUrl(null)) + // Top-level + store-level (the profile-list display) must match too. + assertEquals(customUrl, AuthManager.getBaseUrl()) + assertEquals(customUrl, AuthManager.serverStore.getLatestState().resolvedBaseUrl) + } + + @Test + fun authLogin_connect_sequence_resolvesToLoginUrl_notLoopback() { + // Mirror the exact AuthLoginViewModel.connect() sequence the user hits: + // the Default profile is seeded (with the loopback default) and the user + // logs in via a custom LAN URL. After connect the Default profile, the + // top-level base URL, and the store-level display must all reflect the + // login URL — never the hardcoded 127.0.0.1:9119 (issue #647). + val loginUrl = "http://192.168.1.57:9119/" + val loopback = "https://127.0.0.1:9119/" + + // Seed the Default profile the way the app does (host/port -> loopback). + AuthManager.ensureDefaultProfile() + // Confirm the initial seed really is the loopback default. + val seeded = + AuthManager.getConnectionProfiles().first { + it.id == AuthManager.DEFAULT_PROFILE_ID + } + assertEquals(loopback, seeded.resolveBaseUrl(null)) + + // Replicate AuthLoginViewModel.connect(): select -> setBaseUrl(login) -> token. + AuthManager.setSelectedProfileId(AuthManager.DEFAULT_PROFILE_ID) + AuthManager.setBaseUrl(loginUrl) + AuthManager.setToken("ws-cred") + + val default = + AuthManager.getConnectionProfiles().first { + it.id == AuthManager.DEFAULT_PROFILE_ID + } + assertEquals(loginUrl, default.resolveBaseUrl(null)) + assertEquals(loginUrl, AuthManager.getBaseUrl()) + assertEquals(loginUrl, AuthManager.serverStore.getLatestState().resolvedBaseUrl) + assertNotEquals(loopback, AuthManager.getBaseUrl()) + } + + @Test + fun defaultProfile_withNullBaseUrl_fallsBackToLoginUrlNotLoopback() { + // A Default profile whose own baseUrl was never stamped must still show + // the top-level login URL, never the hardcoded loopback (issue #647). + val customUrl = "http://192.1.1.190:9119/" + val defaultNoUrl = + ConnectionProfile( + id = AuthManager.DEFAULT_PROFILE_ID, + name = AuthManager.DEFAULT_PROFILE_NAME, + baseUrl = null, + ) + AuthManager.saveConnectionProfiles(listOf(defaultNoUrl)) + AuthManager.setSelectedProfileId(AuthManager.DEFAULT_PROFILE_ID) + // Top-level login URL is set, but the profile's own baseUrl remains null. + AuthManager.setBaseUrl(customUrl) + + assertEquals(customUrl, defaultNoUrl.resolveBaseUrl(customUrl)) + assertEquals(customUrl, AuthManager.getBaseUrl()) + } + + @Test + fun profileWithBaseUrl_ignoresTopLevelLoginUrl() { + // When the profile has its own URL, that wins — no leakage from the + // top-level field. + val own = "http://10.0.0.5:9119/" + val top = "http://192.1.1.190:9119/" + val profile = + ConnectionProfile( + id = "prof-x", + name = "X", + baseUrl = own, + ) + assertEquals(own, profile.resolveBaseUrl(top)) + } + + @Test + fun freshInstall_withoutSavedUrl_defaultsToLoopback() { + // No login URL anywhere: a null-baseUrl profile falls back to the + // legacy loopback (http://127.0.0.1:9119/), preserving the fresh-install + // default (issue #647 acceptance criteria). In practice the Default + // profile is seeded from the top-level https default, but the pure + // fallback must remain loopback. + val fresh = + ConnectionProfile( + id = AuthManager.DEFAULT_PROFILE_ID, + name = AuthManager.DEFAULT_PROFILE_NAME, + baseUrl = null, + ) + assertEquals( + "http://127.0.0.1:9119/", + fresh.resolveBaseUrl(null), + ) + } + + @Test + fun serverStoreState_selectedProfileNullBaseUrl_usesTopLevel() { + // Store-level resolution: selected profile with null baseUrl uses the + // top-level login URL, not loopback. + val customUrl = "http://192.1.1.190:9119/" + val state = + ServerStoreState( + baseUrl = customUrl, + connectionProfiles = + listOf( + ConnectionProfile( + id = AuthManager.DEFAULT_PROFILE_ID, + name = AuthManager.DEFAULT_PROFILE_NAME, + baseUrl = null, + ), + ), + selectedProfileId = AuthManager.DEFAULT_PROFILE_ID, + ) + assertEquals(customUrl, state.resolvedBaseUrl) + } +}