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.

Sign up for GitHub

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

Jump to bottom

VPN clean-up #1041

Merged
merged 6 commits into from
Nov 6, 2024
Merged

VPN clean-up #1041

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
24783ae
Remove unused VPN code.
samsymons Oct 30, 2024
70cbb19
Restore `throws` for a function that macOS depends on being able to t…
samsymons Oct 30, 2024
337ebbb
Merge branch 'main' into sam/vpn-clean-up
samsymons Nov 4, 2024
6fe4744
Merge branch 'main' into sam/vpn-clean-up
samsymons Nov 4, 2024
bca4c1a
Update comment to remove reference to `stopTunnel(with:)`.
samsymons Nov 4, 2024
fdd840c
Merge branch 'main' into sam/vpn-clean-up
samsymons Nov 6, 2024
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
20 changes: 5 additions & 15 deletions Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,7 @@ final class NetworkProtectionConnectionTester {
let tunnelInterface = try await networkInterface(forInterfaceNamed: tunnelIfName)
self.tunnelInterface = tunnelInterface

do {
try await scheduleTimer(testImmediately: testImmediately)
} catch {
Logger.networkProtectionConnectionTester.log("🔴 Stopping connection tester early")
throw error
}
await scheduleTimer(testImmediately: testImmediately)
}

