From 5e0590184b0503816783b751f374daea794b4c2d Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 18 Oct 2018 22:51:37 +0200 Subject: [PATCH 1/2] Require credentials for providers only Not always the case, but PIA certainly requires them. Will make it an infrastructure option later. Only require credentials as a configuration check, everything else has defaults. --- Passepartout-iOS/Scenes/ServiceViewController.swift | 6 +++--- Passepartout/Resources/en.lproj/Localizable.strings | 2 +- Passepartout/Sources/Model/ConnectionProfile.swift | 2 +- Passepartout/Sources/Model/ConnectionService.swift | 13 ++++++------- .../Model/Profiles/HostConnectionProfile.swift | 4 ++-- .../Model/Profiles/ProviderConnectionProfile.swift | 6 +++--- Passepartout/Sources/SwiftGen+Strings.swift | 6 +++--- Podfile.lock | 2 +- 8 files changed, 20 insertions(+), 21 deletions(-) diff --git a/Passepartout-iOS/Scenes/ServiceViewController.swift b/Passepartout-iOS/Scenes/ServiceViewController.swift index a673bd39..274ef0e5 100644 --- a/Passepartout-iOS/Scenes/ServiceViewController.swift +++ b/Passepartout-iOS/Scenes/ServiceViewController.swift @@ -190,10 +190,10 @@ class ServiceViewController: UIViewController, TableModelHost { private func toggleVpnService(cell: ToggleTableViewCell) { if cell.isOn { - guard service.canActivate(uncheckedProfile) else { + guard !service.needsCredentials(for: uncheckedProfile) else { let alert = Macros.alert( L10n.Service.Sections.Vpn.header, - L10n.Service.Alerts.ConfigurationNeeded.message + L10n.Service.Alerts.CredentialsNeeded.message ) alert.addCancelAction(L10n.Global.ok) { cell.setOn(false, animated: true) @@ -522,7 +522,7 @@ extension ServiceViewController: UITableViewDataSource, UITableViewDelegate, Tog cell.applyVPN(Theme.current, with: vpn.isEnabled ? vpn.status : nil) cell.leftText = L10n.Service.Cells.ConnectionStatus.caption cell.accessoryType = .none - cell.isTappable = service.canActivate(uncheckedProfile) && vpn.isEnabled + cell.isTappable = !service.needsCredentials(for: uncheckedProfile) && vpn.isEnabled return cell case .useProfile: diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index d5705010..d40146be 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -98,7 +98,7 @@ "service.cells.data_count.caption" = "Exchanged bytes count"; "service.cells.debug_log.caption" = "Debug log"; -"service.alerts.configuration_needed.message" = "You need to finish configuration first."; +"service.alerts.credentials_needed.message" = "You need to enter account credentials first."; "service.alerts.reconnect_vpn.message" = "Do you want to reconnect to the VPN?"; "service.alerts.trusted.no_network.message" = "You are not connected to any Wi-Fi network."; "service.alerts.trusted.will_disconnect_trusted.message" = "By trusting this network, the VPN may be disconnected. Continue?"; diff --git a/Passepartout/Sources/Model/ConnectionProfile.swift b/Passepartout/Sources/Model/ConnectionProfile.swift index 5aa7c3b3..53777d7a 100644 --- a/Passepartout/Sources/Model/ConnectionProfile.swift +++ b/Passepartout/Sources/Model/ConnectionProfile.swift @@ -34,7 +34,7 @@ protocol ConnectionProfile: class, EndpointDataSource { var username: String? { get set } - var isConfigured: Bool { get } + var requiresCredentials: Bool { get } func generate(from configuration: TunnelKitProvider.Configuration, preferences: Preferences) throws -> TunnelKitProvider.Configuration } diff --git a/Passepartout/Sources/Model/ConnectionService.swift b/Passepartout/Sources/Model/ConnectionService.swift index e4734bdf..f16ed007 100644 --- a/Passepartout/Sources/Model/ConnectionService.swift +++ b/Passepartout/Sources/Model/ConnectionService.swift @@ -180,21 +180,20 @@ class ConnectionService: Codable { return profile.id == activeProfileId } - func canActivate(_ profile: ConnectionProfile) -> Bool { - return profile.isConfigured && hasCredentials(for: profile) - } - func activateProfile(_ profile: ConnectionProfile) { activeProfileId = profile.id } // MARK: Credentials - func hasCredentials(for profile: ConnectionProfile) -> Bool { - guard let creds = credentials(for: profile) else { + func needsCredentials(for profile: ConnectionProfile) -> Bool { + guard profile.requiresCredentials else { return false } - return !creds.isEmpty + guard let creds = credentials(for: profile) else { + return true + } + return creds.isEmpty } func credentials(for profile: ConnectionProfile) -> Credentials? { diff --git a/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift b/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift index 3c47e54f..130cb08a 100644 --- a/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift +++ b/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift @@ -47,8 +47,8 @@ class HostConnectionProfile: ConnectionProfile, Codable, Equatable { var username: String? - var isConfigured: Bool { - return !parameters.endpointProtocols.isEmpty + var requiresCredentials: Bool { + return false } func generate(from configuration: TunnelKitProvider.Configuration, preferences: Preferences) throws -> TunnelKitProvider.Configuration { diff --git a/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift b/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift index 1f66a73c..22c28c9b 100644 --- a/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift +++ b/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift @@ -101,10 +101,10 @@ class ProviderConnectionProfile: ConnectionProfile, Codable, Equatable { var username: String? - var isConfigured: Bool { - return (pool != nil) && (preset != nil) + var requiresCredentials: Bool { + return true } - + func generate(from configuration: TunnelKitProvider.Configuration, preferences: Preferences) throws -> TunnelKitProvider.Configuration { guard let pool = pool else { throw ApplicationError.providerPool diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index f0495143..1042013a 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -423,9 +423,9 @@ internal enum L10n { internal enum Alerts { - internal enum ConfigurationNeeded { - /// You need to finish configuration first. - internal static let message = L10n.tr("Localizable", "service.alerts.configuration_needed.message") + internal enum CredentialsNeeded { + /// You need to enter account credentials first. + internal static let message = L10n.tr("Localizable", "service.alerts.credentials_needed.message") } internal enum DataCount { diff --git a/Podfile.lock b/Podfile.lock index f64e2f08..19040e0d 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -40,4 +40,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: f6888fe9e046f88d139af75dd5308916c5be8b7b -COCOAPODS: 1.6.0.beta.1 +COCOAPODS: 1.6.0.beta.2 From ae2bd3d876f3033754cbe8d5817306c3fa8f7e8a Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Fri, 19 Oct 2018 01:01:11 +0200 Subject: [PATCH 2/2] Replace some profile exceptions with preconditions Misconfigured profiles must fall back to a consistent state, this is a programming error. - provider.pool: fall back to default pool (should always be there) - provider.preset: why would one remove a preset? - host.endpointProtocols: .ovpn with no remotes shouldn't get this far --- Passepartout/Sources/ApplicationError.swift | 6 ------ .../Sources/Model/Profiles/HostConnectionProfile.swift | 4 +--- .../Sources/Model/Profiles/ProviderConnectionProfile.swift | 6 +++--- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/Passepartout/Sources/ApplicationError.swift b/Passepartout/Sources/ApplicationError.swift index d99bf98a..fe26d9e6 100644 --- a/Passepartout/Sources/ApplicationError.swift +++ b/Passepartout/Sources/ApplicationError.swift @@ -30,12 +30,6 @@ enum ApplicationError: Error { case missingCredentials - case providerPool - - case providerPreset - - case hostEndpoints - case missingCA case emptyRemotes diff --git a/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift b/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift index 130cb08a..0e683e0c 100644 --- a/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift +++ b/Passepartout/Sources/Model/Profiles/HostConnectionProfile.swift @@ -52,9 +52,7 @@ class HostConnectionProfile: ConnectionProfile, Codable, Equatable { } func generate(from configuration: TunnelKitProvider.Configuration, preferences: Preferences) throws -> TunnelKitProvider.Configuration { - guard !parameters.endpointProtocols.isEmpty else { - throw ApplicationError.hostEndpoints - } + precondition(!parameters.endpointProtocols.isEmpty) // XXX: copy paste, error prone var builder = parameters.builder() diff --git a/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift b/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift index 22c28c9b..7e86c93c 100644 --- a/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift +++ b/Passepartout/Sources/Model/Profiles/ProviderConnectionProfile.swift @@ -40,7 +40,7 @@ class ProviderConnectionProfile: ConnectionProfile, Codable, Equatable { } var pool: Pool? { - return infrastructure.pool(for: poolId) + return infrastructure.pool(for: poolId) ?? infrastructure.pool(for: infrastructure.defaults.pool) } var presetId: String { @@ -107,10 +107,10 @@ class ProviderConnectionProfile: ConnectionProfile, Codable, Equatable { func generate(from configuration: TunnelKitProvider.Configuration, preferences: Preferences) throws -> TunnelKitProvider.Configuration { guard let pool = pool else { - throw ApplicationError.providerPool + preconditionFailure("Nil pool?") } guard let preset = preset else { - throw ApplicationError.providerPreset + preconditionFailure("Nil preset?") } // assert(!pool.numericAddresses.isEmpty)