-
Notifications
You must be signed in to change notification settings - Fork 224
[Valet 4.0] Remove .always accessibility specifier #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
dfed
merged 8 commits into
develop--4.0
from
dfed--remove-always-accessibility-specifier
Dec 30, 2019
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
149c8f4
Remove Always accessibility specifier
dfed a72acd2
@testable import Valet
dfed 46d4b20
Fix test
dfed 8d84d2b
Formatting fixes
dfed 2d5b829
Ensure test environment is signed before testing shared access keychain
dfed 06ae396
Update comments
dfed 9e6ed00
Better comment
dfed ce5c0e5
Add migration helper methods
dfed File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,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 | ||
| } | ||
| return SecureEnclave.set(object: object, forKey: key, options: keychainQuery) | ||
| } | ||
| } | ||
|
|
@@ -125,16 +135,22 @@ 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<Data> { | ||
| return execute(in: lock) { | ||
| guard let keychainQuery = keychainQuery else { | ||
| return .itemNotFound | ||
| } | ||
| return SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: keychainQuery) | ||
| } | ||
| } | ||
|
|
||
| /// - 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 { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should note in the header doc that it will return false if the keychain isn't accessible. |
||
| return execute(in: lock) { | ||
| guard let keychainQuery = keychainQuery else { | ||
| return false | ||
| } | ||
| return SecureEnclave.containsObject(forKey: key, options: keychainQuery) | ||
| } | ||
| } | ||
|
|
@@ -146,15 +162,21 @@ 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<String> { | ||
| return execute(in: lock) { | ||
| guard let keychainQuery = keychainQuery else { | ||
| return .itemNotFound | ||
| } | ||
| return SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: keychainQuery) | ||
| } | ||
| } | ||
|
|
@@ -165,6 +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 | ||
| } | ||
| return Keychain.removeObject(forKey: key, options: keychainQuery).didSucceed | ||
| } | ||
| } | ||
|
|
@@ -175,6 +200,9 @@ 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 +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 | ||
| } | ||
| return Keychain.migrateObjects(matching: query, into: keychainQuery, removeOnCompletion: removeOnCompletion) | ||
| } | ||
| } | ||
|
|
@@ -196,19 +227,31 @@ 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 | ||
|
|
||
| internal let service: Service | ||
|
|
||
| // MARK: Private Properties | ||
|
|
||
| private let lock = NSLock() | ||
| private let keychainQuery: [String : AnyHashable] | ||
| private var keychainQuery: [String : AnyHashable]? { | ||
| if let keychainQuery = _keychainQuery { | ||
| return keychainQuery | ||
| } else { | ||
| _keychainQuery = service.generateBaseQuery() | ||
| return _keychainQuery | ||
| } | ||
| } | ||
|
|
||
| private var _keychainQuery: [String : AnyHashable]? | ||
| } | ||
|
|
||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a new result case for
.keychainNotAccessible? (applies throughout diff)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per the comment above:
So we're doing the right thing per the comment. Adding a new case here would be a breaking change. The Valet 4.0 code will
throwthe right error (couldNotAccessKeychain) in this case assuming #198 lands