Refactor GitCredential to simplify it and to add tests

pass.xcodeproj/project.pbxproj

@@ -25,6 +25,8 @@
 		30650E7323F847FC005CCD5E /* KeyImporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30650E7223F847FC005CCD5E /* KeyImporter.swift */; };
 		306623332406F1A8000E2AD6 /* PasswordGeneratorTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 306623322406F1A7000E2AD6 /* PasswordGeneratorTest.swift */; };
 		3066AD6823EE0D6500F65535 /* PGPKeyImporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3066AD6723EE0D6500F65535 /* PGPKeyImporter.swift */; };
+		30695E2024FA6C6500C9D46E /* PasswordAlertPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30695E1F24FA6C6500C9D46E /* PasswordAlertPresenter.swift */; };
+		30695E2524FAEF2600C9D46E /* GitCredentialTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30695E2424FAEF2600C9D46E /* GitCredentialTest.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 */; };
@@ -101,7 +103,6 @@
 		8BA607EB4C9C8258741AC18C /* Pods_passExtension.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 14E955B67C88672AA3A40BA0 /* Pods_passExtension.framework */; };
 		9A652414244BB33300DA0A41 /* UIAlertActionExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A652413244BB33300DA0A41 /* UIAlertActionExtension.swift */; };
 		9A8A8387402FCCCECB1232A4 /* Pods_passKitTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3B2B2F844061EFA534FE9506 /* Pods_passKitTests.framework */; };
-		9AA710CA23939C68009E3213 /* GitCredentialPassword.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9AA710C923939C68009E3213 /* GitCredentialPassword.swift */; };
 		9ADC954124418A5F0005402E /* PasswordStoreTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ADC954024418A5F0005402E /* PasswordStoreTest.swift */; };
 		A20691F41F2A3D0E0096483D /* SecurePasteboard.swift in Sources */ = {isa = PBXBuildFile; fileRef = A20691F31F2A3D0E0096483D /* SecurePasteboard.swift */; };
 		A217ACE41E9BBBBD00A1A6CF /* GitConfigSettingsTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = A217ACE31E9BBBBD00A1A6CF /* GitConfigSettingsTableViewController.swift */; };
