From 4fc4092b4979e80b51f780eb3d587be6e3e2e95b Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 5 Feb 2024 12:50:01 +0000 Subject: [PATCH 1/5] fix ddg cookies on fire button --- Core/WebCacheManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 2c66cdd06b..0e94bf2383 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -389,7 +389,7 @@ extension WKWebsiteDataStore: WebCacheManagerDataStore { public func preservedCookies(_ preservedLogins: PreserveLogins) async -> [HTTPCookie] { let allCookies = await self.httpCookieStore.allCookies() return allCookies.filter { - preservedLogins.isAllowed(cookieDomain: $0.domain) + "duckduckgo.com" == $0.domain || preservedLogins.isAllowed(cookieDomain: $0.domain) } } From 8e5c2f4cd21ed2d3e1a262144c7139299033ed21 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 5 Feb 2024 13:55:36 +0000 Subject: [PATCH 2/5] be more comprehensive with ddg cookies --- Core/CookieStorage.swift | 2 +- Core/WebCacheManager.swift | 10 +++------- DuckDuckGoTests/CookieStorageTests.swift | 6 ++++++ DuckDuckGoTests/WebCacheManagerTests.swift | 8 +++++--- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Core/CookieStorage.swift b/Core/CookieStorage.swift index 5624827ee1..0a83d6cba1 100644 --- a/Core/CookieStorage.swift +++ b/Core/CookieStorage.swift @@ -114,7 +114,7 @@ public class CookieStorage { } persistedCookiesByDomain.keys.forEach { - guard $0 != "duckduckgo.com" else { return } // DDG cookies are for SERP settings only + guard !URL.isDuckDuckGo(domain: $0) else { return } // DDG cookies are for SERP settings only if !preservedLogins.isAllowed(cookieDomain: $0) { persistedCookiesByDomain.removeValue(forKey: $0) diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 0e94bf2383..d9b91bb2f9 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -56,10 +56,6 @@ extension WebCacheManagerDataStore { } public class WebCacheManager { - - private struct Constants { - static let cookieDomainsToPreserve = ["duckduckgo.com", "surveys.duckduckgo.com"] - } public static var shared = WebCacheManager() @@ -223,7 +219,7 @@ public class WebCacheManager { func keep(_ cookie: HTTPCookie) -> Bool { return logins.isAllowed(cookieDomain: cookie.domain) || - Constants.cookieDomainsToPreserve.contains(cookie.domain) + URL.isDuckDuckGo(domain: cookie.domain) } let dataStore = WKWebsiteDataStore.default() @@ -270,7 +266,7 @@ public class WebCacheManager { // Remove legacy HTTPCookieStorage cookies let storageCookies = HTTPCookieStorage.shared.cookies ?? [] let storageCookiesToRemove = storageCookies.filter { - !logins.isAllowed(cookieDomain: $0.domain) && !Constants.cookieDomainsToPreserve.contains($0.domain) + !logins.isAllowed(cookieDomain: $0.domain) && !URL.isDuckDuckGo(domain: $0.domain) } let protectedStorageCookiesCount = storageCookies.count - storageCookiesToRemove.count @@ -389,7 +385,7 @@ extension WKWebsiteDataStore: WebCacheManagerDataStore { public func preservedCookies(_ preservedLogins: PreserveLogins) async -> [HTTPCookie] { let allCookies = await self.httpCookieStore.allCookies() return allCookies.filter { - "duckduckgo.com" == $0.domain || preservedLogins.isAllowed(cookieDomain: $0.domain) + URL.isDuckDuckGo(domain: $0.domain) || preservedLogins.isAllowed(cookieDomain: $0.domain) } } diff --git a/DuckDuckGoTests/CookieStorageTests.swift b/DuckDuckGoTests/CookieStorageTests.swift index 79b0dc2751..4a78e3e006 100644 --- a/DuckDuckGoTests/CookieStorageTests.swift +++ b/DuckDuckGoTests/CookieStorageTests.swift @@ -51,6 +51,12 @@ public class CookieStorageTests: XCTestCase { XCTAssertEqual(2, storage.cookies.count) + storage.updateCookies([ + make("usedev1.duckduckgo.com", name: "x", value: "1"), + ], keepingPreservedLogins: logins) + + XCTAssertEqual(3, storage.cookies.count) + } func testWhenUpdatedThenCookiesWithFutureExpirationAreNotRemoved() { diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index 488960990e..b0c3dc2b2c 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -128,7 +128,8 @@ class WebCacheManagerTests: XCTestCase { let dataStore = MockDataStore() let cookieStore = MockHTTPCookieStore(cookies: [ - .make(domain: "duckduckgo.com") + .make(domain: "duckduckgo.com"), + .make(domain: "subdomain.duckduckgo.com") ]) dataStore.cookieStore = cookieStore @@ -139,8 +140,9 @@ class WebCacheManagerTests: XCTestCase { } wait(for: [expect], timeout: 5.0) - XCTAssertEqual(cookieStore.cookies.count, 1) - XCTAssertEqual(cookieStore.cookies[0].domain, "duckduckgo.com") + XCTAssertEqual(cookieStore.cookies.count, 2) + XCTAssertTrue(cookieStore.cookies.contains(where: { $0.domain == "duckduckgo.com" })) + XCTAssertTrue(cookieStore.cookies.contains(where: { $0.domain == "subdomain.duckduckgo.com" })) } @MainActor From ca3680a5a04eb3e4aa1176ff53bccd8314a8b168 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 6 Feb 2024 09:36:42 +0000 Subject: [PATCH 3/5] Run the reference tests through the clear data logic --- .../FireButtonReferenceTests.swift | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index 2f04f6fbc3..fee9f7e9ec 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -44,6 +44,54 @@ final class FireButtonReferenceTests: XCTestCase { return url.host! } + func testClearData() async { + let preservedLogins = PreserveLogins.shared + preservedLogins.clearAll() + + for site in testData.fireButtonFireproofing.fireproofedSites { + let sanitizedSite = sanitizedSite(site) + os_log("Adding %s to fireproofed sites", sanitizedSite) + preservedLogins.addToAllowed(domain: sanitizedSite) + } + + let referenceTests = testData.fireButtonFireproofing.tests.filter { + $0.exceptPlatforms.contains("ios-browser") == false + } + + let cookieStorage = CookieStorage() + let idManager = DataStoreIdManager() + for test in referenceTests { + guard let cookie = cookie(for: test) else { + XCTFail("Cookie should exist for test \(test.name)") + return + } + + // Set directly to avoid logic to remove non-preserved cookies + cookieStorage.cookies = [ + cookie + ] + + idManager.allocateNewContainerId() + await withCheckedContinuation { continuation in + WebCacheManager.shared.clear(cookieStorage: cookieStorage, logins: preservedLogins, dataStoreIdManager: idManager) { + continuation.resume() + } + } + + let testCookie = cookieStorage.cookies.filter { $0.name == test.cookieName }.first + + if test.expectCookieRemoved { + XCTAssertNil(testCookie, "Cookie should not exist for test: \(test.name)") + } else { + XCTAssertNotNil(testCookie, "Cookie should exist for test: \(test.name)") + } + + // Reset cache + cookieStorage.cookies = [] + } + + } + func testCookieStorage() { let preservedLogins = PreserveLogins.shared preservedLogins.clearAll() From d7b37e084987f3a8c63651952fdfff8ca914cdc4 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 6 Feb 2024 11:04:39 +0000 Subject: [PATCH 4/5] use nice unwrap --- DuckDuckGoTests/FireButtonReferenceTests.swift | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index fee9f7e9ec..e0153c3e44 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -44,7 +44,7 @@ final class FireButtonReferenceTests: XCTestCase { return url.host! } - func testClearData() async { + func testClearData() async throws { let preservedLogins = PreserveLogins.shared preservedLogins.clearAll() @@ -60,11 +60,8 @@ final class FireButtonReferenceTests: XCTestCase { let cookieStorage = CookieStorage() let idManager = DataStoreIdManager() - for test in referenceTests { - guard let cookie = cookie(for: test) else { - XCTFail("Cookie should exist for test \(test.name)") - return - } + for test in referenceTests { + let cookie = try XCTUnwrap(cookie(for: test)) // Set directly to avoid logic to remove non-preserved cookies cookieStorage.cookies = [ @@ -92,7 +89,7 @@ final class FireButtonReferenceTests: XCTestCase { } - func testCookieStorage() { + func testCookieStorage() throws { let preservedLogins = PreserveLogins.shared preservedLogins.clearAll() @@ -108,11 +105,8 @@ final class FireButtonReferenceTests: XCTestCase { let cookieStorage = CookieStorage() for test in referenceTests { - guard let cookie = cookie(for: test) else { - XCTFail("Cookie should exist for test \(test.name)") - return - } - + let cookie = try XCTUnwrap(cookie(for: test)) + cookieStorage.updateCookies([ cookie ], keepingPreservedLogins: preservedLogins) From 977f933a9f584038bad82b76a7ee1d0e70564736 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 6 Feb 2024 11:11:00 +0000 Subject: [PATCH 5/5] swiftlint --- DuckDuckGoTests/FireButtonReferenceTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index e0153c3e44..6e1ea46555 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -60,7 +60,7 @@ final class FireButtonReferenceTests: XCTestCase { let cookieStorage = CookieStorage() let idManager = DataStoreIdManager() - for test in referenceTests { + for test in referenceTests { let cookie = try XCTUnwrap(cookie(for: test)) // Set directly to avoid logic to remove non-preserved cookies