Skip to content

Commit

Permalink
Update VPN active user check and debug options (#2269)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1206165514403612/f
Tech Design URL:
CC:

Description:

This PR updates the active user pixel check to accurately reflect the conditions under which you need to meet to be counted as a potential user. The old check was looking for the active flag only, which isn't accurate as it includes non-US users.
  • Loading branch information
samsymons authored Dec 16, 2023
1 parent 2eb7baa commit 9c26ac1
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 25 deletions.
17 changes: 9 additions & 8 deletions DuckDuckGo/AppDelegate+Waitlists.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,21 @@ extension AppDelegate {

#if NETWORK_PROTECTION
private func checkNetworkProtectionWaitlist() {
if AppDependencyProvider.shared.featureFlagger.isFeatureOn(.networkProtectionWaitlistAccess) {
let accessController = NetworkProtectionAccessController()
if accessController.isPotentialOrCurrentWaitlistUser {
DailyPixel.fire(pixel: .networkProtectionWaitlistUserActive)
}

VPNWaitlist.shared.fetchInviteCodeIfAvailable { [weak self] error in
guard error == nil else {
#if !DEBUG
// If the user already has an invite code but their auth token has gone missing, attempt to redeem it again.
let tokenStore = NetworkProtectionKeychainTokenStore()
let waitlistStorage = VPNWaitlist.shared.waitlistStorage
if error == .alreadyHasInviteCode,
let inviteCode = waitlistStorage.getWaitlistInviteCode(),
!tokenStore.isFeatureActivated {
self?.fetchVPNWaitlistAuthToken(inviteCode: inviteCode)
if error == .alreadyHasInviteCode {
// If the user already has an invite code but their auth token has gone missing, attempt to redeem it again.
let tokenStore = NetworkProtectionKeychainTokenStore()
let waitlistStorage = VPNWaitlist.shared.waitlistStorage
if let inviteCode = waitlistStorage.getWaitlistInviteCode(), !tokenStore.isFeatureActivated {
self?.fetchVPNWaitlistAuthToken(inviteCode: inviteCode)
}
}
#endif
return
Expand Down
26 changes: 19 additions & 7 deletions DuckDuckGo/NetworkProtectionAccessController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ struct NetworkProtectionAccessController: NetworkProtectionAccess {
return (regionCode ?? "US") == "US"
}

var isPotentialOrCurrentWaitlistUser: Bool {
switch self.networkProtectionAccessType() {
case .none, .inviteCodeInvited:
return false
case .waitlistAvailable, .waitlistJoined, .waitlistInvitedPendingTermsAcceptance, .waitlistInvited:
return true
}
}

init(
networkProtectionActivation: NetworkProtectionFeatureActivation = NetworkProtectionKeychainTokenStore(),
Expand Down Expand Up @@ -130,14 +138,18 @@ struct NetworkProtectionAccessController: NetworkProtectionAccess {
}

if !featureFlagger.isFeatureOn(.networkProtectionWaitlistActive) {
networkProtectionWaitlistStorage.deleteWaitlistState()
try? NetworkProtectionKeychainTokenStore().deleteToken()
revokeNetworkProtectionAccess()
}
}

Task {
let controller = NetworkProtectionTunnelController()
await controller.stop()
await controller.removeVPN()
}
func revokeNetworkProtectionAccess() {
networkProtectionWaitlistStorage.deleteWaitlistState()
try? NetworkProtectionKeychainTokenStore().deleteToken()

Task {
let controller = NetworkProtectionTunnelController()
await controller.stop()
await controller.removeVPN()
}
}

Expand Down
28 changes: 19 additions & 9 deletions DuckDuckGo/NetworkProtectionDebugViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import NetworkProtection
// swiftlint:disable:next type_body_length
final class NetworkProtectionDebugViewController: UITableViewController {
private let titles = [
Sections.keychain: "Keychain",
Sections.clearData: "Clear Data",
Sections.debugFeature: "Debug Features",
Sections.simulateFailure: "Simulate Failure",
Sections.registrationKey: "Registration Key",
Expand All @@ -49,7 +49,7 @@ final class NetworkProtectionDebugViewController: UITableViewController {
]

enum Sections: Int, CaseIterable {
case keychain
case clearData
case debugFeature
case simulateFailure
case registrationKey
Expand All @@ -59,9 +59,10 @@ final class NetworkProtectionDebugViewController: UITableViewController {
case vpnConfiguration
}

enum KeychainRows: Int, CaseIterable {
enum ClearDataRows: Int, CaseIterable {

case clearAuthToken
case clearAllVPNData

}

Expand Down Expand Up @@ -162,10 +163,12 @@ final class NetworkProtectionDebugViewController: UITableViewController {

switch Sections(rawValue: indexPath.section) {

case .keychain:
switch KeychainRows(rawValue: indexPath.row) {
case .clearData:
switch ClearDataRows(rawValue: indexPath.row) {
case .clearAuthToken:
cell.textLabel?.text = "Clear auth token"
cell.textLabel?.text = "Clear Auth Token"
case .clearAllVPNData:
cell.textLabel?.text = "Reset VPN State Completely"
case .none:
break
}
Expand Down Expand Up @@ -200,7 +203,7 @@ final class NetworkProtectionDebugViewController: UITableViewController {

override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
switch Sections(rawValue: section) {
case .keychain: return KeychainRows.allCases.count
case .clearData: return ClearDataRows.allCases.count
case .debugFeature: return DebugFeatureRows.allCases.count
case .simulateFailure: return SimulateFailureRows.allCases.count
case .registrationKey: return RegistrationKeyRows.allCases.count
Expand All @@ -216,9 +219,10 @@ final class NetworkProtectionDebugViewController: UITableViewController {
// swiftlint:disable:next cyclomatic_complexity
override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
switch Sections(rawValue: indexPath.section) {
case .keychain:
switch KeychainRows(rawValue: indexPath.row) {
case .clearData:
switch ClearDataRows(rawValue: indexPath.row) {
case .clearAuthToken: clearAuthToken()
case .clearAllVPNData: clearAllVPNData()
default: break
}
case .debugFeature:
Expand Down Expand Up @@ -542,6 +546,12 @@ final class NetworkProtectionDebugViewController: UITableViewController {
private func clearAuthToken() {
try? tokenStore.deleteToken()
}

private func clearAllVPNData() {
let accessController = NetworkProtectionAccessController()
accessController.revokeNetworkProtectionAccess()
}

}


Expand Down
41 changes: 40 additions & 1 deletion DuckDuckGoTests/NetworkProtectionAccessControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,43 @@ final class NetworkProtectionAccessControllerTests: XCTestCase {
XCTAssertEqual(controller.networkProtectionAccessType(), .waitlistInvited)
}

func testWhenUserHasWaitlistAccess_ThenWaitlistUserCheckIsTrue() {
let controller = createMockAccessController(
isInternalUser: true,
featureActivated: true,
termsAccepted: true,
featureFlagsEnabled: true,
hasJoinedWaitlist: true,
hasBeenInvited: true
)

XCTAssertTrue(controller.isPotentialOrCurrentWaitlistUser)
}

func testWhenUserDoesNotHaveWaitlistAccess_ThenWaitlistUserCheckIsFalse() {
let controller = createMockAccessController(
featureActivated: false,
termsAccepted: false,
featureFlagsEnabled: false,
hasJoinedWaitlist: false,
hasBeenInvited: false
)

XCTAssertFalse(controller.isPotentialOrCurrentWaitlistUser)
}

func testWhenUserIsInternal_ThenWaitlistUserCheckIsFalse() {
let controller = createMockAccessController(
featureActivated: true,
termsAccepted: false,
featureFlagsEnabled: false,
hasJoinedWaitlist: false,
hasBeenInvited: false
)

XCTAssertFalse(controller.isPotentialOrCurrentWaitlistUser)
}

// MARK: - Mock Creation

private func createMockAccessController(
Expand Down Expand Up @@ -146,12 +183,14 @@ final class NetworkProtectionAccessControllerTests: XCTestCase {
let mockTermsAndConditionsStore = MockNetworkProtectionTermsAndConditionsStore()
mockTermsAndConditionsStore.networkProtectionWaitlistTermsAndConditionsAccepted = termsAccepted
let mockFeatureFlagger = createFeatureFlagger(withSubfeatureEnabled: featureFlagsEnabled)
let internalUserDecider = DefaultInternalUserDecider(store: internalUserDeciderStore)

return NetworkProtectionAccessController(
networkProtectionActivation: mockActivation,
networkProtectionWaitlistStorage: mockWaitlistStorage,
networkProtectionTermsAndConditionsStore: mockTermsAndConditionsStore,
featureFlagger: mockFeatureFlagger
featureFlagger: mockFeatureFlagger,
internalUserDecider: internalUserDecider
)
}

Expand Down

0 comments on commit 9c26ac1

Please sign in to comment.