Skip to content

Commit

Permalink
Reports on toggle protections off (#2312)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/72649045549333/1205734628204384/f

**Description**:
Send a simplified breakage report through Pixel when user disables
protections on iOS and macOS as such volume information from the toggle
can help us identify breakage beyond what we can learn from regular
breakage reports.

We aim to:
- Explicitly request permission for such report without directing the
user toward a specific response.
- Be transparent by openly listing the exact information we intend to
send.

**Steps to test this PR**:
See: https://app.asana.com/0/0/1206747477156794/f
  • Loading branch information
@jaceklyp
jaceklyp authored Mar 11, 2024
1 parent 62e1b3b commit 91f1f9a
Show file tree
Hide file tree
Showing 20 changed files with 192 additions and 88 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13770,7 +13770,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 120.0.0;
version = 121.0.0;
};
};
AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = {
Expand Down
8 changes: 4 additions & 4 deletions DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "cea7c43e5ab1d7484ab29abeda429350ed8a7dc1",
"version" : "120.0.0"
"revision" : "4555c3dbf265f1dca0304c69e7013b9d46a758b3",
"version" : "121.0.0"
}
},
{
Expand Down Expand Up @@ -95,8 +95,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/privacy-dashboard",
"state" : {
"revision" : "c67d268bf234760f49034a0fe7a6137a1b216b05",
"version" : "3.2.0"
"revision" : "43a6e1c1864846679a254e60c91332c3fbd922ee",
"version" : "3.3.0"
}
},
{
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/Application/AppDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel
#if DBP
DataBrokerProtectionAppEvents().applicationDidBecomeActive()
#endif
AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager.toggleProtectionsCounter.sendEventsIfNeeded()
}

func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
Expand Down
10 changes: 10 additions & 0 deletions DuckDuckGo/ContentBlocker/ContentBlocking.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ final class AppContentBlocking {
embeddedDataProvider: AppPrivacyConfigurationDataProvider(),
localProtection: LocalUnprotectedDomains.shared,
errorReporting: Self.debugEvents,
toggleProtectionsCounterEventReporting: toggleProtectionsEvents,
internalUserDecider: internalUserDecider)

trackerDataManager = TrackerDataManager(etag: ConfigurationStore.shared.loadEtag(for: .trackerDataSet),
Expand Down Expand Up @@ -96,6 +97,15 @@ final class AppContentBlocking {
log: .attribution)
}

private let toggleProtectionsEvents = EventMapping<ToggleProtectionsCounterEvent> { event, _, parameters, _ in
let domainEvent: Pixel.Event
switch event {
case .toggleProtectionsCounterDaily:
domainEvent = .toggleProtectionsDailyCount
}
Pixel.fire(domainEvent, withAdditionalParameters: parameters ?? [:])
}

private static let debugEvents = EventMapping<ContentBlockerDebugEvents> { event, error, parameters, onComplete in
guard NSApp.runType.requiresEnvironment else { return }

Expand Down
3 changes: 3 additions & 0 deletions DuckDuckGo/ContentBlocker/Mocks/MockPrivacyConfiguration.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import BrowserServicesKit
import Combine
import Common

#if DEBUG

Expand Down Expand Up @@ -95,6 +96,8 @@ final class MockPrivacyConfigurationManager: NSObject, PrivacyConfigurationManag
var updatesPublisher: AnyPublisher<Void, Never> = Just(()).eraseToAnyPublisher()
var privacyConfig: PrivacyConfiguration = MockPrivacyConfiguration()
var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider()
var toggleProtectionsCounter: ToggleProtectionsCounter = ToggleProtectionsCounter(eventReporting: EventMapping<ToggleProtectionsCounterEvent> { _, _, _, _ in
})
}

