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

Add DDG domain name check for fireproofing storage #3549

Merged
merged 5 commits into from
Nov 17, 2024
Merged

Add DDG domain name check for fireproofing storage #3549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc25d0f
WIP: Fire storage
Bunn Nov 13, 2024
69c292d
Change ddg domain name
Bunn Nov 14, 2024
b3a4c14
Add webview to fix tests running locally
Bunn Nov 14, 2024
f34862d
Add tests for storage
Bunn Nov 14, 2024
425b56f
Add datatype check as well
Bunn Nov 14, 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
4 changes: 2 additions & 2 deletions DuckDuckGo/Fireproofing/Extensions/FireproofingURLExtensions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private typealias URLPatterns = [String: [NSRegularExpression]]

extension URL {

static let cookieDomain = "duckduckgo.com"
static let duckduckgoDomain = "duckduckgo.com"

private static let loginPattern = regex("login|sign-in|signin|session")

Expand Down Expand Up @@ -53,7 +53,7 @@ extension URL {

var canFireproof: Bool {
guard let host = self.host, self.navigationalScheme?.isHypertextScheme == true else { return false }
return (host != Self.cookieDomain)
return (host != Self.duckduckgoDomain)
}

var showFireproofStatus: Bool {
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/Tab/Services/WebsiteDataStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ internal class WebCacheManager {
let removableRecords = allRecords.filter { record in
// For Local Storage, only remove records that *exactly match* the display name.
// Subdomains or root domains should be excluded.
!fireproofDomains.fireproofDomains.contains(record.displayName)
!URL.duckduckgoDomain.contains(record.displayName) && !fireproofDomains.fireproofDomains.contains(record.displayName)
}

await websiteDataStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypesExceptCookies, for: removableRecords)
Expand All @@ -155,7 +155,7 @@ internal class WebCacheManager {

// Don't clear fireproof domains
let cookiesToRemove = cookies.filter { cookie in
!self.fireproofDomains.isFireproof(cookieDomain: cookie.domain) && ![URL.cookieDomain, SubscriptionCookieManager.cookieDomain].contains(cookie.domain)
!self.fireproofDomains.isFireproof(cookieDomain: cookie.domain) && ![URL.duckduckgoDomain, SubscriptionCookieManager.cookieDomain].contains(cookie.domain)
}

for cookie in cookiesToRemove {
Expand Down
37 changes: 23 additions & 14 deletions UnitTests/Tab/Services/WebsiteDataStoreTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ import XCTest
@testable import DuckDuckGo_Privacy_Browser

final class WebCacheManagerTests: XCTestCase {
/// The webView is necessary to manage the shared state of WKWebsiteDataRecord
var webView: WKWebView?

override func setUp() {
super.setUp()
webView = WKWebView()
}

override func tearDown() {
webView = nil
super.tearDown()
}

func testWhenCookiesHaveSubDomainsOnSubDomainsAndWildcardsThenAllCookiesRetained() {
let logins = MockPreservedLogins(domains: [
Expand Down Expand Up @@ -89,7 +101,7 @@ final class WebCacheManagerTests: XCTestCase {

}

func testWhenClearedThenDDGCookiesAreRetained() {
@MainActor func testWhenClearedThenDDGCookiesAndStorageAreRetained() async {
let logins = MockPreservedLogins(domains: [
"example.com"
])
Expand All @@ -104,16 +116,17 @@ final class WebCacheManagerTests: XCTestCase {
MockDataRecord(recordName: "duckduckgo.com")
]

let expect = expectation(description: #function)
let webCacheManager = WebCacheManager(fireproofDomains: logins, websiteDataStore: dataStore)
Task {
await webCacheManager.clear()
expect.fulfill()
}
wait(for: [expect], timeout: 30.0)

// Await the clear function directly
await webCacheManager.clear()

// Assertions after the async operation
XCTAssertEqual(cookieStore.cookies.count, 1)
XCTAssertEqual(cookieStore.cookies[0].domain, "duckduckgo.com")

XCTAssertEqual(dataStore.records.count, 1)
XCTAssertEqual(dataStore.records.first?.displayName, "duckduckgo.com")
}

func testWhenClearedThenCookiesForLoginsAreRetained() {
Expand Down Expand Up @@ -184,15 +197,11 @@ final class WebCacheManagerTests: XCTestCase {
}
}

func removeData(ofTypes dataTypes: Set<String>, for records: [WKWebsiteDataRecord]) async {
func removeData(ofTypes dataTypes: Set<String>, for recordsToRemove: [WKWebsiteDataRecord]) async {
removeDataCalledCount += 1

// In the real implementation, records will be selectively removed or edited based on their Fireproof status. For simplicity in this test,
// only remove records if all data types are removed, so that we can tell whether records for given domains still exist in some form.
if dataTypes == WKWebsiteDataStore.allWebsiteDataTypes() {
self.records = records.filter {
dataTypes == $0.dataTypes
}
self.records = self.records.filter { record in
!recordsToRemove.contains(where: { $0 == record && $0.dataTypes.isSubset(of: dataTypes)})
}
}

Expand Down
Loading