Skip to content

Commit

Permalink
Additional breakage form metadata, part 1 (#2054)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1206333443611243/f
Tech Design URL:
CC:

Description:

This PR adds the following new fields to the breakage form:

• isAdminUser, to help us determine if a user is having installation issues because they don't have high enough privileges
• isInApplicationsDirectory, to let us know if the user's problems are because their app is in the wrong place
cpuArchitecture, so we can see if there's any difference in Intel vs Apple Silicon reports
• It also updates the version number in the report to include the build, in case internal users are filing feedback. Lastly, there is a new option in the Network Protection debug menu named "Log Feedback Metadata to Console", to help test the metadata collection more easily.
  • Loading branch information
samsymons authored Jan 22, 2024
1 parent 475a092 commit e6cb4da
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 5 deletions.
13 changes: 13 additions & 0 deletions DuckDuckGo/Common/Extensions/BundleExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@ extension Bundle {
return appGroup
}

var isInApplicationsDirectory: Bool {
let directoryPaths = NSSearchPathForDirectoriesInDomains(.applicationDirectory, .localDomainMask, true)

guard let applicationsPath = directoryPaths.first else {
// Default to true to be safe. In theory this should always return a valid path and the else branch will never be run, but some app logic
// depends on this check in order to allow users to proceed, so we should avoid blocking them in case this assumption is ever wrong.
return true
}

let path = self.bundlePath
return path.hasPrefix(applicationsPath)
}

}

