From defa6cf08482115f6d4fc14b6a6264c5d89b6948 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Tue, 25 Aug 2026 22:40:47 +0200 Subject: [PATCH] Make the account picker and feed route see account logins Round three of review. The subscription picker is fed by SubscriptionAccountValidator, which still decided availability from the instance-keyed credential managers. A login created after the revamp writes only an account-scoped key, so it never appeared in the picker at all - which meant the account resolver added last round could not even be reached. Availability now comes from AccountsManager and account status. SubscriptionFeedCache dispatched on the legacy setting and picked its instance from it, so an account waiting on its credential produced subscriptions from local data while the feed reported notAuthenticated, and deleting the selected account could leave the route pointing at its server. Both the route and the credential now come from the effective context, with the legacy lookup kept only as a fallback for state that predates accounts. Foreground activation re-resolved the identity scope but only recomputed statuses when the scope actually moved. An iCloud Keychain credential arriving after launch does not move it, so the account stayed credentialMissing and reads stayed on the local fallback until relaunch. Statuses are now recomputed on every activation in both scene-phase branches. Providers are bound to an account but never told it anything: a revoked SID left the account signedIn, so the validator kept treating it as valid and neither the banner nor the read fallback engaged. Server calls now record their outcome. Conservatively - a raw 401 marks the credential rejected only when no reverse-proxy basic auth is configured, since otherwise the 401 may have come from the proxy. The two-stage probe that disambiguates the rest is Phase 2; until then an ambiguous failure leaves the status alone rather than wrongly telling the user to sign in again. --- Yattee/Core/AppEnvironment.swift | 20 ++++++++++ Yattee/Services/SubscriptionFeedCache.swift | 39 +++++++++++++----- .../InvidiousSubscriptionProvider.swift | 40 ++++++++++++++++++- .../PipedSubscriptionProvider.swift | 36 ++++++++++++++++- .../SubscriptionAccountValidator.swift | 19 ++++++++- Yattee/YatteeApp.swift | 6 +++ 6 files changed, 144 insertions(+), 16 deletions(-) diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 9b203365..8f3622a2 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -365,6 +365,7 @@ final class AppEnvironment { dataContextValidator.setAppEnvironment(self) subscriptionService.setContextValidator(dataContextValidator) + subscriptionAccountValidator.setAccountsManager(accounts) // Statuses are cleared whenever the identity namespace reloads, so they // have to be recomputed once the new scope's Keychain is readable — @@ -510,6 +511,25 @@ final class AppEnvironment { } } + /// Which feed implementation a context needs. + /// + /// `.local` covers both the local data set and any account whose instance + /// or type cannot supply a feed — those fall back rather than failing. + enum FeedBackend { case local, invidious, piped } + + func feedBackend(for context: DataContext) -> FeedBackend { + guard case let .account(id) = context, + let account = accountsManager.account(id: id), + let instance = accountsManager.instance(for: account) + else { return .local } + + switch instance.type { + case .invidious: return .invidious + case .piped: return .piped + default: return .local + } + } + /// The selected context's account, when it lives on an instance of the /// given type. Used by paths that still need to know which backend they are /// talking to before they can build a request. diff --git a/Yattee/Services/SubscriptionFeedCache.swift b/Yattee/Services/SubscriptionFeedCache.swift index 1826ea36..95248efb 100644 --- a/Yattee/Services/SubscriptionFeedCache.swift +++ b/Yattee/Services/SubscriptionFeedCache.swift @@ -229,8 +229,12 @@ final class SubscriptionFeedCache { return } - let accountType = appEnvironment.settingsManager.subscriptionAccount.type - LoggingService.shared.debug("Starting feed refresh for account type: \(accountType)", category: .general) + // The *effective* context, so an account still waiting on its instance + // or credential falls back to local rather than reporting + // notAuthenticated while the subscriptions list already shows local data. + let context = appEnvironment.dataContextValidator.effectiveContext + let backend = appEnvironment.feedBackend(for: context) + LoggingService.shared.debug("Starting feed refresh for \(backend)", category: .general) // Use a detached task to prevent SwiftUI from cancelling the request // when @State properties change during the refresh operation. @@ -238,7 +242,7 @@ final class SubscriptionFeedCache { // before feedCache.refresh() completes, which can cause SwiftUI to // cancel the parent task. await Task.detached { @MainActor in - switch accountType { + switch backend { case .local: await self.refreshLocalAccount(using: appEnvironment) case .invidious: @@ -284,12 +288,18 @@ final class SubscriptionFeedCache { /// Gets the authenticated Invidious instance and session ID. private func getInvidiousAuth(using appEnvironment: AppEnvironment) -> (Instance, String)? { - // Get the instance ID from subscription account settings - let account = appEnvironment.settingsManager.subscriptionAccount - + // Resolve through the selected account so a second login on the same + // instance uses its own credential. + if let account = appEnvironment.account(forSelectedContext: .invidious), + let instance = appEnvironment.accountsManager.instance(for: account), + let sid = appEnvironment.accountCredentialsManager(for: account)?.credential(for: account) { + return (instance, sid) + } + + let legacyAccount = appEnvironment.settingsManager.subscriptionAccount + let instance: Instance? - if let instanceID = account.instanceID { - // Use the specific instance from account settings + if let instanceID = legacyAccount.instanceID { instance = appEnvironment.instancesManager.instances.first { $0.id == instanceID } } else { // Fall back to first enabled Invidious instance @@ -349,9 +359,15 @@ final class SubscriptionFeedCache { /// Gets the authenticated Piped instance and auth token. private func getPipedAuth(using appEnvironment: AppEnvironment) -> (Instance, String)? { - let account = appEnvironment.settingsManager.subscriptionAccount + if let account = appEnvironment.account(forSelectedContext: .piped), + let instance = appEnvironment.accountsManager.instance(for: account), + let token = appEnvironment.accountCredentialsManager(for: account)?.credential(for: account) { + return (instance, token) + } - guard let instanceID = account.instanceID, + let legacyAccount = appEnvironment.settingsManager.subscriptionAccount + + guard let instanceID = legacyAccount.instanceID, let instance = appEnvironment.instancesManager.instances.first(where: { $0.id == instanceID }), let authToken = appEnvironment.credential(forInstance: instance) else { LoggingService.shared.debug("No authenticated Piped instance found for subscription account", category: .general) @@ -414,7 +430,8 @@ final class SubscriptionFeedCache { func loadMoreInvidiousFeed(using appEnvironment: AppEnvironment) async { guard !isLoading else { return } guard hasMorePages else { return } - guard appEnvironment.settingsManager.subscriptionAccount.type == .invidious else { return } + guard appEnvironment.feedBackend(for: appEnvironment.dataContextValidator.effectiveContext) == .invidious + else { return } guard let (instance, sid) = getInvidiousAuth(using: appEnvironment) else { return diff --git a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift index a25cf079..e4400ac9 100644 --- a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift @@ -23,6 +23,26 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { private let invidiousAPI: InvidiousAPI private let credentialsManager: InvidiousCredentialsManager private let accountsManager: AccountsManager + + /// Records the outcome of a server call against the account. + /// + /// Deliberately conservative: a raw 401 marks the credential rejected only + /// when no reverse-proxy basic auth is configured, because otherwise the + /// 401 may well have come from the proxy rather than the account. The + /// two-stage probe that disambiguates the remaining case is Phase 2 work; + /// until then an ambiguous failure leaves the status untouched rather than + /// wrongly telling the user to sign in again. + private func recordOutcome(_ error: Error?) { + guard let error else { + accountsManager.setStatus(.signedIn, for: account.id) + return + } + + guard case APIError.unauthorized = error else { return } + guard accountsManager.instance(for: account)?.usesBasicAuth == false else { return } + + accountsManager.setStatus(.credentialRejected, for: account.id) + } private let settingsManager: SettingsManager /// In-memory cache of subscribed channel IDs for fast lookup. @@ -56,7 +76,9 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { let (instance, sid) = try await getAuthenticatedInstance() // Fetch subscriptions from Invidious - let subscriptions = try await invidiousAPI.subscriptions(instance: instance, sid: sid) + let subscriptions = try await tracking { + try await invidiousAPI.subscriptions(instance: instance, sid: sid) + } // Update in-memory cache cachedChannelIDs = Set(subscriptions.map(\.authorId)) @@ -72,7 +94,9 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { let (instance, sid) = try await getAuthenticatedInstance() // Subscribe on Invidious server - try await invidiousAPI.subscribe(to: channel.id.channelID, instance: instance, sid: sid) + try await tracking { + try await invidiousAPI.subscribe(to: channel.id.channelID, instance: instance, sid: sid) + } // Update in-memory cache only cachedChannelIDs.insert(channel.id.channelID) @@ -121,6 +145,18 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { // MARK: - Private Helpers /// Gets the authenticated Invidious instance and session ID from account settings. + /// Runs a server call and records what it implies about the credential. + private func tracking(_ operation: () async throws -> T) async rethrows -> T { + do { + let value = try await operation() + recordOutcome(nil) + return value + } catch { + recordOutcome(error) + throw error + } + } + private func getAuthenticatedInstance() async throws -> (Instance, String) { // The account names its instance directly - no falling back to // "some enabled Invidious instance", which could silently send one diff --git a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift index 0e02752a..61da5db7 100644 --- a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift @@ -23,6 +23,26 @@ final class PipedSubscriptionProvider: SubscriptionProvider { private let pipedAPI: PipedAPI private let credentialsManager: PipedCredentialsManager private let accountsManager: AccountsManager + + /// Records the outcome of a server call against the account. + /// + /// Deliberately conservative: a raw 401 marks the credential rejected only + /// when no reverse-proxy basic auth is configured, because otherwise the + /// 401 may well have come from the proxy rather than the account. The + /// two-stage probe that disambiguates the remaining case is Phase 2 work; + /// until then an ambiguous failure leaves the status untouched rather than + /// wrongly telling the user to sign in again. + private func recordOutcome(_ error: Error?) { + guard let error else { + accountsManager.setStatus(.signedIn, for: account.id) + return + } + + guard case APIError.unauthorized = error else { return } + guard accountsManager.instance(for: account)?.usesBasicAuth == false else { return } + + accountsManager.setStatus(.credentialRejected, for: account.id) + } private let settingsManager: SettingsManager /// In-memory cache of subscribed channel IDs for fast lookup. @@ -56,7 +76,9 @@ final class PipedSubscriptionProvider: SubscriptionProvider { let (instance, authToken) = try await getAuthenticatedInstance() // Fetch subscriptions from Piped - let subscriptions = try await pipedAPI.subscriptions(instance: instance, authToken: authToken) + let subscriptions = try await tracking { + try await pipedAPI.subscriptions(instance: instance, authToken: authToken) + } // Update in-memory cache cachedChannelIDs = Set(subscriptions.map(\.channelId)) @@ -121,6 +143,18 @@ final class PipedSubscriptionProvider: SubscriptionProvider { // MARK: - Private Helpers /// Gets the authenticated Piped instance and auth token from account settings. + /// Runs a server call and records what it implies about the credential. + private func tracking(_ operation: () async throws -> T) async rethrows -> T { + do { + let value = try await operation() + recordOutcome(nil) + return value + } catch { + recordOutcome(error) + throw error + } + } + private func getAuthenticatedInstance() async throws -> (Instance, String) { // The account names its instance directly - no falling back to // "some enabled Piped instance", which could silently send one diff --git a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift index d744d58c..9c0f2722 100644 --- a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift +++ b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift @@ -21,6 +21,10 @@ final class SubscriptionAccountValidator { private let pipedCredentialsManager: PipedCredentialsManager private let toastManager: ToastManager private let feedCache: SubscriptionFeedCache + /// Availability is decided by accounts now: a login created after the + /// revamp writes only an account-scoped Keychain key, so the instance-keyed + /// check this validator used would never see it. + private weak var accountsManager: AccountsManager? // MARK: - Observation State @@ -46,6 +50,17 @@ final class SubscriptionAccountValidator { startObserving() } + func setAccountsManager(_ manager: AccountsManager) { + accountsManager = manager + } + + /// Whether any account on this instance is usable. + private func hasSignedInAccount(for instance: Instance) -> Bool { + guard let accountsManager else { return false } + return accountsManager.accounts(forInstance: instance.id) + .contains { accountsManager.status(for: $0.id) == .signedIn } + } + /// Stops observation. Call before releasing the validator. func stopObserving() { observationTask?.cancel() @@ -71,7 +86,7 @@ final class SubscriptionAccountValidator { for instance in instancesManager.instances { if instance.type == .invidious && instance.isEnabled && - invidiousCredentialsManager.isLoggedIn(for: instance) { + hasSignedInAccount(for: instance) { accounts.append(.invidious(instance.id)) } } @@ -80,7 +95,7 @@ final class SubscriptionAccountValidator { for instance in instancesManager.instances { if instance.type == .piped && instance.isEnabled && - pipedCredentialsManager.isLoggedIn(for: instance) { + hasSignedInAccount(for: instance) { accounts.append(.piped(instance.id)) } } diff --git a/Yattee/YatteeApp.swift b/Yattee/YatteeApp.swift index d68f10cd..f438b9ee 100644 --- a/Yattee/YatteeApp.swift +++ b/Yattee/YatteeApp.swift @@ -292,6 +292,9 @@ struct YatteeApp: App { // launched offline) - account credentials stay unavailable until it does Task { await appEnvironment.credentialIdentityScope.resolveIfNeeded() + // A credential can arrive over iCloud Keychain without the + // scope moving, so statuses are recomputed unconditionally. + appEnvironment.refreshAccountStatuses() } // Validate subscription account (auto-correct if invalid) @@ -345,6 +348,9 @@ struct YatteeApp: App { // launched offline) - account credentials stay unavailable until it does Task { await appEnvironment.credentialIdentityScope.resolveIfNeeded() + // A credential can arrive over iCloud Keychain without the + // scope moving, so statuses are recomputed unconditionally. + appEnvironment.refreshAccountStatuses() } // Validate subscription account (auto-correct if invalid)