From 7bee780b4602b54f5f64b61a13f7217a97b1cb8b Mon Sep 17 00:00:00 2001 From: Danny Moesch Date: Sat, 29 Jun 2019 23:09:24 +0200 Subject: [PATCH] Extract key importing logic and put it into separate class --- pass.xcodeproj/project.pbxproj | 12 +++++ ...GPKeyArmorSettingTableViewController.swift | 4 +- passKit/Helpers/AppError.swift | 2 +- passKit/Helpers/KeyFileManager.swift | 40 +++++++++++++++ passKit/Helpers/PgpKeyType.swift | 30 ++++++++++++ passKit/Models/PasswordStore.swift | 47 ++++++------------ passKitTests/Helpers/KeyFileManagerTest.swift | 49 +++++++++++++++++++ 7 files changed, 148 insertions(+), 36 deletions(-) create mode 100644 passKit/Helpers/KeyFileManager.swift create mode 100644 passKit/Helpers/PgpKeyType.swift create mode 100644 passKitTests/Helpers/KeyFileManagerTest.swift diff --git a/pass.xcodeproj/project.pbxproj b/pass.xcodeproj/project.pbxproj index 05fb57a..285e701 100644 --- a/pass.xcodeproj/project.pbxproj +++ b/pass.xcodeproj/project.pbxproj @@ -14,6 +14,9 @@ 302B2C9822C2BDE700D831EE /* AppKeychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = 302B2C9722C2BDE700D831EE /* AppKeychain.swift */; }; 302E85612125ECC70031BA64 /* Parser.swift in Sources */ = {isa = PBXBuildFile; fileRef = 302E85602125ECC70031BA64 /* Parser.swift */; }; 302E85632125EE550031BA64 /* Constants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 302E85622125EE550031BA64 /* Constants.swift */; }; + 3032327422C7F710009EBD9C /* KeyFileManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3032327322C7F710009EBD9C /* KeyFileManager.swift */; }; + 3032327622C7F7B9009EBD9C /* PgpKeyType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3032327522C7F7B9009EBD9C /* PgpKeyType.swift */; }; + 3032328A22C9FBA2009EBD9C /* KeyFileManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3032328922C9FBA2009EBD9C /* KeyFileManagerTest.swift */; }; 30697C2A21F63C5A0064FCAC /* NotificationNames.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30697C2321F63C580064FCAC /* NotificationNames.swift */; }; 30697C2B21F63C5A0064FCAC /* Globals.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30697C2421F63C590064FCAC /* Globals.swift */; }; 30697C2C21F63C5A0064FCAC /* FileManagerExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30697C2521F63C590064FCAC /* FileManagerExtension.swift */; }; @@ -212,6 +215,9 @@ 302B2C9722C2BDE700D831EE /* AppKeychain.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppKeychain.swift; sourceTree = ""; }; 302E85602125ECC70031BA64 /* Parser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Parser.swift; sourceTree = ""; }; 302E85622125EE550031BA64 /* Constants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Constants.swift; sourceTree = ""; }; + 3032327322C7F710009EBD9C /* KeyFileManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyFileManager.swift; sourceTree = ""; }; + 3032327522C7F7B9009EBD9C /* PgpKeyType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PgpKeyType.swift; sourceTree = ""; }; + 3032328922C9FBA2009EBD9C /* KeyFileManagerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyFileManagerTest.swift; sourceTree = ""; }; 30697C2321F63C580064FCAC /* NotificationNames.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NotificationNames.swift; sourceTree = ""; }; 30697C2421F63C590064FCAC /* Globals.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Globals.swift; sourceTree = ""; }; 30697C2521F63C590064FCAC /* FileManagerExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileManagerExtension.swift; sourceTree = ""; }; @@ -395,6 +401,7 @@ isa = PBXGroup; children = ( 30A1D29B21AF451E00E2D1F7 /* PasswordGeneratorFlavourTest.swift */, + 3032328922C9FBA2009EBD9C /* KeyFileManagerTest.swift */, ); path = Helpers; sourceTree = ""; @@ -571,8 +578,10 @@ 30697C2821F63C590064FCAC /* DefaultsKeys.swift */, 30697C2521F63C590064FCAC /* FileManagerExtension.swift */, 30697C2421F63C590064FCAC /* Globals.swift */, + 3032327322C7F710009EBD9C /* KeyFileManager.swift */, 30697C2321F63C580064FCAC /* NotificationNames.swift */, 30697C2621F63C590064FCAC /* PasswordGeneratorFlavour.swift */, + 3032327522C7F7B9009EBD9C /* PgpKeyType.swift */, 302202EE222F14E400555236 /* SearchBarScope.swift */, 30697C2721F63C590064FCAC /* Utils.swift */, ); @@ -1058,6 +1067,7 @@ 30A1D2A821B2D53200E2D1F7 /* PasswordChange.swift in Sources */, 30697C3E21F63C990064FCAC /* String+Utilities.swift in Sources */, 302B2C9822C2BDE700D831EE /* AppKeychain.swift in Sources */, + 3032327422C7F710009EBD9C /* KeyFileManager.swift in Sources */, 30697C3B21F63C990064FCAC /* String+Localization.swift in Sources */, 302E85612125ECC70031BA64 /* Parser.swift in Sources */, 30697C4621F63CAB0064FCAC /* GitCredential.swift in Sources */, @@ -1068,6 +1078,7 @@ 30697C2C21F63C5A0064FCAC /* FileManagerExtension.swift in Sources */, 30697C3321F63C8B0064FCAC /* PasscodeLockPresenter.swift in Sources */, 30697C3D21F63C990064FCAC /* UIViewExtension.swift in Sources */, + 3032327622C7F7B9009EBD9C /* PgpKeyType.swift in Sources */, 30697C3A21F63C990064FCAC /* UIViewControllerExtension.swift in Sources */, 30697C2E21F63C5A0064FCAC /* Utils.swift in Sources */, 30697C4521F63CAB0064FCAC /* Password.swift in Sources */, @@ -1085,6 +1096,7 @@ 30B04860209A5141001013CA /* PasswordTest.swift in Sources */, 307BF39921BC2298003A082D /* TestBase.swift in Sources */, 30697C5F21F674800064FCAC /* String+UtilitiesTest.swift in Sources */, + 3032328A22C9FBA2009EBD9C /* KeyFileManagerTest.swift in Sources */, 30A1D2AA21B32A0100E2D1F7 /* OtpTypeTest.swift in Sources */, 301F6468216165290071A4CE /* ConstantsTest.swift in Sources */, 30A1D29C21AF451E00E2D1F7 /* PasswordGeneratorFlavourTest.swift in Sources */, diff --git a/pass/Controllers/PGPKeyArmorSettingTableViewController.swift b/pass/Controllers/PGPKeyArmorSettingTableViewController.swift index e380534..6a71839 100644 --- a/pass/Controllers/PGPKeyArmorSettingTableViewController.swift +++ b/pass/Controllers/PGPKeyArmorSettingTableViewController.swift @@ -91,10 +91,10 @@ class PGPKeyArmorSettingTableViewController: AutoCellHeightUITableViewController override func viewDidLoad() { super.viewDidLoad() - if let publicKey: Data = AppKeychain.get(for: PasswordStore.PGPKeyType.PUBLIC.rawValue) { + if let publicKey: Data = AppKeychain.get(for: PgpKeyType.PUBLIC.getKeychainKey()) { armorPublicKeyTextView.text = String(data: publicKey, encoding: .ascii) } - if let privateKey: Data = AppKeychain.get(for: PasswordStore.PGPKeyType.PRIVATE.rawValue) { + if let privateKey: Data = AppKeychain.get(for: PgpKeyType.PRIVATE.getKeychainKey()) { armorPrivateKeyTextView.text = String(data: privateKey, encoding: .ascii) } diff --git a/passKit/Helpers/AppError.swift b/passKit/Helpers/AppError.swift index cb3854a..a9bfad2 100644 --- a/passKit/Helpers/AppError.swift +++ b/passKit/Helpers/AppError.swift @@ -6,7 +6,7 @@ // Copyright © 2017 Bob Sun. All rights reserved. // -public enum AppError: Error { +public enum AppError: Error, Equatable { case RepositoryNotSet case RepositoryRemoteBranchNotFound(_: String) case RepositoryBranchNotFound(_: String) diff --git a/passKit/Helpers/KeyFileManager.swift b/passKit/Helpers/KeyFileManager.swift new file mode 100644 index 0000000..0a88643 --- /dev/null +++ b/passKit/Helpers/KeyFileManager.swift @@ -0,0 +1,40 @@ +// +// KeyFileManager.swift +// passKit +// +// Created by Danny Moesch on 29.06.19. +// Copyright © 2019 Bob Sun. All rights reserved. +// + +public class KeyFileManager { + public typealias KeyHandler = (Data, String) -> () + + public static let PublicPgp = KeyFileManager(keyType: PgpKeyType.PUBLIC) + public static let PrivatePgp = KeyFileManager(keyType: PgpKeyType.PRIVATE) + + private let keyType: PgpKeyType + private let keyPath: String + private let keyHandler: KeyHandler + + private convenience init(keyType: PgpKeyType) { + self.init(keyType: keyType, keyPath: keyType.getFileSharingPath()) + } + + public init(keyType: PgpKeyType, keyPath: String, keyHandler: @escaping KeyHandler = AppKeychain.add) { + self.keyType = keyType + self.keyPath = keyPath + self.keyHandler = keyHandler + } + + public func importKeyAndDeleteFile() throws { + guard let keyFileContent = FileManager.default.contents(atPath: keyPath) else { + throw AppError.ReadingFile(URL(fileURLWithPath: keyPath).lastPathComponent) + } + keyHandler(keyFileContent, keyType.getKeychainKey()) + try FileManager.default.removeItem(atPath: keyPath) + } + + public func doesKeyFileExist() -> Bool { + return FileManager.default.fileExists(atPath: keyPath) + } +} diff --git a/passKit/Helpers/PgpKeyType.swift b/passKit/Helpers/PgpKeyType.swift new file mode 100644 index 0000000..2776366 --- /dev/null +++ b/passKit/Helpers/PgpKeyType.swift @@ -0,0 +1,30 @@ +// +// PgpKeyType.swift +// passKit +// +// Created by Danny Moesch on 29.06.19. +// Copyright © 2019 Bob Sun. All rights reserved. +// + +public enum PgpKeyType { + case PUBLIC + case PRIVATE + + public func getKeychainKey() -> String { + switch self { + case .PUBLIC: + return "pgpPublicKey" + case .PRIVATE: + return "pgpPrivateKey" + } + } + + func getFileSharingPath() -> String { + switch self { + case .PUBLIC: + return Globals.iTunesFileSharingPGPPublic + case .PRIVATE: + return Globals.iTunesFileSharingPGPPrivate + } + } +} diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index e2c0836..a2a43ef 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -34,10 +34,6 @@ public class PasswordStore { } } public var privateKey: GopenpgpwrapperKey? - public enum PGPKeyType: String { - case PUBLIC = "pgpPublicKey" - case PRIVATE = "pgpPrivateKey" - } public var gitSignatureForNow: GTSignature? { get { @@ -192,14 +188,8 @@ public class PasswordStore { private func importExistingKeysIntoKeychain() { do { - if let publicKey = fm.contents(atPath: Globals.pgpPublicKeyPath) { - AppKeychain.add(data: publicKey, for: PGPKeyType.PUBLIC.rawValue) - try fm.removeItem(atPath: Globals.pgpPublicKeyPath) - } - if let privateKey = fm.contents(atPath: Globals.pgpPrivateKeyPath) { - AppKeychain.add(data: privateKey, for: PGPKeyType.PRIVATE.rawValue) - try fm.removeItem(atPath: Globals.pgpPrivateKeyPath) - } + try KeyFileManager(keyType: PgpKeyType.PUBLIC, keyPath: Globals.pgpPublicKeyPath).importKeyAndDeleteFile() + try KeyFileManager(keyType: PgpKeyType.PRIVATE, keyPath: Globals.pgpPrivateKeyPath).importKeyAndDeleteFile() SharedDefaults[.pgpKeySource] = "file" } catch { print("MigrationError".localize(error)) @@ -220,8 +210,8 @@ public class PasswordStore { try initPGPKey(.PRIVATE) } - private func initPGPKey(_ keyType: PGPKeyType) throws { - if let key = GopenpgpwrapperReadKey(AppKeychain.get(for: keyType.rawValue)) { + private func initPGPKey(_ keyType: PgpKeyType) throws { + if let key = GopenpgpwrapperReadKey(AppKeychain.get(for: keyType.getKeychainKey())) { switch keyType { case .PUBLIC: self.publicKey = key @@ -232,16 +222,16 @@ public class PasswordStore { } throw AppError.KeyImport } - - public func initPGPKey(from url: URL, keyType: PGPKeyType) throws { + + public func initPGPKey(from url: URL, keyType: PgpKeyType) throws { let pgpKeyData = try Data(contentsOf: url) - AppKeychain.add(data: pgpKeyData, for: keyType.rawValue) + AppKeychain.add(data: pgpKeyData, for: keyType.getKeychainKey()) try initPGPKey(keyType) } - public func initPGPKey(with armorKey: String, keyType: PGPKeyType) throws { + public func initPGPKey(with armorKey: String, keyType: PgpKeyType) throws { let pgpKeyData = armorKey.data(using: .ascii)! - AppKeychain.add(data: pgpKeyData, for: keyType.rawValue) + AppKeychain.add(data: pgpKeyData, for: keyType.getKeychainKey()) try initPGPKey(keyType) } @@ -849,8 +839,8 @@ public class PasswordStore { SharedDefaults.remove(.pgpPrivateKeyURL) SharedDefaults.remove(.pgpPublicKeyURL) AppKeychain.removeContent(for: "pgpKeyPassphrase") - AppKeychain.removeContent(for: PGPKeyType.PUBLIC.rawValue) - AppKeychain.removeContent(for: PGPKeyType.PRIVATE.rawValue) + AppKeychain.removeContent(for: PgpKeyType.PUBLIC.getKeychainKey()) + AppKeychain.removeContent(for: PgpKeyType.PRIVATE.getKeychainKey()) publicKey = nil privateKey = nil } @@ -874,7 +864,7 @@ public class PasswordStore { if inFileSharing == false { return fm.fileExists(atPath: Globals.pgpPublicKeyPath) && fm.fileExists(atPath: Globals.pgpPrivateKeyPath) } else { - return fm.fileExists(atPath: Globals.iTunesFileSharingPGPPublic) && fm.fileExists(atPath: Globals.iTunesFileSharingPGPPrivate) + return KeyFileManager.PublicPgp.doesKeyFileExist() && KeyFileManager.PrivatePgp.doesKeyFileExist() } } @@ -883,16 +873,7 @@ public class PasswordStore { } public func pgpKeyImportFromFileSharing() throws { - guard let publicKeyFileContent = fm.contents(atPath: Globals.iTunesFileSharingPGPPublic) else { - throw AppError.ReadingFile(Globals.iTunesFileSharingPGPPublic) - } - AppKeychain.add(data: publicKeyFileContent, for: PGPKeyType.PUBLIC.rawValue) - try fm.removeItem(atPath: Globals.iTunesFileSharingPGPPublic) - - guard let privateKeyFileContent = fm.contents(atPath: Globals.iTunesFileSharingPGPPrivate) else { - throw AppError.ReadingFile(Globals.iTunesFileSharingPGPPrivate) - } - AppKeychain.add(data: privateKeyFileContent, for: PGPKeyType.PRIVATE.rawValue) - try fm.removeItem(atPath: Globals.iTunesFileSharingPGPPrivate) + try KeyFileManager.PublicPgp.importKeyAndDeleteFile() + try KeyFileManager.PrivatePgp.importKeyAndDeleteFile() } } diff --git a/passKitTests/Helpers/KeyFileManagerTest.swift b/passKitTests/Helpers/KeyFileManagerTest.swift new file mode 100644 index 0000000..ac9e289 --- /dev/null +++ b/passKitTests/Helpers/KeyFileManagerTest.swift @@ -0,0 +1,49 @@ +// +// KeyFileManagerTest.swift +// passKitTests +// +// Created by Danny Moesch on 01.07.19. +// Copyright © 2019 Bob Sun. All rights reserved. +// + +import XCTest + +@testable import passKit + +class KeyFileManagerTest: XCTestCase { + private static let filePath = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("test.txt").path + private static let keyFileManager = KeyFileManager(keyType: .PUBLIC, keyPath: filePath) { _, _ in } + + override func tearDown() { + try? FileManager.default.removeItem(atPath: KeyFileManagerTest.filePath) + super.tearDown() + } + + func testImportKeyAndDeleteFile() throws { + let fileContent = "content".data(using: .ascii) + var storage: [String: Data] = [:] + let keyFileManager = KeyFileManager(keyType: .PRIVATE, keyPath: KeyFileManagerTest.filePath) { storage[$1] = $0 } + + FileManager.default.createFile(atPath: KeyFileManagerTest.filePath, contents: fileContent, attributes: nil) + try keyFileManager.importKeyAndDeleteFile() + + XCTAssertFalse(FileManager.default.fileExists(atPath: KeyFileManagerTest.filePath)) + XCTAssertTrue(storage[PgpKeyType.PRIVATE.getKeychainKey()] == fileContent) + } + + func testErrorReadingFile() throws { + XCTAssertThrowsError(try KeyFileManagerTest.keyFileManager.importKeyAndDeleteFile()) { + XCTAssertEqual($0 as! AppError, AppError.ReadingFile("test.txt")) + } + } + + func testFileExists() { + FileManager.default.createFile(atPath: KeyFileManagerTest.filePath, contents: nil, attributes: nil) + + XCTAssertTrue(KeyFileManagerTest.keyFileManager.doesKeyFileExist()) + } + + func testFileDoesNotExist() { + XCTAssertFalse(KeyFileManagerTest.keyFileManager.doesKeyFileExist()) + } +}