From ac4b3a0b60e7d7177608cea36747dde1c712b717 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 21 Feb 2024 13:19:05 +0100 Subject: [PATCH 01/25] Add separate SubscriptionTokenStorage to store token to be shared --- Sources/Subscription/AccountManager.swift | 5 + .../SubscriptionTokenKeychainStorage.swift | 130 ++++++++++++++++++ .../SubscriptionTokenStorage.swift | 25 ++++ 3 files changed, 160 insertions(+) create mode 100644 Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift create mode 100644 Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 395c0e29f..7ea540a32 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -37,6 +37,8 @@ public protocol AccountManaging { public class AccountManager: AccountManaging { private let storage: AccountStorage + private let tokenStorage: SubscriptionTokenStorage + public weak var delegate: AccountManagerKeychainAccessDelegate? public var isUserAuthenticated: Bool { @@ -45,6 +47,7 @@ public class AccountManager: AccountManaging { public init(storage: AccountStorage = AccountKeychainStorage()) { self.storage = storage + self.tokenStorage = SubscriptionTokenKeychainStorage() } public var authToken: String? { @@ -122,6 +125,7 @@ public class AccountManager: AccountManaging { do { try storage.store(accessToken: token) + try tokenStorage.store(accessToken: token) } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .storeAccessToken, error: error) @@ -157,6 +161,7 @@ public class AccountManager: AccountManaging { do { try storage.clearAuthenticationState() + try tokenStorage.removeAccessToken() } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .clearAuthenticationData, error: error) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift new file mode 100644 index 000000000..e229c158c --- /dev/null +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -0,0 +1,130 @@ +// +// SubscriptionTokenKeychainStorage.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { + + public init() {} + + public func getAccessToken() throws -> String? { + try Self.getString(forField: .accessToken) + } + + public func store(accessToken: String) throws { + try Self.set(string: accessToken, forField: .accessToken) + } + + public func removeAccessToken() throws { + try Self.deleteItem(forField: .accessToken) + } + +} + +private extension SubscriptionTokenKeychainStorage { + + /* + Uses just kSecAttrService as the primary key, since we don't want to store + multiple accounts/tokens at the same time + */ + enum AccountKeychainField: String, CaseIterable { + case authToken = "account.authToken" + case accessToken = "account.accessToken" + case email = "account.email" + case externalID = "account.external_id" + + var keyValue: String { + (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + "." + rawValue + } + } + + static func getString(forField field: AccountKeychainField) throws -> String? { + guard let data = try retrieveData(forField: field) else { + return nil + } + + if let decodedString = String(data: data, encoding: String.Encoding.utf8) { + return decodedString + } else { + throw AccountKeychainAccessError.failedToDecodeKeychainDataAsString + } + } + + static func retrieveData(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws -> Data? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecMatchLimit as String: kSecMatchLimitOne, + kSecAttrService as String: field.keyValue, + kSecReturnData as String: true, + kSecUseDataProtectionKeychain as String: useDataProtectionKeychain + ] + + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + + if status == errSecSuccess { + if let existingItem = item as? Data { + return existingItem + } else { + throw AccountKeychainAccessError.failedToDecodeKeychainValueAsData + } + } else if status == errSecItemNotFound { + return nil + } else { + throw AccountKeychainAccessError.keychainLookupFailure(status) + } + } + + static func set(string: String, forField field: AccountKeychainField) throws { + guard let stringData = string.data(using: .utf8) else { + return + } + + try deleteItem(forField: field) + try store(data: stringData, forField: field) + } + + static func store(data: Data, forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { + let query = [ + kSecClass: kSecClassGenericPassword, + kSecAttrSynchronizable: false, + kSecAttrService: field.keyValue, + kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlock, + kSecValueData: data, + kSecUseDataProtectionKeychain: useDataProtectionKeychain] as [String: Any] + + let status = SecItemAdd(query as CFDictionary, nil) + + if status != errSecSuccess { + throw AccountKeychainAccessError.keychainSaveFailure(status) + } + } + + static func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: field.keyValue, + kSecUseDataProtectionKeychain as String: useDataProtectionKeychain] + + let status = SecItemDelete(query as CFDictionary) + + if status != errSecSuccess && status != errSecItemNotFound { + throw AccountKeychainAccessError.keychainDeleteFailure(status) + } + } +} diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift new file mode 100644 index 000000000..58c7e74b6 --- /dev/null +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift @@ -0,0 +1,25 @@ +// +// SubscriptionTokenStorage.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +public protocol SubscriptionTokenStorage: AnyObject { + func getAccessToken() throws -> String? + func store(accessToken: String) throws + func removeAccessToken() throws +} From 50a634438bddee27cb588b2a3e02475e09b06a83 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 21 Feb 2024 14:01:39 +0100 Subject: [PATCH 02/25] Store and read access token from its separate storage --- Sources/Subscription/AccountManager.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 7ea540a32..a29e7390e 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -66,7 +66,8 @@ public class AccountManager: AccountManaging { public var accessToken: String? { do { - return try storage.getAccessToken() +// return try storage.getAccessToken() + return try tokenStorage.getAccessToken() } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .getAccessToken, error: error) @@ -124,7 +125,7 @@ public class AccountManager: AccountManaging { os_log(.info, log: .subscription, "[AccountManager] storeAccount") do { - try storage.store(accessToken: token) +// try storage.store(accessToken: token) try tokenStorage.store(accessToken: token) } catch { if let error = error as? AccountKeychainAccessError { From d4dfb757f96b7053778c3c3d23c2fc95b151eee4 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 21 Feb 2024 15:00:01 +0100 Subject: [PATCH 03/25] Refactor how queries are built --- .../SubscriptionTokenKeychainStorage.swift | 105 +++++++++++++----- 1 file changed, 75 insertions(+), 30 deletions(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index e229c158c..b77884e72 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -20,18 +20,27 @@ import Foundation public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { - public init() {} + private let label: String + private let keychainType: KeychainType + + public init() { + self.label = "DuckDuckGo Subscription Access Token Test 1" + +// self.keychainType = .dataProtection(.named("asd")) + self.keychainType = .fileBased + + } public func getAccessToken() throws -> String? { - try Self.getString(forField: .accessToken) + try getString(forField: .accessToken) } public func store(accessToken: String) throws { - try Self.set(string: accessToken, forField: .accessToken) + try set(string: accessToken, forField: .accessToken) } public func removeAccessToken() throws { - try Self.deleteItem(forField: .accessToken) + try deleteItem(forField: .accessToken) } } @@ -43,17 +52,14 @@ private extension SubscriptionTokenKeychainStorage { multiple accounts/tokens at the same time */ enum AccountKeychainField: String, CaseIterable { - case authToken = "account.authToken" case accessToken = "account.accessToken" - case email = "account.email" - case externalID = "account.external_id" var keyValue: String { (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + "." + rawValue } } - static func getString(forField field: AccountKeychainField) throws -> String? { + func getString(forField field: AccountKeychainField) throws -> String? { guard let data = try retrieveData(forField: field) else { return nil } @@ -65,14 +71,11 @@ private extension SubscriptionTokenKeychainStorage { } } - static func retrieveData(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws -> Data? { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecAttrService as String: field.keyValue, - kSecReturnData as String: true, - kSecUseDataProtectionKeychain as String: useDataProtectionKeychain - ] + func retrieveData(forField field: AccountKeychainField) throws -> Data? { + var query = defaultAttributes() + query[kSecAttrService] = field.keyValue + query[kSecMatchLimit] = kSecMatchLimitOne + query[kSecReturnData] = true var item: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &item) @@ -90,7 +93,7 @@ private extension SubscriptionTokenKeychainStorage { } } - static func set(string: String, forField field: AccountKeychainField) throws { + func set(string: String, forField field: AccountKeychainField) throws { guard let stringData = string.data(using: .utf8) else { return } @@ -99,27 +102,23 @@ private extension SubscriptionTokenKeychainStorage { try store(data: stringData, forField: field) } - static func store(data: Data, forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { - let query = [ - kSecClass: kSecClassGenericPassword, - kSecAttrSynchronizable: false, - kSecAttrService: field.keyValue, - kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlock, - kSecValueData: data, - kSecUseDataProtectionKeychain: useDataProtectionKeychain] as [String: Any] + func store(data: Data, forField field: AccountKeychainField) throws { + var query = defaultAttributes() + query[kSecAttrService] = field.keyValue + query[kSecAttrAccessible] = kSecAttrAccessibleAfterFirstUnlock + query[kSecValueData] = data let status = SecItemAdd(query as CFDictionary, nil) if status != errSecSuccess { throw AccountKeychainAccessError.keychainSaveFailure(status) } + + } - static func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: field.keyValue, - kSecUseDataProtectionKeychain as String: useDataProtectionKeychain] + func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { + var query = defaultAttributes() let status = SecItemDelete(query as CFDictionary) @@ -127,4 +126,50 @@ private extension SubscriptionTokenKeychainStorage { throw AccountKeychainAccessError.keychainDeleteFailure(status) } } + + private func defaultAttributes() -> [CFString: Any] { + var attributes: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrSynchronizable: false, + kSecAttrLabel: label, + ] + + attributes.merge(keychainType.queryAttributes()) { $1 } + + return attributes + } +} + +public enum KeychainType { + case dataProtection(_ accessGroup: AccessGroup) + + /// Uses the system keychain. + /// + case system + + case fileBased + + public enum AccessGroup { + case unspecified + case named(_ name: String) + } + + func queryAttributes() -> [CFString: Any] { + switch self { + case .dataProtection(let accessGroup): + switch accessGroup { + case .unspecified: + return [kSecUseDataProtectionKeychain: true] + case .named(let accessGroup): + return [ + kSecUseDataProtectionKeychain: true, + kSecAttrAccessGroup: accessGroup + ] + } + case .system: + return [kSecUseDataProtectionKeychain: false] + case .fileBased: + return [kSecUseDataProtectionKeychain: false] + } + } } From 257f8d1d637cb822795ca76179cf2dc9f1234df8 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 21 Feb 2024 15:01:59 +0100 Subject: [PATCH 04/25] Clean up --- .../SubscriptionTokenKeychainStorage.swift | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index b77884e72..6ad82c1f0 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -24,11 +24,8 @@ public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { private let keychainType: KeychainType public init() { - self.label = "DuckDuckGo Subscription Access Token Test 1" - -// self.keychainType = .dataProtection(.named("asd")) + self.label = "DuckDuckGo Subscription Access Token" self.keychainType = .fileBased - } public func getAccessToken() throws -> String? { @@ -113,12 +110,10 @@ private extension SubscriptionTokenKeychainStorage { if status != errSecSuccess { throw AccountKeychainAccessError.keychainSaveFailure(status) } - - } func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { - var query = defaultAttributes() + let query = defaultAttributes() let status = SecItemDelete(query as CFDictionary) From c0c2963012b19481243139561e48b0592b313058 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Wed, 21 Feb 2024 18:56:18 +0100 Subject: [PATCH 05/25] Update to SubscriptionTokenKeychainStorage to support testString --- .../SubscriptionTokenKeychainStorage.swift | 50 ++++++++++++++++--- .../SubscriptionTokenStorage.swift | 4 ++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index 6ad82c1f0..4f48a4f27 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -20,11 +20,9 @@ import Foundation public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { - private let label: String private let keychainType: KeychainType public init() { - self.label = "DuckDuckGo Subscription Access Token" self.keychainType = .fileBased } @@ -40,6 +38,24 @@ public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { try deleteItem(forField: .accessToken) } + public func getTestString() throws -> String? { + try getString(forField: .testString) + } + + public func updateTestString() throws { + let dateFormatter = DateFormatter() + dateFormatter.dateStyle = .medium + dateFormatter.timeStyle = .medium + let testString = dateFormatter.string(from: Date()) + + print(testString) + + try set(string: testString, forField: .testString) + } + + public func removeTestString() throws { + try deleteItem(forField: .testString) + } } private extension SubscriptionTokenKeychainStorage { @@ -49,7 +65,8 @@ private extension SubscriptionTokenKeychainStorage { multiple accounts/tokens at the same time */ enum AccountKeychainField: String, CaseIterable { - case accessToken = "account.accessToken" + case accessToken = "subscription.account.accessToken" + case testString = "subscription.account.testString" var keyValue: String { (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + "." + rawValue @@ -95,7 +112,6 @@ private extension SubscriptionTokenKeychainStorage { return } - try deleteItem(forField: field) try store(data: stringData, forField: field) } @@ -107,11 +123,32 @@ private extension SubscriptionTokenKeychainStorage { let status = SecItemAdd(query as CFDictionary, nil) - if status != errSecSuccess { + switch status { + case errSecSuccess: + return + case errSecDuplicateItem: + let updateStatus = updateData(data, forField: field) + + if updateStatus != errSecSuccess { + throw AccountKeychainAccessError.keychainSaveFailure(status) + } + default: throw AccountKeychainAccessError.keychainSaveFailure(status) } } + private func updateData(_ data: Data, forField field: AccountKeychainField) -> OSStatus { + var query = defaultAttributes() + query[kSecAttrService] = field.keyValue + + let newAttributes = [ + kSecValueData: data, + kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlock + ] as [CFString: Any] + + return SecItemUpdate(query as CFDictionary, newAttributes as CFDictionary) + } + func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws { let query = defaultAttributes() @@ -125,8 +162,7 @@ private extension SubscriptionTokenKeychainStorage { private func defaultAttributes() -> [CFString: Any] { var attributes: [CFString: Any] = [ kSecClass: kSecClassGenericPassword, - kSecAttrSynchronizable: false, - kSecAttrLabel: label, + kSecAttrSynchronizable: false ] attributes.merge(keychainType.queryAttributes()) { $1 } diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift index 58c7e74b6..5aed907b6 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift @@ -22,4 +22,8 @@ public protocol SubscriptionTokenStorage: AnyObject { func getAccessToken() throws -> String? func store(accessToken: String) throws func removeAccessToken() throws + + func getTestString() throws -> String? + func updateTestString() throws + func removeTestString() throws } From fa7fab43a88d619f89730f9bd5ddc6e05f7a17b8 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 00:30:19 -0500 Subject: [PATCH 06/25] Add notification for messaging changes --- Sources/NetworkProtection/PacketTunnelProvider.swift | 5 ++++- .../Extensions/UserDefaults+showEntitlementMessaging.swift | 4 ++-- Sources/NetworkProtection/Settings/VPNSettings.swift | 7 +++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 5009022d6..03bc6917c 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -777,6 +777,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { os_log("🔵 Expired subscription", log: .networkProtection, type: .error) settings.enableEntitlementMessaging() + + /// We add a delay here so the notification has a chance to show up + try? await Task.sleep(interval: .seconds(5)) + throw TunnelError.vpnAccessRevoked } @@ -909,7 +913,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { completionHandler?(nil) } case .setShowEntitlementNotification: - // todo - https://app.asana.com/0/0/1206409081785857/f if settings.showEntitlementNotification { notificationsPresenter.showEntitlementNotification { [weak self] error in guard error == nil else { return } diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index 9fa1029ed..42169bc6c 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -22,7 +22,7 @@ import Common extension UserDefaults { private var showEntitlementAlertKey: String { - "networkProtectionShowEntitlementAlertRawValue" + "showEntitlementAlert" } @objc @@ -48,7 +48,7 @@ extension UserDefaults { } private var showEntitlementNotificationKey: String { - "networkProtectionShowEntitlementNotificationRawValue" + "showEntitlementNotification" } @objc diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 0a14b9be1..5dda31c95 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -18,6 +18,7 @@ import Combine import Foundation +import Common /// Persists and publishes changes to tunnel settings. /// @@ -544,9 +545,15 @@ public final class VPNSettings { public func enableEntitlementMessaging() { apply(change: .setShowEntitlementAlert(true)) apply(change: .setShowEntitlementNotification(true)) + os_log("🔵 Posting vpnEntitlementMessagingDidChange", log: .networkProtection) + NotificationCenter.default.post(name: .vpnEntitlementMessagingDidChange, object: nil) } public func resetEntitlementMessaging() { defaults.resetEntitlementMessaging() } } + +public extension Notification.Name { + static let vpnEntitlementMessagingDidChange = Notification.Name("com.duckduckgo.network-protection.entitlement-messaging-changed") +} From ffa482c1939aac1bb175e0d1cfb005af87d56dc1 Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:26:54 +0100 Subject: [PATCH 07/25] wip --- Sources/Subscription/AccountManager.swift | 22 ++++++++++++------- .../SubscriptionTokenKeychainStorage.swift | 9 ++++++-- .../AppStoreAccountManagementFlow.swift | 9 ++++---- .../Flows/AppStore/AppStorePurchaseFlow.swift | 12 +++++----- .../Flows/AppStore/AppStoreRestoreFlow.swift | 4 ++-- .../Flows/Stripe/StripePurchaseFlow.swift | 10 ++++----- 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index a29e7390e..7b46bbbe8 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -37,7 +37,7 @@ public protocol AccountManaging { public class AccountManager: AccountManaging { private let storage: AccountStorage - private let tokenStorage: SubscriptionTokenStorage + private let accessTokenStorage: SubscriptionTokenStorage public weak var delegate: AccountManagerKeychainAccessDelegate? @@ -45,9 +45,15 @@ public class AccountManager: AccountManaging { return accessToken != nil } - public init(storage: AccountStorage = AccountKeychainStorage()) { + public convenience init(appGroup: String) { + let accessTokenStorage = SubscriptionTokenKeychainStorage(keychainType: .dataProtection(.named(appGroup))) + self.init(accessTokenStorage: accessTokenStorage) + } + + public init(storage: AccountStorage = AccountKeychainStorage(), + accessTokenStorage: SubscriptionTokenStorage) { self.storage = storage - self.tokenStorage = SubscriptionTokenKeychainStorage() + self.accessTokenStorage = accessTokenStorage } public var authToken: String? { @@ -67,7 +73,7 @@ public class AccountManager: AccountManaging { public var accessToken: String? { do { // return try storage.getAccessToken() - return try tokenStorage.getAccessToken() + return try accessTokenStorage.getAccessToken() } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .getAccessToken, error: error) @@ -126,7 +132,7 @@ public class AccountManager: AccountManaging { do { // try storage.store(accessToken: token) - try tokenStorage.store(accessToken: token) + try accessTokenStorage.store(accessToken: token) } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .storeAccessToken, error: error) @@ -162,7 +168,7 @@ public class AccountManager: AccountManaging { do { try storage.clearAuthenticationState() - try tokenStorage.removeAccessToken() + try accessTokenStorage.removeAccessToken() } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .clearAuthenticationData, error: error) @@ -244,12 +250,12 @@ public class AccountManager: AccountManaging { } @discardableResult - public static func checkForEntitlements(wait waitTime: Double, retry retryCount: Int) async -> Bool { + public static func checkForEntitlements(subscriptionAppGroup: String, wait waitTime: Double, retry retryCount: Int) async -> Bool { var count = 0 var hasEntitlements = false repeat { - switch await AccountManager().fetchEntitlements() { + switch await AccountManager(appGroup: subscriptionAppGroup).fetchEntitlements() { case .success(let entitlements): hasEntitlements = !entitlements.isEmpty case .failure: diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index 4f48a4f27..53ec1b8b0 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -22,8 +22,8 @@ public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { private let keychainType: KeychainType - public init() { - self.keychainType = .fileBased + public init(keychainType: KeychainType = .dataProtection(.unspecified)) { + self.keychainType = keychainType } public func getAccessToken() throws -> String? { @@ -85,11 +85,16 @@ private extension SubscriptionTokenKeychainStorage { } } + struct Defaults { + static let tokenStoreEntryLabel = "DuckDuckGo Privacy Pro Auth Token" + } + func retrieveData(forField field: AccountKeychainField) throws -> Data? { var query = defaultAttributes() query[kSecAttrService] = field.keyValue query[kSecMatchLimit] = kSecMatchLimitOne query[kSecReturnData] = true + //query[kSecAttrLabel] = Defaults.tokenStoreEntryLabel var item: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &item) diff --git a/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift index 7b502d214..08ccdb62a 100644 --- a/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift @@ -29,10 +29,11 @@ public final class AppStoreAccountManagementFlow { } @discardableResult - public static func refreshAuthTokenIfNeeded() async -> Result { + public static func refreshAuthTokenIfNeeded(subscriptionAppGroup: String) async -> Result { os_log(.info, log: .subscription, "[AppStoreAccountManagementFlow] refreshAuthTokenIfNeeded") + let accountManager = AccountManager(appGroup: subscriptionAppGroup) - var authToken = AccountManager().authToken ?? "" + var authToken = accountManager.authToken ?? "" // Check if auth token if still valid if case let .failure(validateTokenError) = await AuthService.validateToken(accessToken: authToken) { @@ -43,9 +44,9 @@ public final class AppStoreAccountManagementFlow { switch await AuthService.storeLogin(signature: lastTransactionJWSRepresentation) { case .success(let response): - if response.externalID == AccountManager().externalID { + if response.externalID == accountManager.externalID { authToken = response.authToken - AccountManager().storeAuthToken(token: authToken) + accountManager.storeAuthToken(token: authToken) } case .failure(let storeLoginError): os_log(.error, log: .subscription, "[AppStoreAccountManagementFlow] storeLogin error: %{public}s", String(reflecting: storeLoginError)) diff --git a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift index a223ef071..d39d94e6d 100644 --- a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift @@ -57,14 +57,14 @@ public final class AppStorePurchaseFlow { } // swiftlint:disable cyclomatic_complexity - public static func purchaseSubscription(with subscriptionIdentifier: String, emailAccessToken: String?) async -> Result { + public static func purchaseSubscription(with subscriptionIdentifier: String, emailAccessToken: String?, subscriptionAppGroup: String) async -> Result { os_log(.info, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription") - let accountManager = AccountManager() + let accountManager = AccountManager(appGroup: subscriptionAppGroup) let externalID: String // Check for past transactions most recent - switch await AppStoreRestoreFlow.restoreAccountFromPastPurchase() { + switch await AppStoreRestoreFlow.restoreAccountFromPastPurchase(appGroup: subscriptionAppGroup) { case .success: os_log(.info, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription -> restoreAccountFromPastPurchase: activeSubscriptionAlreadyPresent") return .failure(.activeSubscriptionAlreadyPresent) @@ -99,7 +99,7 @@ public final class AppStorePurchaseFlow { return .success(()) case .failure(let error): os_log(.error, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription error: %{public}s", String(reflecting: error)) - AccountManager().signOut() + AccountManager(appGroup: subscriptionAppGroup).signOut() switch error { case .purchaseCancelledByUser: return .failure(.cancelledByUser) @@ -111,10 +111,10 @@ public final class AppStorePurchaseFlow { // swiftlint:enable cyclomatic_complexity @discardableResult - public static func completeSubscriptionPurchase() async -> Result { + public static func completeSubscriptionPurchase(subscriptionAppGroup: String) async -> Result { os_log(.info, log: .subscription, "[AppStorePurchaseFlow] completeSubscriptionPurchase") - let result = await AccountManager.checkForEntitlements(wait: 2.0, retry: 20) + let result = await AccountManager.checkForEntitlements(subscriptionAppGroup: subscriptionAppGroup, wait: 2.0, retry: 20) return result ? .success(PurchaseUpdate(type: "completed")) : .failure(.missingEntitlements) } diff --git a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift index c0ee4df79..c816f917c 100644 --- a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift @@ -34,7 +34,7 @@ public final class AppStoreRestoreFlow { case subscriptionExpired(accountDetails: RestoredAccountDetails) } - public static func restoreAccountFromPastPurchase() async -> Result { + public static func restoreAccountFromPastPurchase(appGroup: String) async -> Result { os_log(.info, log: .subscription, "[AppStoreRestoreFlow] restoreAccountFromPastPurchase") guard let lastTransactionJWSRepresentation = await PurchaseManager.mostRecentTransaction() else { @@ -42,7 +42,7 @@ public final class AppStoreRestoreFlow { return .failure(.missingAccountOrTransactions) } - let accountManager = AccountManager() + let accountManager = AccountManager(appGroup: appGroup) // Do the store login to get short-lived token let authToken: String diff --git a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift index 9bd19a034..6257ce089 100644 --- a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift +++ b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift @@ -61,7 +61,7 @@ public final class StripePurchaseFlow { features: features)) } - public static func prepareSubscriptionPurchase(emailAccessToken: String?) async -> Result { + public static func prepareSubscriptionPurchase(emailAccessToken: String?, subscriptionAppGroup: String) async -> Result { os_log(.info, log: .subscription, "[StripePurchaseFlow] prepareSubscriptionPurchase") var authToken: String = "" @@ -69,7 +69,7 @@ public final class StripePurchaseFlow { switch await AuthService.createAccount(emailAccessToken: emailAccessToken) { case .success(let response): authToken = response.authToken - AccountManager().storeAuthToken(token: authToken) + AccountManager(appGroup: subscriptionAppGroup).storeAuthToken(token: authToken) case .failure: os_log(.error, log: .subscription, "[StripePurchaseFlow] Error: accountCreationFailed") return .failure(.accountCreationFailed) @@ -78,10 +78,10 @@ public final class StripePurchaseFlow { return .success(PurchaseUpdate(type: "redirect", token: authToken)) } - public static func completeSubscriptionPurchase() async { + public static func completeSubscriptionPurchase(subscriptionAppGroup: String) async { os_log(.info, log: .subscription, "[StripePurchaseFlow] completeSubscriptionPurchase") - let accountManager = AccountManager() + let accountManager = AccountManager(appGroup: subscriptionAppGroup) if let authToken = accountManager.authToken { if case let .success(accessToken) = await accountManager.exchangeAuthTokenToAccessToken(authToken), @@ -91,6 +91,6 @@ public final class StripePurchaseFlow { } } - await AccountManager.checkForEntitlements(wait: 2.0, retry: 5) + await AccountManager.checkForEntitlements(subscriptionAppGroup: subscriptionAppGroup, wait: 2.0, retry: 5) } } From 5d056b559b5b0840b6fb05b3e274384cee4600d3 Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Wed, 28 Feb 2024 20:57:35 +0100 Subject: [PATCH 08/25] Remove test string code --- .../SubscriptionTokenKeychainStorage.swift | 19 ------------------- .../SubscriptionTokenStorage.swift | 4 ---- 2 files changed, 23 deletions(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index 53ec1b8b0..7e110d3a6 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -37,25 +37,6 @@ public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage { public func removeAccessToken() throws { try deleteItem(forField: .accessToken) } - - public func getTestString() throws -> String? { - try getString(forField: .testString) - } - - public func updateTestString() throws { - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .medium - dateFormatter.timeStyle = .medium - let testString = dateFormatter.string(from: Date()) - - print(testString) - - try set(string: testString, forField: .testString) - } - - public func removeTestString() throws { - try deleteItem(forField: .testString) - } } private extension SubscriptionTokenKeychainStorage { diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift index 5aed907b6..58c7e74b6 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenStorage.swift @@ -22,8 +22,4 @@ public protocol SubscriptionTokenStorage: AnyObject { func getAccessToken() throws -> String? func store(accessToken: String) throws func removeAccessToken() throws - - func getTestString() throws -> String? - func updateTestString() throws - func removeTestString() throws } From ff7d1ae9e405eb1b014ddb351238d745ec86965f Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Wed, 28 Feb 2024 20:59:38 +0100 Subject: [PATCH 09/25] REmove label constant etc --- .../AccountStorage/SubscriptionTokenKeychainStorage.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index 7e110d3a6..79e586833 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -65,17 +65,11 @@ private extension SubscriptionTokenKeychainStorage { throw AccountKeychainAccessError.failedToDecodeKeychainDataAsString } } - - struct Defaults { - static let tokenStoreEntryLabel = "DuckDuckGo Privacy Pro Auth Token" - } - func retrieveData(forField field: AccountKeychainField) throws -> Data? { var query = defaultAttributes() query[kSecAttrService] = field.keyValue query[kSecMatchLimit] = kSecMatchLimitOne query[kSecReturnData] = true - //query[kSecAttrLabel] = Defaults.tokenStoreEntryLabel var item: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &item) From 4348c74e76c36a607dcc092b72cc8020e54a7508 Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Wed, 28 Feb 2024 20:59:58 +0100 Subject: [PATCH 10/25] Fix bug caused by mismatch of key values --- .../AccountStorage/SubscriptionTokenKeychainStorage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift index 79e586833..9b7a86bcc 100644 --- a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift +++ b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift @@ -50,7 +50,7 @@ private extension SubscriptionTokenKeychainStorage { case testString = "subscription.account.testString" var keyValue: String { - (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + "." + rawValue + "com.duckduckgo" + "." + rawValue } } From ad650ce89f4ec5902b70cea3258c2fa72e28b3f6 Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Fri, 1 Mar 2024 14:11:07 +0100 Subject: [PATCH 11/25] Add basic migration of token --- Sources/Subscription/AccountManager.swift | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 7b46bbbe8..0e0e35075 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -72,8 +72,15 @@ public class AccountManager: AccountManaging { public var accessToken: String? { do { -// return try storage.getAccessToken() - return try accessTokenStorage.getAccessToken() + // This migration is to prevent breaking internal tests and should be removed before launch + if let newAccessToken = try accessTokenStorage.getAccessToken() { + return newAccessToken + } else if let oldAccessToken = try storage.getAccessToken() { + try accessTokenStorage.store(accessToken: oldAccessToken) + return oldAccessToken + } else { + return nil + } } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .getAccessToken, error: error) @@ -131,7 +138,6 @@ public class AccountManager: AccountManaging { os_log(.info, log: .subscription, "[AccountManager] storeAccount") do { -// try storage.store(accessToken: token) try accessTokenStorage.store(accessToken: token) } catch { if let error = error as? AccountKeychainAccessError { From 6bb195f6196317e955001418999a4f3c046ffafa Mon Sep 17 00:00:00 2001 From: Graeme Arthur <2030310+graeme@users.noreply.github.com> Date: Fri, 1 Mar 2024 14:42:23 +0100 Subject: [PATCH 12/25] Move migration to dedicated public function --- Sources/Subscription/AccountManager.swift | 28 +++++++++++++++-------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 0e0e35075..35ae303b8 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -72,15 +72,7 @@ public class AccountManager: AccountManaging { public var accessToken: String? { do { - // This migration is to prevent breaking internal tests and should be removed before launch - if let newAccessToken = try accessTokenStorage.getAccessToken() { - return newAccessToken - } else if let oldAccessToken = try storage.getAccessToken() { - try accessTokenStorage.store(accessToken: oldAccessToken) - return oldAccessToken - } else { - return nil - } + return try accessTokenStorage.getAccessToken() } catch { if let error = error as? AccountKeychainAccessError { delegate?.accountManagerKeychainAccessFailed(accessType: .getAccessToken, error: error) @@ -186,6 +178,24 @@ public class AccountManager: AccountManaging { NotificationCenter.default.post(name: .accountDidSignOut, object: self, userInfo: nil) } + public func migrateAccessTokenToNewStore() throws { + // This migration is to prevent breaking internal tests and should be removed before launch + do { + if let newAccessToken = try accessTokenStorage.getAccessToken() { + throw MigrationError.noMigrationNeeded + } else if let oldAccessToken = try storage.getAccessToken() { + try accessTokenStorage.store(accessToken: oldAccessToken) + } + } catch { + throw MigrationError.migrationFailed + } + } + + public enum MigrationError: Error { + case migrationFailed + case noMigrationNeeded + } + // MARK: - public enum Entitlement: String { From 815903cf727bb8be9b9a8d228b4f7739255e87a5 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 13:38:32 -0500 Subject: [PATCH 13/25] Refactor --- .../PacketTunnelProvider.swift | 23 +++---- ...serDefaults+showEntitlementMessaging.swift | 20 +++--- .../Settings/VPNSettings.swift | 67 ------------------- 3 files changed, 21 insertions(+), 89 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 03bc6917c..02fcbe144 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -124,6 +124,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private let settings: VPNSettings + // MARK: - User Defaults + + private let defaults: UserDefaults + // MARK: - Server Selection public var lastSelectedServerInfo: NetworkProtectionServerInfo? { @@ -299,6 +303,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { debugEvents: EventMapping?, providerEvents: EventMapping, settings: VPNSettings, + defaults: UserDefaults, isSubscriptionEnabled: Bool, entitlementCheck: (() async -> Result)?) { os_log("[+] PacketTunnelProvider", log: .networkProtectionMemoryLog, type: .debug) @@ -311,6 +316,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self.tunnelHealth = tunnelHealthStore self.controllerErrorStore = controllerErrorStore self.settings = settings + self.defaults = defaults self.isSubscriptionEnabled = isSubscriptionEnabled self.entitlementCheck = isSubscriptionEnabled ? entitlementCheck : nil @@ -776,7 +782,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } catch { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { os_log("🔵 Expired subscription", log: .networkProtection, type: .error) - settings.enableEntitlementMessaging() + defaults.enableEntitlementMessaging() /// We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) @@ -912,14 +918,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } completionHandler?(nil) } - case .setShowEntitlementNotification: - if settings.showEntitlementNotification { - notificationsPresenter.showEntitlementNotification { [weak self] error in - guard error == nil else { return } - self?.settings.apply(change: .setShowEntitlementNotification(false)) - } - } - completionHandler?(nil) case .setConnectOnLogin, .setIncludeAllNetworks, .setEnforceRoutes, @@ -929,8 +927,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { .setShowInMenuBar, .setVPNFirstEnabled, .setNetworkPathChange, - .setDisableRekeying, - .setShowEntitlementAlert: + .setDisableRekeying: // Intentional no-op, as some setting changes don't require any further operation completionHandler?(nil) } @@ -1198,9 +1195,9 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { await entitlementMonitor.start(entitlementCheck: entitlementCheck) { [weak self] result in switch result { case .validEntitlement: - self?.settings.resetEntitlementMessaging() + self?.defaults.resetEntitlementMessaging() case .invalidEntitlement: - self?.settings.enableEntitlementMessaging() + self?.defaults.enableEntitlementMessaging() Task { [weak self] in await self?.attemptToShutdown() } diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index 42169bc6c..20990a9e6 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -26,7 +26,7 @@ extension UserDefaults { } @objc - dynamic var showEntitlementAlert: Bool { + public dynamic var showEntitlementAlert: Bool { get { value(forKey: showEntitlementAlertKey) as? Bool ?? false } @@ -43,16 +43,12 @@ extension UserDefaults { } } - var showEntitlementAlertPublisher: AnyPublisher { - publisher(for: \.showEntitlementAlert).eraseToAnyPublisher() - } - private var showEntitlementNotificationKey: String { "showEntitlementNotification" } @objc - dynamic var showEntitlementNotification: Bool { + public dynamic var showEntitlementNotification: Bool { get { value(forKey: showEntitlementNotificationKey) as? Bool ?? false } @@ -69,12 +65,18 @@ extension UserDefaults { } } - var showEntitlementNotificationPublisher: AnyPublisher { - publisher(for: \.showEntitlementNotification).eraseToAnyPublisher() + public func enableEntitlementMessaging() { + showEntitlementAlert = true + showEntitlementNotification = true + NotificationCenter.default.post(name: .vpnEntitlementMessagingDidChange, object: nil) } - func resetEntitlementMessaging() { + public func resetEntitlementMessaging() { removeObject(forKey: showEntitlementAlertKey) removeObject(forKey: showEntitlementNotificationKey) } } + +public extension Notification.Name { + static let vpnEntitlementMessagingDidChange = Notification.Name("com.duckduckgo.network-protection.entitlement-messaging-changed") +} diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 5dda31c95..bf5801cca 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -42,8 +42,6 @@ public final class VPNSettings { case setVPNFirstEnabled(_ vpnFirstEnabled: Date?) case setNetworkPathChange(_ newPath: String?) case setDisableRekeying(_ disableRekeying: Bool) - case setShowEntitlementAlert(_ showsAlert: Bool) - case setShowEntitlementNotification(_ showsNotification: Bool) } public enum RegistrationKeyValidity: Codable, Equatable { @@ -186,20 +184,6 @@ public final class VPNSettings { Change.setDisableRekeying(disableRekeying) }.eraseToAnyPublisher() - let showEntitlementAlertPublisher = showEntitlementAlertPublisher - .dropFirst() - .removeDuplicates() - .map { showsAlert in - Change.setShowEntitlementAlert(showsAlert) - }.eraseToAnyPublisher() - - let showEntitlementNotificationPublisher = showEntitlementNotificationPublisher - .dropFirst() - .removeDuplicates() - .map { showsNotification in - Change.setShowEntitlementNotification(showsNotification) - }.eraseToAnyPublisher() - return Publishers.MergeMany( connectOnLoginPublisher, includeAllNetworksPublisher, @@ -212,8 +196,6 @@ public final class VPNSettings { showInMenuBarPublisher, vpnFirstEnabledPublisher, networkPathChangePublisher, - showEntitlementAlertPublisher, - showEntitlementNotificationPublisher, disableRekeyingPublisher).eraseToAnyPublisher() }() @@ -268,10 +250,6 @@ public final class VPNSettings { newPath: newPath ?? "unknown") case .setDisableRekeying(let disableRekeying): self.disableRekeying = disableRekeying - case .setShowEntitlementAlert(let showsAlert): - self.showEntitlementAlert = showsAlert - case .setShowEntitlementNotification(let showsNotification): - self.showEntitlementNotification = showsNotification } } // swiftlint:enable cyclomatic_complexity @@ -511,49 +489,4 @@ public final class VPNSettings { defaults.networkProtectionSettingDisableRekeying = newValue } } - - // MARK: - Whether to show expired entitlement messaging - - public var showEntitlementAlertPublisher: AnyPublisher { - defaults.showEntitlementAlertPublisher - } - - public var showEntitlementAlert: Bool { - get { - defaults.showEntitlementAlert - } - - set { - defaults.showEntitlementAlert = newValue - } - } - - public var showEntitlementNotificationPublisher: AnyPublisher { - defaults.showEntitlementNotificationPublisher - } - - public var showEntitlementNotification: Bool { - get { - defaults.showEntitlementNotification - } - - set { - defaults.showEntitlementNotification = newValue - } - } - - public func enableEntitlementMessaging() { - apply(change: .setShowEntitlementAlert(true)) - apply(change: .setShowEntitlementNotification(true)) - os_log("🔵 Posting vpnEntitlementMessagingDidChange", log: .networkProtection) - NotificationCenter.default.post(name: .vpnEntitlementMessagingDidChange, object: nil) - } - - public func resetEntitlementMessaging() { - defaults.resetEntitlementMessaging() - } -} - -public extension Notification.Name { - static let vpnEntitlementMessagingDidChange = Notification.Name("com.duckduckgo.network-protection.entitlement-messaging-changed") } From fc8ee8dd753b5bb6f8056916df194075624b0c44 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 20:18:34 -0500 Subject: [PATCH 14/25] Use Darwin notification instead --- .../Extensions/UserDefaults+showEntitlementMessaging.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index 20990a9e6..10433e0dc 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -68,7 +68,10 @@ extension UserDefaults { public func enableEntitlementMessaging() { showEntitlementAlert = true showEntitlementNotification = true - NotificationCenter.default.post(name: .vpnEntitlementMessagingDidChange, object: nil) + + CFNotificationCenterPostNotification(CFNotificationCenterGetDarwinNotifyCenter(), + CFNotificationName(rawValue: Notification.Name.vpnEntitlementMessagingDidChange.rawValue as CFString), + nil, nil, true) } public func resetEntitlementMessaging() { From 6d46796ac79179e66a1c72cdddf574ceb9816a7c Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 20:27:32 -0500 Subject: [PATCH 15/25] Show notification from app extension right away --- Sources/NetworkProtection/PacketTunnelProvider.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 02fcbe144..13d00e672 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -783,6 +783,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { os_log("🔵 Expired subscription", log: .networkProtection, type: .error) defaults.enableEntitlementMessaging() + notificationsPresenter.showEntitlementNotification { [weak self] error in + guard error == nil else { return } + self?.defaults.showEntitlementNotification = false + } /// We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) @@ -1198,6 +1202,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self?.defaults.resetEntitlementMessaging() case .invalidEntitlement: self?.defaults.enableEntitlementMessaging() + self?.notificationsPresenter.showEntitlementNotification { [weak self] error in + guard error == nil else { return } + self?.defaults.showEntitlementNotification = false + } Task { [weak self] in await self?.attemptToShutdown() } From 7f854c241b591747d9794444f9df7d299c5408b7 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 20:38:04 -0500 Subject: [PATCH 16/25] Move presentation logic inside TogglableDecorator --- .../NetworkProtection/PacketTunnelProvider.swift | 16 +++++++--------- ...otificationsPresenterTogglableDecorator.swift | 11 +++++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 13d00e672..1a1e938c3 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -783,12 +783,9 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { os_log("🔵 Expired subscription", log: .networkProtection, type: .error) defaults.enableEntitlementMessaging() - notificationsPresenter.showEntitlementNotification { [weak self] error in - guard error == nil else { return } - self?.defaults.showEntitlementNotification = false - } + notificationsPresenter.showEntitlementNotification { _ in } - /// We add a delay here so the notification has a chance to show up + // We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) throw TunnelError.vpnAccessRevoked @@ -1202,11 +1199,12 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self?.defaults.resetEntitlementMessaging() case .invalidEntitlement: self?.defaults.enableEntitlementMessaging() - self?.notificationsPresenter.showEntitlementNotification { [weak self] error in - guard error == nil else { return } - self?.defaults.showEntitlementNotification = false - } + self?.notificationsPresenter.showEntitlementNotification { _ in } + Task { [weak self] in + // We add a delay here so the notification has a chance to show up + try? await Task.sleep(interval: .seconds(5)) + await self?.attemptToShutdown() } case .error: diff --git a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift index 8f96484db..739a38279 100644 --- a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift +++ b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift @@ -20,10 +20,12 @@ import Foundation final public class NetworkProtectionNotificationsPresenterTogglableDecorator: NetworkProtectionNotificationsPresenter { private let settings: VPNSettings + private let defaults: UserDefaults private let wrappeePresenter: NetworkProtectionNotificationsPresenter - public init(settings: VPNSettings, wrappee: NetworkProtectionNotificationsPresenter) { + public init(settings: VPNSettings, defaults: UserDefaults, wrappee: NetworkProtectionNotificationsPresenter) { self.settings = settings + self.defaults = defaults self.wrappeePresenter = wrappee } @@ -58,6 +60,11 @@ final public class NetworkProtectionNotificationsPresenterTogglableDecorator: Ne } public func showEntitlementNotification(completion: @escaping (Error?) -> Void) { - wrappeePresenter.showEntitlementNotification(completion: completion) + if defaults.showEntitlementNotification { + wrappeePresenter.showEntitlementNotification { [weak self] error in + guard error == nil else { return } + self?.defaults.showEntitlementNotification = false + } + } } } From c104e609be374433c3b36e0c0c0089fd6f129373 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 28 Feb 2024 21:01:15 -0500 Subject: [PATCH 17/25] Refactoring --- Sources/NetworkProtection/PacketTunnelProvider.swift | 6 +++--- .../UserDefaults+showEntitlementMessaging.swift | 2 ++ .../NetworkProtectionNotificationsPresenter.swift | 2 +- ...otectionNotificationsPresenterTogglableDecorator.swift | 8 +++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 1a1e938c3..4f5178ab7 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -783,7 +783,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { os_log("🔵 Expired subscription", log: .networkProtection, type: .error) defaults.enableEntitlementMessaging() - notificationsPresenter.showEntitlementNotification { _ in } + notificationsPresenter.showEntitlementNotification() // We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) @@ -1199,12 +1199,12 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self?.defaults.resetEntitlementMessaging() case .invalidEntitlement: self?.defaults.enableEntitlementMessaging() - self?.notificationsPresenter.showEntitlementNotification { _ in } + self?.notificationsPresenter.showEntitlementNotification() Task { [weak self] in // We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) - + await self?.attemptToShutdown() } case .error: diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index 10433e0dc..cea0a17d9 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -69,9 +69,11 @@ extension UserDefaults { showEntitlementAlert = true showEntitlementNotification = true +#if os(iOS) CFNotificationCenterPostNotification(CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName(rawValue: Notification.Name.vpnEntitlementMessagingDidChange.rawValue as CFString), nil, nil, true) +#endif } public func resetEntitlementMessaging() { diff --git a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenter.swift b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenter.swift index 68189d1eb..700589350 100644 --- a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenter.swift +++ b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenter.swift @@ -39,5 +39,5 @@ public protocol NetworkProtectionNotificationsPresenter { func showTestNotification() /// Present a "expired subscription" notification to the user. - func showEntitlementNotification(completion: @escaping (Error?) -> Void) + func showEntitlementNotification() } diff --git a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift index 739a38279..a105f8f8c 100644 --- a/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift +++ b/Sources/NetworkProtection/Status/UserNotifications/NetworkProtectionNotificationsPresenterTogglableDecorator.swift @@ -59,12 +59,10 @@ final public class NetworkProtectionNotificationsPresenterTogglableDecorator: Ne } } - public func showEntitlementNotification(completion: @escaping (Error?) -> Void) { + public func showEntitlementNotification() { if defaults.showEntitlementNotification { - wrappeePresenter.showEntitlementNotification { [weak self] error in - guard error == nil else { return } - self?.defaults.showEntitlementNotification = false - } + defaults.showEntitlementNotification = false + wrappeePresenter.showEntitlementNotification() } } } From e69f05f7524a584628dd71dcebe66dc8f5c5660c Mon Sep 17 00:00:00 2001 From: Anh Do Date: Fri, 1 Mar 2024 00:11:08 -0500 Subject: [PATCH 18/25] Block all traffic --- .../ExtensionMessage/ExtensionRequest.swift | 1 + .../PacketTunnelProvider.swift | 67 +++++++++++++++---- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift b/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift index e4f95b4b1..9b170d7da 100644 --- a/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift +++ b/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift @@ -23,6 +23,7 @@ public enum DebugCommand: Codable { case removeSystemExtension case removeVPNConfiguration case sendTestNotification + case blockAllTraffic case disableConnectOnDemandAndShutDown } diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 4f5178ab7..2841d5f41 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -810,7 +810,46 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { /// This will block all traffic @MainActor private func updatePlaceholderTunnelConfiguration() async throws { - // todo + let interface = InterfaceConfiguration( + privateKey: PrivateKey(), + addresses: [IPAddressRange(from: "0.0.0.0/0")!], + includedRoutes: [], + excludedRoutes: [], + listenPort: 0, + dns: [DNSServer(address: IPv4Address.loopback)] + ) + + var peerConfiguration = PeerConfiguration(publicKey: PrivateKey().publicKey) + peerConfiguration.endpoint = Endpoint(host: "127.0.0.1", port: 9090) + + let tunnelConfiguration = TunnelConfiguration(name: "Placeholder", interface: interface, peers: [peerConfiguration]) + + try await withCheckedThrowingContinuation { [weak self] (continuation: CheckedContinuation) in + guard let self = self else { + continuation.resume() + return + } + + self.adapter.update(tunnelConfiguration: tunnelConfiguration, reassert: true) { [weak self] error in + if let error = error { + os_log("🔵 Failed to update the placeholder configuration: %{public}@", type: .error, error.localizedDescription) + self?.debugEvents?.fire(error.networkProtectionError) + continuation.resume(throwing: error) + return + } + + Task { [weak self] in + do { + try await self?.handleAdapterStarted(startReason: .reconnected) + } catch { + continuation.resume(throwing: error) + return + } + + continuation.resume() + } + } + } } // MARK: - App Messages @@ -943,6 +982,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { handleExpireRegistrationKey(completionHandler: completionHandler) case .sendTestNotification: handleSendTestNotification(completionHandler: completionHandler) + case .blockAllTraffic: + handleBlockAllTraffic(completionHandler: completionHandler) case .disableConnectOnDemandAndShutDown: if #available(iOS 17, *) { handleShutDown(completionHandler: completionHandler) @@ -1041,6 +1082,14 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { completionHandler?(nil) } + public func handleBlockAllTraffic(completionHandler: ((Data?) -> Void)? = nil) { + Task { @MainActor [weak self] in + await self?.stopMonitors() + try? await self?.updatePlaceholderTunnelConfiguration() + completionHandler?(nil) + } + } + @available(iOS 17, *) public func handleShutDown(completionHandler: ((Data?) -> Void)? = nil) { Task { @MainActor in @@ -1201,11 +1250,13 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self?.defaults.enableEntitlementMessaging() self?.notificationsPresenter.showEntitlementNotification() - Task { [weak self] in + Task { @MainActor [weak self] in + await self?.stopMonitors() + // We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) - await self?.attemptToShutdown() + try? await self?.updatePlaceholderTunnelConfiguration() } case .error: break @@ -1242,16 +1293,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { return true } - private func attemptToShutdown() async { - await stopMonitors() - - if #available(iOS 17, *) { - handleShutDown() - } else { - try? await updatePlaceholderTunnelConfiguration() - } - } - // MARK: - Connection Tester private enum ConnectionTesterError: Error { From 5092a367fd4c35883c29b955b1bf19f5b17073f4 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Mon, 4 Mar 2024 12:00:46 -0500 Subject: [PATCH 19/25] Fix Swiftlint --- Sources/NetworkProtection/PacketTunnelProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 2841d5f41..6de8e0e88 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -916,7 +916,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { settings.apply(change: change) } - // swiftlint:disable:next cyclomatic_complexity function_body_length + // swiftlint:disable:next cyclomatic_complexity private func handleSettingsChange(_ change: VPNSettings.Change, completionHandler: ((Data?) -> Void)? = nil) { switch change { case .setExcludeLocalNetworks: From 1ab1e29502a81ebef374a726499405d7c965bdf4 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Tue, 5 Mar 2024 22:48:02 -0500 Subject: [PATCH 20/25] Attempt shutdown --- .../ExtensionMessage/ExtensionRequest.swift | 1 - .../PacketTunnelProvider.swift | 93 +++++-------------- 2 files changed, 25 insertions(+), 69 deletions(-) diff --git a/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift b/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift index 9b170d7da..e4f95b4b1 100644 --- a/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift +++ b/Sources/NetworkProtection/ExtensionMessage/ExtensionRequest.swift @@ -23,7 +23,6 @@ public enum DebugCommand: Codable { case removeSystemExtension case removeVPNConfiguration case sendTestNotification - case blockAllTraffic case disableConnectOnDemandAndShutDown } diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 6de8e0e88..88215edd2 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -806,52 +806,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { return configurationResult.0 } - /// Placeholder configuration to switch to when the entitlement expires - /// This will block all traffic - @MainActor - private func updatePlaceholderTunnelConfiguration() async throws { - let interface = InterfaceConfiguration( - privateKey: PrivateKey(), - addresses: [IPAddressRange(from: "0.0.0.0/0")!], - includedRoutes: [], - excludedRoutes: [], - listenPort: 0, - dns: [DNSServer(address: IPv4Address.loopback)] - ) - - var peerConfiguration = PeerConfiguration(publicKey: PrivateKey().publicKey) - peerConfiguration.endpoint = Endpoint(host: "127.0.0.1", port: 9090) - - let tunnelConfiguration = TunnelConfiguration(name: "Placeholder", interface: interface, peers: [peerConfiguration]) - - try await withCheckedThrowingContinuation { [weak self] (continuation: CheckedContinuation) in - guard let self = self else { - continuation.resume() - return - } - - self.adapter.update(tunnelConfiguration: tunnelConfiguration, reassert: true) { [weak self] error in - if let error = error { - os_log("🔵 Failed to update the placeholder configuration: %{public}@", type: .error, error.localizedDescription) - self?.debugEvents?.fire(error.networkProtectionError) - continuation.resume(throwing: error) - return - } - - Task { [weak self] in - do { - try await self?.handleAdapterStarted(startReason: .reconnected) - } catch { - continuation.resume(throwing: error) - return - } - - continuation.resume() - } - } - } - } - // MARK: - App Messages // swiftlint:disable:next cyclomatic_complexity @@ -982,11 +936,14 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { handleExpireRegistrationKey(completionHandler: completionHandler) case .sendTestNotification: handleSendTestNotification(completionHandler: completionHandler) - case .blockAllTraffic: - handleBlockAllTraffic(completionHandler: completionHandler) case .disableConnectOnDemandAndShutDown: if #available(iOS 17, *) { handleShutDown(completionHandler: completionHandler) + } else { + Task { + await rekey() + completionHandler?(nil) + } } case .removeVPNConfiguration: // Since the VPN configuration is being removed we may as well reset all state @@ -1082,14 +1039,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { completionHandler?(nil) } - public func handleBlockAllTraffic(completionHandler: ((Data?) -> Void)? = nil) { - Task { @MainActor [weak self] in - await self?.stopMonitors() - try? await self?.updatePlaceholderTunnelConfiguration() - completionHandler?(nil) - } - } - @available(iOS 17, *) public func handleShutDown(completionHandler: ((Data?) -> Void)? = nil) { Task { @MainActor in @@ -1243,23 +1192,31 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { guard isSubscriptionEnabled, let entitlementCheck else { return } await entitlementMonitor.start(entitlementCheck: entitlementCheck) { [weak self] result in + /// Attempt tunnel shutdown & show messaging iff the entitlement is verified to be invalid + /// Ignore otherwise switch result { - case .validEntitlement: - self?.defaults.resetEntitlementMessaging() case .invalidEntitlement: - self?.defaults.enableEntitlementMessaging() - self?.notificationsPresenter.showEntitlementNotification() + self?.handleInvalidEntitlement() + case .validEntitlement, .error: + break + } + } + } - Task { @MainActor [weak self] in - await self?.stopMonitors() + private func handleInvalidEntitlement() { + defaults.enableEntitlementMessaging() + notificationsPresenter.showEntitlementNotification() - // We add a delay here so the notification has a chance to show up - try? await Task.sleep(interval: .seconds(5)) + Task { @MainActor [weak self] in + await self?.stopMonitors() - try? await self?.updatePlaceholderTunnelConfiguration() - } - case .error: - break + // We add a delay here so the notification has a chance to show up + try? await Task.sleep(interval: .seconds(5)) + + if #available(iOS 17, *) { + self?.handleShutDown() + } else { + await self?.rekey() } } } From 1de20d1850d25210dad38935b64b422f9bca832c Mon Sep 17 00:00:00 2001 From: Anh Do Date: Tue, 5 Mar 2024 23:02:13 -0500 Subject: [PATCH 21/25] Refactor --- .../PacketTunnelProvider.swift | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 88215edd2..873bde65c 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -781,13 +781,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { ) } catch { if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { - os_log("🔵 Expired subscription", log: .networkProtection, type: .error) - defaults.enableEntitlementMessaging() - notificationsPresenter.showEntitlementNotification() - - // We add a delay here so the notification has a chance to show up - try? await Task.sleep(interval: .seconds(5)) - + await handleInvalidEntitlement(attemptsShutdown: false) throw TunnelError.vpnAccessRevoked } @@ -1196,28 +1190,31 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { /// Ignore otherwise switch result { case .invalidEntitlement: - self?.handleInvalidEntitlement() + Task { [weak self] in + await self?.handleInvalidEntitlement(attemptsShutdown: true) + } case .validEntitlement, .error: break } } } - private func handleInvalidEntitlement() { + @MainActor + private func handleInvalidEntitlement(attemptsShutdown: Bool) async { defaults.enableEntitlementMessaging() notificationsPresenter.showEntitlementNotification() - Task { @MainActor [weak self] in - await self?.stopMonitors() + await stopMonitors() + + // We add a delay here so the notification has a chance to show up + try? await Task.sleep(interval: .seconds(5)) - // We add a delay here so the notification has a chance to show up - try? await Task.sleep(interval: .seconds(5)) + guard attemptsShutdown else { return } - if #available(iOS 17, *) { - self?.handleShutDown() - } else { - await self?.rekey() - } + if #available(iOS 17, *) { + handleShutDown() + } else { + await rekey() } } From dfc2e5c9a6be04f1e07c8ca9ce4c960c32eb5fa2 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Tue, 5 Mar 2024 23:09:28 -0500 Subject: [PATCH 22/25] More refactoring --- .../PacketTunnelProvider.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 873bde65c..bea266fa4 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -931,11 +931,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case .sendTestNotification: handleSendTestNotification(completionHandler: completionHandler) case .disableConnectOnDemandAndShutDown: - if #available(iOS 17, *) { - handleShutDown(completionHandler: completionHandler) - } else { - Task { - await rekey() + Task { [weak self] in + await self?.attemptShutdown { completionHandler?(nil) } } @@ -1209,13 +1206,21 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // We add a delay here so the notification has a chance to show up try? await Task.sleep(interval: .seconds(5)) - guard attemptsShutdown else { return } + if attemptsShutdown { + await attemptShutdown() + } + } + // Attempt to shut down the tunnel + // On iOS 16 and below, as a workaround, we rekey to force a 403 error so that the tunnel fails to restart + @MainActor + private func attemptShutdown(completion: (() -> Void)? = nil) async { if #available(iOS 17, *) { handleShutDown() } else { await rekey() } + completion?() } @MainActor From d1d42f98db6494a2261ad5e2bf0c03cf5fa8b348 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 6 Mar 2024 16:20:21 -0500 Subject: [PATCH 23/25] Remove unnecessary import --- Sources/NetworkProtection/Settings/VPNSettings.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index bf5801cca..b71dc2336 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -18,7 +18,6 @@ import Combine import Foundation -import Common /// Persists and publishes changes to tunnel settings. /// From ac7db80abead0693c7bbec490e8abaeb4edf9cf7 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 6 Mar 2024 16:22:58 -0500 Subject: [PATCH 24/25] Update property visibility --- .../Extensions/UserDefaults+showEntitlementMessaging.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index cea0a17d9..f95e05827 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -26,7 +26,7 @@ extension UserDefaults { } @objc - public dynamic var showEntitlementAlert: Bool { + dynamic var showEntitlementAlert: Bool { get { value(forKey: showEntitlementAlertKey) as? Bool ?? false } @@ -48,7 +48,7 @@ extension UserDefaults { } @objc - public dynamic var showEntitlementNotification: Bool { + dynamic var showEntitlementNotification: Bool { get { value(forKey: showEntitlementNotificationKey) as? Bool ?? false } From 4e3a1b66cf711f28abd1a6eef7e37ac4d805e4f7 Mon Sep 17 00:00:00 2001 From: Anh Do Date: Wed, 6 Mar 2024 16:24:05 -0500 Subject: [PATCH 25/25] Revert "Update property visibility" This reverts commit ac7db80abead0693c7bbec490e8abaeb4edf9cf7. --- .../Extensions/UserDefaults+showEntitlementMessaging.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift index f95e05827..cea0a17d9 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+showEntitlementMessaging.swift @@ -26,7 +26,7 @@ extension UserDefaults { } @objc - dynamic var showEntitlementAlert: Bool { + public dynamic var showEntitlementAlert: Bool { get { value(forKey: showEntitlementAlertKey) as? Bool ?? false } @@ -48,7 +48,7 @@ extension UserDefaults { } @objc - dynamic var showEntitlementNotification: Bool { + public dynamic var showEntitlementNotification: Bool { get { value(forKey: showEntitlementNotificationKey) as? Bool ?? false }