Skip to content
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

Ensure token item and metadata keychain accessibility settings are consistent. #200

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

mikenachbaur-okta
Copy link
Contributor

This addresses #198 by:

  1. Ensuring the token metadata is stored with the appropriate accessibility settings, respecting the parent item's iCloud sync setting.
  2. Returning token IDs only if both their item and metadata keychain entries can be found.
  3. Establishing the Credential.Security.isDefaultSynchronizable property, allowing the default identifier to be shared.

This also adds unit test coverage to validate these changes.

…nsistent.

This addresses #198 by:

1. Ensuring the token metadata is stored with the appropriate accessibility settings, respecting the parent item's iCloud sync setting.
2. Returning token IDs only if both their item and metadata keychain entries can be found.
3. Establishing the `Credential.Security.isDefaultSynchronizable` property, allowing the default identifier to be shared.

This also adds unit test coverage to validate these changes.
Comment on lines -49 to +58
return try Keychain
let itemIDs = try Keychain
.Search(service: KeychainTokenStorage.serviceName)
.list()
.sorted(by: { $0.creationDate < $1.creationDate })
.map(\.account)
let metadataIDs = try Keychain
.Search(service: KeychainTokenStorage.metadataName)
.list()
.map(\.account)
return itemIDs.filter { metadataIDs.contains($0) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this prevent the find function from short circuiting for users already missing items/tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baksha97 correct, this ensures that only tokens that have both their metadata and item keychain entries available to the device will be returned. Therefore, the find and other functions will succeed.

This will still allow legitimate errors (such as an unsuccessful biometric prompt) to flow through.

@mikenachbaur-okta mikenachbaur-okta merged commit 15a3286 into master Jul 18, 2024
8 checks passed
@mikenachbaur-okta mikenachbaur-okta deleted the tokenstorage-metadata branch July 18, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants