Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #7373: Remove duplicate shields settings (Phishing and Malware) (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Brandon-T authored and iccub committed May 10, 2023
1 parent 35f63bb commit 29c4d7f
Show file tree
Hide file tree
Showing 12 changed files with 3 additions and 63 deletions.
1 change: 0 additions & 1 deletion Sources/Brave/Frontend/Browser/BrowserViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2995,7 +2995,6 @@ extension BrowserViewController: PreferencesObserver {
tabManager.selectedTab?.updatePullToRefreshVisibility()
case Preferences.Shields.blockAdsAndTracking.key,
Preferences.Shields.blockScripts.key,
Preferences.Shields.blockPhishingAndMalware.key,
Preferences.Shields.blockImages.key,
Preferences.Shields.fingerprintingProtection.key,
Preferences.Shields.useRegionAdBlock.key:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ extension BrowserViewController {
// Q54 On how many domains has the user set the FP setting to be higher (block more) than the default?
let fingerprintingAboveGlobalCount = Domain.totalDomainsWithFingerprintingProtectionIncreasedFromGlobal()
UmaHistogramRecordValueToBucket("Brave.Shields.DomainFingerprintSettingsAboveGlobal", buckets: buckets, value: fingerprintingAboveGlobalCount)
case .AllOff, .NoScript, .SafeBrowsing:
case .AllOff, .NoScript:
break
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ extension BrowserViewController: WKNavigationDelegate {
}

let domainForMainFrame = Domain.getOrCreate(forUrl: mainDocumentURL, persistent: !isPrivateBrowsing)
// Enable safe browsing (frodulent website warnings)
webView.configuration.preferences.isFraudulentWebsiteWarningEnabled = domainForMainFrame.isShieldExpected(.SafeBrowsing, considerAllShieldsOption: true)

// Debouncing logic
// Handle debouncing for main frame only and only if the site (etld+1) changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,6 @@ extension PlaylistWebLoader: WKNavigationDelegate {
let scriptTypes = await tab.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
tab.setCustomUserScript(scripts: scriptTypes)
}

webView.configuration.preferences.isFraudulentWebsiteWarningEnabled = domainForMainFrame.isShieldExpected(.SafeBrowsing, considerAllShieldsOption: true)
}

