From 7b78b46340c9981b9774352be08c3c28b3a19011 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Fri, 1 Nov 2024 13:02:07 +0100 Subject: [PATCH] Update to subscription cookie (#1053) Please review the release process for BrowserServicesKit [here](https://app.asana.com/0/1200194497630846/1200837094583426). **Required**: Task/Issue URL: https://app.asana.com/0/1108686900785972/1208264562025859/f iOS PR: https://github.com/duckduckgo/iOS/pull/3512 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3489 What kind of version bump will this require?: Patch **Description**: Update to how the subscription cookie operates: - constraint the cookie to `subscriptions.duckduckgo.com` - on sign out do not fully remove the cookie - just clear the value - gate the feature behind the `setAccessTokenCookieForSubscriptionDomains` privacy config feature flag **Steps to test this PR**: See client PRs **OS Testing**: * [ ] iOS 14 * [ ] iOS 15 * [ ] iOS 16 * [ ] macOS 10.15 * [ ] macOS 11 * [ ] macOS 12 --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) --- .../Features/PrivacyFeature.swift | 1 + .../SubscriptionCookieManager.swift | 83 ++++++++++++------- .../SubscriptionCookieManagerEvent.swift | 5 +- .../SubscriptionCookieManagerMock.swift | 2 + .../SubscriptionCookieManagerTests.swift | 20 +++-- 5 files changed, 71 insertions(+), 40 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index 478aa8c4a..487819d36 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -152,6 +152,7 @@ public enum PrivacyProSubfeature: String, Equatable, PrivacySubfeature { case isLaunchedOverride case isLaunchedOverrideStripe case useUnifiedFeedback + case setAccessTokenCookieForSubscriptionDomains } public enum SslCertificatesSubfeature: String, PrivacySubfeature { diff --git a/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManager.swift b/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManager.swift index 7d7099d75..eff9f2e69 100644 --- a/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManager.swift +++ b/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManager.swift @@ -22,6 +22,9 @@ import os.log public protocol SubscriptionCookieManaging { init(subscriptionManager: SubscriptionManager, currentCookieStore: @MainActor @escaping () -> HTTPCookieStore?, eventMapping: EventMapping) + func enableSettingSubscriptionCookie() + func disableSettingSubscriptionCookie() async + func refreshSubscriptionCookie() async func resetLastRefreshDate() @@ -30,7 +33,7 @@ public protocol SubscriptionCookieManaging { public final class SubscriptionCookieManager: SubscriptionCookieManaging { - public static let cookieDomain = ".duckduckgo.com" + public static let cookieDomain = "subscriptions.duckduckgo.com" public static let cookieName = "privacy_pro_access_token" private static let defaultRefreshTimeInterval: TimeInterval = .hours(4) @@ -41,6 +44,7 @@ public final class SubscriptionCookieManager: SubscriptionCookieManaging { public private(set) var lastRefreshDate: Date? private let refreshTimeInterval: TimeInterval + private var isSettingSubscriptionCookieEnabled: Bool = false convenience nonisolated public required init(subscriptionManager: SubscriptionManager, currentCookieStore: @MainActor @escaping () -> HTTPCookieStore?, @@ -60,17 +64,28 @@ public final class SubscriptionCookieManager: SubscriptionCookieManaging { self.eventMapping = eventMapping self.refreshTimeInterval = refreshTimeInterval - registerForSubscriptionAccountManagerEvents() - } - - private func registerForSubscriptionAccountManagerEvents() { NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignIn), name: .accountDidSignIn, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil) } + public func enableSettingSubscriptionCookie() { + isSettingSubscriptionCookieEnabled = true + } + + public func disableSettingSubscriptionCookie() async { + isSettingSubscriptionCookieEnabled = false + if let cookieStore = await currentCookieStore(), + let cookie = await cookieStore.fetchCurrentSubscriptionCookie() { + await cookieStore.deleteCookie(cookie) + } + } + @objc private func handleAccountDidSignIn() { Task { - guard let cookieStore = await currentCookieStore() else { return } + guard isSettingSubscriptionCookieEnabled, + let cookieStore = await currentCookieStore() + else { return } + guard let accessToken = subscriptionManager.accountManager.accessToken else { Logger.subscription.error("[SubscriptionCookieManager] Handle .accountDidSignIn - can't set the cookie, token is missing") eventMapping.fire(.errorHandlingAccountDidSignInTokenIsMissing) @@ -89,21 +104,24 @@ public final class SubscriptionCookieManager: SubscriptionCookieManaging { @objc private func handleAccountDidSignOut() { Task { - guard let cookieStore = await currentCookieStore() else { return } - guard let subscriptionCookie = await cookieStore.fetchCurrentSubscriptionCookie() else { - Logger.subscription.error("[SubscriptionCookieManager] Handle .accountDidSignOut - can't delete the cookie, cookie is missing") - eventMapping.fire(.errorHandlingAccountDidSignOutCookieIsMissing) - return - } + guard isSettingSubscriptionCookieEnabled, + let cookieStore = await currentCookieStore() + else { return } Logger.subscription.info("[SubscriptionCookieManager] Handle .accountDidSignOut - deleting cookie") - await cookieStore.deleteCookie(subscriptionCookie) - updateLastRefreshDateToNow() + + do { + try await cookieStore.setEmptySubscriptionCookie() + updateLastRefreshDateToNow() + } catch { + eventMapping.fire(.failedToSetSubscriptionCookie) + } } } public func refreshSubscriptionCookie() async { - guard shouldRefreshSubscriptionCookie() else { return } - guard let cookieStore = await currentCookieStore() else { return } + guard isSettingSubscriptionCookieEnabled, + shouldRefreshSubscriptionCookie(), + let cookieStore = await currentCookieStore() else { return } Logger.subscription.info("[SubscriptionCookieManager] Refresh subscription cookie") updateLastRefreshDateToNow() @@ -111,25 +129,22 @@ public final class SubscriptionCookieManager: SubscriptionCookieManaging { let accessToken: String? = subscriptionManager.accountManager.accessToken let subscriptionCookie = await cookieStore.fetchCurrentSubscriptionCookie() - if let accessToken { - if subscriptionCookie == nil || subscriptionCookie?.value != accessToken { - Logger.subscription.info("[SubscriptionCookieManager] Refresh: No cookie or one with different value") - do { + let noCookieOrWithUnexpectedValue = (accessToken ?? "") != subscriptionCookie?.value + + do { + if noCookieOrWithUnexpectedValue { + Logger.subscription.info("[SubscriptionCookieManager] Refresh: No cookie or one with unexpected value") + + if let accessToken { try await cookieStore.setSubscriptionCookie(for: accessToken) - eventMapping.fire(.subscriptionCookieRefreshedWithUpdate) - } catch { - eventMapping.fire(.failedToSetSubscriptionCookie) + eventMapping.fire(.subscriptionCookieRefreshedWithAccessToken) + } else { + try await cookieStore.setEmptySubscriptionCookie() + eventMapping.fire(.subscriptionCookieRefreshedWithEmptyValue) } - } else { - Logger.subscription.info("[SubscriptionCookieManager] Refresh: Cookie exists and is up to date") - return - } - } else { - if let subscriptionCookie { - Logger.subscription.info("[SubscriptionCookieManager] Refresh: No access token but old cookie exists, deleting it") - await cookieStore.deleteCookie(subscriptionCookie) - eventMapping.fire(.subscriptionCookieRefreshedWithDelete) } + } catch { + eventMapping.fire(.failedToSetSubscriptionCookie) } } @@ -161,6 +176,10 @@ private extension HTTPCookieStore { await allCookies().first { $0.domain == SubscriptionCookieManager.cookieDomain && $0.name == SubscriptionCookieManager.cookieName } } + func setEmptySubscriptionCookie() async throws { + try await setSubscriptionCookie(for: "") + } + func setSubscriptionCookie(for token: String) async throws { guard let cookie = HTTPCookie(properties: [ .domain: SubscriptionCookieManager.cookieDomain, diff --git a/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManagerEvent.swift b/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManagerEvent.swift index 86f604d3d..ffedd13c2 100644 --- a/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManagerEvent.swift +++ b/Sources/Subscription/SubscriptionCookie/SubscriptionCookieManagerEvent.swift @@ -20,10 +20,9 @@ import Foundation public enum SubscriptionCookieManagerEvent { case errorHandlingAccountDidSignInTokenIsMissing - case errorHandlingAccountDidSignOutCookieIsMissing - case subscriptionCookieRefreshedWithUpdate - case subscriptionCookieRefreshedWithDelete + case subscriptionCookieRefreshedWithAccessToken + case subscriptionCookieRefreshedWithEmptyValue case failedToSetSubscriptionCookie } diff --git a/Sources/SubscriptionTestingUtilities/SubscriptionCookie/SubscriptionCookieManagerMock.swift b/Sources/SubscriptionTestingUtilities/SubscriptionCookie/SubscriptionCookieManagerMock.swift index 56e970ebb..a9166689a 100644 --- a/Sources/SubscriptionTestingUtilities/SubscriptionCookie/SubscriptionCookieManagerMock.swift +++ b/Sources/SubscriptionTestingUtilities/SubscriptionCookie/SubscriptionCookieManagerMock.swift @@ -48,6 +48,8 @@ public final class SubscriptionCookieManagerMock: SubscriptionCookieManaging { } + public func enableSettingSubscriptionCookie() { } + public func disableSettingSubscriptionCookie() async { } public func refreshSubscriptionCookie() async { } public func resetLastRefreshDate() { } } diff --git a/Tests/SubscriptionTests/SubscriptionCookie/SubscriptionCookieManagerTests.swift b/Tests/SubscriptionTests/SubscriptionCookie/SubscriptionCookieManagerTests.swift index 04dfe7b8b..07fb12bd4 100644 --- a/Tests/SubscriptionTests/SubscriptionCookie/SubscriptionCookieManagerTests.swift +++ b/Tests/SubscriptionTests/SubscriptionCookie/SubscriptionCookieManagerTests.swift @@ -76,6 +76,7 @@ final class SubscriptionCookieManagerTests: XCTestCase { accountManager.accessToken = Constants.accessToken // When + subscriptionCookieManager.enableSettingSubscriptionCookie() NotificationCenter.default.post(name: .accountDidSignIn, object: self, userInfo: nil) try await Task.sleep(seconds: 0.1) @@ -88,11 +89,12 @@ final class SubscriptionCookieManagerTests: XCTestCase { await ensureSubscriptionCookieIsInTheCookieStore() // When + subscriptionCookieManager.enableSettingSubscriptionCookie() NotificationCenter.default.post(name: .accountDidSignOut, object: self, userInfo: nil) try await Task.sleep(seconds: 0.1) // Then - await checkSubscriptionCookieIsNotPresent() + await checkSubscriptionCookieIsHasEmptyValue() } func testRefreshWhenSignedInButCookieIsMissing() async throws { @@ -101,6 +103,7 @@ final class SubscriptionCookieManagerTests: XCTestCase { await ensureNoSubscriptionCookieInTheCookieStore() // When + subscriptionCookieManager.enableSettingSubscriptionCookie() await subscriptionCookieManager.refreshSubscriptionCookie() try await Task.sleep(seconds: 0.1) @@ -114,11 +117,12 @@ final class SubscriptionCookieManagerTests: XCTestCase { await ensureSubscriptionCookieIsInTheCookieStore() // When + subscriptionCookieManager.enableSettingSubscriptionCookie() await subscriptionCookieManager.refreshSubscriptionCookie() try await Task.sleep(seconds: 0.1) // Then - await checkSubscriptionCookieIsNotPresent() + await checkSubscriptionCookieIsHasEmptyValue() } func testRefreshNotTriggeredTwiceWithinSetRefreshInterval() async throws { @@ -127,6 +131,7 @@ final class SubscriptionCookieManagerTests: XCTestCase { let secondRefreshDate: Date? // When + subscriptionCookieManager.enableSettingSubscriptionCookie() await subscriptionCookieManager.refreshSubscriptionCookie() firstRefreshDate = subscriptionCookieManager.lastRefreshDate @@ -145,6 +150,7 @@ final class SubscriptionCookieManagerTests: XCTestCase { let secondRefreshDate: Date? // When + subscriptionCookieManager.enableSettingSubscriptionCookie() await subscriptionCookieManager.refreshSubscriptionCookie() firstRefreshDate = subscriptionCookieManager.lastRefreshDate @@ -186,9 +192,12 @@ final class SubscriptionCookieManagerTests: XCTestCase { XCTAssertEqual(subscriptionCookie.value, Constants.accessToken) } - private func checkSubscriptionCookieIsNotPresent() async { - let cookie = await cookieStore.fetchSubscriptionCookie() - XCTAssertNil(cookie) + private func checkSubscriptionCookieIsHasEmptyValue() async { + guard let subscriptionCookie = await cookieStore.fetchSubscriptionCookie() else { + XCTFail("No subscription cookie in the store") + return + } + XCTAssertEqual(subscriptionCookie.value, "") } } @@ -213,6 +222,7 @@ class MockHTTPCookieStore: HTTPCookieStore { } func setCookie(_ cookie: HTTPCookie) async { + cookies.removeAll { $0.domain == cookie.domain } cookies.append(cookie) }