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.

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

NetP x Subscription Clean-up #709

Merged
merged 13 commits into from
Mar 14, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public protocol NetworkProtectionCodeRedeeming {
func redeem(_ code: String) async throws

/// Exchanges an access token for an auth token, and stores the resulting auth token
@available(*, deprecated, message: "[NetP Subscription] Use subscription access token instead")
func exchange(accessToken: String) async throws

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import Foundation
import Common

public protocol NetworkProtectionTokenStore {

/// Store an auth token.
///
@available(*, deprecated, message: "[NetP Subscription] Do not manually manage auth token")
quanganhdo marked this conversation as resolved.
Show resolved Hide resolved
func store(_ token: String) throws

/// Obtain the current auth token.
Expand All @@ -31,17 +31,8 @@ public protocol NetworkProtectionTokenStore {

/// Delete the stored auth token.
///
@available(*, deprecated, message: "[NetP Subscription] Do not manually manage auth token")
func deleteToken() throws

/// Convert DDG-access-token to NetP-auth-token
///
/// todo - https://app.asana.com/0/0/1206541966681608/f
static func makeToken(from subscriptionAccessToken: String) -> String

/// Check if a given token is derived from DDG-access-token
///
/// todo - https://app.asana.com/0/0/1206541966681608/f
static func isSubscriptionAccessToken(_ token: String) -> Bool
}

/// Store an auth token for NetworkProtection on behalf of the user. This key is then used to authenticate requests for registration and server fetches from the Network Protection backend servers.
Expand All @@ -50,6 +41,9 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
private let keychainStore: NetworkProtectionKeychainStore
private let errorEvents: EventMapping<NetworkProtectionError>?
private let isSubscriptionEnabled: Bool
private let accessTokenProvider: () -> String?

private static var authTokenPrefix: String { "ddg:" }

public struct Defaults {
static let tokenStoreEntryLabel = "DuckDuckGo Network Protection Auth Token"
Expand All @@ -60,12 +54,14 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
public init(keychainType: KeychainType,
serviceName: String = Defaults.tokenStoreService,
errorEvents: EventMapping<NetworkProtectionError>?,
isSubscriptionEnabled: Bool) {
isSubscriptionEnabled: Bool,
accessTokenProvider: @escaping () -> String?) {
keychainStore = NetworkProtectionKeychainStore(label: Defaults.tokenStoreEntryLabel,
serviceName: serviceName,
keychainType: keychainType)
self.errorEvents = errorEvents
self.isSubscriptionEnabled = isSubscriptionEnabled
self.accessTokenProvider = accessTokenProvider
}

public func store(_ token: String) throws {
Expand All @@ -78,7 +74,15 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
}
}

public func makeToken(from subscriptionAccessToken: String) -> String {
Self.authTokenPrefix + subscriptionAccessToken
}

public func fetchToken() throws -> String? {
if isSubscriptionEnabled, let authToken = accessTokenProvider() {
return makeToken(from: authToken)
}

do {
return try keychainStore.readData(named: Defaults.tokenStoreName).flatMap {
String(data: $0, encoding: .utf8)
Expand All @@ -91,12 +95,6 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt

public func deleteToken() throws {
do {
// Skip deleting DDG-access-token as it's used for entitlement validity check
// todo - https://app.asana.com/0/0/1206541966681608/f
if let token = try? fetchToken(), Self.isSubscriptionAccessToken(token) {
return
}

try keychainStore.deleteAll()
} catch {
handle(error)
Expand All @@ -118,13 +116,4 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
}

extension NetworkProtectionTokenStore {
private static var authTokenPrefix: String { "ddg:" }

public static func makeToken(from subscriptionAccessToken: String) -> String {
"\(authTokenPrefix)\(subscriptionAccessToken)"
}

public static func isSubscriptionAccessToken(_ token: String) -> Bool {
token.hasPrefix(authTokenPrefix)
}
}
2 changes: 1 addition & 1 deletion Sources/NetworkProtection/PacketTunnelProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
)
} catch {
if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error {
await handleInvalidEntitlement(attemptsShutdown: false)
await handleInvalidEntitlement(attemptsShutdown: true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I thought there's no need to attempt a shutdown; but this is necessary when the app tries to automatically reconnect for whatever reason.

throw TunnelError.vpnAccessRevoked
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,9 @@ public final class MockNetworkProtectionTokenStorage: NetworkProtectionTokenStor
public func deleteToken() throws {
didCallDeleteToken = true
}

public func fetchSubscriptionToken() throws -> String? {
try fetchToken()
}

}
4 changes: 2 additions & 2 deletions Sources/Subscription/AccountManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ public class AccountManager: AccountManaging {
case noCachedData
}

public func hasEntitlement(for entitlement: Entitlement.ProductName) async -> Result<Bool, Error> {
switch await fetchEntitlements() {
public func hasEntitlement(for entitlement: Entitlement.ProductName, cachePolicy: CachePolicy = .returnCacheDataElseLoad) async -> Result<Bool, Error> {
switch await fetchEntitlements(policy: cachePolicy) {
case .success(let entitlements):
return .success(entitlements.compactMap { $0.product }.contains(entitlement))
case .failure(let error):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ final class NetworkProtectionTokenStoreMock: NetworkProtectionTokenStore {
}

func deleteToken() {
if let token = fetchToken(), Self.isSubscriptionAccessToken(token) {
return
}
self.token = nil
}

func fetchSubscriptionToken() throws -> String? {
"ddg:accessToken"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -222,22 +222,6 @@ final class NetworkProtectionDeviceManagerTests: XCTestCase {
XCTAssertEqual(try? serverListStore.storedNetworkProtectionServerList(), [registeredServer])
XCTAssertNotNil(networkClient.spyRegister)
}

func testStoringAccessToken() {
tokenStore.store(NetworkProtectionTokenStoreMock.makeToken(from: "access-token"))
XCTAssertEqual(tokenStore.fetchToken(), "ddg:access-token")

tokenStore.deleteToken()
XCTAssertEqual(tokenStore.fetchToken(), "ddg:access-token")
}

func testStoringAuthToken() {
tokenStore.store("auth-token")
XCTAssertEqual(tokenStore.fetchToken(), "auth-token")

tokenStore.deleteToken()
XCTAssertNil(tokenStore.fetchToken())
}
}

extension NetworkProtectionDeviceManager {
Expand Down
Loading