From 0b7ab7bca662031071fda11e3309f39a9b91040a Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 14:50:46 +0200 Subject: [PATCH 1/4] Intercept potentially unsupported options As warnings. --- .../TunnelKitProvider+FileConfiguration.swift | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift index 75899eec..b683eeb5 100644 --- a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift +++ b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift @@ -37,6 +37,8 @@ struct ParsedFile { let configuration: TunnelKitProvider.Configuration let strippedLines: [String]? + + let warning: ApplicationError? } extension TunnelKitProvider.Configuration { @@ -78,6 +80,7 @@ extension TunnelKitProvider.Configuration { static func parsed(from url: URL, returnsStripped: Bool = false) throws -> ParsedFile { let lines = try String(contentsOf: url).trimmedLines() var strippedLines: [String]? = returnsStripped ? [] : nil + var warning: ApplicationError? = nil var defaultProto: TunnelKitProvider.SocketType? var defaultPort: UInt16? @@ -221,13 +224,23 @@ extension TunnelKitProvider.Configuration { unsupportedError = ApplicationError.unsupportedConfiguration(option: "auth \(rawValue)") } } - Regex.compLZO.enumerateComponents(in: line) { _ in + Regex.compLZO.enumerateArguments(in: line) { isHandled = true compressionFraming = .compLZO + + guard let arg = $0.first, arg == "no" else { + warning = .unsupportedConfiguration(option: "compression") + return + } } - Regex.compress.enumerateComponents(in: line) { _ in + Regex.compress.enumerateArguments(in: line) { isHandled = true compressionFraming = .compress + + guard $0.isEmpty else { + warning = .unsupportedConfiguration(option: "compression") + return + } } Regex.keyDirection.enumerateArguments(in: line) { isHandled = true @@ -325,7 +338,13 @@ extension TunnelKitProvider.Configuration { var builder = TunnelKitProvider.ConfigurationBuilder(sessionConfiguration: sessionBuilder.build()) builder.endpointProtocols = endpointProtocols - return ParsedFile(url: url, hostname: hostname, configuration: builder.build(), strippedLines: strippedLines) + return ParsedFile( + url: url, + hostname: hostname, + configuration: builder.build(), + strippedLines: strippedLines, + warning: warning + ) } } From 8d2ce2e7aefe823c36a0078e18f1958d6a707b91 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 14:14:46 +0200 Subject: [PATCH 2/4] Handle potentially unsupported as a warning alert Configuration is imported anyway, so alert must be asynchronous. --- Passepartout-iOS/AppDelegate.swift | 8 ++- .../Global/ParsedFile+Alerts.swift | 65 +++++++++++++++---- .../Resources/en.lproj/Localizable.strings | 1 + Passepartout/Sources/SwiftGen+Strings.swift | 7 ++ 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index ddd18ddb..9fccea82 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -95,12 +95,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele try? fm.removeItem(at: url) return true } + handleParsedFile(parsedFile, in: root) + return true + } + + private func handleParsedFile(_ parsedFile: ParsedFile, in root: UIViewController) { // already presented: update parsed configuration if let nav = root.presentedViewController as? UINavigationController, let wizard = nav.topViewController as? WizardHostViewController { wizard.parsedFile = parsedFile wizard.removesConfigurationOnCancel = true - return true + return } // present now @@ -121,7 +126,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele } nav.modalPresentationStyle = .formSheet root.present(nav, animated: true, completion: nil) - return true } } diff --git a/Passepartout-iOS/Global/ParsedFile+Alerts.swift b/Passepartout-iOS/Global/ParsedFile+Alerts.swift index 7cc370dd..abd3cdc1 100644 --- a/Passepartout-iOS/Global/ParsedFile+Alerts.swift +++ b/Passepartout-iOS/Global/ParsedFile+Alerts.swift @@ -32,26 +32,19 @@ private let log = SwiftyBeaver.self extension ParsedFile { static func from(_ url: URL, withErrorAlertIn viewController: UIViewController) -> ParsedFile? { + let file: ParsedFile log.debug("Parsing configuration URL: \(url)") do { - return try TunnelKitProvider.Configuration.parsed(from: url) - } catch ApplicationError.missingConfiguration(let option) { - log.error("Could not parse configuration URL: missing configuration, \(option)") - let message = L10n.ParsedFile.Alerts.Missing.message(option) - alertConfigurationImportError(url: url, in: viewController, withMessage: message) - } catch ApplicationError.unsupportedConfiguration(let option) { - log.error("Could not parse configuration URL: unsupported configuration, \(option)") - let message = L10n.ParsedFile.Alerts.Unsupported.message(option) - alertConfigurationImportError(url: url, in: viewController, withMessage: message) + file = try TunnelKitProvider.Configuration.parsed(from: url) } catch let e { - log.error("Could not parse configuration URL: \(e)") - let message = L10n.ParsedFile.Alerts.Parsing.message(e.localizedDescription) - alertConfigurationImportError(url: url, in: viewController, withMessage: message) + let message = localizedMessage(forError: e) + alertImportError(url: url, in: viewController, withMessage: message) + return nil } - return nil + return file } - private static func alertConfigurationImportError(url: URL, in vc: UIViewController, withMessage message: String) { + private static func alertImportError(url: URL, in vc: UIViewController, withMessage message: String) { let alert = Macros.alert(url.normalizedFilename, message) // alert.addDefaultAction(L10n.ParsedFile.Alerts.Buttons.report) { // var attach = IssueReporter.Attachments(debugLog: false, configurationURL: url) @@ -61,4 +54,48 @@ extension ParsedFile { alert.addCancelAction(L10n.Global.ok) vc.present(alert, animated: true, completion: nil) } + + static func alertImportWarning(url: URL, in vc: UIViewController, withWarning warning: ApplicationError, completionHandler: @escaping (Bool) -> Void) { + let message = details(forWarning: warning) + let alert = Macros.alert(url.normalizedFilename, L10n.ParsedFile.Alerts.PotentiallyUnsupported.message(message)) + alert.addDefaultAction(L10n.Global.ok) { + completionHandler(true) + } + alert.addCancelAction(L10n.Global.cancel) { + completionHandler(false) + } + vc.present(alert, animated: true, completion: nil) + } + + private static func localizedMessage(forError error: Error) -> String { + if let appError = error as? ApplicationError { + switch appError { + case .missingConfiguration(let option): + log.error("Could not parse configuration URL: missing configuration, \(option)") + return L10n.ParsedFile.Alerts.Missing.message(option) + + case .unsupportedConfiguration(let option): + log.error("Could not parse configuration URL: unsupported configuration, \(option)") + return L10n.ParsedFile.Alerts.Unsupported.message(option) + + default: + break + } + } + log.error("Could not parse configuration URL: \(error)") + return L10n.ParsedFile.Alerts.Parsing.message(error.localizedDescription) + } + + private static func details(forWarning warning: ApplicationError) -> String { + switch warning { + case .missingConfiguration(let option): + return option + + case .unsupportedConfiguration(let option): + return option + + default: + fatalError("Only use .missingConfiguration or .unsupportedConfiguration for warnings") + } + } } diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index e3a06b30..28a2c496 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -55,6 +55,7 @@ "parsed_file.alerts.missing.message" = "The configuration file lacks a required option (%@)."; "parsed_file.alerts.unsupported.message" = "The configuration file contains an unsupported option (%@)."; +"parsed_file.alerts.potentially_unsupported.message" = "The configuration file is correct but contains a potentially unsupported option (%@).\n\nConnectivity may break depending on server settings."; "parsed_file.alerts.parsing.message" = "Unable to parse the provided configuration file (%@)."; "parsed_file.alerts.buttons.report" = "Report an issue"; diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index 7859a287..4ff76209 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -432,6 +432,13 @@ internal enum L10n { } } + internal enum PotentiallyUnsupported { + /// The configuration file is correct but contains a potentially unsupported option (%@).\n\nConnectivity may break depending on server settings. + internal static func message(_ p1: String) -> String { + return L10n.tr("Localizable", "parsed_file.alerts.potentially_unsupported.message", p1) + } + } + internal enum Unsupported { /// The configuration file contains an unsupported option (%@). internal static func message(_ p1: String) -> String { From 0f43255676283318cdb31eb82c6498874f6ef354 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 14:40:52 +0200 Subject: [PATCH 3/4] Show warning alert before continuing import Fixes #16 --- Passepartout-iOS/AppDelegate.swift | 10 ++++ .../Global/SwiftGen+Storyboards.swift | 1 + .../ImportedHostsViewController.swift | 48 +++++++++++++++---- .../Organizer/OrganizerViewController.swift | 2 +- .../en.lproj/Organizer.storyboard | 6 +-- 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index 9fccea82..a7723c9d 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -95,6 +95,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele try? fm.removeItem(at: url) return true } + if let warning = parsedFile.warning { + ParsedFile.alertImportWarning(url: url, in: root, withWarning: warning) { + if $0 { + self.handleParsedFile(parsedFile, in: root) + } else { + try? fm.removeItem(at: url) + } + } + return true + } handleParsedFile(parsedFile, in: root) return true } diff --git a/Passepartout-iOS/Global/SwiftGen+Storyboards.swift b/Passepartout-iOS/Global/SwiftGen+Storyboards.swift index 9b6efc0c..5f85c889 100644 --- a/Passepartout-iOS/Global/SwiftGen+Storyboards.swift +++ b/Passepartout-iOS/Global/SwiftGen+Storyboards.swift @@ -92,6 +92,7 @@ internal enum StoryboardSegue { case disclaimerSegueIdentifier = "DisclaimerSegueIdentifier" case importHostSegueIdentifier = "ImportHostSegueIdentifier" case selectProfileSegueIdentifier = "SelectProfileSegueIdentifier" + case showImportedHostsSegueIdentifier = "ShowImportedHostsSegueIdentifier" case versionSegueIdentifier = "VersionSegueIdentifier" } } diff --git a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift index 6a095b6d..5cf3ff00 100644 --- a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift @@ -41,6 +41,12 @@ class ImportedHostsViewController: UITableViewController { title = L10n.ImportedHosts.title } + + override func viewWillAppear(_ animated: Bool) { + super.viewWillAppear(animated) + + parsedFile = nil + } override func viewDidAppear(_ animated: Bool) { super.viewDidAppear(animated) @@ -56,22 +62,40 @@ class ImportedHostsViewController: UITableViewController { present(alert, animated: true, completion: nil) return } + if let selectedIP = tableView.indexPathForSelectedRow { + tableView.deselectRow(at: selectedIP, animated: true) + } } // MARK: Actions override func shouldPerformSegue(withIdentifier identifier: String, sender: Any?) -> Bool { - guard let cell = sender as? UITableViewCell, let indexPath = tableView.indexPath(for: cell) else { - return false - } - let url = pendingConfigurationURLs[indexPath.row] - guard let parsedFile = ParsedFile.from(url, withErrorAlertIn: self) else { - if let selectedIP = tableView.indexPathForSelectedRow { - tableView.deselectRow(at: selectedIP, animated: true) + + // segue parses configuration file if not yet + if parsedFile == nil { + guard let cell = sender as? UITableViewCell, let indexPath = tableView.indexPath(for: cell) else { + return false + } + let url = pendingConfigurationURLs[indexPath.row] + guard let parsedFile = ParsedFile.from(url, withErrorAlertIn: self) else { + deselectSelectedRow() + return false + } + self.parsedFile = parsedFile + + // postpone segue until alert dismissal + if let warning = parsedFile.warning { + ParsedFile.alertImportWarning(url: url, in: self, withWarning: warning) { + self.deselectSelectedRow() + if $0 { + self.perform(segue: StoryboardSegue.Organizer.importHostSegueIdentifier) + } else { + self.parsedFile = nil + } + } + return false } - return false } - self.parsedFile = parsedFile return true } @@ -86,6 +110,12 @@ class ImportedHostsViewController: UITableViewController { @IBAction private func close() { dismiss(animated: true, completion: nil) } + + private func deselectSelectedRow() { + if let selectedIP = tableView.indexPathForSelectedRow { + tableView.deselectRow(at: selectedIP, animated: true) + } + } } extension ImportedHostsViewController { diff --git a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift index f503499d..5e6ae8e5 100644 --- a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift @@ -176,7 +176,7 @@ class OrganizerViewController: UITableViewController, TableModelHost { } private func addNewHost() { - perform(segue: StoryboardSegue.Organizer.importHostSegueIdentifier) + perform(segue: StoryboardSegue.Organizer.showImportedHostsSegueIdentifier) } private func removeProfile(at indexPath: IndexPath) { diff --git a/Passepartout-iOS/en.lproj/Organizer.storyboard b/Passepartout-iOS/en.lproj/Organizer.storyboard index f4006b7c..c5c6af32 100644 --- a/Passepartout-iOS/en.lproj/Organizer.storyboard +++ b/Passepartout-iOS/en.lproj/Organizer.storyboard @@ -194,7 +194,7 @@ - + @@ -293,7 +293,7 @@ - + @@ -567,6 +567,6 @@ - + From ad063965da36cc801158f1ed7b11b2520c1883cc Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 14:43:09 +0200 Subject: [PATCH 4/4] Retain back button in import flow --- .../Scenes/Organizer/ImportedHostsViewController.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift index 5cf3ff00..6d5e57c5 100644 --- a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift @@ -105,6 +105,9 @@ class ImportedHostsViewController: UITableViewController { } wizard.parsedFile = parsedFile wizard.delegate = wizardDelegate + + // retain back button + wizard.navigationItem.leftBarButtonItem = nil } @IBAction private func close() {