From b0c4c0ae69084e171c352e2c32b05e47c80b2ede Mon Sep 17 00:00:00 2001 From: Matt Lichtenstein Date: Wed, 1 Nov 2023 19:31:04 -0400 Subject: [PATCH] VPN-5532: Prevent persistent VPN config system prompt (#8430) --- .../org/mozilla/firefox/vpn/qt/VPNActivity.java | 6 ++++++ src/connectionmanager.cpp | 13 ++++++++++++- src/connectionmanager.h | 1 + src/platforms/android/androidcontroller.cpp | 9 +++++++++ src/platforms/android/androidvpnactivity.cpp | 3 +++ src/platforms/android/androidvpnactivity.h | 6 ++++++ src/platforms/ios/ioscontroller.mm | 4 ++++ src/platforms/ios/ioscontroller.swift | 11 ++++++++--- 8 files changed, 49 insertions(+), 4 deletions(-) diff --git a/android/vpnClient/src/main/java/org/mozilla/firefox/vpn/qt/VPNActivity.java b/android/vpnClient/src/main/java/org/mozilla/firefox/vpn/qt/VPNActivity.java index 735f25e5bc..ed16d23640 100644 --- a/android/vpnClient/src/main/java/org/mozilla/firefox/vpn/qt/VPNActivity.java +++ b/android/vpnClient/src/main/java/org/mozilla/firefox/vpn/qt/VPNActivity.java @@ -163,6 +163,7 @@ private void initServiceConnection(){ private final int EVENT_PERMISSION_REQURED = 6; private final int EVENT_DISCONNECTED = 2; private final int EVENT_ONBOARDING_COMPLETED = 9; + private final int EVENT_VPN_CONFIG_PERMISSION_RESPONSE = 10; public void onPermissionRequest(int code, Parcel data) { if(code != EVENT_PERMISSION_REQURED){ @@ -183,6 +184,11 @@ public void onNotificationPermissionRequest() { protected void onActivityResult(int requestCode, int resultCode, Intent data) { if(requestCode == PERMISSION_TRANSACTION){ // THATS US! + + // At this point, the user has made a selection on the system config permission modal to either allow or not allow + // the vpn configuration to be created, so it is safe to run activation retries via ConnectionManager::startHandshakeTimer() + // without the possibility or re-prompting (flickering) the modal while it is currently being displayed + onServiceMessage(EVENT_VPN_CONFIG_PERMISSION_RESPONSE,""); if( resultCode == RESULT_OK ){ // Prompt accepted, tell service to retry. dispatchParcel(ACTION_RESUME_ACTIVATE,""); diff --git a/src/connectionmanager.cpp b/src/connectionmanager.cpp index 9dcfd2b1e5..ef06d233c9 100644 --- a/src/connectionmanager.cpp +++ b/src/connectionmanager.cpp @@ -262,6 +262,10 @@ void ConnectionManager::deleteOSTunnelConfig() { } } +void ConnectionManager::startHandshakeTimer() { + m_handshakeTimer.start(HANDSHAKE_TIMEOUT_SEC * 1000); +} + void ConnectionManager::handshakeTimeout() { logger.debug() << "Timeout while waiting for handshake"; @@ -571,7 +575,14 @@ void ConnectionManager::activateNext() { const InterfaceConfig& config = m_activationQueue.first(); logger.debug() << "Activating peer" << logger.keys(config.m_serverPublicKey); - m_handshakeTimer.start(HANDSHAKE_TIMEOUT_SEC * 1000); + +// Mobile platforms will begin handshake timer once we know the VPN +// configuration has been set with the system (to avoid persistently +// re-triggering the system prompt - VPN-5532) +#if defined(MZ_LINUX) || defined(MZ_WINDOWS) || defined(MZ_MACOS) + startHandshakeTimer(); +#endif + m_impl->activate(config, stateToReason(m_state)); // Move to the confirming state if we are awaiting any connection handshakes. diff --git a/src/connectionmanager.h b/src/connectionmanager.h index f394e225fa..eaf9f5cecc 100644 --- a/src/connectionmanager.h +++ b/src/connectionmanager.h @@ -78,6 +78,7 @@ class ConnectionManager : public QObject, public LogSerializer { void backendFailure(); void updateRequired(); void deleteOSTunnelConfig(); + void startHandshakeTimer(); const ServerData& currentServer() const { return m_serverData; } diff --git a/src/platforms/android/androidcontroller.cpp b/src/platforms/android/androidcontroller.cpp index e1aad38e06..659ecd819b 100644 --- a/src/platforms/android/androidcontroller.cpp +++ b/src/platforms/android/androidcontroller.cpp @@ -14,6 +14,7 @@ #include "androidutils.h" #include "androidvpnactivity.h" +#include "connectionmanager.h" #include "errorhandler.h" #include "feature.h" #include "i18nstrings.h" @@ -110,6 +111,14 @@ AndroidController::AndroidController() { } }, Qt::QueuedConnection); + connect( + activity, &AndroidVPNActivity::eventVpnConfigPermissionResponse, this, + []() { + ConnectionManager* connectionManager = + MozillaVPN::instance()->connectionManager(); + connectionManager->startHandshakeTimer(); + }, + Qt::QueuedConnection); } AndroidController::~AndroidController() { MZ_COUNT_DTOR(AndroidController); } diff --git a/src/platforms/android/androidvpnactivity.cpp b/src/platforms/android/androidvpnactivity.cpp index ff308a821f..11a45175e0 100644 --- a/src/platforms/android/androidvpnactivity.cpp +++ b/src/platforms/android/androidvpnactivity.cpp @@ -142,6 +142,9 @@ void AndroidVPNActivity::handleServiceMessage(int code, const QString& data) { case ServiceEvents::EVENT_ONBOARDING_COMPLETED: emit eventOnboardingCompleted(); break; + case ServiceEvents::EVENT_VPN_CONFIG_PERMISSION_RESPONSE: + emit eventVpnConfigPermissionResponse(); + break; default: Q_ASSERT(false); } diff --git a/src/platforms/android/androidvpnactivity.h b/src/platforms/android/androidvpnactivity.h index 6f40c15c11..66c958c225 100644 --- a/src/platforms/android/androidvpnactivity.h +++ b/src/platforms/android/androidvpnactivity.h @@ -72,7 +72,12 @@ enum ServiceEvents { // The Daemon need's the app to ask for notification // permissions, to show the "you're connected" messages. EVENT_REQUEST_NOTIFICATION_PERMISSION = 8, + // Signals MozillaVPN that we have completed onboarding EVENT_ONBOARDING_COMPLETED = 9, + // Signals the ConnectionManager that it may now allow + // activation retries now that the system vpn config modal + // has been responded to + EVENT_VPN_CONFIG_PERMISSION_RESPONSE = 10, }; typedef enum ServiceEvents ServiceEvents; @@ -96,6 +101,7 @@ class AndroidVPNActivity : public QObject { void eventStatisticUpdate(const QString& data); void eventActivationError(const QString& data); void eventOnboardingCompleted(); + void eventVpnConfigPermissionResponse(); void eventRequestGleanUploadEnabledState(); private: diff --git a/src/platforms/ios/ioscontroller.mm b/src/platforms/ios/ioscontroller.mm index f26cb49439..8a5701337e 100644 --- a/src/platforms/ios/ioscontroller.mm +++ b/src/platforms/ios/ioscontroller.mm @@ -160,6 +160,10 @@ } onboardingCompletedCallback:^() { MozillaVPN::instance()->onboardingCompleted(); + } + vpnConfigPermissionResponseCallback:^() { + ConnectionManager* connectionManager = MozillaVPN::instance()->connectionManager(); + connectionManager->startHandshakeTimer(); }]; } diff --git a/src/platforms/ios/ioscontroller.swift b/src/platforms/ios/ioscontroller.swift index d22b47214e..9f3561577b 100644 --- a/src/platforms/ios/ioscontroller.swift +++ b/src/platforms/ios/ioscontroller.swift @@ -116,7 +116,7 @@ public class IOSControllerImpl : NSObject { } } - @objc func connect(dnsServer: String, serverIpv6Gateway: String, serverPublicKey: String, serverIpv4AddrIn: String, serverPort: Int, allowedIPAddressRanges: Array, reason: Int, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) { + @objc func connect(dnsServer: String, serverIpv6Gateway: String, serverPublicKey: String, serverIpv4AddrIn: String, serverPort: Int, allowedIPAddressRanges: Array, reason: Int, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void, vpnConfigPermissionResponseCallback: @escaping () -> Void) { IOSControllerImpl.logger.debug(message: "Connecting") TunnelManager.withTunnel { tunnel in @@ -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, disconnectCallback: disconnectCallback, onboardingCompletedCallback: onboardingCompletedCallback) + return self.configureTunnel(config: config, reason: reason, serverName: serverIpv4AddrIn + ":\(serverPort )", gleanDebugTag: gleanDebugTag, isSuperDooperFeatureActive: isSuperDooperFeatureActive, installationId: installationId, isOnboarding: isOnboarding, disconnectCallback: disconnectCallback, onboardingCompletedCallback: onboardingCompletedCallback, vpnConfigPermissionResponseCallback: vpnConfigPermissionResponseCallback) } } - func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @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, vpnConfigPermissionResponseCallback: @escaping () -> Void) { TunnelManager.withTunnel { tunnel in let proto = NETunnelProviderProtocol(tunnelConfiguration: config) proto!.providerBundleIdentifier = TunnelManager.vpnBundleId @@ -185,6 +185,11 @@ public class IOSControllerImpl : NSObject { tunnel.isEnabled = true return tunnel.saveToPreferences { saveError in + // At this point, the user has made a selection on the system config permission modal to either allow or not allow + // the vpn configuration to be created, so it is safe to run activation retries via ConnectionManager::startHandshakeTimer() + // without the possibility or re-prompting (flickering) the modal while it is currently being displayed + vpnConfigPermissionResponseCallback() + if let error = saveError { IOSControllerImpl.logger.error(message: "Connect Tunnel Save Error: \(error)") disconnectCallback()