deduplicate decrypt logic in PGPAgent

This commit is contained in:
Lysann Tranvouez 2026-03-11 16:30:53 +01:00
parent b103337083
commit 054f333bac
2 changed files with 10 additions and 147 deletions

View file

@ -53,36 +53,13 @@ public class PGPAgent {
return pgpInterface?.getShortKeyIDs(type: type).sorted() ?? []
}
public func decrypt(encryptedData: Data, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? {
// Remember the previous status and set the current status
let previousDecryptStatus = latestDecryptStatus
latestDecryptStatus = false
// Init keys.
try checkAndInit()
// Get the PGP key passphrase.
let providePassPhraseForKey = { (selectedKeyID: String) -> String in
if previousDecryptStatus == false {
return requestPGPKeyPassphrase(selectedKeyID)
}
return self.keyStore.get(for: AppKeychain.getPGPKeyPassphraseKey(keyID: selectedKeyID)) ?? requestPGPKeyPassphrase(selectedKeyID)
}
// Decrypt.
guard let result = try pgpInterface!.decrypt(encryptedData: encryptedData, keyIDHint: nil, passPhraseForKey: providePassPhraseForKey) else {
return nil
}
// The decryption step has succeed.
latestDecryptStatus = true
return result
}
public func decrypt(encryptedData: Data, keyID: String, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? {
// Init keys.
public func decrypt(encryptedData: Data, keyID: String? = nil, requestPGPKeyPassphrase: @escaping (String) -> String) throws -> Data? {
try checkAndInit()
guard let pgpInterface else {
throw AppError.decryption
}
if !pgpInterface.containsPrivateKey(with: keyID) {
if let keyID, !pgpInterface.containsPrivateKey(with: keyID) {
throw AppError.pgpPrivateKeyNotFound(keyID: keyID)
}

View file

@ -125,7 +125,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
/// After a failed decrypt (latestDecryptStatus=false), requestPGPKeyPassphrase is ALWAYS called,
/// even if the keystore has a cached passphrase.
func testDecryptWithKeyID_afterFailure_alwaysRequestsPassphrase() throws {
func testDecrypt_afterFailure_alwaysRequestsPassphrase() throws {
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
keychain.add(string: "stored-passphrase", for: AppKeychain.getPGPKeyPassphraseKey(keyID: longFingerprint))
@ -148,7 +148,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
}
/// After a successful decrypt, the next call uses keystore first (latestDecryptStatus=true).
func testDecryptWithKeyID_afterSuccess_usesKeystoreFirst() throws {
func testDecrypt_afterSuccess_usesKeystoreFirst() throws {
let shortID = "a1024dae"
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
@ -173,7 +173,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
// MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - Return Values & Error Propagation
/// When pgpInterface.decrypt returns nil, agent.decrypt returns nil.
func testDecryptWithKeyID_interfaceReturnsNil_returnsNil() throws {
func testDecrypt_interfaceReturnsNil_returnsNil() throws {
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
mockPGP.decryptResult = nil
@ -185,7 +185,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
/// When pgpInterface.decrypt returns nil, latestDecryptStatus stays false
/// (next call will always request passphrase).
func testDecryptWithKeyID_interfaceReturnsNil_statusStaysFalse() throws {
func testDecrypt_interfaceReturnsNil_statusStaysFalse() throws {
let shortID = "d862027e"
let longFingerprint = "787eae1a5fa3e749aa34cc6aa0645ebed862027e"
mockPGP.privateKeyIDs = [longFingerprint]
@ -209,7 +209,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
}
/// When pgpInterface.decrypt throws, the error propagates and latestDecryptStatus stays false.
func testDecryptWithKeyID_interfaceThrows_propagatesError() throws {
func testDecrypt_interfaceThrows_propagatesError() throws {
let shortID = "a1024dae"
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
@ -238,7 +238,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
}
/// After successful decrypt, latestDecryptStatus is true.
func testDecryptWithKeyID_success_setsStatusTrue() throws {
func testDecrypt_success_setsStatusTrue() throws {
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
@ -269,7 +269,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
/// checkAndInit re-initializes if pgpKeyPassphrase is missing from keystore.
/// Since we're using a mock as pgpInterface, initKeys would overwrite it; verify the precondition holds.
func testDecryptWithKeyID_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws {
func testDecrypt_checkAndInit_requiresPGPKeyPassphraseInKeystore() throws {
// Remove the pgpKeyPassphrase sentinel, which will trigger checkAndInit -> initKeys.
keychain.removeContent(for: Globals.pgpKeyPassphrase)
// initKeys needs real PGP keys, which we don't have. It should throw keyImport.
@ -279,7 +279,7 @@ final class PGPAgentLowLevelTests: XCTestCase {
XCTAssertEqual(passphraseRequests, [], "requestPGPKeyPassphrase should not be called when checkAndInit fails")
}
// MARK: - decrypt(encryptedData:requestPGPKeyPassphrase:) - No KeyID Overload
// MARK: - decrypt(encryptedData:keyID:requestPGPKeyPassphrase:) - nil keyID
/// The no-keyID overload passes nil as keyID to pgpInterface.decrypt
func testDecryptNoKeyID_passesNilKeyIDToInterface() throws {
@ -290,70 +290,6 @@ final class PGPAgentLowLevelTests: XCTestCase {
XCTAssertNil(mockPGP.decryptCalls[0].keyID)
}
/// After failure, the no-keyID overload always requests passphrase.
func testDecryptNoKeyID_afterFailure_alwaysRequestsPassphrase() throws {
// Force a failure.
mockPGP.decryptError = AppError.wrongPassphrase
_ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad"))
// Next one can succeed
keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: ""))
mockPGP.decryptError = nil
mockPGP.decryptCalls.removeAll()
mockPGP.resolvedPassphrases.removeAll()
mockPGP.selectedKeyForPassphrase = ""
passphraseRequests.removeAll()
_ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh"))
XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"])
XCTAssertEqual(passphraseRequests, [""])
}
/// The no-keyID overload returns nil when interface returns nil.
func testDecryptNoKeyID_interfaceReturnsNil_returnsNil() throws {
mockPGP.decryptResult = nil
let result = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))
XCTAssertNil(result)
}
/// The no-keyID overload propagates errors from the interface.
func testDecryptNoKeyID_interfaceThrows_propagatesError() {
mockPGP.decryptError = AppError.wrongPassphrase
XCTAssertThrowsError(try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))) { error in
XCTAssertEqual(error as? AppError, AppError.wrongPassphrase)
}
}
/// The no-keyID overload sets latestDecryptStatus to true on success,
/// which affects subsequent passphrase lookups.
func testDecryptNoKeyID_success_setsStatusTrue() throws {
// Force failure first.
mockPGP.decryptError = AppError.wrongPassphrase
_ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad"))
mockPGP.decryptError = nil
mockPGP.decryptCalls.removeAll()
passphraseRequests.removeAll()
// Succeed.
_ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("good"))
// Third call: should try keystore.
keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: ""))
mockPGP.decryptCalls.removeAll()
mockPGP.resolvedPassphrases.removeAll()
mockPGP.selectedKeyForPassphrase = ""
passphraseRequests.removeAll()
_ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("nope"))
XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached"])
XCTAssertEqual(passphraseRequests, [])
}
/// The no-keyID overload doesn't check containsPrivateKey.
func testDecryptNoKeyID_doesNotCheckPrivateKey() throws {
_ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("pass"))
@ -384,56 +320,6 @@ final class PGPAgentLowLevelTests: XCTestCase {
XCTAssertEqual(mockPGP.resolvedPassphrases, ["cached-pass"])
}
// MARK: - Cross-overload latestDecryptStatus interaction
/// latestDecryptStatus is shared between both overloads.
/// A failure in the keyID overload affects the no-keyID overload.
func testDecryptStatusSharedBetweenOverloads_failureInKeyIDOverload() throws {
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
// Fail via keyID overload.
mockPGP.decryptError = AppError.wrongPassphrase
_ = try? agent.decrypt(encryptedData: testEncryptedData, keyID: longFingerprint, requestPGPKeyPassphrase: passphraseCallback("bad"))
// Next call via no-keyID overload should always request.
keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: ""))
mockPGP.decryptError = nil
mockPGP.decryptCalls.removeAll()
mockPGP.resolvedPassphrases.removeAll()
mockPGP.selectedKeyForPassphrase = ""
passphraseRequests.removeAll()
_ = try agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("fresh"))
XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"])
XCTAssertEqual(passphraseRequests, [""], "Failure in keyID overload should affect no-keyID overload")
}
/// A failure in the no-keyID overload affects the keyID overload.
func testDecryptStatusSharedBetweenOverloads_failureInNoKeyIDOverload() throws {
let shortID = "a1024dae"
let longFingerprint = "4712286271220db299883ea7062e678da1024dae"
mockPGP.privateKeyIDs = [longFingerprint]
// Fail via no-keyID overload.
mockPGP.decryptError = AppError.wrongPassphrase
_ = try? agent.decrypt(encryptedData: testEncryptedData, requestPGPKeyPassphrase: passphraseCallback("bad"))
// Next call via keyID overload should always request.
mockPGP.decryptError = nil
mockPGP.decryptCalls.removeAll()
mockPGP.resolvedPassphrases.removeAll()
mockPGP.selectedKeyForPassphrase = shortID
keychain.add(string: "cached", for: AppKeychain.getPGPKeyPassphraseKey(keyID: shortID))
passphraseRequests.removeAll()
_ = try agent.decrypt(encryptedData: testEncryptedData, keyID: shortID, requestPGPKeyPassphrase: passphraseCallback("fresh"))
XCTAssertEqual(mockPGP.resolvedPassphrases, ["fresh"])
XCTAssertEqual(passphraseRequests, [shortID], "Failure in no-keyID overload should affect keyID overload")
}
// MARK: - Short vs long key ID behavior
/// When caller passes a short ID and containsPrivateKey matches it (via suffix), the short ID