Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/auth-refresh-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Select pinned Xcode
run: ./scripts/select-ci-xcode.sh
- name: Exercise shared refresh transport and deadlines without credentials
run: swift test --package-path vendor/stack-auth-swift-sdk-prerelease --filter 'RefreshLifecycleTests|RefreshDeadlineTests|RefreshTransportTimingTests|RateLimitRetryTests|TokenRefreshAlgorithmTests/[a-zA-Z]*Fresh'
run: swift test --package-path vendor/stack-auth-swift-sdk-prerelease --filter 'RefreshLifecycleTests|RefreshRejectionTests|RefreshDeadlineTests|RefreshTransportTimingTests|RateLimitRetryTests|TokenRefreshAlgorithmTests/[a-zA-Z]*Fresh'
- name: Exercise coordinator cancellation, sessions, and recovery
run: swift test --package-path Packages/Shared/CmuxAuthRuntime --filter 'AuthCoordinator|AuthError|HostBrowserSignInFlow|StackRateLimitBoundaryTests|StackTokenRefreshBoundaryTests'
- name: Verify retired session watcher completion over loopback
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ extension AuthCoordinator {
preserveCachedSessionAfterValidationFailure()
return
}
// The pre-read probe can race with the Keychain becoming unreadable.
// Ask the store whether the actual token reads were definitive before
// treating an empty result as a signed-out session.
let tokenStorageReadsWereAvailable = await isTokenStorageAvailable()
guard generation == sessionGeneration else { return }

#if DEBUG
Expand Down Expand Up @@ -181,16 +185,7 @@ extension AuthCoordinator {
}
#endif

if hasStoredTokens {
sessionCache.setHasTokens(true)
if currentUser == nil, let cachedUser {
currentUser = cachedUser
}
await validateCachedSession(generation: generation, storeWriteHighWater: storeWriteHighWater)
return
}

