fix: persist instance removal before deleting keychain on logout (#48)

logout deleted keychain credentials first, then removed the SwiftData instance
with a swallowed `try? modelContext.save()`. If the save failed, the instance
survived with its credentials gone, leaving an account that could not
authenticate. A `nil` default modelContext also let the instance removal be
skipped entirely while credentials were still wiped.

Reorder to delete + save the SwiftData instance first (matching
InstanceListView.deleteInstance) and only delete credentials once that succeeds;
bail out on save failure so the account stays usable. Make modelContext
required to remove the latent orphan path (all call sites already pass one).
This commit is contained in:
Stefan Hausotte 2026-06-04 13:47:25 +02:00
parent 66fe573cb6
commit a4da4de009

View file

@ -95,9 +95,21 @@ class AuthenticationService {
return true
}
func logout(modelContext: ModelContext? = nil) async {
func logout(modelContext: ModelContext) async {
if let instance = currentInstance {
let normalizedURL = ForgejoClient.normalizeServerURL(instance.serverURL)
// Remove the instance from SwiftData first. If the save fails, keep the
// credentials so the account stays usable instead of becoming an
// unauthenticatable orphan, and bail out without clearing the session.
instance.isDefault = false
modelContext.delete(instance)
do {
try modelContext.save()
} catch {
assertionFailure("SwiftData save failed during logout: \(error)")
return
}
// Delete credentials only after the instance is gone from SwiftData.
do {
try await KeychainManager.shared.deleteCredentials(
for: normalizedURL,
@ -109,12 +121,6 @@ class AuthenticationService {
print("Keychain delete failed during logout: \(error)")
#endif
}
// Remove the instance from SwiftData and unset default
if let modelContext {
instance.isDefault = false
modelContext.delete(instance)
try? modelContext.save()
}
}
// Clears cache for all instances since keys are hashed and cannot be scoped per-instance
DiskCache.removeAll()