fix: filter Keychain items to include only those with AfterFirstUnlock or AfterFirstUnlockThisDeviceOnly accessibility.

Author: AlejandroOrozco

TwilioVerifyDemo/TwilioVerifyDemo/Common/AppDelegate.swift

@@ -86,7 +86,7 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
       return
     }
 
-    showChallenge(payload: payload)
+    handleChallengeApproval(with: payload)
   }
 }
 

TwilioVerifySDK/TwilioSecurity/Sources/Keychain/Keychain.swift

@@ -26,6 +26,7 @@ protocol KeychainProtocol {
   func representation(forKey key: SecKey) throws -> Data
   func generateKeyPair(withParameters parameters: [String: Any], allowIphoneMigration: Bool) throws -> KeyPair
   func copyItemMatching(query: Query) throws -> AnyObject
+  func copyItemsMatching(queries: [Query]) throws -> [AnyObject]
   func addItem(withQuery query: Query) -> OSStatus
   func updateItem(withQuery query: Query, attributes: CFDictionary) -> OSStatus
   @discardableResult func deleteItem(withQuery query: Query) -> OSStatus
@@ -170,7 +171,35 @@ class Keychain: KeychainProtocol {
 
     return result
   }
-  
+
+  func copyItemsMatching(queries: [Query]) throws -> [AnyObject] {
+    var allResults: [AnyObject] = []
+
+    for query in queries {
+      var result: AnyObject?
+
+      let status: OSStatus = retry {
+        SecItemCopyMatching(query as CFDictionary, &result)
+      } validation: { status in
+        status == errSecSuccess
+      }
+
+      if status == errSecSuccess, let result = result {
+        if let resultArray = result as? [AnyObject] {
+          allResults.append(contentsOf: resultArray)
+        } else {
+          allResults.append(result)
+        }
+      } else if status != errSecItemNotFound {
+        let error: KeychainError = .invalidStatusCode(code: Int(status))
+        Logger.shared.log(withLevel: .error, message: error.localizedDescription)
+        throw error
+      }
+    }
+
+    return allResults
+  }
+
   func addItem(withQuery query: Query) -> OSStatus {
     deleteItem(withQuery: query)
     Logger.shared.log(withLevel: .debug, message: "Added item for \(query)")

TwilioVerifySDK/TwilioSecurity/Sources/Keychain/KeychainQuery.swift

@@ -41,7 +41,7 @@ protocol KeychainQueryProtocol {
   func deleteKey(withAlias alias: String) -> Query
   func save(data: Data, withKey key: String, withServiceName service: String?, allowIphoneMigration: Bool) -> Query
   func getData(withKey key: String) -> Query
-  func getAll(withServiceName service: String?) -> Query
+  func getAll(withServiceName service: String?) -> [Query]
   func delete(withKey key: String) -> Query
   func deleteItems(withServiceName service: String?) -> Query
 }
@@ -107,15 +107,31 @@ struct KeychainQuery: KeychainQueryProtocol {
     ])
   }
 