enum BundleGroup {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ final class NetworkProtectionDebugMenu: NSMenu {
NSMenuItem(title: "Send Test Notification", action: #selector(NetworkProtectionDebugMenu.sendTestNotification))
.targetting(self)

NSMenuItem(title: "Log Feedback Metadata to Console", action: #selector(NetworkProtectionDebugMenu.logFeedbackMetadataToConsole))
.targetting(self)

NSMenuItem(title: "Onboarding")
.submenu(NetworkProtectionOnboardingMenu())

Expand Down Expand Up @@ -236,6 +239,17 @@ final class NetworkProtectionDebugMenu: NSMenu {
}
}

/// Prints feedback collector metadata to the console. This is to facilitate easier iteration of the metadata collector, without having to go through the feedback form flow every time.
///
@objc func logFeedbackMetadataToConsole(_ sender: Any?) {
Task { @MainActor in
let collector = DefaultVPNMetadataCollector()
let metadata = await collector.collectMetadata()

print(metadata.toPrettyPrintedJSON()!)
}
}

/// Sets the selected server.
///
@objc func setSelectedServer(_ menuItem: NSMenuItem) {
Expand Down
88 changes: 85 additions & 3 deletions DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ struct VPNMetadata: Encodable {
let appVersion: String
let lastVersionRun: String
let isInternalUser: Bool
let isAdminUser: String
let isInApplicationsDirectory: Bool
}

struct DeviceInfo: Encodable {
let osVersion: String
let buildFlavor: String
let lowPowerModeEnabled: Bool
let cpuArchitecture: String
}

struct NetworkInfo: Encodable {
Expand Down Expand Up @@ -151,11 +154,19 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector {
// MARK: - Metadata Collection

private func collectAppInfoMetadata() -> VPNMetadata.AppInfo {
let appVersion = AppVersion.shared.versionNumber
let appVersion = AppVersion.shared.versionAndBuildNumber
let versionStore = NetworkProtectionLastVersionRunStore()
let isInternalUser = NSApp.delegateTyped.internalUserDecider.isInternalUser
let isAdminUser = isAdminUser()
let isInApplicationsDirectory = Bundle.main.isInApplicationsDirectory

return .init(appVersion: appVersion, lastVersionRun: versionStore.lastVersionRun ?? "Unknown", isInternalUser: isInternalUser)
return .init(
appVersion: appVersion,
lastVersionRun: versionStore.lastVersionRun ?? "Unknown",
isInternalUser: isInternalUser,
isAdminUser: isAdminUser,
isInApplicationsDirectory: isInApplicationsDirectory
)
}

private func collectDeviceInfoMetadata() -> VPNMetadata.DeviceInfo {
Expand All @@ -169,7 +180,23 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector {
lowPowerModeEnabled = false
}

return .init(osVersion: osVersion, buildFlavor: buildFlavor, lowPowerModeEnabled: lowPowerModeEnabled)
let architecture = getMachineArchitecture()

return .init(osVersion: osVersion, buildFlavor: buildFlavor, lowPowerModeEnabled: lowPowerModeEnabled, cpuArchitecture: architecture)
}

private func getMachineArchitecture() -> String {
#if arch(arm)
return "arm"
#elseif arch(arm64)
return "arm64"
#elseif arch(i386)
return "i386"
#elseif arch(x86_64)
return "x86_64"
#else
return "unknown"
#endif
}

func collectNetworkInformation() async -> VPNMetadata.NetworkInfo {
Expand Down Expand Up @@ -249,4 +276,59 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector {

}

// MARK: - Admin User

private enum AdminQueryError: Error {
case queryExecutionFailed
case queriedWithoutResult
}

extension VPNMetadataCollector {

private func getUser() throws -> CSIdentity? {
let query = CSIdentityQueryCreateForCurrentUser(kCFAllocatorDefault).takeRetainedValue()
let flags = CSIdentityQueryFlags()

guard CSIdentityQueryExecute(query, flags, nil) else {
throw AdminQueryError.queryExecutionFailed
}

let users = CSIdentityQueryCopyResults(query).takeRetainedValue() as? [CSIdentity]
return users?.first
}

private func getAdminGroup() throws -> CSIdentity {
let privilegeGroup = "admin" as CFString
let authority = CSGetDefaultIdentityAuthority().takeRetainedValue()
let query = CSIdentityQueryCreateForName(kCFAllocatorDefault,
privilegeGroup,
kCSIdentityQueryStringEquals,
kCSIdentityClassGroup,
authority).takeRetainedValue()
let flags = CSIdentityQueryFlags()

guard CSIdentityQueryExecute(query, flags, nil) else { throw AdminQueryError.queryExecutionFailed }
let groups = CSIdentityQueryCopyResults(query).takeRetainedValue() as? [CSIdentity]

guard let adminGroup = groups?.first else {
throw AdminQueryError.queriedWithoutResult
}

return adminGroup
}

fileprivate func isAdminUser() -> String {
do {
let user = try self.getUser()
let group = try self.getAdminGroup()

let isAdmin = CSIdentityIsMemberOfGroup(user, group)
return String(describing: isAdmin)
} catch {
return "error checking status: \(error.localizedDescription)"
}
}

}

#endif
17 changes: 15 additions & 2 deletions UnitTests/VPNFeedbackForm/VPNFeedbackFormViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,21 @@ private class MockVPNMetadataCollector: VPNMetadataCollector {
func collectMetadata() async -> VPNMetadata {
self.collectedMetadata = true

let appInfo = VPNMetadata.AppInfo(appVersion: "1.2.3", lastVersionRun: "1.2.3", isInternalUser: false)
let deviceInfo = VPNMetadata.DeviceInfo(osVersion: "14.0.0", buildFlavor: "dmg", lowPowerModeEnabled: false)
let appInfo = VPNMetadata.AppInfo(
appVersion: "1.2.3",
lastVersionRun: "1.2.3",
isInternalUser: false,
isAdminUser: "true",
isInApplicationsDirectory: true
)

let deviceInfo = VPNMetadata.DeviceInfo(
osVersion: "14.0.0",
buildFlavor: "dmg",
lowPowerModeEnabled: false,
cpuArchitecture: "arm64"
)

let networkInfo = VPNMetadata.NetworkInfo(currentPath: "path")

let vpnState = VPNMetadata.VPNState(
Expand Down

0 comments on commit e6cb4da

Please sign in to comment.