Skip to content

Commit

Permalink
macOS VPN: Add pixels to track VPN start and stop attempts through IPC (
Browse files Browse the repository at this point in the history 
#2622)

Task/Issue URL:
https://app.asana.com/0/1199230911884351/1207077286880106/f

## Description

In macOS we don't have visibility on how often and how IPC start and
stop attempts are failing for our tunnel.

This PR adds pixels to track those attempts.
  • Loading branch information
@diegoreymendez
diegoreymendez authored Apr 18, 2024
1 parent 1a5bc96 commit b06570a
Show file tree
Hide file tree
Showing 14 changed files with 629 additions and 53 deletions.
20 changes: 20 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
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
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
}

/// 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
}
}
}

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
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
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
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) {

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
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

0 comments on commit b06570a

Please sign in to comment.