Keychain Deserialization Errors

.codecov.yml

@@ -7,3 +7,10 @@ codecov:
 ignore:
   - Tests
   - OneTimePasswordLegacyTests
+
+coverage:
+  status:
+    patch:
+      default:
+        # Allow patch to be 0% covered without marking a PR with a failing status.
+        target: 0

Sources/Keychain.swift

@@ -40,14 +40,14 @@ public final class Keychain {
     /// - throws: A `Keychain.Error` if an error occurred.
     /// - returns: The persistent token, or `nil` if no token matched the given identifier.
     public func persistentToken(withIdentifier identifier: Data) throws -> PersistentToken? {
-        return try keychainItem(forPersistentRef: identifier).flatMap(PersistentToken.init(keychainDictionary:))
+        return try keychainItem(forPersistentRef: identifier).map(PersistentToken.init(keychainDictionary:))
     }
 
     /// Returns the set of all persistent tokens found in the keychain.
     ///
     /// - throws: A `Keychain.Error` if an error occurred.
     public func allPersistentTokens() throws -> Set<PersistentToken> {
-        return Set(try allKeychainItems().flatMap(PersistentToken.init(keychainDictionary:)))
+        return Set(try allKeychainItems().map(PersistentToken.init(keychainDictionary:)))
     }
 
     // MARK: Write
@@ -123,15 +123,28 @@ private extension Token {
 }
 
 private extension PersistentToken {
-    init?(keychainDictionary: NSDictionary) {
-        guard let urlData = keychainDictionary[kSecAttrGeneric as String] as? Data,
-            let urlString = String(data: urlData, encoding: urlStringEncoding),
-            let secret = keychainDictionary[kSecValueData as String] as? Data,
-            let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data,
-            let url = URL(string: urlString as String),
-            let token = Token(url: url, secret: secret) else {
-                return nil
+    enum DeserializationError: Error {
+        case missingData
+        case missingSecret
+        case missingPersistentRef
+        case unreadableData
+    }
+
+    init(keychainDictionary: NSDictionary) throws {
+        guard let urlData = keychainDictionary[kSecAttrGeneric as String] as? Data else {
+            throw DeserializationError.missingData
+        }
+        guard let secret = keychainDictionary[kSecValueData as String] as? Data else {
+            throw DeserializationError.missingSecret
+        }
+        guard let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data else {
+            throw DeserializationError.missingPersistentRef
+        }
+        guard let urlString = String(data: urlData, encoding: urlStringEncoding),
+            let url = URL(string: urlString) else {
+                throw DeserializationError.unreadableData
         }
+        let token = try Token(_url: url, secret: secret)
         self.init(token: token, identifier: keychainItemRef)
     }
 }

Sources/Token+URL.swift

@@ -42,11 +42,15 @@ public extension Token {
 
     /// Attempts to initialize a token represented by the give URL.
     init?(url: URL, secret: Data? = nil) {
-        do {
-            self = try token(from: url, secret: secret)
-        } catch {
-            return nil
-        }
+        try? self.init(_url: url, secret: secret)
+    }
+
+    // Eventually, this throwing initializer will replace the failable initializer above. For now, the failable
+    // initializer remains to maintain a consistent public API. Since two different initializers cannot overload the
+    // same initializer signature with both throwing an failable versions, this new initializer is currently prefixed
+    // with an underscore and marked as internal.
+    internal init(_url url: URL, secret: Data? = nil) throws {
+        self = try token(from: url, secret: secret)
     }
 }
 

Tests/KeychainTests.swift

@@ -222,4 +222,107 @@ class KeychainTests: XCTestCase {
             XCTFail("allPersistentTokens() failed with error: \(error)")
         }
     }
+
+    func testMissingData() throws {
+        let keychainAttributes: [String: AnyObject] = [
+            kSecValueData as String:    testToken.generator.secret as NSData,
+        ]
+
+        let persistentRef = try addKeychainItem(withAttributes: keychainAttributes)
+
+        XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef))
+        XCTAssertThrowsError(try keychain.allPersistentTokens())
+
+        XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef),
+                         "Failed to delete the test token from the keychain. This may cause future test runs to fail.")
+    }
+
+    func testMissingSecret() throws {
+        let data = try testToken.toURL().absoluteString.data(using: .utf8)!
+
+        let keychainAttributes: [String: AnyObject] = [
+            kSecAttrGeneric as String:  data as NSData,
+        ]
+
+        let persistentRef = try addKeychainItem(withAttributes: keychainAttributes)
+
+        XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef))
+        XCTAssertThrowsError(try keychain.allPersistentTokens())
+
+        XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef),
+                         "Failed to delete the test token from the keychain. This may cause future test runs to fail.")
+    }
+
+    func testBadData() throws {
+        let badData = " ".data(using: .utf8)!
+
+        let keychainAttributes: [String: AnyObject] = [
+            kSecAttrGeneric as String:  badData as NSData,
+            kSecValueData as String:    testToken.generator.secret as NSData,
+        ]
+
+        let persistentRef = try addKeychainItem(withAttributes: keychainAttributes)
+
+        XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef))
+        XCTAssertThrowsError(try keychain.allPersistentTokens())
+
+        XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef),
+                         "Failed to delete the test token from the keychain. This may cause future test runs to fail.")
+    }
+
+    func testBadURL() throws {
+        let badData = "http://example.com".data(using: .utf8)!
+
+        let keychainAttributes: [String: AnyObject] = [
+            kSecAttrGeneric as String:  badData as NSData,
+            kSecValueData as String:    testToken.generator.secret as NSData,
+        ]
+
+        let persistentRef = try addKeychainItem(withAttributes: keychainAttributes)
+
+        XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef))
+        XCTAssertThrowsError(try keychain.allPersistentTokens())
+
+        XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef),
+                         "Failed to delete the test token from the keychain. This may cause future test runs to fail.")
+    }
+}
+
+// MARK: Keychain helpers
+
+private func addKeychainItem(withAttributes attributes: [String: AnyObject]) throws -> Data {
+    var mutableAttributes = attributes
+    mutableAttributes[kSecClass as String] = kSecClassGenericPassword
+    mutableAttributes[kSecReturnPersistentRef as String] = kCFBooleanTrue
+    // Set a random string for the account name.
+    // We never query by or display this value, but the keychain requires it to be unique.
+    if mutableAttributes[kSecAttrAccount as String] == nil {
+        mutableAttributes[kSecAttrAccount as String] = UUID().uuidString as NSString
+    }
+
+    var result: AnyObject?
+    let resultCode: OSStatus = withUnsafeMutablePointer(to: &result) {
+        SecItemAdd(mutableAttributes as CFDictionary, $0)
+    }
+
+    guard resultCode == errSecSuccess else {
+        throw Keychain.Error.systemError(resultCode)
+    }
+    guard let persistentRef = result as? Data else {
+        throw Keychain.Error.incorrectReturnType
+    }
+    return persistentRef
+}
+
+public func deleteKeychainItem(forPersistentRef persistentRef: Data) throws {
+    let queryDict: [String : AnyObject] = [
+        kSecClass as String:               kSecClassGenericPassword,
+        kSecValuePersistentRef as String:  persistentRef as NSData,
+    ]
+
+    let resultCode = SecItemDelete(queryDict as CFDictionary)
+
+    guard resultCode == errSecSuccess else {
+        throw Keychain.Error.systemError(resultCode)
+    }
 }