From e326db5d927f1be9c13501127f9b95e2674ca099 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 12:36:56 -0300 Subject: [PATCH 1/8] fix(auth)!: return nil from Keychain reads when the item is absent A missing item threw .itemNotFound instead of returning nil, contradicting AuthLocalStorage's documented contract. Every fresh launch with no session logged an error, and try? at the call sites collapsed genuine Keychain failures into the same nil as 'no session'. Delete is now idempotent. --- Sources/Auth/Internal/Keychain.swift | 36 ++++++++++++++++--- Tests/AuthTests/KeychainTests.swift | 52 ++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 Tests/AuthTests/KeychainTests.swift diff --git a/Sources/Auth/Internal/Keychain.swift b/Sources/Auth/Internal/Keychain.swift index 0c6bf07cf..5fd500d8a 100644 --- a/Sources/Auth/Internal/Keychain.swift +++ b/Sources/Auth/Internal/Keychain.swift @@ -20,10 +20,18 @@ } } - func data(forKey key: String) throws -> Data { - let query = getOneQuery(byKey: key) - var result: AnyObject? - try assertSuccess(forStatus: SecItemCopyMatching(query as CFDictionary, &result)) + /// Maps a `SecItemCopyMatching` status and its result into a value. + /// + /// - Returns: The stored bytes, or `nil` when the item does not exist. + /// - Throws: ``KeychainError`` for any status other than success or not-found. + static func mapReadStatus(_ status: OSStatus, result: AnyObject?) throws -> Data? { + if status == errSecItemNotFound { + return nil + } + + if status != errSecSuccess { + throw KeychainError(code: KeychainError.Code(rawValue: status)) + } guard let data = result as? Data else { let message = "Unable to cast the retrieved item to a Data value" @@ -33,6 +41,24 @@ return data } + /// Maps a `SecItemDelete` status, treating a missing item as success. + /// + /// - Throws: ``KeychainError`` for any status other than success or not-found. + static func mapDeleteStatus(_ status: OSStatus) throws { + if status == errSecItemNotFound || status == errSecSuccess { + return + } + + throw KeychainError(code: KeychainError.Code(rawValue: status)) + } + + func data(forKey key: String) throws -> Data? { + let query = getOneQuery(byKey: key) + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + return try Self.mapReadStatus(status, result: result) + } + func set(_ data: Data, forKey key: String) throws { let addItemQuery = setQuery(forKey: key, data: data) let addStatus = SecItemAdd(addItemQuery as CFDictionary, nil) @@ -50,7 +76,7 @@ func deleteItem(forKey key: String) throws { let query = baseQuery(withKey: key) - try assertSuccess(forStatus: SecItemDelete(query as CFDictionary)) + try Self.mapDeleteStatus(SecItemDelete(query as CFDictionary)) } private func baseQuery(withKey key: String? = nil, data: Data? = nil) -> [String: Any] { diff --git a/Tests/AuthTests/KeychainTests.swift b/Tests/AuthTests/KeychainTests.swift new file mode 100644 index 000000000..ee3f13240 --- /dev/null +++ b/Tests/AuthTests/KeychainTests.swift @@ -0,0 +1,52 @@ +#if !os(Windows) && !os(Linux) && !os(Android) + import Foundation + import Security + import Testing + + @testable import Auth + + @Suite + struct KeychainTests { + @Test + func mapReadStatusItemNotFoundReturnsNil() throws { + #expect(try Keychain.mapReadStatus(errSecItemNotFound, result: nil) == nil) + } + + @Test + func mapReadStatusSuccessReturnsData() throws { + let data = Data("hello".utf8) + #expect(try Keychain.mapReadStatus(errSecSuccess, result: data as AnyObject) == data) + } + + @Test + func mapReadStatusSuccessWithNonDataThrows() { + #expect(throws: KeychainError.self) { + try Keychain.mapReadStatus(errSecSuccess, result: "not data" as AnyObject) + } + } + + @Test + func mapReadStatusFailureThrowsMappedError() { + #expect(throws: KeychainError(code: .authFailed)) { + try Keychain.mapReadStatus(errSecAuthFailed, result: nil) + } + } + + @Test + func mapDeleteStatusItemNotFoundDoesNotThrow() throws { + try Keychain.mapDeleteStatus(errSecItemNotFound) + } + + @Test + func mapDeleteStatusSuccessDoesNotThrow() throws { + try Keychain.mapDeleteStatus(errSecSuccess) + } + + @Test + func mapDeleteStatusFailureThrows() { + #expect(throws: KeychainError(code: .authFailed)) { + try Keychain.mapDeleteStatus(errSecAuthFailed) + } + } + } +#endif From 95ea7ae7c8a737b9ebc48c573f20469ccb4cc1e8 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 12:41:49 -0300 Subject: [PATCH 2/8] feat(auth): add opt-in data-protection Keychain support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a useDataProtectionKeychain flag threaded through every query, and stops sending kSecAttrAccessible on macOS unless it is enabled — the file-based Keychain ignores that attribute, so today it is inert. --- Sources/Auth/Internal/Keychain.swift | 23 ++++++++++++-- Tests/AuthTests/KeychainTests.swift | 47 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/Sources/Auth/Internal/Keychain.swift b/Sources/Auth/Internal/Keychain.swift index 5fd500d8a..518245ed6 100644 --- a/Sources/Auth/Internal/Keychain.swift +++ b/Sources/Auth/Internal/Keychain.swift @@ -5,13 +5,16 @@ struct Keychain { let service: String? let accessGroup: String? + let useDataProtectionKeychain: Bool init( service: String?, - accessGroup: String? = nil + accessGroup: String? = nil, + useDataProtectionKeychain: Bool = false ) { self.service = service self.accessGroup = accessGroup + self.useDataProtectionKeychain = useDataProtectionKeychain } private func assertSuccess(forStatus status: OSStatus) throws { @@ -79,7 +82,7 @@ try Self.mapDeleteStatus(SecItemDelete(query as CFDictionary)) } - private func baseQuery(withKey key: String? = nil, data: Data? = nil) -> [String: Any] { + func baseQuery(withKey key: String? = nil, data: Data? = nil) -> [String: Any] { var query: [String: Any] = [:] query[kSecClass as String] = kSecClassGenericPassword @@ -95,6 +98,11 @@ if let accessGroup { query[kSecAttrAccessGroup as String] = accessGroup } + if useDataProtectionKeychain { + // Stored as a Swift Bool rather than kCFBooleanTrue so the value round-trips through + // `as? Bool` in tests. It bridges to a CFBoolean when the dictionary becomes a CFDictionary. + query[kSecUseDataProtectionKeychain as String] = true + } return query } @@ -109,7 +117,16 @@ func setQuery(forKey key: String, data: Data) -> [String: Any] { var query = baseQuery(withKey: key, data: data) - query[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + #if os(macOS) + // kSecAttrAccessible does not apply to the legacy file-based Keychain, which is what + // SecItem targets on macOS by default. Only send it once the data-protection Keychain + // is in use. https://developer.apple.com/documentation/security/ksecattraccessible + if useDataProtectionKeychain { + query[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + } + #else + query[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + #endif return query } diff --git a/Tests/AuthTests/KeychainTests.swift b/Tests/AuthTests/KeychainTests.swift index ee3f13240..90e328126 100644 --- a/Tests/AuthTests/KeychainTests.swift +++ b/Tests/AuthTests/KeychainTests.swift @@ -48,5 +48,52 @@ try Keychain.mapDeleteStatus(errSecAuthFailed) } } + + @Test + func baseQueryWithoutDataProtectionOmitsAttribute() { + let keychain = Keychain(service: "service") + let query = keychain.baseQuery(withKey: "key") + #expect(query[kSecUseDataProtectionKeychain as String] == nil) + } + + @Test + func baseQueryWithDataProtectionSetsAttribute() { + let keychain = Keychain(service: "service", useDataProtectionKeychain: true) + let query = keychain.baseQuery(withKey: "key") + #expect(query[kSecUseDataProtectionKeychain as String] as? Bool == true) + } + + @Test + func getOneQueryCarriesDataProtectionAttribute() { + let keychain = Keychain(service: "service", useDataProtectionKeychain: true) + let query = keychain.getOneQuery(byKey: "key") + #expect(query[kSecUseDataProtectionKeychain as String] as? Bool == true) + } + + @Test + func setQueryCarriesDataProtectionAttribute() { + let keychain = Keychain(service: "service", useDataProtectionKeychain: true) + let query = keychain.setQuery(forKey: "key", data: Data()) + #expect(query[kSecUseDataProtectionKeychain as String] as? Bool == true) + } + + @Test + func setQueryAccessibilityDefault() { + let keychain = Keychain(service: "service") + let query = keychain.setQuery(forKey: "key", data: Data()) + #if os(macOS) + // The file-based Keychain ignores kSecAttrAccessible, so we must not send it. + #expect(query[kSecAttrAccessible as String] == nil) + #else + #expect(query[kSecAttrAccessible as String] != nil) + #endif + } + + @Test + func setQueryWithDataProtectionSetsAccessibility() { + let keychain = Keychain(service: "service", useDataProtectionKeychain: true) + let query = keychain.setQuery(forKey: "key", data: Data()) + #expect(query[kSecAttrAccessible as String] != nil) + } } #endif From 04388bfda5b5a798f814e6cc0622f04701632353 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 12:46:18 -0300 Subject: [PATCH 3/8] feat(auth): add Keychain location resolution helpers --- .../Auth/Storage/KeychainLocalStorage.swift | 61 ++++++++++++ .../AuthTests/KeychainLocalStorageTests.swift | 96 +++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 Tests/AuthTests/KeychainLocalStorageTests.swift diff --git a/Sources/Auth/Storage/KeychainLocalStorage.swift b/Sources/Auth/Storage/KeychainLocalStorage.swift index 5ffecd954..92e20715b 100644 --- a/Sources/Auth/Storage/KeychainLocalStorage.swift +++ b/Sources/Auth/Storage/KeychainLocalStorage.swift @@ -1,6 +1,16 @@ #if !os(Windows) && !os(Linux) && !os(Android) public import Foundation + /// The Keychain service used by versions of the SDK before v3. + let legacyKeychainService = "supabase.gotrue.swift" + + /// Identifies a single Keychain location. + struct KeychainConfiguration: Equatable, Sendable { + var service: String? + var accessGroup: String? + var useDataProtectionKeychain: Bool + } + /// ``AuthLocalStorage`` implementation using Keychain. This is the default local storage used by the library. public struct KeychainLocalStorage: AuthLocalStorage { private let keychain: Keychain @@ -42,4 +52,55 @@ try keychain.deleteItem(forKey: key) } } + + extension KeychainLocalStorage { + /// Resolves the Keychain location used when the caller accepts the default service. + /// + /// Falls back to ``legacyKeychainService`` when there is no bundle identifier, which is the + /// case for command-line tools and some test bundles. + static func primaryConfiguration( + bundleIdentifier: String?, + accessGroup: String?, + useDataProtectionKeychain: Bool + ) -> KeychainConfiguration { + KeychainConfiguration( + service: bundleIdentifier ?? legacyKeychainService, + accessGroup: accessGroup, + useDataProtectionKeychain: useDataProtectionKeychain + ) + } + + /// The locations to probe, in order, when `primary` holds no value. + /// + /// Entries equal to `primary`, and duplicates, are removed. + static func legacyConfigurations( + primary: KeychainConfiguration + ) -> [KeychainConfiguration] { + var candidates: [KeychainConfiguration] = [ + KeychainConfiguration( + service: legacyKeychainService, + accessGroup: primary.accessGroup, + useDataProtectionKeychain: false + ) + ] + + if primary.useDataProtectionKeychain { + // Items do not move between the two macOS Keychain implementations, so the previously + // used file-based location has to be probed too. + candidates.append( + KeychainConfiguration( + service: primary.service, + accessGroup: primary.accessGroup, + useDataProtectionKeychain: false + ) + ) + } + + var result: [KeychainConfiguration] = [] + for candidate in candidates where candidate != primary && !result.contains(candidate) { + result.append(candidate) + } + return result + } + } #endif diff --git a/Tests/AuthTests/KeychainLocalStorageTests.swift b/Tests/AuthTests/KeychainLocalStorageTests.swift new file mode 100644 index 000000000..64a4e9ba9 --- /dev/null +++ b/Tests/AuthTests/KeychainLocalStorageTests.swift @@ -0,0 +1,96 @@ +#if !os(Windows) && !os(Linux) && !os(Android) + import Foundation + import Testing + + @testable import Auth + + @Suite + struct KeychainLocalStorageTests { + @Test + func primaryConfigurationUsesBundleIdentifier() { + let configuration = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: "com.example.app", + accessGroup: nil, + useDataProtectionKeychain: false + ) + #expect(configuration.service == "com.example.app") + } + + @Test + func primaryConfigurationFallsBackToLegacyService() { + let configuration = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: nil, + accessGroup: nil, + useDataProtectionKeychain: false + ) + #expect(configuration.service == "supabase.gotrue.swift") + } + + @Test + func legacyConfigurationsContainsLegacyService() { + let primary = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: "com.example.app", + accessGroup: nil, + useDataProtectionKeychain: false + ) + let legacy = KeychainLocalStorage.legacyConfigurations(primary: primary) + + #expect( + legacy == [ + KeychainConfiguration( + service: "supabase.gotrue.swift", + accessGroup: nil, + useDataProtectionKeychain: false + ) + ] + ) + } + + @Test + func legacyConfigurationsWithDataProtectionAlsoProbesFileBasedPrimary() { + let primary = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: "com.example.app", + accessGroup: "group", + useDataProtectionKeychain: true + ) + let legacy = KeychainLocalStorage.legacyConfigurations(primary: primary) + + #expect( + legacy == [ + KeychainConfiguration( + service: "supabase.gotrue.swift", + accessGroup: "group", + useDataProtectionKeychain: false + ), + KeychainConfiguration( + service: "com.example.app", + accessGroup: "group", + useDataProtectionKeychain: false + ), + ] + ) + } + + @Test + func legacyConfigurationsExcludesPrimary() { + // No bundle identifier means the primary already is the legacy location. + let primary = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: nil, + accessGroup: nil, + useDataProtectionKeychain: false + ) + #expect(KeychainLocalStorage.legacyConfigurations(primary: primary).isEmpty) + } + + @Test + func legacyConfigurationsDeduplicates() { + // No bundle identifier plus data protection would otherwise yield the same entry twice. + let primary = KeychainLocalStorage.primaryConfiguration( + bundleIdentifier: nil, + accessGroup: nil, + useDataProtectionKeychain: true + ) + #expect(KeychainLocalStorage.legacyConfigurations(primary: primary).count == 1) + } + } +#endif From b660c04d6572e5ef38ae778d8f88af7daddf6d89 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 12:51:42 -0300 Subject: [PATCH 4/8] fix(auth)!: default Keychain service to the host app bundle identifier Items were namespaced under a fixed 'supabase.gotrue.swift' service shared by every app embedding the SDK. The service now defaults to the host app's bundle identifier, matching KeychainAccess and SimpleKeychain. Existing sessions migrate on first read. --- Sources/Auth/Internal/Keychain.swift | 20 ++++ .../Auth/Storage/KeychainLocalStorage.swift | 78 ++++++++++-- .../AuthTests/KeychainLocalStorageTests.swift | 112 ++++++++++++++++++ 3 files changed, 202 insertions(+), 8 deletions(-) diff --git a/Sources/Auth/Internal/Keychain.swift b/Sources/Auth/Internal/Keychain.swift index 518245ed6..8d62e7b07 100644 --- a/Sources/Auth/Internal/Keychain.swift +++ b/Sources/Auth/Internal/Keychain.swift @@ -17,6 +17,14 @@ self.useDataProtectionKeychain = useDataProtectionKeychain } + init(_ configuration: KeychainConfiguration) { + self.init( + service: configuration.service, + accessGroup: configuration.accessGroup, + useDataProtectionKeychain: configuration.useDataProtectionKeychain + ) + } + private func assertSuccess(forStatus status: OSStatus) throws { if status != errSecSuccess { throw KeychainError(code: KeychainError.Code(rawValue: status)) @@ -271,4 +279,16 @@ lhs.code == rhs.code && lhs.localizedDescription == rhs.localizedDescription } } + + /// The Keychain operations ``KeychainLocalStorage`` depends on. + /// + /// Exists so the migration logic can be tested without reaching the real Keychain, which is + /// not available to an SPM test bundle. + protocol KeychainProtocol: Sendable { + func data(forKey key: String) throws -> Data? + func set(_ data: Data, forKey key: String) throws + func deleteItem(forKey key: String) throws + } + + extension Keychain: KeychainProtocol {} #endif diff --git a/Sources/Auth/Storage/KeychainLocalStorage.swift b/Sources/Auth/Storage/KeychainLocalStorage.swift index 92e20715b..f36e75e65 100644 --- a/Sources/Auth/Storage/KeychainLocalStorage.swift +++ b/Sources/Auth/Storage/KeychainLocalStorage.swift @@ -13,16 +13,58 @@ /// ``AuthLocalStorage`` implementation using Keychain. This is the default local storage used by the library. public struct KeychainLocalStorage: AuthLocalStorage { - private let keychain: Keychain + let keychain: any KeychainProtocol + let legacyKeychains: [any KeychainProtocol] - /// Creates a Keychain-backed storage instance. + /// Creates a Keychain-backed storage instance scoped to the host application. + /// + /// The Keychain service defaults to the host app's bundle identifier, so items are namespaced + /// per application. Sessions written by earlier SDK versions, which used a fixed + /// `"supabase.gotrue.swift"` service, migrate automatically on first read. + /// + /// - Parameters: + /// - accessGroup: An optional Keychain access group for sharing items between apps. + /// - useDataProtectionKeychain: Targets the macOS data-protection Keychain instead of the + /// legacy file-based one. This removes the macOS consent prompt, but requires the app to + /// be signed with entitlements authorised by a provisioning profile — otherwise Keychain + /// operations fail with `errSecMissingEntitlement` (-34018). Has no effect on platforms + /// other than macOS. Defaults to `false`. + public init(accessGroup: String? = nil, useDataProtectionKeychain: Bool = false) { + let primary = Self.primaryConfiguration( + bundleIdentifier: Bundle.main.bundleIdentifier, + accessGroup: accessGroup, + useDataProtectionKeychain: useDataProtectionKeychain + ) + + keychain = Keychain(primary) + legacyKeychains = Self.legacyConfigurations(primary: primary).map { Keychain($0) } + } + + /// Creates a Keychain-backed storage instance with an explicit service. + /// + /// No migration is performed: the given service is used exactly as provided. /// /// - Parameters: - /// - service: The Keychain service name used to namespace stored items. - /// Defaults to `"supabase.gotrue.swift"`. + /// - service: The Keychain service name used to namespace stored items. Pass `nil` to omit + /// the attribute entirely. /// - accessGroup: An optional Keychain access group for sharing items between apps. - public init(service: String? = "supabase.gotrue.swift", accessGroup: String? = nil) { - keychain = Keychain(service: service, accessGroup: accessGroup) + /// - useDataProtectionKeychain: See ``init(accessGroup:useDataProtectionKeychain:)``. + public init( + service: String?, + accessGroup: String? = nil, + useDataProtectionKeychain: Bool = false + ) { + keychain = Keychain( + service: service, + accessGroup: accessGroup, + useDataProtectionKeychain: useDataProtectionKeychain + ) + legacyKeychains = [] + } + + init(keychain: any KeychainProtocol, legacyKeychains: [any KeychainProtocol]) { + self.keychain = keychain + self.legacyKeychains = legacyKeychains } /// Stores `value` in the Keychain under `key`. @@ -37,19 +79,39 @@ /// Returns the data stored in the Keychain for `key`, or `nil` if not present. /// + /// If the item is absent but exists in a location used by an earlier SDK version, it is moved + /// to the current location and returned. + /// /// - Parameter key: The Keychain item key. /// - Returns: The stored bytes, or `nil` if the item does not exist. /// - Throws: A Keychain error if the read fails. public func retrieve(key: String) throws -> Data? { - try keychain.data(forKey: key) + if let data = try keychain.data(forKey: key) { + return data + } + + for legacy in legacyKeychains { + // A failing probe must not break a fresh install, so failures are ignored here. + guard let data = try? legacy.data(forKey: key) else { continue } + + try keychain.set(data, forKey: key) + try? legacy.deleteItem(forKey: key) + return data + } + + return nil } - /// Removes the Keychain item for `key`. + /// Removes the Keychain item for `key`, including any left in a legacy location. /// /// - Parameter key: The Keychain item key to delete. /// - Throws: A Keychain error if the delete fails. public func remove(key: String) throws { try keychain.deleteItem(forKey: key) + + for legacy in legacyKeychains { + try? legacy.deleteItem(forKey: key) + } } } diff --git a/Tests/AuthTests/KeychainLocalStorageTests.swift b/Tests/AuthTests/KeychainLocalStorageTests.swift index 64a4e9ba9..18b48e60b 100644 --- a/Tests/AuthTests/KeychainLocalStorageTests.swift +++ b/Tests/AuthTests/KeychainLocalStorageTests.swift @@ -1,4 +1,5 @@ #if !os(Windows) && !os(Linux) && !os(Android) + import ConcurrencyExtras import Foundation import Testing @@ -92,5 +93,116 @@ ) #expect(KeychainLocalStorage.legacyConfigurations(primary: primary).count == 1) } + + @Test + func explicitServiceDoesNotMigrate() { + let storage = KeychainLocalStorage(service: "custom") + #expect(storage.legacyKeychains.isEmpty) + } + + @Test + func retrieveMigratesFromLegacyLocation() throws { + let value = Data("session".utf8) + let primary = FakeKeychain() + let legacy = FakeKeychain(items: ["key": value]) + let storage = KeychainLocalStorage(keychain: primary, legacyKeychains: [legacy]) + + #expect(try storage.retrieve(key: "key") == value) + #expect(primary.items.value["key"] == value) + #expect(legacy.items.value["key"] == nil) + } + + @Test + func retrievePrefersPrimaryAndLeavesLegacyUntouched() throws { + let primaryValue = Data("new".utf8) + let legacyValue = Data("old".utf8) + let primary = FakeKeychain(items: ["key": primaryValue]) + let legacy = FakeKeychain(items: ["key": legacyValue]) + let storage = KeychainLocalStorage(keychain: primary, legacyKeychains: [legacy]) + + #expect(try storage.retrieve(key: "key") == primaryValue) + #expect(legacy.items.value["key"] == legacyValue) + } + + @Test + func retrieveProbesLegacyLocationsInOrder() throws { + let first = FakeKeychain(items: ["key": Data("first".utf8)]) + let second = FakeKeychain(items: ["key": Data("second".utf8)]) + let storage = KeychainLocalStorage( + keychain: FakeKeychain(), + legacyKeychains: [first, second] + ) + + #expect(try storage.retrieve(key: "key") == Data("first".utf8)) + #expect(second.items.value["key"] == Data("second".utf8)) + } + + @Test + func retrieveMissingEverywhereReturnsNil() throws { + let storage = KeychainLocalStorage( + keychain: FakeKeychain(), + legacyKeychains: [FakeKeychain()] + ) + #expect(try storage.retrieve(key: "key") == nil) + } + + @Test + func retrieveToleratesFailingLegacyProbe() throws { + struct ProbeFailure: Error {} + let storage = KeychainLocalStorage( + keychain: FakeKeychain(), + legacyKeychains: [FakeKeychain(readError: ProbeFailure())] + ) + #expect(try storage.retrieve(key: "key") == nil) + } + + @Test + func retrievePropagatesPrimaryFailure() { + struct PrimaryFailure: Error {} + let storage = KeychainLocalStorage( + keychain: FakeKeychain(readError: PrimaryFailure()), + legacyKeychains: [] + ) + #expect(throws: PrimaryFailure.self) { + try storage.retrieve(key: "key") + } + } + + @Test + func removeClearsPrimaryAndLegacyLocations() throws { + let primary = FakeKeychain(items: ["key": Data()]) + let legacy = FakeKeychain(items: ["key": Data()]) + let storage = KeychainLocalStorage(keychain: primary, legacyKeychains: [legacy]) + + try storage.remove(key: "key") + + #expect(primary.items.value["key"] == nil) + #expect(legacy.items.value["key"] == nil) + } + } + + final class FakeKeychain: KeychainProtocol, @unchecked Sendable { + let items: LockIsolated<[String: Data]> + let readError: (any Error)? + + init(items: [String: Data] = [:], readError: (any Error)? = nil) { + self.items = LockIsolated(items) + self.readError = readError + } + + func data(forKey key: String) throws -> Data? { + if let readError { + throw readError + } + return items.value[key] + } + + func set(_ data: Data, forKey key: String) throws { + items.withValue { $0[key] = data } + } + + func deleteItem(forKey key: String) throws { + items.withValue { $0[key] = nil } + } } #endif From 920fddf6cb7e839a36454be994a93bb72d0ab6bc Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 13:17:00 -0300 Subject: [PATCH 5/8] docs: add v3 migration entries for Keychain storage changes --- .../Auth/Storage/KeychainLocalStorage.swift | 2 +- V3_MIGRATION.md | 96 +++++++++++++++++++ dictionary.txt | 1 + 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/Sources/Auth/Storage/KeychainLocalStorage.swift b/Sources/Auth/Storage/KeychainLocalStorage.swift index f36e75e65..2fd5f0cbc 100644 --- a/Sources/Auth/Storage/KeychainLocalStorage.swift +++ b/Sources/Auth/Storage/KeychainLocalStorage.swift @@ -26,7 +26,7 @@ /// - accessGroup: An optional Keychain access group for sharing items between apps. /// - useDataProtectionKeychain: Targets the macOS data-protection Keychain instead of the /// legacy file-based one. This removes the macOS consent prompt, but requires the app to - /// be signed with entitlements authorised by a provisioning profile — otherwise Keychain + /// be signed with entitlements authorized by a provisioning profile — otherwise Keychain /// operations fail with `errSecMissingEntitlement` (-34018). Has no effect on platforms /// other than macOS. Defaults to `false`. public init(accessGroup: String? = nil, useDataProtectionKeychain: Bool = false) { diff --git a/V3_MIGRATION.md b/V3_MIGRATION.md index 619812fc2..894d6e81a 100644 --- a/V3_MIGRATION.md +++ b/V3_MIGRATION.md @@ -206,3 +206,99 @@ removed: `ObservationToken.remove()` has been removed — use `.cancel()` instead. `PostgrestError.detail` and `PostgrestError.init(detail:hint:code:message:)` have been removed — use `.details` and `init(details:hint:code:message:)`. + +## `KeychainLocalStorage`'s default Keychain service is now the host app's bundle identifier + +`KeychainLocalStorage()` no longer stores sessions under the fixed service +`"supabase.gotrue.swift"`. It now defaults to `Bundle.main.bundleIdentifier`, falling back to the +old constant only when there is no bundle identifier to read (command-line tools, some test +bundles). + +The fixed string put every app that embeds the SDK in the same Keychain namespace: two unrelated +apps on the same device, or two apps sharing an access group, could read and overwrite each +other's session under that one service name. Scoping the service to the bundle identifier gives +each app its own Keychain location by default. + +Existing sessions are not lost. On the first `retrieve` after upgrading, `KeychainLocalStorage` +probes the old `"supabase.gotrue.swift"` location, moves whatever it finds to the new +per-app location, and returns it — so users stay signed in. This is a behavior change, not a +compile error: nothing in the type signature changed, but the on-disk Keychain location did. If +you rely on the exact service name (for example, to inspect the Keychain from another tool, or +because several of your own apps intentionally shared the old namespace), pass it explicitly to +keep the pre-v3 location: + +```swift +// Before (implicit, shared "supabase.gotrue.swift" service) +let storage = KeychainLocalStorage() + +// After: keeps the pre-v3 location, no migration performed +let storage = KeychainLocalStorage(service: "supabase.gotrue.swift") +``` + +Note that passing `service:` explicitly — whether the old constant or a new value of your own — +selects the second, non-migrating initializer: `init(service:accessGroup:useDataProtectionKeychain:)`. +Only the parameterless-service initializer, `init(accessGroup:useDataProtectionKeychain:)`, probes +the legacy location. + +## `AuthLocalStorage.retrieve` returns `nil` for a missing key instead of throwing + +`AuthLocalStorage.retrieve(key:)` has always been documented as returning `nil` when the key is +absent, but `KeychainLocalStorage` didn't honor that: a missing item made the underlying +`SecItemCopyMatching` call return `errSecItemNotFound`, and that status was surfaced as a thrown +`KeychainError`, not as `nil`. `retrieve` now matches its own documentation and returns `nil` for +a missing item, throwing only when the Keychain read itself fails for another reason. + +Two consequences of the old behavior made this worth fixing rather than just documenting: every +app launch with no stored session threw and typically got logged as an error, since "no session +yet" is the normal state on a fresh install; and call sites that wrapped the read in `try?` to +treat "no session" as `nil` also swallowed genuine Keychain failures (for example +`errSecInteractionNotAllowed` when the device is locked) into that same `nil`, turning a real error +into a silent, incorrect sign-out. + +This is a behavior change, not a compile error — `retrieve`'s signature is unchanged. Search your +code for places that catch an error from `AuthLocalStorage.retrieve`/`KeychainLocalStorage.retrieve` +specifically to detect a missing session; that error no longer occurs, and you should instead +check the returned value for `nil`: + +```swift +// Before +do { + let data = try storage.retrieve(key: "supabase.session") + // handle existing session +} catch { + // this also ran for a plain "no session yet", not just real failures +} + +// After +if let data = try storage.retrieve(key: "supabase.session") { + // handle existing session +} else { + // no session stored — the normal case on first launch +} +``` + +If you have a custom `AuthLocalStorage` implementation, update it to return `nil` when the key is +absent and reserve `throw` for genuine failures. `remove(key:)` was changed the same way: deleting +an already-absent key is no longer an error and is treated as a no-op. + +## Opt-in macOS data-protection Keychain + +`KeychainLocalStorage`'s two initializers gained a `useDataProtectionKeychain` parameter, +defaulting to `false`. This is additive — existing call sites keep compiling and keep their +current behavior — but it's documented here because it's the fix for a common source of +confusion: on macOS, the legacy file-based Keychain that `KeychainLocalStorage` targets by default +shows the user a consent prompt naming `supabase.gotrue.swift`, even after the service-namespacing +change above, because the prompt is tied to the Keychain implementation, not the service name. +Passing `useDataProtectionKeychain: true` moves storage to the data-protection Keychain, which +does not show that prompt. + +```swift +let storage = KeychainLocalStorage(useDataProtectionKeychain: true) +``` + +This has a real requirement, not just a flag flip: the data-protection Keychain only works in an +app signed with entitlements authorized by a provisioning profile. Without them, every Keychain +operation fails with `errSecMissingEntitlement` (`-34018`) instead of storing anything. Verify the +flag works with your app's actual signing configuration — a debug build run from Xcode with the +right entitlements is not the same guarantee as your release signing — before enabling it in +production. The parameter has no effect on platforms other than macOS. diff --git a/dictionary.txt b/dictionary.txt index 634d2247d..910ce3cf4 100644 --- a/dictionary.txt +++ b/dictionary.txt @@ -114,6 +114,7 @@ posix omitempty openssl opentelemetry +parameterless passout passwordless Passwordless From ca3dbad7b7d783f26da14efa908a5afa7c06bd59 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 13:24:07 -0300 Subject: [PATCH 6/8] docs: narrow macOS consent-prompt claim to what TN3137 supports Review found the Keychain migration entry overstated what the prior research established: it asserted the file-based Keychain prompt still displays the literal old "supabase.gotrue.swift" string, but the service-name-drives-dialog-text claim was only ever community-sourced, not Apple-documented (per the design doc's Non-goals section, which is also why a kSecAttrLabel change was dropped from this work). Reword to describe only the documented mechanism instead: the prompt is tied to the app's designated requirement/ACL, governed by code-signing identity rather than kSecAttrService, per Apple TN3137. --- V3_MIGRATION.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/V3_MIGRATION.md b/V3_MIGRATION.md index 894d6e81a..bf806dfad 100644 --- a/V3_MIGRATION.md +++ b/V3_MIGRATION.md @@ -287,8 +287,9 @@ an already-absent key is no longer an error and is treated as a no-op. defaulting to `false`. This is additive — existing call sites keep compiling and keep their current behavior — but it's documented here because it's the fix for a common source of confusion: on macOS, the legacy file-based Keychain that `KeychainLocalStorage` targets by default -shows the user a consent prompt naming `supabase.gotrue.swift`, even after the service-namespacing -change above, because the prompt is tied to the Keychain implementation, not the service name. +still shows the user a consent prompt tied to your app's designated requirement, regardless of the +service name — the service-namespacing change above does not affect it, since the ACL that +triggers the prompt is governed by code-signing identity, not by `kSecAttrService` (see [Apple TN3137](https://developer.apple.com/documentation/technotes/tn3137-on-mac-keychains)). Passing `useDataProtectionKeychain: true` moves storage to the data-protection Keychain, which does not show that prompt. From 6415c48ff844086b5460114c0c9bb67ecda57f5c Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 13:36:41 -0300 Subject: [PATCH 7/8] fix(auth): make Keychain legacy cleanup and migration failure-safe remove(key:) previously exited before running the legacy-cleanup loop whenever the primary delete threw, leaving a stale legacy session in place. The next retrieve would migrate it back, signing a signed-out user back in. remove now always attempts every legacy location and re-throws the primary error afterward. retrieve(key:) previously discarded a successfully-read legacy value if the migration write to the primary location failed, surfacing the error and making the user appear signed out despite having a valid stored session. The write and the legacy delete are now sequenced so a failed write leaves the legacy copy in place (for the next read to retry) while still returning the value that was read. Also documents the best-effort semantics in the DocC comments, scopes an overclaim in V3_MIGRATION.md about cross-app Keychain collisions to the platforms where it actually applies, and extends FakeKeychain with writeError/deleteError to cover both fixes with new tests. --- .../Auth/Storage/KeychainLocalStorage.swift | 28 +++++++++-- .../AuthTests/KeychainLocalStorageTests.swift | 50 +++++++++++++++++-- V3_MIGRATION.md | 13 +++-- 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/Sources/Auth/Storage/KeychainLocalStorage.swift b/Sources/Auth/Storage/KeychainLocalStorage.swift index 2fd5f0cbc..e2a62fe7e 100644 --- a/Sources/Auth/Storage/KeychainLocalStorage.swift +++ b/Sources/Auth/Storage/KeychainLocalStorage.swift @@ -84,7 +84,9 @@ /// /// - Parameter key: The Keychain item key. /// - Returns: The stored bytes, or `nil` if the item does not exist. - /// - Throws: A Keychain error if the read fails. + /// - Throws: A Keychain error if the read from the current location fails. Failures while + /// probing or migrating from a legacy location are ignored — a value that was read is + /// always returned. public func retrieve(key: String) throws -> Data? { if let data = try keychain.data(forKey: key) { return data @@ -94,8 +96,14 @@ // A failing probe must not break a fresh install, so failures are ignored here. guard let data = try? legacy.data(forKey: key) else { continue } - try keychain.set(data, forKey: key) - try? legacy.deleteItem(forKey: key) + do { + try keychain.set(data, forKey: key) + // Only drop the legacy copy once the new one has landed. + try? legacy.deleteItem(forKey: key) + } catch { + // Leave the legacy copy in place; the next read retries the migration. + } + return data } @@ -105,13 +113,23 @@ /// Removes the Keychain item for `key`, including any left in a legacy location. /// /// - Parameter key: The Keychain item key to delete. - /// - Throws: A Keychain error if the delete fails. + /// - Throws: A Keychain error if deleting from the current location fails. Legacy-location + /// delete failures are ignored, with every location attempted regardless. public func remove(key: String) throws { - try keychain.deleteItem(forKey: key) + var primaryError: (any Error)? + do { + try keychain.deleteItem(forKey: key) + } catch { + primaryError = error + } for legacy in legacyKeychains { try? legacy.deleteItem(forKey: key) } + + if let primaryError { + throw primaryError + } } } diff --git a/Tests/AuthTests/KeychainLocalStorageTests.swift b/Tests/AuthTests/KeychainLocalStorageTests.swift index 18b48e60b..be2a4cdf8 100644 --- a/Tests/AuthTests/KeychainLocalStorageTests.swift +++ b/Tests/AuthTests/KeychainLocalStorageTests.swift @@ -128,12 +128,15 @@ func retrieveProbesLegacyLocationsInOrder() throws { let first = FakeKeychain(items: ["key": Data("first".utf8)]) let second = FakeKeychain(items: ["key": Data("second".utf8)]) + let primary = FakeKeychain() let storage = KeychainLocalStorage( - keychain: FakeKeychain(), + keychain: primary, legacyKeychains: [first, second] ) #expect(try storage.retrieve(key: "key") == Data("first".utf8)) + #expect(primary.items.value["key"] == Data("first".utf8)) + #expect(first.items.value["key"] == nil) #expect(second.items.value["key"] == Data("second".utf8)) } @@ -179,15 +182,50 @@ #expect(primary.items.value["key"] == nil) #expect(legacy.items.value["key"] == nil) } + + @Test + func removeClearsLegacyLocationsEvenWhenPrimaryDeleteFails() { + struct DeleteFailure: Error {} + let primary = FakeKeychain(items: ["key": Data()], deleteError: DeleteFailure()) + let legacy = FakeKeychain(items: ["key": Data()]) + let storage = KeychainLocalStorage(keychain: primary, legacyKeychains: [legacy]) + + #expect(throws: DeleteFailure.self) { + try storage.remove(key: "key") + } + #expect(legacy.items.value["key"] == nil) + } + + @Test + func retrieveReturnsLegacyValueAndKeepsItWhenPrimaryWriteFails() throws { + struct WriteFailure: Error {} + let legacyValue = Data("session".utf8) + let primary = FakeKeychain(writeError: WriteFailure()) + let legacy = FakeKeychain(items: ["key": legacyValue]) + let storage = KeychainLocalStorage(keychain: primary, legacyKeychains: [legacy]) + + #expect(try storage.retrieve(key: "key") == legacyValue) + #expect(legacy.items.value["key"] == legacyValue) + #expect(primary.items.value["key"] == nil) + } } final class FakeKeychain: KeychainProtocol, @unchecked Sendable { let items: LockIsolated<[String: Data]> let readError: (any Error)? - - init(items: [String: Data] = [:], readError: (any Error)? = nil) { + let writeError: (any Error)? + let deleteError: (any Error)? + + init( + items: [String: Data] = [:], + readError: (any Error)? = nil, + writeError: (any Error)? = nil, + deleteError: (any Error)? = nil + ) { self.items = LockIsolated(items) self.readError = readError + self.writeError = writeError + self.deleteError = deleteError } func data(forKey key: String) throws -> Data? { @@ -198,10 +236,16 @@ } func set(_ data: Data, forKey key: String) throws { + if let writeError { + throw writeError + } items.withValue { $0[key] = data } } func deleteItem(forKey key: String) throws { + if let deleteError { + throw deleteError + } items.withValue { $0[key] = nil } } } diff --git a/V3_MIGRATION.md b/V3_MIGRATION.md index bf806dfad..e4f4e65cd 100644 --- a/V3_MIGRATION.md +++ b/V3_MIGRATION.md @@ -214,10 +214,15 @@ and `PostgrestError.init(detail:hint:code:message:)` have been removed — use ` old constant only when there is no bundle identifier to read (command-line tools, some test bundles). -The fixed string put every app that embeds the SDK in the same Keychain namespace: two unrelated -apps on the same device, or two apps sharing an access group, could read and overwrite each -other's session under that one service name. Scoping the service to the bundle identifier gives -each app its own Keychain location by default. +The fixed string put every app that embeds the SDK in the same Keychain namespace. On +iOS/iPadOS/tvOS/watchOS/visionOS this was not a cross-app collision risk, since items are +implicitly scoped to the app's own default access group +(`$(AppIdentifierPrefix)$(CFBundleIdentifier)`), so unrelated apps could not read or overwrite each +other's session there. On macOS's file-based login Keychain, and for any apps deliberately sharing +an access group on any platform, the shared service name was a real collision risk: two such apps +could read and overwrite each other's session under that one service name. Either way, sharing a +single hardcoded service name is poor namespacing hygiene. Scoping the service to the bundle +identifier gives each app its own Keychain location by default. Existing sessions are not lost. On the first `retrieve` after upgrading, `KeychainLocalStorage` probes the old `"supabase.gotrue.swift"` location, moves whatever it finds to the new From b10f23c065532bb0981b7ab6275773f4c6201cf5 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Thu, 13 Aug 2026 16:10:05 -0300 Subject: [PATCH 8/8] fix(auth): propagate genuine legacy Keychain read failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR review feedback. The `try?` on the legacy probe was justified by "a failing probe must not break a fresh install", but that rationale went stale once `data(forKey:)` started mapping errSecItemNotFound to nil. An absent legacy item now reads as nil, so anything thrown from the probe is a genuine failure — a locked Keychain, a denied ACL prompt — and swallowing it reports "no session" for what is really an error. That is the exact failure this PR set out to fix, reintroduced one layer down. The `try?` on the two legacy *delete* sites is kept: best-effort cleanup of a legacy copy should not fail a sign-out or an otherwise successful migration, and both are documented as such. Also corrects two migration-guide overclaims: - The missing-key contract entry was written as if it fixed AuthLocalStorage protocol-wide. It fixes the Apple implementation only; WinCredLocalStorage still throws on ERROR_NOT_FOUND. Scoped the heading and body, and noted the Windows implementation is being dropped in v3 separately. - Enabling the data-protection Keychain does not avoid the consent prompt on the upgrade read: the migration deliberately probes the old file-based location, which can show the prompt one last time before the value moves. --- Sources/Auth/Storage/KeychainLocalStorage.swift | 11 ++++++----- Tests/AuthTests/KeychainLocalStorageTests.swift | 9 +++++++-- V3_MIGRATION.md | 17 ++++++++++++++++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/Sources/Auth/Storage/KeychainLocalStorage.swift b/Sources/Auth/Storage/KeychainLocalStorage.swift index e2a62fe7e..bd01c5bd7 100644 --- a/Sources/Auth/Storage/KeychainLocalStorage.swift +++ b/Sources/Auth/Storage/KeychainLocalStorage.swift @@ -84,17 +84,18 @@ /// /// - Parameter key: The Keychain item key. /// - Returns: The stored bytes, or `nil` if the item does not exist. - /// - Throws: A Keychain error if the read from the current location fails. Failures while - /// probing or migrating from a legacy location are ignored — a value that was read is - /// always returned. + /// - Throws: A Keychain error if reading the current location fails, or if probing a legacy + /// location fails. A failure to write the migrated value is not thrown — the value that was + /// read is returned and the migration is retried on the next read. public func retrieve(key: String) throws -> Data? { if let data = try keychain.data(forKey: key) { return data } for legacy in legacyKeychains { - // A failing probe must not break a fresh install, so failures are ignored here. - guard let data = try? legacy.data(forKey: key) else { continue } + // An absent legacy item reads as nil, so anything thrown here is a genuine failure — + // a locked Keychain, a denied ACL prompt — and must not be reported as "no session". + guard let data = try legacy.data(forKey: key) else { continue } do { try keychain.set(data, forKey: key) diff --git a/Tests/AuthTests/KeychainLocalStorageTests.swift b/Tests/AuthTests/KeychainLocalStorageTests.swift index be2a4cdf8..151401500 100644 --- a/Tests/AuthTests/KeychainLocalStorageTests.swift +++ b/Tests/AuthTests/KeychainLocalStorageTests.swift @@ -150,13 +150,18 @@ } @Test - func retrieveToleratesFailingLegacyProbe() throws { + func retrievePropagatesLegacyProbeFailure() { struct ProbeFailure: Error {} let storage = KeychainLocalStorage( keychain: FakeKeychain(), legacyKeychains: [FakeKeychain(readError: ProbeFailure())] ) - #expect(try storage.retrieve(key: "key") == nil) + + // A genuine legacy failure must surface rather than read as "no session" — an absent + // item already returns nil, so a throw here is never just a miss. + #expect(throws: ProbeFailure.self) { + try storage.retrieve(key: "key") + } } @Test diff --git a/V3_MIGRATION.md b/V3_MIGRATION.md index e4f4e65cd..ebff9df1f 100644 --- a/V3_MIGRATION.md +++ b/V3_MIGRATION.md @@ -245,7 +245,7 @@ selects the second, non-migrating initializer: `init(service:accessGroup:useData Only the parameterless-service initializer, `init(accessGroup:useDataProtectionKeychain:)`, probes the legacy location. -## `AuthLocalStorage.retrieve` returns `nil` for a missing key instead of throwing +## `KeychainLocalStorage.retrieve` returns `nil` for a missing key instead of throwing `AuthLocalStorage.retrieve(key:)` has always been documented as returning `nil` when the key is absent, but `KeychainLocalStorage` didn't honor that: a missing item made the underlying @@ -260,6 +260,15 @@ treat "no session" as `nil` also swallowed genuine Keychain failures (for exampl `errSecInteractionNotAllowed` when the device is locked) into that same `nil`, turning a real error into a silent, incorrect sign-out. +This fixes the Apple-platform implementation only. `WinCredLocalStorage`, the default on Windows, +still throws `WinCredLocalStorageError.windows` when `CredReadW` reports `ERROR_NOT_FOUND`, and its +`remove` is likewise not idempotent — so on Windows the protocol's documented contract is still not +honored. That implementation is being dropped in v3 in favor of requiring Windows callers to supply +their own `AuthLocalStorage`, tracked separately. + +If you implement `AuthLocalStorage` yourself, follow the documented contract: return `nil` for an +absent key, and throw only on a genuine failure. + This is a behavior change, not a compile error — `retrieve`'s signature is unchanged. Search your code for places that catch an error from `AuthLocalStorage.retrieve`/`KeychainLocalStorage.retrieve` specifically to detect a missing session; that error no longer occurs, and you should instead @@ -302,6 +311,12 @@ does not show that prompt. let storage = KeychainLocalStorage(useDataProtectionKeychain: true) ``` +One qualification for existing installs: items do not move between the two Keychain +implementations, so the first read after you enable the flag still probes the old file-based +location to migrate the session across. Reading an ACL-protected item there can show the prompt +one final time. Once the value has migrated, the file-based location is no longer read and the +prompt stops. + This has a real requirement, not just a flag flip: the data-protection Keychain only works in an app signed with entitlements authorized by a provisioning profile. Without them, every Keychain operation fails with `errSecMissingEntitlement` (`-34018`) instead of storing anything. Verify the