From 6a2b3250ce15635a8e70256c93d1af8bf7354277 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Fri, 28 Aug 2026 22:54:37 +0200 Subject: [PATCH] Remember credential and instance deletions across launches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The credential digests are persisted. A login replaced on another device while this one was not running left the Apple account, the context and the status all unchanged, so nothing noticed on the next launch that the cached feed belonged to whoever was signed in before. Deleting an instance is remembered too. Its accounts may still be in transit — CloudKit and the instance list travel separately — and one arriving afterwards was installed as a permanent orphan, credential and account record intact, with no screen yet that could remove it. Late arrivals are now deleted like the rest. The source editor and the instance browser recompute their signed-in state when a status changes, not only when the account list does: a credential arriving over iCloud Keychain moves nothing else, and the authenticated imports and the Feed tab stayed hidden until the view was rebuilt. --- Yattee/Core/AccountsManager.swift | 39 +++++++++++++++++++ Yattee/Core/AppEnvironment.swift | 37 ++++++++++++++++++ .../Views/Instances/InstanceBrowseView.swift | 19 +++++++-- Yattee/Views/Settings/EditSourceView.swift | 5 +++ YatteeTests/AccountsManagerTests.swift | 18 +++++++++ 5 files changed, 114 insertions(+), 4 deletions(-) diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 5484b8b0..b9481b12 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -75,6 +75,14 @@ final class AccountsManager { /// Whether the selected context still has to reach the server. private(set) var pendingContextUpload = false + /// Instances the user has deleted here. + /// + /// Deleting an instance deletes the accounts on it, but those accounts may + /// not have arrived yet — CloudKit and the instance list travel + /// separately. Remembered so a late arrival is deleted rather than + /// installed as an orphan whose credential nothing can reach. + private(set) var deletedInstanceIDs: Set = [] + /// Accounts known to be deleted. Permanent, and the only thing that /// authorises removing an account that arrives from sync: a record's /// *absence* proves nothing, because a fetch resumes from a change token @@ -343,6 +351,14 @@ final class AccountsManager { save() } + /// Records that an instance was deleted here, so accounts on it that have + /// yet to arrive are deleted on sight rather than orphaned. + func recordInstanceDeletion(_ instanceID: UUID) { + guard !deletedInstanceIDs.contains(instanceID) else { return } + deletedInstanceIDs.insert(instanceID) + save() + } + func isDeleted(_ accountID: UUID) -> Bool { deletedAccountIDs.contains(accountID) } @@ -394,6 +410,23 @@ final class AccountsManager { // server copy; here it simply never becomes local state again. guard !deletedAccountIDs.contains(incoming.id) else { return false } + // Its instance was deleted here while this account was still in + // transit. Deleting an instance deletes its accounts, so this one is + // deleted too — arriving late does not exempt it, and installing it + // would leave an orphan nothing can reach the credential of. + guard !deletedInstanceIDs.contains(incoming.instanceID) else { + pendingDeletions.insert(incoming.id) + deletedAccountIDs.insert(incoming.id) + save() + + LoggingService.shared.info( + "An account arrived for an instance that was deleted here — deleting it too", + category: .general, + details: "accountID=\(incoming.id)" + ) + return false + } + if let index = accounts.firstIndex(where: { $0.id == incoming.id }) { let existing = accounts[index] guard existing.isOutranked(by: incoming) else { @@ -799,6 +832,10 @@ final class AccountsManager { deletedAccountIDs = Set(ids.compactMap(UUID.init(uuidString:))) } + if let ids = defaults.array(forKey: key("deletedInstances")) as? [String] { + deletedInstanceIDs = Set(ids.compactMap(UUID.init(uuidString:))) + } + pendingContextUpload = defaults.bool(forKey: key("pendingContextUpload")) } @@ -814,6 +851,7 @@ final class AccountsManager { defaults.set(pendingDeletions.map(\.uuidString), forKey: key("pendingDeletions")) defaults.set(pendingMarkerUploads.map(\.uuidString), forKey: key("pendingMarkerUploads")) defaults.set(deletedAccountIDs.map(\.uuidString), forKey: key("deletedAccounts")) + defaults.set(deletedInstanceIDs.map(\.uuidString), forKey: key("deletedInstances")) defaults.set(pendingContextUpload, forKey: key("pendingContextUpload")) } @@ -836,6 +874,7 @@ final class AccountsManager { pendingDeletions = [] pendingMarkerUploads = [] deletedAccountIDs = [] + deletedInstanceIDs = [] pendingContextUpload = false load() diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index cc6ec437..d5f0675c 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -75,8 +75,32 @@ final class AppEnvironment { /// A digest of each account's stored credential, so a replacement arriving /// over iCloud Keychain can be told from the same secret being re-read. + /// + /// Persisted, because the replacement can arrive while the app is not + /// running: the Apple account and the context are unchanged on the next + /// launch, so nothing else would notice that the cached feed belongs to + /// whoever was signed in before. @ObservationIgnored private var credentialFingerprints: [UUID: String] = [:] + private var credentialFingerprintsKey: String { + "accountCredentialFingerprints.\(credentialIdentityScope.current ?? "unresolved")" + } + + private func loadCredentialFingerprints() { + credentialFingerprints = (UserDefaults.standard + .dictionary(forKey: credentialFingerprintsKey) as? [String: String] ?? [:]) + .reduce(into: [:]) { result, entry in + if let id = UUID(uuidString: entry.key) { result[id] = entry.value } + } + } + + private func saveCredentialFingerprints() { + let stored = credentialFingerprints.reduce(into: [String: String]()) { result, entry in + result[entry.key.uuidString] = entry.value + } + UserDefaults.standard.set(stored, forKey: credentialFingerprintsKey) + } + /// Re-reads the Keychain for a while after an account turns up without one. @ObservationIgnored private var credentialArrivalTask: Task? @@ -438,6 +462,9 @@ final class AppEnvironment { // silently fragile. Task { @MainActor [weak self] in guard let self else { return } + // Fingerprints are recorded per identity, like everything else + // keyed to one. + self.loadCredentialFingerprints() self.refreshAccountStatuses() // Compared against the recorded owner rather than cleared @@ -449,6 +476,7 @@ final class AppEnvironment { } } + loadCredentialFingerprints() discardCachesFromAnotherIdentityIfNeeded(atLaunch: true) // Turn any pre-revamp single-instance logins into accounts. Repeatable: @@ -737,6 +765,11 @@ final class AppEnvironment { /// iCloud leaves its accounts orphaned rather than deleted, because absence /// is not a deletion. func removeInstance(_ instance: Instance) { + // Recorded first: the accounts on this instance may not have arrived + // yet, and one turning up afterwards has to be deleted rather than + // left as an orphan. + accountsManager.recordInstanceDeletion(instance.id) + for account in accountsManager.accounts(forInstance: instance.id) { let credentials = accountCredentialsManager(for: account) credentials?.deleteCredential(for: account) @@ -829,6 +862,9 @@ final class AppEnvironment { /// credential is something only the server can tell us, so that state is set /// by the API layer when a request actually fails. func refreshAccountStatuses() { + var didChangeFingerprints = false + defer { if didChangeFingerprints { saveCredentialFingerprints() } } + for account in accountsManager.accounts { guard let credentials = accountCredentialsManager(for: account) else { continue } @@ -865,6 +901,7 @@ final class AppEnvironment { let previousFingerprint = credentialFingerprints[account.id] let didReplaceCredential = previousFingerprint != nil && previousFingerprint != fingerprint credentialFingerprints[account.id] = fingerprint + didChangeFingerprints = true if didReplaceCredential { LoggingService.shared.info( diff --git a/Yattee/Views/Instances/InstanceBrowseView.swift b/Yattee/Views/Instances/InstanceBrowseView.swift index 65134b06..cdaf7f56 100644 --- a/Yattee/Views/Instances/InstanceBrowseView.swift +++ b/Yattee/Views/Instances/InstanceBrowseView.swift @@ -417,10 +417,7 @@ struct InstanceBrowseView: View { ) } - // Check login status for instances that support authentication - if instance.supportsAuthentication { - isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false - } + refreshLoginState() // Set initial tab only once (not on navigation back) if !hasInitializedTab { @@ -441,6 +438,15 @@ struct InstanceBrowseView: View { .onReceive(NotificationCenter.default.publisher(for: .watchHistoryDidChange)) { _ in loadWatchEntries() } + // A credential can arrive over iCloud Keychain, or an account be added, + // while this stays on screen — the Feed tab depends on it and would + // otherwise stay missing until the view was rebuilt. + .onReceive(NotificationCenter.default.publisher(for: .accountStatusesDidChange)) { _ in + refreshLoginState() + } + .onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in + refreshLoginState() + } .onChange(of: selectedTab) { _, _ in isLoading = true errorMessage = nil @@ -1173,6 +1179,11 @@ struct InstanceBrowseView: View { // MARK: - Data Loading + private func refreshLoginState() { + guard instance.supportsAuthentication else { return } + isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false + } + private func loadWatchEntries() { watchEntriesMap = appEnvironment?.dataManager.watchEntriesMap() ?? [:] } diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index bbec3d62..582011d2 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -333,6 +333,11 @@ private struct EditRemoteServerContent: View { .onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false } + // A credential arriving over iCloud Keychain changes only the status, + // and the authenticated import actions depend on it. + .onReceive(NotificationCenter.default.publisher(for: .accountStatusesDidChange)) { _ in + isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false + } .onAppear { isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 9fc907c0..7106cf4a 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -186,6 +186,24 @@ struct AccountsManagerTests { #expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id)) } + @Test("An account arriving after its instance was deleted is deleted too") + func lateAccountForDeletedInstanceIsDeleted() { + let manager = makeManager() + let instanceID = UUID() + + manager.recordInstanceDeletion(instanceID) + + // Deleting an instance deletes the accounts on it; one still in transit + // does not escape that by arriving late, and installing it would leave + // an orphan whose credential nothing can reach. + let late = Account(instanceID: instanceID) + #expect(!manager.applySyncedAccount(late)) + + #expect(manager.account(id: late.id) == nil) + #expect(manager.isDeleted(late.id)) + #expect(manager.pendingDeletions.contains(late.id)) + } + @Test("An unreachable server is retried rather than written off") func unreachableStatusExpires() async throws { let manager = AccountsManager(