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(