diff --git a/apps/ios/ADE.xcodeproj/project.pbxproj b/apps/ios/ADE.xcodeproj/project.pbxproj index ff61faa73..b071d4341 100644 --- a/apps/ios/ADE.xcodeproj/project.pbxproj +++ b/apps/ios/ADE.xcodeproj/project.pbxproj @@ -75,6 +75,7 @@ 28CFE3D489EA1B208D231519 /* SyncService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 66B5024B0A05F3D9754101F1 /* SyncService.swift */; }; B70000000000000000000002 /* SyncRecoveryPolicy.swift in Sources */ = {isa = PBXBuildFile; fileRef = B70000000000000000000001 /* SyncRecoveryPolicy.swift */; }; B70000000000000000000004 /* SyncRecoveryPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */; }; + B7000000000000000000002F /* PairedHostCredentialStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7000000000000000000001F /* PairedHostCredentialStateTests.swift */; }; B70000000000000000000099 /* SyncAccountConnectRecoveryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */; }; B90000000000000000000002 /* SyncTransportSelectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B90000000000000000000001 /* SyncTransportSelectionTests.swift */; }; B80000000000000000000002 /* SyncConnectionRace.swift in Sources */ = {isa = PBXBuildFile; fileRef = B80000000000000000000001 /* SyncConnectionRace.swift */; }; @@ -271,6 +272,7 @@ F2A1C9D8456E7B3C1D2E4F90 /* FilesCodeSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6F9B21C0E4A6D8F1B3C5A77 /* FilesCodeSupport.swift */; }; FBEEF09EFB4911FEAC6A7E87 /* RemoteModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = 483C5F1818BAE74B19B84617 /* RemoteModels.swift */; }; C5B000000000000000000001 /* MobileTrustResetPolicy.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5A000000000000000000001 /* MobileTrustResetPolicy.swift */; }; + C5B00000000000000000001F /* PairedHostCredentialState.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5A00000000000000000001F /* PairedHostCredentialState.swift */; }; C5B000000000000000000002 /* SSHBootstrapModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5A000000000000000000002 /* SSHBootstrapModels.swift */; }; C5B000000000000000000003 /* SSHPrivateKeyParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5A000000000000000000003 /* SSHPrivateKeyParser.swift */; }; C5B000000000000000000004 /* SSHGeneratedKey.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5A000000000000000000004 /* SSHGeneratedKey.swift */; }; @@ -477,6 +479,7 @@ 66B5024B0A05F3D9754101F1 /* SyncService.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncService.swift; path = ADE/Services/SyncService.swift; sourceTree = ""; }; B70000000000000000000001 /* SyncRecoveryPolicy.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncRecoveryPolicy.swift; path = ADE/Services/SyncRecoveryPolicy.swift; sourceTree = ""; }; B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncRecoveryPolicyTests.swift; path = ADETests/SyncRecoveryPolicyTests.swift; sourceTree = ""; }; + B7000000000000000000001F /* PairedHostCredentialStateTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PairedHostCredentialStateTests.swift; path = ADETests/PairedHostCredentialStateTests.swift; sourceTree = ""; }; B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncAccountConnectRecoveryTests.swift; path = ADETests/SyncAccountConnectRecoveryTests.swift; sourceTree = ""; }; B90000000000000000000001 /* SyncTransportSelectionTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncTransportSelectionTests.swift; path = ADETests/SyncTransportSelectionTests.swift; sourceTree = ""; }; B80000000000000000000001 /* SyncConnectionRace.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncConnectionRace.swift; path = ADE/Services/SyncConnectionRace.swift; sourceTree = ""; }; @@ -568,6 +571,7 @@ D6F9B21C0E4A6D8F1B3C5A77 /* FilesCodeSupport.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = FilesCodeSupport.swift; path = ADE/Views/Components/FilesCodeSupport.swift; sourceTree = ""; }; E3A5721EB84321D201716BC3 /* ADETests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = ADETests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; C5A000000000000000000001 /* MobileTrustResetPolicy.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = MobileTrustResetPolicy.swift; path = ADE/Services/MobileTrustResetPolicy.swift; sourceTree = ""; }; + C5A00000000000000000001F /* PairedHostCredentialState.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PairedHostCredentialState.swift; path = ADE/Services/PairedHostCredentialState.swift; sourceTree = ""; }; C5A000000000000000000002 /* SSHBootstrapModels.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHBootstrapModels.swift; path = ADE/Services/SSHBootstrapModels.swift; sourceTree = ""; }; C5A000000000000000000003 /* SSHPrivateKeyParser.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHPrivateKeyParser.swift; path = ADE/Services/SSHPrivateKeyParser.swift; sourceTree = ""; }; C5A000000000000000000004 /* SSHGeneratedKey.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHGeneratedKey.swift; path = ADE/Services/SSHGeneratedKey.swift; sourceTree = ""; }; @@ -928,6 +932,7 @@ AD0000000000000000000A05 /* AccountEmailAuthFlow.swift */, AD0000000000000000000A02 /* AccountDirectory.swift */, C5A000000000000000000001 /* MobileTrustResetPolicy.swift */, + C5A00000000000000000001F /* PairedHostCredentialState.swift */, C5A000000000000000000002 /* SSHBootstrapModels.swift */, C5A000000000000000000003 /* SSHPrivateKeyParser.swift */, C5A000000000000000000004 /* SSHGeneratedKey.swift */, @@ -1088,6 +1093,7 @@ 14C0DF7FEB4C2EB854BAC888 /* ADETests.swift */, AD0000000000000000000A06 /* AccountEmailAuthFlowTests.swift */, B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */, + B7000000000000000000001F /* PairedHostCredentialStateTests.swift */, B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */, B90000000000000000000001 /* SyncTransportSelectionTests.swift */, AF00000000000000000000A4 /* PairingAndDpopTests.swift */, @@ -1411,6 +1417,7 @@ 0375D32BA5870617FA1758C6 /* KeychainService.swift in Sources */, 28CFE3D489EA1B208D231519 /* SyncService.swift in Sources */, C5B000000000000000000001 /* MobileTrustResetPolicy.swift in Sources */, + C5B00000000000000000001F /* PairedHostCredentialState.swift in Sources */, C5B000000000000000000002 /* SSHBootstrapModels.swift in Sources */, C5B000000000000000000003 /* SSHPrivateKeyParser.swift in Sources */, C5B000000000000000000004 /* SSHGeneratedKey.swift in Sources */, @@ -1600,6 +1607,7 @@ 7B70BE6839672E5D2D006B28 /* ADETests.swift in Sources */, AD0000000000000000000B06 /* AccountEmailAuthFlowTests.swift in Sources */, B70000000000000000000004 /* SyncRecoveryPolicyTests.swift in Sources */, + B7000000000000000000002F /* PairedHostCredentialStateTests.swift in Sources */, B70000000000000000000099 /* SyncAccountConnectRecoveryTests.swift in Sources */, B90000000000000000000002 /* SyncTransportSelectionTests.swift in Sources */, AF00000000000000000000C4 /* PairingAndDpopTests.swift in Sources */, diff --git a/apps/ios/ADE/App/ContentView.swift b/apps/ios/ADE/App/ContentView.swift index 5a70164cc..b1e5ddf7d 100644 --- a/apps/ios/ADE/App/ContentView.swift +++ b/apps/ios/ADE/App/ContentView.swift @@ -145,6 +145,7 @@ struct ContentView: View { accountLoading: accountService.phase == .loading, accountSignedIn: accountService.isSignedIn, hasPairedHost: syncService.hasPairedHost, + credentialUnreadable: syncService.pairedHostCredentialState == .credentialUnreadable, onContinue: { mobileLaunchAccess.grantAccess() if syncService.hasPairedHost, syncService.connectionState.isHostUnreachable { diff --git a/apps/ios/ADE/Services/PairedHostCredentialState.swift b/apps/ios/ADE/Services/PairedHostCredentialState.swift new file mode 100644 index 000000000..74beb37fa --- /dev/null +++ b/apps/ios/ADE/Services/PairedHostCredentialState.swift @@ -0,0 +1,40 @@ +import Foundation + +/// Why a saved pairing is or is not usable right now. +/// +/// `hasPairedHost` used to answer this as one boolean, which made "this phone +/// has never been paired" and "this phone is paired but cannot read its +/// credential" indistinguishable. They need different words: the first is an +/// instruction, the second is a fault, and showing the first when the second is +/// true tells the user to do something they have already done. +enum PairedHostCredentialState: Equatable { + /// No saved pairing at all. Pairing is the next step. + case notPaired + /// Saved pairing with a readable credential — the phone can connect. + case ready + /// A pairing was saved, but its credential cannot be read back. The Keychain + /// entry is missing, was cleared out from under the app, or the read failed + /// (`errSecMissingEntitlement`, a locked or reset Keychain). The pairing + /// cannot be used and must be redone. + case credentialUnreadable + + /// Whether the phone can act on the saved pairing. This is the exact + /// predicate `hasPairedHost` has always answered. + var isUsable: Bool { self == .ready } +} + +/// Classifies a saved pairing, given a credential lookup. +/// +/// Pure and lookup-injected so the unreadable-credential branch can be tested +/// directly. That branch is otherwise only reachable with a real Keychain +/// fault, which is exactly the case that has shipped broken before. +func syncPairedHostCredentialState( + profile: HostConnectionProfile?, + authKind: String? = nil, + credentialLookup: (HostConnectionProfile) -> String? +) -> PairedHostCredentialState { + guard let profile else { return .notPaired } + let kind = authKind ?? profile.authKind + guard kind == "paired" else { return .notPaired } + return credentialLookup(profile) == nil ? .credentialUnreadable : .ready +} diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 2a89e0352..7e5fcfe46 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -18671,11 +18671,17 @@ final class SyncService: ObservableObject { /// True when this device is paired to a machine (active or last-saved /// profile). Push registration only runs while paired. - var hasPairedHost: Bool { - guard let profile = activeHostProfile ?? loadProfile(), profile.authKind == "paired" else { - return false + /// Whether a saved pairing exists and, separately, whether its credential can + /// still be read. A pairing whose credential is gone is not the same thing as + /// no pairing, and the difference is what the user is told. + var pairedHostCredentialState: PairedHostCredentialState { + syncPairedHostCredentialState(profile: activeHostProfile ?? loadProfile()) { profile in + tokenForProfile(profile) } - return tokenForProfile(profile) != nil + } + + var hasPairedHost: Bool { + pairedHostCredentialState.isUsable } var pairingDeviceId: String { deviceId } diff --git a/apps/ios/ADE/Views/Account/MobileAccessGateView.swift b/apps/ios/ADE/Views/Account/MobileAccessGateView.swift index 24bd4a81f..82025a8fc 100644 --- a/apps/ios/ADE/Views/Account/MobileAccessGateView.swift +++ b/apps/ios/ADE/Views/Account/MobileAccessGateView.swift @@ -5,6 +5,10 @@ struct MobileAccessGateView: View { let accountLoading: Bool let accountSignedIn: Bool let hasPairedHost: Bool + /// A saved pairing exists but its credential cannot be read back. Distinct + /// from "not paired": the user has already done the pairing step, so telling + /// them to do it again without saying why reads as the app ignoring them. + var credentialUnreadable: Bool = false let onContinue: () -> Void @EnvironmentObject private var syncService: SyncService @@ -79,15 +83,32 @@ struct MobileAccessGateView: View { presentedSheet = .pairMachine } } label: { - Text("Continue without an account") + Text(credentialUnreadable ? "Pair again" : "Continue without an account") .font(.subheadline.weight(.semibold)) - .foregroundStyle(ADEColor.textSecondary) + .foregroundStyle(credentialUnreadable ? ADEColor.accent : ADEColor.textSecondary) .frame(minHeight: 44) .frame(maxWidth: .infinity) .contentShape(Rectangle()) } .buttonStyle(.plain) + // A pairing whose credential cannot be read is a fault, not a + // missing step. Without this the button re-presents the same + // connect sheet the user already completed, with nothing to + // explain why it did not take. + if credentialUnreadable { + Text( + """ + This iPhone is paired with a computer, but the saved key for it \ + can no longer be read. Pair again to restore the connection. + """ + ) + .font(.footnote) + .foregroundStyle(ADEColor.textMuted) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + } + if syncService.connectionState == .connecting, let stage = syncService.accountConnectStageLabel { Text(stage) diff --git a/apps/ios/ADETests/PairedHostCredentialStateTests.swift b/apps/ios/ADETests/PairedHostCredentialStateTests.swift new file mode 100644 index 000000000..829f67cfb --- /dev/null +++ b/apps/ios/ADETests/PairedHostCredentialStateTests.swift @@ -0,0 +1,73 @@ +import XCTest +@testable import ADE + +/// A saved pairing whose credential cannot be read is a fault, not an +/// instruction to pair. Conflating the two is what made a real Keychain failure +/// present as "you have never paired" and silently re-open the connect sheet the +/// user had already completed. +final class PairedHostCredentialStateTests: XCTestCase { + private func pairedProfile(authKind: String = "paired") -> HostConnectionProfile { + HostConnectionProfile( + hostIdentity: "machine-identity", + hostName: "Arul's Mac Studio", + siteId: nil, + port: 8787, + authKind: authKind, + pairedDeviceId: "device-1", + lastRemoteDbVersion: 0, + lastHostDeviceId: "machine-identity", + lastSuccessfulAddress: "192.168.1.20", + savedAddressCandidates: ["192.168.1.20"], + discoveredLanAddresses: ["192.168.1.20"], + tailscaleAddress: nil, + savedRelayCandidates: nil + ) + } + + func testNoProfileIsNotPaired() { + XCTAssertEqual( + syncPairedHostCredentialState(profile: nil) { _ in "secret" }, + .notPaired + ) + } + + func testReadableCredentialIsReadyAndUsable() { + let state = syncPairedHostCredentialState(profile: pairedProfile()) { _ in "secret" } + XCTAssertEqual(state, .ready) + XCTAssertTrue(state.isUsable) + } + + /// The shipped failure: a persisted paired profile whose Keychain read fails. + /// On a real device this is a cleared or unreadable entry; the symptom that + /// exposed it was `errSecMissingEntitlement` (-34018), where every + /// `SecItemCopyMatching` returns nothing while the profile persists intact. + func testPersistedProfileWithFailingKeychainReadIsUnreadableNotUnpaired() { + var lookups = 0 + let state = syncPairedHostCredentialState(profile: pairedProfile()) { _ in + lookups += 1 + return nil // stands in for the -34018 read failure + } + + XCTAssertEqual(lookups, 1, "the credential must actually be consulted") + XCTAssertEqual( + state, .credentialUnreadable, + "a persisted pairing with an unreadable credential must not report as never-paired" + ) + XCTAssertNotEqual(state, .notPaired) + XCTAssertFalse(state.isUsable, "an unusable credential must still gate entry") + } + + /// `hasPairedHost` is defined as `isUsable`, so both failure modes keep + /// gating exactly as before — only the words the user sees change. + func testBothFailureModesRemainUnusable() { + let unreadable = syncPairedHostCredentialState(profile: pairedProfile()) { _ in nil } + let unpaired = syncPairedHostCredentialState(profile: nil) { _ in "secret" } + XCTAssertFalse(unreadable.isUsable) + XCTAssertFalse(unpaired.isUsable) + } + + func testAccountAuthKindIsNotTreatedAsAPairing() { + let state = syncPairedHostCredentialState(profile: pairedProfile(authKind: "account")) { _ in nil } + XCTAssertEqual(state, .notPaired, "only a paired profile can have an unreadable pairing credential") + } +}