From 98b7117405d95b005e1211b4decbc71664388e7f Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 18 Dec 2023 12:12:17 +0100 Subject: [PATCH 1/5] Change to exponential retry delay --- mullvad-daemon/src/geoip.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mullvad-daemon/src/geoip.rs b/mullvad-daemon/src/geoip.rs index 02ee09d1c0d7..0af495e8bd83 100644 --- a/mullvad-daemon/src/geoip.rs +++ b/mullvad-daemon/src/geoip.rs @@ -8,12 +8,9 @@ use mullvad_api::{ }; use mullvad_types::location::{AmIMullvad, GeoIpLocation}; use once_cell::sync::Lazy; -use talpid_core::future_retry::{retry_future, ConstantInterval}; +use talpid_core::future_retry::{retry_future, ExponentialBackoff, Jittered}; use talpid_types::ErrorExt; -/// Retry interval for fetching location -const RETRY_LOCATION_STRATEGY: ConstantInterval = ConstantInterval::new(Duration::ZERO, Some(3)); - // Define the Mullvad connection checking api endpoint. // // In a development build the host name for the connection checking endpoint can @@ -43,6 +40,9 @@ static MULLVAD_CONNCHECK_HOST: Lazy = Lazy::new(|| { host.to_string() }); +const LOCATION_RETRY_STRATEGY: Jittered = + Jittered::jitter(ExponentialBackoff::new(Duration::from_secs(1), 4)); + /// Fetch the current `GeoIpLocation` with retrys pub async fn get_geo_location( rest_service: RequestServiceHandle, @@ -56,7 +56,7 @@ pub async fn get_geo_location( Err(error) if error.is_network_error() => !api_handle.get_state().is_offline(), _ => false, }, - RETRY_LOCATION_STRATEGY, + LOCATION_RETRY_STRATEGY, ) .await { From 72a22cf7d00dffdc99e4b2ae26a713416a2ed735 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 18 Dec 2023 12:12:17 +0100 Subject: [PATCH 2/5] Remove `GetCurrentLocation`. Make the daemon send two tunnel state updates, one with out IP being empty, and another with it being filled when am.i.mullvad.net responds. Update CLI for this change. Other front ends are left out. --- CHANGELOG.md | 7 + mullvad-cli/src/cmds/status.rs | 63 +++------ mullvad-cli/src/format.rs | 25 +++- mullvad-daemon/src/geoip.rs | 13 +- mullvad-daemon/src/lib.rs | 129 ++++++++++-------- mullvad-daemon/src/management_interface.rs | 11 -- .../proto/management_interface.proto | 3 +- mullvad-management-interface/src/client.rs | 25 +--- .../src/types/conversions/states.rs | 14 +- mullvad-types/src/states.rs | 4 +- test/test-manager/src/tests/helpers.rs | 2 +- test/test-manager/src/tests/settings.rs | 3 +- test/test-manager/src/tests/tunnel_state.rs | 8 +- 13 files changed, 146 insertions(+), 161 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02927b8545b5..69ee65f28ddd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,14 @@ Line wrap the file at 100 chars. Th * **Security**: in case of vulnerabilities. ## [Unreleased] +### Fixed +#### Linux +- Out IP missing forever when am.i.mullvad.net returns error + ### Changed +- Remove `--location` flag from `mullvad status` CLI. Location and IP will now always + be printed (if available). `mullvad status listen` does no longer print location info. + #### Android - Migrated to Compose Navigation - Allow for full rotation diff --git a/mullvad-cli/src/cmds/status.rs b/mullvad-cli/src/cmds/status.rs index a4a8add15e5d..f8338685167b 100644 --- a/mullvad-cli/src/cmds/status.rs +++ b/mullvad-cli/src/cmds/status.rs @@ -18,10 +18,7 @@ pub struct StatusArgs { #[arg(long, short = 'v')] verbose: bool, - /// Print the current location and IP, based on GeoIP lookups - #[arg(long, short = 'l')] - location: bool, - + // TODO: changelog about removing location flag /// Enable debug output #[arg(long, short = 'd')] debug: bool, @@ -29,22 +26,29 @@ pub struct StatusArgs { impl Status { pub async fn listen(mut rpc: MullvadProxyClient, args: StatusArgs) -> Result<()> { + let mut previous_tunnel_state = None; + while let Some(event) = rpc.events_listen().await?.next().await { match event? { DaemonEvent::TunnelState(new_state) => { if args.debug { println!("New tunnel state: {new_state:#?}"); } else { - format::print_state(&new_state, args.verbose); - } - - match new_state { - TunnelState::Connected { .. } | TunnelState::Disconnected => { - if args.location { - print_location(&mut rpc).await?; - } + // When we enter the connected or disconnected state, am.i.mullvad.net will + // be polled to get IP information. When it arrives, we will get another + // tunnel state of the same enum type, but with the IP filled in. This + // match statement checks for duplicate tunnel states and skips the second + // print to avoid spamming the user. + match (&previous_tunnel_state, &new_state) { + (Some(TunnelState::Disconnected(_)), TunnelState::Disconnected(_)) + | ( + Some(TunnelState::Connected { .. }), + TunnelState::Connected { .. }, + ) => continue, + _ => {} } - _ => {} + format::print_state(&new_state, args.verbose); + previous_tunnel_state = Some(new_state); } } DaemonEvent::Settings(settings) => { @@ -88,11 +92,9 @@ pub async fn handle(cmd: Option, args: StatusArgs) -> Result<()> { if args.debug { println!("Tunnel state: {state:#?}"); } else { + // TODO: respect location arg? format::print_state(&state, args.verbose); - } - - if args.location { - print_location(&mut rpc).await?; + format::print_location(&state); } if cmd == Some(Status::Listen) { @@ -101,31 +103,6 @@ pub async fn handle(cmd: Option, args: StatusArgs) -> Result<()> { Ok(()) } -async fn print_location(rpc: &mut MullvadProxyClient) -> Result<()> { - let location = match rpc.get_current_location().await { - Ok(location) => location, - Err(error) => match &error { - mullvad_management_interface::Error::NoLocationData => { - println!("Location data unavailable"); - return Ok(()); - } - _ => return Err(error.into()), - }, - }; - if let Some(ipv4) = location.ipv4 { - println!("IPv4: {ipv4}"); - } - if let Some(ipv6) = location.ipv6 { - println!("IPv6: {ipv6}"); - } - - println!( - "Position: {:.5}°N, {:.5}°W", - location.latitude, location.longitude - ); - Ok(()) -} - fn print_account_loggedout(state: &TunnelState, device: &DeviceState) { match state { TunnelState::Connecting { .. } | TunnelState::Connected { .. } | TunnelState::Error(_) => { @@ -137,6 +114,6 @@ fn print_account_loggedout(state: &TunnelState, device: &DeviceState) { DeviceState::LoggedIn(_) => (), } } - TunnelState::Disconnected | TunnelState::Disconnecting(_) => (), + TunnelState::Disconnected(_) | TunnelState::Disconnecting(_) => (), } } diff --git a/mullvad-cli/src/format.rs b/mullvad-cli/src/format.rs index 2938842cefcf..0e920b621826 100644 --- a/mullvad-cli/src/format.rs +++ b/mullvad-cli/src/format.rs @@ -37,11 +37,34 @@ pub fn print_state(state: &TunnelState, verbose: bool) { format_relay_connection(endpoint, location.as_ref(), verbose) ); } - Disconnected => println!("Disconnected"), + Disconnected(_) => { + println!("Disconnected"); + } Disconnecting(_) => println!("Disconnecting..."), } } +pub fn print_location(state: &TunnelState) { + let location = match state { + TunnelState::Disconnected(location) => location, + TunnelState::Connected { location, .. } => location, + _ => return, + }; + if let Some(location) = location { + print!("Your connection appears from: {}", location.country); + if let Some(city) = &location.city { + print!(", {}", city); + } + if let Some(ipv4) = location.ipv4 { + print!(". IPv4: {ipv4}"); + } + if let Some(ipv6) = location.ipv6 { + print!(", IPv6: {ipv6}"); + } + println!(); + } +} + fn format_relay_connection( endpoint: &TunnelEndpoint, location: Option<&GeoIpLocation>, diff --git a/mullvad-daemon/src/geoip.rs b/mullvad-daemon/src/geoip.rs index 0af495e8bd83..b4ed662fa1e1 100644 --- a/mullvad-daemon/src/geoip.rs +++ b/mullvad-daemon/src/geoip.rs @@ -44,13 +44,13 @@ const LOCATION_RETRY_STRATEGY: Jittered = Jittered::jitter(ExponentialBackoff::new(Duration::from_secs(1), 4)); /// Fetch the current `GeoIpLocation` with retrys -pub async fn get_geo_location( +pub async fn get_geo_location_with_retry( rest_service: RequestServiceHandle, use_ipv6: bool, api_handle: ApiAvailabilityHandle, -) -> Option { +) -> Result { log::debug!("Fetching GeoIpLocation"); - match retry_future( + retry_future( move || send_location_request(rest_service.clone(), use_ipv6), move |result| match result { Err(error) if error.is_network_error() => !api_handle.get_state().is_offline(), @@ -59,13 +59,6 @@ pub async fn get_geo_location( LOCATION_RETRY_STRATEGY, ) .await - { - Ok(loc) => Some(loc), - Err(e) => { - log::warn!("Unable to fetch GeoIP location: {}", e.display_chain()); - None - } - } } async fn send_location_request( diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 58069db4fa85..6c78b3452de2 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -27,7 +27,7 @@ mod tunnel; pub mod version; mod version_check; -use crate::{geoip::get_geo_location, target_state::PersistentTargetState}; +use crate::target_state::PersistentTargetState; use device::{AccountEvent, PrivateAccountAndDevice, PrivateDeviceEvent}; use futures::{ channel::{mpsc, oneshot}, @@ -203,8 +203,6 @@ pub enum DaemonCommand { Reconnect(oneshot::Sender), /// Request the current state. GetState(oneshot::Sender), - /// Get the current geographical location. - GetCurrentLocation(oneshot::Sender>), CreateNewAccount(ResponseTx), /// Request the metadata for an account. GetAccountData( @@ -420,7 +418,7 @@ impl DaemonExecutionState { match self { Running => { match tunnel_state { - TunnelState::Disconnected => mem::replace(self, Finished), + TunnelState::Disconnected(_) => mem::replace(self, Finished), _ => mem::replace(self, Exiting), }; } @@ -617,6 +615,7 @@ pub struct Daemon { tunnel_state_machine_handle: TunnelStateMachineHandle, #[cfg(target_os = "windows")] volume_update_tx: mpsc::UnboundedSender<()>, + location_abort_handle: Option, } impl Daemon @@ -831,7 +830,7 @@ where relay_list_updater.update().await; let daemon = Daemon { - tunnel_state: TunnelState::Disconnected, + tunnel_state: TunnelState::Disconnected(None), target_state, state: DaemonExecutionState::Running, #[cfg(target_os = "linux")] @@ -857,6 +856,7 @@ where tunnel_state_machine_handle, #[cfg(target_os = "windows")] volume_update_tx, + location_abort_handle: None, }; api_availability.unsuspend(); @@ -960,15 +960,26 @@ where .handle_state_transition(&tunnel_state_transition); let tunnel_state = match tunnel_state_transition { - TunnelStateTransition::Disconnected => TunnelState::Disconnected, + TunnelStateTransition::Disconnected => { + self.notify_tunnel_state_when_ip_arrives(None, TunnelState::Disconnected) + .await; + TunnelState::Disconnected(None) + } TunnelStateTransition::Connecting(endpoint) => TunnelState::Connecting { endpoint, location: self.parameters_generator.get_last_location().await, }, - TunnelStateTransition::Connected(endpoint) => TunnelState::Connected { - endpoint, - location: self.parameters_generator.get_last_location().await, - }, + TunnelStateTransition::Connected(endpoint) => { + let location = self.parameters_generator.get_last_location().await; + let make_tunnel_state = |location| TunnelState::Connected { endpoint, location }; + self.notify_tunnel_state_when_ip_arrives( + location.clone(), + make_tunnel_state.clone(), + ) + .await; + + make_tunnel_state(location) + } TunnelStateTransition::Disconnecting(after_disconnect) => { TunnelState::Disconnecting(after_disconnect) } @@ -985,7 +996,7 @@ where log::debug!("New tunnel state: {:?}", tunnel_state); match tunnel_state { - TunnelState::Disconnected => { + TunnelState::Disconnected(_) => { self.api_handle.availability.reset_inactivity_timer(); } _ => { @@ -994,7 +1005,7 @@ where } match &tunnel_state { - TunnelState::Disconnected => self.state.disconnected(), + TunnelState::Disconnected(_) => self.state.disconnected(), TunnelState::Connecting { .. } => { log::debug!("Settings: {}", self.settings.summary()); } @@ -1024,6 +1035,53 @@ where self.event_listener.notify_new_state(tunnel_state); } + /// Get the out IP from am.i.mullvad.net. When it arrives, send another + /// TunnelState with the IP filled in. + async fn notify_tunnel_state_when_ip_arrives( + &mut self, + old_location: Option, + make_tunnel_state: impl FnOnce(Option) -> TunnelState + Send + 'static, + ) { + // Abort any ongoing calls to `self.notify_tunnel_state_when_ip_arrives` from + // previous tunnel state transitions to avoid sending an outdated information. + if let Some(handle) = self.location_abort_handle.take() { + handle.abort(); + } + let rest_service = self.api_runtime.rest_handle().await; + let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6; + let api_handle = self.api_handle.availability.clone(); + let even_listener = self.event_listener.clone(); + + self.location_abort_handle = Some( + tokio::spawn(async move { + let merged_location = + geoip::get_geo_location_with_retry(rest_service, use_ipv6, api_handle) + .await + .ok() + // Replace the old location with new information from am.i.mullvad.net, + // but keep the hostname as it is always none + .map(|new_location| match old_location { + Some(old_location) => GeoIpLocation { + hostname: old_location.hostname, + bridge_hostname: old_location.bridge_hostname, + entry_hostname: old_location.entry_hostname, + obfuscator_hostname: old_location.obfuscator_hostname, + ..new_location + }, + None => GeoIpLocation { + hostname: None, + bridge_hostname: None, + entry_hostname: None, + obfuscator_hostname: None, + ..new_location + }, + }); + even_listener.notify_new_state(make_tunnel_state(merged_location)); + }) + .abort_handle(), + ); + } + fn reset_rpc_sockets_on_tunnel_state_transition( &mut self, tunnel_state_transition: &TunnelStateTransition, @@ -1075,7 +1133,6 @@ where SetTargetState(tx, state) => self.on_set_target_state(tx, state).await, Reconnect(tx) => self.on_reconnect(tx), GetState(tx) => self.on_get_state(tx), - GetCurrentLocation(tx) => self.on_get_current_location(tx).await, CreateNewAccount(tx) => self.on_create_new_account(tx), GetAccountData(tx, account_token) => self.on_get_account_data(tx, account_token), GetWwwAuthToken(tx) => self.on_get_www_auth_token(tx).await, @@ -1326,52 +1383,6 @@ where Self::oneshot_send(tx, performing_post_upgrade, "performing post upgrade"); } - async fn on_get_current_location(&mut self, tx: oneshot::Sender>) { - use self::TunnelState::*; - - match &self.tunnel_state { - Disconnected => self.update_and_send_geo_location(tx, None).await, - Connecting { location, .. } => { - Self::oneshot_send(tx, location.clone(), "current location") - } - Disconnecting(..) => Self::oneshot_send( - tx, - self.parameters_generator.get_last_location().await, - "current location", - ), - Connected { location, .. } => { - self.update_and_send_geo_location(tx, location.clone()) - .await - } - // We are not online at all at this stage so no location data is available. - Error(_) => Self::oneshot_send(tx, None, "current location"), - }; - } - - /// Fetch the current `GeoIpLocation` and send it on the return channel, - /// in a non-blocking fashion. Optionally give a chached previous location. - async fn update_and_send_geo_location( - &mut self, - tx: oneshot::Sender>, - current_relay_location: Option, - ) { - let rest_service = self.api_runtime.rest_handle().await; - let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6; - let api_handle = self.api_handle.availability.clone(); - tokio::spawn(async move { - let new_location = get_geo_location(rest_service, use_ipv6, api_handle).await; - Self::oneshot_send( - tx, - new_location.map(|fetched_location| GeoIpLocation { - ipv4: fetched_location.ipv4, - ipv6: fetched_location.ipv6, - ..current_relay_location.unwrap_or(fetched_location) - }), - "current location", - ); - }); - } - fn on_create_new_account(&mut self, tx: ResponseTx) { let account_manager = self.account_manager.clone(); tokio::spawn(async move { diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index c194825a3438..cf007e665d37 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -191,17 +191,6 @@ impl ManagementService for ManagementServiceImpl { .map(|relays| Response::new(types::RelayList::from(relays))) } - async fn get_current_location(&self, _: Request<()>) -> ServiceResult { - log::debug!("get_current_location"); - let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::GetCurrentLocation(tx))?; - let result = self.wait_for_result(rx).await?; - match result { - Some(geoip) => Ok(Response::new(types::GeoIpLocation::from(geoip))), - None => Err(Status::not_found("no location was found")), - } - } - async fn set_bridge_settings( &self, request: Request, diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index d66707b79f7f..97507ca8a2c6 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -28,7 +28,6 @@ service ManagementService { // Relays and tunnel constraints rpc UpdateRelayLocations(google.protobuf.Empty) returns (google.protobuf.Empty) {} rpc GetRelayLocations(google.protobuf.Empty) returns (RelayList) {} - rpc GetCurrentLocation(google.protobuf.Empty) returns (GeoIpLocation) {} rpc SetRelaySettings(RelaySettings) returns (google.protobuf.Empty) {} rpc SetBridgeSettings(BridgeSettings) returns (google.protobuf.Empty) {} rpc SetBridgeState(BridgeState) returns (google.protobuf.Empty) {} @@ -172,7 +171,7 @@ message ErrorState { } message TunnelState { - message Disconnected {} + message Disconnected { GeoIpLocation disconnected_location = 1; } message Connecting { TunnelStateRelayInfo relay_info = 1; } message Connected { TunnelStateRelayInfo relay_info = 1; } message Disconnecting { AfterDisconnect after_disconnect = 1; } diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs index 1c9d80b2e8f6..85048e29e75e 100644 --- a/mullvad-management-interface/src/client.rs +++ b/mullvad-management-interface/src/client.rs @@ -7,7 +7,6 @@ use mullvad_types::{ account::{AccountData, AccountToken, VoucherSubmission}, custom_list::{CustomList, Id}, device::{Device, DeviceEvent, DeviceId, DeviceState, RemoveDeviceEvent}, - location::GeoIpLocation, relay_constraints::{ BridgeSettings, BridgeState, ObfuscationSettings, RelayOverride, RelaySettings, }, @@ -234,16 +233,6 @@ impl MullvadProxyClient { Ok(()) } - pub async fn get_current_location(&mut self) -> Result { - let location = self - .0 - .get_current_location(()) - .await - .map_err(map_location_error)? - .into_inner(); - GeoIpLocation::try_from(location).map_err(Error::InvalidResponse) - } - pub async fn set_bridge_settings(&mut self, settings: BridgeSettings) -> Result<()> { let settings = types::BridgeSettings::from(settings); self.0 @@ -571,10 +560,9 @@ impl MullvadProxyClient { /// Set the [`AccessMethod`] which [`ApiConnectionModeProvider`] should /// pick. /// - /// - `access_method`: If `Some(access_method)`, [`ApiConnectionModeProvider`] will skip - /// ahead and return `access_method` when asked for a new access method. - /// If `None`, [`ApiConnectionModeProvider`] will pick the next access - /// method "randomly" + /// - `access_method`: If `Some(access_method)`, [`ApiConnectionModeProvider`] will skip ahead + /// and return `access_method` when asked for a new access method. If `None`, + /// [`ApiConnectionModeProvider`] will pick the next access method "randomly" /// /// [`ApiConnectionModeProvider`]: mullvad_daemon::api::ApiConnectionModeProvider pub async fn set_access_method(&mut self, api_access_method: access_method::Id) -> Result<()> { @@ -696,13 +684,6 @@ fn map_device_error(status: Status) -> Error { } } -fn map_location_error(status: Status) -> Error { - match status.code() { - Code::NotFound => Error::NoLocationData, - _other => Error::Rpc(status), - } -} - fn map_custom_list_error(status: Status) -> Error { match status.code() { Code::NotFound => { diff --git a/mullvad-management-interface/src/types/conversions/states.rs b/mullvad-management-interface/src/types/conversions/states.rs index b742aa7b004c..2541859e2745 100644 --- a/mullvad-management-interface/src/types/conversions/states.rs +++ b/mullvad-management-interface/src/types/conversions/states.rs @@ -32,8 +32,10 @@ impl From for proto::TunnelState { }; let state = match state { - MullvadTunnelState::Disconnected => { - proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected {}) + MullvadTunnelState::Disconnected(disconnected_location) => { + proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected { + disconnected_location: disconnected_location.map(proto::GeoIpLocation::from), + }) } MullvadTunnelState::Connecting { endpoint, location } => { proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting { @@ -189,7 +191,13 @@ impl TryFrom for mullvad_types::states::TunnelState { use talpid_types::{net as talpid_net, tunnel as talpid_tunnel}; let state = match state.state { - Some(proto::tunnel_state::State::Disconnected(_)) => MullvadState::Disconnected, + Some(proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected { + disconnected_location, + })) => MullvadState::Disconnected( + disconnected_location + .map(mullvad_types::location::GeoIpLocation::try_from) + .transpose()?, + ), Some(proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting { relay_info: Some(proto::TunnelStateRelayInfo { diff --git a/mullvad-types/src/states.rs b/mullvad-types/src/states.rs index f6131711cbcd..4a266d1d6e90 100644 --- a/mullvad-types/src/states.rs +++ b/mullvad-types/src/states.rs @@ -34,7 +34,7 @@ impl fmt::Display for TargetState { #[cfg_attr(target_os = "android", derive(IntoJava))] #[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] pub enum TunnelState { - Disconnected, + Disconnected(Option), Connecting { endpoint: TunnelEndpoint, location: Option, @@ -60,6 +60,6 @@ impl TunnelState { /// Returns true if the tunnel state is in the disconnected state. pub fn is_disconnected(&self) -> bool { - matches!(self, TunnelState::Disconnected) + matches!(self, TunnelState::Disconnected(_)) } } diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs index 48dba20aac65..3f64aef1bee1 100644 --- a/test/test-manager/src/tests/helpers.rs +++ b/test/test-manager/src/tests/helpers.rs @@ -229,7 +229,7 @@ pub async fn disconnect_and_wait( .await .map_err(|error| Error::DaemonError(format!("failed to begin disconnecting: {}", error)))?; wait_for_tunnel_state(mullvad_client.clone(), |state| { - matches!(state, TunnelState::Disconnected) + matches!(state, TunnelState::Disconnected(_)) }) .await?; diff --git a/test/test-manager/src/tests/settings.rs b/test/test-manager/src/tests/settings.rs index aea81028fe38..d2bde57fcc1e 100644 --- a/test/test-manager/src/tests/settings.rs +++ b/test/test-manager/src/tests/settings.rs @@ -107,9 +107,8 @@ pub async fn test_lockdown( let inet_destination: SocketAddr = "1.1.1.1:1337".parse().unwrap(); log::info!("Verify tunnel state: disconnected"); - assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected); + assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected(_)); - // // Enable lockdown mode // mullvad_client diff --git a/test/test-manager/src/tests/tunnel_state.rs b/test/test-manager/src/tests/tunnel_state.rs index 9ae817c7ee9d..d8cce41b3a1b 100644 --- a/test/test-manager/src/tests/tunnel_state.rs +++ b/test/test-manager/src/tests/tunnel_state.rs @@ -32,9 +32,8 @@ pub async fn test_disconnected_state( let inet_destination = "1.3.3.7:1337".parse().unwrap(); log::info!("Verify tunnel state: disconnected"); - assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected); + assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected(_)); - // // Test whether outgoing packets can be observed // @@ -90,7 +89,7 @@ pub async fn test_connecting_state( let lan_dns: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 53); log::info!("Verify tunnel state: disconnected"); - assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected); + assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected(_)); let relay_settings = RelaySettings::CustomTunnelEndpoint(CustomTunnelEndpoint { host: "1.3.3.7".to_owned(), @@ -172,9 +171,8 @@ pub async fn test_error_state( let lan_dns: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 53); log::info!("Verify tunnel state: disconnected"); - assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected); + assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected(_)); - // // Connect to non-existent location // From a9e75d44832865eff883f1efdb914e261de27cd5 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 18 Dec 2023 12:12:17 +0100 Subject: [PATCH 3/5] Handle am.i.mullvad.net with internal daemon event Add `geoip::GeoIpHandler`, which sends an `InternalDaemonEvent::LocationEvent` when the location arrives. It also handles aborting in-flight requests and retries. --- CHANGELOG.md | 2 +- mullvad-cli/src/cmds/status.rs | 4 +- mullvad-cli/src/format.rs | 2 +- mullvad-daemon/src/geoip.rs | 66 ++++++++++-- mullvad-daemon/src/lib.rs | 149 ++++++++++++++++------------ mullvad-jni/src/daemon_interface.rs | 1 - mullvad-types/src/location.rs | 7 ++ 7 files changed, 156 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69ee65f28ddd..e248d5de6a60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ Line wrap the file at 100 chars. Th ### Changed - Remove `--location` flag from `mullvad status` CLI. Location and IP will now always - be printed (if available). `mullvad status listen` does no longer print location info. + be printed (if available). `mullvad status listen` no longer prints location info. #### Android - Migrated to Compose Navigation diff --git a/mullvad-cli/src/cmds/status.rs b/mullvad-cli/src/cmds/status.rs index f8338685167b..5bed82b4c057 100644 --- a/mullvad-cli/src/cmds/status.rs +++ b/mullvad-cli/src/cmds/status.rs @@ -18,7 +18,6 @@ pub struct StatusArgs { #[arg(long, short = 'v')] verbose: bool, - // TODO: changelog about removing location flag /// Enable debug output #[arg(long, short = 'd')] debug: bool, @@ -36,7 +35,7 @@ impl Status { } else { // When we enter the connected or disconnected state, am.i.mullvad.net will // be polled to get IP information. When it arrives, we will get another - // tunnel state of the same enum type, but with the IP filled in. This + // tunnel state of the same enum type, but with the IP filled in. This // match statement checks for duplicate tunnel states and skips the second // print to avoid spamming the user. match (&previous_tunnel_state, &new_state) { @@ -92,7 +91,6 @@ pub async fn handle(cmd: Option, args: StatusArgs) -> Result<()> { if args.debug { println!("Tunnel state: {state:#?}"); } else { - // TODO: respect location arg? format::print_state(&state, args.verbose); format::print_location(&state); } diff --git a/mullvad-cli/src/format.rs b/mullvad-cli/src/format.rs index 0e920b621826..512b632dc873 100644 --- a/mullvad-cli/src/format.rs +++ b/mullvad-cli/src/format.rs @@ -51,7 +51,7 @@ pub fn print_location(state: &TunnelState) { _ => return, }; if let Some(location) = location { - print!("Your connection appears from: {}", location.country); + print!("Your connection appears to be from: {}", location.country); if let Some(city) = &location.city { print!(", {}", city); } diff --git a/mullvad-daemon/src/geoip.rs b/mullvad-daemon/src/geoip.rs index b4ed662fa1e1..3874e206b86a 100644 --- a/mullvad-daemon/src/geoip.rs +++ b/mullvad-daemon/src/geoip.rs @@ -3,14 +3,18 @@ use std::time::Duration; use futures::join; use mullvad_api::{ self, - availability::ApiAvailabilityHandle, rest::{Error, RequestServiceHandle}, }; -use mullvad_types::location::{AmIMullvad, GeoIpLocation}; +use mullvad_types::location::{AmIMullvad, GeoIpLocation, LocationEventData}; use once_cell::sync::Lazy; -use talpid_core::future_retry::{retry_future, ExponentialBackoff, Jittered}; +use talpid_core::{ + future_retry::{retry_future, ExponentialBackoff, Jittered}, + mpsc::Sender, +}; use talpid_types::ErrorExt; +use crate::{DaemonEventSender, InternalDaemonEvent}; + // Define the Mullvad connection checking api endpoint. // // In a development build the host name for the connection checking endpoint can @@ -43,17 +47,64 @@ static MULLVAD_CONNCHECK_HOST: Lazy = Lazy::new(|| { const LOCATION_RETRY_STRATEGY: Jittered = Jittered::jitter(ExponentialBackoff::new(Duration::from_secs(1), 4)); -/// Fetch the current `GeoIpLocation` with retrys -pub async fn get_geo_location_with_retry( +/// Handler for request to am.i.mullvad.net, manages in-flight request and validity of responses. +pub(crate) struct GeoIpHandler { + /// Unique ID for each request. If the ID attached to the + /// [`InternalDaemonEvent::LocationEvent`] used by [`crate::Daemon::handle_location_event`] to + /// determine if the location belongs to the current tunnel state. + pub request_id: usize, rest_service: RequestServiceHandle, + location_sender: DaemonEventSender, +} + +impl GeoIpHandler { + pub fn new(rest_service: RequestServiceHandle, location_sender: DaemonEventSender) -> Self { + Self { + request_id: 0, + rest_service, + location_sender, + } + } + + /// Send a location request to am.i.mullvad.net. When it arrives, send an + /// [`InternalDaemonEvent::LocationEvent`], which triggers an update of the current + /// tunnel state with the `ipv4` and/or `ipv6` fields filled in. + pub fn send_geo_location_request(&mut self, use_ipv6: bool) { + // Increment request ID + self.request_id = self.request_id.wrapping_add(1); + + self.abort_current_request(); + + let request_id = self.request_id; + let rest_service = self.rest_service.clone(); + let location_sender = self.location_sender.clone(); + tokio::spawn(async move { + if let Ok(location) = get_geo_location_with_retry(use_ipv6, rest_service).await { + let _ = + location_sender.send(InternalDaemonEvent::LocationEvent(LocationEventData { + request_id, + location, + })); + } + }); + } + + /// Abort any ongoing call to am.i.mullvad.net + pub fn abort_current_request(&mut self) { + self.rest_service.reset(); + } +} + +/// Fetch the current `GeoIpLocation` from am.i.mullvad.net. Handles retries on network errors. +async fn get_geo_location_with_retry( use_ipv6: bool, - api_handle: ApiAvailabilityHandle, + rest_service: RequestServiceHandle, ) -> Result { log::debug!("Fetching GeoIpLocation"); retry_future( move || send_location_request(rest_service.clone(), use_ipv6), move |result| match result { - Err(error) if error.is_network_error() => !api_handle.get_state().is_offline(), + Err(error) => error.is_network_error(), _ => false, }, LOCATION_RETRY_STRATEGY, @@ -76,6 +127,7 @@ async fn send_location_request( if use_ipv6 { let uri_v6 = format!("https://ipv6.{}/json", *MULLVAD_CONNCHECK_HOST); let location = send_location_request_internal(&uri_v6, v6_sender).await; + log::warn!("{location:?}"); Some(location.map(GeoIpLocation::from)) } else { None diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 6c78b3452de2..19e1d02dfb44 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -34,6 +34,7 @@ use futures::{ future::{abortable, AbortHandle, Future, LocalBoxFuture}, StreamExt, }; +use geoip::GeoIpHandler; use mullvad_relay_selector::{ updater::{RelayListUpdater, RelayListUpdaterHandle}, RelaySelector, SelectorConfig, @@ -46,7 +47,7 @@ use mullvad_types::{ auth_failed::AuthFailed, custom_list::CustomList, device::{Device, DeviceEvent, DeviceEventCause, DeviceId, DeviceState, RemoveDeviceEvent}, - location::GeoIpLocation, + location::{GeoIpLocation, LocationEventData}, relay_constraints::{ BridgeSettings, BridgeState, ObfuscationSettings, RelayOverride, RelaySettings, }, @@ -80,7 +81,7 @@ use talpid_types::android::AndroidContext; #[cfg(target_os = "windows")] use talpid_types::split_tunnel::ExcludedProcess; use talpid_types::{ - net::{TunnelEndpoint, TunnelType}, + net::{IpVersion, TunnelEndpoint, TunnelType}, tunnel::{ErrorStateCause, TunnelStateTransition}, ErrorExt, }; @@ -369,6 +370,8 @@ pub(crate) enum InternalDaemonEvent { DeviceEvent(AccountEvent), /// Handles updates from versions without devices. DeviceMigrationEvent(Result), + /// A geographical location has has been received from am.i.mullvad.net + LocationEvent(LocationEventData), /// The split tunnel paths or state were updated. #[cfg(target_os = "windows")] ExcludedPathsEvent(ExcludedPathsUpdate, oneshot::Sender>), @@ -615,7 +618,7 @@ pub struct Daemon { tunnel_state_machine_handle: TunnelStateMachineHandle, #[cfg(target_os = "windows")] volume_update_tx: mpsc::UnboundedSender<()>, - location_abort_handle: Option, + location_handler: GeoIpHandler, } impl Daemon @@ -829,6 +832,11 @@ where // Attempt to download a fresh relay list relay_list_updater.update().await; + let location_handler = GeoIpHandler::new( + api_runtime.rest_handle().await, + internal_event_tx.clone().to_specialized_sender(), + ); + let daemon = Daemon { tunnel_state: TunnelState::Disconnected(None), target_state, @@ -856,7 +864,7 @@ where tunnel_state_machine_handle, #[cfg(target_os = "windows")] volume_update_tx, - location_abort_handle: None, + location_handler, }; api_availability.unsuspend(); @@ -867,8 +875,16 @@ where /// Consume the `Daemon` and run the main event loop. Blocks until an error happens or a /// shutdown event is received. pub async fn run(mut self) -> Result<(), Error> { - if *self.target_state == TargetState::Secured { - self.connect_tunnel(); + match *self.target_state { + TargetState::Secured => { + self.connect_tunnel(); + } + TargetState::Unsecured => { + // Fetching GeoIpLocation is automatically done when connecting. + // If TargetState is Unsecured we will not connect on lauch and + // so we have to explicitly fetch this information. + self.fetch_am_i_mullvad() + } } while let Some(event) = self.rx.next().await { @@ -946,6 +962,7 @@ where } DeviceEvent(event) => self.handle_device_event(event).await, DeviceMigrationEvent(event) => self.handle_device_migration_event(event), + LocationEvent(location_data) => self.handle_location_event(location_data), #[cfg(windows)] ExcludedPathsEvent(update, tx) => self.handle_new_excluded_paths(update, tx).await, } @@ -960,25 +977,14 @@ where .handle_state_transition(&tunnel_state_transition); let tunnel_state = match tunnel_state_transition { - TunnelStateTransition::Disconnected => { - self.notify_tunnel_state_when_ip_arrives(None, TunnelState::Disconnected) - .await; - TunnelState::Disconnected(None) - } + TunnelStateTransition::Disconnected => TunnelState::Disconnected(None), TunnelStateTransition::Connecting(endpoint) => TunnelState::Connecting { endpoint, location: self.parameters_generator.get_last_location().await, }, TunnelStateTransition::Connected(endpoint) => { let location = self.parameters_generator.get_last_location().await; - let make_tunnel_state = |location| TunnelState::Connected { endpoint, location }; - self.notify_tunnel_state_when_ip_arrives( - location.clone(), - make_tunnel_state.clone(), - ) - .await; - - make_tunnel_state(location) + TunnelState::Connected { endpoint, location } } TunnelStateTransition::Disconnecting(after_disconnect) => { TunnelState::Disconnecting(after_disconnect) @@ -1033,53 +1039,72 @@ where self.tunnel_state = tunnel_state.clone(); self.event_listener.notify_new_state(tunnel_state); + self.fetch_am_i_mullvad(); + } + + /// Get the geographical location from am.i.mullvad.net. When it arrives, + /// update the "Out IP" field of the front ends by sending a + /// [`InternalDaemonEvent::LocationEvent`]. + /// + /// See [`Daemon::handle_location_event()`] + fn fetch_am_i_mullvad(&mut self) { + // Always abort any ongoing request when entering a new tunnel state + self.location_handler.abort_current_request(); + + // Whether or not to poll for an IPv6 exit IP + let use_ipv6 = match &self.tunnel_state { + // If connected, refer to the tunnel setting + TunnelState::Connected { .. } => self.settings.tunnel_options.generic.enable_ipv6, + // If not connected, we have to guess whether the users local connection supports IPv6. + // The only thing we have to go on is the wireguard setting. + TunnelState::Disconnected(_) => { + if let RelaySettings::Normal(relay_constraints) = &self.settings.relay_settings { + // Note that `Constraint::Any` corresponds to just IPv4 + matches!( + relay_constraints.wireguard_constraints.ip_version, + mullvad_types::relay_constraints::Constraint::Only(IpVersion::V6) + ) + } else { + false + } + } + // Fetching IP from am.i.mullvad.net should only be done from a tunnel state where a + // connection is available. Otherwise we just exist. + _ => return, + }; + + self.location_handler.send_geo_location_request(use_ipv6); } - /// Get the out IP from am.i.mullvad.net. When it arrives, send another - /// TunnelState with the IP filled in. - async fn notify_tunnel_state_when_ip_arrives( - &mut self, - old_location: Option, - make_tunnel_state: impl FnOnce(Option) -> TunnelState + Send + 'static, - ) { - // Abort any ongoing calls to `self.notify_tunnel_state_when_ip_arrives` from - // previous tunnel state transitions to avoid sending an outdated information. - if let Some(handle) = self.location_abort_handle.take() { - handle.abort(); + /// Recieves and handles the geographical exit location received from am.i.mullvad.net, i.e. the + /// [`InternalDaemonEvent::LocationEvent`] event. + fn handle_location_event(&mut self, location_data: LocationEventData) { + let LocationEventData { + request_id, + location: fetched_location, + } = location_data; + + if self.location_handler.request_id != request_id { + log::debug!("Location from am.i.mullvad.net belongs to an outdated tunnel state"); + return; } - let rest_service = self.api_runtime.rest_handle().await; - let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6; - let api_handle = self.api_handle.availability.clone(); - let even_listener = self.event_listener.clone(); - self.location_abort_handle = Some( - tokio::spawn(async move { - let merged_location = - geoip::get_geo_location_with_retry(rest_service, use_ipv6, api_handle) - .await - .ok() - // Replace the old location with new information from am.i.mullvad.net, - // but keep the hostname as it is always none - .map(|new_location| match old_location { - Some(old_location) => GeoIpLocation { - hostname: old_location.hostname, - bridge_hostname: old_location.bridge_hostname, - entry_hostname: old_location.entry_hostname, - obfuscator_hostname: old_location.obfuscator_hostname, - ..new_location - }, - None => GeoIpLocation { - hostname: None, - bridge_hostname: None, - entry_hostname: None, - obfuscator_hostname: None, - ..new_location - }, - }); - even_listener.notify_new_state(make_tunnel_state(merged_location)); - }) - .abort_handle(), - ); + match self.tunnel_state { + TunnelState::Disconnected(ref mut location) => *location = Some(fetched_location), + TunnelState::Connected { + ref mut location, .. + } => { + *location = Some(GeoIpLocation { + ipv4: fetched_location.ipv4, + ipv6: fetched_location.ipv6, + ..location.clone().unwrap_or(fetched_location) + }) + } + _ => return, + }; + + self.event_listener + .notify_new_state(self.tunnel_state.clone()); } fn reset_rpc_sockets_on_tunnel_state_transition( diff --git a/mullvad-jni/src/daemon_interface.rs b/mullvad-jni/src/daemon_interface.rs index 236c864a01e9..ff00f4a825b8 100644 --- a/mullvad-jni/src/daemon_interface.rs +++ b/mullvad-jni/src/daemon_interface.rs @@ -3,7 +3,6 @@ use mullvad_daemon::{device, DaemonCommand, DaemonCommandSender}; use mullvad_types::{ account::{AccountData, AccountToken, PlayPurchase, VoucherSubmission}, device::{Device, DeviceState}, - location::GeoIpLocation, relay_constraints::{ObfuscationSettings, RelaySettings}, relay_list::RelayList, settings::{DnsOptions, Settings}, diff --git a/mullvad-types/src/location.rs b/mullvad-types/src/location.rs index 6453d1771783..07e2b1e90ce3 100644 --- a/mullvad-types/src/location.rs +++ b/mullvad-types/src/location.rs @@ -181,6 +181,13 @@ impl From for GeoIpLocation { } } +pub struct LocationEventData { + /// Keep track of which request led to this event being triggered + pub request_id: usize, + /// New location information + pub location: GeoIpLocation, +} + #[cfg(test)] mod tests { use super::Coordinates; From c70509a345b6db8caf12c880b3ee3bf4c70bf79d Mon Sep 17 00:00:00 2001 From: Oskar Nyberg Date: Mon, 18 Dec 2023 12:12:17 +0100 Subject: [PATCH 4/5] Remove location fetch in Electron app --- gui/src/main/daemon-rpc.ts | 11 +++---- gui/src/main/index.ts | 2 -- gui/src/main/tunnel-state.ts | 19 +++++++++-- gui/src/renderer/app.tsx | 51 ++++++------------------------ gui/src/shared/daemon-rpc-types.ts | 4 +-- gui/src/shared/ipc-schema.ts | 4 --- gui/test/e2e/setup/main.ts | 4 +-- 7 files changed, 33 insertions(+), 62 deletions(-) diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts index 174b2aea77ca..ad7b60dec202 100644 --- a/gui/src/main/daemon-rpc.ts +++ b/gui/src/main/daemon-rpc.ts @@ -33,7 +33,6 @@ import { IDevice, IDeviceRemoval, IDnsOptions, - ILocation, IObfuscationEndpoint, IOpenVpnConstraints, IProxyEndpoint, @@ -434,11 +433,6 @@ export class DaemonRpc { await this.callEmpty(this.client.reconnectTunnel); } - public async getLocation(): Promise { - const response = await this.callEmpty(this.client.getCurrentLocation); - return response.toObject(); - } - public async getState(): Promise { const response = await this.callEmpty(this.client.getTunnelState); return convertFromTunnelState(response)!; @@ -851,7 +845,10 @@ function convertFromTunnelState(tunnelState: grpcTypes.TunnelState): TunnelState case grpcTypes.TunnelState.StateCase.STATE_NOT_SET: return undefined; case grpcTypes.TunnelState.StateCase.DISCONNECTED: - return { state: 'disconnected' }; + return { + state: 'disconnected', + location: tunnelStateObject.disconnected!.disconnectedLocation, + }; case grpcTypes.TunnelState.StateCase.DISCONNECTING: { const detailsMap: Record = { [grpcTypes.AfterDisconnect.NOTHING]: 'nothing', diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts index da3d677cc0fc..6c3a6373fb04 100644 --- a/gui/src/main/index.ts +++ b/gui/src/main/index.ts @@ -728,8 +728,6 @@ class ApplicationMain navigationHistory: this.navigationHistory, })); - IpcMainEventChannel.location.handleGet(() => this.daemonRpc.getLocation()); - IpcMainEventChannel.tunnel.handleConnect(this.connectTunnel); IpcMainEventChannel.tunnel.handleReconnect(this.reconnectTunnel); IpcMainEventChannel.tunnel.handleDisconnect(this.disconnectTunnel); diff --git a/gui/src/main/tunnel-state.ts b/gui/src/main/tunnel-state.ts index 029386f3b792..682911881865 100644 --- a/gui/src/main/tunnel-state.ts +++ b/gui/src/main/tunnel-state.ts @@ -1,5 +1,5 @@ import { connectEnabled, disconnectEnabled, reconnectEnabled } from '../shared/connect-helper'; -import { TunnelState } from '../shared/daemon-rpc-types'; +import { ILocation, TunnelState } from '../shared/daemon-rpc-types'; import { Scheduler } from '../shared/scheduler'; export interface TunnelStateProvider { @@ -20,6 +20,8 @@ export default class TunnelStateHandler { // Scheduler for discarding the assumed next state. private tunnelStateFallbackScheduler = new Scheduler(); + private lastKnownDisconnectedLocation: Partial | undefined; + public constructor(private delegate: TunnelStateHandlerDelegate) {} public get tunnelState() { @@ -37,7 +39,9 @@ export default class TunnelStateHandler { this.tunnelStateFallback = this.tunnelState; this.setTunnelState( - state === 'disconnecting' ? { state, details: 'nothing' as const } : { state }, + state === 'disconnecting' + ? { state, details: 'nothing' as const, location: this.lastKnownDisconnectedLocation } + : { state }, ); this.tunnelStateFallbackScheduler.schedule(() => { @@ -67,6 +71,17 @@ export default class TunnelStateHandler { this.expectNextTunnelState('connecting'); this.tunnelStateFallback = newState; } else { + if (newState.state === 'disconnected' && newState.location !== undefined) { + this.lastKnownDisconnectedLocation = newState.location; + } + + if ( + newState.state === 'disconnecting' || + (newState.state === 'disconnected' && newState.location === undefined) + ) { + newState.location = this.lastKnownDisconnectedLocation; + } + this.setTunnelState(newState); } } diff --git a/gui/src/renderer/app.tsx b/gui/src/renderer/app.tsx index 584e80ed4f4b..b5eacf17b36b 100644 --- a/gui/src/renderer/app.tsx +++ b/gui/src/renderer/app.tsx @@ -94,7 +94,6 @@ export default class AppRenderer { }; private location?: Partial; - private lastDisconnectedLocation?: Partial; private relayList?: IRelayListWithEndpointData; private tunnelState!: TunnelState; private settings!: ISettings; @@ -103,7 +102,6 @@ export default class AppRenderer { private previousLoginState: LoginState = 'none'; private loginScheduler = new Scheduler(); private connectedToDaemon = false; - private getLocationPromise?: Promise; constructor() { log.addOutput(new ConsoleOutput(LogLevel.debug)); @@ -255,7 +253,7 @@ export default class AppRenderer { ); } - void this.updateLocation(); + this.updateLocation(); if (initialState.navigationHistory) { // Set last action to POP to trigger automatic scrolling to saved coordinates. @@ -765,7 +763,7 @@ export default class AppRenderer { } // Update the location when entering a new tunnel state since it's likely changed. - void this.updateLocation(); + this.updateLocation(); }); } @@ -944,22 +942,16 @@ export default class AppRenderer { this.reduxActions.userInterface.setForceShowChanges(forceShowChanges); } - private async updateLocation() { + private updateLocation() { switch (this.tunnelState.state) { - case 'disconnected': { - if (this.lastDisconnectedLocation) { - this.setLocation(this.lastDisconnectedLocation); - } - const location = await this.fetchLocation(); - if (location) { - this.setLocation(location); - this.lastDisconnectedLocation = location; + case 'disconnected': + if (this.tunnelState.location) { + this.setLocation(this.tunnelState.location); } break; - } case 'disconnecting': - if (this.lastDisconnectedLocation) { - this.setLocation(this.lastDisconnectedLocation); + if (this.tunnelState.location) { + this.setLocation(this.tunnelState.location); } else { // If there's no previous location while disconnecting we remove the location. We keep the // coordinates to prevent the map from jumping around. @@ -968,38 +960,13 @@ export default class AppRenderer { } break; case 'connecting': - this.setLocation(this.tunnelState.details?.location ?? this.getLocationFromConstraints()); - break; case 'connected': { - if (this.tunnelState.details?.location) { - this.setLocation(this.tunnelState.details.location); - } - const location = await this.fetchLocation(); - if (location) { - this.setLocation(location); - } + this.setLocation(this.tunnelState.details?.location ?? this.getLocationFromConstraints()); break; } } } - private async fetchLocation(): Promise { - try { - // Fetch the new user location - const getLocationPromise = IpcRendererEventChannel.location.get(); - this.getLocationPromise = getLocationPromise; - const location = await getLocationPromise; - // If the location is currently unavailable, do nothing! This only ever happens when a - // custom relay is set or we are in a blocked state. - if (location && getLocationPromise === this.getLocationPromise) { - return location; - } - } catch (e) { - const error = e as Error; - log.error(`Failed to update the location: ${error.message}`); - } - } - private getLocationFromConstraints(): Partial { const state = this.reduxStore.getState(); const coordinates = { diff --git a/gui/src/shared/daemon-rpc-types.ts b/gui/src/shared/daemon-rpc-types.ts index 6977be437520..e83f9e5afc2a 100644 --- a/gui/src/shared/daemon-rpc-types.ts +++ b/gui/src/shared/daemon-rpc-types.ts @@ -181,10 +181,10 @@ export interface ITunnelStateRelayInfo { } export type TunnelState = - | { state: 'disconnected' } + | { state: 'disconnected'; location?: Partial } | { state: 'connecting'; details?: ITunnelStateRelayInfo } | { state: 'connected'; details: ITunnelStateRelayInfo } - | { state: 'disconnecting'; details: AfterDisconnect } + | { state: 'disconnecting'; details: AfterDisconnect; location?: Partial } | { state: 'error'; details: ErrorState }; export interface RelayLocationCountry extends Partial { diff --git a/gui/src/shared/ipc-schema.ts b/gui/src/shared/ipc-schema.ts index 994bacdcd66a..ecf34e93fb1e 100644 --- a/gui/src/shared/ipc-schema.ts +++ b/gui/src/shared/ipc-schema.ts @@ -15,7 +15,6 @@ import { IDevice, IDeviceRemoval, IDnsOptions, - ILocation, IRelayListWithEndpointData, ISettings, ObfuscationSettings, @@ -153,9 +152,6 @@ export const ipcSchema = { showOpenDialog: invoke(), showLaunchDaemonSettings: invoke(), }, - location: { - get: invoke(), - }, tunnel: { '': notifyRenderer(), connect: invoke(), diff --git a/gui/test/e2e/setup/main.ts b/gui/test/e2e/setup/main.ts index 430dfdb70d56..e645e8b7946f 100644 --- a/gui/test/e2e/setup/main.ts +++ b/gui/test/e2e/setup/main.ts @@ -153,7 +153,7 @@ class ApplicationMain { autoStart: false, accountData: this.accountData, accountHistory: undefined, - tunnelState: { state: 'disconnected' }, + tunnelState: { state: 'disconnected', location: this.location }, settings: this.settings, isPerformingPostUpgrade: false, deviceState: this.deviceState, @@ -174,8 +174,6 @@ class ApplicationMain { scrollPositions: {}, })); - IpcMainEventChannel.location.handleGet(() => Promise.resolve(this.location)); - IpcMainEventChannel.guiSettings.handleSetPreferredLocale((locale) => { this.updateCurrentLocale(locale); IpcMainEventChannel.guiSettings.notify?.(this.guiSettings); From 8ed53a012b6c510a0dfd92925b8b0e742fd79da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Mon, 18 Dec 2023 12:12:17 +0100 Subject: [PATCH 5/5] Support new tunnel state API in the Android frontend. Remove `get_current_location` from jni. --- .../compose/screen/ConnectScreenTest.kt | 72 ++------- .../compose/component/ConnectionStatusText.kt | 2 +- .../compose/screen/ConnectScreen.kt | 11 +- .../compose/screen/OutOfTimeScreen.kt | 2 +- .../compose/state/ConnectUiState.kt | 6 +- .../compose/state/OutOfTimeUiState.kt | 2 +- .../compose/state/WelcomeUiState.kt | 2 +- .../ui/serviceconnection/ConnectionProxy.kt | 4 +- .../ui/serviceconnection/LocationInfoCache.kt | 26 ---- .../ServiceConnectionContainer.kt | 2 - .../usecase/TunnelStateNotificationUseCase.kt | 2 +- .../mullvadvpn/viewmodel/ConnectViewModel.kt | 44 +++--- .../usecase/OutOfTimeUseCaseTest.kt | 2 +- .../TunnelStateNotificationUseCaseTest.kt | 2 +- .../viewmodel/ConnectViewModelTest.kt | 49 ++---- .../viewmodel/DeviceRevokedViewModelTest.kt | 2 +- .../viewmodel/OutOfTimeViewModelTest.kt | 3 +- .../viewmodel/WelcomeViewModelTest.kt | 2 +- .../net/mullvad/mullvadvpn/lib/ipc/Event.kt | 3 - .../mullvad/mullvadvpn/model/TunnelState.kt | 72 ++------- .../mullvadvpn/service/MullvadDaemon.kt | 11 +- .../mullvadvpn/service/MullvadVpnService.kt | 2 +- .../service/endpoint/ConnectionProxy.kt | 2 +- .../service/endpoint/LocationInfoCache.kt | 139 ------------------ .../service/endpoint/ServiceEndpoint.kt | 3 - .../notifications/TunnelStateNotification.kt | 2 +- .../service/util/ExponentialBackoff.kt | 52 ------- .../mullvadvpn/tile/ServiceConnection.kt | 2 +- mullvad-jni/src/daemon_interface.rs | 8 - mullvad-jni/src/lib.rs | 28 ---- 30 files changed, 82 insertions(+), 477 deletions(-) delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/LocationInfoCache.kt delete mode 100644 android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/LocationInfoCache.kt delete mode 100644 android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/util/ExponentialBackoff.kt diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt index dae3bf2ed3eb..3838bdc7a0f2 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt @@ -79,7 +79,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.TunnelStateBlocked, @@ -115,7 +114,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.TunnelStateBlocked, @@ -149,7 +147,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -182,7 +179,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -216,7 +212,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = true, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -245,12 +240,11 @@ class ConnectScreenTest { ConnectUiState( location = null, relayLocation = mockRelayLocation, - tunnelUiState = TunnelState.Disconnected, - tunnelRealState = TunnelState.Disconnected, + tunnelUiState = TunnelState.Disconnected(), + tunnelRealState = TunnelState.Disconnected(), inAddress = null, outAddress = "", showLocation = true, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -286,7 +280,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = true, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = @@ -326,7 +319,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = true, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = @@ -363,7 +355,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.TunnelStateBlocked, @@ -399,7 +390,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = true, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.TunnelStateBlocked, @@ -430,12 +420,11 @@ class ConnectScreenTest { ConnectUiState( location = null, relayLocation = mockRelayLocation, - tunnelUiState = TunnelState.Disconnected, - tunnelRealState = TunnelState.Disconnected, + tunnelUiState = TunnelState.Disconnected(), + tunnelRealState = TunnelState.Disconnected(), inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -468,7 +457,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -501,7 +489,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -528,12 +515,11 @@ class ConnectScreenTest { ConnectUiState( location = null, relayLocation = null, - tunnelUiState = TunnelState.Disconnected, - tunnelRealState = TunnelState.Disconnected, + tunnelUiState = TunnelState.Disconnected(), + tunnelRealState = TunnelState.Disconnected(), inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -565,7 +551,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -582,39 +567,6 @@ class ConnectScreenTest { verify { mockedClickHandler.invoke() } } - @Test - fun testToggleTunnelInfo() { - // Arrange - val mockedClickHandler: () -> Unit = mockk(relaxed = true) - val dummyLocation = GeoIpLocation(null, null, "dummy country", null, "dummy hostname") - composeTestRule.setContentWithTheme { - ConnectScreen( - uiState = - ConnectUiState( - location = dummyLocation, - relayLocation = null, - tunnelUiState = TunnelState.Connecting(null, null), - tunnelRealState = TunnelState.Connecting(null, null), - inAddress = null, - outAddress = "", - showLocation = false, - isTunnelInfoExpanded = false, - deviceName = "", - daysLeftUntilExpiry = null, - inAppNotification = null, - isPlayBuild = false - ), - onToggleTunnelInfo = mockedClickHandler - ) - } - - // Act - composeTestRule.onNodeWithTag(LOCATION_INFO_TEST_TAG).performClick() - - // Assert - verify { mockedClickHandler.invoke() } - } - @Test fun showLocationInfo() { // Arrange @@ -638,7 +590,6 @@ class ConnectScreenTest { inAddress = mockInAddress, outAddress = mockOutAddress, showLocation = false, - isTunnelInfoExpanded = true, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = null, @@ -647,6 +598,8 @@ class ConnectScreenTest { ) } + composeTestRule.onNodeWithTag(LOCATION_INFO_TEST_TAG).performClick() + // Assert composeTestRule.apply { onNodeWithText(mockHostName).assertExists() @@ -677,7 +630,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.UpdateAvailable(versionInfo), @@ -714,7 +666,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.UnsupportedVersion(versionInfo), @@ -748,7 +699,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.AccountExpiry(expiryDate), @@ -787,7 +737,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.UnsupportedVersion(versionInfo), @@ -820,7 +769,6 @@ class ConnectScreenTest { inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, deviceName = "", daysLeftUntilExpiry = null, inAppNotification = InAppNotification.AccountExpiry(expiryDate), @@ -838,10 +786,8 @@ class ConnectScreenTest { @Test fun testOpenAccountView() { - - val onAccountClickMockk: () -> Unit = mockk(relaxed = true) - // Arrange + val onAccountClickMockk: () -> Unit = mockk(relaxed = true) composeTestRule.setContentWithTheme { ConnectScreen(uiState = ConnectUiState.INITIAL, onAccountClick = onAccountClickMockk) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt index 742302ce91ae..903bb412dc25 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt @@ -18,7 +18,7 @@ import net.mullvad.talpid.tunnel.ErrorStateCause private fun PreviewConnectionStatusText() { AppTheme { SpacedColumn { - ConnectionStatusText(TunnelState.Disconnected) + ConnectionStatusText(TunnelState.Disconnected()) ConnectionStatusText(TunnelState.Connecting(null, null)) ConnectionStatusText( state = TunnelState.Error(ErrorState(ErrorStateCause.Ipv6Unavailable, true)) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt index b1de1ad80962..646e89c9873f 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt @@ -19,7 +19,9 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableLongStateOf +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -108,7 +110,6 @@ fun Connect(navigator: DestinationsNavigator) { onSwitchLocationClick = { navigator.navigate(SelectLocationDestination) { launchSingleTop = true } }, - onToggleTunnelInfo = connectViewModel::toggleTunnelInfoExpansion, onUpdateVersionClick = { val intent = Intent( @@ -137,7 +138,6 @@ fun ConnectScreen( onConnectClick: () -> Unit = {}, onCancelClick: () -> Unit = {}, onSwitchLocationClick: () -> Unit = {}, - onToggleTunnelInfo: () -> Unit = {}, onUpdateVersionClick: () -> Unit = {}, onManageAccountClick: () -> Unit = {}, onSettingsClick: () -> Unit = {}, @@ -233,12 +233,13 @@ fun ConnectScreen( color = MaterialTheme.colorScheme.onPrimary, modifier = Modifier.padding(horizontal = Dimens.sideMargin) ) + var expanded by rememberSaveable { mutableStateOf(false) } LocationInfo( - onToggleTunnelInfo = onToggleTunnelInfo, + onToggleTunnelInfo = { expanded = !expanded }, isVisible = - uiState.tunnelRealState != TunnelState.Disconnected && + uiState.tunnelRealState !is TunnelState.Disconnected && uiState.location?.hostname != null, - isExpanded = uiState.isTunnelInfoExpanded, + isExpanded = expanded, location = uiState.location, inAddress = uiState.inAddress, outAddress = uiState.outAddress, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt index d9071be7d85e..d0d0c7460d8b 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt @@ -62,7 +62,7 @@ private fun PreviewOutOfTimeScreenDisconnected() { OutOfTimeScreen( uiState = OutOfTimeUiState( - tunnelState = TunnelState.Disconnected, + tunnelState = TunnelState.Disconnected(), "Heroic Frog", showSitePayment = true ), diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt index 54c1a0d7c050..dc26e24741df 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt @@ -15,7 +15,6 @@ data class ConnectUiState( val outAddress: String, val showLocation: Boolean, val inAppNotification: InAppNotification?, - val isTunnelInfoExpanded: Boolean, val deviceName: String?, val daysLeftUntilExpiry: Int?, val isPlayBuild: Boolean @@ -25,12 +24,11 @@ data class ConnectUiState( ConnectUiState( location = null, relayLocation = null, - tunnelUiState = TunnelState.Disconnected, - tunnelRealState = TunnelState.Disconnected, + tunnelUiState = TunnelState.Disconnected(), + tunnelRealState = TunnelState.Disconnected(), inAddress = null, outAddress = "", showLocation = false, - isTunnelInfoExpanded = false, inAppNotification = null, deviceName = null, daysLeftUntilExpiry = null, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/OutOfTimeUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/OutOfTimeUiState.kt index 54fd414f866a..d72e01519494 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/OutOfTimeUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/OutOfTimeUiState.kt @@ -3,7 +3,7 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.model.TunnelState data class OutOfTimeUiState( - val tunnelState: TunnelState = TunnelState.Disconnected, + val tunnelState: TunnelState = TunnelState.Disconnected(), val deviceName: String = "", val showSitePayment: Boolean = false, val billingPaymentState: PaymentState? = null, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/WelcomeUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/WelcomeUiState.kt index e2673a0ddf38..e43cf6bb9887 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/WelcomeUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/WelcomeUiState.kt @@ -3,7 +3,7 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.model.TunnelState data class WelcomeUiState( - val tunnelState: TunnelState = TunnelState.Disconnected, + val tunnelState: TunnelState = TunnelState.Disconnected(), val accountNumber: String? = null, val deviceName: String? = null, val showSitePayment: Boolean = false, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt index d51bad461d9d..bbc267b2fac5 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt @@ -19,8 +19,8 @@ const val ANTICIPATED_STATE_TIMEOUT_MS = 1500L class ConnectionProxy(private val connection: Messenger, eventDispatcher: EventDispatcher) { private var resetAnticipatedStateJob: Job? = null - val onStateChange = EventNotifier(TunnelState.Disconnected) - val onUiStateChange = EventNotifier(TunnelState.Disconnected) + val onStateChange = EventNotifier(TunnelState.Disconnected()) + val onUiStateChange = EventNotifier(TunnelState.Disconnected()) var state by onStateChange.notifiable() private set diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/LocationInfoCache.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/LocationInfoCache.kt deleted file mode 100644 index 48f77d397dd9..000000000000 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/LocationInfoCache.kt +++ /dev/null @@ -1,26 +0,0 @@ -package net.mullvad.mullvadvpn.ui.serviceconnection - -import kotlin.properties.Delegates.observable -import net.mullvad.mullvadvpn.lib.ipc.Event -import net.mullvad.mullvadvpn.lib.ipc.EventDispatcher -import net.mullvad.mullvadvpn.model.GeoIpLocation - -class LocationInfoCache(eventDispatcher: EventDispatcher) { - private var location: GeoIpLocation? by - observable(null) { _, _, newLocation -> onNewLocation?.invoke(newLocation) } - - var onNewLocation by - observable<((GeoIpLocation?) -> Unit)?>(null) { _, _, callback -> - callback?.invoke(location) - } - - init { - eventDispatcher.registerHandler(Event.NewLocation::class) { event -> - location = event.location - } - } - - fun onDestroy() { - onNewLocation = null - } -} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionContainer.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionContainer.kt index ca156bed66e8..8aabe6c9f56c 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionContainer.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionContainer.kt @@ -28,7 +28,6 @@ class ServiceConnectionContainer( val authTokenCache = AuthTokenCache(connection, dispatcher) val connectionProxy = ConnectionProxy(connection, dispatcher) val deviceDataSource = ServiceConnectionDeviceDataSource(connection, dispatcher) - val locationInfoCache = LocationInfoCache(dispatcher) val settingsListener = SettingsListener(connection, dispatcher) val splitTunneling = SplitTunneling(connection, dispatcher) @@ -62,7 +61,6 @@ class ServiceConnectionContainer( authTokenCache.onDestroy() connectionProxy.onDestroy() - locationInfoCache.onDestroy() settingsListener.onDestroy() voucherRedeemer.onDestroy() diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt index f228bd7dbecc..dec794c86c8e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt @@ -39,7 +39,7 @@ class TunnelStateNotificationUseCase( } is TunnelState.Error -> InAppNotification.TunnelStateError(tunnelUiState.errorState) is TunnelState.Connected, - TunnelState.Disconnected -> null + is TunnelState.Disconnected -> null } private fun ConnectionProxy.tunnelUiStateFlow(): Flow = diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt index 306600608382..d25f360b51fe 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt @@ -5,30 +5,30 @@ import androidx.lifecycle.viewModelScope import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import net.mullvad.mullvadvpn.compose.state.ConnectUiState +import net.mullvad.mullvadvpn.model.GeoIpLocation import net.mullvad.mullvadvpn.model.TunnelState import net.mullvad.mullvadvpn.repository.AccountRepository import net.mullvad.mullvadvpn.repository.DeviceRepository import net.mullvad.mullvadvpn.repository.InAppNotificationController import net.mullvad.mullvadvpn.ui.serviceconnection.ConnectionProxy -import net.mullvad.mullvadvpn.ui.serviceconnection.LocationInfoCache import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionContainer import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionState @@ -71,48 +71,46 @@ class ConnectViewModel( } .shareIn(viewModelScope, SharingStarted.WhileSubscribed()) - private val _isTunnelInfoExpanded = MutableStateFlow(false) - val uiState: StateFlow = _shared .flatMapLatest { serviceConnection -> combine( - serviceConnection.locationInfoCache.locationCallbackFlow(), relayListUseCase.selectedRelayItem(), inAppNotificationController.notifications, serviceConnection.connectionProxy.tunnelUiStateFlow(), serviceConnection.connectionProxy.tunnelRealStateFlow(), + serviceConnection.connectionProxy.lastKnownDisconnectedLocation(), accountRepository.accountExpiryState, - _isTunnelInfoExpanded, deviceRepository.deviceState.map { it.deviceName() } ) { - location, relayLocation, notifications, tunnelUiState, tunnelRealState, + lastKnownDisconnectedLocation, accountExpiry, - isTunnelInfoExpanded, deviceName -> ConnectUiState( location = when (tunnelRealState) { - is TunnelState.Connected -> tunnelRealState.location + is TunnelState.Disconnected -> tunnelRealState.location() + ?: lastKnownDisconnectedLocation is TunnelState.Connecting -> tunnelRealState.location - else -> null - } - ?: location, + ?: relayLocation?.location?.location + is TunnelState.Connected -> tunnelRealState.location + is TunnelState.Disconnecting -> lastKnownDisconnectedLocation + is TunnelState.Error -> null + }, relayLocation = relayLocation, tunnelUiState = tunnelUiState, tunnelRealState = tunnelRealState, - isTunnelInfoExpanded = isTunnelInfoExpanded, inAddress = when (tunnelRealState) { is TunnelState.Connected -> tunnelRealState.endpoint.toInAddress() is TunnelState.Connecting -> tunnelRealState.endpoint?.toInAddress() else -> null }, - outAddress = location?.toOutAddress() ?: "", + outAddress = tunnelRealState.location()?.toOutAddress() ?: "", showLocation = when (tunnelUiState) { is TunnelState.Disconnected -> true @@ -149,20 +147,18 @@ class ConnectViewModel( } } - private fun LocationInfoCache.locationCallbackFlow() = callbackFlow { - onNewLocation = { this.trySend(it) } - awaitClose { onNewLocation = null } - } - private fun ConnectionProxy.tunnelUiStateFlow(): Flow = callbackFlowFromNotifier(this.onUiStateChange) private fun ConnectionProxy.tunnelRealStateFlow(): Flow = callbackFlowFromNotifier(this.onStateChange) - fun toggleTunnelInfoExpansion() { - _isTunnelInfoExpanded.value = _isTunnelInfoExpanded.value.not() - } + private fun ConnectionProxy.lastKnownDisconnectedLocation(): Flow = + tunnelRealStateFlow() + .filterIsInstance() + .filter { it.location != null } + .map { it.location } + .onStart { emit(null) } fun onDisconnectClick() { serviceConnectionManager.connectionProxy()?.disconnect() diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/OutOfTimeUseCaseTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/OutOfTimeUseCaseTest.kt index 74683813aede..bdc5ea49b859 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/OutOfTimeUseCaseTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/OutOfTimeUseCaseTest.kt @@ -63,7 +63,7 @@ class OutOfTimeUseCaseTest { val expiredAccountExpiry = AccountExpiry.Available(DateTime.now().plusDays(1)) val tunnelStateChanges = listOf( - TunnelState.Disconnected, + TunnelState.Disconnected(), TunnelState.Connected(mockk(), null), TunnelState.Connecting(null, null), TunnelState.Disconnecting(mockk()), diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCaseTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCaseTest.kt index 1b89c92be732..e67630642dc9 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCaseTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCaseTest.kt @@ -35,7 +35,7 @@ class TunnelStateNotificationUseCaseTest { MutableStateFlow(ServiceConnectionState.Disconnected) private lateinit var tunnelStateNotificationUseCase: TunnelStateNotificationUseCase - private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected) + private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected()) @Before fun setup() { diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt index 7271c074334e..47364652ceda 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt @@ -6,12 +6,11 @@ import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic -import io.mockk.slot import io.mockk.unmockkAll import io.mockk.verify import kotlin.test.assertEquals import kotlin.test.assertIs -import kotlin.test.assertTrue +import kotlin.test.assertNull import kotlinx.coroutines.async import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow @@ -33,7 +32,6 @@ import net.mullvad.mullvadvpn.ui.VersionInfo import net.mullvad.mullvadvpn.ui.serviceconnection.AppVersionInfoCache import net.mullvad.mullvadvpn.ui.serviceconnection.AuthTokenCache import net.mullvad.mullvadvpn.ui.serviceconnection.ConnectionProxy -import net.mullvad.mullvadvpn.ui.serviceconnection.LocationInfoCache import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionContainer import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionState @@ -73,7 +71,6 @@ class ConnectViewModelTest { // Service connections private val mockServiceConnectionContainer: ServiceConnectionContainer = mockk() - private val mockLocationInfoCache: LocationInfoCache = mockk(relaxUnitFun = true) private lateinit var mockAppVersionInfoCache: AppVersionInfoCache private val mockConnectionProxy: ConnectionProxy = mockk() private val mockLocation: GeoIpLocation = mockk(relaxed = true) @@ -93,12 +90,10 @@ class ConnectViewModelTest { // Payment use case private val mockPaymentUseCase: PaymentUseCase = mockk(relaxed = true) - // Captures - private val locationSlot = slot<((GeoIpLocation?) -> Unit)>() - // Event notifiers - private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected) - private val eventNotifierTunnelRealState = EventNotifier(TunnelState.Disconnected) + private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected()) + private val eventNotifierTunnelRealState = + EventNotifier(TunnelState.Disconnected()) // Flows private val selectedRelayFlow = MutableStateFlow(null) @@ -118,7 +113,6 @@ class ConnectViewModelTest { } every { mockServiceConnectionManager.connectionState } returns serviceConnectionState - every { mockServiceConnectionContainer.locationInfoCache } returns mockLocationInfoCache every { mockServiceConnectionContainer.appVersionInfoCache } returns mockAppVersionInfoCache every { mockServiceConnectionContainer.connectionProxy } returns mockConnectionProxy @@ -134,7 +128,6 @@ class ConnectViewModelTest { every { mockLocation.country } returns "dummy country" // Listeners - every { mockLocationInfoCache.onNewLocation = capture(locationSlot) } answers {} every { mockAppVersionInfoCache.onUpdate = any() } answers {} // Flows @@ -166,30 +159,15 @@ class ConnectViewModelTest { viewModel.uiState.test { assertEquals(ConnectUiState.INITIAL, awaitItem()) } } - @Test - fun testTunnelInfoExpandedUpdate() = - runTest(testCoroutineRule.testDispatcher) { - viewModel.uiState.test { - assertEquals(ConnectUiState.INITIAL, awaitItem()) - serviceConnectionState.value = - ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) - viewModel.toggleTunnelInfoExpansion() - val result = awaitItem() - assertTrue(result.isTunnelInfoExpanded) - } - } - @Test fun testTunnelRealStateUpdate() = runTest(testCoroutineRule.testDispatcher) { - val tunnelRealStateTestItem = TunnelState.Connected(mockk(relaxed = true), mockk()) + val tunnelRealStateTestItem = TunnelState.Connected(mockk(relaxed = true), null) viewModel.uiState.test { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) eventNotifierTunnelRealState.notify(tunnelRealStateTestItem) val result = awaitItem() assertEquals(tunnelRealStateTestItem, result.tunnelRealState) @@ -205,7 +183,6 @@ class ConnectViewModelTest { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) eventNotifierTunnelUiState.notify(tunnelUiStateTestItem) val result = awaitItem() assertEquals(tunnelUiStateTestItem, result.tunnelUiState) @@ -223,7 +200,6 @@ class ConnectViewModelTest { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) val result = awaitItem() assertEquals(relayTestItem, result.relayLocation) } @@ -241,13 +217,19 @@ class ConnectViewModelTest { hostname = "Host" ) + // Act, Assert viewModel.uiState.test { assertEquals(ConnectUiState.INITIAL, awaitItem()) + eventNotifierTunnelRealState.notify(TunnelState.Disconnected(null)) + serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(locationTestItem) - val result = awaitItem() - assertEquals(locationTestItem, result.location) + // Start of with no location + assertNull(awaitItem().location) + + // After updated we show latest + eventNotifierTunnelRealState.notify(TunnelState.Disconnected(locationTestItem)) + assertEquals(locationTestItem, awaitItem().location) } } @@ -262,7 +244,6 @@ class ConnectViewModelTest { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(locationTestItem) expectNoEvents() val result = awaitItem() assertEquals(locationTestItem, result.location) @@ -320,7 +301,6 @@ class ConnectViewModelTest { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) eventNotifierTunnelUiState.notify(tunnelUiState) val result = awaitItem() assertEquals(expectedConnectNotificationState, result.inAppNotification) @@ -356,7 +336,6 @@ class ConnectViewModelTest { awaitItem() serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - locationSlot.captured.invoke(mockLocation) outOfTimeViewFlow.value = true awaitItem() } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/DeviceRevokedViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/DeviceRevokedViewModelTest.kt index 8959b1f9e375..a73ecfc4e786 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/DeviceRevokedViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/DeviceRevokedViewModelTest.kt @@ -105,7 +105,7 @@ class DeviceRevokedViewModelTest { // Arrange val mockedContainer = mockk().also { - every { it.connectionProxy.state } returns TunnelState.Disconnected + every { it.connectionProxy.state } returns TunnelState.Disconnected() every { it.connectionProxy.disconnect() } just Runs every { mockedAccountRepository.logout() } just Runs } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModelTest.kt index ab861b1e208b..50a9d8fb981a 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModelTest.kt @@ -59,7 +59,8 @@ class OutOfTimeViewModelTest { private val mockConnectionProxy: ConnectionProxy = mockk() // Event notifiers - private val eventNotifierTunnelRealState = EventNotifier(TunnelState.Disconnected) + private val eventNotifierTunnelRealState = + EventNotifier(TunnelState.Disconnected()) private val mockAccountRepository: AccountRepository = mockk(relaxed = true) private val mockDeviceRepository: DeviceRepository = mockk() diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/WelcomeViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/WelcomeViewModelTest.kt index ac047db2ea08..29b0dfde7505 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/WelcomeViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/WelcomeViewModelTest.kt @@ -58,7 +58,7 @@ class WelcomeViewModelTest { private val mockConnectionProxy: ConnectionProxy = mockk() // Event notifiers - private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected) + private val eventNotifierTunnelUiState = EventNotifier(TunnelState.Disconnected()) private val mockAccountRepository: AccountRepository = mockk(relaxed = true) private val mockDeviceRepository: DeviceRepository = mockk() diff --git a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt index e1079807f169..69c28bb379da 100644 --- a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt +++ b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt @@ -7,7 +7,6 @@ import net.mullvad.mullvadvpn.model.AccountExpiry import net.mullvad.mullvadvpn.model.AccountHistory import net.mullvad.mullvadvpn.model.DeviceListEvent import net.mullvad.mullvadvpn.model.DeviceState -import net.mullvad.mullvadvpn.model.GeoIpLocation import net.mullvad.mullvadvpn.model.LoginResult import net.mullvad.mullvadvpn.model.PlayPurchaseInitResult import net.mullvad.mullvadvpn.model.PlayPurchaseVerifyResult @@ -45,8 +44,6 @@ sealed class Event : Message.EventMessage() { @Parcelize data class LoginEvent(val result: LoginResult) : Event() - @Parcelize data class NewLocation(val location: GeoIpLocation?) : Event() - @Parcelize data class NewRelayList(val relayList: RelayList?) : Event() @Parcelize data class SettingsUpdate(val settings: Settings?) : Event() diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/TunnelState.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/TunnelState.kt index 26f776b2236b..4ab925d01427 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/TunnelState.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/TunnelState.kt @@ -5,11 +5,10 @@ import kotlinx.parcelize.Parcelize import net.mullvad.talpid.net.TunnelEndpoint import net.mullvad.talpid.tunnel.ActionAfterDisconnect import net.mullvad.talpid.tunnel.ErrorState -import net.mullvad.talpid.tunnel.ErrorStateCause -import net.mullvad.talpid.tunnel.FirewallPolicyError sealed class TunnelState : Parcelable { - @Parcelize object Disconnected : TunnelState(), Parcelable + @Parcelize + data class Disconnected(val location: GeoIpLocation? = null) : TunnelState(), Parcelable @Parcelize class Connecting(val endpoint: TunnelEndpoint?, val location: GeoIpLocation?) : @@ -25,6 +24,16 @@ sealed class TunnelState : Parcelable { @Parcelize class Error(val errorState: ErrorState) : TunnelState(), Parcelable + fun location(): GeoIpLocation? { + return when (this) { + is Connected -> location + is Connecting -> location + is Disconnecting -> null + is Disconnected -> location + is Error -> null + } + } + fun isSecured(): Boolean { return when (this) { is Connected, @@ -34,61 +43,4 @@ sealed class TunnelState : Parcelable { is Error -> this.errorState.isBlocking } } - - override fun toString(): String = - when (this) { - is Disconnected -> DISCONNECTED - is Connecting -> CONNECTING - is Connected -> CONNECTED - is Disconnecting -> { - if (actionAfterDisconnect == ActionAfterDisconnect.Reconnect) { - RECONNECTING - } else { - DISCONNECTING - } - } - is Error -> { - if (errorState.isBlocking) { - BLOCKING - } else { - ERROR - } - } - } - - companion object { - const val DISCONNECTED = "disconnected" - const val CONNECTING = "connecting" - const val CONNECTED = "connected" - const val RECONNECTING = "reconnecting" - const val DISCONNECTING = "disconnecting" - const val BLOCKING = "blocking" - const val ERROR = "error" - - fun fromString(description: String, endpoint: TunnelEndpoint?): TunnelState { - return when (description) { - DISCONNECTED -> Disconnected - CONNECTING -> Connecting(endpoint, null) - CONNECTED -> Connected(endpoint!!, null) - RECONNECTING -> Disconnecting(ActionAfterDisconnect.Reconnect) - DISCONNECTING -> Disconnecting(ActionAfterDisconnect.Nothing) - BLOCKING -> Error(ErrorState(ErrorStateCause.StartTunnelError, true)) - ERROR -> { - Error( - ErrorState( - ErrorStateCause.SetFirewallPolicyError(FirewallPolicyError.Generic), - false - ) - ) - } - else -> - Error( - ErrorState( - ErrorStateCause.SetFirewallPolicyError(FirewallPolicyError.Generic), - false - ) - ) - } - } - } } diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt index 0cfd0e1f7db3..107f5d35fe2f 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt @@ -10,7 +10,6 @@ import net.mullvad.mullvadvpn.model.DeviceEvent import net.mullvad.mullvadvpn.model.DeviceListEvent import net.mullvad.mullvadvpn.model.DeviceState import net.mullvad.mullvadvpn.model.DnsOptions -import net.mullvad.mullvadvpn.model.GeoIpLocation import net.mullvad.mullvadvpn.model.GetAccountDataResult import net.mullvad.mullvadvpn.model.LoginResult import net.mullvad.mullvadvpn.model.ObfuscationSettings @@ -34,7 +33,7 @@ class MullvadDaemon( protected var daemonInterfaceAddress = 0L val onSettingsChange = EventNotifier(null) - var onTunnelStateChange = EventNotifier(TunnelState.Disconnected) + var onTunnelStateChange = EventNotifier(TunnelState.Disconnected()) var onAppVersionInfoChange: ((AppVersionInfo) -> Unit)? = null var onRelayListChange: ((RelayList) -> Unit)? = null @@ -58,7 +57,7 @@ class MullvadDaemon( onSettingsChange.notify(getSettings()) - onTunnelStateChange.notify(getState() ?: TunnelState.Disconnected) + onTunnelStateChange.notify(getState() ?: TunnelState.Disconnected()) } fun connect() { @@ -85,10 +84,6 @@ class MullvadDaemon( return getWwwAuthToken(daemonInterfaceAddress) ?: "" } - fun getCurrentLocation(): GeoIpLocation? { - return getCurrentLocation(daemonInterfaceAddress) - } - fun getCurrentVersion(): String? { return getCurrentVersion(daemonInterfaceAddress) } @@ -229,8 +224,6 @@ class MullvadDaemon( private external fun getWwwAuthToken(daemonInterfaceAddress: Long): String? - private external fun getCurrentLocation(daemonInterfaceAddress: Long): GeoIpLocation? - private external fun getCurrentVersion(daemonInterfaceAddress: Long): String? private external fun getRelayLocations(daemonInterfaceAddress: Long): RelayList? diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt index e714295d9c46..b0c55540a672 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt @@ -189,7 +189,7 @@ class MullvadVpnService : TalpidVpnService() { override fun onTaskRemoved(rootIntent: Intent?) { connectionProxy.onStateChange.latestEvent.let { tunnelState -> Log.d(TAG, "Task removed (tunnelState=$tunnelState)") - if (tunnelState == TunnelState.Disconnected) { + if (tunnelState is TunnelState.Disconnected) { notificationManager.cancelNotification() stop() } diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ConnectionProxy.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ConnectionProxy.kt index a2c97a05bd1a..65a27c8f695b 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ConnectionProxy.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ConnectionProxy.kt @@ -20,7 +20,7 @@ class ConnectionProxy(val vpnPermission: VpnPermission, endpoint: ServiceEndpoin private val commandChannel = spawnActor() private val daemon = endpoint.intermittentDaemon - private val initialState = TunnelState.Disconnected + private val initialState = TunnelState.Disconnected() var onStateChange = EventNotifier(initialState) diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/LocationInfoCache.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/LocationInfoCache.kt deleted file mode 100644 index 2d06cc109f73..000000000000 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/LocationInfoCache.kt +++ /dev/null @@ -1,139 +0,0 @@ -package net.mullvad.mullvadvpn.service.endpoint - -import kotlin.properties.Delegates.observable -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.channels.ClosedReceiveChannelException -import kotlinx.coroutines.channels.ReceiveChannel -import kotlinx.coroutines.channels.actor -import kotlinx.coroutines.channels.trySendBlocking -import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.receiveAsFlow -import net.mullvad.mullvadvpn.lib.common.util.toGeographicLocationConstraint -import net.mullvad.mullvadvpn.lib.ipc.Event -import net.mullvad.mullvadvpn.model.Constraint -import net.mullvad.mullvadvpn.model.GeoIpLocation -import net.mullvad.mullvadvpn.model.RelaySettings -import net.mullvad.mullvadvpn.model.TunnelState -import net.mullvad.mullvadvpn.service.util.ExponentialBackoff -import net.mullvad.talpid.tunnel.ActionAfterDisconnect - -class LocationInfoCache(private val endpoint: ServiceEndpoint) { - - private val fetchRetryDelays = - ExponentialBackoff().apply { - scale = 50 - cap = 30 /* min */ * 60 /* s */ * 1000 /* ms */ - count = 17 // ceil(log2(cap / scale) + 1) - } - - private val fetchRequestChannel = runFetcher() - - private val daemon - get() = endpoint.intermittentDaemon - - private var lastKnownRealLocation: GeoIpLocation? = null - private var selectedRelayLocation: GeoIpLocation? = null - - var location: GeoIpLocation? by - observable(null) { _, _, newLocation -> endpoint.sendEvent(Event.NewLocation(newLocation)) } - - var state by - observable(TunnelState.Disconnected) { _, _, newState -> - when (newState) { - is TunnelState.Disconnected -> { - location = lastKnownRealLocation - fetchRequestChannel.trySendBlocking(RequestFetch.ForRealLocation) - } - is TunnelState.Connecting -> location = newState.location - is TunnelState.Connected -> { - location = newState.location - fetchRequestChannel.trySendBlocking(RequestFetch.ForRelayLocation) - } - is TunnelState.Disconnecting -> { - when (newState.actionAfterDisconnect) { - ActionAfterDisconnect.Nothing -> location = lastKnownRealLocation - ActionAfterDisconnect.Block -> location = null - ActionAfterDisconnect.Reconnect -> { - lastKnownRealLocation?.let { location = it } - } - } - } - is TunnelState.Error -> location = null - } - } - - init { - endpoint.connectionProxy.onStateChange.subscribe(this) { newState -> state = newState } - - endpoint.connectivityListener.connectivityNotifier.subscribe(this) { isConnected -> - if (isConnected && state is TunnelState.Disconnected) { - fetchRequestChannel.trySendBlocking(RequestFetch.ForRealLocation) - } - } - - endpoint.settingsListener.relaySettingsNotifier.subscribe(this, ::updateSelectedLocation) - } - - fun onDestroy() { - endpoint.connectionProxy.onStateChange.unsubscribe(this) - endpoint.connectivityListener.connectivityNotifier.unsubscribe(this) - endpoint.settingsListener.relaySettingsNotifier.unsubscribe(this) - - fetchRequestChannel.close() - } - - private fun runFetcher() = - GlobalScope.actor(Dispatchers.Default, Channel.CONFLATED) { - try { - fetcherLoop(channel) - } catch (exception: ClosedReceiveChannelException) {} - } - - private suspend fun fetcherLoop(channel: ReceiveChannel) { - channel - .receiveAsFlow() - .flatMapLatest(::fetchCurrentLocation) - .collect(::handleFetchedLocation) - } - - private fun fetchCurrentLocation(fetchType: RequestFetch) = flow { - var newLocation = daemon.await().getCurrentLocation() - - fetchRetryDelays.reset() - - while (newLocation == null) { - delay(fetchRetryDelays.next()) - newLocation = daemon.await().getCurrentLocation() - } - - emit(Pair(newLocation, fetchType)) - } - - private suspend fun handleFetchedLocation(pairItem: Pair) { - val (newLocation, fetchType) = pairItem - - if (fetchType == RequestFetch.ForRealLocation) { - lastKnownRealLocation = newLocation - } - - location = newLocation - } - - private fun updateSelectedLocation(relaySettings: RelaySettings?) { - val settings = relaySettings as? RelaySettings.Normal - val constraint = settings?.relayConstraints?.location as? Constraint.Only - - selectedRelayLocation = constraint?.value?.toGeographicLocationConstraint()?.location - } - - companion object { - private enum class RequestFetch { - ForRealLocation, - ForRelayLocation, - } - } -} diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt index 70e7807ff921..09fc73e5d3cd 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt @@ -46,7 +46,6 @@ class ServiceEndpoint( val appVersionInfoCache = AppVersionInfoCache(this) val authTokenCache = AuthTokenCache(this) val customDns = CustomDns(this) - val locationInfoCache = LocationInfoCache(this) val relayListListener = RelayListListener(this) val splitTunneling = SplitTunneling(SplitTunnelingPersistence(context), this) val voucherRedeemer = VoucherRedeemer(this, accountCache) @@ -77,7 +76,6 @@ class ServiceEndpoint( connectionProxy.onDestroy() customDns.onDestroy() deviceRepositoryBackend.onDestroy() - locationInfoCache.onDestroy() relayListListener.onDestroy() settingsListener.onDestroy() splitTunneling.onDestroy() @@ -127,7 +125,6 @@ class ServiceEndpoint( Event.TunnelStateChange(connectionProxy.state), Event.AccountHistoryEvent(accountCache.onAccountHistoryChange.latestEvent), Event.SettingsUpdate(settingsListener.settings), - Event.NewLocation(locationInfoCache.location), Event.SplitTunnelingUpdate(splitTunneling.onChange.latestEvent), Event.CurrentVersion(appVersionInfoCache.currentVersion), Event.AppVersionInfo(appVersionInfoCache.appVersionInfo), diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt index 97bddc860805..ede083a2a182 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt @@ -77,7 +77,7 @@ class TunnelStateNotification(val context: Context) { var showAction by observable(false) { _, _, _ -> update() } var tunnelState by - observable(TunnelState.Disconnected) { _, _, newState -> + observable(TunnelState.Disconnected()) { _, _, newState -> val isReconnecting = newState is TunnelState.Connecting && reconnecting val shouldBeginReconnecting = (newState as? TunnelState.Disconnecting)?.actionAfterDisconnect == diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/util/ExponentialBackoff.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/util/ExponentialBackoff.kt deleted file mode 100644 index 12e94a92411b..000000000000 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/util/ExponentialBackoff.kt +++ /dev/null @@ -1,52 +0,0 @@ -package net.mullvad.mullvadvpn.service.util - -// Calculates a series of delays that increase exponentially. -// -// The delays follow the formula: -// -// (base ^ retryAttempt) * scale -// -// but it is never larger than the specified cap value. -class ExponentialBackoff : Iterator { - private var unscaledValue = 1L - private var current = 1L - - var iteration = 1 - private set - - var base = 2L - var scale = 1000L - var cap = Long.MAX_VALUE - var count: Int? = null - - override fun hasNext(): Boolean { - val maxIterations = count - - if (maxIterations != null) { - return iteration < maxIterations - } else { - return true - } - } - - override fun next(): Long { - iteration += 1 - - if (current >= cap) { - return cap - } else { - val value = current - - unscaledValue *= base - current = Math.min(cap, scale * unscaledValue) - - return value - } - } - - fun reset() { - unscaledValue = 1L - current = 1L - iteration = 1 - } -} diff --git a/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/ServiceConnection.kt b/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/ServiceConnection.kt index a26351162d02..93218c66dcdf 100644 --- a/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/ServiceConnection.kt +++ b/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/ServiceConnection.kt @@ -60,7 +60,7 @@ class ServiceConnection(context: Context, scope: CoroutineScope) { subscribeToState( Event.TunnelStateChange::class, scope, - TunnelState.Disconnected + TunnelState.Disconnected() ) { tunnelState } diff --git a/mullvad-jni/src/daemon_interface.rs b/mullvad-jni/src/daemon_interface.rs index ff00f4a825b8..feea551557dd 100644 --- a/mullvad-jni/src/daemon_interface.rs +++ b/mullvad-jni/src/daemon_interface.rs @@ -114,14 +114,6 @@ impl DaemonInterface { .map_err(Error::from) } - pub fn get_current_location(&self) -> Result> { - let (tx, rx) = oneshot::channel(); - - self.send_command(DaemonCommand::GetCurrentLocation(tx))?; - - block_on(rx).map_err(|_| Error::NoResponse) - } - pub fn get_current_version(&self) -> Result { let (tx, rx) = oneshot::channel(); diff --git a/mullvad-jni/src/lib.rs b/mullvad-jni/src/lib.rs index a2c35864315b..8c5115789665 100644 --- a/mullvad-jni/src/lib.rs +++ b/mullvad-jni/src/lib.rs @@ -760,34 +760,6 @@ pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_getWwwA } } -#[no_mangle] -#[allow(non_snake_case)] -pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_getCurrentLocation< - 'env, ->( - env: JNIEnv<'env>, - _: JObject<'_>, - daemon_interface_address: jlong, -) -> JObject<'env> { - let env = JnixEnv::from(env); - - // SAFETY: The address points to an instance valid for the duration of this function call - if let Some(daemon_interface) = unsafe { get_daemon_interface(daemon_interface_address) } { - match daemon_interface.get_current_location() { - Ok(location) => location.into_java(&env).forget(), - Err(error) => { - log::error!( - "{}", - error.display_chain_with_msg("Failed to get current location") - ); - JObject::null() - } - } - } else { - JObject::null() - } -} - #[no_mangle] #[allow(non_snake_case)] pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_getCurrentVersion<'env>(