diff --git a/Yattee/Services/CloudKit/CloudKitIdentity.swift b/Yattee/Services/CloudKit/CloudKitIdentity.swift index 6bbb3fbd..a05c176d 100644 --- a/Yattee/Services/CloudKit/CloudKitIdentity.swift +++ b/Yattee/Services/CloudKit/CloudKitIdentity.swift @@ -42,6 +42,44 @@ enum CloudKitIdentity { return shortDigest(of: data) } + /// Whether an Apple account is signed in, as distinct from unreachable. + /// + /// `ubiquityIdentityToken` alone cannot answer this: Foundation returns + /// `nil` both when the user is signed out *and* when iCloud Drive is + /// unavailable or switched off, and Yattee declares only the CloudKit + /// service (no ubiquity container), so the token's availability tracks + /// system iCloud Drive state rather than the account. Treating a nil token + /// as "signed out" would collapse two different Apple accounts into the + /// same `local` credential scope. + enum AccountAvailability { + /// No Apple account — the `local` scope is genuinely correct. + case noAccount + /// Signed in; the user record can be fetched. + case available + /// Unreachable, restricted, or undetermined. Never assume either way. + case indeterminate + } + + static func accountAvailability() async -> AccountAvailability { + guard CloudKitAvailability.isAvailable else { return .noAccount } + + do { + let container = CKContainer(identifier: AppIdentifiers.iCloudContainer) + switch try await container.accountStatus() { + case .noAccount: return .noAccount + case .available: return .available + default: return .indeterminate + } + } catch { + LoggingService.shared.debug( + "Could not determine CloudKit account status", + category: .cloudKit, + details: "\(error)" + ) + return .indeterminate + } + } + /// The CloudKit user record name for the current identity. /// /// Returns `nil` when CloudKit is unavailable (sideloaded build), when no diff --git a/Yattee/Services/Credentials/AccountKeychainCredentials.swift b/Yattee/Services/Credentials/AccountKeychainCredentials.swift index eef397b1..30d01dfc 100644 --- a/Yattee/Services/Credentials/AccountKeychainCredentials.swift +++ b/Yattee/Services/Credentials/AccountKeychainCredentials.swift @@ -170,7 +170,7 @@ final class AccountKeychainCredentials { func hasAdoptableLegacyCredential(for account: Account) -> Bool { guard let scope = currentScope() else { return false } - guard !markers.hasAdopted(instanceID: account.instanceID, scope: scope) else { return false } + guard !markers.hasAdopted(service: store.service, instanceID: account.instanceID, scope: scope) else { return false } guard credential(for: account) == nil else { return false } return legacyCredential(for: account) != nil @@ -186,7 +186,7 @@ final class AccountKeychainCredentials { @discardableResult func adoptLegacyCredential(for account: Account) -> Bool { guard let scope = currentScope() else { return false } - guard !markers.hasAdopted(instanceID: account.instanceID, scope: scope) else { return false } + guard !markers.hasAdopted(service: store.service, instanceID: account.instanceID, scope: scope) else { return false } guard let legacy = legacyCredential(for: account) else { return false } // Eligibility is re-checked here, not just when the offer was rendered: @@ -249,6 +249,7 @@ final class AccountKeychainCredentials { } markers.record( + service: store.service, instanceID: account.instanceID, accountID: account.id, scope: scope, @@ -272,6 +273,7 @@ final class AccountKeychainCredentials { let currentValue = store.value(forKey: legacyKey) guard markers.canDeleteLegacyItem( + service: store.service, instanceID: account.instanceID, accountID: account.id, scope: scope, @@ -286,7 +288,7 @@ final class AccountKeychainCredentials { } store.delete(key: legacyKey) - markers.removeMarker(instanceID: account.instanceID, scope: scope) + markers.removeMarker(service: store.service, instanceID: account.instanceID, scope: scope) LoggingService.shared.info( "Deleted the legacy \(credentialNoun) alongside its account", diff --git a/Yattee/Services/Credentials/CredentialAdoptionMarker.swift b/Yattee/Services/Credentials/CredentialAdoptionMarker.swift index 975ebf51..ddabce42 100644 --- a/Yattee/Services/Credentials/CredentialAdoptionMarker.swift +++ b/Yattee/Services/Credentials/CredentialAdoptionMarker.swift @@ -20,6 +20,12 @@ struct CredentialAdoptionMarker: Codable, Hashable, Sendable { /// The identity scope that adopted the item. Apple account A's marker must /// never authorize anything under account B. let identityScope: String + /// Keychain service the adopted item lives in. The Invidious and Piped + /// managers keep separate services but share this store, and an instance + /// UUID can outlive a change of backend — without this, adopting one + /// backend's credential would suppress the other's offer and point + /// deletion at the wrong secret. + let service: String let instanceID: UUID let accountID: UUID /// Salted, truncated digest of the adopted credential — non-secret, and @@ -56,22 +62,30 @@ final class CredentialAdoptionMarkerStore { /// /// Once true, the app stops offering adoption and stops reading the legacy /// key entirely for that account. - func hasAdopted(instanceID: UUID, scope: String) -> Bool { - markers().contains { $0.instanceID == instanceID && $0.identityScope == scope } + func hasAdopted(service: String, instanceID: UUID, scope: String) -> Bool { + marker(service: service, instanceID: instanceID, scope: scope) != nil + } + + private func marker(service: String, instanceID: UUID, scope: String) -> CredentialAdoptionMarker? { + markers().first { + $0.service == service && $0.instanceID == instanceID && $0.identityScope == scope + } } func record( + service: String, instanceID: UUID, accountID: UUID, scope: String, secret: String ) { var all = markers().filter { - !($0.instanceID == instanceID && $0.identityScope == scope) + !($0.service == service && $0.instanceID == instanceID && $0.identityScope == scope) } all.append( CredentialAdoptionMarker( identityScope: scope, + service: service, instanceID: instanceID, accountID: accountID, secretFingerprint: fingerprint(of: secret), @@ -81,8 +95,10 @@ final class CredentialAdoptionMarkerStore { save(all) } - func removeMarker(instanceID: UUID, scope: String) { - save(markers().filter { !($0.instanceID == instanceID && $0.identityScope == scope) }) + func removeMarker(service: String, instanceID: UUID, scope: String) { + save(markers().filter { + !($0.service == service && $0.instanceID == instanceID && $0.identityScope == scope) + }) } // MARK: - Legacy deletion authority @@ -107,14 +123,15 @@ final class CredentialAdoptionMarkerStore { /// Deleting a synchronizable item propagates everywhere, so a wrong answer /// here logs other devices out. func canDeleteLegacyItem( + service: String, instanceID: UUID, accountID: UUID, scope: String, currentLegacyValue: String? ) -> Bool { - guard let marker = markers().first(where: { - $0.instanceID == instanceID && $0.identityScope == scope - }) else { return false } + guard let marker = marker(service: service, instanceID: instanceID, scope: scope) else { + return false + } guard marker.accountID == accountID else { return false } guard let currentLegacyValue else { return false } diff --git a/Yattee/Services/Credentials/CredentialIdentityScope.swift b/Yattee/Services/Credentials/CredentialIdentityScope.swift index 56dc540f..424c0c73 100644 --- a/Yattee/Services/Credentials/CredentialIdentityScope.swift +++ b/Yattee/Services/Credentials/CredentialIdentityScope.swift @@ -59,6 +59,7 @@ final class CredentialIdentityScope { private let defaults: UserDefaults private let resolveUserRecordID: @Sendable () async -> String? private let fingerprintProvider: @Sendable () -> String? + private let accountAvailabilityProvider: @Sendable () async -> CloudKitIdentity.AccountAvailability /// Backoff between attempts. Injectable so tests don't pay real seconds. private let retryDelay: @Sendable (Int) async -> Void /// Written only on the main actor; read in `deinit`, which runs after the @@ -89,6 +90,9 @@ final class CredentialIdentityScope { defaults: UserDefaults = .standard, fingerprintProvider: @escaping @Sendable () -> String? = { CloudKitIdentity.ubiquityFingerprint }, resolveUserRecordID: @escaping @Sendable () async -> String? = { await CloudKitIdentity.userRecordID() }, + accountAvailabilityProvider: @escaping @Sendable () async -> CloudKitIdentity.AccountAvailability = { + await CloudKitIdentity.accountAvailability() + }, retryDelay: @escaping @Sendable (Int) async -> Void = { attempt in try? await Task.sleep(for: .seconds(pow(2.0, Double(attempt)))) } @@ -96,6 +100,7 @@ final class CredentialIdentityScope { self.defaults = defaults self.fingerprintProvider = fingerprintProvider self.resolveUserRecordID = resolveUserRecordID + self.accountAvailabilityProvider = accountAvailabilityProvider self.retryDelay = retryDelay adoptCurrentIdentity() observeIdentityChanges() @@ -137,6 +142,7 @@ final class CredentialIdentityScope { self.defaults = defaults fingerprintProvider = { nil } resolveUserRecordID = { nil } + accountAvailabilityProvider = { .indeterminate } retryDelay = { _ in } fingerprint = nil state = fixedScope.map { State.resolved($0) } ?? .unresolved @@ -164,9 +170,13 @@ final class CredentialIdentityScope { } guard let current else { - // Signed out. Local scope is correct *and* isolating: items written - // here never collide with a signed-in account's keys. - state = .resolved(Self.localScope) + // A nil token does NOT establish sign-out — Foundation also returns + // nil when iCloud Drive is unavailable or switched off, and Yattee + // declares only the CloudKit service. Scoping to `local` here would + // collapse two different signed-in Apple accounts into one + // namespace, which is exactly the collision this class prevents. + // CloudKit's account status settles it during resolution. + state = .unresolved return } @@ -212,9 +222,35 @@ final class CredentialIdentityScope { // for B. let requested = fingerprint - while resolutionAttempts < Self.maxResolutionAttempts { + // Labelled: an unqualified `break` inside the switch below would leave + // only the switch and fall through to the user-record lookup, which + // must not run when the account status is undetermined. + attempts: while resolutionAttempts < Self.maxResolutionAttempts { resolutionAttempts += 1 + // With no ubiquity token, only CloudKit can distinguish "signed + // out" (where `local` is correct) from "signed in but iCloud Drive + // is off" (where `local` would be a cross-account collision). + if requested == nil { + switch await accountAvailabilityProvider() { + case .noAccount: + guard fingerprintProvider() == nil, fingerprint == nil else { return } + state = .resolved(Self.localScope) + LoggingService.shared.info( + "No Apple account — credentials scoped locally", + category: .keychain + ) + return + case .available: + break // fall through to the user-record lookup below + case .indeterminate: + guard resolutionAttempts < Self.maxResolutionAttempts else { break attempts } + await retryDelay(resolutionAttempts) + guard fingerprint == requested else { return } + continue attempts + } + } + if let recordID = await resolveUserRecordID() { guard fingerprintProvider() == requested, fingerprint == requested else { LoggingService.shared.info( @@ -224,6 +260,9 @@ final class CredentialIdentityScope { return } + // Only cacheable against a token. Without one there is no key + // to cache under that could be trusted to mean this account, + // so this lookup repeats each launch. if let requested { cache(recordID, for: requested) } @@ -232,12 +271,12 @@ final class CredentialIdentityScope { LoggingService.shared.info( "Resolved credential identity scope", category: .keychain, - details: "fingerprint=\(requested ?? "none"), attempt=\(resolutionAttempts)" + details: "fingerprint=\(requested ?? "none (no ubiquity token)"), attempt=\(resolutionAttempts)" ) return } - guard resolutionAttempts < Self.maxResolutionAttempts else { break } + guard resolutionAttempts < Self.maxResolutionAttempts else { break attempts } await retryDelay(resolutionAttempts) guard fingerprint == requested else { return } diff --git a/YatteeTests/AccountCredentialsTests.swift b/YatteeTests/AccountCredentialsTests.swift index c2121236..742624bd 100644 --- a/YatteeTests/AccountCredentialsTests.swift +++ b/YatteeTests/AccountCredentialsTests.swift @@ -321,6 +321,42 @@ struct LegacyCredentialAdoptionTests { .delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID)) } + @Test("Adoption in one backend does not suppress the other's offer") + func markersAreNamespacedByService() { + // Both credential managers share one marker store, and an instance UUID + // can outlive a change of backend. + let invidiousService = makeService() + let pipedService = makeService() + let markers = makeMarkers() + let instanceID = UUID() + let account = Account.migrated(from: instanceID) + + seedLegacyItem(service: invidiousService, instanceID: instanceID, value: "invidious-sid") + seedLegacyItem(service: pipedService, instanceID: instanceID, value: "piped-token") + + let invidious = makeCredentials(service: invidiousService, scope: "identity-A", markers: markers) + let piped = makeCredentials(service: pipedService, scope: "identity-A", markers: markers) + + #expect(invidious.adoptLegacyCredential(for: account) == true) + + // Piped's own legacy item is untouched and still offered. + #expect(piped.hasAdoptableLegacyCredential(for: account) == true) + #expect(piped.adoptLegacyCredential(for: account) == true) + #expect(piped.credential(for: account) == "piped-token") + #expect(invidious.credential(for: account) == "invidious-sid") + + // Each deletion consults its own marker and its own secret. + invidious.deleteLegacyCredentialIfOwned(for: account) + #expect(legacyValue(service: invidiousService, instanceID: instanceID) == nil) + #expect(legacyValue(service: pipedService, instanceID: instanceID) == "piped-token") + + piped.deleteLegacyCredentialIfOwned(for: account) + #expect(legacyValue(service: pipedService, instanceID: instanceID) == nil) + + invidious.deleteCredential(for: account) + piped.deleteCredential(for: account) + } + @Test("Adoption is refused while the identity scope is unresolved") func adoptionRefusedWhenUnresolved() { let service = makeService() @@ -355,6 +391,7 @@ struct CredentialIdentityScopeTests { var fingerprint: String? = "identity-A" var resultsByFingerprint: [String: String] = [:] var failuresRemaining = 0 + var availability: CloudKitIdentity.AccountAvailability = .available var onResolveStart: (() -> Void)? var switchTask: Task? @@ -379,6 +416,7 @@ struct CredentialIdentityScopeTests { defaults: makeDefaults(), fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { await harness.resolve() }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) @@ -397,6 +435,7 @@ struct CredentialIdentityScopeTests { defaults: makeDefaults(), fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { await harness.resolve() }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) @@ -418,6 +457,7 @@ struct CredentialIdentityScopeTests { defaults: makeDefaults(), fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { await harness.resolve() }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) @@ -440,21 +480,69 @@ struct CredentialIdentityScopeTests { #expect(scope.current == "_recordB") } - @Test("Signing out scopes to local without a lookup") - func signedOutUsesLocalScope() { + @Test("A missing ubiquity token alone never scopes to local") + func missingTokenDoesNotImplySignedOut() async { + // iCloud Drive off while still signed in: Foundation returns no token, + // but the account is real. Scoping to local here would put two + // different Apple accounts in one namespace. let harness = Harness() harness.fingerprint = nil + harness.availability = .available + harness.resultsByFingerprint = [:] + + let scope = CredentialIdentityScope( + defaults: makeDefaults(), + fingerprintProvider: { harness.fingerprint }, + resolveUserRecordID: { "_recordA" }, + accountAvailabilityProvider: { harness.availability }, + retryDelay: { _ in } + ) + + #expect(scope.current == nil) + await scope.resolve() + #expect(scope.current == "_recordA") + #expect(scope.current != CredentialIdentityScope.localScope) + } + + @Test("Signing out scopes to local, confirmed by CloudKit") + func signedOutUsesLocalScope() async { + let harness = Harness() + harness.fingerprint = nil + harness.availability = .noAccount let scope = CredentialIdentityScope( defaults: makeDefaults(), fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { await harness.resolve() }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) + // Not assumed up front — only CloudKit can tell "signed out" from + // "token unavailable". + #expect(scope.current == nil) + await scope.resolve() #expect(scope.current == CredentialIdentityScope.localScope) } + @Test("An undeterminable account status leaves the scope unresolved") + func indeterminateStatusStaysUnresolved() async { + let harness = Harness() + harness.fingerprint = nil + harness.availability = .indeterminate + + let scope = CredentialIdentityScope( + defaults: makeDefaults(), + fingerprintProvider: { harness.fingerprint }, + resolveUserRecordID: { "_shouldNotBeUsed" }, + accountAvailabilityProvider: { harness.availability }, + retryDelay: { _ in } + ) + + await scope.resolve() + #expect(scope.current == nil) + } + @Test("A known Apple account resolves synchronously on relaunch") func cachedScopeResolvesSynchronously() async { let defaults = makeDefaults() @@ -465,6 +553,7 @@ struct CredentialIdentityScopeTests { defaults: defaults, fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { await harness.resolve() }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) await first.resolve() @@ -475,6 +564,7 @@ struct CredentialIdentityScopeTests { defaults: defaults, fingerprintProvider: { harness.fingerprint }, resolveUserRecordID: { nil }, + accountAvailabilityProvider: { harness.availability }, retryDelay: { _ in } ) #expect(second.current == "_recordA")