if ["http", "https", "data", "blob", "file"].contains(url.scheme) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
rows: [
.boolRow(title: Strings.blockAdsAndTracking, detailText: Strings.blockAdsAndTrackingDescription, option: Preferences.Shields.blockAdsAndTracking),
.boolRow(title: Strings.HTTPSEverywhere, detailText: Strings.HTTPSEverywhereDescription, option: Preferences.Shields.httpsEverywhere),
.boolRow(title: Strings.blockPhishingAndMalware, option: Preferences.Shields.blockPhishingAndMalware),
.boolRow(title: Strings.autoRedirectAMPPages, detailText: Strings.autoRedirectAMPPagesDescription, option: Preferences.Shields.autoRedirectAMPPages),
.boolRow(title: Strings.autoRedirectTrackingURLs, detailText: Strings.autoRedirectTrackingURLsDescription, option: Preferences.Shields.autoRedirectTrackingURLs),
.boolRow(title: Strings.blockScripts, detailText: Strings.blockScriptsDescription, option: Preferences.Shields.blockScripts),
Expand Down
1 change: 0 additions & 1 deletion Sources/Brave/Frontend/Shields/ShieldsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
/// Groups the shield types with their control and global preference
private lazy var shieldControlMapping: [(BraveShield, AdvancedShieldsView.ToggleView, Preferences.Option<Bool>?)] = [
(.AdblockAndTp, shieldsView.advancedShieldView.adsTrackersControl, Preferences.Shields.blockAdsAndTracking),
(.SafeBrowsing, shieldsView.advancedShieldView.blockMalwareControl, Preferences.Shields.blockPhishingAndMalware),
(.NoScript, shieldsView.advancedShieldView.blockScriptsControl, Preferences.Shields.blockScripts),
(.FpProtection, shieldsView.advancedShieldView.fingerprintingControl, Preferences.Shields.fingerprintingProtection),
]
Expand Down
1 change: 0 additions & 1 deletion Sources/Brave/Migration/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ fileprivate extension Preferences {
// Shields
migrate(key: "braveBlockAdsAndTracking", to: Preferences.Shields.blockAdsAndTracking)
migrate(key: "braveHttpsEverywhere", to: Preferences.Shields.httpsEverywhere)
migrate(key: "braveSafeBrowsing", to: Preferences.Shields.blockPhishingAndMalware)
migrate(key: "noscript_on", to: Preferences.Shields.blockScripts)
migrate(key: "fingerprintprotection_on", to: Preferences.Shields.fingerprintingProtection)
migrate(key: "braveAdblockUseRegional", to: Preferences.Shields.useRegionAdBlock)
Expand Down
3 changes: 0 additions & 3 deletions Sources/BraveShields/BraveShield.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import Preferences
public enum BraveShield {
case AllOff
case AdblockAndTp
case SafeBrowsing
case FpProtection
case NoScript

Expand All @@ -19,8 +18,6 @@ public enum BraveShield {
return false
case .AdblockAndTp:
return Preferences.Shields.blockAdsAndTracking.value
case .SafeBrowsing:
return Preferences.Shields.blockPhishingAndMalware.value
case .FpProtection:
return Preferences.Shields.fingerprintingProtection.value
case .NoScript:
Expand Down
1 change: 0 additions & 1 deletion Sources/BraveStrings/BraveStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,6 @@ extension Strings {
public static let blockAdsAndTrackingDescription = NSLocalizedString("BlockAdsAndTrackingDescription", tableName: "BraveShared", bundle: .module, value: "Prevents ads, popups, and trackers from loading.", comment: "")
public static let HTTPSEverywhere = NSLocalizedString("HTTPSEverywhere", tableName: "BraveShared", bundle: .module, value: "Upgrade Connections to HTTPS", comment: "")
public static let HTTPSEverywhereDescription = NSLocalizedString("HTTPSEverywhereDescription", tableName: "BraveShared", bundle: .module, value: "Opens sites using secure HTTPS instead of HTTP when possible.", comment: "")
public static let blockPhishingAndMalware = NSLocalizedString("BlockPhishingAndMalware", tableName: "BraveShared", bundle: .module, value: "Block Phishing and Malware", comment: "")
public static let googleSafeBrowsing = NSLocalizedString("GoogleSafeBrowsing", tableName: "BraveShared", bundle: .module, value: "Block Dangerous Sites", comment: "")
public static let googleSafeBrowsingUsingWebKitDescription = NSLocalizedString("GoogleSafeBrowsingUsingWebKitDescription", tableName: "BraveShared", bundle: .module, value: "Sends obfuscated URLs of some pages you visit to the Google Safe Browsing service, when your security is at risk.", comment: "")
public static let blockScripts = NSLocalizedString("BlockScripts", tableName: "BraveShared", bundle: .module, value: "Block Scripts", comment: "")
Expand Down
5 changes: 0 additions & 5 deletions Sources/Data/models/Domain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ public final class Domain: NSManagedObject, CRUD {
return self.shield_allOff?.boolValue ?? false
case .AdblockAndTp:
return self.shield_adblockAndTp?.boolValue ?? Preferences.Shields.blockAdsAndTracking.value
case .SafeBrowsing:
return self.shield_safeBrowsing?.boolValue ?? Preferences.Shields.blockPhishingAndMalware.value
case .FpProtection:
return self.shield_fpProtection?.boolValue ?? Preferences.Shields.fingerprintingProtection.value
case .NoScript:
Expand Down Expand Up @@ -402,7 +400,6 @@ extension Domain {
switch shield {
case .AllOff: shield_allOff = setting
case .AdblockAndTp: shield_adblockAndTp = setting
case .SafeBrowsing: shield_safeBrowsing = setting
case .FpProtection: shield_fpProtection = setting
case .NoScript: shield_noScript = setting
}
Expand All @@ -415,8 +412,6 @@ extension Domain {
return self.shield_allOff?.boolValue
case .AdblockAndTp:
return self.shield_adblockAndTp?.boolValue
case .SafeBrowsing:
return self.shield_safeBrowsing?.boolValue
case .FpProtection:
return self.shield_fpProtection?.boolValue
case .NoScript:
Expand Down
4 changes: 1 addition & 3 deletions Sources/Preferences/GlobalPreferences.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,14 @@ extension Preferences {
}

public final class Shields {
public static let allShields = [blockAdsAndTracking, httpsEverywhere, blockPhishingAndMalware, googleSafeBrowsing, blockScripts, fingerprintingProtection, blockImages]
public static let allShields = [blockAdsAndTracking, httpsEverywhere, googleSafeBrowsing, blockScripts, fingerprintingProtection, blockImages]

/// Shields will block ads and tracking if enabled
public static let blockAdsAndTracking = Option<Bool>(key: "shields.block-ads-and-tracking", default: true)
/// Websites will be upgraded to HTTPS if a loaded page attempts to use HTTP
public static let httpsEverywhere = Option<Bool>(key: "shields.https-everywhere", default: true)
/// Enable Google Safe Browsing
public static let googleSafeBrowsing = Option<Bool>(key: "shields.google-safe-browsing", default: true)
/// Shields will block websites related to potential phishing and malware
public static let blockPhishingAndMalware = Option<Bool>(key: "shields.block-phishing-and-malware", default: true)
/// Disables JavaScript execution in the browser
public static let blockScripts = Option<Bool>(key: "shields.block-scripts", default: false)
/// Enforces fingerprinting protection on the users session
Expand Down
43 changes: 1 addition & 42 deletions Tests/DataTests/DomainTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class DomainTests: CoreDataTestCase {
@MainActor func testDefaultShieldSettings() {
let domain = Domain.getOrCreate(forUrl: url, persistent: true)
XCTAssertTrue(domain.isShieldExpected(BraveShield.AdblockAndTp, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.SafeBrowsing, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.AllOff, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.NoScript, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.FpProtection, considerAllShieldsOption: true))
Expand All @@ -79,7 +78,6 @@ class DomainTests: CoreDataTestCase {
}

XCTAssertFalse(domain.isShieldExpected(BraveShield.AdblockAndTp, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.SafeBrowsing, considerAllShieldsOption: false))
XCTAssertFalse(domain.isShieldExpected(BraveShield.AllOff, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.NoScript, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.FpProtection, considerAllShieldsOption: true))
Expand All @@ -89,40 +87,26 @@ class DomainTests: CoreDataTestCase {
}

XCTAssertTrue(domain.isShieldExpected(BraveShield.AdblockAndTp, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.SafeBrowsing, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.AllOff, considerAllShieldsOption: true))
XCTAssertFalse(domain.isShieldExpected(BraveShield.NoScript, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.FpProtection, considerAllShieldsOption: true))
}

/// Tests non-HTTPSE shields
@MainActor func testNormalShieldSettings() {

backgroundSaveAndWaitForExpectation {
Domain.setBraveShield(forUrl: url2HTTPS, shield: .SafeBrowsing, isOn: true, isPrivateBrowsing: false)
}

backgroundSaveAndWaitForExpectation {
Domain.setBraveShield(forUrl: url2HTTPS, shield: .AdblockAndTp, isOn: false, isPrivateBrowsing: false)
}

let domain = Domain.getOrCreate(forUrl: url2HTTPS, persistent: true)
XCTAssertTrue(domain.isShieldExpected(BraveShield.SafeBrowsing, considerAllShieldsOption: true))

// These should be the same in this situation
XCTAssertFalse(domain.isShieldExpected(BraveShield.AdblockAndTp, considerAllShieldsOption: true))

// Setting to "new" values
// Setting to same value
backgroundSaveAndWaitForExpectation {
Domain.setBraveShield(forUrl: url2HTTPS, shield: .SafeBrowsing, isOn: true, isPrivateBrowsing: false)
}

backgroundSaveAndWaitForExpectation {
Domain.setBraveShield(forUrl: url2HTTPS, shield: .AdblockAndTp, isOn: true, isPrivateBrowsing: false)
}

domain.managedObjectContext?.refreshAllObjects()
XCTAssertTrue(domain.isShieldExpected(BraveShield.SafeBrowsing, considerAllShieldsOption: true))
XCTAssertTrue(domain.isShieldExpected(BraveShield.AdblockAndTp, considerAllShieldsOption: true))
}

Expand Down Expand Up @@ -199,29 +183,4 @@ class DomainTests: CoreDataTestCase {
}
XCTAssertFalse(raydiumDomain.walletPermissions(for: .sol, account: walletSolAccount))
}

@MainActor func testSafeBrowsing() {
Preferences.Shields.blockPhishingAndMalware.reset()
DataController.shared = DataController()
DataController.shared.initializeOnce()

let urls = [
URL(string: "http://brave.com")!,
URL(string: "https://brave.com")!,
URL(string: "https://www.brave.com")!,
URL(string: "https://example.com")!
]

for url in urls {
let domain = Domain.getOrCreate(forUrl: url, persistent: false)
XCTAssertTrue(domain.isShieldExpected(.SafeBrowsing, considerAllShieldsOption: true))
}

Preferences.Shields.blockPhishingAndMalware.value = false

for url in urls {
let domain = Domain.getOrCreate(forUrl: url, persistent: false)
XCTAssertFalse(domain.isShieldExpected(.SafeBrowsing, considerAllShieldsOption: true))
}
}
}

0 comments on commit 29c4d7f

Please sign in to comment.