diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 7960bd1d..4c2fbe70 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -639,14 +639,25 @@ final class AccountsManager { accountStatuses[accountID] ?? (identityScope.isResolved ? .credentialMissing : .checking) } + /// Asked to confirm an auth-shaped failure before anything is concluded + /// from it. `nil` — tests, and the window before wiring completes — means + /// nothing is concluded at all, which is the safe direction. + /// + /// Held here as a hook rather than as a dependency because the confirmation + /// needs an API client, and this manager deliberately knows nothing about + /// the network. + @ObservationIgnored var onSuspectedCredentialRejection: (@MainActor (Account) -> Void)? + /// Records what a server call implies about the account that made it. /// - /// Deliberately conservative about rejection: a raw 401 marks the - /// credential only when no reverse-proxy basic auth is configured, because - /// otherwise the 401 may well have come from the proxy. The two-stage probe - /// that disambiguates the rest is Phase 2 work; until then an ambiguous - /// failure leaves the status alone rather than wrongly telling the user to - /// sign in again. + /// Nothing here rejects a credential. A raw 401 or 403 is *suspicion*: the + /// same statuses come back from a reverse proxy, from endpoints a valid + /// session may not touch, and — on Invidious — from a genuinely revoked + /// session, and the request that failed cannot tell them apart. It is + /// handed to `onSuspectedCredentialRejection`, which asks the question again + /// against an endpoint that exists to be authenticated; only that second + /// answer marks the account. Until it arrives the status is left alone, + /// rather than wrongly telling the user to sign in again. /// /// Shared by the subscription providers and the feed cache: a credential /// that expires during a feed-only refresh has to mark the account too, or @@ -667,13 +678,31 @@ final class AccountsManager { case APIError.cancelled: return case APIError.unauthorized: - guard instance(for: account)?.usesBasicAuth == false else { return } - setStatus(.credentialRejected, for: account.id) + suspectRejection(of: account) + case let APIError.httpError(statusCode, _) where statusCode == 401 || statusCode == 403: + // 403 arrives untranslated on purpose: the shared HTTP mapping + // turns only 401 into `.unauthorized`, because unauthenticated call + // sites read 403 as "this instance is blocked" rather than "this + // login is". Both shapes mean the same thing here. + suspectRejection(of: account) default: return } } + /// Passes an auth-shaped failure on for confirmation, unless the instance + /// itself can explain it. + private func suspectRejection(of account: Account) { + // An instance behind HTTP Basic Auth answers 401 at the proxy for every + // request made to it, whichever account made it — so the status says + // nothing about this credential, and a probe would only be answered by + // the same proxy. An orphaned account, whose instance is unknown, is + // skipped by the same guard: there is nothing to probe against. + guard instance(for: account)?.usesBasicAuth == false else { return } + + onSuspectedCredentialRejection?(account) + } + // MARK: - Legacy migration /// Turns each pre-revamp single-login instance into an account. diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 6b27cb5a..7535cfbf 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -56,6 +56,7 @@ final class AppEnvironment { let homeInstanceCache: HomeInstanceCache let invidiousAPI: InvidiousAPI let pipedAPI: PipedAPI + let credentialProbeService: CredentialProbeService let subscriptionAccountValidator: SubscriptionAccountValidator let dataContextValidator: DataContextValidator let playerControlsLayoutService: PlayerControlsLayoutService @@ -213,6 +214,45 @@ final class AppEnvironment { let pipedAPI = PipedAPI(httpClient: client) self.pipedAPI = pipedAPI + // A 401 or 403 from an ordinary request is never allowed to reject a + // login on its own — it is re-asked here against an endpoint that + // exists to be authenticated, and only that answer marks the account. + let probeService = CredentialProbeService( + accountsManager: accounts, + credentialReader: { [weak accounts, weak invidiousCreds, weak pipedCreds] account in + guard let type = accounts?.instance(for: account)?.type else { return nil } + switch type { + case .invidious: return invidiousCreds?.credential(for: account) + case .piped: return pipedCreds?.credential(for: account) + default: return nil + } + }, + transport: { instance, credential in + do { + switch instance.type { + case .invidious: + // GET /api/v1/auth/subscriptions — the cheapest call + // that does nothing except require a session. + _ = try await invidiousAPI.subscriptions(instance: instance, sid: credential) + case .piped: + _ = try await pipedAPI.subscriptions(instance: instance, authToken: credential) + default: + return .indeterminate + } + return .authenticated + } catch { + return CredentialProbeService.classify(error) + } + } + ) + self.credentialProbeService = probeService + + // Weak: the manager is owned for the life of the app and the probe + // service holds it back, so a strong capture here would be a cycle. + accounts.onSuspectedCredentialRejection = { [weak probeService] account in + probeService?.probeSuspectedRejection(for: account) + } + // Initialize SubscriptionService with all required dependencies self.subscriptionService = SubscriptionService( dataManager: dm, @@ -726,6 +766,13 @@ final class AppEnvironment { subscriptionService.discardProvider(for: account.id) HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed) + // Reached by both ways a credential is replaced — the user signing in + // again here, and a replacement arriving over iCloud Keychain that the + // fingerprint digest spots — so the probe service's per-account dedup + // is retired here rather than at either call site. A verdict earned by + // the old secret must never delay asking about the new one. + credentialProbeService.credentialDidChange(for: account.id) + if accountsManager.currentContext == .account(account.id) { SubscriptionFeedCache.shared.handleAccountChange() } diff --git a/Yattee/Services/Credentials/CredentialProbeService.swift b/Yattee/Services/Credentials/CredentialProbeService.swift new file mode 100644 index 00000000..6c6f6564 --- /dev/null +++ b/Yattee/Services/Credentials/CredentialProbeService.swift @@ -0,0 +1,233 @@ +// +// CredentialProbeService.swift +// Yattee +// +// Confirms — or refuses to confirm — that a server really rejected an +// account's credential. +// + +import Foundation + +/// Turns an auth-shaped failure into a verdict about the credential itself. +/// +/// A raw 401 or 403 from an ordinary account request is not evidence: Invidious +/// answers 403 `{"error":"Request must be authenticated"}` for a revoked +/// session, but also for endpoints an otherwise valid session may not touch, +/// and a reverse proxy in front of either backend answers 401 for reasons that +/// have nothing to do with the account. Acting on the raw status would tell +/// users with a perfectly good login to sign in again — and drop their reads to +/// local data — on the strength of one unlucky request. +/// +/// So the failure only *suspects* rejection. This service asks the question +/// again, deliberately, against an endpoint that exists purely to be +/// authenticated (the subscriptions list), and only that answer is allowed to +/// set `credentialRejected`. Everything else the probe can return maps to a +/// state that lifts by itself. +/// +/// The transport and the credential lookup are injected so tests can script the +/// answer without a server, and so this file stays free of any API-client +/// knowledge. +@MainActor +final class CredentialProbeService { + /// What a probe concluded about the credential it was given. + enum Verdict: Equatable { + /// The server accepted it — the original failure was about the + /// operation, not the login. + case authenticated + /// The server refused it. The only verdict that marks an account. + case rejected + /// The server could not be asked. Says nothing about the credential. + case unreachable + /// The probe produced an answer that proves nothing either way — a + /// decoding failure, a 500, a cancelled task. Status is left alone. + case indeterminate + } + + /// Performs one authenticated request with the given credential. + typealias Transport = @MainActor (Instance, String) async -> Verdict + + /// The credential an account currently holds, or `nil` when there is none + /// to be read — including while the identity scope is unresolved, which is + /// "cannot tell yet" rather than "signed out". + typealias CredentialReader = @MainActor (Account) -> String? + + private let accountsManager: AccountsManager + private let credentialReader: CredentialReader + private let transport: Transport + + /// How long an *answered* probe stands before another may run for the same + /// account. A failing feed refresh can produce several auth-shaped failures + /// in a row; without this each one would cost a round trip to say the same + /// thing. Injectable so tests do not have to wait a real minute. + private let cooldown: TimeInterval + + /// Probes currently in flight, keyed by account. + /// + /// Exposed because it is also the only way to await one: the probe is + /// deliberately fire-and-forget from the caller's side — `recordServerOutcome` + /// cannot become async — so tests need a handle to join on. + private(set) var probeTasks: [UUID: Task] = [:] + + /// When a probe last produced an answer *about the credential the account + /// still holds*. A verdict dropped as stale never lands here: it says + /// nothing about what is in place now, so it may not delay asking. + private var lastProbeFinished: [UUID: Date] = [:] + + /// Bumped every time an account's credential is replaced. + /// + /// A probe carries the value it was spawned under and may only speak while + /// it still matches. Cancellation alone would not do: the transport may not + /// notice it for a while, and comparing the credential is not enough either + /// — a replacement can happen to be the same string, and the comparison + /// races with the gates being cleared. + private var generations: [UUID: Int] = [:] + + init( + accountsManager: AccountsManager, + credentialReader: @escaping CredentialReader, + transport: @escaping Transport, + cooldown: TimeInterval = 60 + ) { + self.accountsManager = accountsManager + self.credentialReader = credentialReader + self.transport = transport + self.cooldown = cooldown + } + + // MARK: - Probing + + /// Asks the server whether this account's credential is still good. + /// + /// Returns immediately; the verdict is applied to `AccountsManager` when the + /// answer arrives, and only if it still describes the same credential on the + /// same account. + func probeSuspectedRejection(for account: Account) { + guard probeTasks[account.id] == nil else { return } + + if let finished = lastProbeFinished[account.id], + Date().timeIntervalSince(finished) < cooldown { + return + } + + // No credential to test. `refreshAccountStatuses` owns that verdict — + // it can tell a missing credential from an unresolved identity scope, + // and this service deliberately cannot. + guard let credential = credentialReader(account) else { return } + + // An account whose instance has not arrived (or is already gone) has + // nothing to probe against. + guard let instance = accountsManager.instance(for: account) else { return } + + let accountID = account.id + let generation = generations[accountID, default: 0] + + probeTasks[accountID] = Task { [weak self] in + let verdict = await self?.transport(instance, credential) + + guard let self else { return } + defer { + // Only if this probe is still the one on record. A credential + // change takes the slot away and a fresh probe may already be + // using it; clearing it here would strand that one's dedup. + if self.generations[accountID, default: 0] == generation { + self.probeTasks[accountID] = nil + } + } + + guard let verdict else { return } + + // The world may have moved while the probe was out. A verdict about + // a credential that has since been replaced — or read under an + // identity scope that has since flipped, which makes the reader + // return `nil` — describes nothing that is still true, and applying + // it would reject a login the user just fixed. + guard self.generations[accountID, default: 0] == generation, + let current = self.accountsManager.account(id: accountID), + self.credentialReader(current) == credential + else { + LoggingService.shared.debug( + "Dropped a credential probe verdict — the account or its credential changed", + category: .general, + details: "accountID=\(accountID)" + ) + return + } + + // Stamped only here, on the one path where an answer about the + // credential now in place was actually obtained. A dropped verdict + // describes a secret that is gone, so it may not silence the first + // probe of its replacement — the login the user just fixed is + // exactly the one whose rejection has to be found quickly. Probe + // storms are held off by the in-flight guard regardless. + self.lastProbeFinished[accountID] = Date() + + self.apply(verdict, to: accountID) + } + } + + /// Retires everything this service knows about an account's *previous* + /// credential. + /// + /// Both gates in `probeSuspectedRejection` are keyed by account alone and + /// are checked before the credential is even read, so without this a + /// replacement inherits the old secret's dedup: a finished probe would + /// silence the new login's first failure for the whole cooldown, and one + /// still in flight would silence it until that round trip returned. The + /// replacement is the login the user just fixed — a rejection of *it* is + /// the one worth finding immediately. + /// + /// The in-flight probe is cancelled as well as disowned, so a network call + /// nobody will listen to is not left running. + func credentialDidChange(for accountID: UUID) { + generations[accountID, default: 0] += 1 + lastProbeFinished[accountID] = nil + probeTasks.removeValue(forKey: accountID)?.cancel() + } + + private func apply(_ verdict: Verdict, to accountID: UUID) { + switch verdict { + case .authenticated: + // The credential works; whatever failed was about the request. + accountsManager.setStatus(.signedIn, for: accountID) + case .rejected: + LoggingService.shared.info( + "A credential probe confirmed the server rejects this account's login", + category: .general, + details: "accountID=\(accountID)" + ) + accountsManager.setStatus(.credentialRejected, for: accountID) + case .unreachable: + // Recorded rather than ignored so reads fall back to local data, + // and lifted again by the manager's own retry timer. + accountsManager.setStatus(.unreachable, for: accountID) + case .indeterminate: + break + } + } + + // MARK: - Classification + + /// Maps a probe's outcome onto a verdict. + /// + /// `nil` means the request succeeded. Only the two auth statuses reject: + /// 403 is included because Invidious answers it — not 401 — for a session + /// that has been revoked, and it reaches here as a plain `httpError` + /// because the global 403 mapping in `HTTPClient` is shared with + /// unauthenticated call sites that read it as "blocked", not "signed out". + static func classify(_ error: Error?) -> Verdict { + guard let error else { return .authenticated } + + switch error { + case APIError.unauthorized: + return .rejected + case let APIError.httpError(statusCode, _) where statusCode == 401 || statusCode == 403: + return .rejected + case APIError.timeout, APIError.noConnection: + return .unreachable + default: + // Cancellation included: a probe that was torn down never asked the + // question, so it may not answer it either. + return .indeterminate + } + } +} diff --git a/YatteeTests/CredentialProbeTests.swift b/YatteeTests/CredentialProbeTests.swift new file mode 100644 index 00000000..a37683c2 --- /dev/null +++ b/YatteeTests/CredentialProbeTests.swift @@ -0,0 +1,486 @@ +// +// CredentialProbeTests.swift +// YatteeTests +// +// The two-stage rule: an auth-shaped failure only ever *suspects* rejection, +// and nothing but a deliberate probe may confirm it. +// + +import Testing +import Foundation +@testable import Yattee + +/// A probe service wired to a scripted transport instead of a server. +/// +/// Holds the credential in a mutable box so a test can replace it mid-probe, +/// and can park every probe inside the transport so the window between "the +/// request failed" and "the probe answered" can be inspected — that window is +/// where the whole design lives. +@MainActor +private final class ProbeHarness { + let instances: InstancesManager + let instance: Instance + let accounts: AccountsManager + let account: Account + private(set) var service: CredentialProbeService! + + /// What the credential reader reports right now. + var credential: String? = "sid-original" + + /// What the next probe concludes. + var verdict: CredentialProbeService.Verdict = .authenticated + + /// Per-credential answers, for the tests where two probes overlap and each + /// has to conclude something different. + var verdictsByCredential: [String: CredentialProbeService.Verdict] = [:] + + private(set) var transportCalls = 0 + private(set) var probedCredentials: [String] = [] + + /// While true, every probe suspends inside the transport until `release()`. + var holdsProbesOpen = false + private var parked: [(credential: String, continuation: CheckedContinuation)] = [] + + init( + type: InstanceType = .invidious, + usesBasicAuth: Bool = false, + cooldown: TimeInterval = 0, + unreachableRetryInterval: TimeInterval = 60 + ) { + instances = InstancesManager() + instance = Instance( + type: type, + url: URL(string: "https://probe-\(UUID().uuidString).example.com")!, + usesBasicAuth: usesBasicAuth + ) + instances.add(instance) + + accounts = AccountsManager( + identityScope: CredentialIdentityScope(fixedScope: "identity-A"), + instancesManager: instances, + defaults: UserDefaults(suiteName: "yattee.tests.\(UUID().uuidString)")!, + unreachableRetryInterval: unreachableRetryInterval + ) + account = accounts.addAccount(instanceID: instance.id) + accounts.setStatus(.signedIn, for: account.id) + + service = CredentialProbeService( + accountsManager: accounts, + credentialReader: { [weak self] _ in self?.credential }, + transport: { [weak self] _, credential in + guard let self else { return .indeterminate } + return await self.runProbe(credential) + }, + cooldown: cooldown + ) + + accounts.onSuspectedCredentialRejection = { [weak self] account in + self?.service.probeSuspectedRejection(for: account) + } + } + + /// Instances live in the shared defaults, so each harness has to take its + /// own back out again. + func tearDown() { + instances.remove(instance) + } + + var status: AccountStatus { + accounts.status(for: account.id) + } + + var probeTask: Task? { + service.probeTasks[account.id] + } + + func record(_ error: Error?) { + accounts.recordServerOutcome(error, for: account) + } + + private func runProbe(_ credential: String) async -> CredentialProbeService.Verdict { + transportCalls += 1 + probedCredentials.append(credential) + + if holdsProbesOpen { + await withCheckedContinuation { parked.append((credential, $0)) } + } + + return verdictsByCredential[credential] ?? verdict + } + + /// Lets parked probes finish — the one probing `credential`, or every one + /// of them when it is `nil`, which also stops new probes being parked. + func release(credential: String? = nil) { + guard let credential else { + holdsProbesOpen = false + let waiting = parked + parked = [] + waiting.forEach { $0.continuation.resume() } + return + } + + let matching = parked.filter { $0.credential == credential } + parked.removeAll { $0.credential == credential } + matching.forEach { $0.continuation.resume() } + } + + /// Waits until the transport has been entered at least `count` times. + func waitForTransport(count: Int) async throws { + for _ in 0 ..< 200 { + if transportCalls >= count { return } + try await Task.sleep(for: .milliseconds(5)) + } + Issue.record("The transport was never entered \(count) time(s)") + } +} + +@Suite("Credential probe", .serialized) +@MainActor +struct CredentialProbeTests { + // MARK: - The raw status never decides + + @Test("A 403 asks the server again instead of rejecting the login") + func forbiddenNeverRejectsDirectly() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 403, message: "Request must be authenticated")) + + // Invidious answers exactly this for a revoked session — and for + // several things that are not one. The failed request may not conclude + // anything by itself. + #expect(harness.status == .signedIn) + + try await harness.waitForTransport(count: 1) + #expect(harness.status == .signedIn) + + let task = try #require(harness.probeTask) + harness.release() + await task.value + + // Only the probe's own answer marks the account. + #expect(harness.status == .credentialRejected) + } + + @Test("An unauthorized error goes through the probe as well") + func unauthorizedRoutesThroughTheProbe() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdict = .rejected + + harness.record(APIError.unauthorized) + + // A bare 401 used to reject on the spot. A reverse proxy answers it too. + #expect(harness.status == .signedIn) + + try await harness.waitForTransport(count: 1) + let task = try #require(harness.probeTask) + harness.release() + await task.value + + #expect(harness.status == .credentialRejected) + } + + // MARK: - Verdicts + + @Test("A confirmed rejection makes the selected context fall back to local") + func confirmedRejectionEngagesTheFallback() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.verdict = .rejected + + let validator = DataContextValidator( + accountsManager: harness.accounts, + instancesManager: harness.instances, + toastManager: ToastManager() + ) + harness.accounts.setContext(.account(harness.account.id)) + #expect(validator.validity == .valid) + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + await harness.probeTask?.value + + #expect(harness.status == .credentialRejected) + + // The point of the whole exercise: reads stop going to a server that + // will not answer them, without the user's choice being thrown away. + #expect(validator.validity == .temporarilyUnavailable(.credentialRejected)) + #expect(validator.effectiveContext == .local) + #expect(!validator.allowsServerMutations) + #expect(harness.accounts.currentContext == .account(harness.account.id)) + } + + @Test("A probe the server accepts leaves the account signed in") + func acceptedProbeKeepsTheAccountUsable() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.verdict = .authenticated + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + await harness.probeTask?.value + + // The credential is fine; the operation that failed was not allowed. + #expect(harness.transportCalls == 1) + #expect(harness.status == .signedIn) + } + + @Test("A probe that cannot reach the server is retried, not written off") + func unreachableProbeLiftsItself() async throws { + let harness = ProbeHarness(unreachableRetryInterval: 0.05) + defer { harness.tearDown() } + harness.verdict = .unreachable + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + await harness.probeTask?.value + + #expect(harness.status == .unreachable) + + // Nothing about the credential was learned, so the state has to expire + // on its own — the same auto-expiry every unreachable account gets. + try await Task.sleep(for: .seconds(0.5)) + #expect(harness.status == .signedIn) + } + + // MARK: - What must not be probed + + @Test("An instance behind basic auth is never probed") + func basicAuthInstanceIsNotProbed() async throws { + let harness = ProbeHarness(usesBasicAuth: true) + defer { harness.tearDown() } + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 401, message: nil)) + harness.record(APIError.unauthorized) + + try await Task.sleep(for: .milliseconds(50)) + + // The proxy answers 401 for every request to this instance whoever + // makes it, and would answer the probe identically. + #expect(harness.transportCalls == 0) + #expect(harness.probeTask == nil) + #expect(harness.status == .signedIn) + } + + @Test("Errors that are not about authentication start no probe") + func nonAuthErrorsDoNotProbe() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 500, message: "Internal Server Error")) + #expect(harness.probeTask == nil) + + harness.record(APIError.decodingError("unexpected shape")) + #expect(harness.probeTask == nil) + + try await Task.sleep(for: .milliseconds(50)) + #expect(harness.transportCalls == 0) + #expect(harness.status == .signedIn) + } + + // MARK: - Staleness and dedup + + @Test("A verdict about a credential that has since been replaced is dropped") + func replacedCredentialDropsTheVerdict() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + try await harness.waitForTransport(count: 1) + + // A replacement arriving over iCloud Keychain — or the user signing in + // again — while the probe was out. The answer describes the old secret. + harness.credential = "sid-replacement" + + let task = try #require(harness.probeTask) + harness.release() + await task.value + + #expect(harness.probedCredentials == ["sid-original"]) + #expect(harness.status == .signedIn) + } + + @Test("A dropped verdict never silences the replacement credential's first probe") + func droppedVerdictStartsNoCooldown() async throws { + let harness = ProbeHarness(cooldown: 60) + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + try await harness.waitForTransport(count: 1) + + harness.credential = "sid-replacement" + let dropped = try #require(harness.probeTask) + harness.release() + await dropped.value + #expect(harness.status == .signedIn) + + // The replacement can be rejected too, and nothing has asked about it + // yet — the login the user just fixed is exactly the one whose failure + // has to be found quickly, not a minute later. + harness.record(APIError.httpError(statusCode: 403, message: nil)) + let second = try #require(harness.probeTask) + await second.value + + #expect(harness.transportCalls == 2) + #expect(harness.probedCredentials == ["sid-original", "sid-replacement"]) + #expect(harness.status == .credentialRejected) + } + + @Test("Replacing the credential retires the cooldown the old one earned") + func credentialChangeClearsTheCooldown() async throws { + let harness = ProbeHarness(cooldown: 60) + defer { harness.tearDown() } + harness.verdict = .authenticated + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + await harness.probeTask?.value + #expect(harness.transportCalls == 1) + + // Both gates are keyed by account alone and are checked before the + // credential is even read, so the new login would otherwise inherit the + // old one's silence for a full minute. + harness.credential = "sid-replacement" + harness.service.credentialDidChange(for: harness.account.id) + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + let second = try #require(harness.probeTask) + await second.value + + #expect(harness.transportCalls == 2) + #expect(harness.probedCredentials == ["sid-original", "sid-replacement"]) + #expect(harness.status == .credentialRejected) + } + + @Test("A probe left over from a replaced credential is disowned, not merely stale") + func credentialChangeInvalidatesAProbeInFlight() async throws { + let harness = ProbeHarness(cooldown: 60) + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdictsByCredential = [ + "sid-original": .rejected, + "sid-replacement": .authenticated + ] + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + try await harness.waitForTransport(count: 1) + let stale = try #require(harness.probeTask) + + harness.credential = "sid-replacement" + harness.service.credentialDidChange(for: harness.account.id) + + // Disowned and cancelled: nobody is going to listen to its answer. + #expect(harness.probeTask == nil) + #expect(stale.isCancelled) + + // The replacement's own failure is examined at once rather than waiting + // out a round trip that is about a secret nobody holds any more. + harness.record(APIError.httpError(statusCode: 403, message: nil)) + let fresh = try #require(harness.probeTask) + #expect(fresh != stale) + try await harness.waitForTransport(count: 2) + + harness.release(credential: "sid-original") + await stale.value + + // The late rejection belongs to the old secret and may not be spoken — + // nor may finishing evict the probe that has taken its place. + #expect(harness.status == .signedIn) + #expect(harness.probeTask == fresh) + + harness.release(credential: "sid-replacement") + await fresh.value + + #expect(harness.transportCalls == 2) + #expect(harness.probedCredentials == ["sid-original", "sid-replacement"]) + #expect(harness.status == .signedIn) + #expect(harness.probeTask == nil) + } + + @Test("Repeated failures while a probe is in flight cost one round trip") + func concurrentTriggersDedupe() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + harness.holdsProbesOpen = true + harness.verdict = .rejected + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + try await harness.waitForTransport(count: 1) + + // A feed refresh produces a burst of these. + harness.record(APIError.httpError(statusCode: 403, message: nil)) + harness.record(APIError.unauthorized) + + let task = try #require(harness.probeTask) + harness.release() + await task.value + + #expect(harness.transportCalls == 1) + #expect(harness.status == .credentialRejected) + } + + @Test("A finished probe stands for the cooldown") + func cooldownSuppressesAnImmediateReprobe() async throws { + let harness = ProbeHarness(cooldown: 60) + defer { harness.tearDown() } + harness.verdict = .authenticated + + harness.record(APIError.httpError(statusCode: 403, message: nil)) + await harness.probeTask?.value + #expect(harness.transportCalls == 1) + + // The next failed request arrives moments later and asks the same + // question; the answer cannot have changed. + harness.record(APIError.httpError(statusCode: 403, message: nil)) + #expect(harness.probeTask == nil) + + try await Task.sleep(for: .milliseconds(50)) + #expect(harness.transportCalls == 1) + #expect(harness.status == .signedIn) + } + + // MARK: - Classification + + @Test("Only the two authentication statuses reject") + func classifyMapsOutcomes() { + #expect(CredentialProbeService.classify(nil) == .authenticated) + + #expect(CredentialProbeService.classify(APIError.unauthorized) == .rejected) + #expect(CredentialProbeService.classify( + APIError.httpError(statusCode: 401, message: nil) + ) == .rejected) + // The one the shared HTTP mapping leaves untranslated. + #expect(CredentialProbeService.classify( + APIError.httpError(statusCode: 403, message: "Request must be authenticated") + ) == .rejected) + + #expect(CredentialProbeService.classify(APIError.timeout) == .unreachable) + #expect(CredentialProbeService.classify(APIError.noConnection) == .unreachable) + + // A probe that never completed answers nothing. + #expect(CredentialProbeService.classify(APIError.cancelled) == .indeterminate) + #expect(CredentialProbeService.classify( + APIError.httpError(statusCode: 500, message: nil) + ) == .indeterminate) + } + + // MARK: - Regression + + @Test("A successful request still lifts a rejection") + func successLiftsARejection() { + let harness = ProbeHarness() + defer { harness.tearDown() } + + harness.accounts.setStatus(.credentialRejected, for: harness.account.id) + harness.record(nil) + + #expect(harness.status == .signedIn) + } +}