From f87cc1da0b93b119512e5e71558d7b70ff626f9f Mon Sep 17 00:00:00 2001 From: Davide Date: Sun, 1 Dec 2024 21:33:58 +0100 Subject: [PATCH] Fix OpenVPN/WireGuard import error messages (#967) OpenVPN parser was indirectly swallowing WireGuard errors. --- .github/workflows/test.yml | 3 +- Library/Package.resolved | 5 +- Library/Package.swift | 8 +-- .../AppUIMain/Business/ProfileImporter.swift | 12 ++--- .../Views/App/ProfileImporterModifier.swift | 4 +- .../AppUIMainTests/ProfileImporterTests.swift | 49 ++++++++++--------- 6 files changed, 44 insertions(+), 37 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 607435a2..227fa26e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,6 +23,7 @@ jobs: - uses: passepartoutvpn/action-prepare-xcode-build@master with: access_token: ${{ secrets.ACCESS_TOKEN }} - - run: | + - name: "Run tests" + run: | cd Library swift test diff --git a/Library/Package.resolved b/Library/Package.resolved index 71dc8df0..9ec767f9 100644 --- a/Library/Package.resolved +++ b/Library/Package.resolved @@ -41,7 +41,7 @@ "kind" : "remoteSourceControl", "location" : "git@github.com:passepartoutvpn/passepartoutkit-source", "state" : { - "revision" : "5211efce28b6d96a38747a6004a4bf03e169b27c" + "revision" : "aa85d745f8419def59ef630501d71a491a32829e" } }, { @@ -58,7 +58,8 @@ "kind" : "remoteSourceControl", "location" : "git@github.com:passepartoutvpn/passepartoutkit-source-wireguard-go", "state" : { - "revision" : "68fceaa664913988b2d9053405738682a30b87b8" + "revision" : "c2bfbf8744e04571a3e858385f49c98863fe9616", + "version" : "0.12.0" } }, { diff --git a/Library/Package.swift b/Library/Package.swift index 46ebe707..f0b88400 100644 --- a/Library/Package.swift +++ b/Library/Package.swift @@ -51,14 +51,14 @@ let package = Package( ) ], dependencies: [ -// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.11.0"), - .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "5211efce28b6d96a38747a6004a4bf03e169b27c"), +// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", from: "0.12.0"), + .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source", revision: "aa85d745f8419def59ef630501d71a491a32829e"), // .package(path: "../../passepartoutkit-source"), .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", from: "0.9.1"), // .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-openvpn-openssl", revision: "031863a1cd683962a7dfe68e20b91fa820a1ecce"), // .package(path: "../../passepartoutkit-source-openvpn-openssl"), -// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-wireguard-go", from: "0.9.2"), - .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-wireguard-go", revision: "68fceaa664913988b2d9053405738682a30b87b8"), + .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-wireguard-go", from: "0.12.0"), +// .package(url: "git@github.com:passepartoutvpn/passepartoutkit-source-wireguard-go", revision: "68fceaa664913988b2d9053405738682a30b87b8"), // .package(path: "../../passepartoutkit-source-wireguard-go"), .package(url: "https://github.com/Cocoanetics/Kvitto", from: "1.0.0") ], diff --git a/Library/Sources/AppUIMain/Business/ProfileImporter.swift b/Library/Sources/AppUIMain/Business/ProfileImporter.swift index e23e28a0..1916faf9 100644 --- a/Library/Sources/AppUIMain/Business/ProfileImporter.swift +++ b/Library/Sources/AppUIMain/Business/ProfileImporter.swift @@ -46,7 +46,7 @@ final class ProfileImporter: ObservableObject { func tryImport( urls: [URL], profileManager: ProfileManager, - registry: Registry + importer: ModuleImporter ) async throws { var withPassphrase: [URL] = [] @@ -56,7 +56,7 @@ final class ProfileImporter: ObservableObject { url, withPassphrase: nil, profileManager: profileManager, - registry: registry + importer: importer ) } catch { if let error = error as? PassepartoutError, error.code == .OpenVPN.passphraseRequired { @@ -74,13 +74,13 @@ final class ProfileImporter: ObservableObject { } } - func reImport(url: URL, profileManager: ProfileManager, registry: Registry) async throws { + func reImport(url: URL, profileManager: ProfileManager, importer: ModuleImporter) async throws { do { try await importURL( url, withPassphrase: currentPassphrase, profileManager: profileManager, - registry: registry + importer: importer ) urlsRequiringPassphrase.removeFirst() scheduleNextImport() @@ -113,7 +113,7 @@ private extension ProfileImporter { _ url: URL, withPassphrase passphrase: String?, profileManager: ProfileManager, - registry: Registry + importer: ModuleImporter ) async throws { guard url.startAccessingSecurityScopedResource() else { throw AppError.permissionDenied @@ -122,7 +122,7 @@ private extension ProfileImporter { url.stopAccessingSecurityScopedResource() } - let module = try registry.module(fromURL: url, object: passphrase) + let module = try importer.module(fromURL: url, object: passphrase) let onDemandModule = OnDemandModule.Builder().tryBuild() var builder = Profile.Builder() diff --git a/Library/Sources/AppUIMain/Views/App/ProfileImporterModifier.swift b/Library/Sources/AppUIMain/Views/App/ProfileImporterModifier.swift index 3369377a..0ced004b 100644 --- a/Library/Sources/AppUIMain/Views/App/ProfileImporterModifier.swift +++ b/Library/Sources/AppUIMain/Views/App/ProfileImporterModifier.swift @@ -75,7 +75,7 @@ private extension ProfileImporterModifier { try await importer.reImport( url: url, profileManager: profileManager, - registry: registry + importer: registry ) } } @@ -95,7 +95,7 @@ private extension ProfileImporterModifier { try await importer.tryImport( urls: urls, profileManager: profileManager, - registry: registry + importer: registry ) } catch { await errorHandler.handle( diff --git a/Library/Tests/AppUIMainTests/ProfileImporterTests.swift b/Library/Tests/AppUIMainTests/ProfileImporterTests.swift index dd6d75af..67e714b0 100644 --- a/Library/Tests/AppUIMainTests/ProfileImporterTests.swift +++ b/Library/Tests/AppUIMainTests/ProfileImporterTests.swift @@ -31,10 +31,7 @@ import PassepartoutKit import XCTest final class ProfileImporterTests: XCTestCase { - private let registry = Registry( - allHandlers: [SomeModule.moduleHandler], - allImplementations: [SomeModule.Implementation()] - ) + private let moduleImporter = SomeModule.Implementation() private var subscriptions: Set = [] } @@ -45,7 +42,7 @@ extension ProfileImporterTests { let sut = ProfileImporter() let profileManager = ProfileManager(profiles: []) - try await sut.tryImport(urls: [], profileManager: profileManager, registry: registry) + try await sut.tryImport(urls: [], profileManager: profileManager, importer: moduleImporter) XCTAssertEqual(sut.nextURL, nil) XCTAssertTrue(profileManager.previews.isEmpty) } @@ -75,7 +72,7 @@ extension ProfileImporterTests { try await sut.tryImport( urls: [url], profileManager: profileManager, - registry: registry + importer: moduleImporter ) XCTAssertEqual(sut.nextURL, nil) @@ -107,12 +104,12 @@ extension ProfileImporterTests { try await sut.tryImport( urls: [url], profileManager: profileManager, - registry: registry + importer: moduleImporter ) XCTAssertEqual(sut.nextURL, url) sut.currentPassphrase = "passphrase" - try await sut.reImport(url: url, profileManager: profileManager, registry: registry) + try await sut.reImport(url: url, profileManager: profileManager, importer: moduleImporter) XCTAssertEqual(sut.nextURL, nil) await fulfillment(of: [exp]) @@ -126,29 +123,37 @@ extension ProfileImporterTests { try await sut.tryImport( urls: [url, url, url], profileManager: profileManager, - registry: registry + importer: moduleImporter ) XCTAssertEqual(sut.nextURL, url) XCTAssertEqual(sut.urlsRequiringPassphrase.count, 3) } } +// MARK: - + private struct SomeModule: Module { - struct Implementation: ModuleImplementation, ModuleImporter { + struct Implementation: ModuleImplementation { var moduleHandlerId: ModuleHandler.ID { moduleHandler.id } - - func module(fromURL url: URL, object: Any?) throws -> Module { - if url.absoluteString.hasSuffix(".encrypted") { - guard let passphrase = object as? String else { - throw PassepartoutError(.OpenVPN.passphraseRequired) - } - guard passphrase == "passphrase" else { - throw PassepartoutError(.crypto) - } - } - return SomeModule() - } + } +} + +extension SomeModule.Implementation: ModuleImporter { + func module(fromContents contents: String, object: Any?) throws -> Module { + fatalError() + } + + func module(fromURL url: URL, object: Any?) throws -> Module { + if url.absoluteString.hasSuffix(".encrypted") { + guard let passphrase = object as? String else { + throw PassepartoutError(.OpenVPN.passphraseRequired) + } + guard passphrase == "passphrase" else { + throw PassepartoutError(.crypto) + } + } + return SomeModule() } }