From 149c8f449743f8417cf530efb049d72a17639b7a Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Mon, 23 Dec 2019 19:16:53 -0800 Subject: [PATCH 1/8] Remove Always accessibility specifier --- Sources/Valet/Accessibility.swift | 26 ++------- Sources/Valet/CloudAccessibility.swift | 9 +-- Sources/Valet/Internal/SecItem.swift | 13 ++--- Sources/Valet/Internal/Service.swift | 11 ++-- Sources/Valet/KeychainQueryConvertible.swift | 29 ---------- Sources/Valet/SecureEnclaveValet.swift | 51 ++++++++++++----- .../SinglePromptSecureEnclaveValet.swift | 55 +++++++++++++----- Sources/Valet/Valet.swift | 56 +++++++++++++------ .../ValetBackwardsCompatibilityTests.swift | 2 - .../ValetIntegrationTests.swift | 23 +++++--- Tests/ValetTests/ValetTests.swift | 2 +- Valet.xcodeproj/project.pbxproj | 10 ---- 12 files changed, 156 insertions(+), 131 deletions(-) delete mode 100644 Sources/Valet/KeychainQueryConvertible.swift diff --git a/Sources/Valet/Accessibility.swift b/Sources/Valet/Accessibility.swift index a413acd0..913fd9c0 100644 --- a/Sources/Valet/Accessibility.swift +++ b/Sources/Valet/Accessibility.swift @@ -26,19 +26,15 @@ public enum Accessibility: Int, CustomStringConvertible, Equatable { /// Valet data can only be accessed while the device is unlocked. This attribute is recommended for data that only needs to be accessible while the application is in the foreground. Valet data with this attribute will migrate to a new device when using encrypted backups. case whenUnlocked = 1 /// Valet data can only be accessed once the device has been unlocked after a restart. This attribute is recommended for data that needs to be accessible by background applications. Valet data with this attribute will migrate to a new device when using encrypted backups. - case afterFirstUnlock - /// Valet data can always be accessed regardless of the lock state of the device. This attribute is not recommended. Valet data with this attribute will migrate to a new device when using encrypted backups. - case always - + case afterFirstUnlock = 2 + /// Valet data can only be accessed while the device is unlocked. This attribute is recommended for items that only need to be accessible while the application is in the foreground. Valet data with this attribute will never migrate to a new device, so these items will be missing after a backup is restored to a new device. No items can be stored in this class on devices without a passcode. Disabling the device passcode will cause all items in this class to be deleted. - case whenPasscodeSetThisDeviceOnly + case whenPasscodeSetThisDeviceOnly = 4 /// Valet data can only be accessed while the device is unlocked. This is recommended for data that only needs to be accessible while the application is in the foreground. Valet data with this attribute will never migrate to a new device, so these items will be missing after a backup is restored to a new device. - case whenUnlockedThisDeviceOnly + case whenUnlockedThisDeviceOnly = 5 /// Valet data can only be accessed once the device has been unlocked after a restart. This is recommended for items that need to be accessible by background applications. Valet data with this attribute will never migrate to a new device, so these items will be missing after a backup is restored to a new device. - case afterFirstUnlockThisDeviceOnly - /// Valet data can always be accessed regardless of the lock state of the device. This option is not recommended. Valet data with this attribute will never migrate to a new device, so these items will be missing after a backup is restored to a new device. - case alwaysThisDeviceOnly - + case afterFirstUnlockThisDeviceOnly = 6 + // MARK: CustomStringConvertible public var description: String { @@ -47,10 +43,6 @@ public enum Accessibility: Int, CustomStringConvertible, Equatable { return "AccessibleAfterFirstUnlock" case .afterFirstUnlockThisDeviceOnly: return "AccessibleAfterFirstUnlockThisDeviceOnly" - case .always: - return "AccessibleAlways" - case .alwaysThisDeviceOnly: - return "AccessibleAlwaysThisDeviceOnly" case .whenPasscodeSetThisDeviceOnly: return "AccessibleWhenPasscodeSetThisDeviceOnly" case .whenUnlocked: @@ -70,10 +62,6 @@ public enum Accessibility: Int, CustomStringConvertible, Equatable { accessibilityAttribute = kSecAttrAccessibleAfterFirstUnlock case .afterFirstUnlockThisDeviceOnly: accessibilityAttribute = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly - case .always: - accessibilityAttribute = kSecAttrAccessibleAlways - case .alwaysThisDeviceOnly: - accessibilityAttribute = kSecAttrAccessibleAlwaysThisDeviceOnly case .whenPasscodeSetThisDeviceOnly: accessibilityAttribute = kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly case .whenUnlocked: @@ -91,11 +79,9 @@ public enum Accessibility: Int, CustomStringConvertible, Equatable { return [ .whenUnlocked, .afterFirstUnlock, - .always, .whenPasscodeSetThisDeviceOnly, .whenUnlockedThisDeviceOnly, .afterFirstUnlockThisDeviceOnly, - .alwaysThisDeviceOnly ] } } diff --git a/Sources/Valet/CloudAccessibility.swift b/Sources/Valet/CloudAccessibility.swift index acf126bc..5f97ad29 100644 --- a/Sources/Valet/CloudAccessibility.swift +++ b/Sources/Valet/CloudAccessibility.swift @@ -26,10 +26,8 @@ public enum CloudAccessibility: Int, CustomStringConvertible, Equatable { /// Valet data can only be accessed while the device is unlocked. This attribute is recommended for data that only needs to be accessible while the application is in the foreground. Valet data with this attribute will migrate to a new device when using encrypted backups. case whenUnlocked = 1 /// Valet data can only be accessed once the device has been unlocked after a restart. This attribute is recommended for data that needs to be accessible by background applications. Valet data with this attribute will migrate to a new device when using encrypted backups. - case afterFirstUnlock - /// Valet data can always be accessed regardless of the lock state of the device. This attribute is not recommended. Valet data with this attribute will migrate to a new device when using encrypted backups. - case always - + case afterFirstUnlock = 2 + // MARK: CustomStringConvertible public var description: String { @@ -44,8 +42,6 @@ public enum CloudAccessibility: Int, CustomStringConvertible, Equatable { return .whenUnlocked case .afterFirstUnlock: return .afterFirstUnlock - case .always: - return .always } } @@ -59,7 +55,6 @@ public enum CloudAccessibility: Int, CustomStringConvertible, Equatable { return [ .whenUnlocked, .afterFirstUnlock, - .always ] } } diff --git a/Sources/Valet/Internal/SecItem.swift b/Sources/Valet/Internal/SecItem.swift index af42034a..9bf98da1 100644 --- a/Sources/Valet/Internal/SecItem.swift +++ b/Sources/Valet/Internal/SecItem.swift @@ -69,12 +69,12 @@ internal final class SecItem { // MARK: Internal Class Properties /// Programatically grab the required prefix for the shared access group (i.e. Bundle Seed ID). The value for the kSecAttrAccessGroup key in queries for data that is shared between apps must be of the format bundleSeedID.sharedAccessGroup. For more information on the Bundle Seed ID, see https://developer.apple.com/library/ios/qa/qa1713/_index.html - internal static var sharedAccessGroupPrefix: String { + internal static var sharedAccessGroupPrefix: String? { let query = [ kSecClass : kSecClassGenericPassword, - kSecAttrAccount : "SharedAccessGroupAlwaysAccessiblePrefixPlaceholder", + kSecAttrAccount : "SharedAccessGroupPrefixPlaceholder", kSecReturnAttributes : true, - kSecAttrAccessible : Accessibility.alwaysThisDeviceOnly.secAccessibilityAttribute + kSecAttrAccessible : Accessibility.afterFirstUnlockThisDeviceOnly.secAccessibilityAttribute ] as CFDictionary secItemLock.lock() @@ -90,11 +90,10 @@ internal final class SecItem { } guard status == errSecSuccess, let queryResult = result as? [CFString : AnyHashable], let accessGroup = queryResult[kSecAttrAccessGroup] as? String else { - ErrorHandler.assertionFailure("Could not find shared access group prefix.") - // We should always be able to access the shared access group prefix because the accessibility of the above keychain data is set to `always`. + // We may not be able to access the shared access group prefix because the accessibility of the above keychain data is set to `afterFirstUnlock`. // In other words, we should never hit this code. This code is here as a failsafe to prevent a crash in a scenario where the keychain is entirely hosed. // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error. - return "INVALID_SHARED_ACCESS_GROUP_PREFIX" + return nil } let components = accessGroup.components(separatedBy: ".") @@ -105,7 +104,7 @@ internal final class SecItem { // We should always be able to access the shared access group prefix because the accessibility of the above keychain data is set to `always`. // In other words, we should never hit this code. This code is here as a failsafe to prevent a crash in a scenario where the keychain is entirely hosed. // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error. - return "INVALID_SHARED_ACCESS_GROUP_PREFIX" + return nil } } diff --git a/Sources/Valet/Internal/Service.swift b/Sources/Valet/Internal/Service.swift index 540bf8fc..8e25b9ac 100644 --- a/Sources/Valet/Internal/Service.swift +++ b/Sources/Valet/Internal/Service.swift @@ -24,7 +24,7 @@ import Foundation internal enum Service: CustomStringConvertible, Equatable { case standard(Identifier, Configuration) case sharedAccessGroup(Identifier, Configuration) - + // MARK: Equatable internal static func ==(lhs: Service, rhs: Service) -> Bool { @@ -39,7 +39,7 @@ internal enum Service: CustomStringConvertible, Equatable { // MARK: Internal Methods - internal func generateBaseQuery() -> [String : AnyHashable] { + internal func generateBaseQuery() -> [String : AnyHashable]? { var baseQuery: [String : AnyHashable] = [ kSecClass as String : kSecClassGenericPassword as String, kSecAttrService as String : secService @@ -51,8 +51,11 @@ internal enum Service: CustomStringConvertible, Equatable { configuration = desiredConfiguration case let .sharedAccessGroup(identifier, desiredConfiguration): - ErrorHandler.assert(!identifier.description.hasPrefix("\(SecItem.sharedAccessGroupPrefix)."), "Do not add the Bundle Seed ID as a prefix to your identifier. Valet prepends this value for you. Your Valet will not be able to access the keychain with the provided configuration") - baseQuery[kSecAttrAccessGroup as String] = "\(SecItem.sharedAccessGroupPrefix).\(identifier.description)" + guard let sharedAccessGroupPrefix = SecItem.sharedAccessGroupPrefix else { + return nil + } + ErrorHandler.assert(!identifier.description.hasPrefix("\(sharedAccessGroupPrefix)."), "Do not add the Bundle Seed ID as a prefix to your identifier. Valet prepends this value for you. Your Valet will not be able to access the keychain with the provided configuration") + baseQuery[kSecAttrAccessGroup as String] = "\(sharedAccessGroupPrefix).\(identifier.description)" configuration = desiredConfiguration } diff --git a/Sources/Valet/KeychainQueryConvertible.swift b/Sources/Valet/KeychainQueryConvertible.swift deleted file mode 100644 index c79d5096..00000000 --- a/Sources/Valet/KeychainQueryConvertible.swift +++ /dev/null @@ -1,29 +0,0 @@ -// -// KeychainQueryConvertible.swift -// Valet -// -// Created by Dan Federman and Eric Muller on 9/17/17. -// Copyright © 2017 Square, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -//    http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Foundation - - -@objc(VALKeychainQueryConvertible) -public protocol KeychainQueryConvertible { - - var keychainQuery: [String : AnyHashable] { get } - -} diff --git a/Sources/Valet/SecureEnclaveValet.swift b/Sources/Valet/SecureEnclaveValet.swift index bc98d4e2..10757943 100644 --- a/Sources/Valet/SecureEnclaveValet.swift +++ b/Sources/Valet/SecureEnclaveValet.swift @@ -74,18 +74,25 @@ public final class SecureEnclaveValet: NSObject { fatalError("Use the class methods above to create usable SecureEnclaveValet objects") } - private init(identifier: Identifier, accessControl: SecureEnclaveAccessControl) { - service = .standard(identifier, .secureEnclave(accessControl)) - keychainQuery = service.generateBaseQuery() - self.identifier = identifier - self.accessControl = accessControl + private convenience init(identifier: Identifier, accessControl: SecureEnclaveAccessControl) { + self.init( + identifier: identifier, + service: .standard(identifier, .secureEnclave(accessControl)), + accessControl: accessControl) } - private init(sharedAccess identifier: Identifier, accessControl: SecureEnclaveAccessControl) { - service = .sharedAccessGroup(identifier, .secureEnclave(accessControl)) - keychainQuery = service.generateBaseQuery() + private convenience init(sharedAccess identifier: Identifier, accessControl: SecureEnclaveAccessControl) { + self.init( + identifier: identifier, + service: .sharedAccessGroup(identifier, .secureEnclave(accessControl)), + accessControl: accessControl) + } + + private init(identifier: Identifier, service: Service, accessControl: SecureEnclaveAccessControl) { self.identifier = identifier + self.service = service self.accessControl = accessControl + _keychainQuery = service.generateBaseQuery() } // MARK: Hashable @@ -116,6 +123,7 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return SecureEnclave.set(object: object, forKey: key, options: keychainQuery) } } @@ -125,6 +133,7 @@ public final class SecureEnclaveValet: NSObject { /// - returns: The data currently stored in the keychain for the provided key. Returns `.itemNotFound` if no object exists in the keychain for the specified key, or if the keychain is inaccessible. Returns `.userCancelled` if the user cancels the user-presence prompt. public func object(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return .itemNotFound } return SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: keychainQuery) } } @@ -135,6 +144,7 @@ public final class SecureEnclaveValet: NSObject { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return SecureEnclave.containsObject(forKey: key, options: keychainQuery) } } @@ -146,15 +156,17 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return SecureEnclave.set(string: string, forKey: key, options: keychainQuery) } } /// - parameter key: A Key used to retrieve the desired object from the keychain. /// - parameter userPrompt: The prompt displayed to the user in Apple's Face ID, Touch ID, or passcode entry UI. - /// - returns: The string currently stored in the keychain for the provided key. Returns `nil` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. + /// - returns: The string currently stored in the keychain for the provided key. Returns `itemNotFound` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. public func string(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return .itemNotFound } return SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: keychainQuery) } } @@ -165,6 +177,7 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.removeObject(forKey: key, options: keychainQuery).didSucceed } } @@ -175,6 +188,7 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.removeAllObjects(matching: keychainQuery).didSucceed } } @@ -187,6 +201,7 @@ public final class SecureEnclaveValet: NSObject { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return .couldNotReadKeychain } return Keychain.migrateObjects(matching: query, into: keychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -196,9 +211,10 @@ public final class SecureEnclaveValet: NSObject { /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychfain if the migration succeeds. /// - returns: Whether the migration succeeded or failed. /// - note: The keychain is not modified if a failure occurs. - @objc(migrateObjectsFromKeychain:removeOnCompletion:) - public func migrateObjects(from keychain: KeychainQueryConvertible, removeOnCompletion: Bool) -> MigrationResult { - return migrateObjects(matching: keychain.keychainQuery, removeOnCompletion: removeOnCompletion) + @objc(migrateObjectsFromValet:removeOnCompletion:) + public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { + guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } // MARK: Internal Properties @@ -208,7 +224,16 @@ public final class SecureEnclaveValet: NSObject { // MARK: Private Properties private let lock = NSLock() - private let keychainQuery: [String : AnyHashable] + internal var keychainQuery: [String : AnyHashable]? { + if let keychainQuery = _keychainQuery { + return keychainQuery + } else { + _keychainQuery = service.generateBaseQuery() + return _keychainQuery + } + } + + private var _keychainQuery: [String : AnyHashable]? } diff --git a/Sources/Valet/SinglePromptSecureEnclaveValet.swift b/Sources/Valet/SinglePromptSecureEnclaveValet.swift index d0bf24d7..0d88f542 100644 --- a/Sources/Valet/SinglePromptSecureEnclaveValet.swift +++ b/Sources/Valet/SinglePromptSecureEnclaveValet.swift @@ -77,18 +77,25 @@ public final class SinglePromptSecureEnclaveValet: NSObject { fatalError("Use the class methods above to create usable SinglePromptSecureEnclaveValet objects") } - private init(identifier: Identifier, accessControl: SecureEnclaveAccessControl) { - service = .standard(identifier, .singlePromptSecureEnclave(accessControl)) - baseKeychainQuery = service.generateBaseQuery() - self.identifier = identifier - self.accessControl = accessControl + private convenience init(identifier: Identifier, accessControl: SecureEnclaveAccessControl) { + self.init( + identifier: identifier, + service: .standard(identifier, .singlePromptSecureEnclave(accessControl)), + accessControl: accessControl) } - private init(sharedAccess identifier: Identifier, accessControl: SecureEnclaveAccessControl) { - service = .sharedAccessGroup(identifier, .singlePromptSecureEnclave(accessControl)) - baseKeychainQuery = service.generateBaseQuery() + private convenience init(sharedAccess identifier: Identifier, accessControl: SecureEnclaveAccessControl) { + self.init( + identifier: identifier, + service: .sharedAccessGroup(identifier, .singlePromptSecureEnclave(accessControl)), + accessControl: accessControl) + } + + private init(identifier: Identifier, service: Service, accessControl: SecureEnclaveAccessControl) { self.identifier = identifier + self.service = service self.accessControl = accessControl + _baseKeychainQuery = service.generateBaseQuery() } // MARK: Hashable @@ -119,6 +126,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return false } return SecureEnclave.set(object: object, forKey: key, options: baseKeychainQuery) } } @@ -128,6 +136,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// - returns: The data currently stored in the keychain for the provided key. Returns `.itemNotFound` if no object exists in the keychain for the specified key, or if the keychain is inaccessible. Returns `.userCancelled` if the user cancels the user-presence prompt. public func object(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return .itemNotFound } return SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: continuedAuthenticationKeychainQuery) } } @@ -138,6 +147,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return false } return SecureEnclave.containsObject(forKey: key, options: baseKeychainQuery) } } @@ -149,15 +159,17 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return false } return SecureEnclave.set(string: string, forKey: key, options: baseKeychainQuery) } } /// - parameter key: A Key used to retrieve the desired object from the keychain. /// - parameter userPrompt: The prompt displayed to the user in Apple's Face ID, Touch ID, or passcode entry UI. If the `SinglePromptSecureEnclaveValet` has already been unlocked, no prompt will be shown. - /// - returns: The string currently stored in the keychain for the provided key. Returns `nil` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. + /// - returns: The string currently stored in the keychain for the provided key. Returns `itemNotFound` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. public func string(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return .itemNotFound } return SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: continuedAuthenticationKeychainQuery) } } @@ -176,6 +188,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(allKeysWithUserPrompt:) public func allKeys(userPrompt: String) -> Set { return execute(in: lock) { + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return Set() } var secItemQuery = continuedAuthenticationKeychainQuery if !userPrompt.isEmpty { secItemQuery[kSecUseOperationPrompt as String] = userPrompt @@ -191,6 +204,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return false } return Keychain.removeObject(forKey: key, options: baseKeychainQuery).didSucceed } } @@ -201,6 +215,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return false } return Keychain.removeAllObjects(matching: baseKeychainQuery).didSucceed } } @@ -213,6 +228,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { + guard let baseKeychainQuery = baseKeychainQuery else { return .couldNotReadKeychain } return Keychain.migrateObjects(matching: query, into: baseKeychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -222,9 +238,10 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychfain if the migration succeeds. /// - returns: Whether the migration succeeded or failed. /// - note: The keychain is not modified if a failure occurs. - @objc(migrateObjectsFromKeychain:removeOnCompletion:) - public func migrateObjects(from keychain: KeychainQueryConvertible, removeOnCompletion: Bool) -> MigrationResult { - return migrateObjects(matching: keychain.keychainQuery, removeOnCompletion: removeOnCompletion) + @objc(migrateObjectsFromValet:removeOnCompletion:) + public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { + guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } // MARK: Internal Properties @@ -234,14 +251,24 @@ public final class SinglePromptSecureEnclaveValet: NSObject { // MARK: Private Properties private let lock = NSLock() - private let baseKeychainQuery: [String : AnyHashable] private var localAuthenticationContext = LAContext() + private var baseKeychainQuery: [String : AnyHashable]? { + if let baseKeychainQuery = _baseKeychainQuery { + return baseKeychainQuery + } else { + _baseKeychainQuery = service.generateBaseQuery() + return _baseKeychainQuery + } + } + private var _baseKeychainQuery: [String : AnyHashable]? + /// A keychain query dictionary that allows for continued read access to the Secure Enclave after the a single unlock event. /// This query should be used when retrieving keychain data, but should not be used for keychain writes or `containsObject` checks. /// Using this query in a `containsObject` check can cause a false positive in the case where an element has been removed from /// the keychain by the operating system due to a face, fingerprint, or password change. - private var continuedAuthenticationKeychainQuery: [String : AnyHashable] { + private var continuedAuthenticationKeychainQuery: [String : AnyHashable]? { + guard let baseKeychainQuery = baseKeychainQuery else { return nil } var keychainQuery = baseKeychainQuery keychainQuery[kSecUseAuthenticationContext as String] = localAuthenticationContext return keychainQuery diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index 51a2b3ba..b8d6ada4 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -23,7 +23,7 @@ import Foundation /// Reads and writes keychain elements. @objc(VALValet) -public final class Valet: NSObject, KeychainQueryConvertible { +public final class Valet: NSObject { // MARK: Public Class Methods @@ -94,20 +94,26 @@ public final class Valet: NSObject, KeychainQueryConvertible { fatalError("Use the class methods above to create usable Valet objects") } - private init(identifier: Identifier, configuration: Configuration) { - self.identifier = identifier - self.configuration = configuration - service = .standard(identifier, configuration) - accessibility = configuration.accessibility - keychainQuery = service.generateBaseQuery() + private convenience init(identifier: Identifier, configuration: Configuration) { + self.init( + identifier: identifier, + service: .standard(identifier, configuration), + configuration: configuration) } - private init(sharedAccess identifier: Identifier, configuration: Configuration) { + private convenience init(sharedAccess identifier: Identifier, configuration: Configuration) { + self.init( + identifier: identifier, + service: .sharedAccessGroup(identifier, configuration), + configuration: configuration) + } + + private init(identifier: Identifier, service: Service, configuration: Configuration) { self.identifier = identifier self.configuration = configuration - service = .sharedAccessGroup(identifier, configuration) + self.service = service accessibility = configuration.accessibility - keychainQuery = service.generateBaseQuery() + _keychainQuery = service.generateBaseQuery() } // MARK: CustomStringConvertible @@ -115,11 +121,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { public override var description: String { return "\(super.description) \(identifier.description) \(configuration.prettyDescription)" } - - // MARK: KeychainQueryConvertible - - public let keychainQuery: [String : AnyHashable] - + // MARK: Hashable public override var hash: Int { @@ -139,6 +141,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc public func canAccessKeychain() -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.canAccess(attributes: keychainQuery) } } @@ -150,6 +153,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.set(object: object, forKey: key, options: keychainQuery).didSucceed } } @@ -159,6 +163,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc(objectForKey:) public func object(forKey key: String) -> Data? { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return nil } return Keychain.object(forKey: key, options: keychainQuery).value } } @@ -168,6 +173,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.containsObject(forKey: key, options: keychainQuery).didSucceed } } @@ -179,6 +185,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.set(string: string, forKey: key, options: keychainQuery).didSucceed } } @@ -188,6 +195,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc(stringForKey:) public func string(forKey key: String) -> String? { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return nil } return Keychain.string(forKey: key, options: keychainQuery).value } } @@ -196,6 +204,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc public func allKeys() -> Set { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return Set() } return Keychain.allKeys(options: keychainQuery).value ?? Set() } } @@ -206,6 +215,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.removeObject(forKey: key, options: keychainQuery).didSucceed } } @@ -216,6 +226,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return false } return Keychain.removeAllObjects(matching: keychainQuery).didSucceed } } @@ -228,6 +239,7 @@ public final class Valet: NSObject, KeychainQueryConvertible { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { + guard let keychainQuery = keychainQuery else { return .couldNotReadKeychain } return Keychain.migrateObjects(matching: query, into: keychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -238,18 +250,28 @@ public final class Valet: NSObject, KeychainQueryConvertible { /// - returns: Whether the migration succeeded or failed. /// - note: The keychain is not modified if a failure occurs. @objc(migrateObjectsFromValet:removeOnCompletion:) - public func migrateObjects(from keychain: KeychainQueryConvertible, removeOnCompletion: Bool) -> MigrationResult { - return migrateObjects(matching: keychain.keychainQuery, removeOnCompletion: removeOnCompletion) + public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { + guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } // MARK: Internal Properties internal let configuration: Configuration internal let service: Service + internal var keychainQuery: [String : AnyHashable]? { + if let keychainQuery = _keychainQuery { + return keychainQuery + } else { + _keychainQuery = service.generateBaseQuery() + return _keychainQuery + } + } // MARK: Private Properties private let lock = NSLock() + private var _keychainQuery: [String : AnyHashable]? } diff --git a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift index 937cc058..173d745b 100644 --- a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift +++ b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift @@ -37,8 +37,6 @@ internal extension Valet { switch accessibility { case .afterFirstUnlock: return .afterFirstUnlock case .afterFirstUnlockThisDeviceOnly: return .afterFirstUnlockThisDeviceOnly - case .always: return .always - case .alwaysThisDeviceOnly: return .alwaysThisDeviceOnly case .whenPasscodeSetThisDeviceOnly: return .whenPasscodeSetThisDeviceOnly case .whenUnlocked: return .whenUnlocked case .whenUnlockedThisDeviceOnly: return .whenUnlockedThisDeviceOnly diff --git a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift index d761a734..f8b9bb4d 100644 --- a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift +++ b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift @@ -173,7 +173,7 @@ class ValetIntegrationTests: XCTestCase func test_valetsWithDifferingAccessibility_areNotEqual() { - let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .whenUnlockedThisDeviceOnly) XCTAssertNotEqual(valet, differingAccessibility) } @@ -249,7 +249,7 @@ class ValetIntegrationTests: XCTestCase XCTAssertEqual(differingIdentifier.allKeys(), Set()) // Different Accessibility - let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .whenUnlockedThisDeviceOnly) XCTAssertEqual(differingAccessibility.allKeys(), Set()) // Different Kind @@ -492,7 +492,7 @@ class ValetIntegrationTests: XCTestCase func test_removeObjectForKey_isDistinctForDifferingAccessibility() { - let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .whenUnlockedThisDeviceOnly) XCTAssertTrue(valet.set(string: passcode, forKey: key)) XCTAssertTrue(differingAccessibility.removeObject(forKey: key)) @@ -535,11 +535,15 @@ class ValetIntegrationTests: XCTestCase valet.set(string: passcode, forKey: key) + guard let valetKeychainQuery = valet.keychainQuery else { + XCTFail() + return + } // Test for base query success. - XCTAssertEqual(anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: false), .success) + XCTAssertEqual(anotherFlavor.migrateObjects(matching: valetKeychainQuery, removeOnCompletion: false), .success) XCTAssertEqual(passcode, anotherFlavor.string(forKey: key)) - var mutableQuery = valet.keychainQuery + var mutableQuery = valetKeychainQuery mutableQuery.removeValue(forKey: kSecClass as String) // Without a kSecClass, the migration should fail. @@ -566,9 +570,14 @@ class ValetIntegrationTests: XCTestCase XCTAssertEqual(noItemsFoundError, valet.migrateObjects(matching: queryWithNoMatches, removeOnCompletion: false)) XCTAssertEqual(noItemsFoundError, valet.migrateObjects(matching: queryWithNoMatches, removeOnCompletion: true)) + guard let valetKeychainQuery = valet.keychainQuery else { + XCTFail() + return + } + // Our test Valet has not yet been written to, migration should fail: - XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: false)) - XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: true)) + XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valetKeychainQuery, removeOnCompletion: false)) + XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valetKeychainQuery, removeOnCompletion: true)) } // FIXME: Looks to me like this test may no longer be valid, need to dig a bit diff --git a/Tests/ValetTests/ValetTests.swift b/Tests/ValetTests/ValetTests.swift index 82c3123f..e42a971b 100644 --- a/Tests/ValetTests/ValetTests.swift +++ b/Tests/ValetTests/ValetTests.swift @@ -107,7 +107,7 @@ class ValetTests: XCTestCase func test_valetsWithDifferingAccessibility_areNotEqual() { - let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .whenUnlockedThisDeviceOnly) XCTAssertNotEqual(valet, differingAccessibility) } diff --git a/Valet.xcodeproj/project.pbxproj b/Valet.xcodeproj/project.pbxproj index 7aa124e4..23950b62 100644 --- a/Valet.xcodeproj/project.pbxproj +++ b/Valet.xcodeproj/project.pbxproj @@ -45,10 +45,6 @@ 1612FD7E22A9CAAB00FC1142 /* MigrationResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6B22A9CAAB00FC1142 /* MigrationResult.swift */; }; 1612FD7F22A9CAAB00FC1142 /* MigrationResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6B22A9CAAB00FC1142 /* MigrationResult.swift */; }; 1612FD8022A9CAAB00FC1142 /* MigrationResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6B22A9CAAB00FC1142 /* MigrationResult.swift */; }; - 1612FD8122A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */; }; - 1612FD8222A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */; }; - 1612FD8322A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */; }; - 1612FD8422A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */; }; 1612FD8522A9CAAB00FC1142 /* SecureEnclave.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6D22A9CAAB00FC1142 /* SecureEnclave.swift */; }; 1612FD8622A9CAAB00FC1142 /* SecureEnclave.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6D22A9CAAB00FC1142 /* SecureEnclave.swift */; }; 1612FD8722A9CAAB00FC1142 /* SecureEnclave.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1612FD6D22A9CAAB00FC1142 /* SecureEnclave.swift */; }; @@ -369,7 +365,6 @@ 1612FD0C22A9C95500FC1142 /* CloudTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CloudTests.swift; sourceTree = ""; }; 1612FD0D22A9C95500FC1142 /* ValetTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ValetTests.swift; sourceTree = ""; }; 1612FD6B22A9CAAB00FC1142 /* MigrationResult.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MigrationResult.swift; sourceTree = ""; }; - 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = KeychainQueryConvertible.swift; sourceTree = ""; }; 1612FD6D22A9CAAB00FC1142 /* SecureEnclave.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SecureEnclave.swift; sourceTree = ""; }; 1612FD6E22A9CAAB00FC1142 /* CloudAccessibility.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CloudAccessibility.swift; sourceTree = ""; }; 1612FD6F22A9CAAB00FC1142 /* Valet.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Valet.swift; sourceTree = ""; }; @@ -603,7 +598,6 @@ isa = PBXGroup; children = ( 1612FD6B22A9CAAB00FC1142 /* MigrationResult.swift */, - 1612FD6C22A9CAAB00FC1142 /* KeychainQueryConvertible.swift */, 1612FD6D22A9CAAB00FC1142 /* SecureEnclave.swift */, 1612FD6E22A9CAAB00FC1142 /* CloudAccessibility.swift */, 1612FD6F22A9CAAB00FC1142 /* Valet.swift */, @@ -1522,7 +1516,6 @@ 1612FD7F22A9CAAB00FC1142 /* MigrationResult.swift in Sources */, 1612FD9722A9CAAB00FC1142 /* Service.swift in Sources */, 1612FDA722A9CAAB00FC1142 /* ErrorHandler.swift in Sources */, - 1612FD8322A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1560,7 +1553,6 @@ 1612FD8022A9CAAB00FC1142 /* MigrationResult.swift in Sources */, 1612FD9822A9CAAB00FC1142 /* Service.swift in Sources */, 1612FDA822A9CAAB00FC1142 /* ErrorHandler.swift in Sources */, - 1612FD8422A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1592,7 +1584,6 @@ 1612FD7D22A9CAAB00FC1142 /* MigrationResult.swift in Sources */, 1612FD9522A9CAAB00FC1142 /* Service.swift in Sources */, 1612FDA522A9CAAB00FC1142 /* ErrorHandler.swift in Sources */, - 1612FD8122A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1615,7 +1606,6 @@ 1612FD7E22A9CAAB00FC1142 /* MigrationResult.swift in Sources */, 1612FD9622A9CAAB00FC1142 /* Service.swift in Sources */, 1612FDA622A9CAAB00FC1142 /* ErrorHandler.swift in Sources */, - 1612FD8222A9CAAB00FC1142 /* KeychainQueryConvertible.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; From a72acd28a69f12558cb21c1a36335437c7fd931f Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 24 Dec 2019 14:06:58 -0800 Subject: [PATCH 2/8] @testable import Valet --- Tests/ValetIntegrationTests/MacTests.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/ValetIntegrationTests/MacTests.swift b/Tests/ValetIntegrationTests/MacTests.swift index c727d827..b2f57917 100644 --- a/Tests/ValetIntegrationTests/MacTests.swift +++ b/Tests/ValetIntegrationTests/MacTests.swift @@ -20,7 +20,8 @@ import Foundation import XCTest -import Valet + +@testable import Valet #if os(macOS) class ValetMacTests: XCTestCase From 46d4b2001626ef746703f15db593ee47f52f67d0 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 24 Dec 2019 14:07:47 -0800 Subject: [PATCH 3/8] Fix test --- Tests/ValetIntegrationTests/MacTests.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Tests/ValetIntegrationTests/MacTests.swift b/Tests/ValetIntegrationTests/MacTests.swift index b2f57917..2b657940 100644 --- a/Tests/ValetIntegrationTests/MacTests.swift +++ b/Tests/ValetIntegrationTests/MacTests.swift @@ -36,7 +36,11 @@ class ValetMacTests: XCTestCase let vulnValue = "Secret" valet.removeObject(forKey: vulnKey) - var query = valet.keychainQuery + guard let keychainQuery = valet.keychainQuery else { + XCTFail() + return + } + var query = keychainQuery query[kSecAttrAccount as String] = vulnKey var accessList: SecAccess? From 8d84d2be046c021c1cac24368fb7a6e8a4ecbcea Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 24 Dec 2019 14:14:05 -0800 Subject: [PATCH 4/8] Formatting fixes --- Sources/Valet/SecureEnclaveValet.swift | 40 ++++++++++++----- .../SinglePromptSecureEnclaveValet.swift | 44 ++++++++++++++----- Sources/Valet/Valet.swift | 44 ++++++++++++++----- 3 files changed, 95 insertions(+), 33 deletions(-) diff --git a/Sources/Valet/SecureEnclaveValet.swift b/Sources/Valet/SecureEnclaveValet.swift index 10757943..2ae9884b 100644 --- a/Sources/Valet/SecureEnclaveValet.swift +++ b/Sources/Valet/SecureEnclaveValet.swift @@ -123,7 +123,9 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return SecureEnclave.set(object: object, forKey: key, options: keychainQuery) } } @@ -133,7 +135,9 @@ public final class SecureEnclaveValet: NSObject { /// - returns: The data currently stored in the keychain for the provided key. Returns `.itemNotFound` if no object exists in the keychain for the specified key, or if the keychain is inaccessible. Returns `.userCancelled` if the user cancels the user-presence prompt. public func object(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return .itemNotFound } + guard let keychainQuery = keychainQuery else { + return .itemNotFound + } return SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: keychainQuery) } } @@ -144,7 +148,9 @@ public final class SecureEnclaveValet: NSObject { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return SecureEnclave.containsObject(forKey: key, options: keychainQuery) } } @@ -156,7 +162,9 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return SecureEnclave.set(string: string, forKey: key, options: keychainQuery) } } @@ -166,7 +174,9 @@ public final class SecureEnclaveValet: NSObject { /// - returns: The string currently stored in the keychain for the provided key. Returns `itemNotFound` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. public func string(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return .itemNotFound } + guard let keychainQuery = keychainQuery else { + return .itemNotFound + } return SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: keychainQuery) } } @@ -177,7 +187,9 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.removeObject(forKey: key, options: keychainQuery).didSucceed } } @@ -188,7 +200,9 @@ public final class SecureEnclaveValet: NSObject { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.removeAllObjects(matching: keychainQuery).didSucceed } } @@ -201,7 +215,9 @@ public final class SecureEnclaveValet: NSObject { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return .couldNotReadKeychain } + guard let keychainQuery = keychainQuery else { + return .couldNotReadKeychain + } return Keychain.migrateObjects(matching: query, into: keychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -213,7 +229,9 @@ public final class SecureEnclaveValet: NSObject { /// - note: The keychain is not modified if a failure occurs. @objc(migrateObjectsFromValet:removeOnCompletion:) public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { - guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + guard let keychainQuery = valet.keychainQuery else { + return .couldNotReadKeychain + } return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } @@ -222,9 +240,9 @@ public final class SecureEnclaveValet: NSObject { internal let service: Service // MARK: Private Properties - + private let lock = NSLock() - internal var keychainQuery: [String : AnyHashable]? { + private var keychainQuery: [String : AnyHashable]? { if let keychainQuery = _keychainQuery { return keychainQuery } else { diff --git a/Sources/Valet/SinglePromptSecureEnclaveValet.swift b/Sources/Valet/SinglePromptSecureEnclaveValet.swift index 0d88f542..6304bca9 100644 --- a/Sources/Valet/SinglePromptSecureEnclaveValet.swift +++ b/Sources/Valet/SinglePromptSecureEnclaveValet.swift @@ -126,7 +126,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return false } + guard let baseKeychainQuery = baseKeychainQuery else { + return false + } return SecureEnclave.set(object: object, forKey: key, options: baseKeychainQuery) } } @@ -136,7 +138,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// - returns: The data currently stored in the keychain for the provided key. Returns `.itemNotFound` if no object exists in the keychain for the specified key, or if the keychain is inaccessible. Returns `.userCancelled` if the user cancels the user-presence prompt. public func object(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { - guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return .itemNotFound } + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { + return .itemNotFound + } return SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: continuedAuthenticationKeychainQuery) } } @@ -147,7 +151,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return false } + guard let baseKeychainQuery = baseKeychainQuery else { + return false + } return SecureEnclave.containsObject(forKey: key, options: baseKeychainQuery) } } @@ -159,7 +165,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return false } + guard let baseKeychainQuery = baseKeychainQuery else { + return false + } return SecureEnclave.set(string: string, forKey: key, options: baseKeychainQuery) } } @@ -169,7 +177,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// - returns: The string currently stored in the keychain for the provided key. Returns `itemNotFound` if no string exists in the keychain for the specified key, or if the keychain is inaccessible. public func string(forKey key: String, withPrompt userPrompt: String) -> SecureEnclave.Result { return execute(in: lock) { - guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return .itemNotFound } + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { + return .itemNotFound + } return SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: continuedAuthenticationKeychainQuery) } } @@ -188,7 +198,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(allKeysWithUserPrompt:) public func allKeys(userPrompt: String) -> Set { return execute(in: lock) { - guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { return Set() } + guard let continuedAuthenticationKeychainQuery = continuedAuthenticationKeychainQuery else { + return Set() + } var secItemQuery = continuedAuthenticationKeychainQuery if !userPrompt.isEmpty { secItemQuery[kSecUseOperationPrompt as String] = userPrompt @@ -204,7 +216,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return false } + guard let baseKeychainQuery = baseKeychainQuery else { + return false + } return Keychain.removeObject(forKey: key, options: baseKeychainQuery).didSucceed } } @@ -215,7 +229,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return false } + guard let baseKeychainQuery = baseKeychainQuery else { + return false + } return Keychain.removeAllObjects(matching: baseKeychainQuery).didSucceed } } @@ -228,7 +244,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { - guard let baseKeychainQuery = baseKeychainQuery else { return .couldNotReadKeychain } + guard let baseKeychainQuery = baseKeychainQuery else { + return .couldNotReadKeychain + } return Keychain.migrateObjects(matching: query, into: baseKeychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -240,7 +258,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// - note: The keychain is not modified if a failure occurs. @objc(migrateObjectsFromValet:removeOnCompletion:) public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { - guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + guard let keychainQuery = valet.keychainQuery else { + return .couldNotReadKeychain + } return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } @@ -268,7 +288,9 @@ public final class SinglePromptSecureEnclaveValet: NSObject { /// Using this query in a `containsObject` check can cause a false positive in the case where an element has been removed from /// the keychain by the operating system due to a face, fingerprint, or password change. private var continuedAuthenticationKeychainQuery: [String : AnyHashable]? { - guard let baseKeychainQuery = baseKeychainQuery else { return nil } + guard let baseKeychainQuery = baseKeychainQuery else { + return nil + } var keychainQuery = baseKeychainQuery keychainQuery[kSecUseAuthenticationContext as String] = localAuthenticationContext return keychainQuery diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index b8d6ada4..245eebd1 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -141,7 +141,9 @@ public final class Valet: NSObject { @objc public func canAccessKeychain() -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.canAccess(attributes: keychainQuery) } } @@ -153,7 +155,9 @@ public final class Valet: NSObject { @discardableResult public func set(object: Data, forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.set(object: object, forKey: key, options: keychainQuery).didSucceed } } @@ -163,7 +167,9 @@ public final class Valet: NSObject { @objc(objectForKey:) public func object(forKey key: String) -> Data? { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return nil } + guard let keychainQuery = keychainQuery else { + return nil + } return Keychain.object(forKey: key, options: keychainQuery).value } } @@ -173,7 +179,9 @@ public final class Valet: NSObject { @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.containsObject(forKey: key, options: keychainQuery).didSucceed } } @@ -185,7 +193,9 @@ public final class Valet: NSObject { @discardableResult public func set(string: String, forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.set(string: string, forKey: key, options: keychainQuery).didSucceed } } @@ -195,7 +205,9 @@ public final class Valet: NSObject { @objc(stringForKey:) public func string(forKey key: String) -> String? { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return nil } + guard let keychainQuery = keychainQuery else { + return nil + } return Keychain.string(forKey: key, options: keychainQuery).value } } @@ -204,7 +216,9 @@ public final class Valet: NSObject { @objc public func allKeys() -> Set { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return Set() } + guard let keychainQuery = keychainQuery else { + return Set() + } return Keychain.allKeys(options: keychainQuery).value ?? Set() } } @@ -215,7 +229,9 @@ public final class Valet: NSObject { @discardableResult public func removeObject(forKey key: String) -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.removeObject(forKey: key, options: keychainQuery).didSucceed } } @@ -226,7 +242,9 @@ public final class Valet: NSObject { @discardableResult public func removeAllObjects() -> Bool { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return false } + guard let keychainQuery = keychainQuery else { + return false + } return Keychain.removeAllObjects(matching: keychainQuery).didSucceed } } @@ -239,7 +257,9 @@ public final class Valet: NSObject { @objc(migrateObjectsMatchingQuery:removeOnCompletion:) public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult { return execute(in: lock) { - guard let keychainQuery = keychainQuery else { return .couldNotReadKeychain } + guard let keychainQuery = keychainQuery else { + return .couldNotReadKeychain + } return Keychain.migrateObjects(matching: query, into: keychainQuery, removeOnCompletion: removeOnCompletion) } } @@ -251,7 +271,9 @@ public final class Valet: NSObject { /// - note: The keychain is not modified if a failure occurs. @objc(migrateObjectsFromValet:removeOnCompletion:) public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) -> MigrationResult { - guard let keychainQuery = valet.keychainQuery else { return .couldNotReadKeychain } + guard let keychainQuery = valet.keychainQuery else { + return .couldNotReadKeychain + } return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } From 2d5b8295f4d5b600d3d0dacc032b1947e9da82dc Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Wed, 25 Dec 2019 13:25:46 -0800 Subject: [PATCH 5/8] Ensure test environment is signed before testing shared access keychain --- .../ValetBackwardsCompatibilityTests.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift index 173d745b..0612cfb8 100644 --- a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift +++ b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift @@ -96,6 +96,9 @@ class ValetBackwardsCompatibilityIntegrationTests: ValetIntegrationTests { } func test_backwardsCompatibility_withLegacySharedAccessGroupValet() { + guard testEnvironmentIsSigned() else { + return + } Valet.currentAndLegacyPermutations(with: Valet.sharedAccessGroupIdentifier, shared: true).forEach { permutation, legacyValet in legacyValet.setString(passcode, forKey: key) From 06ae396d8639594856a5833e1e9240bbd9479507 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sun, 29 Dec 2019 12:51:59 -0800 Subject: [PATCH 6/8] Update comments --- Sources/Valet/Internal/SecItem.swift | 4 +--- Sources/Valet/SecureEnclaveValet.swift | 2 +- Sources/Valet/SinglePromptSecureEnclaveValet.swift | 4 ++-- Sources/Valet/Valet.swift | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Sources/Valet/Internal/SecItem.swift b/Sources/Valet/Internal/SecItem.swift index 9bf98da1..5fa18118 100644 --- a/Sources/Valet/Internal/SecItem.swift +++ b/Sources/Valet/Internal/SecItem.swift @@ -91,7 +91,6 @@ internal final class SecItem { guard status == errSecSuccess, let queryResult = result as? [CFString : AnyHashable], let accessGroup = queryResult[kSecAttrAccessGroup] as? String else { // We may not be able to access the shared access group prefix because the accessibility of the above keychain data is set to `afterFirstUnlock`. - // In other words, we should never hit this code. This code is here as a failsafe to prevent a crash in a scenario where the keychain is entirely hosed. // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error. return nil } @@ -101,8 +100,7 @@ internal final class SecItem { return bundleSeedIdentifier } else { - // We should always be able to access the shared access group prefix because the accessibility of the above keychain data is set to `always`. - // In other words, we should never hit this code. This code is here as a failsafe to prevent a crash in a scenario where the keychain is entirely hosed. + // We may not be able to access the shared access group prefix because the accessibility of the above keychain data is set to `afterFirstUnlock`. // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error. return nil } diff --git a/Sources/Valet/SecureEnclaveValet.swift b/Sources/Valet/SecureEnclaveValet.swift index 2ae9884b..8aff9e37 100644 --- a/Sources/Valet/SecureEnclaveValet.swift +++ b/Sources/Valet/SecureEnclaveValet.swift @@ -143,7 +143,7 @@ public final class SecureEnclaveValet: NSObject { } /// - parameter key: The key to look up in the keychain. - /// - returns: `true` if a value has been set for the given key, `false` otherwise. + /// - returns: `true` if a value has been set for the given key, `false` otherwise. Will return `false` if the keychain is not accessible. /// - note: Will never prompt the user for Face ID, Touch ID, or password. @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { diff --git a/Sources/Valet/SinglePromptSecureEnclaveValet.swift b/Sources/Valet/SinglePromptSecureEnclaveValet.swift index 6304bca9..d3a28953 100644 --- a/Sources/Valet/SinglePromptSecureEnclaveValet.swift +++ b/Sources/Valet/SinglePromptSecureEnclaveValet.swift @@ -146,7 +146,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { } /// - parameter key: The key to look up in the keychain. - /// - returns: `true` if a value has been set for the given key, `false` otherwise. + /// - returns: `true` if a value has been set for the given key, `false` otherwise. Will return `false` if the keychain is not accessible. /// - note: Will never prompt the user for Face ID, Touch ID, or password. @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { @@ -194,7 +194,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject { } /// - parameter userPrompt: The prompt displayed to the user in Apple's Face ID, Touch ID, or passcode entry UI. If the `SinglePromptSecureEnclaveValet` has already been unlocked, no prompt will be shown. - /// - returns: The set of all (String) keys currently stored in this Valet instance. + /// - returns: The set of all (String) keys currently stored in this Valet instance. Will return an empty set if the keychain is not accessible. @objc(allKeysWithUserPrompt:) public func allKeys(userPrompt: String) -> Set { return execute(in: lock) { diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index 245eebd1..02a90d5b 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -175,7 +175,7 @@ public final class Valet: NSObject { } /// - parameter key: The key to look up in the keychain. - /// - returns: `true` if a value has been set for the given key, `false` otherwise. + /// - returns: `true` if a value has been set for the given key, `false` otherwise. Will return `false` if the keychain is not accessible. @objc(containsObjectForKey:) public func containsObject(forKey key: String) -> Bool { return execute(in: lock) { @@ -212,7 +212,7 @@ public final class Valet: NSObject { } } - /// - returns: The set of all (String) keys currently stored in this Valet instance. + /// - returns: The set of all (String) keys currently stored in this Valet instance. Will return an empty set if the keychain is not accessible. @objc public func allKeys() -> Set { return execute(in: lock) { From 9e6ed0063dc62a57f151378f2b1583de0a1edb22 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sun, 29 Dec 2019 13:00:02 -0800 Subject: [PATCH 7/8] Better comment --- Sources/Valet/Valet.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index 02a90d5b..afd724ee 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -264,9 +264,9 @@ public final class Valet: NSObject { } } - /// Migrates objects matching the vended keychain query into the receiving Valet instance. - /// - parameter keychain: An objects whose vended keychain query is used to retrieve existing keychain data via a call to SecItemCopyMatching. - /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychfain if the migration succeeds. + /// Migrates objects in the input Valet into the receiving Valet instance. + /// - parameter valet: A Valet whose objects should be migrated. + /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychain if the migration succeeds. /// - returns: Whether the migration succeeded or failed. /// - note: The keychain is not modified if a failure occurs. @objc(migrateObjectsFromValet:removeOnCompletion:) From ce5c0e55e6648cf23dd3cdc67d125a018c04ce09 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sun, 29 Dec 2019 13:06:26 -0800 Subject: [PATCH 8/8] Add migration helper methods --- Sources/Valet/Valet.swift | 28 + .../ValetIntegrationTests 2.swift | 738 ++++++++++++++++++ 2 files changed, 766 insertions(+) create mode 100644 Tests/ValetIntegrationTests/ValetIntegrationTests 2.swift diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index afd724ee..379245a2 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -277,6 +277,34 @@ public final class Valet: NSObject { return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) } + /// Call this method if your Valet used to have its accessibility set to `always`. + /// This method migrates objects set on a Valet with the same type and identifier, but with its accessibility set to `always` (which was possible prior to Valet 4.0) to the current Valet. + /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychain if the migration succeeds. + /// - returns: Whether the migration succeeded or failed. + /// - note: The keychain is not modified if a failure occurs. + @objc(migrateObjectsFromAlwaysAccessibleValetAndRemoveOnCompletion:) + public func migrateObjectsFromAlwaysAccessibleValet(removeOnCompletion: Bool) -> MigrationResult { + guard var keychainQuery = keychainQuery else { + return .couldNotReadKeychain + } + keychainQuery[kSecAttrAccessible as String] = "dk" // kSecAttrAccessibleAlways, but with the value hardcoded to avoid a build warning. + return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) + } + + /// Call this method if your Valet used to have its accessibility set to `alwaysThisDeviceOnly`. + /// This method migrates objects set on a Valet with the same type and identifier, but with its accessibility set to `alwaysThisDeviceOnly` (which was possible prior to Valet 4.0) to the current Valet. + /// - parameter removeOnCompletion: If `true`, the migrated data will be removed from the keychain if the migration succeeds. + /// - returns: Whether the migration succeeded or failed. + /// - note: The keychain is not modified if a failure occurs. + @objc(migrateObjectsFromAlwaysAccessibleThisDeviceOnlyValetAndRemoveOnCompletion:) + public func migrateObjectsFromAlwaysAccessibleThisDeviceOnlyValet(removeOnCompletion: Bool) -> MigrationResult { + guard var keychainQuery = keychainQuery else { + return .couldNotReadKeychain + } + keychainQuery[kSecAttrAccessible as String] = "dku" // kSecAttrAccessibleAlwaysThisDeviceOnly, but with the value hardcoded to avoid a build warning. + return migrateObjects(matching: keychainQuery, removeOnCompletion: removeOnCompletion) + } + // MARK: Internal Properties internal let configuration: Configuration diff --git a/Tests/ValetIntegrationTests/ValetIntegrationTests 2.swift b/Tests/ValetIntegrationTests/ValetIntegrationTests 2.swift new file mode 100644 index 00000000..09adbdf8 --- /dev/null +++ b/Tests/ValetIntegrationTests/ValetIntegrationTests 2.swift @@ -0,0 +1,738 @@ +// +// ValetTests.swift +// Valet +// +// Created by Eric Muller on 4/25/16. +// Copyright © 2016 Square, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +//    http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import XCTest + +@testable import Valet + + +/// - returns: `true` when the test environment is signed. +/// - The Valet Mac Tests target is left without a host app on master. Mac test host app signing requires CI to have the Developer team credentials down in keychain, which we can't easily accomplish. +/// - note: In order to test changes locally, set the Valet Mac Tests host to Valet macOS Test Host App, delete all VAL_* keychain items in your keychain via Keychain Access.app, and run Mac tests. +func testEnvironmentIsSigned() -> Bool { + // Our test host apps for iOS and Mac are both signed, so testing for a custom bundle identifier is analogous to testing signing. + guard Bundle.main.bundleIdentifier != nil && Bundle.main.bundleIdentifier != "com.apple.dt.xctest.tool" else { + #if os(iOS) || os(tvOS) + XCTFail("test bundle should be signed") + #endif + + return false + } + + if let simulatorVersionInfo = ProcessInfo.processInfo.environment["SIMULATOR_VERSION_INFO"], + simulatorVersionInfo.contains("iOS 13") || simulatorVersionInfo.contains("tvOS 13") + { + // Xcode 11's simulator does not support code-signing. + return false + } else { + return true + } +} + + +internal extension Valet { + + // MARK: Shared Access Group + + static var sharedAccessGroupIdentifier: Identifier = { + let sharedAccessGroupIdentifier: Identifier + #if os(iOS) + sharedAccessGroupIdentifier = Identifier(nonEmpty: "com.squareup.Valet-iOS-Test-Host-App")! + #elseif os(macOS) + sharedAccessGroupIdentifier = Identifier(nonEmpty: "com.squareup.Valet-macOS-Test-Host-App")! + #elseif os(tvOS) + sharedAccessGroupIdentifier = Identifier(nonEmpty: "com.squareup.Valet-tvOS-Test-Host-App")! + #elseif os(watchOS) + sharedAccessGroupIdentifier = Identifier(nonEmpty: "com.squareup.ValetTouchIDTestApp.watchkitapp.watchkitextension")! + #else + XCTFail() + #endif + return sharedAccessGroupIdentifier + }() + +} + + +class ValetIntegrationTests: XCTestCase +{ + static let identifier = Identifier(nonEmpty: "valet_testing")! + let valet = Valet.valet(with: identifier, accessibility: .whenUnlocked) + + // FIXME: Need a different flavor (Synchronizable can't be tested on Mac currently + let anotherFlavor = Valet.iCloudValet(with: identifier, accessibility: .whenUnlocked) + + let key = "key" + let passcode = "topsecret" + lazy var passcodeData: Data = { return Data(self.passcode.utf8) }() + + // MARK: XCTestCase + + override func setUp() + { + super.setUp() + + ErrorHandler.customAssertBody = { _, _, _, _ in + // Nothing to do here. + } + + valet.removeAllObjects() + anotherFlavor.removeAllObjects() + + let identifier = ValetTests.identifier + let allPermutations = Valet.permutations(with: identifier) + Valet.permutations(with: identifier, shared: true) + allPermutations.forEach { testingValet in testingValet.removeAllObjects() } + + XCTAssertTrue(valet.allKeys().isEmpty) + XCTAssertTrue(anotherFlavor.allKeys().isEmpty) + } + + // MARK: Initialization + + func test_init_createsCorrectBackingService() { + let identifier = ValetTests.identifier + + Accessibility.allValues().forEach { accessibility in + let backingService = Valet.valet(with: identifier, accessibility: accessibility).service + XCTAssertEqual(backingService, Service.standard(identifier, .valet(accessibility))) + } + } + + func test_init_createsCorrectBackingService_sharedAccess() { + let identifier = ValetTests.identifier + + Accessibility.allValues().forEach { accessibility in + let backingService = Valet.sharedAccessGroupValet(with: identifier, accessibility: accessibility).service + XCTAssertEqual(backingService, Service.sharedAccessGroup(identifier, .valet(accessibility))) + } + } + + func test_init_createsCorrectBackingService_cloud() { + let identifier = ValetTests.identifier + + CloudAccessibility.allValues().forEach { accessibility in + let backingService = Valet.iCloudValet(with: identifier, accessibility: accessibility).service + XCTAssertEqual(backingService, Service.standard(identifier, .iCloud(accessibility))) + } + } + + func test_init_createsCorrectBackingService_cloudSharedAccess() { + let identifier = ValetTests.identifier + + CloudAccessibility.allValues().forEach { accessibility in + let backingService = Valet.iCloudSharedAccessGroupValet(with: identifier, accessibility: accessibility).service + XCTAssertEqual(backingService, Service.sharedAccessGroup(identifier, .iCloud(accessibility))) + } + } + + // MARK: Equality + + func test_valetsWithSameConfiguration_areEqual() + { + let equalValet = Valet.valet(with: valet.identifier, accessibility: valet.accessibility) + XCTAssertTrue(equalValet == valet) + XCTAssertTrue(equalValet === valet) + } + + func test_differentValetFlavorsWithEquivalentConfiguration_areNotEqual() + { + XCTAssertFalse(valet == anotherFlavor) + XCTAssertFalse(valet === anotherFlavor) + } + + func test_valetsWithDifferingIdentifier_areNotEqual() + { + let differingIdentifier = Valet.valet(with: Identifier(nonEmpty: "nope")!, accessibility: valet.accessibility) + XCTAssertNotEqual(valet, differingIdentifier) + } + + func test_valetsWithDifferingAccessibility_areNotEqual() + { + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + XCTAssertNotEqual(valet, differingAccessibility) + } + + // MARK: canAccessKeychain + + func test_canAccessKeychain() + { + Valet.permutations(with: valet.identifier).forEach { permutation in + XCTAssertTrue(permutation.canAccessKeychain(), "\(permutation) could not access keychain.") + } + } + + func test_canAccessKeychain_sharedAccessGroup() + { + guard testEnvironmentIsSigned() else { + return + } + + Valet.permutations(with: Valet.sharedAccessGroupIdentifier, shared: true).forEach { permutation in + XCTAssertTrue(permutation.canAccessKeychain(), "\(permutation) could not access keychain.") + } + } + + func test_canAccessKeychain_Performance() + { + measure { + _ = self.valet.canAccessKeychain() + } + } + + // MARK: containsObjectForKey + + func test_containsObjectForKey() + { + XCTAssertFalse(valet.containsObject(forKey: key)) + + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertTrue(valet.containsObject(forKey: key)) + + XCTAssertTrue(valet.removeObject(forKey: key)) + XCTAssertFalse(valet.containsObject(forKey: key)) + } + + // MARK: allKeys + + func test_allKeys() + { + XCTAssertEqual(valet.allKeys(), Set()) + + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertEqual(valet.allKeys(), Set(arrayLiteral: key)) + + XCTAssertTrue(valet.set(string: "monster", forKey: "cookie")) + XCTAssertEqual(valet.allKeys(), Set(arrayLiteral: key, "cookie")) + + valet.removeAllObjects() + XCTAssertEqual(valet.allKeys(), Set()) + } + + func test_allKeys_doesNotReflectValetImplementationDetails() { + // Under the hood, Valet inserts a canary when calling `canAccessKeychain()` - this should not appear in `allKeys()`. + _ = valet.canAccessKeychain() + XCTAssertEqual(valet.allKeys(), Set()) + } + + func test_allKeys_remainsUntouchedForUnequalValets() + { + valet.set(string: passcode, forKey: key) + XCTAssertEqual(valet.allKeys(), Set(arrayLiteral: key)) + + // Different Identifier + let differingIdentifier = Valet.valet(with: Identifier(nonEmpty: "nope")!, accessibility: valet.accessibility) + XCTAssertEqual(differingIdentifier.allKeys(), Set()) + + // Different Accessibility + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + XCTAssertEqual(differingAccessibility.allKeys(), Set()) + + // Different Kind + XCTAssertEqual(anotherFlavor.allKeys(), Set()) + } + + // MARK: string(forKey:) + + func test_stringForKey_isNilForInvalidKey() + { + XCTAssertNil(valet.string(forKey: key)) + } + + func test_stringForKey_retrievesStringForValidKey() + { + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertEqual(passcode, valet.string(forKey: key)) + } + + func test_stringForKey_equivalentValetsCanAccessSameData() + { + let equalValet = Valet.valet(with: valet.identifier, accessibility: valet.accessibility) + XCTAssertEqual(0, equalValet.allKeys().count) + XCTAssertEqual(valet, equalValet) + XCTAssertTrue(valet.set(string: "monster", forKey: "cookie")) + XCTAssertEqual("monster", equalValet.string(forKey: "cookie")) + } + + func test_stringForKey_withDifferingIdentifier_isNil() + { + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertEqual(passcode, valet.string(forKey: key)) + + let differingIdentifier = Valet.valet(with: Identifier(nonEmpty: "wat")!, accessibility: valet.accessibility) + XCTAssertNil(differingIdentifier.string(forKey: key)) + } + + func test_stringForKey_withDifferingAccessibility_isNil() + { + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertEqual(passcode, valet.string(forKey: key)) + + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .afterFirstUnlockThisDeviceOnly) + XCTAssertNil(differingAccessibility.string(forKey: key)) + } + + func test_stringForKey_withEquivalentConfigurationButDifferingFlavor_isNil() + { + guard testEnvironmentIsSigned() else { + return + } + + XCTAssertTrue(valet.set(string: "monster", forKey: "cookie")) + XCTAssertEqual("monster", valet.string(forKey: "cookie")) + + XCTAssertNil(anotherFlavor.string(forKey: "cookie")) + } + + // MARK: set(string:forKey:) + + func test_setStringForKey_successfullyUpdatesExistingKey() + { + XCTAssertNil(valet.string(forKey: key)) + valet.set(string: "1", forKey: key) + XCTAssertEqual("1", valet.string(forKey: key)) + valet.set(string: "2", forKey: key) + XCTAssertEqual("2", valet.string(forKey: key)) + } + + func test_setStringForKey_failsForInvalidValue() { + XCTAssertFalse(valet.set(string: "", forKey: key)) + } + + func test_setStringForKey_failsForInvalidKey() { + XCTAssertFalse(valet.set(string: passcode, forKey: "")) + } + + // MARK: object(forKey:) + + func test_objectForKey_isNilForInvalidKey() { + XCTAssertNil(valet.object(forKey: key)) + } + + func test_objectForKey_succeedsForValidKey() { + valet.set(object: passcodeData, forKey: key) + XCTAssertEqual(passcodeData, valet.object(forKey: key)) + } + + func test_objectForKey_equivalentValetsCanAccessSameData() { + let equalValet = Valet.valet(with: valet.identifier, accessibility: valet.accessibility) + XCTAssertEqual(0, equalValet.allKeys().count) + XCTAssertEqual(valet, equalValet) + XCTAssertTrue(valet.set(object: passcodeData, forKey: key)) + XCTAssertEqual(passcodeData, equalValet.object(forKey: key)) + } + + func test_objectForKey_withDifferingIdentifier_isNil() { + XCTAssertTrue(valet.set(object: passcodeData, forKey: key)) + XCTAssertEqual(passcodeData, valet.object(forKey: key)) + + let differingIdentifier = Valet.valet(with: Identifier(nonEmpty: "wat")!, accessibility: valet.accessibility) + XCTAssertNil(differingIdentifier.object(forKey: key)) + } + + func test_objectForKey_withDifferingAccessibility_isNil() { + XCTAssertTrue(valet.set(object: passcodeData, forKey: key)) + XCTAssertEqual(passcodeData, valet.object(forKey: key)) + + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .afterFirstUnlockThisDeviceOnly) + XCTAssertNil(differingAccessibility.object(forKey: key)) + } + + func test_objectForKey_withEquivalentConfigurationButDifferingFlavor_isNil() { + guard testEnvironmentIsSigned() else { + return + } + + XCTAssertTrue(valet.set(object: passcodeData, forKey: key)) + XCTAssertEqual(passcodeData, valet.object(forKey: key)) + + XCTAssertNil(anotherFlavor.object(forKey: key)) + } + + // MARK: set(object:forKey:) + + func test_setObjectForKey_successfullyUpdatesExistingKey() { + let firstValue = Data("first".utf8) + let secondValue = Data("second".utf8) + valet.set(object: firstValue, forKey: key) + XCTAssertEqual(firstValue, valet.object(forKey: key)) + valet.set(object: secondValue, forKey: key) + XCTAssertEqual(secondValue, valet.object(forKey: key)) + } + + func test_setObjectForKey_failsForInvalidKey() { + XCTAssertFalse(valet.set(object: passcodeData, forKey: "")) + } + + func test_setObjectForKey_failsForEmptyData() { + let emptyData = Data() + XCTAssertTrue(emptyData.isEmpty) + XCTAssertFalse(valet.set(object: emptyData, forKey: key)) + } + + // Mark: String/Object Equivalence + + func test_stringForKey_succeedsForDataBackedByString() { + XCTAssertTrue(valet.set(object: passcodeData, forKey: key)) + XCTAssertEqual(passcode, valet.string(forKey: key)) + } + + func test_stringForKey_failsForDataNotBackedByString() { + let dictionary = [ "that's no" : "moon" ] + let nonStringData = NSKeyedArchiver.archivedData(withRootObject: dictionary) + XCTAssertTrue(valet.set(object: nonStringData, forKey: key)) + XCTAssertNil(valet.string(forKey: key)) + } + + func test_objectForKey_succeedsForStrings() { + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertEqual(passcodeData, valet.object(forKey: key)) + } + + // MARK: Concurrency + + func test_concurrentSetAndRemoveOperations() + { + let setQueue = DispatchQueue(label: "Set String Queue", attributes: .concurrent) + let removeQueue = DispatchQueue(label: "Remove Object Queue", attributes: .concurrent) + + for _ in 1...50 { + setQueue.async { XCTAssertTrue(self.valet.set(string: self.passcode, forKey: self.key)) } + removeQueue.async { XCTAssertTrue(self.valet.removeObject(forKey: self.key)) } + } + + let setQueueExpectation = expectation(description: "\(#function): Set String Queue") + let removeQueueExpectation = expectation(description: "\(#function): Remove String Queue") + + setQueue.async(flags: .barrier) { + setQueueExpectation.fulfill() + } + removeQueue.async(flags: .barrier) { + removeQueueExpectation.fulfill() + } + + waitForExpectations(timeout: 10.0, handler: nil) + } + + func test_stringForKey_canReadDataWrittenOnAnotherThread() + { + let setStringQueue = DispatchQueue(label: "Set String Queue", attributes: .concurrent) + let stringForKeyQueue = DispatchQueue(label: "String For Key Queue", attributes: .concurrent) + + let expectation = self.expectation(description: #function) + + setStringQueue.async { + XCTAssertTrue(self.valet.set(string: self.passcode, forKey: self.key)) + stringForKeyQueue.async { + XCTAssertEqual(self.valet.string(forKey: self.key), self.passcode) + expectation.fulfill() + } + } + + waitForExpectations(timeout: 5.0, handler: nil) + } + + func test_stringForKey_canReadDataWrittenToValetAllocatedOnDifferentThread() + { + let setStringQueue = DispatchQueue(label: "Set String Queue", attributes: .concurrent) + let stringForKeyQueue = DispatchQueue(label: "String For Key Queue", attributes: .concurrent) + + let backgroundIdentifier = Identifier(nonEmpty: "valet_background_testing")! + let expectation = self.expectation(description: #function) + + setStringQueue.async { + let backgroundValet = Valet.valet(with: backgroundIdentifier, accessibility: .whenUnlocked) + XCTAssertTrue(backgroundValet.set(string: self.passcode, forKey: self.key)) + stringForKeyQueue.async { + XCTAssertEqual(backgroundValet.string(forKey: self.key), self.passcode) + expectation.fulfill() + } + } + + waitForExpectations(timeout: 5.0, handler: nil) + } + + // MARK: Removal + + func test_removeObjectForKey_succeedsWhenKeyIsNotPresent() + { + XCTAssertTrue(valet.removeObject(forKey: "derp")) + } + + func test_removeObjectForKey_succeedsWhenKeyIsPresent() + { + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertTrue(valet.removeObject(forKey: key)) + XCTAssertNil(valet.string(forKey: key)) + } + + func test_removeObjectForKey_isDistinctForDifferingAccessibility() + { + let differingAccessibility = Valet.valet(with: valet.identifier, accessibility: .always) + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + + XCTAssertTrue(differingAccessibility.removeObject(forKey: key)) + + XCTAssertEqual(passcode, valet.string(forKey: key)) + } + + func test_removeObjectForKey_isDistinctForDifferingIdentifier() + { + let differingIdentifier = Valet.valet(with: Identifier(nonEmpty: "no")!, accessibility: valet.accessibility) + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + + XCTAssertTrue(differingIdentifier.removeObject(forKey: key)) + + XCTAssertEqual(passcode, valet.string(forKey: key)) + } + + func test_removeObjectForKey_isDistinctForDifferingClasses() + { + guard testEnvironmentIsSigned() else { + return + } + + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertTrue(anotherFlavor.set(string: passcode, forKey: key)) + + XCTAssertTrue(valet.removeObject(forKey: key)) + + XCTAssertNil(valet.string(forKey: key)) + XCTAssertEqual(passcode, anotherFlavor.string(forKey: key)) + } + + // MARK: Migration - Query + + func test_migrateObjectsMatching_failsIfQueryHasNoInputClass() + { + guard testEnvironmentIsSigned() else { + return + } + + valet.set(string: passcode, forKey: key) + + // Test for base query success. + XCTAssertEqual(anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: false), .success) + XCTAssertEqual(passcode, anotherFlavor.string(forKey: key)) + + var mutableQuery = valet.keychainQuery + mutableQuery.removeValue(forKey: kSecClass as String) + + // Without a kSecClass, the migration should fail. + XCTAssertEqual(.invalidQuery, anotherFlavor.migrateObjects(matching: mutableQuery, removeOnCompletion: false)) + + mutableQuery[kSecClass as String] = kSecClassInternetPassword + // Without a kSecClass set to something other than kSecClassGenericPassword, the migration should fail. + XCTAssertEqual(.invalidQuery, anotherFlavor.migrateObjects(matching: mutableQuery, removeOnCompletion: false)) + } + + func test_migrateObjectsMatching_failsIfNoItemsMatchQuery() + { + guard testEnvironmentIsSigned() else { + return + } + + let noItemsFoundError = MigrationResult.noItemsToMigrateFound + + let queryWithNoMatches = [ + kSecClass as String: kSecClassGenericPassword as String, + kSecAttrService as String: "Valet_Does_Not_Exist" + ] + + XCTAssertEqual(noItemsFoundError, valet.migrateObjects(matching: queryWithNoMatches, removeOnCompletion: false)) + XCTAssertEqual(noItemsFoundError, valet.migrateObjects(matching: queryWithNoMatches, removeOnCompletion: true)) + + // Our test Valet has not yet been written to, migration should fail: + XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: false)) + XCTAssertEqual(noItemsFoundError, anotherFlavor.migrateObjects(matching: valet.keychainQuery, removeOnCompletion: true)) + } + + // FIXME: Looks to me like this test may no longer be valid, need to dig a bit + func disabled_test_migrateObjectsMatching_bailsOutIfConflictExistsInQueryResult() + { + guard testEnvironmentIsSigned() else { + return + } + + let migrationValet = Valet.valet(with: Identifier(nonEmpty: "Migrate_Me")!, accessibility: .afterFirstUnlock) + migrationValet.removeAllObjects() + + XCTAssertTrue(valet.set(string: passcode, forKey: key)) + XCTAssertTrue(anotherFlavor.set(string: passcode, forKey:key)) + + let conflictingQuery = [ + kSecClass as String: kSecClassGenericPassword as String, + kSecAttrAccount as String: key + ] + + XCTAssertEqual(.duplicateKeyInQueryResult, migrationValet.migrateObjects(matching: conflictingQuery, removeOnCompletion: false)) + } + + func test_migrateObjectsMatching_withAccountNameAsData_doesNotRaiseException() + { + let identifier = "Keychain_With_Account_Name_As_NSData" + + // kSecAttrAccount entry is expected to be a CFString, but a CFDataRef can also be stored as a value. + let keychainData = [ + kSecAttrService: identifier, + kSecClass : kSecClassGenericPassword, + kSecAttrAccount: passcodeData, + kSecValueData: passcodeData + ] as CFDictionary + + SecItemDelete(keychainData) + let status = SecItemAdd(keychainData, nil) + XCTAssertEqual(status, errSecSuccess) + + let query: [String : AnyHashable] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: identifier + ] + let migrationResult = valet.migrateObjects(matching: query, removeOnCompletion: false) + + XCTAssertEqual(migrationResult, .keyInQueryResultInvalid) + } + + // MARK: Migration - Valet + + func test_migrateObjectsFromValet_migratesSingleKeyValuePairSuccessfully() + { + guard testEnvironmentIsSigned() else { + return + } + + anotherFlavor.set(string: "foo", forKey: "bar") + _ = valet.migrateObjects(from: anotherFlavor, removeOnCompletion: false) + _ = valet.allKeys() + XCTAssertEqual("foo", valet.string(forKey: "bar")) + } + + func test_migrateObjectsFromValet_migratesMultipleKeyValuePairsSuccessfully() + { + guard testEnvironmentIsSigned() else { + return + } + + let keyValuePairs = [ + "yo": "dawg", + "we": "heard", + "you": "like", + "migrating": "to", + "other": "valets" + ] + + for (key, value) in keyValuePairs { + anotherFlavor.set(string: value, forKey: key) + } + + XCTAssertEqual(valet.migrateObjects(from: anotherFlavor, removeOnCompletion: false), .success) + + // Both the migration target and the previous Valet should hold all key/value pairs. + XCTAssertEqual(valet.allKeys(), anotherFlavor.allKeys()) + for (key, value) in keyValuePairs { + XCTAssertEqual(valet.string(forKey: key), value) + XCTAssertEqual(anotherFlavor.string(forKey: key), value) + } + } + + func test_migrateObjectsFromValet_removesOnCompletionWhenRequested() + { + guard testEnvironmentIsSigned() else { + return + } + + let keyValuePairs = [ + "yo": "dawg", + "we": "heard", + "you": "like", + "migrating": "to", + "other": "valets" + ] + + for (key, value) in keyValuePairs { + anotherFlavor.set(string: value, forKey: key) + } + + XCTAssertEqual(valet.migrateObjects(from: anotherFlavor, removeOnCompletion: true), .success) + + // The migration target should hold all key/value pairs, the previous Valet should be empty. + XCTAssertEqual(0, anotherFlavor.allKeys().count) + for (key, value) in keyValuePairs { + XCTAssertEqual(valet.string(forKey: key), value) + XCTAssertNil(anotherFlavor.string(forKey: key)) + } + } + + func test_migrateObjectsFromValet_leavesKeychainUntouchedWhenConflictsExist() + { + guard testEnvironmentIsSigned() else { + return + } + + let keyValuePairs = [ + "yo": "dawg", + "we": "heard", + "you": "like", + "migrating": "to", + "other": "valets" + ] + + for (key, value) in keyValuePairs { + anotherFlavor.set(string: value, forKey: key) + } + + valet.set(string: "adrian", forKey: "yo") + + XCTAssertEqual(1, valet.allKeys().count) + XCTAssertEqual(keyValuePairs.count, anotherFlavor.allKeys().count) + + XCTAssertEqual(.keyInQueryResultAlreadyExistsInValet, valet.migrateObjects(from: anotherFlavor, removeOnCompletion: true)) + + // Neither Valet should have seen any changes. + XCTAssertEqual(1, valet.allKeys().count) + XCTAssertEqual(keyValuePairs.count, anotherFlavor.allKeys().count) + + XCTAssertEqual("adrian", valet.string(forKey: "yo")) + for (key, value) in keyValuePairs { + XCTAssertEqual(anotherFlavor.string(forKey: key), value) + } + } + + func test_migrateObjectsFromValetRemoveOnCompletion_migratesDataSuccessfullyWhenBothValetsHavePreviouslyCalled_canAccessKeychain() { + let otherValet = Valet.valet(with: Identifier(nonEmpty: "Migrate_Me_To_Valet")!, accessibility: .afterFirstUnlock) + + // Clean up any dangling keychain items before we start this test. + otherValet.removeAllObjects() + + let keyStringPairToMigrateMap = ["foo" : "bar", "testing" : "migration", "is" : "quite", "entertaining" : "if", "you" : "don't", "screw" : "up"] + for (key, value) in keyStringPairToMigrateMap { + XCTAssertTrue(otherValet.set(string: value, forKey: key)) + } + + XCTAssertTrue(valet.canAccessKeychain()) + XCTAssertTrue(otherValet.canAccessKeychain()) + XCTAssertEqual(valet.migrateObjects(from: otherValet, removeOnCompletion: false), .success) + + for (key, value) in keyStringPairToMigrateMap { + XCTAssertEqual(valet.string(forKey: key), value ) + XCTAssertEqual(otherValet.string(forKey: key), value) + } + } + +}