Skip to content

Tell a paired phone when its saved key is unreadable #1066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arul28 merged 1 commit into from
Aug 10, 2026
Merged

Tell a paired phone when its saved key is unreadable #1066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/ios/ADE.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 */; };
Expand Down Expand Up @@ -477,6 +479,7 @@
66B5024B0A05F3D9754101F1 /* SyncService.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncService.swift; path = ADE/Services/SyncService.swift; sourceTree = "<group>"; };
B70000000000000000000001 /* SyncRecoveryPolicy.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncRecoveryPolicy.swift; path = ADE/Services/SyncRecoveryPolicy.swift; sourceTree = "<group>"; };
B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncRecoveryPolicyTests.swift; path = ADETests/SyncRecoveryPolicyTests.swift; sourceTree = "<group>"; };
B7000000000000000000001F /* PairedHostCredentialStateTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PairedHostCredentialStateTests.swift; path = ADETests/PairedHostCredentialStateTests.swift; sourceTree = "<group>"; };
B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncAccountConnectRecoveryTests.swift; path = ADETests/SyncAccountConnectRecoveryTests.swift; sourceTree = "<group>"; };
B90000000000000000000001 /* SyncTransportSelectionTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncTransportSelectionTests.swift; path = ADETests/SyncTransportSelectionTests.swift; sourceTree = "<group>"; };
B80000000000000000000001 /* SyncConnectionRace.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncConnectionRace.swift; path = ADE/Services/SyncConnectionRace.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -568,6 +571,7 @@
D6F9B21C0E4A6D8F1B3C5A77 /* FilesCodeSupport.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = FilesCodeSupport.swift; path = ADE/Views/Components/FilesCodeSupport.swift; sourceTree = "<group>"; };
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 = "<group>"; };
C5A00000000000000000001F /* PairedHostCredentialState.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PairedHostCredentialState.swift; path = ADE/Services/PairedHostCredentialState.swift; sourceTree = "<group>"; };
C5A000000000000000000002 /* SSHBootstrapModels.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHBootstrapModels.swift; path = ADE/Services/SSHBootstrapModels.swift; sourceTree = "<group>"; };
C5A000000000000000000003 /* SSHPrivateKeyParser.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHPrivateKeyParser.swift; path = ADE/Services/SSHPrivateKeyParser.swift; sourceTree = "<group>"; };
C5A000000000000000000004 /* SSHGeneratedKey.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SSHGeneratedKey.swift; path = ADE/Services/SSHGeneratedKey.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
1 change: 1 addition & 0 deletions apps/ios/ADE/App/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 40 additions & 0 deletions apps/ios/ADE/Services/PairedHostCredentialState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 10 additions & 4 deletions apps/ios/ADE/Services/SyncService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
25 changes: 23 additions & 2 deletions apps/ios/ADE/Views/Account/MobileAccessGateView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
73 changes: 73 additions & 0 deletions apps/ios/ADETests/PairedHostCredentialStateTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")
}
}
Loading