Fix keychain key bug#237
Merged
Merged
Conversation
This change fixes a bug where the String representation of these keys were supplied to the keychain query as opposed to the actual key names.
…nd watchOS as well
This check isn't needed since the enum case already has restricted availability.
Collaborator
Author
|
Hi @olvrlrnz please take a look and help us to make sure that this change meets the need. Thanks! |
yalkhder
requested changes
Jan 26, 2024
|
Just tried it in our code base, LGTM |
olvrlrnz
approved these changes
Jan 30, 2024
Also update use of kSecAttrAccessible on macOS to only occur if also using kSecUseDataProtectionKeychain per Apple docs: https://developer.apple.com/documentation/security/ksecattraccessible\?language\=objc
The other existing methods either take an accessibility param or an optional param.
yalkhder
approved these changes
Jan 31, 2024
Collaborator
Author
|
@olvrlrnz Sorry for bugging you again, but I made some changes since you last looked. Would you mind double checking for me? 🙏 |
olvrlrnz
reviewed
Feb 1, 2024
olvrlrnz
reviewed
Feb 1, 2024
olvrlrnz
reviewed
Feb 1, 2024
olvrlrnz
approved these changes
Feb 2, 2024
yalkhder
approved these changes
Feb 5, 2024
olvrlrnz
approved these changes
Feb 5, 2024
|
Thanks for fixing this. Would it be possible to create a 4.0.1 release so we can update the package in Xcode? |
Collaborator
Author
|
Absolutely. We need to figure out what version we call this. In any case, we will be making a new release to include a privacy manifest very shortly. I think that will come out in the next few weeks. Stay tuned for that, and sorry for the delay. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR addresses a bug where the key attributes used to query the keychain were not being added correctly, and where therefore unused. The mistake was that the key names were assumed to match the static constants.
Fixes #236.