-  func getAll(withServiceName service: String?) -> Query {
-    var query = [kSecClass: kSecClassGenericPassword,
-     kSecReturnAttributes: true,
-     kSecReturnData: true,
-     kSecMatchLimit: kSecMatchLimitAll] as Query
-    if let service = service {
-      query[kSecAttrService] = service
+  func getAll(withServiceName service: String?) -> [Query] {
+    let accessibilityOptions = [
+      kSecAttrAccessibleAfterFirstUnlock,
+      kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
+    ]
+
+    var queries: [Query] = []
+
+    for accessibility in accessibilityOptions {
+      var query = [
+        kSecClass: kSecClassGenericPassword,
+        kSecReturnAttributes: true,
+        kSecReturnData: true,
+        kSecMatchLimit: kSecMatchLimitAll,
+        kSecAttrAccessible: accessibility
+      ] as Query
+
+      if let service = service {
+        query[kSecAttrService] = service
+      }
+
+      queries.append(properties(query))
     }
-    return properties(query)
+
+    return queries
   }
 
   func delete(withKey key: String) -> Query {

TwilioVerifySDK/TwilioSecurity/Sources/Storage/SecureStorage.swift

@@ -92,15 +92,15 @@ extension SecureStorage: SecureStorageProvider {
 
   public func getAll(withServiceName service: String?) throws -> [Data] {
     Logger.shared.log(withLevel: .info, message: "Getting all values")
-    let query = keychainQuery.getAll(withServiceName: service)
+    let queries = keychainQuery.getAll(withServiceName: service)
     do {
-      let result = try keychain.copyItemMatching(query: query)
-      guard let resultArray = result as? [Any] else {
-        return []
+      let results = try keychain.copyItemsMatching(queries: queries)
+
+      let objectsData = results.compactMap { result -> Data? in
+        guard let dict = result as? [String: Any] else { return nil }
+        return dict[kSecValueData as String] as? Data
       }
-      let objectsData = resultArray.map {
-        (($0 as? [String: Any])?[kSecValueData as String]) as? Data
-      }.compactMap { $0 }
+
       Logger.shared.log(withLevel: .info, message: "Return all values")
       return objectsData
     } catch {

TwilioVerifySDKTests/TwilioSecurity/Sources/Keychain/KeychainQueryTests.swift

@@ -98,16 +98,18 @@ class KeychainQueryTests: XCTestCase {
   }
 
   func testGetAllData_shouldReturnValidQuery() {
-    let query = keychainQuery.getAll(withServiceName: nil)
-    let keyClass = query[kSecClass] as! CFString
-    let returnAttributes = query[kSecReturnAttributes] as! CFBoolean
-    let returnData = query[kSecReturnData] as! CFBoolean
-    let matchLimit = query[kSecMatchLimit] as! CFString
-    
-    XCTAssertEqual(keyClass, kSecClassGenericPassword)
-    XCTAssertTrue(returnAttributes as! Bool)
-    XCTAssertTrue(returnData as! Bool)
-    XCTAssertEqual(matchLimit, kSecMatchLimitAll)
+    let queries = keychainQuery.getAll(withServiceName: nil)
+    for query in queries {
+      let keyClass = query[kSecClass] as! CFString
+      let returnAttributes = query[kSecReturnAttributes] as! CFBoolean
+      let returnData = query[kSecReturnData] as! CFBoolean
+      let matchLimit = query[kSecMatchLimit] as! CFString
+
+      XCTAssertEqual(keyClass, kSecClassGenericPassword)
+      XCTAssertTrue(returnAttributes as! Bool)
+      XCTAssertTrue(returnData as! Bool)
+      XCTAssertEqual(matchLimit, kSecMatchLimitAll)
+    }
   }
 
   func testDelete_withKey_shouldReturnValidQuery() {

TwilioVerifySDKTests/TwilioSecurity/Sources/Keychain/Mocks/KeychainMock.swift

@@ -30,6 +30,7 @@ enum KeychainMethods {
   case addItem
   case updateItem
   case deleteItem
+  case copyItemsMatching
 }
 
 class KeychainMock {
@@ -42,12 +43,13 @@ class KeychainMock {
   var deleteItemStatus: OSStatus!
   var keyPair: KeyPair!
   var keys: [AnyObject]!
+  var multiQueryResults: [[AnyObject]]!
   var copyItemMitmatchingHandler: (() -> Void)?
   private(set) var callsToAddItem = 0
   private(set) var callsToUpdateItem = 0
   private(set) var callOrder = [KeychainMethods]()
   private(set) var callsToCopyItemMatching = -1
-  
+  private(set) var callsToCopyItemsMatching = 0
 }
 
 extension KeychainMock: KeychainProtocol {
@@ -102,7 +104,22 @@ extension KeychainMock: KeychainProtocol {
 
     return keys[callsToCopyItemMatching]
   }
-  
+
+  func copyItemsMatching(queries: [Query]) throws -> [AnyObject] {
+    callsToCopyItemsMatching += 1
+    callOrder.append(.copyItemsMatching)
+
+    if let error = error {
+      throw error
+    }
+
+    if let multiQueryResults = multiQueryResults {
+      return multiQueryResults.flatMap { $0 }
+    }
+
+    return []
+  }
+
   func addItem(withQuery query: Query) -> OSStatus {
     callsToAddItem += 1
     callOrder.append(.addItem)

TwilioVerifySDKTests/TwilioSecurity/Sources/Storage/SecureStorageTests.swift

@@ -109,17 +109,19 @@ class SecureStorageTests: XCTestCase {
     let expectedData2 = "data2".data(using: .utf8)!
     let valueData2 = [kSecValueData as String: expectedData2]
     var data: [Data?]!
-    keychain.keys = [[valueData1, valueData2] as AnyObject]
+    keychain.multiQueryResults = [[valueData1 as AnyObject, valueData2 as AnyObject] ]
     XCTAssertNoThrow(data = try storage.getAll(withServiceName: nil), "Get all should not throw")
     XCTAssertEqual(data.first, expectedData1, "Data should be \(expectedData1) but was \(data!.first!!)")
     XCTAssertEqual(data.last, expectedData2, "Data should be \(expectedData2) but was \(data!.last!!)")
+    XCTAssertTrue(keychain.callOrder.contains(.copyItemsMatching), "copyItemsMatching should be called")
   }
-  
+
   func testGetAllData_withoutDataSaved_shouldReturnEmptyArray() {
     var data: [Data?]!
-    keychain.keys = ["data".data(using: .utf8)! as AnyObject]
+    keychain.multiQueryResults = [["data".data(using: .utf8)! as AnyObject]]
     XCTAssertNoThrow(data = try storage.getAll(withServiceName: nil), "Get all should not throw")
     XCTAssertTrue(data.isEmpty)
+    XCTAssertTrue(keychain.callOrder.contains(.copyItemsMatching), "copyItemsMatching should be called")
   }
 
   func testRemoveValue_valueExistsForKey_shouldReturnValue() {
@@ -137,23 +139,25 @@ class SecureStorageTests: XCTestCase {
   func testClear_itemsExist_shouldClearKeychain() {
     let expectedData2 = "data2".data(using: .utf8)!
     let valueData2 = [kSecValueData as String: expectedData2]
-    keychain.keys = [[valueData2] as AnyObject]
+    keychain.multiQueryResults = [[valueData2 as AnyObject]]
     keychain.deleteItemStatus = errSecSuccess
     XCTAssertNoThrow(try storage.clear(withServiceName: "service"), "Clear should not throw")
     XCTAssertTrue(keychain.callOrder.contains(.deleteItem), "Delete should be called")
+    XCTAssertTrue(keychain.callOrder.contains(.copyItemsMatching), "copyItemsMatching should be called")
   }
 
   func testClear_noItems_shouldNotClearKeychain() {
     keychain.error = NSError(domain: "No items", code: Int(errSecItemNotFound), userInfo: nil)
     XCTAssertNoThrow(try storage.clear(withServiceName: "service"), "Clear should not throw")
     XCTAssertFalse(keychain.callOrder.contains(.deleteItem), "Delete should not be called")
+    XCTAssertTrue(keychain.callOrder.contains(.copyItemsMatching), "copyItemsMatching should be called")
   }
 
   func testClear_itemsExistAndDeleteStatusError_shouldThrowError() {
     let expectedLocalizedDescription = "Invalid status code operation received: -25300"
     let expectedData2 = "data2".data(using: .utf8)!
     let valueData2 = [kSecValueData as String: expectedData2]
-    keychain.keys = [[valueData2] as AnyObject]
+    keychain.multiQueryResults = [[valueData2 as AnyObject]]
     keychain.deleteItemStatus = errSecItemNotFound
 
     XCTAssertThrowsError(
@@ -214,6 +218,51 @@ class SecureStorageTests: XCTestCase {
     XCTAssertEqual(data, expectedData, "Data should be \(expectedData) but was \(data)")
   }
 
+  func testGetAllData_withMultipleAccessibilityTypes_shouldReturnAllValues() throws {
+    // Given
+    let expectedData1 = "data1".data(using: .utf8)!
+    let valueData1 = [kSecValueData as String: expectedData1]
+    let expectedData2 = "data2".data(using: .utf8)!
+    let valueData2 = [kSecValueData as String: expectedData2]
+    let expectedData3 = "data3".data(using: .utf8)!
+    let valueData3 = [kSecValueData as String: expectedData3]
+
+    keychain.multiQueryResults = [
+      [valueData1 as AnyObject, valueData2 as AnyObject],
+      [valueData3 as AnyObject]
+    ]
+
+    // When
+    var data: [Data?]!
+    XCTAssertNoThrow(data = try storage.getAll(withServiceName: "testService"), "Get all should not throw")
+
+    // Then
+    XCTAssertEqual(data.count, 3, "Should return 3 items from both queries")
+    XCTAssertTrue(data.contains(expectedData1), "Results should contain data1")
+    XCTAssertTrue(data.contains(expectedData2), "Results should contain data2")
+    XCTAssertTrue(data.contains(expectedData3), "Results should contain data3")
+    XCTAssertTrue(keychain.callOrder.contains(.copyItemsMatching), "copyItemsMatching should be called")
+    XCTAssertEqual(keychain.callsToCopyItemsMatching, 1, "copyItemsMatching should be called once")
+  }
+
+  func testGetAllData_errorFromKeychain_shouldThrow() {
+    // Given
+    keychain.error = TestError.operationFailed
+
+    // When/Then
+    XCTAssertThrowsError(
+      try storage.getAll(withServiceName: nil),
+      "Get all should throw when keychain fails"
+    ) { error in
+      XCTAssertEqual(
+        error as! TestError,
+        TestError.operationFailed,
+        "Error should be \(TestError.operationFailed), but was \(error)"
+      )
+    }
+    XCTAssertEqual(keychain.callsToCopyItemsMatching, 1, "copyItemsMatching should be called once")
+  }
+
   func testGet_valueExistsForMultipleTries_shouldThrowIfTheErrorPersist() throws {
     // Given
     guard let expectedData = "data".data(using: .utf8) else {

TwilioVerifySDKTests/TwilioVerify/Sources/Data/Mocks/SecureStorageMock.swift

@@ -55,6 +55,7 @@ extension SecureStorageMock: SecureStorageProvider {
     if let error = error {
       throw error
     }
+
     return Array(objectsData.values)
   }
 

fastlane/Fastfile

@@ -1,11 +1,10 @@
 output_directory = './fastlane/Test Output/'
 coverage_directory = '/coverage'
 complete_suite = 'CompleteSuite'
-single_ftl_device = [{ios_model_id: 'iphone8', ios_version_id: '16.6'}]
+single_ftl_device = [{ios_model_id: 'iphone13pro', ios_version_id: '16.6'}]
 all_ftl_devices = [
-  {ios_model_id: 'iphone11pro', ios_version_id: '16.6'},
   {ios_model_id: 'ipad10', ios_version_id: '16.6'},
-  {ios_model_id: 'iphone8', ios_version_id: '15.7'}
+  {ios_model_id: 'iphone13pro', ios_version_id: '16.6'}
 ]
 
 default_platform(:ios)