From 5c3c4cfdd8ff16193fdae5e4c822c2c9f9edfed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Modro=C3=B1o=20Vara?= Date: Wed, 3 Jun 2026 13:38:41 +0200 Subject: [PATCH] Share credentials via the data-protection keychain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The token was stored in the file-based login keychain with no access group. The app could read it, but the File Provider extension — a separate binary whose code signature changes when Sparkle replaces the bundle — could not, because file-based keychain access is bound to specific code rather than to an entitlement. The domain then showed as "signed out" in Finder and "Open in Finder" did nothing, even though the app itself was signed in and syncing. Store credentials in the data-protection keychain, where the app and extension share items through their common keychain-access-group entitlement; access is granted by entitlement, so it survives the app being re-signed. retrieveToken transparently migrates any token left in the legacy file-based keychain forward, so existing users don't have to sign in again. --- Sources/Networking/Auth/KeychainManager.swift | 119 ++++++++++++------ 1 file changed, 82 insertions(+), 37 deletions(-) diff --git a/Sources/Networking/Auth/KeychainManager.swift b/Sources/Networking/Auth/KeychainManager.swift index 3511510..a7cb95a 100644 --- a/Sources/Networking/Auth/KeychainManager.swift +++ b/Sources/Networking/Auth/KeychainManager.swift @@ -18,6 +18,13 @@ public final class KeychainManager: Sendable { private init() {} /// Store a token for a given account. + /// + /// Writes to the data-protection keychain so the token is shared with the + /// File Provider extension through their common `keychain-access-group` + /// entitlement. Access there is granted by entitlement rather than a + /// per-binary ACL, so it survives the app being re-signed (e.g. by a Sparkle + /// update) — unlike the file-based keychain, where a re-signed extension + /// silently loses read access and the domain shows as "signed out". public func storeToken(_ token: String, forAccount account: String) throws { let data = Data(token.utf8) @@ -26,17 +33,13 @@ public final class KeychainManager: Sendable { // the user would silently be logged out. SecItemUpdate is atomic // and also normalises any accessibility-class mismatch from older // builds (where the item may have been stored with WhenUnlocked). - let lookupQuery: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: account - ] let updateAttributes: [String: Any] = [ kSecValueData as String: data, kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock ] - let updateStatus = SecItemUpdate(lookupQuery as CFDictionary, updateAttributes as CFDictionary) + let updateStatus = SecItemUpdate(query(account: account) as CFDictionary, updateAttributes as CFDictionary) if updateStatus == errSecSuccess { + try? deleteLegacyToken(account: account) return } guard updateStatus == errSecItemNotFound else { @@ -45,69 +48,111 @@ public final class KeychainManager: Sendable { } // Item doesn't exist yet — add it. - let addQuery: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: account, - kSecValueData as String: data, - kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock - ] + var addQuery = query(account: account) + addQuery[kSecValueData as String] = data + addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock let addStatus = SecItemAdd(addQuery as CFDictionary, nil) guard addStatus == errSecSuccess else { logger.error("Keychain add failed: \(addStatus)") throw KeychainError.storeFailed(status: addStatus) } + try? deleteLegacyToken(account: account) } /// Retrieve a token for a given account. public func retrieveToken(forAccount account: String) throws -> String? { - let query: [String: Any] = [ + // Preferred: the shared data-protection keychain. + if let token = try copyToken(account: account, dataProtection: true) { + return token + } + + // Legacy: older builds stored the token in the file-based login keychain, + // which the File Provider extension cannot read after the app is + // re-signed. Migrate any such token into the shared keychain so the + // extension regains access without the user signing in again. + if let legacy = (try? copyToken(account: account, dataProtection: false)) ?? nil { + try? storeToken(legacy, forAccount: account) + return legacy + } + + return nil + } + + /// Delete a token for a given account. + public func deleteToken(forAccount account: String) throws { + let status = SecItemDelete(query(account: account) as CFDictionary) + guard status == errSecSuccess || status == errSecItemNotFound else { + logger.error("Failed to delete token: \(status)") + throw KeychainError.deleteFailed(status: status) + } + try? deleteLegacyToken(account: account) + } + + /// Delete all tokens for this app, in both the shared and legacy keychains. + public func deleteAllTokens() throws { + for dataProtection in [true, false] { + var deleteQuery: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service + ] + deleteQuery[kSecUseDataProtectionKeychain as String] = dataProtection + let status = SecItemDelete(deleteQuery as CFDictionary) + guard status == errSecSuccess || status == errSecItemNotFound else { + throw KeychainError.deleteFailed(status: status) + } + } + } + + // MARK: - Keychain query helpers + + /// Base query against the data-protection keychain. Items land in the app's + /// default access group — the sole `keychain-access-groups` entitlement entry + /// shared by the app and the extension — so no explicit access group is set. + private func query(account: String?) -> [String: Any] { + var q: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecUseDataProtectionKeychain as String: true + ] + if let account { + q[kSecAttrAccount as String] = account + } + return q + } + + private func copyToken(account: String, dataProtection: Bool) throws -> String? { + let copyQuery: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, kSecAttrAccount as String: account, + kSecUseDataProtectionKeychain as String: dataProtection, kSecReturnData as String: true, kSecMatchLimit as String: kSecMatchLimitOne ] var result: AnyObject? - let status = SecItemCopyMatching(query as CFDictionary, &result) + let status = SecItemCopyMatching(copyQuery as CFDictionary, &result) if status == errSecItemNotFound { return nil } - guard status == errSecSuccess, let data = result as? Data else { logger.error("Failed to retrieve token: \(status)") throw KeychainError.retrieveFailed(status: status) } - return String(data: data, encoding: .utf8) } - /// Delete a token for a given account. - public func deleteToken(forAccount account: String) throws { - let query: [String: Any] = [ + /// Remove the token from the legacy file-based keychain used by older builds. + private func deleteLegacyToken(account: String) throws { + let legacyQuery: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, - kSecAttrAccount as String: account - ] - - let status = SecItemDelete(query as CFDictionary) - guard status == errSecSuccess || status == errSecItemNotFound else { - logger.error("Failed to delete token: \(status)") - throw KeychainError.deleteFailed(status: status) - } - } - - /// Delete all tokens for this app. - public func deleteAllTokens() throws { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service + kSecAttrAccount as String: account, + kSecUseDataProtectionKeychain as String: false ] - - let status = SecItemDelete(query as CFDictionary) + let status = SecItemDelete(legacyQuery as CFDictionary) guard status == errSecSuccess || status == errSecItemNotFound else { throw KeychainError.deleteFailed(status: status) }