[Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS #209

dfed · 2020-01-19T16:28:12Z

This PR is a follow-up to #191. I enabled SinglePromptSecureEnclave for the tvOS platform without enabling the tests on that platform.

Note that XCTest doesn't handle mixed availability well – it doesn't respect @available flags. So in order to prevent a warning and stop these tests crashing on tvOS 9, I had to turn the let valet: SinglePromptSecureEnclaveValet into func valet() -> SinglePromptSecureEnclaveValet

I did not include SinglePromptSecureEnclaveBackwardsCompatibilityTests in the tvOS target, since tvOS was not supported in Valet 2.0.

…S 10.

dfed · 2020-01-20T16:57:22Z

So, the tests have revealed that allKeys doesn't seem to work on a SinglePromptSecureEnclave on tvOS 10. The query returns a -26276, which isn't anywhere to be found in Apple's header files. According to secureblackbox.com's list of error codes, this code represents errSecInternal: An internal error occured in the Security framework`.

I have no idea why including an LAContext in a query that returns multiple objects would fail only on tvOS 10. My simple solution is to make that API available only on the operating systems we know it works. In f6d41b8 I've made only this API available in tvOS 11+. Does that seem reasonable? Or should we instead make the entire SinglePromptSecureEnclave only available on tvOS 11+?

dfed · 2020-01-20T18:00:36Z

Nevermind. It looks like copying anything out of the keychain on tvOS 10 fails with errSecInternal when including a LAContext in the parameters. I'll make the whole class minimum tvOS 11.

codecov-io · 2020-01-20T18:22:44Z

Codecov Report

Merging #209 into develop--4.0 will increase coverage by 0.36%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           develop--4.0     #209      +/-   ##
================================================
+ Coverage         84.41%   84.77%   +0.36%     
================================================
  Files                14       14              
  Lines              1110     1110              
================================================
+ Hits                937      941       +4     
+ Misses              173      169       -4

Impacted Files	Coverage Δ
Sources/Valet/CloudAccessibility.swift	`100% <0%> (+26.66%)`	⬆️

…ntegrationTests-on-tvOS

fdiaz · 2020-01-23T20:21:07Z

Tests/ValetIntegrationTests/SinglePromptSecureEnclaveIntegrationTests.swift

-    let valet = SinglePromptSecureEnclaveValet.valet(with: SinglePromptSecureEnclaveTests.identifier, accessControl: .userPresence)
+
+    @available(tvOS 11.0, *)
+    func valet() -> SinglePromptSecureEnclaveValet {


In the PR comment you mentioned that this was done for tvOS 9 support. Is this still needed now that this API is only available on tvOS 11+?

See this comment for clarification on why I’m making the minimum target tvOS 11: #209 (comment)

* Do not build branch build on every push on PR branches * Update cocoapods * Start validating podspec on Xcode 11 * Drop Xcode 9 and 10 support * Swift version to 5.0 * Bump minor version of osx_image on .travis.yml in order to access simulators for older OSes * Bump destination for iOS 11 to get CI working * Update tests to reflext iOS 13 simulator's inability to store items that require a passcode to be set * Enable running CI on iOS 10, tvOS 10, and watchOS 3 * Allow kSecAttrService to be a customer-friendly string on Mac * Add a section to the README on choosing the best identifier on a Mac * Update README documentation on choosing a user-friendly identifier * Add ObjC compat layer for new initializers * Remove Always accessibility specifier * Ensure test environment is signed before testing shared access keychain * Add migration helper methods * Use throws rather than return types to indicate error. * Get rid of ErrorHandler * couldNotReadKeychain -> .couldNotAccessKeychain * Make Objective-C bridging methods for accessing values with prompt redundant * Swift 5 updates * Bring API in line with Apple's naming guidelines * Adopt Swift 5 syntax, and enable support for SinglePromptSecureEnclaveValet on tvOS * Fix migrateObjectsFromAlwaysAccessible methods * Fix warning introduced by merge * Bump version to 4.0.0 * Get new migration methods working with Catalina * Update README * Set up code coverage * Run more tests on a single machine * Standardize method naming * Run test coverage on every target * Modernize doc comments * Remove returns * Use SeeAlso * Modernize doc comments * Add Warning * Add objc example * findOrCreate(explicitlySet must use a key that combines service, configuration, accessibility, and sharedAccessGroup to prevent returning the wrong Valet * Utilize testEnvironmentIsSigned before using shared keychains * Use explicitlySetSharedAccessGroupIdentifier when dealing with shared access groups * removeAllObjects() to avoid collisions in tests * Fail test on setup failure * Update whitespace * containsObject(forKey should throw in Swift * Introduce Throws doc comment * Catch closer to the source * Add simple Objective-C compatibility layer tests * Update copyright * Fixup whitespace * try? less in tests * If deleting items throws, then we should surface the failure. * Remove runtime assert, since we will throw the error anyways * Update Mac tests to use try on containsObject * Add description to KeychainError * Fix macOS tests after throwing on removeAllObjects in setUp * Increase test coverage of error files * Use permutation valet rather than vanillaValet multiple times * Indentation and test separation * Add comment re why we're checking for errSecInteractionNotAllowed * Better comment formatting * Use Throws rather than Note * Add final to test classes * Rename internal containsObject methods to performCopy * [Valet 4.0] Add explicit tests for CloudAccessibility (#210) * [Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS (#209) * Get SinglePromptSecureEnclaveIntegrationTests running on tvOS * Make SinglePromptSecureEnclaveValet available on tvOS 11, not tvOS 10 * [Valet 4.0] Add explicit tests for Configuration (#211) * Use CaseIterable instead of allValues where possible (#212) * Get ValetTouchIDTest building again * Require that App ID Prefix be explicitly passed into Shared Access Group Valets (#218) * Require that App ID Prefix be explicitly passed into Shared Access Group Valets * Add App ID prefix to tests * Create and adopt SharedAccessGroupIdentifier * Update documentation * [Valet 4.0] Update migration guide (#221) * Update migration guide for Valet 4.0 * NickEntin feedback Co-Authored-By: Nick Entin <nckentn@gmail.com> * Remove version from Package.swift (#223) * Add headerdoc comment for removing an object from the keychain * Update headerdoc comments for parameters of type SharedAccessGroupIdentifier * Update headerdoc comment for migration method * Update headerdoc comments for objc compatibility methods * Rename MigrationError cases with `InQueryResult` to `ToMigrate` (#227) * Rename InQueryResult -> ToMigrate * Update comments * Fix typo in README (#229) * Create 'Changing an Accessibility Value After Persisting Data' section in README (#232) * Use correct Valet name in README example * Create Changing an Accessibility Value After Persisting Data section * Get watchOS tests running locally (#233) * Support sharing keychain items using App Groups (#230) * Add App Group group.valet.test * Update syntax for Swift 5 * Enable SharedAccessGroup code to semantically handle AppGroups. Rename SharedAccessGroup -> SharedGroup * README updates * Add sharedAppGroupIdentifier tests to Valet * Add sharedAppGroupIdentifier test to SecureEnclave * Add sharedAppGroupIdentifier test to SinglePromptSecureEnclave * Add objective-c compatibility layer

Get SinglePromptSecureEnclaveIntegrationTests running on tvOS

47f2c6e

dfed requested review from fdiaz and NickEntin January 19, 2020 16:28

dfed added 2 commits January 19, 2020 10:07

Update backwards compatibility tests

4291466

Make allKeys only available on tvOS 11+, since it doesn't work on tvO…

f6d41b8

…S 10.

Fix allKeys test for tvOS 11

ba950bb

Make SinglePromptSecureEnclaveValet available on tvOS 11, not tvOS 10

a5a851c

dfed and others added 5 commits January 20, 2020 19:11

Remove duplicative @available

45e7ebb

Merge branch 'develop--4.0' into dfed--run-SinglePromptSecureEnclaveI…

08cbf57

…ntegrationTests-on-tvOS

Add availability test to ObjectiveC bridge tests

7b0bb13

Fix tests

7124c02

Merge branch 'develop--4.0' into dfed--run-SinglePromptSecureEnclaveI…

8a4d3ae

…ntegrationTests-on-tvOS

fdiaz reviewed Jan 23, 2020

View reviewed changes

fdiaz approved these changes Jan 23, 2020

View reviewed changes

dfed merged commit b56a353 into develop--4.0 Jan 23, 2020

dfed deleted the dfed--run-SinglePromptSecureEnclaveIntegrationTests-on-tvOS branch January 23, 2020 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS #209

[Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS #209

dfed commented Jan 19, 2020 •

edited

Loading

dfed commented Jan 20, 2020

dfed commented Jan 20, 2020

codecov-io commented Jan 20, 2020 •

edited

Loading

fdiaz Jan 23, 2020

dfed Jan 23, 2020

[Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS #209

[Valet 4.0] Get SinglePromptSecureEnclaveIntegrationTests running on tvOS #209

Conversation

dfed commented Jan 19, 2020 • edited Loading

dfed commented Jan 20, 2020

dfed commented Jan 20, 2020

codecov-io commented Jan 20, 2020 • edited Loading

Codecov Report

fdiaz Jan 23, 2020

Choose a reason for hiding this comment

dfed Jan 23, 2020

Choose a reason for hiding this comment

dfed commented Jan 19, 2020 •

edited

Loading

codecov-io commented Jan 20, 2020 •

edited

Loading