@@ -281,6 +282,8 @@
 		30650E7223F847FC005CCD5E /* KeyImporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyImporter.swift; sourceTree = "<group>"; };
 		306623322406F1A7000E2AD6 /* PasswordGeneratorTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordGeneratorTest.swift; sourceTree = "<group>"; };
 		3066AD6723EE0D6500F65535 /* PGPKeyImporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PGPKeyImporter.swift; sourceTree = "<group>"; };
+		30695E1F24FA6C6500C9D46E /* PasswordAlertPresenter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PasswordAlertPresenter.swift; sourceTree = "<group>"; };
+		30695E2424FAEF2600C9D46E /* GitCredentialTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitCredentialTest.swift; sourceTree = "<group>"; };
 		30697C2321F63C580064FCAC /* NotificationNames.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NotificationNames.swift; sourceTree = "<group>"; };
 		30697C2421F63C590064FCAC /* Globals.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Globals.swift; sourceTree = "<group>"; };
 		30697C2521F63C590064FCAC /* FileManagerExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileManagerExtension.swift; sourceTree = "<group>"; };
@@ -361,7 +364,6 @@
 		9A1EF0B524C50EE00074FEAC /* passBetaExtension.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBetaExtension.entitlements; sourceTree = "<group>"; };
 		9A1EF0B624C50FEA0074FEAC /* passBetaShortcuts.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = passBetaShortcuts.entitlements; sourceTree = "<group>"; };
 		9A652413244BB33300DA0A41 /* UIAlertActionExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIAlertActionExtension.swift; sourceTree = "<group>"; };
-		9AA710C923939C68009E3213 /* GitCredentialPassword.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitCredentialPassword.swift; sourceTree = "<group>"; };
 		9ADC954024418A5F0005402E /* PasswordStoreTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordStoreTest.swift; sourceTree = "<group>"; };
 		A20691F31F2A3D0E0096483D /* SecurePasteboard.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SecurePasteboard.swift; sourceTree = "<group>"; };
 		A217ACE31E9BBBBD00A1A6CF /* GitConfigSettingsTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = GitConfigSettingsTableViewController.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
@@ -668,9 +670,10 @@
 		30C015A7214ED378005BB6DF /* Models */ = {
 			isa = PBXGroup;
 			children = (
+				30695E2424FAEF2600C9D46E /* GitCredentialTest.swift */,
+				9ADC954024418A5F0005402E /* PasswordStoreTest.swift */,
 				A2699ACE24027D9500F36323 /* PasswordTableEntryTest.swift */,
 				30B0485F209A5141001013CA /* PasswordTest.swift */,
-				9ADC954024418A5F0005402E /* PasswordStoreTest.swift */,
 			);
 			path = Models;
 			sourceTree = "<group>";
@@ -861,9 +864,9 @@
 			isa = PBXGroup;
 			children = (
 				A2BC54C71EEE5669001FAFBD /* Objective-CBridgingHeader.h */,
+				30695E1F24FA6C6500C9D46E /* PasswordAlertPresenter.swift */,
 				A20691F31F2A3D0E0096483D /* SecurePasteboard.swift */,
 				A2A61C1F1EEFABAD00CFE063 /* UtilsExtension.swift */,
-				9AA710C923939C68009E3213 /* GitCredentialPassword.swift */,
 			);
 			path = Helpers;
 			sourceTree = "<group>";
@@ -1571,6 +1574,7 @@
 				A2699ACF24027D9500F36323 /* PasswordTableEntryTest.swift in Sources */,
 				30FD2F78214D9E0E005E0A92 /* ParserTest.swift in Sources */,
 				A2AA934622DE3A8000D79A00 /* PGPAgentTest.swift in Sources */,
+				30695E2524FAEF2600C9D46E /* GitCredentialTest.swift in Sources */,
 				30BAC8C622E3BAAF00438475 /* TestBase.swift in Sources */,
 				30B04860209A5141001013CA /* PasswordTest.swift in Sources */,
 				30697C5F21F674800064FCAC /* String+UtilitiesTest.swift in Sources */,
@@ -1644,8 +1648,8 @@
 				DC037CB21E4CAB1700609409 /* AboutRepositoryTableViewController.swift in Sources */,
 				30650E7323F847FC005CCD5E /* KeyImporter.swift in Sources */,
 				A217ACE41E9BBBBD00A1A6CF /* GitConfigSettingsTableViewController.swift in Sources */,
-				9AA710CA23939C68009E3213 /* GitCredentialPassword.swift in Sources */,
 				DC037CB01E4CA51F00609409 /* GeneralSettingsTableViewController.swift in Sources */,
+				30695E2024FA6C6500C9D46E /* PasswordAlertPresenter.swift in Sources */,
 				DC037CB81E4DD1A500609409 /* AddPasswordTableViewController.swift in Sources */,
 				DCC441521E8F6C06008A90C4 /* RawPasswordViewController.swift in Sources */,
 				DC917BD71E2E8231000FDF54 /* AppDelegate.swift in Sources */,

pass/Controllers/GitRepositorySettingsTableViewController.swift

@@ -10,7 +10,7 @@ import passKit
 import SVProgressHUD
 import UIKit
 
-class GitRepositorySettingsTableViewController: UITableViewController {
+class GitRepositorySettingsTableViewController: UITableViewController, PasswordAlertPresenter {
     // MARK: - View Outlet
 
     @IBOutlet var gitURLTextField: UITextField!
@@ -25,6 +25,14 @@ class GitRepositorySettingsTableViewController: UITableViewController {
 
     private var sshLabel: UILabel?
     private let passwordStore = PasswordStore.shared
+    private let keychain = AppKeychain.shared
+    private var gitCredential: GitCredential {
+        GitCredential.from(
+            authenticationMethod: Defaults.gitAuthenticationMethod,
+            userName: Defaults.gitUsername,
+            keyStore: keychain
+        )
+    }
     private var gitAuthenticationMethod: GitAuthenticationMethod {
         get { Defaults.gitAuthenticationMethod }
         set {
@@ -48,16 +56,6 @@ class GitRepositorySettingsTableViewController: UITableViewController {
         set { Defaults.gitUsername = newValue }
     }
 
-    private var gitCredential: GitCredential {
-        switch Defaults.gitAuthenticationMethod {
-        case .password:
-            return GitCredential(credential: .http(userName: Defaults.gitUsername))
-        case .key:
-            let privateKey: String = AppKeychain.shared.get(for: SshKey.PRIVATE.getKeychainKey()) ?? ""
-            return GitCredential(credential: .ssh(userName: Defaults.gitUsername, privateKey: privateKey))
-        }
-    }
-
     // MARK: - View Controller Lifecycle
 
     override func viewDidLoad() {
@@ -72,7 +70,7 @@ class GitRepositorySettingsTableViewController: UITableViewController {
     override func viewWillAppear(_ animated: Bool) {
         super.viewWillAppear(animated)
         // Grey out ssh option if ssh_key is not present.
-        sshLabel?.isEnabled = AppKeychain.shared.contains(key: SshKey.PRIVATE.getKeychainKey())
+        sshLabel?.isEnabled = keychain.contains(key: SshKey.PRIVATE.getKeychainKey())
         updateAuthenticationMethodCheckView(for: gitAuthenticationMethod)
     }
 
@@ -97,7 +95,7 @@ class GitRepositorySettingsTableViewController: UITableViewController {
         if cell == authPasswordCell {
             gitAuthenticationMethod = .password
         } else if cell == authSSHKeyCell {
-            if !AppKeychain.shared.contains(key: SshKey.PRIVATE.getKeychainKey()) {
+            if !keychain.contains(key: SshKey.PRIVATE.getKeychainKey()) {
                 Utils.alert(title: "CannotSelectSshKey".localize(), message: "PleaseSetupSshKeyFirst.".localize(), controller: self)
                 gitAuthenticationMethod = .password
             } else {
@@ -177,11 +175,12 @@ class GitRepositorySettingsTableViewController: UITableViewController {
                     SVProgressHUD.showProgress(progress, status: "CheckingOutBranch".localize(self.gitBranchName))
                 }
 
+                let options = self.gitCredential.getCredentialOptions(passwordProvider: self.present)
+
                 try self.passwordStore.cloneRepository(
                     remoteRepoURL: self.gitUrl,
-                    credential: self.gitCredential,
                     branchName: self.gitBranchName,
-                    requestCredentialPassword: self.requestCredentialPassword,
+                    options: options,
                     transferProgressBlock: transferProgressBlock,
                     checkoutProgressBlock: checkoutProgressBlock
                 )
@@ -301,10 +300,6 @@ class GitRepositorySettingsTableViewController: UITableViewController {
         present(optionMenu, animated: true)
     }
 
-    private func requestCredentialPassword(credential: GitCredential.Credential, lastPassword: String?) -> String? {
-        requestGitCredentialPassword(credential: credential, lastPassword: lastPassword, controller: self)
-    }
-
     private func updateAuthenticationMethodCheckView(for method: GitAuthenticationMethod) {
         let passwordCheckView = authPasswordCell.viewWithTag(1001)
         let sshKeyCheckView = authSSHKeyCell.viewWithTag(1001)

pass/Controllers/PasswordsViewController.swift

@@ -10,7 +10,7 @@ import passKit
 import SVProgressHUD
 import UIKit
 
-class PasswordsViewController: UIViewController, UITableViewDataSource, UITableViewDelegate, UITabBarControllerDelegate, UISearchBarDelegate {
+class PasswordsViewController: UIViewController, UITableViewDataSource, UITableViewDelegate, UITabBarControllerDelegate, UISearchBarDelegate, PasswordAlertPresenter {
     // Arbitrary threshold to decide whether to show folders or not for only a few entries.
     private static let hideSectionHeaderThreshold = 6
 
@@ -19,6 +19,13 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
     private var parentPasswordEntity: PasswordEntity?
     private let passwordStore = PasswordStore.shared
     private let keychain = AppKeychain.shared
+    private var gitCredential: GitCredential {
+        GitCredential.from(
+            authenticationMethod: Defaults.gitAuthenticationMethod,
+            userName: Defaults.gitUsername,
+            keyStore: keychain
+        )
+    }
 
     private var tapTabBarTime: TimeInterval = 0
     private var tapNavigationBarGestureRecognizer: UITapGestureRecognizer!
@@ -30,16 +37,6 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
         case unsynced
     }
 
-    private var gitCredential: GitCredential {
-        switch Defaults.gitAuthenticationMethod {
-        case .password:
-            return GitCredential(credential: .http(userName: Defaults.gitUsername))
-        case .key:
-            let privateKey: String = AppKeychain.shared.get(for: SshKey.PRIVATE.getKeychainKey()) ?? ""
-            return GitCredential(credential: .ssh(userName: Defaults.gitUsername, privateKey: privateKey))
-        }
-    }
-
     private lazy var searchController: UISearchController = {
         let uiSearchController = UISearchController(searchResultsController: nil)
         uiSearchController.searchResultsUpdater = self
@@ -193,13 +190,15 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
 
         DispatchQueue.global(qos: .userInitiated).async { [unowned self] in
             do {
-                try self.passwordStore.pullRepository(credential: self.gitCredential, requestCredentialPassword: self.requestCredentialPassword) { git_transfer_progress, _ in
+                let pullOptions = self.gitCredential.getCredentialOptions(passwordProvider: self.present)
+                try self.passwordStore.pullRepository(options: pullOptions) { git_transfer_progress, _ in
                     DispatchQueue.main.async {
                         SVProgressHUD.showProgress(Float(git_transfer_progress.pointee.received_objects) / Float(git_transfer_progress.pointee.total_objects), status: "PullingFromRemoteRepository".localize())
                     }
                 }
                 if self.passwordStore.numberOfLocalCommits > 0 {
-                    try self.passwordStore.pushRepository(credential: self.gitCredential, requestCredentialPassword: self.requestCredentialPassword) { current, total, _, _ in
+                    let pushOptions = self.gitCredential.getCredentialOptions(passwordProvider: self.present)
+                    try self.passwordStore.pushRepository(options: pushOptions) { current, total, _, _ in
                         DispatchQueue.main.async {
                             SVProgressHUD.showProgress(Float(current) / Float(total), status: "PushingToRemoteRepository".localize())
                         }
@@ -212,6 +211,7 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
                     self.syncControl.endRefreshing()
                 }
             } catch {
+                self.gitCredential.delete()
                 DispatchQueue.main.async {
                     SVProgressHUD.dismiss()
                     self.syncControl.endRefreshing()
@@ -699,10 +699,6 @@ class PasswordsViewController: UIViewController, UITableViewDataSource, UITableV
         updateSearchResults(for: searchController)
         return true
     }
-
-    private func requestCredentialPassword(credential: GitCredential.Credential, lastPassword: String?) -> String? {
-        requestGitCredentialPassword(credential: credential, lastPassword: lastPassword, controller: self)
-    }
 }
 
 extension PasswordsViewController: UISearchResultsUpdating {

pass/Helpers/GitCredentialPassword.swift

@@ -1,53 +0,0 @@
-//
-//  GitCredentialPassword.swift
-//  pass
-//
-//  Created by Sun, Mingshen on 11/30/19.
-//  Copyright © 2019 Bob Sun. All rights reserved.
-//
-
-import Foundation
-import passKit
-import SVProgressHUD
-
-public func requestGitCredentialPassword(
-    credential: GitCredential.Credential,
-    lastPassword: String?,
-    controller: UIViewController
-) -> String? {
-    let sem = DispatchSemaphore(value: 0)
-    var password: String?
-    let message: String = {
-        switch credential {
-        case .http:
-            return "FillInGitAccountPassword.".localize()
-        case .ssh:
-            return "FillInSshKeyPassphrase.".localize()
-        }
-    }()
-
-    DispatchQueue.main.async {
-        SVProgressHUD.dismiss()
-        let alert = UIAlertController(title: "Password".localize(), message: message, preferredStyle: .alert)
-        alert.addTextField {
-            $0.text = lastPassword ?? ""
-            $0.isSecureTextEntry = true
-        }
-        alert.addAction(
-            UIAlertAction.ok { _ in
-                password = alert.textFields?.first?.text
-                sem.signal()
-            }
-        )
-        alert.addAction(
-            UIAlertAction.cancel { _ in
-                password = nil
-                sem.signal()
-            }
-        )
-        controller.present(alert, animated: true)
-    }
-
-    _ = sem.wait(timeout: .distantFuture)
-    return password
-}

pass/Helpers/PasswordAlertPresenter.swift

@@ -0,0 +1,45 @@
+//
+//  PasswordAlertPresenter.swift
+//  pass
+//
+//  Created by Danny Moesch on 23.08.20.
+//  Copyright © 2020 Bob Sun. All rights reserved.
+//
+
+import SVProgressHUD
+
+protocol PasswordAlertPresenter {
+    func present(message: String, lastPassword: String?) -> String?
+}
+
+extension PasswordAlertPresenter where Self: UIViewController {
+    func present(message: String, lastPassword: String?) -> String? {
+        let sem = DispatchSemaphore(value: 0)
+        var password: String?
+
+        DispatchQueue.main.async {
+            SVProgressHUD.dismiss()
+            let alert = UIAlertController(title: "Password".localize(), message: message, preferredStyle: .alert)
+            alert.addTextField {
+                $0.text = lastPassword ?? ""
+                $0.isSecureTextEntry = true
+            }
+            alert.addAction(
+                .ok { _ in
+                    password = alert.textFields?.first?.text
+                    sem.signal()
+                }
+            )
+            alert.addAction(
+                .cancel { _ in
+                    password = nil
+                    sem.signal()
+                }
+            )
+            self.present(alert, animated: true)
+        }
+
+        _ = sem.wait(timeout: .distantFuture)
+        return password
+    }
+}

passKit/Extensions/UIAlertActionExtension.swift

@@ -16,16 +16,12 @@ extension UIAlertAction {
         }
     }
 
-    public static func cancel(handler: ((UIAlertAction) -> Void)? = nil) -> UIAlertAction {
+    public static func cancel(title: String = "Cancel".localize(), handler: ((UIAlertAction) -> Void)? = nil) -> UIAlertAction {
-        cancel(with: "Cancel", handler: handler)
+        UIAlertAction(title: title, style: .cancel, handler: handler)
     }
 
     public static func dismiss(handler: ((UIAlertAction) -> Void)? = nil) -> UIAlertAction {
-        cancel(with: "Dismiss", handler: handler)
+        cancel(title: "Dismiss".localize(), handler: handler)
-    }
-
-    public static func cancel(with _: String, handler: ((UIAlertAction) -> Void)? = nil) -> UIAlertAction {
-        UIAlertAction(title: "Cancel".localize(), style: .cancel, handler: handler)
     }
 
     public static func ok(handler: ((UIAlertAction) -> Void)? = nil) -> UIAlertAction {

passKit/Models/GitCredential.swift

@@ -6,75 +6,103 @@
 //  Copyright © 2017 Bob Sun. All rights reserved.
 //
 
-import Foundation
 import ObjectiveGit
+import SVProgressHUD
 
 public struct GitCredential {
-    private var credential: Credential
+    public typealias PasswordProvider = (String, String?) -> String?
-    private let passwordStore = PasswordStore.shared
 
-    public enum Credential {
+    private let credentialType: CredentialType
+    private let keyStore: KeyStore
+
+    private enum CredentialType {
         case http(userName: String)
         case ssh(userName: String, privateKey: String)
-    }
 
-    public init(credential: Credential) {
+        var requestPassphraseMessage: String {
-        self.credential = credential
+            switch self {
-    }
+            case .http:
-
+                return "FillInGitAccountPassword.".localize()
-    public func credentialProvider(requestCredentialPassword: @escaping (Credential, String?) -> String?) throws -> GTCredentialProvider {
+            case .ssh:
-        var attempts = 0
+                return "FillInSshKeyPassphrase.".localize()
-        return GTCredentialProvider { _, _, _ -> (GTCredential?) in
-            var credential: GTCredential?
-
-            switch self.credential {
-            case let .http(userName):
-                if attempts > 3 {
-                    // After too many failures (say six), the error message "failed to authenticate ssh session" might be confusing.
-                    return nil
-                }
-                var lastPassword = self.passwordStore.gitPassword
-                if lastPassword == nil || attempts != 0 {
-                    if let requestedPassword = requestCredentialPassword(self.credential, lastPassword) {
-                        if Defaults.isRememberGitCredentialPassphraseOn {
-                            self.passwordStore.gitPassword = requestedPassword
-                        }
-                        lastPassword = requestedPassword
-                    } else {
-                        return nil
-                    }
-                }
-                attempts += 1
-                credential = try? GTCredential(userName: userName, password: lastPassword!)
-            case let .ssh(userName, privateKey):
-                if attempts > 0 {
-                    // The passphrase seems correct, but the previous authentification failed.
-                    return nil
-                }
-                var lastPassword = self.passwordStore.gitSSHPrivateKeyPassphrase
-                if lastPassword == nil || attempts != 0 {
-                    if let requestedPassword = requestCredentialPassword(self.credential, lastPassword) {
-                        if Defaults.isRememberGitCredentialPassphraseOn {
-                            self.passwordStore.gitSSHPrivateKeyPassphrase = requestedPassword
-                        }
-                        lastPassword = requestedPassword
-                    } else {
-                        return nil
-                    }
-                }
-                attempts += 1
-                credential = try? GTCredential(userName: userName, publicKeyString: nil, privateKeyString: privateKey, passphrase: lastPassword!)
             }
-            return credential
+        }
+
+        var keyStoreKey: String {
+            switch self {
+            case .http:
+                return Globals.gitPassword
+            case .ssh:
+                return Globals.gitSSHPrivateKeyPassphrase
+            }
+        }
+
+        var allowedAttempts: Int {
+            switch self {
+            case .http:
+                return 4
+            case .ssh:
+                return 1
+            }
+        }
+
+        func createGTCredential(password: String) throws -> GTCredential {
+            switch self {
+            case let .http(userName):
+                return try GTCredential(userName: userName, password: password)
+            case let .ssh(userName, privateKey):
+                return try GTCredential(userName: userName, publicKeyString: nil, privateKeyString: privateKey, passphrase: password)
+            }
+        }
+    }
+
+    public static func from(authenticationMethod: GitAuthenticationMethod, userName: String, keyStore: KeyStore) -> Self {
+        switch authenticationMethod {
+        case .password:
+            return Self(credentialType: .http(userName: userName), keyStore: keyStore)
+        case .key:
+            let privateKey: String = keyStore.get(for: SshKey.PRIVATE.getKeychainKey()) ?? ""
+            return Self(credentialType: .ssh(userName: userName, privateKey: privateKey), keyStore: keyStore)
+        }
+    }
+
+    public func getCredentialOptions(passwordProvider: @escaping PasswordProvider = { _, _ in nil }) -> [String: Any] {
+        let credentialProvider = createCredentialProvider(passwordProvider)
+        return [
+            GTRepositoryCloneOptionsCredentialProvider: credentialProvider,
+            GTRepositoryRemoteOptionsCredentialProvider: credentialProvider,
+        ]
+    }
+
+    private func createCredentialProvider(_ passwordProvider: @escaping PasswordProvider) -> GTCredentialProvider {
+        var attempts = 1
+        return GTCredentialProvider { _, _, _ -> GTCredential? in
+            if attempts > self.credentialType.allowedAttempts {
+                return nil
+            }
+            guard let password = self.getPassword(attempts: attempts, passwordProvider: passwordProvider) else {
+                return nil
+            }
+            attempts += 1
+            return try? self.credentialType.createGTCredential(password: password)
         }
     }
 
     public func delete() {
-        switch credential {
+        keyStore.removeContent(for: credentialType.keyStoreKey)
-        case .http:
+    }
-            passwordStore.gitPassword = nil
+
-        case .ssh:
+    private func getPassword(attempts: Int, passwordProvider: @escaping PasswordProvider) -> String? {
-            passwordStore.gitSSHPrivateKeyPassphrase = nil
+        let lastPassword: String? = keyStore.get(for: credentialType.keyStoreKey)
+        if lastPassword == nil || attempts != 1 {
+            guard let requestedPassword = passwordProvider(credentialType.requestPassphraseMessage, lastPassword) else {
+                return nil
+            }
+            if Defaults.isRememberGitCredentialPassphraseOn {
+                keyStore.add(string: requestedPassword, for: credentialType.keyStoreKey)
+            }
+            return requestedPassword
         }
+        return lastPassword
     }
 }

passKit/Models/PasswordStore.swift

@@ -173,35 +173,10 @@ public class PasswordStore {
 
     public func cloneRepository(
         remoteRepoURL: URL,
-        credential: GitCredential,
         branchName: String,
-        requestCredentialPassword: @escaping (GitCredential.Credential, String?) -> String?,
-        transferProgressBlock: @escaping (UnsafePointer<git_transfer_progress>, UnsafeMutablePointer<ObjCBool>) -> Void,
-        checkoutProgressBlock: @escaping (String, UInt, UInt) -> Void
-    ) throws {
-        do {
-            let credentialProvider = try credential.credentialProvider(requestCredentialPassword: requestCredentialPassword)
-            let options = [GTRepositoryCloneOptionsCredentialProvider: credentialProvider]
-            try cloneRepository(
-                remoteRepoURL: remoteRepoURL,
-                branchName: branchName,
-                transferProgressBlock: transferProgressBlock,
-                checkoutProgressBlock: checkoutProgressBlock,
-                options: options
-            )
-        } catch {
-            credential.delete()
-            throw (error)
-        }
-    }
-
-    public func cloneRepository(
-        remoteRepoURL: URL,
-        branchName: String,
-        transferProgressBlock: @escaping (UnsafePointer<git_transfer_progress>, UnsafeMutablePointer<ObjCBool>) -> Void,
-        checkoutProgressBlock: @escaping (String, UInt, UInt) -> Void,
         options: [AnyHashable: Any]? = nil,
-        completion: @escaping () -> Void = {}
+        transferProgressBlock: @escaping (UnsafePointer<git_transfer_progress>, UnsafeMutablePointer<ObjCBool>) -> Void = { _, _ in },
+        checkoutProgressBlock: @escaping (String, UInt, UInt) -> Void = { _, _, _ in }
     ) throws {
         try? fm.removeItem(at: storeURL)
         try? fm.removeItem(at: tempStoreURL)
@@ -231,7 +206,6 @@ public class PasswordStore {
         DispatchQueue.main.async {
             self.updatePasswordEntityCoreData()
             NotificationCenter.default.post(name: .passwordStoreUpdated, object: nil)
-            completion()
         }
     }
 
@@ -252,15 +226,12 @@ public class PasswordStore {
     }
 
     public func pullRepository(
-        credential: GitCredential,
+        options: [String: Any],
-        requestCredentialPassword: @escaping (GitCredential.Credential, String?) -> String?,
+        progressBlock: @escaping (UnsafePointer<git_transfer_progress>, UnsafeMutablePointer<ObjCBool>) -> Void = { _, _ in }
-        progressBlock: @escaping (UnsafePointer<git_transfer_progress>, UnsafeMutablePointer<ObjCBool>) -> Void
     ) throws {
         guard let storeRepository = storeRepository else {
             throw AppError.RepositoryNotSet
         }
-        let credentialProvider = try credential.credentialProvider(requestCredentialPassword: requestCredentialPassword)
-        let options = [GTRepositoryRemoteOptionsCredentialProvider: credentialProvider]
         let remote = try GTRemote(name: "origin", in: storeRepository)
         try storeRepository.pull(storeRepository.currentBranch(), from: remote, withOptions: options, progress: progressBlock)
         Defaults.lastSyncedTime = Date()
@@ -472,12 +443,13 @@ public class PasswordStore {
         return branches.first
     }
 
-    public func pushRepository(credential: GitCredential, requestCredentialPassword: @escaping (GitCredential.Credential, String?) -> String?, transferProgressBlock: @escaping (UInt32, UInt32, Int, UnsafeMutablePointer<ObjCBool>) -> Void) throws {
+    public func pushRepository(
+        options: [String: Any],
+        transferProgressBlock: @escaping (UInt32, UInt32, Int, UnsafeMutablePointer<ObjCBool>) -> Void = { _, _, _, _ in }
+    ) throws {
         guard let storeRepository = storeRepository else {
             throw AppError.RepositoryNotSet
         }
-        let credentialProvider = try credential.credentialProvider(requestCredentialPassword: requestCredentialPassword)
-        let options = [GTRepositoryRemoteOptionsCredentialProvider: credentialProvider]
         if let branch = try getLocalBranch(withName: Defaults.gitBranchName) {
             let remote = try GTRemote(name: "origin", in: storeRepository)
             try storeRepository.push(branch, to: remote, withOptions: options, progress: transferProgressBlock)
@@ -741,9 +713,5 @@ public func findGPGID(from url: URL) -> String {
     }
     path = path.appendingPathComponent(".gpg-id")
 
-    do {
+    return (try? String(contentsOf: path))?.trimmed ?? ""
-        return try String(contentsOf: path).trimmed
-    } catch {
-        return ""
-    }
 }

passKitTests/Models/GitCredentialTest.swift

@@ -0,0 +1,107 @@
+//
+//  GitCredentialTest.swift
+//  passKitTests
+//
+//  Created by Danny Moesch on 29.08.20.
+//  Copyright © 2020 Bob Sun. All rights reserved.
+//
+
+import XCTest
+
+import ObjectiveGit
+import SwiftyUserDefaults
+@testable import passKit
+
+class GitCredentialTest: XCTestCase {
+    private static let defaultsID = "SharedDefaultsForGitCredentialTest"
+
+    private let keyStore = DictBasedKeychain()
+
+    override func setUp() {
+        super.setUp()
+
+        keyStore.add(string: "password", for: Globals.gitPassword)
+        keyStore.add(string: "passphrase", for: Globals.gitSSHPrivateKeyPassphrase)
+
+        UserDefaults().removePersistentDomain(forName: Self.defaultsID)
+        passKit.Defaults = DefaultsAdapter(defaults: UserDefaults(suiteName: Self.defaultsID)!, keyStore: DefaultsKeys())
+    }
+
+    override func tearDown() {
+        UserDefaults().removePersistentDomain(forName: Self.defaultsID)
+
+        super.tearDown()
+    }
+
+    func testDelete() {
+        let password = GitCredential.from(authenticationMethod: .password, userName: "user", keyStore: keyStore)
+        password.delete()
+        XCTAssertFalse(keyStore.contains(key: Globals.gitPassword))
+        XCTAssertTrue(keyStore.contains(key: Globals.gitSSHPrivateKeyPassphrase))
+
+        let key = GitCredential.from(authenticationMethod: .key, userName: "user", keyStore: keyStore)
+        key.delete()
+        XCTAssertFalse(keyStore.contains(key: Globals.gitPassword))
+        XCTAssertFalse(keyStore.contains(key: Globals.gitSSHPrivateKeyPassphrase))
+    }
+
+    func testOptions() {
+        let password = GitCredential.from(authenticationMethod: .password, userName: "user", keyStore: keyStore)
+
+        let options = password.getCredentialOptions()
+        XCTAssertEqual(options.count, 2)
+
+        let cloneCredentialProvider = options[GTRepositoryCloneOptionsCredentialProvider] as! GTCredentialProvider
+        let remoteCredentialProvider = options[GTRepositoryRemoteOptionsCredentialProvider] as! GTCredentialProvider
+        XCTAssertNotNil(cloneCredentialProvider)
+        XCTAssertEqual(cloneCredentialProvider, remoteCredentialProvider)
+    }
+
+    func testPasswordCredentialProvider() {
+        let password = GitCredential.from(authenticationMethod: .password, userName: "user", keyStore: keyStore)
+        let expectation = self.expectation(description: "Password is requested.")
+        expectation.assertForOverFulfill = true
+        expectation.expectedFulfillmentCount = 3
+        let options = password.getCredentialOptions { _, _ in
+            expectation.fulfill()
+            return "otherPassword"
+        }
+        let credentialProvider = options[GTRepositoryCloneOptionsCredentialProvider] as! GTCredentialProvider
+
+        (1 ..< 5).forEach { _ in
+            XCTAssertNotNil(credentialProvider.credential(for: .userPassPlaintext, url: nil, userName: nil))
+        }
+        XCTAssertNil(credentialProvider.credential(for: .userPassPlaintext, url: nil, userName: nil))
+        wait(for: [expectation], timeout: 0)
+    }
+
+    func testSSHKeyCredentialProvider() {
+        let credentialProvider = getCredentialProvider(authenticationMethod: .key)
+
+        XCTAssertNotNil(credentialProvider.credential(for: .sshCustom, url: nil, userName: nil))
+        XCTAssertNil(credentialProvider.credential(for: .sshCustom, url: nil, userName: nil))
+    }
+
+    func testCannotGetPassword() {
+        let credentialProvider = getCredentialProvider(authenticationMethod: .password)
+
+        XCTAssertNotNil(credentialProvider.credential(for: .userPassPlaintext, url: nil, userName: nil))
+        XCTAssertNil(credentialProvider.credential(for: .userPassPlaintext, url: nil, userName: nil))
+    }
+
+    func testSaveToKeyStore() {
+        let credentialProvider = getCredentialProvider(authenticationMethod: .key, password: "otherPassword")
+
+        passKit.Defaults.isRememberGitCredentialPassphraseOn = true
+        keyStore.removeAllContent()
+        credentialProvider.credential(for: .sshCustom, url: nil, userName: nil)
+
+        XCTAssertEqual(keyStore.get(for: Globals.gitSSHPrivateKeyPassphrase), "otherPassword")
+    }
+
+    private func getCredentialProvider(authenticationMethod: GitAuthenticationMethod, password: String? = nil) -> GTCredentialProvider {
+        let credential = GitCredential.from(authenticationMethod: authenticationMethod, userName: "user", keyStore: keyStore)
+        let options = credential.getCredentialOptions { _, _ in password }
+        return options[GTRepositoryCloneOptionsCredentialProvider] as! GTCredentialProvider
+    }
+}

passKitTests/Models/PasswordStoreTest.swift

@@ -12,27 +12,13 @@ import XCTest
 @testable import passKit
 
 class PasswordStoreTest: XCTestCase {
-    let cloneOptions: [String: GTCredentialProvider] = {
+    private let remoteRepoURL = URL(string: "https://github.com/mssun/passforios-password-store.git")!
-        let credentialProvider = GTCredentialProvider { _, _, _ -> (GTCredential?) in
-            try? GTCredential(userName: "", password: "")
-        }
-        return [GTRepositoryCloneOptionsCredentialProvider: credentialProvider]
-    }()
-
-    let remoteRepoURL = URL(string: "https://github.com/mssun/passforios-password-store.git")!
 
     func testCloneAndDecryptMultiKeys() throws {
         let url = URL(fileURLWithPath: "\(Globals.repositoryPath)-test")
         let passwordStore = PasswordStore(url: url)
-        let expectation = self.expectation(description: "clone")
+        try passwordStore.cloneRepository(remoteRepoURL: remoteRepoURL, branchName: "master")
-        try passwordStore.cloneRepository(
+        expectation(for: NSPredicate { _, _ in FileManager.default.fileExists(atPath: url.path) }, evaluatedWith: nil)
-            remoteRepoURL: remoteRepoURL,
-            branchName: "master",
-            transferProgressBlock: { _, _ in },
-            checkoutProgressBlock: { _, _, _ in },
-            options: cloneOptions,
-            completion: { expectation.fulfill() }
-        )
         waitForExpectations(timeout: 3, handler: nil)
 
         [

passShortcuts/SyncRepositoryIntentHandler.swift

@@ -14,13 +14,11 @@ public class SyncRepositoryIntentHandler: NSObject, SyncRepositoryIntentHandling
     private let keychain = AppKeychain.shared
 
     private var gitCredential: GitCredential {
-        switch Defaults.gitAuthenticationMethod {
+        GitCredential.from(
-        case .password:
+            authenticationMethod: Defaults.gitAuthenticationMethod,
-            return GitCredential(credential: .http(userName: Defaults.gitUsername))
+            userName: Defaults.gitUsername,
-        case .key:
+            keyStore: keychain
-            let privateKey: String = keychain.get(for: SshKey.PRIVATE.getKeychainKey()) ?? ""
+        )
-            return GitCredential(credential: .ssh(userName: Defaults.gitUsername, privateKey: privateKey))
-        }
     }
 
     public func handle(intent _: SyncRepositoryIntent, completion: @escaping (SyncRepositoryIntentResponse) -> Void) {
@@ -33,14 +31,14 @@ public class SyncRepositoryIntentHandler: NSObject, SyncRepositoryIntentHandling
             return
         }
         do {
-            try passwordStore.pullRepository(credential: gitCredential, requestCredentialPassword: { _, _ in nil }, progressBlock: { _, _ in })
+            try passwordStore.pullRepository(options: gitCredential.getCredentialOptions())
         } catch {
             completion(SyncRepositoryIntentResponse(code: .pullFailed, userActivity: nil))
             return
         }
         if passwordStore.numberOfLocalCommits > 0 {
             do {
-                try passwordStore.pushRepository(credential: gitCredential, requestCredentialPassword: { _, _ in nil }, transferProgressBlock: { _, _, _, _ in })
+                try passwordStore.pushRepository(options: gitCredential.getCredentialOptions())
             } catch {
                 completion(SyncRepositoryIntentResponse(code: .pushFailed, userActivity: nil))
                 return