diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index b842fb52..c37f8ea1 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -71,7 +71,7 @@ final class AppEnvironment { @ObservationIgnored private var contextMirrorObservers: [NSObjectProtocol] = [] /// The context the account-scoped caches currently hold data for. - @ObservationIgnored private var lastMirroredContext: DataContext = .local + @ObservationIgnored private var lastCachedContext: DataContext = .local // MARK: - Shared Instance @@ -397,11 +397,6 @@ final class AppEnvironment { // account's credential findable, so statuses are recomputed with them: // an account that appears mid-session would otherwise sit at "sign in // required" until the app was relaunched. - // - // 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, .instancesDidChange] { contextMirrorObservers.append( NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in @@ -475,6 +470,10 @@ final class AppEnvironment { accounts.migrateLegacyContextIfNeeded() refreshAccountStatuses() + // Seeded once the statuses it depends on are known, so launching does + // not look like a change and throw away caches that are still right. + lastCachedContext = dataContextValidator.effectiveContext + // Resolve the credential identity scope if this Apple account has never // been seen on this device. Until it lands, account credentials report // "can't tell yet" rather than "signed out". @@ -580,20 +579,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 + // Cached data is invalidated on the *effective* context — the one whose + // data is actually on screen. Not on the legacy value, because two + // logins on the same instance collapse onto the same one; and not on + // the stored context, because that does not move when a credential + // arrives or disappears and the app falls back to local and back again. + let effective = dataContextValidator.effectiveContext + if effective != lastCachedContext { + lastCachedContext = effective clearAccountScopedCaches() } let legacy: SubscriptionAccount - switch context { + switch accountsManager.currentContext { case .local: legacy = .local @@ -787,10 +786,22 @@ final class AppEnvironment { guard credentials.isScopeResolved else { continue } credentials.refreshLoginStatus(for: account) - accountsManager.setStatus( - credentials.isLoggedIn(for: account) ? .signedIn : .credentialMissing, - for: account.id - ) + + guard credentials.isLoggedIn(for: account) else { + accountsManager.setStatus(.credentialMissing, for: account.id) + continue + } + + // A credential the server has rejected is still a credential: only + // the server can say it works again. Reporting "signed in" here + // because the Keychain item exists would erase the rejection the + // moment it was recorded — the API layer sets it, which posts the + // notification that runs this — and the user would never be asked + // to sign in again. Storing a new credential resets the status + // explicitly. + guard accountsManager.status(for: account.id) != .credentialRejected else { continue } + + accountsManager.setStatus(.signedIn, for: account.id) } } diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index 3a54a069..1118f3f2 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -389,8 +389,11 @@ private struct EditRemoteServerContent: View { /// /// The identifier is stored beside the credential in the Keychain rather /// than on the account record, so it never travels as synced metadata. - private func storeLogin(credential: String, username: String, target: AccountLoginTarget) { - guard let appEnvironment else { return } + /// - Returns: whether the credential was stored. A login the server + /// accepted is not the same as one this device could keep. + @discardableResult + private func storeLogin(credential: String, username: String, target: AccountLoginTarget) -> Bool { + guard let appEnvironment else { return false } // Credential storage refuses to write under an unresolved identity // scope, so creating the account first would leave one with no @@ -400,13 +403,13 @@ private struct EditRemoteServerContent: View { appEnvironment.toastManager.showError( String(localized: "accounts.signIn.identityUnresolved") ) - return + return false } let account = target.account ?? appEnvironment.accountsManager.addAccount(instanceID: instance.id) - guard let credentials = appEnvironment.accountCredentialsManager(for: account) else { return } + guard let credentials = appEnvironment.accountCredentialsManager(for: account) else { return false } credentials.setCredential(credential, for: account) @@ -420,7 +423,7 @@ private struct EditRemoteServerContent: View { appEnvironment.toastManager.showError( String(localized: "accounts.signIn.storeFailed") ) - return + return false } if !username.isEmpty { credentials.setUsername(username, for: account) @@ -442,6 +445,7 @@ private struct EditRemoteServerContent: View { appEnvironment.accountCredentialDidChange(for: account) isLoggedIn = true + return true } private func loadServerInfo() async { diff --git a/Yattee/Views/Settings/InstanceLoginView.swift b/Yattee/Views/Settings/InstanceLoginView.swift index ddd8494a..ad9f5743 100644 --- a/Yattee/Views/Settings/InstanceLoginView.swift +++ b/Yattee/Views/Settings/InstanceLoginView.swift @@ -9,13 +9,18 @@ import SwiftUI struct InstanceLoginView: View { let instance: Instance - /// Receives the credential **and** the identifier the user signed in with. + /// Receives the credential **and** the identifier the user signed in with, + /// and reports whether it was stored. /// /// The identifier is handed back rather than stored here because it belongs /// beside the credential in the Keychain, not in synced account metadata — /// so only the caller, which knows which account this login is for, can /// file it correctly. - let onLoginSuccess: (_ credential: String, _ username: String) -> Void + /// + /// The return value matters because signing in successfully is not the same + /// as having somewhere to keep the result: dismissing on a Keychain failure + /// would throw away what the user typed and make them do it all again. + let onLoginSuccess: (_ credential: String, _ username: String) -> Bool @Environment(\.dismiss) private var dismiss @Environment(\.appEnvironment) private var appEnvironment @@ -153,7 +158,13 @@ struct InstanceLoginView: View { let credential = try await performLogin(appEnvironment: appEnvironment) await MainActor.run { - onLoginSuccess(credential, username) + guard onLoginSuccess(credential, username) else { + // The failure is reported by a toast from the storing + // side; staying open keeps the form filled in so the + // user can retry once it can succeed. + isLoading = false + return + } dismiss() } } catch APIError.unauthorized { @@ -213,13 +224,13 @@ struct InstanceLoginView: View { #Preview("Invidious") { InstanceLoginView( instance: Instance(type: .invidious, url: URL(string: "https://invidious.example.com")!) - ) { _, _ in } + ) { _, _ in true } .appEnvironment(.preview) } #Preview("Piped") { InstanceLoginView( instance: Instance(type: .piped, url: URL(string: "https://piped.example.com")!) - ) { _, _ in } + ) { _, _ in true } .appEnvironment(.preview) }