#endif
2 changes: 1 addition & 1 deletion DuckDuckGo/Menus/MainMenuActions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ extension AppDelegate {
@objc func openReportBrokenSite(_ sender: Any?) {
let storyboard = NSStoryboard(name: "PrivacyDashboard", bundle: nil)
let privacyDashboardViewController = storyboard.instantiateController(identifier: "PrivacyDashboardViewController") { coder in
PrivacyDashboardViewController(coder: coder, initMode: .reportBrokenSite)
PrivacyDashboardViewController(coder: coder, privacyInfo: nil, dashboardMode: .report)
}

privacyDashboardViewController.sizeDelegate = self
Expand Down
145 changes: 97 additions & 48 deletions DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,49 +36,65 @@ final class PrivacyDashboardViewController: NSViewController {
static let initialContentWidth: CGFloat = 360.0
}

/// Type of web page displayed
enum Mode {
case privacyDashboard
case reportBrokenSite
}

private var webView: WKWebView!
private let initMode: Mode

var source: WebsiteBreakage.Source {
initMode == .reportBrokenSite ? .appMenu : .dashboard
}
private let privacyDashboardController: PrivacyDashboardController
private var privacyDashboardDidTriggerDismiss: Bool = false

private let privacyDashboardController = PrivacyDashboardController(privacyInfo: nil)
public let rulesUpdateObserver = ContentBlockingRulesUpdateObserver()

private let websiteBreakageReporter: WebsiteBreakageReporter = {
WebsiteBreakageReporter(pixelHandler: { parameters in
private let brokenSiteReporter: BrokenSiteReporter = {
BrokenSiteReporter(pixelHandler: { parameters in
Pixel.fire(
.brokenSiteReport,
withAdditionalParameters: parameters,
allowedQueryReservedCharacters: WebsiteBreakage.allowedQueryReservedCharacters
allowedQueryReservedCharacters: BrokenSiteReport.allowedQueryReservedCharacters
)
}, keyValueStoring: UserDefaults.standard)
}()

private let toggleProtectionsOffReporter: BrokenSiteReporter = {
BrokenSiteReporter(pixelHandler: { parameters in
Pixel.fire(
.protectionToggledOffBreakageReport,
withAdditionalParameters: parameters,
allowedQueryReservedCharacters: BrokenSiteReport.allowedQueryReservedCharacters)
}, keyValueStoring: UserDefaults.standard)
}()

private let toggleReportEvents = EventMapping<ToggleReportEvents> { event, _, parameters, _ in
let domainEvent: Pixel.Event
switch event {
case .toggleReportDismiss: domainEvent = .toggleReportDismiss
case .toggleReportDoNotSend: domainEvent = .toggleReportDoNotSend
}
Pixel.fire(domainEvent, withAdditionalParameters: parameters)
}

private let permissionHandler = PrivacyDashboardPermissionHandler()
private var preferredMaxHeight: CGFloat = Constants.initialContentHeight
func setPreferredMaxHeight(_ height: CGFloat) {
guard height > Constants.initialContentHeight else { return }

preferredMaxHeight = height
}
var sizeDelegate: PrivacyDashboardViewControllerSizeDelegate?
private weak var tabViewModel: TabViewModel?

required init?(coder: NSCoder, initMode: Mode) {
self.initMode = initMode
required init?(coder: NSCoder,
privacyInfo: PrivacyInfo?,
dashboardMode: PrivacyDashboardMode,
privacyConfigurationManager: PrivacyConfigurationManaging = ContentBlocking.shared.privacyConfigurationManager) {
self.privacyDashboardController = PrivacyDashboardController(privacyInfo: privacyInfo,
dashboardMode: dashboardMode,
privacyConfigurationManager: privacyConfigurationManager,
eventMapping: toggleReportEvents)
super.init(coder: coder)
}

required init?(coder: NSCoder) {
self.initMode = .privacyDashboard
self.privacyDashboardController = PrivacyDashboardController(privacyInfo: nil,
dashboardMode: .dashboard,
privacyConfigurationManager: ContentBlocking.shared.privacyConfigurationManager,
eventMapping: toggleReportEvents)
super.init(coder: coder)
}

Expand All @@ -94,16 +110,23 @@ final class PrivacyDashboardViewController: NSViewController {
}

public override func viewDidLoad() {

super.viewDidLoad()
initWebView()
privacyDashboardController.setup(for: webView, reportBrokenSiteOnly: initMode == .reportBrokenSite ? true : false)
privacyDashboardController.setup(for: webView)
privacyDashboardController.privacyDashboardNavigationDelegate = self
privacyDashboardController.privacyDashboardDelegate = self
privacyDashboardController.privacyDashboardReportBrokenSiteDelegate = self
privacyDashboardController.privacyDashboardToggleReportDelegate = self
privacyDashboardController.preferredLocale = Bundle.main.preferredLocalizations.first
}

override func viewWillDisappear() {
super.viewWillDisappear()
if !privacyDashboardDidTriggerDismiss {
privacyDashboardController.handleViewWillDisappear()
}
}

private func initWebView() {
let configuration = WKWebViewConfiguration()
#if DEBUG
Expand Down Expand Up @@ -146,9 +169,7 @@ final class PrivacyDashboardViewController: NSViewController {
}

private func privacyDashboardProtectionSwitchChangeHandler(state: ProtectionState) {

dismiss()

privacyDashboardDidTriggerDismiss = true
guard let domain = privacyDashboardController.privacyInfo?.url.host else {
return
}
Expand All @@ -175,7 +196,9 @@ extension PrivacyDashboardViewController: PrivacyDashboardControllerDelegate {
// Not used in macOS: Pixel.fire(.privacyDashboardReportBrokenSite)
}

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController, didChangeProtectionSwitch protectionState: ProtectionState) {
func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController,
didChangeProtectionSwitch protectionState: ProtectionState,
didSendReport: Bool) {
privacyDashboardProtectionSwitchChangeHandler(state: protectionState)
}

Expand Down Expand Up @@ -206,7 +229,6 @@ extension PrivacyDashboardViewController: PrivacyDashboardControllerDelegate {

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController, didSetPermission permissionName: String, to state: PermissionAuthorizationState) {
guard let domain = self.privacyDashboardController.privacyInfo?.url.host else { return }

permissionHandler.setPermissionAuthorization(authorizationState: state, domain: domain, permissionName: permissionName)
}

Expand Down Expand Up @@ -235,11 +257,12 @@ extension PrivacyDashboardViewController: PrivacyDashboardReportBrokenSiteDelega
func privacyDashboardController(_ privacyDashboardController: PrivacyDashboard.PrivacyDashboardController,
didRequestSubmitBrokenSiteReportWithCategory category: String,
description: String) {
let source: BrokenSiteReport.Source = privacyDashboardController.initDashboardMode == .report ? .appMenu : .dashboard
do {
let websiteBreakage = try makeWebsiteBreakage(category: category, description: description)
try websiteBreakageReporter.report(breakage: websiteBreakage)
let report = try makeBrokenSiteReport(category: category, description: description, source: source)
try brokenSiteReporter.report(report, reportMode: .regular)
} catch {
os_log("Failed to generate or send the website breakage report: \(error.localizedDescription)", type: .error)
os_log("Failed to generate or send the broken site report: \(error.localizedDescription)", type: .error)
}
}

Expand All @@ -250,20 +273,44 @@ extension PrivacyDashboardViewController: PrivacyDashboardReportBrokenSiteDelega
}
}

// MARK: - PrivacyDashboardToggleReportDelegate

extension PrivacyDashboardViewController: PrivacyDashboardToggleReportDelegate {

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController,
didRequestSubmitToggleReportWithSource source: BrokenSiteReport.Source,
didOpenReportInfo: Bool,
toggleReportCounter: Int?) {
do {
let report = try makeBrokenSiteReport(source: source,
didOpenReportInfo: didOpenReportInfo,
toggleReportCounter: toggleReportCounter)
try toggleProtectionsOffReporter.report(report, reportMode: .toggle)
} catch {
os_log("Failed to generate or send the broken site report: %@", type: .error, error.localizedDescription)
}
}

}

// MARK: - Breakage

extension PrivacyDashboardViewController {

enum WebsiteBreakageError: Error {
enum BrokenSiteReportError: Error {
case failedToFetchTheCurrentURL
}

private func makeWebsiteBreakage(category: String, description: String) throws -> WebsiteBreakage {
private func makeBrokenSiteReport(category: String = "",
description: String = "",
source: BrokenSiteReport.Source,
didOpenReportInfo: Bool = false,
toggleReportCounter: Int? = nil) throws -> BrokenSiteReport {

// ⚠️ To limit privacy risk, site URL is trimmed to not include query and fragment
guard let currentTab = tabViewModel?.tab,
let currentURL = currentTab.content.url?.trimmingQueryItemsAndFragment() else {
throw WebsiteBreakageError.failedToFetchTheCurrentURL
throw BrokenSiteReportError.failedToFetchTheCurrentURL
}
let blockedTrackerDomains = currentTab.privacyInfo?.trackerInfo.trackersBlocked.compactMap { $0.domain } ?? []
let installedSurrogates = currentTab.privacyInfo?.trackerInfo.installedSurrogates.map {$0} ?? []
Expand All @@ -283,22 +330,24 @@ extension PrivacyDashboardViewController {
statusCodes = [httpStatusCode]
}

let websiteBreakage = WebsiteBreakage(siteUrl: currentURL,
category: category.lowercased(),
description: description,
osVersion: "\(ProcessInfo.processInfo.operatingSystemVersion)",
manufacturer: "Apple",
upgradedHttps: currentTab.privacyInfo?.connectionUpgradedTo != nil,
tdsETag: ContentBlocking.shared.contentBlockingManager.currentRules.first?.etag,
blockedTrackerDomains: blockedTrackerDomains,
installedSurrogates: installedSurrogates,
isGPCEnabled: PrivacySecurityPreferences.shared.gpcEnabled,
ampURL: ampURL,
urlParametersRemoved: urlParametersRemoved,
protectionsState: protectionsState,
reportFlow: source,
errors: errors,
httpStatusCodes: statusCodes)
let websiteBreakage = BrokenSiteReport(siteUrl: currentURL,
category: category.lowercased(),
description: description,
osVersion: "\(ProcessInfo.processInfo.operatingSystemVersion)",
manufacturer: "Apple",
upgradedHttps: currentTab.privacyInfo?.connectionUpgradedTo != nil,
tdsETag: ContentBlocking.shared.contentBlockingManager.currentRules.first?.etag,
blockedTrackerDomains: blockedTrackerDomains,
installedSurrogates: installedSurrogates,
isGPCEnabled: PrivacySecurityPreferences.shared.gpcEnabled,
ampURL: ampURL,
urlParametersRemoved: urlParametersRemoved,
protectionsState: protectionsState,
reportFlow: source,
errors: errors,
httpStatusCodes: statusCodes,
didOpenReportInfo: didOpenReportInfo,
toggleReportCounter: toggleReportCounter)
return websiteBreakage
}
}
9 changes: 9 additions & 0 deletions DuckDuckGo/Statistics/PixelEvent.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ extension Pixel {

case dailyPixel(Event, isFirst: Bool)

case protectionToggledOffBreakageReport
case toggleProtectionsDailyCount
case toggleReportDoNotSend
case toggleReportDismiss

enum Debug {
/// This is a convenience pixel that allows us to fire `PixelKitEvents` using our
/// regular `Pixel.fire()` calls. This is a convenience intermediate step to help ensure
Expand Down Expand Up @@ -595,6 +600,10 @@ extension Pixel.Event {
return "m_mac_netp_ev_geoswitching_set_custom"
case .networkProtectionGeoswitchingNoLocations:
return "m_mac_netp_ev_geoswitching_no_locations"
case .protectionToggledOffBreakageReport: return "m_mac_protection-toggled-off-breakage-report"
case .toggleProtectionsDailyCount: return "m_mac_toggle-protections-daily-count"
case .toggleReportDoNotSend: return "m_mac_toggle-report-do-not-send"
case .toggleReportDismiss: return "m_mac_toggle-report-dismiss"
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion DuckDuckGo/Statistics/PixelParameters.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,11 @@ extension Pixel.Event {
.dataBrokerDisableAndDeleteDaily,
.dataBrokerEnableLoginItemDaily,
.dataBrokerDisableLoginItemDaily,
.dataBrokerResetLoginItemDaily:
.dataBrokerResetLoginItemDaily,
.protectionToggledOffBreakageReport,
.toggleProtectionsDailyCount,
.toggleReportDoNotSend,
.toggleReportDismiss:
return nil
}
}
Expand Down
Loading

0 comments on commit 91f1f9a

Please sign in to comment.