Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(cordova/apple): simplify OutlineAppleLib #1660

Merged
merged 1 commit into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import CocoaLumberjackSwift
import NetworkExtension
import Tun2socks

// Manages the system's VPN tunnel through the VpnExtension process.
@objcMembers
Expand All @@ -27,7 +28,6 @@
public private(set) var activeTunnelId: String?
private var tunnelManager: NETunnelProviderManager?
private var vpnStatusObserver: VpnStatusObserver?
private let connectivity: OutlineConnectivity

private enum Action {
static let start = "start"
Expand Down Expand Up @@ -68,7 +68,6 @@
}

override private init() {
connectivity = OutlineConnectivity()
super.init()
getTunnelManager() { manager in
guard manager != nil else {
Expand All @@ -85,24 +84,21 @@
// MARK: Interface

// Starts a VPN tunnel as specified in the OutlineTunnel object.
public func start(_ tunnel: OutlineTunnel, _ completion: @escaping (Callback)) {
guard let tunnelId = tunnel.id else {
DDLogError("Missing tunnel ID")
return completion(ErrorCode.illegalServerConfiguration)
}
if isActive(tunnelId) {
public func start(_ tunnelId: String, configJson: [String: Any], _ completion: @escaping (Callback)) {
guard !isActive(tunnelId) else {

Check warning on line 88 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L87-L88

Added lines #L87 - L88 were not covered by tests
jyyi1 marked this conversation as resolved.
Show resolved Hide resolved
return completion(ErrorCode.noError)
} else if isVpnConnected() {
return restartVpn(tunnelId, config: tunnel.config, completion: completion)
}
self.startVpn(tunnel, isAutoConnect: false, completion)
if isVpnConnected() {
return restartVpn(tunnelId, configJson: configJson, completion: completion)
}
self.startVpn(tunnelId, configJson: configJson, isAutoConnect: false, completion)

Check warning on line 94 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L91-L94

Added lines #L91 - L94 were not covered by tests
}

// Starts the last successful VPN tunnel.
@objc public func startLastSuccessfulTunnel(_ completion: @escaping (Callback)) {
// Explicitly pass an empty tunnel's configuration, so the VpnExtension process retrieves
// the last configuration from disk.
self.startVpn(OutlineTunnel(), isAutoConnect: true, completion)
self.startVpn(nil, configJson:nil, isAutoConnect: true, completion)

Check warning on line 101 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L101

Added line #L101 was not covered by tests
}

// Tears down the VPN if the tunnel with id |tunnelId| is active.
Expand Down Expand Up @@ -130,7 +126,8 @@
completion(ErrorCode(rawValue: rawCode) ?? ErrorCode.serverUnreachable)
}
} else {
connectivity.isServerReachable(host: host, port: port) { isReachable in
DispatchQueue.global(qos: .background).async {
let isReachable = ShadowsocksCheckServerReachable(host, Int(port), nil)

Check warning on line 130 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L129-L130

Added lines #L129 - L130 were not covered by tests
completion(isReachable ? ErrorCode.noError : ErrorCode.serverUnreachable)
}
}
Expand All @@ -151,9 +148,7 @@

// MARK: Helpers

private func startVpn(
_ tunnel: OutlineTunnel, isAutoConnect: Bool, _ completion: @escaping(Callback)) {
let tunnelId = tunnel.id
private func startVpn(_ tunnelId: String?, configJson: [String: Any]?, isAutoConnect: Bool, _ completion: @escaping(Callback)) {

Check warning on line 151 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L151

Added line #L151 was not covered by tests
setupVpn() { error in
if error != nil {
DDLogError("Failed to setup VPN: \(String(describing: error))")
Expand All @@ -163,17 +158,18 @@
self.sendVpnExtensionMessage(message) { response in
self.onStartVpnExtensionMessage(response, completion: completion)
}
var config: [String: String]? = nil
var tunnelOptions: [String: Any]? = nil

Check warning on line 161 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L161

Added line #L161 was not covered by tests
if !isAutoConnect {
config = tunnel.config
config?[MessageKey.tunnelId] = tunnelId
// TODO(fortuna): put this in a subkey
tunnelOptions = configJson
tunnelOptions?[MessageKey.tunnelId] = tunnelId

Check warning on line 165 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L163-L165

Added lines #L163 - L165 were not covered by tests
} else {
// macOS app was started by launcher.
config = [MessageKey.isOnDemand: "true"];
tunnelOptions = [MessageKey.isOnDemand: "true"];

Check warning on line 168 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L168

Added line #L168 was not covered by tests
}
let session = self.tunnelManager?.connection as! NETunnelProviderSession
do {
try session.startTunnel(options: config)
try session.startTunnel(options: tunnelOptions)

Check warning on line 172 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L172

Added line #L172 was not covered by tests
} catch let error as NSError {
DDLogError("Failed to start VPN: \(error)")
completion(ErrorCode.vpnStartFailure)
Expand All @@ -189,13 +185,13 @@
}

// Sends message to extension to restart the tunnel without tearing down the VPN.
private func restartVpn(_ tunnelId: String, config: [String: String],
private func restartVpn(_ tunnelId: String, configJson: [String: Any],
completion: @escaping(Callback)) {
if activeTunnelId != nil {
vpnStatusObserver?(.disconnected, activeTunnelId!)
}
let message = [MessageKey.action: Action.restart, MessageKey.tunnelId: tunnelId,
MessageKey.config:config] as [String : Any]
MessageKey.config: configJson] as [String : Any]

Check warning on line 194 in src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift

View check run for this annotation

Codecov / codecov/patch

src/cordova/apple/OutlineAppleLib/Sources/OutlineTunnelSources/OutlineVpn.swift#L194

Added line #L194 was not covered by tests
self.sendVpnExtensionMessage(message) { response in
self.onStartVpnExtensionMessage(response, completion: completion)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import XCTest

import NetworkExtension
import Tun2socks

@testable import OutlineTunnel
@testable import PacketTunnelProvider
Expand Down Expand Up @@ -37,4 +38,11 @@ final class OutlineTunnelTest: XCTestCase {

XCTAssertEqual(["1.1.1.1", "9.9.9.9", "208.67.222.222", "208.67.220.220"], settings.dnsSettings?.servers)
}

func testReachability() {
// TODO(fortuna): run a local server instead.
XCTAssertTrue(ShadowsocksCheckServerReachable("8.8.8.8", 853, nil))
XCTAssertTrue(ShadowsocksCheckServerReachable("google.com", 443, nil))
XCTAssertFalse(ShadowsocksCheckServerReachable("nonexistent.getoutline.org", 443, nil))
}
}
12 changes: 6 additions & 6 deletions src/cordova/plugin/apple/src/OutlinePlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ class OutlinePlugin: CDVPlugin {
errorCode: OutlineVpn.ErrorCode.illegalServerConfiguration)
}
DDLogInfo("\(Action.start) \(tunnelId)")
guard let config = command.argument(at: 1) as? [String: Any], containsExpectedKeys(config) else {
// TODO(fortuna): Move the config validation to the config parsing code in Go.
guard let configJson = command.argument(at: 1) as? [String: Any], containsExpectedKeys(configJson) else {
return sendError("Invalid configuration", callbackId: command.callbackId,
errorCode: OutlineVpn.ErrorCode.illegalServerConfiguration)
}
let tunnel = OutlineTunnel(id: tunnelId, config: config)
OutlineVpn.shared.start(tunnel) { errorCode in
OutlineVpn.shared.start(tunnelId, configJson:configJson) { errorCode in
if errorCode == OutlineVpn.ErrorCode.noError {
#if os(macOS)
NotificationCenter.default.post(
Expand Down Expand Up @@ -234,9 +234,9 @@ class OutlinePlugin: CDVPlugin {
}

// Returns whether |config| contains all the expected keys
private func containsExpectedKeys(_ config: [String: Any]?) -> Bool {
return config?["host"] != nil && config?["port"] != nil &&
config?["password"] != nil && config?["method"] != nil
private func containsExpectedKeys(_ configJson: [String: Any]?) -> Bool {
return configJson?["host"] != nil && configJson?["port"] != nil &&
configJson?["password"] != nil && configJson?["method"] != nil
}

// MARK: Callback helpers
Expand Down
Loading