mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Keep deleted credentials and cached feeds from outliving their account
A deletion can reach a device before the account it deletes does — the tombstone and the account are separate records, and iCloud Keychain runs on its own timeline — and the credential cleanup was skipped whenever the account was unknown here. The scoped key needs only the account ID, so both credential managers are now asked to drop it; the unscoped legacy item still needs the instance and is left alone. Cached data is invalidated on the data context rather than on the legacy selection it maps to. Two logins on the same instance collapse onto the same legacy value, so switching between them cleared nothing and the subscriptions feed kept showing the previous account's videos. Home feeds are keyed by instance alone and had the same hole across accounts and across Apple accounts; they are dropped with it. The third source-deletion path now cascades to accounts like the other two, and the Invidious private-playlist lookup resolves its session through the account — a login created after the revamp writes only an account-scoped credential, so the instance-keyed lookup missed it and fell through to the public endpoint.
This commit is contained in:
@@ -70,6 +70,9 @@ final class AppEnvironment {
|
||||
@ObservationIgnored private var controlsSettingsObservers: [NSObjectProtocol] = []
|
||||
@ObservationIgnored private var contextMirrorObservers: [NSObjectProtocol] = []
|
||||
|
||||
/// The context the account-scoped caches currently hold data for.
|
||||
@ObservationIgnored private var lastMirroredContext: DataContext = .local
|
||||
|
||||
// MARK: - Shared Instance
|
||||
|
||||
/// The single, process-wide app environment.
|
||||
@@ -390,6 +393,10 @@ final class AppEnvironment {
|
||||
// naming an account that has not synced yet cannot be mapped onto the
|
||||
// legacy shape, and this is what restores the mapping once it can be
|
||||
// made rather than leaving that moment permanent.
|
||||
// Seeded with what is on screen now, so launching does not look like a
|
||||
// change and throw away caches that are still the right ones.
|
||||
lastMirroredContext = accounts.currentContext
|
||||
|
||||
for name: Notification.Name in [.dataContextDidChange, .accountsDidChange] {
|
||||
contextMirrorObservers.append(
|
||||
NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in
|
||||
@@ -401,8 +408,21 @@ final class AppEnvironment {
|
||||
// A deletion that arrives from another device takes the credential with
|
||||
// it. Only a deletion does this — an account merely missing from a
|
||||
// fetch keeps everything, because a fetch is not an enumeration.
|
||||
accountSync.onAccountDeleted = { [weak self] account in
|
||||
accountSync.onAccountDeleted = { [weak self] accountID, account in
|
||||
guard let self else { return }
|
||||
|
||||
guard let account else {
|
||||
// No metadata for this account here, so its backend is unknown
|
||||
// and the right credential manager cannot be picked. The scoped
|
||||
// key is the account ID either way, so both are asked to drop
|
||||
// it; at most one of them holds anything. The unscoped legacy
|
||||
// item is left alone — deleting it needs the instance, and its
|
||||
// ownership check is bound to one.
|
||||
self.invidiousCredentialsManager.deleteCredential(forAccountID: accountID)
|
||||
self.pipedCredentialsManager.deleteCredential(forAccountID: accountID)
|
||||
return
|
||||
}
|
||||
|
||||
let credentials = self.accountCredentialsManager(for: account)
|
||||
credentials?.deleteCredential(for: account)
|
||||
credentials?.deleteLegacyCredentialIfOwned(for: account)
|
||||
@@ -424,11 +444,11 @@ final class AppEnvironment {
|
||||
guard let self else { return }
|
||||
self.refreshAccountStatuses()
|
||||
|
||||
// The feed cache is not keyed by account, so whatever it holds
|
||||
// belongs to the Apple account that just went away. Cleared
|
||||
// unconditionally: unlike a context change, there is no state
|
||||
// here worth keeping across identities.
|
||||
SubscriptionFeedCache.shared.handleAccountChange()
|
||||
// Cleared unconditionally, because the identity moving means
|
||||
// everything cached belongs to the Apple account that just went
|
||||
// away — even when the new namespace happens to select the same
|
||||
// context and the change comparison would find nothing.
|
||||
self.clearAccountScopedCaches()
|
||||
self.mirrorContextToLegacySelection(resetWhenUnresolvable: true)
|
||||
}
|
||||
}
|
||||
@@ -544,9 +564,20 @@ final class AppEnvironment {
|
||||
/// previous identity's selection is worse than losing a mapping, and the
|
||||
/// correct value returns as soon as the new namespace's accounts arrive.
|
||||
private func mirrorContextToLegacySelection(resetWhenUnresolvable: Bool = false) {
|
||||
let context = accountsManager.currentContext
|
||||
|
||||
// Cached data is invalidated on the *context*, not on the legacy value
|
||||
// it maps to: two logins on the same instance collapse onto the same
|
||||
// legacy selection, so comparing that would let one account's private
|
||||
// feed stay on screen after switching to the other.
|
||||
if context != lastMirroredContext {
|
||||
lastMirroredContext = context
|
||||
clearAccountScopedCaches()
|
||||
}
|
||||
|
||||
let legacy: SubscriptionAccount
|
||||
|
||||
switch accountsManager.currentContext {
|
||||
switch context {
|
||||
case .local:
|
||||
legacy = .local
|
||||
|
||||
@@ -554,7 +585,8 @@ final class AppEnvironment {
|
||||
// An account whose instance has not arrived yet cannot be mapped
|
||||
// onto the legacy shape. Leaving the old value alone is right: the
|
||||
// context is preserved, and the validator reports it unavailable
|
||||
// rather than pointing the UI at something else.
|
||||
// rather than pointing the UI at something else. A later arrival
|
||||
// runs this again and restores the mapping.
|
||||
guard let account = accountsManager.account(id: id),
|
||||
let instance = accountsManager.instance(for: account) else {
|
||||
if resetWhenUnresolvable, settingsManager.subscriptionAccount != .local {
|
||||
@@ -573,14 +605,23 @@ final class AppEnvironment {
|
||||
guard settingsManager.subscriptionAccount != legacy else { return }
|
||||
|
||||
settingsManager.subscriptionAccount = legacy
|
||||
SubscriptionFeedCache.shared.handleAccountChange()
|
||||
|
||||
LoggingService.shared.info(
|
||||
"Data context changed — cleared the feed cache and updated the legacy selection",
|
||||
"Data context changed — updated the legacy subscription selection",
|
||||
category: .general
|
||||
)
|
||||
}
|
||||
|
||||
/// Drops everything cached for whichever account was being shown.
|
||||
///
|
||||
/// Neither cache is keyed by account: the subscriptions feed is a single
|
||||
/// global list, and home feeds are keyed by instance alone. Both would
|
||||
/// otherwise keep serving one login's private videos to another.
|
||||
private func clearAccountScopedCaches() {
|
||||
SubscriptionFeedCache.shared.handleAccountChange()
|
||||
HomeInstanceCache.shared.clearAllFeeds()
|
||||
}
|
||||
|
||||
/// Deletes an instance together with the accounts that live on it.
|
||||
///
|
||||
/// The cascade has to happen here, while the instance is still resolvable:
|
||||
|
||||
@@ -58,7 +58,13 @@ final class AccountSyncCoordinator {
|
||||
|
||||
/// Called when an account is deleted anywhere, so its credential and stored
|
||||
/// login identifier go with it.
|
||||
var onAccountDeleted: (@MainActor (Account) -> Void)?
|
||||
///
|
||||
/// The account is `nil` when this device never had its metadata — a
|
||||
/// tombstone and the account it kills are separate records, and a fresh
|
||||
/// device can meet the tombstone first. The credential may still have
|
||||
/// arrived on its own through iCloud Keychain, so the ID alone has to be
|
||||
/// enough to remove it.
|
||||
var onAccountDeleted: (@MainActor (UUID, Account?) -> Void)?
|
||||
|
||||
// MARK: - Identity
|
||||
|
||||
@@ -372,8 +378,7 @@ final class AccountSyncCoordinator {
|
||||
}
|
||||
|
||||
private func cleanUpCredentials(for account: Account?, accountID: UUID) {
|
||||
guard let account else { return }
|
||||
onAccountDeleted?(account)
|
||||
onAccountDeleted?(accountID, account)
|
||||
LoggingService.shared.logCloudKit("Removed deleted account from iCloud: \(accountID)")
|
||||
}
|
||||
|
||||
|
||||
@@ -32,6 +32,15 @@ protocol AccountCredentialsManager: AnyObject, Observable {
|
||||
func credential(for account: Account) -> String?
|
||||
func deleteCredential(for account: Account)
|
||||
|
||||
/// Deletes the credential and stored login identifier for an account this
|
||||
/// device may know nothing else about.
|
||||
///
|
||||
/// A deletion can reach this device before the account it deletes does —
|
||||
/// the tombstone and the account are separate records, and iCloud Keychain
|
||||
/// runs on its own timeline. The secret must go regardless, and the scoped
|
||||
/// key needs only the account's ID.
|
||||
func deleteCredential(forAccountID accountID: UUID)
|
||||
|
||||
/// The login identifier the user typed. Kept in the Keychain beside the
|
||||
/// credential so it never enters synced metadata.
|
||||
func setUsername(_ username: String, for account: Account)
|
||||
|
||||
@@ -107,19 +107,22 @@ final class AccountKeychainCredentials {
|
||||
}
|
||||
|
||||
func deleteCredential(for account: Account) {
|
||||
deleteCredential(forAccountID: account.id)
|
||||
HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed)
|
||||
}
|
||||
|
||||
func deleteCredential(forAccountID accountID: UUID) {
|
||||
guard let scope = currentScope() else { return }
|
||||
|
||||
store.delete(key: KeychainCredentialStore.credentialKey(scope: scope, accountID: account.id))
|
||||
store.delete(key: KeychainCredentialStore.usernameKey(scope: scope, accountID: account.id))
|
||||
loginsByScope[scope]?.remove(account.id)
|
||||
store.delete(key: KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID))
|
||||
store.delete(key: KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID))
|
||||
loginsByScope[scope]?.remove(accountID)
|
||||
|
||||
LoggingService.shared.info(
|
||||
"Deleted \(credentialNoun)",
|
||||
category: .keychain,
|
||||
details: "accountID=\(account.id)"
|
||||
details: "accountID=\(accountID)"
|
||||
)
|
||||
|
||||
HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed)
|
||||
}
|
||||
|
||||
// MARK: - Username
|
||||
|
||||
@@ -285,6 +285,10 @@ extension InvidiousCredentialsManager: AccountCredentialsManager {
|
||||
accountCredentials.deleteCredential(for: account)
|
||||
}
|
||||
|
||||
func deleteCredential(forAccountID accountID: UUID) {
|
||||
accountCredentials.deleteCredential(forAccountID: accountID)
|
||||
}
|
||||
|
||||
func setUsername(_ username: String, for account: Account) {
|
||||
accountCredentials.setUsername(username, for: account)
|
||||
}
|
||||
|
||||
@@ -214,6 +214,10 @@ extension PipedCredentialsManager: AccountCredentialsManager {
|
||||
accountCredentials.deleteCredential(for: account)
|
||||
}
|
||||
|
||||
func deleteCredential(forAccountID accountID: UUID) {
|
||||
accountCredentials.deleteCredential(forAccountID: accountID)
|
||||
}
|
||||
|
||||
func setUsername(_ username: String, for account: Account) {
|
||||
accountCredentials.setUsername(username, for: account)
|
||||
}
|
||||
|
||||
@@ -176,6 +176,32 @@ final class HomeInstanceCache {
|
||||
}
|
||||
|
||||
/// Clears all cached content for a specific instance.
|
||||
/// Drops every cached feed, on every instance.
|
||||
///
|
||||
/// Feeds are the one content type here that is private to a login, and the
|
||||
/// cache key names only the instance — two accounts on the same instance,
|
||||
/// or the same instance ID retained across a change of Apple account, would
|
||||
/// otherwise be served each other's. Popular and trending are public and
|
||||
/// stay.
|
||||
func clearAllFeeds() {
|
||||
let keysToRemove = cache.keys.filter { $0.hasSuffix("_\(InstanceContentType.feed.rawValue)") }
|
||||
guard !keysToRemove.isEmpty else { return }
|
||||
|
||||
for key in keysToRemove {
|
||||
cache.removeValue(forKey: key)
|
||||
lastUpdated.removeValue(forKey: key)
|
||||
}
|
||||
|
||||
LoggingService.shared.debug(
|
||||
"HomeInstanceCache.clearAllFeeds: Cleared \(keysToRemove.count) cached feeds",
|
||||
category: .general
|
||||
)
|
||||
|
||||
Task {
|
||||
await saveToDisk()
|
||||
}
|
||||
}
|
||||
|
||||
func clearAllForInstance(_ instanceID: UUID) {
|
||||
let keysToRemove = cache.keys.filter { $0.hasPrefix(instanceID.uuidString) }
|
||||
for key in keysToRemove {
|
||||
|
||||
@@ -779,7 +779,11 @@ struct UnifiedPlaylistDetailView: View {
|
||||
// Check if this is an Invidious user playlist (IVPL prefix) that requires authentication
|
||||
let isInvidiousUserPlaylist = playlistID.playlistID.hasPrefix("IVPL")
|
||||
let isInvidiousInstance = resolvedInstance.type == .invidious
|
||||
let sid = appEnvironment.invidiousCredentialsManager.sid(for: resolvedInstance)
|
||||
// Resolved through the account, not the instance: a login created
|
||||
// after the revamp only ever writes an account-scoped credential,
|
||||
// so the instance-keyed lookup would miss it and fall through to
|
||||
// the public endpoint — which cannot see a private playlist.
|
||||
let sid = appEnvironment.credential(forInstance: resolvedInstance)
|
||||
|
||||
if isInvidiousUserPlaylist && isInvidiousInstance, let sid {
|
||||
// Use authenticated endpoint for Invidious user playlists (including private ones)
|
||||
|
||||
@@ -584,8 +584,8 @@ struct SourcesListView: View {
|
||||
// MARK: - Actions
|
||||
|
||||
private func confirmDelete() {
|
||||
if let instance = pendingDeleteInstance, let manager = instancesManager {
|
||||
manager.remove(instance)
|
||||
if let instance = pendingDeleteInstance {
|
||||
appEnvironment?.removeInstance(instance)
|
||||
pendingDeleteInstance = nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user