if !tokenStorageWasAvailable {
if !tokenStorageWasAvailable || !tokenStorageReadsWereAvailable {
authLog.info("Token storage unavailable during session restore; preserving cached session")
preserveCachedSessionAfterValidationFailure()
if retryOnStorageAvailable {
Expand All @@ -208,6 +203,15 @@ extension AuthCoordinator {
return
}

if hasStoredTokens {
sessionCache.setHasTokens(true)
if currentUser == nil, let cachedUser {
currentUser = cachedUser
}
await validateCachedSession(generation: generation, storeWriteHighWater: storeWriteHighWater)
return
}

if launch.includesDevAuth, let creds = debugCredentials {
authLog.debug("Auto-login with persisted debug credentials")
await performAutoLogin(creds, generation: generation, storeWriteHighWater: storeWriteHighWater)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public actor FallbackTokenStore: StackAuthTokenStoreProtocol {
private let file: FileStackTokenStore
private let log = AuthDebugLog()
private var keychainWorks: Bool = true
private var tokenReadOccurred = false

/// Creates the composite store.
/// - Parameters:
Expand All @@ -22,6 +23,8 @@ public actor FallbackTokenStore: StackAuthTokenStoreProtocol {
}

public func getStoredAccessToken() async -> String? {
if !tokenReadOccurred { await keychain.resetLastReadStatus() }
tokenReadOccurred = true
if keychainWorks, let value = await keychain.getStoredAccessToken() {
return value
}
Expand All @@ -34,6 +37,8 @@ public actor FallbackTokenStore: StackAuthTokenStoreProtocol {
}

public func getStoredRefreshToken() async -> String? {
if !tokenReadOccurred { await keychain.resetLastReadStatus() }
tokenReadOccurred = true
if keychainWorks, let value = await keychain.getStoredRefreshToken() {
return value
}
Expand All @@ -45,6 +50,25 @@ public actor FallbackTokenStore: StackAuthTokenStoreProtocol {
return fallbackValue
}

/// Whether an empty read means no session is stored. Once this session
/// routes to the file store, the file is authoritative and always readable;
/// otherwise this is the Keychain's answer
/// (``KeychainStackTokenStore/isReadable()``). Pass it to
/// ``AuthCoordinator``'s `isTokenStorageAvailable`.
public func isTokenStorageAvailable() async -> Bool {
guard keychainWorks else { return true }
if tokenReadOccurred {
tokenReadOccurred = false
let readWasDefinitive = await keychain.wasLastReadDefinitive()
if !readWasDefinitive {
await keychain.resetLastReadStatus()
return false
}
return true
}
return await keychain.isReadable()
}

public func setTokens(accessToken: String?, refreshToken: String?) async {
if keychainWorks {
let ok = await keychain.trySetTokens(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {

private var cachedAccessToken: String?
private var cachedRefreshToken: String?
private var lastReadWasDefinitive = true

/// Creates a Keychain store writing under one exact signed access group.
///
Expand Down Expand Up @@ -70,6 +71,59 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {
)
}

/// Whether the Keychain can currently answer whether a session is stored.
///
/// A read that fails comes back as a nil token, which is indistinguishable
/// from a signed-out store. Session restore asks this first so a Keychain
/// that is briefly unreadable (locked, or busy right after the app was
/// replaced by an update) keeps the session instead of signing the user
/// out. Only temporary failures count: a status that never clears (a
/// missing entitlement, a corrupt item, a failed access check) would keep
/// a session with no usable token forever, so it reads as an empty store
/// and ``FallbackTokenStore`` consults the file fallback. A legacy account
/// can fail independently of the current account.
public func isReadable() async -> Bool {
if cachedRefreshToken != nil { return true }
lastReadWasDefinitive = true
#if canImport(Security)
let currentStatus = keychainProbeStatus(account: Self.refreshTokenAccount)
guard Self.isDefinitiveReadStatus(currentStatus) else { return false }
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
guard currentStatus == errSecItemNotFound,
accessGroup != nil,
let legacyProjectID else {
return true
}
return Self.isDefinitiveReadStatus(keychainProbeStatus(
account: "stack-auth-refresh-\(legacyProjectID)",
legacy: true
))
#else
return true
#endif
}

/// Whether the most recent token read returned a definitive Keychain status.
/// This preserves a temporary read failure across the availability check
/// that follows session restore's token reads.
public func wasLastReadDefinitive() -> Bool {
lastReadWasDefinitive
}

/// Forget the consumed token-read status before retrying availability.
public func resetLastReadStatus() {
lastReadWasDefinitive = true
}

/// Whether a Keychain read status is final. Only a temporary failure (the
/// Keychain is locked or busy) is not; it clears without user action.
static func isDefinitiveReadStatus(_ status: Int32) -> Bool {
#if canImport(Security)
return status != errSecInteractionNotAllowed && status != errSecNotAvailable
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
#else
return true
#endif
}

public func setTokens(accessToken: String?, refreshToken: String?) async {
_ = await trySetTokens(accessToken: accessToken, refreshToken: refreshToken)
}
Expand Down Expand Up @@ -210,6 +264,7 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {
query[kSecMatchLimit as String] = kSecMatchLimitOne
var result: CFTypeRef?
let status = SecItemCopyMatching(query as CFDictionary, &result)
lastReadWasDefinitive = lastReadWasDefinitive && Self.isDefinitiveReadStatus(status)
guard status == errSecSuccess, let data = result as? Data else {
if status != errSecItemNotFound {
log.log("keychain READ status=\(status) account=\(account)")
Expand All @@ -219,6 +274,20 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {
return String(data: data, encoding: .utf8)
}

private func keychainProbeStatus(account: String, legacy: Bool = false) -> Int32 {
var query = legacy ? legacyBaseQuery(account: account) : baseQuery(account: account)
// Ask for the data, like a real read: a protected item's data can be
// unavailable while its attributes are not.
query[kSecReturnData as String] = true
query[kSecMatchLimit as String] = kSecMatchLimitOne
var result: CFTypeRef?
let status = SecItemCopyMatching(query as CFDictionary, &result)
if !Self.isDefinitiveReadStatus(status) {
log.log("keychain PROBE status=\(status) account=\(account)")
}
return status
}

private func keychainWrite(_ value: String, account: String) -> Bool {
guard let data = value.data(using: .utf8) else { return false }
let lookup = baseQuery(account: account)
Expand Down Expand Up @@ -251,6 +320,7 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {
query[kSecMatchLimit as String] = kSecMatchLimitOne
var result: CFTypeRef?
let status = SecItemCopyMatching(query as CFDictionary, &result)
lastReadWasDefinitive = lastReadWasDefinitive && Self.isDefinitiveReadStatus(status)
guard status == errSecSuccess, let data = result as? Data else {
if status != errSecItemNotFound {
log.log("keychain legacy READ status=\(status) account=\(account)")
Expand All @@ -267,6 +337,7 @@ public actor KeychainStackTokenStore: StackAuthTokenStoreProtocol {
private func readOrAdoptLegacyToken(account: String, legacyAccount: String?) -> String? { nil }
private func deleteLegacyTokens() {}
private func keychainRead(account: String) -> String? { nil }
private func keychainProbeStatus(account: String, legacy: Bool = false) -> Int32 { 0 }
private func keychainWrite(_ value: String, account: String) -> Bool { false }
private func keychainDelete(account: String) {}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,22 @@ actor LockedStorageAuthClient: AuthClient {
actor TokenStorageAvailabilityProbe {
private var available: Bool
private var unavailableReadsRemaining: Int
private var scriptedResults: [Bool]

init(available: Bool, unavailableFirstReads: Int = 0) {
init(available: Bool, unavailableFirstReads: Int = 0, scriptedResults: [Bool] = []) {
self.available = available
self.unavailableReadsRemaining = unavailableFirstReads
self.scriptedResults = scriptedResults
}

func setAvailable(_ available: Bool) {
self.available = available
}

func isAvailable() -> Bool {
if !scriptedResults.isEmpty {
return scriptedResults.removeFirst()
}
if unavailableReadsRemaining > 0 {
unavailableReadsRemaining -= 1
return false
Expand Down Expand Up @@ -209,6 +214,31 @@ actor TokenStorageAvailabilityProbe {
#expect(await client.clearLocalSessionCallCount == 0)
}

@Test func readBecomingUnavailableAfterSuccessfulProbePreservesCachedSession() async throws {
let cachedUser = CMUXAuthUser(id: "cached", primaryEmail: "cached@example.com", displayName: "Cached")
let client = LockedStorageAuthClient(
access: "access",
refresh: "refresh",
user: nil,
locked: false,
locksFirstReads: 2
)
let availability = TokenStorageAvailabilityProbe(available: true, scriptedResults: [true, false, false])
let (coordinator, store) = try makeCoordinator(
client: client,
cachedUser: cachedUser,
availability: availability
)

coordinator.start()
await coordinator.awaitBootstrapped()

#expect(coordinator.isAuthenticated == true)
#expect(coordinator.currentUser == cachedUser)
#expect(store.bool(forKey: "has_tokens") == true)
#expect(await client.clearLocalSessionCallCount == 0)
}

@Test func availableEmptyStorageClearsSeededCache() async throws {
let cachedUser = CMUXAuthUser(id: "cached", primaryEmail: "cached@example.com", displayName: "Cached")
let client = LockedStorageAuthClient(access: nil, refresh: nil, user: nil, locked: false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,36 @@ struct KeychainStackTokenStoreTests {
)
#expect(result as? Data == unrelatedToken)
}

/// https://github.com/manaflow-ai/cmux/issues/25628: a locked or busy
/// Keychain is not an empty store, so session restore preserves the cached
/// session until it can answer. A failure that never clears (a missing
/// entitlement, a corrupt item, a failed access check) is final: treating
/// it as unavailable would keep a session with no usable token forever.
@Test func onlyTemporaryReadFailuresCountAsUnreadable() {
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecSuccess))
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecItemNotFound))
#expect(!KeychainStackTokenStore.isDefinitiveReadStatus(errSecInteractionNotAllowed))
#expect(!KeychainStackTokenStore.isDefinitiveReadStatus(errSecNotAvailable))
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecMissingEntitlement))
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecDecode))
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecAuthFailed))
#expect(KeychainStackTokenStore.isDefinitiveReadStatus(errSecParam))
}

@Test func fileFallbackStoreIsAlwaysReadable() async {
let directory = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-fallback-\(UUID().uuidString)", isDirectory: true)
defer { try? FileManager.default.removeItem(at: directory) }
let file = FileStackTokenStore(directory: directory)
await file.setTokens(accessToken: "access", refreshToken: "refresh")
let store = FallbackTokenStore(
primary: KeychainStackTokenStore(service: "cmux-test-empty-\(UUID().uuidString)"),
fallback: file
)
// The Keychain has nothing, so the read routes to the file store.
#expect(await store.getStoredRefreshToken() == "refresh")
#expect(await store.isTokenStorageAvailable())
}
#endif
}
3 changes: 3 additions & 0 deletions Sources/Auth/MacAuthComposition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ struct MacAuthComposition {
anchor: anchor,
config: config,
launch: launch,
// A Keychain read that fails (rather than finding nothing) must not
// read as signed out, e.g. on the first launch after an update.
isTokenStorageAvailable: { await tokenStore.isTokenStorageAvailable() },
onSessionWillTransition: {
browserAppSessionSignInRelay.sessionWillTransition()
},
Expand Down
Loading
Loading