mirror of
https://github.com/yattee/yattee.git
synced 2026-10-24 13:42:54 +00:00
Stop a status refresh from erasing a rejected credential
The API layer marking an account `credentialRejected` posts the same notification that recomputes statuses, which saw the Keychain item still present and immediately reported "signed in" again. The rejection could therefore never survive the moment it was recorded, and the user was never asked to sign in again. Only the server can clear a rejection now; storing a new credential resets it explicitly. Cache invalidation follows the effective context rather than the stored one. A credential arriving or disappearing flips reads between the account and local without the stored context moving, so the global feed caches kept whichever side had populated them. The login sheet stays open when the credential could not be stored — an unresolved identity scope or a Keychain failure — instead of dismissing and discarding what the user typed.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user