* fix(#478): rework default login to use unified ConnectionProfile model - AuthManager: drop legacy auth_token path; getToken/setToken now delegate to the selected profile (profile token only, no global fallback) - setHost/setPort always write the selected profile (no top-level host/port branch) - Add DEFAULT_PROFILE_ID + ensureDefaultProfile()/ensureDefaultSelected() so the selected profile is never null; ensureDefaultSelected() only falls back when nothing is selected (never clobbers an existing choice); ensureDefaultProfile() only injects a Default profile on fresh install, never into an existing list - One-time migration folds legacy standalone auth_token into the Default profile, batched into a single SharedPreferences transaction (guarded by legacy_default_migrated) - ConnectViewModel.connect() persists via setToken/setHost/setPort (delegate to the selected profile) — keeps the existing persistence contract - Connect/Settings: remove the '(none)'/standalone profile option; deleteProfile falls back to Default profile instead of null selection - Update AuthManagerTest/ConnectViewModelTest to the unified model; fix test stub ordering + migration re-init; remove dead connect_profile_none / settings_profile_none strings Closes #478 * fix: stub AuthManager.ensureDefaultSelected() in E2eIntegrationTest The #478 ConnectViewModel.connect() success branch now calls AuthManager.ensureDefaultSelected() on the non-saveProfile path. E2eIntegrationTest.setUp() mocks AuthManager as a non-relaxed mockkObject but never stubs that method, so the unstubbed call throws MockKException inside viewModelScope.launch, leaving connectionSuccess unset and failing 3 tests: - testFullUserSessionFlow - testConnectViewModel_onPairingString_base64Format - testConnectViewModel_onPairingString_urlFormat Add the missing stub. Verified locally: full suite 322/322 passing. --------- Co-authored-by: ronia <ronia@m57>
This commit is contained in:
M57andronia authored and GitHub committed
1 parent
b65d0297e0
commit
aa59e704a4
11 files changed
+229
-155
No files matched your search
@@ -8,6 +8,9 @@ class HermesControlApp : Application() {
|
||||
override fun onCreate() {
|
||||
super.onCreate()
|
||||
AuthManager.init(this)
|
||||
// Issue #478: guarantee a "Default" profile is always selected so there is no
|
||||
// separate standalone/default code path anywhere in the app.
|
||||
AuthManager.ensureDefaultProfile()
|
||||
NetworkMonitor.init(this)
|
||||
}
|
||||
}
|
||||
@@ -34,8 +34,12 @@ import kotlinx.coroutines.runBlocking
|
||||
object AuthManager {
|
||||
private const val PREFS_FILE = "hermes_secure_prefs"
|
||||
|
||||
private const val KEY_TOKEN = "auth_token"
|
||||
const val DEFAULT_PROFILE_ID = "default"
|
||||
const val DEFAULT_PROFILE_NAME = "Default"
|
||||
private const val KEY_SELECTED_PROFILE_ID = "selected_profile_id"
|
||||
private const val KEY_SESSION_COOKIE = "session_cookie"
|
||||
private const val KEY_LEGACY_TOKEN = "auth_token"
|
||||
private const val KEY_LEGACY_DEFAULT_MIGRATED = "legacy_default_migrated"
|
||||
|
||||
@Volatile
|
||||
private var prefsDeferred: Deferred<SharedPreferences>? = null
|
||||
@@ -114,6 +118,7 @@ object AuthManager {
|
||||
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
|
||||
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
|
||||
)
|
||||
migrateLegacyDefaultIfNeeded(p)
|
||||
_tokenFlow.value = getTokenInternal(p)
|
||||
p
|
||||
}
|
||||
@@ -182,6 +187,75 @@ object AuthManager {
|
||||
serverStore.update { it.copy(connectionProfiles = profiles) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Guarantees the selected profile is never null (issue #478).
|
||||
*
|
||||
* - If a profile is already selected (default or otherwise), nothing is changed.
|
||||
* - If nothing is selected but other profiles exist, the first one is selected.
|
||||
* - If there are no profiles at all (fresh install / legacy standalone), a [DEFAULT_PROFILE_ID]
|
||||
* profile is created from the current top-level host/port and selected.
|
||||
*
|
||||
* This never injects a Default profile into an existing user's profile list, and never
|
||||
* clobbers a user's explicit selection.
|
||||
*/
|
||||
fun ensureDefaultProfile() {
|
||||
val state = serverStore.getLatestState()
|
||||
val hasDefault = state.connectionProfiles.any { it.id == DEFAULT_PROFILE_ID }
|
||||
val needsSelection = state.selectedProfileId.isNullOrBlank()
|
||||
|
||||
if (!needsSelection && (hasDefault || state.connectionProfiles.isNotEmpty())) return
|
||||
|
||||
if (state.connectionProfiles.isEmpty()) {
|
||||
// Fresh install / legacy standalone: create the Default profile and select it.
|
||||
serverStore.update { s ->
|
||||
s.copy(
|
||||
connectionProfiles =
|
||||
listOf(
|
||||
ConnectionProfile(
|
||||
id = DEFAULT_PROFILE_ID,
|
||||
name = DEFAULT_PROFILE_NAME,
|
||||
host = s.host,
|
||||
port = s.port,
|
||||
),
|
||||
),
|
||||
selectedProfileId = DEFAULT_PROFILE_ID,
|
||||
)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// Profiles exist but nothing is selected: pick the first one so selection is non-null.
|
||||
if (needsSelection) {
|
||||
serverStore.update { s -> s.copy(selectedProfileId = s.connectionProfiles.first().id) }
|
||||
}
|
||||
}
|
||||
|
||||
/** Ensure a profile is selected (the default one if nothing else), so callers never see null. */
|
||||
fun ensureDefaultSelected() {
|
||||
ensureDefaultProfile()
|
||||
if (getSelectedProfileId().isNullOrBlank()) {
|
||||
setSelectedProfileId(DEFAULT_PROFILE_ID)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One-time migration: fold the legacy standalone ([KEY_LEGACY_TOKEN]) credentials into the
|
||||
* new default [ConnectionProfile]. Runs once per install, guarded by
|
||||
* [KEY_LEGACY_DEFAULT_MIGRATED].
|
||||
*/
|
||||
private fun migrateLegacyDefaultIfNeeded(p: SharedPreferences) {
|
||||
if (p.getBoolean(KEY_LEGACY_DEFAULT_MIGRATED, false)) return
|
||||
val legacyToken = p.getString(KEY_LEGACY_TOKEN, null)
|
||||
ensureDefaultProfile()
|
||||
p.edit().apply {
|
||||
if (!legacyToken.isNullOrBlank()) {
|
||||
putString("token_$DEFAULT_PROFILE_ID", legacyToken)
|
||||
}
|
||||
remove(KEY_LEGACY_TOKEN)
|
||||
putBoolean(KEY_LEGACY_DEFAULT_MIGRATED, true)
|
||||
}.apply()
|
||||
}
|
||||
|
||||
// ── Pinned Models ────────────────────────────────────────────────────
|
||||
|
||||
fun getPinnedModels(): List<PinnedModel> = serverStore.getLatestState().pinnedModels
|
||||
@@ -253,12 +327,7 @@ object AuthManager {
|
||||
synchronized(this) {
|
||||
if (tokenInitialized) return cachedToken
|
||||
val selectedId = getSelectedProfileId()
|
||||
val token =
|
||||
if (selectedId != null) {
|
||||
getProfileToken(selectedId) ?: requirePrefs().getString(KEY_TOKEN, null)
|
||||
} else {
|
||||
requirePrefs().getString(KEY_TOKEN, null)
|
||||
}
|
||||
val token = if (selectedId != null) getProfileToken(selectedId) else null
|
||||
cachedToken = token
|
||||
tokenInitialized = true
|
||||
return token
|
||||
@@ -268,20 +337,19 @@ object AuthManager {
|
||||
private fun getTokenInternal(p: SharedPreferences): String? {
|
||||
val selectedId = serverStore.getLatestState().selectedProfileId?.takeIf { it.isNotBlank() }
|
||||
return if (selectedId != null) {
|
||||
p.getString("token_$selectedId", null) ?: p.getString(KEY_TOKEN, null)
|
||||
p.getString("token_$selectedId", null)
|
||||
} else {
|
||||
p.getString(KEY_TOKEN, null)
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
fun setToken(token: String?) {
|
||||
val selectedId = getSelectedProfileId()
|
||||
if (selectedId != null) {
|
||||
setProfileToken(selectedId, token)
|
||||
} else {
|
||||
requirePrefs().edit().putString(KEY_TOKEN, token).apply()
|
||||
_tokenFlow.value = token
|
||||
}
|
||||
val selectedId =
|
||||
getSelectedProfileId() ?: run {
|
||||
ensureDefaultSelected()
|
||||
DEFAULT_PROFILE_ID
|
||||
}
|
||||
setProfileToken(selectedId, token)
|
||||
synchronized(this) {
|
||||
cachedToken = token
|
||||
tokenInitialized = true
|
||||
@@ -293,17 +361,17 @@ object AuthManager {
|
||||
fun getHost(): String = serverStore.getLatestState().resolvedHost
|
||||
|
||||
fun setHost(host: String) {
|
||||
val selectedId = getSelectedProfileId()
|
||||
if (selectedId != null) {
|
||||
serverStore.update { state ->
|
||||
val profiles =
|
||||
state.connectionProfiles.map {
|
||||
if (it.id == selectedId) it.copy(host = host) else it
|
||||
}
|
||||
state.copy(connectionProfiles = profiles)
|
||||
val selectedId =
|
||||
getSelectedProfileId() ?: run {
|
||||
ensureDefaultSelected()
|
||||
DEFAULT_PROFILE_ID
|
||||
}
|
||||
} else {
|
||||
serverStore.update { it.copy(host = host) }
|
||||
serverStore.update { state ->
|
||||
val profiles =
|
||||
state.connectionProfiles.map {
|
||||
if (it.id == selectedId) it.copy(host = host) else it
|
||||
}
|
||||
state.copy(connectionProfiles = profiles)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -312,17 +380,17 @@ object AuthManager {
|
||||
fun getPort(): Int = serverStore.getLatestState().resolvedPort
|
||||
|
||||
fun setPort(port: Int) {
|
||||
val selectedId = getSelectedProfileId()
|
||||
if (selectedId != null) {
|
||||
serverStore.update { state ->
|
||||
val profiles =
|
||||
state.connectionProfiles.map {
|
||||
if (it.id == selectedId) it.copy(port = port) else it
|
||||
}
|
||||
state.copy(connectionProfiles = profiles)
|
||||
val selectedId =
|
||||
getSelectedProfileId() ?: run {
|
||||
ensureDefaultSelected()
|
||||
DEFAULT_PROFILE_ID
|
||||
}
|
||||
} else {
|
||||
serverStore.update { it.copy(port = port) }
|
||||
serverStore.update { state ->
|
||||
val profiles =
|
||||
state.connectionProfiles.map {
|
||||
if (it.id == selectedId) it.copy(port = port) else it
|
||||
}
|
||||
state.copy(connectionProfiles = profiles)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -154,13 +154,6 @@ fun ConnectScreen(
|
||||
onDismissRequest = { profilesExpanded = false },
|
||||
modifier = Modifier.fillMaxWidth(0.85f),
|
||||
) {
|
||||
DropdownMenuItem(
|
||||
text = { Text(stringResource(R.string.connect_profile_none)) },
|
||||
onClick = {
|
||||
viewModel.selectProfile(null)
|
||||
profilesExpanded = false
|
||||
},
|
||||
)
|
||||
state.profiles.forEach { profile ->
|
||||
DropdownMenuItem(
|
||||
text = { Text(profile.name) },
|
||||
|
||||
@@ -54,12 +54,7 @@ class ConnectViewModel(
|
||||
val selectedProfile = profiles.firstOrNull { it.id == selectedId }
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
token =
|
||||
if (selectedProfile != null) {
|
||||
AuthManager.getProfileToken(selectedProfile.id) ?: ""
|
||||
} else {
|
||||
savedToken
|
||||
},
|
||||
token = savedToken,
|
||||
host = selectedProfile?.host ?: savedHost,
|
||||
port = (selectedProfile?.port ?: savedPort).toString(),
|
||||
profiles = profiles,
|
||||
@@ -77,30 +72,17 @@ class ConnectViewModel(
|
||||
_uiState.update { it.copy(saveProfile = value) }
|
||||
}
|
||||
|
||||
fun selectProfile(profile: ConnectionProfile?) {
|
||||
if (profile == null) {
|
||||
AuthManager.setSelectedProfileId(null)
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
selectedProfile = null,
|
||||
profileName = "",
|
||||
token = "",
|
||||
host = "127.0.0.1",
|
||||
port = "9119",
|
||||
)
|
||||
}
|
||||
} else {
|
||||
AuthManager.setSelectedProfileId(profile.id)
|
||||
val token = AuthManager.getProfileToken(profile.id) ?: ""
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
selectedProfile = profile,
|
||||
profileName = profile.name,
|
||||
host = profile.host,
|
||||
port = profile.port.toString(),
|
||||
token = token,
|
||||
)
|
||||
}
|
||||
fun selectProfile(profile: ConnectionProfile) {
|
||||
AuthManager.setSelectedProfileId(profile.id)
|
||||
val token = AuthManager.getProfileToken(profile.id) ?: ""
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
selectedProfile = profile,
|
||||
profileName = profile.name,
|
||||
host = profile.host,
|
||||
port = profile.port.toString(),
|
||||
token = token,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -142,7 +124,9 @@ class ConnectViewModel(
|
||||
}
|
||||
when (result) {
|
||||
is NetworkResult.Success -> {
|
||||
// Only persist credentials to global state upon successful verification
|
||||
// Persist credentials to the selected (Default) profile upon successful verification.
|
||||
// setToken/setHost/setPort delegate to the currently-selected profile, which is
|
||||
// always non-null after issue #478 (never the legacy standalone path).
|
||||
AuthManager.setToken(state.token)
|
||||
AuthManager.setHost(state.host)
|
||||
AuthManager.setPort(port)
|
||||
@@ -177,11 +161,10 @@ class ConnectViewModel(
|
||||
AuthManager.setSelectedProfileId(targetProfile.id)
|
||||
AuthManager.setProfileToken(targetProfile.id, state.token)
|
||||
} else {
|
||||
// Clear selected profile if we connected standalone and didn't check "save profile"
|
||||
AuthManager.setSelectedProfileId(null)
|
||||
AuthManager.setToken(state.token)
|
||||
AuthManager.setHost(state.host)
|
||||
AuthManager.setPort(port)
|
||||
// No explicit profile name — store the connection on the default profile.
|
||||
AuthManager.ensureDefaultSelected()
|
||||
AuthManager.setSelectedProfileId(AuthManager.DEFAULT_PROFILE_ID)
|
||||
AuthManager.setProfileToken(AuthManager.DEFAULT_PROFILE_ID, state.token)
|
||||
}
|
||||
ApiClient.rebuild()
|
||||
_uiState.update {
|
||||
|
||||
@@ -359,13 +359,6 @@ private fun ConnectionSection(
|
||||
onDismissRequest = { profilesExpanded = false },
|
||||
modifier = Modifier.fillMaxWidth(0.85f),
|
||||
) {
|
||||
DropdownMenuItem(
|
||||
text = { Text(stringResource(R.string.settings_profile_none)) },
|
||||
onClick = {
|
||||
viewModel.selectProfile(null)
|
||||
profilesExpanded = false
|
||||
},
|
||||
)
|
||||
state.profiles.forEach { profile ->
|
||||
DropdownMenuItem(
|
||||
text = { Text(profile.name) },
|
||||
|
||||
@@ -127,8 +127,13 @@ class SettingsViewModel(
|
||||
val updatedProfiles = AuthManager.getConnectionProfiles().filter { it.id != profileId }
|
||||
AuthManager.saveConnectionProfiles(updatedProfiles)
|
||||
AuthManager.setProfileToken(profileId, null)
|
||||
// Never leave selection null (issue #478): if the deleted profile was selected,
|
||||
// fall back to the default profile instead of clearing selection.
|
||||
if (AuthManager.getSelectedProfileId() == profileId) {
|
||||
AuthManager.setSelectedProfileId(null)
|
||||
if (updatedProfiles.none { it.id == AuthManager.DEFAULT_PROFILE_ID }) {
|
||||
AuthManager.ensureDefaultProfile()
|
||||
}
|
||||
AuthManager.setSelectedProfileId(AuthManager.DEFAULT_PROFILE_ID)
|
||||
}
|
||||
viewModelScope.launch(ioDispatcher) { loadSettings() }
|
||||
ApiClient.rebuild()
|
||||
|
||||
Reference in new issue
Block a user