func stop() {
Expand Down Expand Up @@ -163,16 +158,11 @@ final class NetworkProtectionConnectionTester {

// MARK: - Timer scheduling

private func scheduleTimer(testImmediately: Bool) async throws {
private func scheduleTimer(testImmediately: Bool) async {
stopScheduledTimer()

if testImmediately {
do {
try await testConnection()
} catch {
Logger.networkProtectionConnectionTester.log("Rethrowing exception")
throw error
}
await testConnection()
}

task = Task.periodic(interval: intervalBetweenTests) { [weak self] in
Expand All @@ -181,7 +171,7 @@ final class NetworkProtectionConnectionTester {
// The error we're ignoring here is only used when this class is initialized
// with an immediate test, to know whether the connection is up while the user
// still sees "Connecting..."
try? await self?.testConnection()
await self?.testConnection()
}
}

Expand All @@ -192,7 +182,7 @@ final class NetworkProtectionConnectionTester {

// MARK: - Testing the connection

func testConnection() async throws {
func testConnection() async {
Comment on lines -195 to +185
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function wasn't throwing anything, but it was causing a chain of try statements above it, so removing throws here lets us clean up a bit of code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice, thanks for taking the time to clean up.

guard let tunnelInterface = tunnelInterface else {
Logger.networkProtectionConnectionTester.error("No interface to test!")
return
Expand Down
17 changes: 1 addition & 16 deletions Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
case failedToEncodeRegisterKeyRequest
case failedToFetchRegisteredServers(Error?)
case failedToParseRegisteredServersResponse(Error)
case failedToEncodeRedeemRequest
case invalidInviteCode
case failedToRedeemInviteCode(Error?)
case failedToRetrieveAuthToken(AuthenticationFailureResponse)
case failedToParseRedeemResponse(Error)
case invalidAuthToken
case serverListInconsistency

Expand Down Expand Up @@ -91,11 +86,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
case .failedToEncodeRegisterKeyRequest: return 104
case .failedToFetchRegisteredServers: return 105
case .failedToParseRegisteredServersResponse: return 106
case .failedToEncodeRedeemRequest: return 107
case .invalidInviteCode: return 108
case .failedToRedeemInviteCode: return 109
case .failedToRetrieveAuthToken: return 110
case .failedToParseRedeemResponse: return 111
case .invalidAuthToken: return 112
case .serverListInconsistency: return 113
case .failedToFetchServerStatus: return 114
Expand Down Expand Up @@ -130,9 +120,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
.couldNotGetPeerHostName,
.couldNotGetInterfaceAddressRange,
.failedToEncodeRegisterKeyRequest,
.failedToEncodeRedeemRequest,
.invalidInviteCode,
.failedToRetrieveAuthToken,
.invalidAuthToken,
.serverListInconsistency,
.failedToCastKeychainValueToData,
Expand All @@ -147,8 +134,7 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
.vpnAccessRevoked:
return [:]
case .failedToFetchServerList(let error),
.failedToFetchRegisteredServers(let error),
.failedToRedeemInviteCode(let error):
.failedToFetchRegisteredServers(let error):
guard let error else {
return [:]
}
Expand All @@ -160,7 +146,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
.failedToFetchLocationList(let error),
.failedToParseLocationListResponse(let error),
.failedToParseRegisteredServersResponse(let error),
.failedToParseRedeemResponse(let error),
.wireGuardSetNetworkSettings(let error),
.startWireGuardBackend(let error),
.setWireguardConfig(let error),
Expand Down
79 changes: 0 additions & 79 deletions Sources/NetworkProtection/Networking/NetworkProtectionClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
case failedToEncodeRegisterKeyRequest
case failedToFetchRegisteredServers(Error)
case failedToParseRegisteredServersResponse(Error)
case failedToEncodeRedeemRequest
case invalidInviteCode
case failedToRedeemInviteCode(Error)
case failedToRetrieveAuthToken(AuthenticationFailureResponse)
case failedToParseRedeemResponse(Error)
case invalidAuthToken
case accessDenied

Expand All @@ -56,11 +51,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
case .failedToEncodeRegisterKeyRequest: return .failedToEncodeRegisterKeyRequest
case .failedToFetchRegisteredServers(let error): return .failedToFetchRegisteredServers(error)
case .failedToParseRegisteredServersResponse(let error): return .failedToParseRegisteredServersResponse(error)
case .failedToEncodeRedeemRequest: return .failedToEncodeRedeemRequest
case .invalidInviteCode: return .invalidInviteCode
case .failedToRedeemInviteCode(let error): return .failedToRedeemInviteCode(error)
case .failedToRetrieveAuthToken(let response): return .failedToRetrieveAuthToken(response)
case .failedToParseRedeemResponse(let error): return .failedToParseRedeemResponse(error)
case .invalidAuthToken: return .invalidAuthToken
case .accessDenied: return .vpnAccessRevoked
}
Expand All @@ -75,11 +65,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
case .failedToEncodeRegisterKeyRequest: return 4
case .failedToFetchRegisteredServers: return 5
case .failedToParseRegisteredServersResponse: return 6
case .failedToEncodeRedeemRequest: return 7
case .invalidInviteCode: return 8
case .failedToRedeemInviteCode: return 9
case .failedToRetrieveAuthToken: return 10
case .failedToParseRedeemResponse: return 11
case .invalidAuthToken: return 12
case .accessDenied: return 13
case .failedToFetchServerStatus: return 14
Expand All @@ -95,15 +80,10 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
.failedToParseServerListResponse(let error),
.failedToFetchRegisteredServers(let error),
.failedToParseRegisteredServersResponse(let error),
.failedToRedeemInviteCode(let error),
.failedToParseRedeemResponse(let error),
.failedToFetchServerStatus(let error),
.failedToParseServerStatusResponse(let error):
return [NSUnderlyingErrorKey: error as NSError]
case .failedToEncodeRegisterKeyRequest,
.failedToEncodeRedeemRequest,
.invalidInviteCode,
.failedToRetrieveAuthToken,
.invalidAuthToken,
.accessDenied:
return [:]
Expand Down Expand Up @@ -159,10 +139,6 @@ enum RegisterServerSelection {
}
}

struct RedeemInviteCodeRequestBody: Encodable {
let code: String
}

struct ExchangeAccessTokenRequestBody: Encodable {
let token: String
}
Expand All @@ -171,10 +147,6 @@ struct AuthenticationSuccessResponse: Decodable {
let token: String
}

public struct AuthenticationFailureResponse: Decodable {
public let message: String
}

final class NetworkProtectionBackendClient: NetworkProtectionClient {

enum Constants {
Expand Down Expand Up @@ -435,57 +407,6 @@ final class NetworkProtectionBackendClient: NetworkProtectionClient {
}
}

private func retrieveAuthToken<RequestBody: Encodable>(
requestBody: RequestBody,
endpoint: URL
) async -> Result<String, NetworkProtectionClientError> {
let requestBodyData: Data

do {
requestBodyData = try JSONEncoder().encode(requestBody)
} catch {
return .failure(.failedToEncodeRedeemRequest)
}

var request = URLRequest(url: endpoint)
request.allHTTPHeaderFields = APIRequest.Headers().httpHeaders
request.addValue("application/json", forHTTPHeaderField: "Content-Type")
request.httpMethod = "POST"
request.httpBody = requestBodyData

let responseData: Data

do {
let (data, response) = try await URLSession.shared.data(for: request)
guard let response = response as? HTTPURLResponse else {
throw AuthTokenError.noResponse
}
switch response.statusCode {
case 200:
responseData = data
case 400:
return .failure(.invalidInviteCode)
default:
do {
// Try to redeem the subscription backend error response first:
let decodedRedemptionResponse = try decoder.decode(AuthenticationFailureResponse.self, from: data)
return .failure(.failedToRetrieveAuthToken(decodedRedemptionResponse))
} catch {
throw AuthTokenError.unexpectedStatus(status: response.statusCode)
}
}
} catch {
return .failure(NetworkProtectionClientError.failedToRedeemInviteCode(error))
}

do {
let decodedRedemptionResponse = try decoder.decode(AuthenticationSuccessResponse.self, from: responseData)
return .success(decodedRedemptionResponse.token)
} catch {
return .failure(NetworkProtectionClientError.failedToParseRedeemResponse(error))
}
}

}

extension URL {
Expand Down
4 changes: 2 additions & 2 deletions Sources/NetworkProtection/PacketTunnelProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
#endif
}

open func loadVendorOptions(from provider: NETunnelProviderProtocol?) throws {
open func loadVendorOptions(from provider: NETunnelProviderProtocol?) {
let vendorOptions = provider?.providerConfiguration

loadRoutes(from: vendorOptions)
Expand Down Expand Up @@ -686,7 +686,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {

do {
try load(options: startupOptions)
try loadVendorOptions(from: tunnelProviderProtocol)
loadVendorOptions(from: tunnelProviderProtocol)

if (try? tokenStore.fetchToken()) == nil {
throw TunnelError.startingTunnelWithoutAuthToken
Expand Down
4 changes: 0 additions & 4 deletions Tests/NetworkProtectionTests/NetworkProtectionErrorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ final class NetworkProtectionErrorTests: XCTestCase {
.couldNotGetPeerHostName,
.couldNotGetInterfaceAddressRange,
.failedToEncodeRegisterKeyRequest,
.failedToEncodeRedeemRequest,
.invalidInviteCode,
.invalidAuthToken,
.serverListInconsistency,
.wireGuardCannotLocateTunnelFileDescriptor,
Expand Down Expand Up @@ -72,8 +70,6 @@ final class NetworkProtectionErrorTests: XCTestCase {
.failedToParseLocationListResponse(underlyingError),
.failedToFetchRegisteredServers(underlyingError),
.failedToParseRegisteredServersResponse(underlyingError),
.failedToRedeemInviteCode(underlyingError),
.failedToParseRedeemResponse(underlyingError),
.wireGuardSetNetworkSettings(underlyingError),
.startWireGuardBackend(underlyingError),
.unhandledError(function: #function, line: #line, error: underlyingError),
Expand Down
Loading