From a221b226917d9448553a8800bebd44e1c3d77ebc Mon Sep 17 00:00:00 2001 From: "evgeniy.chernomortsev" Date: Tue, 9 Dec 2025 14:42:55 +0400 Subject: [PATCH] refactor: consolidate KeychainHelper into KeychainManager (#797) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge KeychainHelper functionality into KeychainManager to provide a single, unified API for all keychain operations. - Remove KeychainHelper.swift and KeychainHelperProtocol - Add generic save/load/delete methods to KeychainManagerProtocol - Update NostrIdentityBridge to use KeychainManagerProtocol - Update FavoritesPersistenceService to use KeychainManagerProtocol - Update PreviewKeychainManager with new methods - Update MockKeychain and add MockKeychainHelper typealias for backwards compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- bitchat/Nostr/KeychainHelper.swift | 50 ---------------- bitchat/Nostr/NostrIdentityBridge.swift | 4 +- .../FavoritesPersistenceService.swift | 6 +- bitchat/Services/KeychainManager.swift | 59 ++++++++++++++++++- .../PreviewKeychainManager.swift | 33 ++++++++--- bitchatTests/Mocks/MockKeychain.swift | 53 +++++++++-------- 6 files changed, 115 insertions(+), 90 deletions(-) delete mode 100644 bitchat/Nostr/KeychainHelper.swift diff --git a/bitchat/Nostr/KeychainHelper.swift b/bitchat/Nostr/KeychainHelper.swift deleted file mode 100644 index 01de3dc4..00000000 --- a/bitchat/Nostr/KeychainHelper.swift +++ /dev/null @@ -1,50 +0,0 @@ -import Foundation - -protocol KeychainHelperProtocol { - func save(key: String, data: Data, service: String, accessible: CFString?) - func load(key: String, service: String) -> Data? - func delete(key: String, service: String) -} - -/// Keychain helper for secure storage -struct KeychainHelper: KeychainHelperProtocol { - func save(key: String, data: Data, service: String, accessible: CFString? = nil) { - var query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: key, - kSecValueData as String: data - ] - if let accessible = accessible { - query[kSecAttrAccessible as String] = accessible - } - - SecItemDelete(query as CFDictionary) - SecItemAdd(query as CFDictionary, nil) - } - - func load(key: String, service: String) -> Data? { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: key, - kSecReturnData as String: true - ] - - var result: AnyObject? - let status = SecItemCopyMatching(query as CFDictionary, &result) - - guard status == errSecSuccess else { return nil } - return result as? Data - } - - func delete(key: String, service: String) { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: key - ] - - SecItemDelete(query as CFDictionary) - } -} diff --git a/bitchat/Nostr/NostrIdentityBridge.swift b/bitchat/Nostr/NostrIdentityBridge.swift index 37e20779..01d929e0 100644 --- a/bitchat/Nostr/NostrIdentityBridge.swift +++ b/bitchat/Nostr/NostrIdentityBridge.swift @@ -12,9 +12,9 @@ final class NostrIdentityBridge { private var derivedIdentityCache: [String: NostrIdentity] = [:] private let cacheLock = NSLock() - private let keychain: KeychainHelperProtocol + private let keychain: KeychainManagerProtocol - init(keychain: KeychainHelperProtocol = KeychainHelper()) { + init(keychain: KeychainManagerProtocol = KeychainManager()) { self.keychain = keychain } diff --git a/bitchat/Services/FavoritesPersistenceService.swift b/bitchat/Services/FavoritesPersistenceService.swift index e892285e..1fe6b332 100644 --- a/bitchat/Services/FavoritesPersistenceService.swift +++ b/bitchat/Services/FavoritesPersistenceService.swift @@ -26,7 +26,7 @@ final class FavoritesPersistenceService: ObservableObject { private static let storageKey = "chat.bitchat.favorites" private static let keychainService = "chat.bitchat.favorites" - private let keychain: KeychainHelperProtocol + private let keychain: KeychainManagerProtocol @Published private(set) var favorites: [Data: FavoriteRelationship] = [:] // Noise pubkey -> relationship @Published private(set) var mutualFavorites: Set = [] @@ -35,8 +35,8 @@ final class FavoritesPersistenceService: ObservableObject { private var cancellables = Set() static let shared = FavoritesPersistenceService() - - init(keychain: KeychainHelperProtocol = KeychainHelper()) { + + init(keychain: KeychainManagerProtocol = KeychainManager()) { self.keychain = keychain loadFavorites() diff --git a/bitchat/Services/KeychainManager.swift b/bitchat/Services/KeychainManager.swift index be6ed784..c6af3d33 100644 --- a/bitchat/Services/KeychainManager.swift +++ b/bitchat/Services/KeychainManager.swift @@ -15,11 +15,19 @@ protocol KeychainManagerProtocol { func getIdentityKey(forKey key: String) -> Data? func deleteIdentityKey(forKey key: String) -> Bool func deleteAllKeychainData() -> Bool - + func secureClear(_ data: inout Data) func secureClear(_ string: inout String) - + func verifyIdentityKeyExists() -> Bool + + // MARK: - Generic Data Storage (consolidated from KeychainHelper) + /// Save data with a custom service name + func save(key: String, data: Data, service: String, accessible: CFString?) + /// Load data from a custom service + func load(key: String, service: String) -> Data? + /// Delete data from a custom service + func delete(key: String, service: String) } final class KeychainManager: KeychainManagerProtocol { @@ -309,9 +317,54 @@ final class KeychainManager: KeychainManagerProtocol { } // MARK: - Debug - + func verifyIdentityKeyExists() -> Bool { let key = "identity_noiseStaticKey" return retrieveData(forKey: key) != nil } + + // MARK: - Generic Data Storage (consolidated from KeychainHelper) + + /// Save data with a custom service name + func save(key: String, data: Data, service customService: String, accessible: CFString?) { + var query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: customService, + kSecAttrAccount as String: key, + kSecValueData as String: data + ] + if let accessible = accessible { + query[kSecAttrAccessible as String] = accessible + } + + SecItemDelete(query as CFDictionary) + SecItemAdd(query as CFDictionary, nil) + } + + /// Load data from a custom service + func load(key: String, service customService: String) -> Data? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: customService, + kSecAttrAccount as String: key, + kSecReturnData as String: true + ] + + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + guard status == errSecSuccess else { return nil } + return result as? Data + } + + /// Delete data from a custom service + func delete(key: String, service customService: String) { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: customService, + kSecAttrAccount as String: key + ] + + SecItemDelete(query as CFDictionary) + } } diff --git a/bitchat/_PreviewHelpers/PreviewKeychainManager.swift b/bitchat/_PreviewHelpers/PreviewKeychainManager.swift index 07542c4e..490fc69d 100644 --- a/bitchat/_PreviewHelpers/PreviewKeychainManager.swift +++ b/bitchat/_PreviewHelpers/PreviewKeychainManager.swift @@ -10,32 +10,51 @@ import Foundation final class PreviewKeychainManager: KeychainManagerProtocol { private var storage: [String: Data] = [:] + private var serviceStorage: [String: [String: Data]] = [:] init() {} - + func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool { storage[key] = keyData return true } - + func getIdentityKey(forKey key: String) -> Data? { storage[key] } - + func deleteIdentityKey(forKey key: String) -> Bool { storage.removeValue(forKey: key) return true } - + func deleteAllKeychainData() -> Bool { storage.removeAll() + serviceStorage.removeAll() return true } - + func secureClear(_ data: inout Data) {} - + func secureClear(_ string: inout String) {} - + func verifyIdentityKeyExists() -> Bool { storage["identity_noiseStaticKey"] != nil } + + // MARK: - Generic Data Storage (consolidated from KeychainHelper) + + func save(key: String, data: Data, service: String, accessible: CFString?) { + if serviceStorage[service] == nil { + serviceStorage[service] = [:] + } + serviceStorage[service]?[key] = data + } + + func load(key: String, service: String) -> Data? { + serviceStorage[service]?[key] + } + + func delete(key: String, service: String) { + serviceStorage[service]?.removeValue(forKey: key) + } } diff --git a/bitchatTests/Mocks/MockKeychain.swift b/bitchatTests/Mocks/MockKeychain.swift index f8718c82..c3ffa9d3 100644 --- a/bitchatTests/Mocks/MockKeychain.swift +++ b/bitchatTests/Mocks/MockKeychain.swift @@ -11,54 +11,57 @@ import Foundation final class MockKeychain: KeychainManagerProtocol { private var storage: [String: Data] = [:] - + private var serviceStorage: [String: [String: Data]] = [:] + func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool { storage[key] = keyData return true } - + func getIdentityKey(forKey key: String) -> Data? { storage[key] } - + func deleteIdentityKey(forKey key: String) -> Bool { storage.removeValue(forKey: key) return true } - + func deleteAllKeychainData() -> Bool { storage.removeAll() + serviceStorage.removeAll() return true } - + func secureClear(_ data: inout Data) { - // data = Data() } - + func secureClear(_ string: inout String) { string = "" } - + func verifyIdentityKeyExists() -> Bool { storage["identity_noiseStaticKey"] != nil } + + // MARK: - Generic Data Storage (consolidated from KeychainHelper) + + func save(key: String, data: Data, service: String, accessible: CFString?) { + if serviceStorage[service] == nil { + serviceStorage[service] = [:] + } + serviceStorage[service]?[key] = data + } + + func load(key: String, service: String) -> Data? { + serviceStorage[service]?[key] + } + + func delete(key: String, service: String) { + serviceStorage[service]?.removeValue(forKey: key) + } } -final class MockKeychainHelper: KeychainHelperProtocol { - private typealias Service = String - private typealias Key = String - private var storage: [Service: [Key: Data]] = [:] - - func save(key: String, data: Data, service: String, accessible: CFString?) { - storage[service]?[key] = data - } - - func load(key: String, service: String) -> Data? { - storage[service]?[key] - } - - func delete(key: String, service: String) { - storage[service]?.removeValue(forKey: key) - } -} +/// Typealias for backwards compatibility with tests using MockKeychainHelper +typealias MockKeychainHelper = MockKeychain