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

macOS VPN: Add pixels to track VPN start and stop attempts through IPC #2622

Merged
merged 11 commits into from
Apr 18, 2024
Merged

macOS VPN: Add pixels to track VPN start and stop attempts through IPC #2622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
581ac35
Adds IPC attempt pixels, unit tests for NetworkProtectionIPCTunnelCon…
diegoreymendez Apr 10, 2024
b6671a8
Adds IPC start pixels for the VPN
diegoreymendez Apr 15, 2024
9d0a49b
Fixes some small mistakes
diegoreymendez Apr 15, 2024
7f893a1
Removes obsolete code
diegoreymendez Apr 15, 2024
66772ec
Rolls back some bad changes
diegoreymendez Apr 15, 2024
6f9652e
Fixes some swiftlint errors
diegoreymendez Apr 17, 2024
8c17a94
Makes several improvements to IPC error handling
diegoreymendez Apr 18, 2024
74e421e
Fixes the unit tests for NetworkProtectionIPCTunnelController and add…
diegoreymendez Apr 18, 2024
79235e2
Rolls back an unintentional change
diegoreymendez Apr 18, 2024
efb8598
Fixes some build issues due to rebasing
diegoreymendez Apr 18, 2024
5ab50fe
Rolls back an unintentional change
diegoreymendez Apr 18, 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: 20 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,10 @@
7BBD45B12A691AB500C83CA9 /* NetworkProtectionDebugUtilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBD45B02A691AB500C83CA9 /* NetworkProtectionDebugUtilities.swift */; };
7BBD45B22A691AB500C83CA9 /* NetworkProtectionDebugUtilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBD45B02A691AB500C83CA9 /* NetworkProtectionDebugUtilities.swift */; };
7BBE2B7B2B61663C00697445 /* NetworkProtectionProxy in Frameworks */ = {isa = PBXBuildFile; productRef = 7BBE2B7A2B61663C00697445 /* NetworkProtectionProxy */; };
7BBE650D2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBE650C2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift */; };
7BBE650E2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBE650C2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift */; };
7BBE65102BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBE650F2BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift */; };
7BBE65112BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BBE650F2BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift */; };
7BD01C192AD8319C0088B32E /* IPCServiceManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BD01C182AD8319C0088B32E /* IPCServiceManager.swift */; };
7BD1688E2AD4A4C400D24876 /* NetworkExtensionController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BD1688D2AD4A4C400D24876 /* NetworkExtensionController.swift */; };
7BD3AF5D2A8E7AF1006F9F56 /* KeychainType+ClientDefault.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BD3AF5C2A8E7AF1006F9F56 /* KeychainType+ClientDefault.swift */; };
Expand Down Expand Up @@ -4135,6 +4139,8 @@
7BB108582A43375D000AB95F /* PFMoveApplication.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = PFMoveApplication.m; sourceTree = "<group>"; };
7BBA7CE52BAB03C1007579A3 /* DefaultSubscriptionFeatureAvailability+DefaultInitializer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DefaultSubscriptionFeatureAvailability+DefaultInitializer.swift"; sourceTree = "<group>"; };
7BBD45B02A691AB500C83CA9 /* NetworkProtectionDebugUtilities.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionDebugUtilities.swift; sourceTree = "<group>"; };
7BBE650C2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionIPCTunnelControllerTests.swift; sourceTree = "<group>"; };
7BBE650F2BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionTestingSupport.swift; sourceTree = "<group>"; };
7BD01C182AD8319C0088B32E /* IPCServiceManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IPCServiceManager.swift; sourceTree = "<group>"; };
7BD1688D2AD4A4C400D24876 /* NetworkExtensionController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NetworkExtensionController.swift; sourceTree = "<group>"; };
7BD3AF5C2A8E7AF1006F9F56 /* KeychainType+ClientDefault.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KeychainType+ClientDefault.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -6470,10 +6476,12 @@
4BCF15E32ABB987F0083F6DF /* NetworkProtection */ = {
isa = PBXGroup;
children = (
7BBE65122BC67EF6008F4EE9 /* Support */,
4BCF15E62ABB98A20083F6DF /* Resources */,
4BCF15E42ABB98990083F6DF /* NetworkProtectionRemoteMessageTests.swift */,
4BD57C032AC112DF00B580EE /* NetworkProtectionRemoteMessagingTests.swift */,
7B09CBA72BA4BE7000CF245B /* NetworkProtectionPixelEventTests.swift */,
7BBE650C2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift */,
);
path = NetworkProtection;
sourceTree = "<group>";
Expand Down Expand Up @@ -6677,6 +6685,14 @@
path = LetsMove1.25;
sourceTree = "<group>";
};
7BBE65122BC67EF6008F4EE9 /* Support */ = {
isa = PBXGroup;
children = (
7BBE650F2BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift */,
);
path = Support;
sourceTree = "<group>";
};
7BDA36E72B7E037200AD5388 /* VPNProxyExtension */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -11218,8 +11234,10 @@
B630E80129C887ED00363609 /* NSErrorAdditionalInfo.swift in Sources */,
3706FE31293F661700E42796 /* TabCollectionViewModelDelegateMock.swift in Sources */,
3706FE32293F661700E42796 /* BookmarksHTMLReaderTests.swift in Sources */,
7BBE650E2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift in Sources */,
3706FE33293F661700E42796 /* FireTests.swift in Sources */,
B60C6F8229B1B4AD007BFAA8 /* TestRunHelper.swift in Sources */,
7BBE65112BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift in Sources */,
567DA94029E8045D008AC5EE /* MockEmailStorage.swift in Sources */,
317295D32AF058D3002C3206 /* MockWaitlistTermsAndConditionsActionHandler.swift in Sources */,
3706FE34293F661700E42796 /* PermissionStoreTests.swift in Sources */,
Expand Down Expand Up @@ -13353,6 +13371,7 @@
1D3B1AC22936B816006F4388 /* BWMessageIdGeneratorTests.swift in Sources */,
B6C2C9F62760B659005B7F0A /* TestDataModel.xcdatamodeld in Sources */,
1DA6D1022A1FFA3700540406 /* HTTPCookieTests.swift in Sources */,
7BBE65102BC67EED008F4EE9 /* NetworkProtectionTestingSupport.swift in Sources */,
1D9FDEC02B9B5FEA0040B78C /* AccessibilityPreferencesTests.swift in Sources */,
B68172AE269EB43F006D1092 /* GeolocationServiceTests.swift in Sources */,
B6AE74342609AFCE005B9B1A /* ProgressEstimationTests.swift in Sources */,
Expand Down Expand Up @@ -13433,6 +13452,7 @@
37CD54B927F1F8AC00F1F7B9 /* AppearancePreferencesTests.swift in Sources */,
EEF53E182950CED5002D78F4 /* JSAlertViewModelTests.swift in Sources */,
376C4DB928A1A48A00CC0F5B /* FirePopoverViewModelTests.swift in Sources */,
7BBE650D2BC67BA0008F4EE9 /* NetworkProtectionIPCTunnelControllerTests.swift in Sources */,
AAEC74B62642CC6A00C2EFBC /* HistoryStoringMock.swift in Sources */,
AA652CB125DD825B009059CC /* LocalBookmarkStoreTests.swift in Sources */,
B630794226731F5400DCEE41 /* WKDownloadMock.swift in Sources */,
Expand Down
20 changes: 19 additions & 1 deletion DuckDuckGo/LoginItems/LoginItemsManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ import Common
import Foundation
import LoginItems

