Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VPN-5688: Prevent VPN activation when completing onboarding #8276

Merged
merged 3 commits into from
Oct 18, 2023
Merged

VPN-5688: Prevent VPN activation when completing onboarding #8276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4e61c0
Prevent VPN activation retries during onboarding
MattLichtenstein Oct 12, 2023
40e263d
prevent disconnect telemetry during onboarding
MattLichtenstein Oct 13, 2023
cf37a35
need to load the tunnel (#8320)
mcleinman Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/connectionmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,9 @@ void ConnectionManager::resetConnectedTime() {
void ConnectionManager::disconnected() {
logger.debug() << "Disconnected from state:" << m_state;

m_pingCanary.stop();
m_handshakeTimer.stop();
m_activationQueue.clear();
clearConnectedTime();
clearRetryCounter();

Expand Down
3 changes: 2 additions & 1 deletion src/platforms/android/androidcontroller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,11 @@ AndroidController::AndroidController() {
Qt::QueuedConnection);
connect(
activity, &AndroidVPNActivity::eventOnboardingCompleted, this,
[]() {
[this]() {
auto vpn = MozillaVPN::instance();
if (vpn->state() == App::StateOnboarding) {
vpn->onboardingCompleted();
emit disconnected();
}
},
Qt::QueuedConnection);
Expand Down
4 changes: 2 additions & 2 deletions src/platforms/ios/ioscontroller.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@
isSuperDooperFeatureActive:Feature::get(Feature::Feature_superDooperMetrics)->isSupported()
installationId:config.m_installationId.toNSString()
isOnboarding:MozillaVPN::instance()->state() == App::StateOnboarding
failureCallback:^() {
logger.error() << "IOSSWiftController - connection failed";
disconnectCallback:^() {
logger.error() << "IOSSWiftController - disconnecting";
emit disconnected();
}
onboardingCompletedCallback:^() {
Expand Down
31 changes: 16 additions & 15 deletions src/platforms/ios/ioscontroller.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public class IOSControllerImpl : NSObject {
}
}

@objc func connect(dnsServer: String, serverIpv6Gateway: String, serverPublicKey: String, serverIpv4AddrIn: String, serverPort: Int, allowedIPAddressRanges: Array<VPNIPAddressRange>, reason: Int, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, failureCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
@objc func connect(dnsServer: String, serverIpv6Gateway: String, serverPublicKey: String, serverIpv4AddrIn: String, serverPort: Int, allowedIPAddressRanges: Array<VPNIPAddressRange>, reason: Int, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
IOSControllerImpl.logger.debug(message: "Connecting")

let _ = TunnelManager.withTunnel { tunnel in
Expand Down Expand Up @@ -152,11 +152,11 @@ public class IOSControllerImpl : NSObject {

let config = TunnelConfiguration(name: VPN_NAME, interface: interface, peers: peerConfigurations)

return self.configureTunnel(config: config, reason: reason, serverName: serverIpv4AddrIn + ":\(serverPort )", gleanDebugTag: gleanDebugTag, isSuperDooperFeatureActive: isSuperDooperFeatureActive, installationId: installationId, isOnboarding: isOnboarding, failureCallback: failureCallback, onboardingCompletedCallback: onboardingCompletedCallback)
return self.configureTunnel(config: config, reason: reason, serverName: serverIpv4AddrIn + ":\(serverPort )", gleanDebugTag: gleanDebugTag, isSuperDooperFeatureActive: isSuperDooperFeatureActive, installationId: installationId, isOnboarding: isOnboarding, disconnectCallback: disconnectCallback, onboardingCompletedCallback: onboardingCompletedCallback)
}
}

func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, failureCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
let _ = TunnelManager.withTunnel { tunnel in
let proto = NETunnelProviderProtocol(tunnelConfiguration: config)
proto!.providerBundleIdentifier = TunnelManager.vpnBundleId
Expand Down Expand Up @@ -187,31 +187,32 @@ public class IOSControllerImpl : NSObject {
return tunnel.saveToPreferences { saveError in
if let error = saveError {
IOSControllerImpl.logger.error(message: "Connect Tunnel Save Error: \(error)")
failureCallback()
disconnectCallback()
if isOnboarding {
IOSControllerImpl.logger.info(message: "Finishing onboarding, but saving tunnel resulted in error...")
onboardingCompletedCallback()
}
return
}

IOSControllerImpl.logger.info(message: "Saving the tunnel succeeded")

//Used to create a VPN configuration without connecting during onboarding
if isOnboarding {
IOSControllerImpl.logger.info(message: "Finishing onboarding... do not turn on the VPN after gaining permission")
onboardingCompletedCallback()
return
}
IOSControllerImpl.logger.info(message: "Saving the tunnel succeeded")

tunnel.loadFromPreferences { error in
if let error = error {
IOSControllerImpl.logger.error(message: "Connect Tunnel Load Error: \(error)")
failureCallback()
disconnectCallback()
return
}

IOSControllerImpl.logger.info(message: "Loading the tunnel succeeded")
IOSControllerImpl.logger.info(message: "Loading the tunnel succeeded")

// Used to create a VPN configuration without connecting during onboarding
if isOnboarding {
IOSControllerImpl.logger.info(message: "Finishing onboarding... do not turn on the VPN after gaining permission")
disconnectCallback()
onboardingCompletedCallback()
return
}

do {
if (reason == 1 /* ReasonSwitching */) {
Expand All @@ -224,7 +225,7 @@ public class IOSControllerImpl : NSObject {
}
} catch let error {
IOSControllerImpl.logger.error(message: "Something went wrong: \(error)")
failureCallback()
disconnectCallback()
return
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/telemetry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ void Telemetry::initialize() {
connectionManager, &ConnectionManager::controllerDisconnected, this,
[this, connectionManager]() {
if (Feature::get(Feature::Feature_superDooperMetrics)->isSupported()) {
if (connectionManager->state() == ConnectionManager::StateOff) {
if (connectionManager->state() == ConnectionManager::StateOff &&
SettingsHolder::instance()->onboardingCompleted()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you - I was worried this "don't log telemetry" was going to be waaaay harder than that. Relatedly, I just learned that there is no :sweatsmile: emoji in GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅

mozilla::glean::session::session_end.set();

mozilla::glean_pings::Vpnsession.submit("end");
Expand Down
Loading