From 5c7d4e55a4c11bee6c6ec220b9a374c20c85fbd3 Mon Sep 17 00:00:00 2001 From: Danny Moesch Date: Sat, 20 Jul 2019 23:36:44 +0200 Subject: [PATCH] Introduce KeyStore protocol in order to provide specialized key store implementations for tests With the DictBasedKeychain the main AppKeychain is not influenced by tests. The previous implementation led to an empty Keychain requiring a new setup of the simulator. --- pass.xcodeproj/project.pbxproj | 8 ++++ passKit/Helpers/AppKeychain.swift | 2 +- passKit/Helpers/KeyStore.swift | 19 +++++++++ passKit/Models/PGPAgent.swift | 30 +++++++------ passKitTests/Models/PGPAgentTest.swift | 16 +++---- passKitTests/Testbase/DictBasedKeychain.swift | 42 +++++++++++++++++++ 6 files changed, 93 insertions(+), 24 deletions(-) create mode 100644 passKit/Helpers/KeyStore.swift create mode 100644 passKitTests/Testbase/DictBasedKeychain.swift diff --git a/pass.xcodeproj/project.pbxproj b/pass.xcodeproj/project.pbxproj index c2da6f2..87d3997 100644 --- a/pass.xcodeproj/project.pbxproj +++ b/pass.xcodeproj/project.pbxproj @@ -52,6 +52,8 @@ 30B04860209A5141001013CA /* PasswordTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30B0485F209A5141001013CA /* PasswordTest.swift */; }; 30BAC8C622E3BAAF00438475 /* TestBase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30BAC8C422E3BAAF00438475 /* TestBase.swift */; }; 30BAC8C722E3BAAF00438475 /* TestPGPKeys.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30BAC8C522E3BAAF00438475 /* TestPGPKeys.swift */; }; + 30BAC8CB22E3BB6C00438475 /* DictBasedKeychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30BAC8CA22E3BB6C00438475 /* DictBasedKeychain.swift */; }; + 30BAC8CD22E3BB9700438475 /* KeyStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30BAC8CC22E3BB9700438475 /* KeyStore.swift */; }; 30BF5EC821EA8FB5000E4154 /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = 30BF5ECA21EA8FB5000E4154 /* Localizable.strings */; }; 30BF5ED721ED2434000E4154 /* Localizable.stringsdict in Resources */ = {isa = PBXBuildFile; fileRef = 30BF5ED521ED2434000E4154 /* Localizable.stringsdict */; }; 30C25DBD21F3599E00BB27BB /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 30C25DBF21F3599E00BB27BB /* InfoPlist.strings */; }; @@ -261,6 +263,8 @@ 30B0485F209A5141001013CA /* PasswordTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordTest.swift; sourceTree = ""; }; 30BAC8C422E3BAAF00438475 /* TestBase.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestBase.swift; sourceTree = ""; }; 30BAC8C522E3BAAF00438475 /* TestPGPKeys.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestPGPKeys.swift; sourceTree = ""; }; + 30BAC8CA22E3BB6C00438475 /* DictBasedKeychain.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DictBasedKeychain.swift; sourceTree = ""; }; + 30BAC8CC22E3BB9700438475 /* KeyStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyStore.swift; sourceTree = ""; }; 30BF5EC921EA8FB5000E4154 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = en; path = en.lproj/Localizable.strings; sourceTree = ""; }; 30BF5ED621ED2434000E4154 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = en; path = en.lproj/Localizable.stringsdict; sourceTree = ""; }; 30C25DA921F34D2800BB27BB /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = en; path = en.lproj/Main.strings; sourceTree = ""; }; @@ -470,6 +474,7 @@ 30BAC8C322E3BA4300438475 /* Testbase */ = { isa = PBXGroup; children = ( + 30BAC8CA22E3BB6C00438475 /* DictBasedKeychain.swift */, 30BAC8C422E3BAAF00438475 /* TestBase.swift */, 30BAC8C522E3BAAF00438475 /* TestPGPKeys.swift */, ); @@ -632,6 +637,7 @@ 30697C2521F63C590064FCAC /* FileManagerExtension.swift */, 30697C2421F63C590064FCAC /* Globals.swift */, 3032327322C7F710009EBD9C /* KeyFileManager.swift */, + 30BAC8CC22E3BB9700438475 /* KeyStore.swift */, 30697C2321F63C580064FCAC /* NotificationNames.swift */, 30697C2621F63C590064FCAC /* PasswordGeneratorFlavour.swift */, 302202EE222F14E400555236 /* SearchBarScope.swift */, @@ -1283,6 +1289,7 @@ 30697C2E21F63C5A0064FCAC /* Utils.swift in Sources */, 30697C4521F63CAB0064FCAC /* Password.swift in Sources */, 30697C4421F63CAB0064FCAC /* PasswordEntity.swift in Sources */, + 30BAC8CD22E3BB9700438475 /* KeyStore.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1292,6 +1299,7 @@ files = ( 30A1D2AC21B32C2A00E2D1F7 /* TokenBuilderTest.swift in Sources */, 301F646D216166AA0071A4CE /* AdditionFieldTest.swift in Sources */, + 30BAC8CB22E3BB6C00438475 /* DictBasedKeychain.swift in Sources */, 30FD2F78214D9E0E005E0A92 /* ParserTest.swift in Sources */, A2AA934622DE3A8000D79A00 /* PGPAgentTest.swift in Sources */, 30BAC8C622E3BAAF00438475 /* TestBase.swift in Sources */, diff --git a/passKit/Helpers/AppKeychain.swift b/passKit/Helpers/AppKeychain.swift index 88cc0dd..8f70492 100644 --- a/passKit/Helpers/AppKeychain.swift +++ b/passKit/Helpers/AppKeychain.swift @@ -8,7 +8,7 @@ import KeychainAccess -public class AppKeychain { +public class AppKeychain: KeyStore { public static let shared = AppKeychain() diff --git a/passKit/Helpers/KeyStore.swift b/passKit/Helpers/KeyStore.swift new file mode 100644 index 0000000..d57a51f --- /dev/null +++ b/passKit/Helpers/KeyStore.swift @@ -0,0 +1,19 @@ +// +// KeyStore.swift +// passKit +// +// Created by Danny Moesch on 20.07.19. +// Copyright © 2019 Bob Sun. All rights reserved. +// + +import Foundation + +public protocol KeyStore { + func add(data: Data?, for key: String) + func add(string: String?, for key: String) + func contains(key: String) -> Bool + func get(for key: String) -> Data? + func get(for key: String) -> String? + func removeContent(for key: String) + func removeAllContent() +} diff --git a/passKit/Models/PGPAgent.swift b/passKit/Models/PGPAgent.swift index bc8c739..871cebe 100644 --- a/passKit/Models/PGPAgent.swift +++ b/passKit/Models/PGPAgent.swift @@ -12,15 +12,21 @@ import KeychainAccess import Gopenpgpwrapper public class PGPAgent { + + private let keyStore: KeyStore + + public init(keyStore: KeyStore = AppKeychain.shared) { + self.keyStore = keyStore + } public var pgpKeyID: String? // PGP passphrase public var passphrase: String? { set { - AppKeychain.shared.add(string: newValue, for: "pgpKeyPassphrase") + keyStore.add(string: newValue, for: "pgpKeyPassphrase") } get { - return AppKeychain.shared.get(for: "pgpKeyPassphrase") + return keyStore.get(for: "pgpKeyPassphrase") } } @@ -68,12 +74,12 @@ public class PGPAgent { } // Read the key data from keychain. - guard let pgpKeyData: Data = AppKeychain.shared.get(for: keyType.getKeychainKey()) else { + guard let pgpKeyData: Data = keyStore.get(for: keyType.getKeychainKey()) else { throw AppError.KeyImport } // Remove the key data from keychain temporary, in case the following step crashes repeatedly. - AppKeychain.shared.removeContent(for: keyType.getKeychainKey()) + keyStore.removeContent(for: keyType.getKeychainKey()) // Try GopenpgpwrapperReadKey first. if let key = GopenpgpwrapperReadKey(pgpKeyData) { @@ -83,7 +89,7 @@ public class PGPAgent { case .PRIVATE: self.privateKey = key } - AppKeychain.shared.add(data: pgpKeyData, for: keyType.getKeychainKey()) + keyStore.add(data: pgpKeyData, for: keyType.getKeychainKey()) return } @@ -98,7 +104,7 @@ public class PGPAgent { case .PRIVATE: self.privateKeyV2 = key } - AppKeychain.shared.add(data: pgpKeyData, for: keyType.getKeychainKey()) + keyStore.add(data: pgpKeyData, for: keyType.getKeychainKey()) return } @@ -107,19 +113,19 @@ public class PGPAgent { public func initPGPKey(from url: URL, keyType: PgpKey) throws { let pgpKeyData = try Data(contentsOf: url) - AppKeychain.shared.add(data: pgpKeyData, for: keyType.getKeychainKey()) + keyStore.add(data: pgpKeyData, for: keyType.getKeychainKey()) try initPGPKey(keyType) } public func initPGPKey(with armorKey: String, keyType: PgpKey) throws { let pgpKeyData = armorKey.data(using: .ascii)! - AppKeychain.shared.add(data: pgpKeyData, for: keyType.getKeychainKey()) + keyStore.add(data: pgpKeyData, for: keyType.getKeychainKey()) try initPGPKey(keyType) } public func initPGPKeyFromFileSharing() throws { - try KeyFileManager.PublicPgp.importKeyAndDeleteFile() - try KeyFileManager.PrivatePgp.importKeyAndDeleteFile() + try KeyFileManager.PublicPgp.importKeyAndDeleteFile(keyHandler: keyStore.add) + try KeyFileManager.PrivatePgp.importKeyAndDeleteFile(keyHandler: keyStore.add) try initPGPKeys() } @@ -167,8 +173,8 @@ public class PGPAgent { } public func removePGPKeys() { - AppKeychain.shared.removeContent(for: PgpKey.PUBLIC.getKeychainKey()) - AppKeychain.shared.removeContent(for: PgpKey.PRIVATE.getKeychainKey()) + keyStore.removeContent(for: PgpKey.PUBLIC.getKeychainKey()) + keyStore.removeContent(for: PgpKey.PRIVATE.getKeychainKey()) passphrase = nil publicKey = nil privateKey = nil diff --git a/passKitTests/Models/PGPAgentTest.swift b/passKitTests/Models/PGPAgentTest.swift index e3f7db0..2c1f18d 100644 --- a/passKitTests/Models/PGPAgentTest.swift +++ b/passKitTests/Models/PGPAgentTest.swift @@ -11,14 +11,8 @@ import XCTest @testable import passKit class PGPAgentTest: XCTestCase { - - override func setUp() { - PGPAgent().removePGPKeys() - } - - override func tearDown() { - PGPAgent().removePGPKeys() - } + + private let keychain = DictBasedKeychain() func basicEncryptDecrypt(pgpAgent: PGPAgent) -> Bool { // Encrypt and decrypt. @@ -33,7 +27,7 @@ class PGPAgentTest: XCTestCase { } func testInitPGPKey() { - let pgpAgent = PGPAgent() + let pgpAgent = PGPAgent(keyStore: keychain) // [RSA2048] Setup keys. try? pgpAgent.initPGPKey(with: PGP_RSA2048_PUBLIC_KEY, keyType: .PUBLIC) @@ -41,7 +35,7 @@ class PGPAgentTest: XCTestCase { XCTAssertTrue(pgpAgent.isImported) XCTAssertEqual(pgpAgent.pgpKeyID, "A1024DAE") XCTAssertTrue(self.basicEncryptDecrypt(pgpAgent: pgpAgent)) - let pgpAgent2 = PGPAgent() + let pgpAgent2 = PGPAgent(keyStore: keychain) try? pgpAgent2.initPGPKeys() // load from the keychain XCTAssertTrue(self.basicEncryptDecrypt(pgpAgent: pgpAgent2)) pgpAgent.removePGPKeys() @@ -88,7 +82,7 @@ class PGPAgentTest: XCTestCase { } func testInitPGPKeyBadPrivateKeys() { - let pgpAgent = PGPAgent() + let pgpAgent = PGPAgent(keyStore: keychain) let plainData = "Hello World!".data(using: .utf8)! // [RSA2048] Setup the public key. diff --git a/passKitTests/Testbase/DictBasedKeychain.swift b/passKitTests/Testbase/DictBasedKeychain.swift new file mode 100644 index 0000000..cf3e7d4 --- /dev/null +++ b/passKitTests/Testbase/DictBasedKeychain.swift @@ -0,0 +1,42 @@ +// +// DictBasedKeychain.swift +// passKitTests +// +// Created by Danny Moesch on 20.07.19. +// Copyright © 2019 Bob Sun. All rights reserved. +// + +import Foundation +import passKit + +class DictBasedKeychain: KeyStore { + private var store: [String: Any] = [:] + + public func add(data: Data?, for key: String) { + store[key] = data + } + + public func add(string: String?, for key: String) { + store[key] = string + } + + public func contains(key: String) -> Bool { + return store[key] != nil + } + + public func get(for key: String) -> Data? { + return store[key] as? Data + } + + public func get(for key: String) -> String? { + return store[key] as? String + } + + public func removeContent(for key: String) { + store.removeValue(forKey: key) + } + + public func removeAllContent() { + store.removeAll() + } +}