protocol LoginItemsManaging {
func throwingEnableLoginItems(_ items: Set<LoginItem>, log: OSLog) throws
}

diegoreymendez marked this conversation as resolved.
Show resolved Hide resolved
/// Class to manage the login items for the VPN and DBP
///
final class LoginItemsManager {
final class LoginItemsManager: LoginItemsManaging {
private enum Action: String {
case enable
case disable
Expand All @@ -42,6 +46,20 @@ final class LoginItemsManager {
}
}

/// Throwing version of enableLoginItems
///
func throwingEnableLoginItems(_ items: Set<LoginItem>, log: OSLog) throws {
for item in items {
do {
try item.enable()
os_log("🟢 Enabled successfully %{public}@", log: log, String(describing: item))
} catch let error as NSError {
handleError(for: item, action: .enable, error: error)
throw error
}
}
}

diegoreymendez marked this conversation as resolved.
Show resolved Hide resolved
func restartLoginItems(_ items: Set<LoginItem>, log: OSLog) {
for item in items {
do {
Expand Down
9 changes: 7 additions & 2 deletions DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#if DEBUG

import AppKit
import Combine
import Foundation
import NetworkProtection
Expand Down Expand Up @@ -63,9 +64,13 @@ final class IPCClientMock: NetworkProtectionIPCClient {
}
var ipcControllerErrorMessageObserver: any NetworkProtection.ControllerErrorMesssageObserver = ControllerErrorMesssageObserverMock()

func start() {}
func start(completion: @escaping (Error?) -> Void) {
completion(nil)
}

func stop() {}
func stop(completion: @escaping (Error?) -> Void) {
completion(nil)
}

}

Expand Down
4 changes: 2 additions & 2 deletions ...o/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ protocol NetworkProtectionIPCClient {
var ipcServerInfoObserver: ConnectionServerInfoObserver { get }
var ipcConnectionErrorObserver: ConnectionErrorObserver { get }

func start()
func stop()
func start(completion: @escaping (Error?) -> Void)
func stop(completion: @escaping (Error?) -> Void)
}

extension TunnelControllerIPCClient: NetworkProtectionIPCClient {
Expand Down
187 changes: 164 additions & 23 deletions ...NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,47 +20,113 @@ import Common
import Foundation
import NetworkProtection
import NetworkProtectionIPC
import PixelKit

final class NetworkProtectionIPCTunnelController: TunnelController {
/// VPN tunnel controller through IPC.
///
final class NetworkProtectionIPCTunnelController {

enum RequestError: CustomNSError {
case notAuthorizedToEnableLoginItem
case internalLoginItemError(_ error: Error)

var errorCode: Int {
switch self {
case .notAuthorizedToEnableLoginItem: return 0
case .internalLoginItemError: return 1
}
}

var errorUserInfo: [String: Any] {
switch self {
case .notAuthorizedToEnableLoginItem:
return [:]
case .internalLoginItemError(let error):
return [NSUnderlyingErrorKey: error as NSError]
}
}
}

private let featureVisibility: NetworkProtectionFeatureVisibility
private let loginItemsManager: LoginItemsManager
private let loginItemsManager: LoginItemsManaging
private let ipcClient: NetworkProtectionIPCClient
private let pixelKit: PixelFiring?

init(featureVisibility: NetworkProtectionFeatureVisibility = DefaultNetworkProtectionVisibility(),
loginItemsManager: LoginItemsManager = LoginItemsManager(),
ipcClient: NetworkProtectionIPCClient) {
loginItemsManager: LoginItemsManaging = LoginItemsManager(),
ipcClient: NetworkProtectionIPCClient,
pixelKit: PixelFiring? = PixelKit.shared) {
diegoreymendez marked this conversation as resolved.
Show resolved Hide resolved

self.featureVisibility = featureVisibility
self.loginItemsManager = loginItemsManager
self.ipcClient = ipcClient
self.pixelKit = pixelKit
}

// MARK: - Login Items Manager

private func enableLoginItems() async throws {
guard try await featureVisibility.canStartVPN() else {
throw RequestError.notAuthorizedToEnableLoginItem
}

do {
try loginItemsManager.throwingEnableLoginItems(LoginItemsManager.networkProtectionLoginItems, log: .networkProtection)
} catch {
throw RequestError.internalLoginItemError(error)
}
}
}

// MARK: - TunnelController Conformance

extension NetworkProtectionIPCTunnelController: TunnelController {

@MainActor
func start() async {
pixelKit?.fire(StartAttempt.begin)

func handleFailure(_ error: Error) {
log(error)
pixelKit?.fire(StartAttempt.failure(error), frequency: .dailyAndContinuous)
}

do {
guard try await enableLoginItems() else {
os_log("🔴 IPC Controller refusing to start the VPN menu app. Not authorized.", log: .networkProtection)
return
}
try await enableLoginItems()

ipcClient.start()
ipcClient.start { [pixelKit] error in
if let error {
handleFailure(error)
} else {
pixelKit?.fire(StartAttempt.success, frequency: .dailyAndContinuous)
}
}
} catch {
os_log("🔴 IPC Controller found en error when starting the VPN: \(error)", log: .networkProtection)
handleFailure(error)
}
}

@MainActor
func stop() async {
pixelKit?.fire(StopAttempt.begin)

func handleFailure(_ error: Error) {
log(error)
pixelKit?.fire(StopAttempt.failure(error), frequency: .dailyAndContinuous)
}

do {
guard try await enableLoginItems() else {
os_log("🔴 IPC Controller refusing to start the VPN. Not authorized.", log: .networkProtection)
return
}
try await enableLoginItems()

ipcClient.stop()
ipcClient.stop { [pixelKit] error in
if let error {
handleFailure(error)
} else {
pixelKit?.fire(StopAttempt.success, frequency: .dailyAndContinuous)
}
}
} catch {
os_log("🔴 IPC Controller found en error when starting the VPN: \(error)", log: .networkProtection)
handleFailure(error)
}
}

Expand All @@ -78,15 +144,90 @@ final class NetworkProtectionIPCTunnelController: TunnelController {
}
}

// MARK: - Login Items Manager
private func log(_ error: Error) {
switch error {
case RequestError.notAuthorizedToEnableLoginItem:
os_log("🔴 IPC Controller not authorized to enable the login item", log: .networkProtection)
case RequestError.internalLoginItemError(let error):
os_log("🔴 IPC Controller found an error while enabling the login item: \(error)", log: .networkProtection)
default:
os_log("🔴 IPC Controller found an unknown error: \(error)", log: .networkProtection)
}
}
}

private func enableLoginItems() async throws -> Bool {
guard try await featureVisibility.canStartVPN() else {
// We shouldn't enable the menu app is the VPN feature is disabled.
return false
// MARK: - Start Attempts

extension NetworkProtectionIPCTunnelController {

enum StartAttempt: PixelKitEventV2 {
case begin
case success
case failure(_ error: Error)

var name: String {
switch self {
case .begin:
return "netp_browser_start_attempt"

case .success:
return "netp_browser_start_success"

case .failure:
return "netp_browser_start_failure"
}
}

loginItemsManager.enableLoginItems(LoginItemsManager.networkProtectionLoginItems, log: .networkProtection)
return true
var parameters: [String: String]? {
return nil
}

var error: Error? {
switch self {
case .begin,
.success:
return nil
case .failure(let error):
return error
}
}
}
}

// MARK: - Stop Attempts

extension NetworkProtectionIPCTunnelController {

enum StopAttempt: PixelKitEventV2 {
case begin
case success
case failure(_ error: Error)

var name: String {
switch self {
case .begin:
return "netp_browser_stop_attempt"

case .success:
return "netp_browser_stop_success"

case .failure:
return "netp_browser_stop_failure"
}
}

var parameters: [String: String]? {
return nil
}

var error: Error? {
switch self {
case .begin,
.success:
return nil
case .failure(let error):
return error
}
}
}
}
4 changes: 3 additions & 1 deletion DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
}

func stop() {
ipcClient.stop()
ipcClient.stop { _ in
// Intentional no-op
}
}

private func enableLoginItems() {
Expand Down
Loading
Loading