From 734cb31f0d0c1669029853faa128980c712fb1c5 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 8 Sep 2024 14:46:23 -0700 Subject: [PATCH 1/2] Clean up feature flag checks. --- ...kProtectionCodeRedemptionCoordinator.swift | 50 ------------------- .../NetworkProtectionTokenStore.swift | 8 +-- .../NetworkProtectionDeviceManager.swift | 20 +++----- .../Networking/NetworkProtectionClient.swift | 6 +-- .../PacketTunnelProvider.swift | 26 ++++------ ...workProtectionLocationListRepository.swift | 13 ++--- ...eRedemptionCoordinatorTestExtensions.swift | 44 ---------------- .../NetworkProtectionClientTests.swift | 2 +- .../NetworkProtectionDeviceManagerTests.swift | 3 +- ...LocationListCompositeRepositoryTests.swift | 3 +- 10 files changed, 29 insertions(+), 146 deletions(-) delete mode 100644 Sources/NetworkProtection/FeatureActivation/NetworkProtectionCodeRedemptionCoordinator.swift delete mode 100644 Sources/NetworkProtectionTestUtils/FeatureActivation/NetworkProtectionCodeRedemptionCoordinatorTestExtensions.swift diff --git a/Sources/NetworkProtection/FeatureActivation/NetworkProtectionCodeRedemptionCoordinator.swift b/Sources/NetworkProtection/FeatureActivation/NetworkProtectionCodeRedemptionCoordinator.swift deleted file mode 100644 index ca5f77bce..000000000 --- a/Sources/NetworkProtection/FeatureActivation/NetworkProtectionCodeRedemptionCoordinator.swift +++ /dev/null @@ -1,50 +0,0 @@ -// -// NetworkProtectionCodeRedemptionCoordinator.swift -// -// Copyright © 2023 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 -import Common - -/// Coordinates calls to the backend and oAuth token storage -public final class NetworkProtectionCodeRedemptionCoordinator { - private let networkClient: NetworkProtectionClient - private let tokenStore: NetworkProtectionTokenStore - private let isManualCodeRedemptionFlow: Bool - private let errorEvents: EventMapping - - convenience public init(environment: VPNSettings.SelectedEnvironment, - tokenStore: NetworkProtectionTokenStore, - isManualCodeRedemptionFlow: Bool = false, - errorEvents: EventMapping, - isSubscriptionEnabled: Bool) { - self.init(networkClient: NetworkProtectionBackendClient(environment: environment, isSubscriptionEnabled: isSubscriptionEnabled), - tokenStore: tokenStore, - isManualCodeRedemptionFlow: isManualCodeRedemptionFlow, - errorEvents: errorEvents) - } - - init(networkClient: NetworkProtectionClient, - tokenStore: NetworkProtectionTokenStore, - isManualCodeRedemptionFlow: Bool = false, - errorEvents: EventMapping) { - self.networkClient = networkClient - self.tokenStore = tokenStore - self.isManualCodeRedemptionFlow = isManualCodeRedemptionFlow - self.errorEvents = errorEvents - } - -} diff --git a/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift b/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift index 18008a8db..4510dc1b6 100644 --- a/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift +++ b/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift @@ -42,7 +42,7 @@ public protocol NetworkProtectionTokenStore { public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenStore { private let keychainStore: NetworkProtectionKeychainStore private let errorEvents: EventMapping? - private let isSubscriptionEnabled: Bool + private let useAccessTokenProvider: Bool public typealias AccessTokenProvider = () -> String? private let accessTokenProvider: AccessTokenProvider @@ -59,13 +59,13 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt public init(keychainType: KeychainType, serviceName: String = Defaults.tokenStoreService, errorEvents: EventMapping?, - isSubscriptionEnabled: Bool, + useAccessTokenProvider: Bool, accessTokenProvider: @escaping AccessTokenProvider) { keychainStore = NetworkProtectionKeychainStore(label: Defaults.tokenStoreEntryLabel, serviceName: serviceName, keychainType: keychainType) self.errorEvents = errorEvents - self.isSubscriptionEnabled = isSubscriptionEnabled + self.useAccessTokenProvider = useAccessTokenProvider self.accessTokenProvider = accessTokenProvider } @@ -84,7 +84,7 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt } public func fetchToken() throws -> String? { - if isSubscriptionEnabled { + if useAccessTokenProvider { return accessTokenProvider().map { makeToken(from: $0) } } diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift index 78c3eb13d..389ddd069 100644 --- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift +++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift @@ -80,30 +80,24 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { private let errorEvents: EventMapping? - private let isSubscriptionEnabled: Bool - public init(environment: VPNSettings.SelectedEnvironment, tokenStore: NetworkProtectionTokenStore, keyStore: NetworkProtectionKeyStore, - errorEvents: EventMapping?, - isSubscriptionEnabled: Bool) { - self.init(networkClient: NetworkProtectionBackendClient(environment: environment, isSubscriptionEnabled: isSubscriptionEnabled), + errorEvents: EventMapping?) { + self.init(networkClient: NetworkProtectionBackendClient(environment: environment), tokenStore: tokenStore, keyStore: keyStore, - errorEvents: errorEvents, - isSubscriptionEnabled: isSubscriptionEnabled) + errorEvents: errorEvents) } init(networkClient: NetworkProtectionClient, tokenStore: NetworkProtectionTokenStore, keyStore: NetworkProtectionKeyStore, - errorEvents: EventMapping?, - isSubscriptionEnabled: Bool) { + errorEvents: EventMapping?) { self.networkClient = networkClient self.tokenStore = tokenStore self.keyStore = keyStore self.errorEvents = errorEvents - self.isSubscriptionEnabled = isSubscriptionEnabled } /// Requests a new server list from the backend and updates it locally. @@ -348,10 +342,8 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { private func handleAccessRevoked(_ error: NetworkProtectionClientError) throws { switch error { case .accessDenied, .invalidAuthToken: - if isSubscriptionEnabled { - errorEvents?.fire(.vpnAccessRevoked) - throw NetworkProtectionError.vpnAccessRevoked - } + errorEvents?.fire(.vpnAccessRevoked) + throw NetworkProtectionError.vpnAccessRevoked default: break } diff --git a/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift b/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift index 8f258665d..8d64e6f2d 100644 --- a/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift +++ b/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift @@ -220,10 +220,8 @@ final class NetworkProtectionBackendClient: NetworkProtectionClient { }() private let endpointURL: URL - private let isSubscriptionEnabled: Bool - init(environment: VPNSettings.SelectedEnvironment = .default, isSubscriptionEnabled: Bool) { - self.isSubscriptionEnabled = isSubscriptionEnabled + init(environment: VPNSettings.SelectedEnvironment = .default) { self.endpointURL = environment.endpointURL } @@ -399,7 +397,7 @@ final class NetworkProtectionBackendClient: NetworkProtectionClient { responseData = data case 401: return .failure(.invalidAuthToken) - case 403 where isSubscriptionEnabled: + case 403: return .failure(.accessDenied) default: throw RegisterError.unexpectedStatus(status: response.statusCode) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 7683f8965..b9e9de1da 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -233,8 +233,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { let locationRepository = NetworkProtectionLocationListCompositeRepository( environment: settings.selectedEnvironment, tokenStore: tokenStore, - errorEvents: debugEvents, - isSubscriptionEnabled: isSubscriptionEnabled + errorEvents: debugEvents ) return VPNServerSelectionResolver(locationListRepository: locationRepository, vpnSettings: settings) }() @@ -420,8 +419,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { environment: self.settings.selectedEnvironment, tokenStore: self.tokenStore, keyStore: self.keyStore, - errorEvents: self.debugEvents, - isSubscriptionEnabled: self.isSubscriptionEnabled + errorEvents: self.debugEvents ) private lazy var tunnelFailureMonitor = NetworkProtectionTunnelFailureMonitor(handshakeReporter: adapter) @@ -429,10 +427,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { public lazy var latencyMonitor = NetworkProtectionLatencyMonitor() public lazy var entitlementMonitor = NetworkProtectionEntitlementMonitor() public lazy var serverStatusMonitor = NetworkProtectionServerStatusMonitor( - networkClient: NetworkProtectionBackendClient( - environment: self.settings.selectedEnvironment, - isSubscriptionEnabled: true - ), + networkClient: NetworkProtectionBackendClient(environment: self.settings.selectedEnvironment), tokenStore: self.tokenStore ) @@ -453,8 +448,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private let keychainType: KeychainType private let debugEvents: EventMapping private let providerEvents: EventMapping - - public let isSubscriptionEnabled: Bool public let entitlementCheck: (() async -> Result)? public init(notificationsPresenter: NetworkProtectionNotificationsPresenter, @@ -469,7 +462,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { providerEvents: EventMapping, settings: VPNSettings, defaults: UserDefaults, - isSubscriptionEnabled: Bool, entitlementCheck: (() async -> Result)?) { Logger.networkProtectionMemory.debug("[+] PacketTunnelProvider") @@ -485,8 +477,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self.wireGuardInterface = wireGuardInterface self.settings = settings self.defaults = defaults - self.isSubscriptionEnabled = isSubscriptionEnabled - self.entitlementCheck = isSubscriptionEnabled ? entitlementCheck : nil + self.entitlementCheck = entitlementCheck super.init() @@ -1021,7 +1012,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { regenerateKey: regenerateKey ) } catch { - if isSubscriptionEnabled, let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { + if let error = error as? NetworkProtectionError, case .vpnAccessRevoked = error { throw TunnelError.vpnAccessRevoked } @@ -1513,7 +1504,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { await latencyMonitor.stop() } - if isSubscriptionEnabled, await isEntitlementInvalid() { + if await isEntitlementInvalid() { return } @@ -1532,7 +1523,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { await entitlementMonitor.stop() } - guard isSubscriptionEnabled, let entitlementCheck else { return } + guard let entitlementCheck else { + assertionFailure("Expected entitlement check but didn't find one") + return + } await entitlementMonitor.start(entitlementCheck: entitlementCheck) { [weak self] result in /// Attempt tunnel shutdown & show messaging iff the entitlement is verified to be invalid diff --git a/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift b/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift index eff282a4a..e4939a6ee 100644 --- a/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift +++ b/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift @@ -38,28 +38,23 @@ final public class NetworkProtectionLocationListCompositeRepository: NetworkProt private let client: NetworkProtectionClient private let tokenStore: NetworkProtectionTokenStore private let errorEvents: EventMapping - private let isSubscriptionEnabled: Bool convenience public init(environment: VPNSettings.SelectedEnvironment, tokenStore: NetworkProtectionTokenStore, - errorEvents: EventMapping, - isSubscriptionEnabled: Bool) { + errorEvents: EventMapping) { self.init( - client: NetworkProtectionBackendClient(environment: environment, isSubscriptionEnabled: isSubscriptionEnabled), + client: NetworkProtectionBackendClient(environment: environment), tokenStore: tokenStore, - errorEvents: errorEvents, - isSubscriptionEnabled: isSubscriptionEnabled + errorEvents: errorEvents ) } init(client: NetworkProtectionClient, tokenStore: NetworkProtectionTokenStore, - errorEvents: EventMapping, - isSubscriptionEnabled: Bool) { + errorEvents: EventMapping) { self.client = client self.tokenStore = tokenStore self.errorEvents = errorEvents - self.isSubscriptionEnabled = isSubscriptionEnabled } @MainActor diff --git a/Sources/NetworkProtectionTestUtils/FeatureActivation/NetworkProtectionCodeRedemptionCoordinatorTestExtensions.swift b/Sources/NetworkProtectionTestUtils/FeatureActivation/NetworkProtectionCodeRedemptionCoordinatorTestExtensions.swift deleted file mode 100644 index 0a6a42239..000000000 --- a/Sources/NetworkProtectionTestUtils/FeatureActivation/NetworkProtectionCodeRedemptionCoordinatorTestExtensions.swift +++ /dev/null @@ -1,44 +0,0 @@ -// -// NetworkProtectionCodeRedemptionCoordinatorTestExtensions.swift -// -// Copyright © 2023 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 -@testable import NetworkProtection -import Common - -public extension NetworkProtectionCodeRedemptionCoordinator { - private static var errorEvents: EventMapping = .init { _, _, _, _ in - } - - class func stubbed() -> NetworkProtectionCodeRedemptionCoordinator { - whereRedeemSucceeds() - } - - class func whereRedeemSucceeds(returning token: String = "") -> NetworkProtectionCodeRedemptionCoordinator { - let client = MockNetworkProtectionClient() - client.stubRedeem = .success(token) - let tokenStore = MockNetworkProtectionTokenStorage() - return NetworkProtectionCodeRedemptionCoordinator(networkClient: client, tokenStore: tokenStore, errorEvents: errorEvents) - } - - class func whereRedeemFails(returning error: NetworkProtectionClientError = .failedToEncodeRedeemRequest) -> NetworkProtectionCodeRedemptionCoordinator { - let client = MockNetworkProtectionClient() - client.stubRedeem = .failure(error) - let tokenStore = MockNetworkProtectionTokenStorage() - return NetworkProtectionCodeRedemptionCoordinator(networkClient: client, tokenStore: tokenStore, errorEvents: errorEvents) - } -} diff --git a/Tests/NetworkProtectionTests/NetworkProtectionClientTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionClientTests.swift index 6b1011273..5252e5db7 100644 --- a/Tests/NetworkProtectionTests/NetworkProtectionClientTests.swift +++ b/Tests/NetworkProtectionTests/NetworkProtectionClientTests.swift @@ -24,7 +24,7 @@ final class NetworkProtectionClientTests: XCTestCase { override func setUp() { super.setUp() - client = NetworkProtectionBackendClient(environment: .production, isSubscriptionEnabled: false) + client = NetworkProtectionBackendClient(environment: .production) } override class func setUp() { diff --git a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift index 3e88d1e3b..5bb3befc4 100644 --- a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift +++ b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift @@ -40,8 +40,7 @@ final class NetworkProtectionDeviceManagerTests: XCTestCase { networkClient: networkClient, tokenStore: tokenStore, keyStore: keyStore, - errorEvents: nil, - isSubscriptionEnabled: false + errorEvents: nil ) } diff --git a/Tests/NetworkProtectionTests/Repositories/NetworkProtectionLocationListCompositeRepositoryTests.swift b/Tests/NetworkProtectionTests/Repositories/NetworkProtectionLocationListCompositeRepositoryTests.swift index 964aa273e..57f992da4 100644 --- a/Tests/NetworkProtectionTests/Repositories/NetworkProtectionLocationListCompositeRepositoryTests.swift +++ b/Tests/NetworkProtectionTests/Repositories/NetworkProtectionLocationListCompositeRepositoryTests.swift @@ -37,8 +37,7 @@ class NetworkProtectionLocationListCompositeRepositoryTests: XCTestCase { tokenStore: tokenStore, errorEvents: .init { [weak self] event, _, _, _ in self?.verifyErrorEvent?(event) - }, - isSubscriptionEnabled: false) + }) } @MainActor From 8da6071a523f14bece02332ba9f84c2aaf9d107d Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 8 Sep 2024 14:46:37 -0700 Subject: [PATCH 2/2] Remove unused source file. --- .../Concurrency/MainActorExtension.swift | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 Sources/Common/Concurrency/MainActorExtension.swift diff --git a/Sources/Common/Concurrency/MainActorExtension.swift b/Sources/Common/Concurrency/MainActorExtension.swift deleted file mode 100644 index cccc5a0a6..000000000 --- a/Sources/Common/Concurrency/MainActorExtension.swift +++ /dev/null @@ -1,45 +0,0 @@ -// -// MainActorExtension.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 - -#if swift(<5.10) -private protocol MainActorPerformer { - func perform(_ operation: @MainActor () throws -> T) rethrows -> T -} -private struct OnMainActor: MainActorPerformer { - private init() {} - static func instance() -> MainActorPerformer { OnMainActor() } - - @MainActor(unsafe) - func perform(_ operation: @MainActor () throws -> T) rethrows -> T { - try operation() - } -} -public extension MainActor { - static func assumeIsolated(_ operation: @MainActor () throws -> T) rethrows -> T { - if #available(macOS 14.0, iOS 17.0, *) { - return try assumeIsolated(operation, file: #fileID, line: #line) - } - dispatchPrecondition(condition: .onQueue(.main)) - return try OnMainActor.instance().perform(operation) - } -} -#else - #warning("This needs to be removed as it‘s no longer necessary.") -#endif