Skip to content

Commit

Permalink
Update to subscription cookie (#1053)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
-->

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: duckduckgo/iOS#3512
macOS PR: duckduckgo/macos-browser#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

<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**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)
  • Loading branch information
miasma13 authored Nov 1, 2024
1 parent d39d04c commit 7b78b46
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public enum PrivacyProSubfeature: String, Equatable, PrivacySubfeature {
case isLaunchedOverride
case isLaunchedOverrideStripe
case useUnifiedFeedback
case setAccessTokenCookieForSubscriptionDomains
}

public enum SslCertificatesSubfeature: String, PrivacySubfeature {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import os.log

public protocol SubscriptionCookieManaging {
init(subscriptionManager: SubscriptionManager, currentCookieStore: @MainActor @escaping () -> HTTPCookieStore?, eventMapping: EventMapping<SubscriptionCookieManagerEvent>)
func enableSettingSubscriptionCookie()
func disableSettingSubscriptionCookie() async

func refreshSubscriptionCookie() async
func resetLastRefreshDate()

Expand All @@ -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)
Expand All @@ -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?,
Expand All @@ -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)
Expand All @@ -89,47 +104,47 @@ 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()

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)
}
}

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ import Foundation

public enum SubscriptionCookieManagerEvent {
case errorHandlingAccountDidSignInTokenIsMissing
case errorHandlingAccountDidSignOutCookieIsMissing

case subscriptionCookieRefreshedWithUpdate
case subscriptionCookieRefreshedWithDelete
case subscriptionCookieRefreshedWithAccessToken
case subscriptionCookieRefreshedWithEmptyValue

case failedToSetSubscriptionCookie
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public final class SubscriptionCookieManagerMock: SubscriptionCookieManaging {

}

public func enableSettingSubscriptionCookie() { }
public func disableSettingSubscriptionCookie() async { }
public func refreshSubscriptionCookie() async { }
public func resetLastRefreshDate() { }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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 {
Expand All @@ -101,6 +103,7 @@ final class SubscriptionCookieManagerTests: XCTestCase {
await ensureNoSubscriptionCookieInTheCookieStore()

// When
subscriptionCookieManager.enableSettingSubscriptionCookie()
await subscriptionCookieManager.refreshSubscriptionCookie()
try await Task.sleep(seconds: 0.1)

Expand All @@ -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 {
Expand All @@ -127,6 +131,7 @@ final class SubscriptionCookieManagerTests: XCTestCase {
let secondRefreshDate: Date?

// When
subscriptionCookieManager.enableSettingSubscriptionCookie()
await subscriptionCookieManager.refreshSubscriptionCookie()
firstRefreshDate = subscriptionCookieManager.lastRefreshDate

Expand All @@ -145,6 +150,7 @@ final class SubscriptionCookieManagerTests: XCTestCase {
let secondRefreshDate: Date?

// When
subscriptionCookieManager.enableSettingSubscriptionCookie()
await subscriptionCookieManager.refreshSubscriptionCookie()
firstRefreshDate = subscriptionCookieManager.lastRefreshDate

Expand Down Expand Up @@ -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, "")
}

}
Expand All @@ -213,6 +222,7 @@ class MockHTTPCookieStore: HTTPCookieStore {
}

func setCookie(_ cookie: HTTPCookie) async {
cookies.removeAll { $0.domain == cookie.domain }
cookies.append(cookie)
}

Expand Down

0 comments on commit 7b78b46

Please sign in to comment.