From 12ad29570624310ff7421b56ff56f52fd274fc7f Mon Sep 17 00:00:00 2001 From: kari-ts <135075563+kari-ts@users.noreply.github.com> Date: Thu, 16 May 2024 16:15:09 -0700 Subject: [PATCH] android: fix connect VPN permissions (#398) -show VPN connection permissions after intro screen -make toggle state and main view take VPN preparedness into consideration Fixes tailscale/tailscale#12148 Signed-off-by: kari-ts --- .../src/main/java/com/tailscale/ipn/App.kt | 24 ------ .../java/com/tailscale/ipn/MainActivity.kt | 48 +++++------ .../src/main/java/com/tailscale/ipn/Peer.java | 28 ------ .../com/tailscale/ipn/ui/view/MainView.kt | 43 ++++++++-- .../ipn/ui/viewModel/IpnViewModel.kt | 10 +-- .../ipn/ui/viewModel/MainViewModel.kt | 85 ++++++++++++++----- android/src/main/res/values/strings.xml | 2 + 7 files changed, 123 insertions(+), 117 deletions(-) delete mode 100644 android/src/main/java/com/tailscale/ipn/Peer.java diff --git a/android/src/main/java/com/tailscale/ipn/App.kt b/android/src/main/java/com/tailscale/ipn/App.kt index 0184910..d4f0acd 100644 --- a/android/src/main/java/com/tailscale/ipn/App.kt +++ b/android/src/main/java/com/tailscale/ipn/App.kt @@ -3,9 +3,7 @@ package com.tailscale.ipn import android.Manifest -import android.app.Activity import android.app.Application -import android.app.Fragment import android.app.Notification import android.app.NotificationChannel import android.app.PendingIntent @@ -18,7 +16,6 @@ import android.net.LinkProperties import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkRequest -import android.net.VpnService import android.os.Build import android.os.Environment import android.util.Log @@ -51,7 +48,6 @@ class App : UninitializedApp(), libtailscale.AppContext { val applicationScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) companion object { - private const val PEER_TAG = "peer" private const val FILE_CHANNEL_ID = "tailscale-files" private const val TAG = "App" private val networkConnectivityRequest = @@ -61,12 +57,6 @@ class App : UninitializedApp(), libtailscale.AppContext { .build() private lateinit var appInstance: App - @JvmStatic - fun startActivityForResult(act: Activity, intent: Intent?, request: Int) { - val f: Fragment = act.fragmentManager.findFragmentByTag(PEER_TAG) - f.startActivityForResult(intent, request) - } - /** * Initializes the app (if necessary) and returns the singleton app instance. Always use this * function to obtain an App reference to make sure the app initializes. @@ -246,20 +236,6 @@ class App : UninitializedApp(), libtailscale.AppContext { return packageManager.hasSystemFeature("android.hardware.type.pc") } - fun prepareVPN(act: Activity, reqCode: Int) { - // We do this with UI in case it's our first time starting the VPN. - act.runOnUiThread { - val prepareIntent = VpnService.prepare(this) - if (prepareIntent == null) { - // No intent here means that we already have permission to be a VPN. - startVPN() - } else { - // An intent here means that we need to prompt for permission to be a VPN. - startActivityForResult(act, prepareIntent, reqCode) - } - } - } - override fun getInterfacesAsString(): String { val interfaces: ArrayList = java.util.Collections.list(NetworkInterface.getNetworkInterfaces()) diff --git a/android/src/main/java/com/tailscale/ipn/MainActivity.kt b/android/src/main/java/com/tailscale/ipn/MainActivity.kt index 6ad6da0..7e905cd 100644 --- a/android/src/main/java/com/tailscale/ipn/MainActivity.kt +++ b/android/src/main/java/com/tailscale/ipn/MainActivity.kt @@ -19,6 +19,7 @@ import androidx.activity.ComponentActivity import androidx.activity.compose.setContent import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContract +import androidx.activity.viewModels import androidx.browser.customtabs.CustomTabsIntent import androidx.compose.animation.core.tween import androidx.compose.animation.fadeIn @@ -38,7 +39,6 @@ import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument -import com.tailscale.ipn.Peer.RequestCodes import com.tailscale.ipn.mdm.MDMSettings import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.notifier.Notifier @@ -68,6 +68,7 @@ import com.tailscale.ipn.ui.view.TailnetLockSetupView import com.tailscale.ipn.ui.view.UserSwitcherNav import com.tailscale.ipn.ui.view.UserSwitcherView import com.tailscale.ipn.ui.viewModel.ExitNodePickerNav +import com.tailscale.ipn.ui.viewModel.MainViewModel import com.tailscale.ipn.ui.viewModel.SettingsNav import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel @@ -78,6 +79,8 @@ import kotlinx.coroutines.launch class MainActivity : ComponentActivity() { private lateinit var requestVpnPermission: ActivityResultLauncher private lateinit var navController: NavHostController + private lateinit var vpnPermissionLauncher: ActivityResultLauncher + private val viewModel: MainViewModel by viewModels() companion object { private const val TAG = "Main Activity" @@ -109,6 +112,18 @@ class MainActivity : ComponentActivity() { installSplashScreen() + vpnPermissionLauncher = + registerForActivityResult(VpnPermissionContract()) { granted -> + if (granted) { + Log.d("VpnPermission", "VPN permission granted") + viewModel.setVpnPrepared(true) + } else { + Log.d("VpnPermission", "VPN permission denied") + viewModel.setVpnPrepared(false) + } + } + viewModel.setVpnPermissionLauncher(vpnPermissionLauncher) + setContent { AppTheme { navController = rememberNavController() @@ -176,7 +191,7 @@ class MainActivity : ComponentActivity() { onNavigateToAuthKey = { navController.navigate("loginWithAuthKey") }) composable("main", enterTransition = { fadeIn(animationSpec = tween(150)) }) { - MainView(loginAtUrl = ::login, navigation = mainViewNav) + MainView(loginAtUrl = ::login, navigation = mainViewNav, viewModel = viewModel) } composable("settings") { SettingsView(settingsNav) } composable("exitNodes") { ExitNodePicker(exitNodePickerNav) } @@ -231,13 +246,6 @@ class MainActivity : ComponentActivity() { } } } - lifecycleScope.launch { - Notifier.state.collect { state -> - if (state > Ipn.State.Stopped) { - App.get().prepareVPN(this@MainActivity, RequestCodes.requestPrepareVPN) - } - } - } } init { @@ -322,10 +330,6 @@ class MainActivity : ComponentActivity() { override fun onStart() { super.onStart() - - // (jonathan) TODO: Requesting VPN permissions onStart is a bit aggressive. This should - // be done when the user initiall starts the VPN - requestVpnPermission() } override fun onStop() { @@ -335,18 +339,6 @@ class MainActivity : ComponentActivity() { lifecycleScope.launch(Dispatchers.IO) { MDMSettings.update(App.get(), restrictionsManager) } } - private fun requestVpnPermission() { - val vpnIntent = VpnService.prepare(this) - if (vpnIntent != null) { - val contract = VpnPermissionContract() - requestVpnPermission = - registerForActivityResult(contract) { granted -> - Log.i("VPN", "VPN permission ${if (granted) "granted" else "denied"}") - } - requestVpnPermission.launch(Unit) - } - } - private fun openApplicationSettings() { val intent = Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply { @@ -368,9 +360,9 @@ class MainActivity : ComponentActivity() { } } -class VpnPermissionContract : ActivityResultContract() { - override fun createIntent(context: Context, input: Unit): Intent { - return VpnService.prepare(context) ?: Intent() +class VpnPermissionContract : ActivityResultContract() { + override fun createIntent(context: Context, input: Intent): Intent { + return input } override fun parseResult(resultCode: Int, intent: Intent?): Boolean { diff --git a/android/src/main/java/com/tailscale/ipn/Peer.java b/android/src/main/java/com/tailscale/ipn/Peer.java deleted file mode 100644 index 8201cc0..0000000 --- a/android/src/main/java/com/tailscale/ipn/Peer.java +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package com.tailscale.ipn; - -import android.app.Fragment; -import android.content.Intent; - -public class Peer extends Fragment { - - private static int resultOK = -1; - - public class RequestCodes { - public static final int requestPrepareVPN = 1001; - } - - @Override - public void onActivityResult(int requestCode, int resultCode, Intent data) { - if (requestCode == RequestCodes.requestPrepareVPN) { - if (resultCode == resultOK) { - UninitializedApp.get().startVPN(); - } else { - App.get().setWantRunning(false); - // notify VPN revoked - } - } - } -} diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt index 786a425..dff51e7 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt @@ -46,6 +46,7 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue +import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -88,6 +89,7 @@ import com.tailscale.ipn.ui.util.PeerSet import com.tailscale.ipn.ui.util.flag import com.tailscale.ipn.ui.util.itemsWithDividers import com.tailscale.ipn.ui.viewModel.MainViewModel +import android.util.Log // Navigation actions for the MainView data class MainViewNavigation( @@ -101,14 +103,17 @@ data class MainViewNavigation( fun MainView( loginAtUrl: (String) -> Unit, navigation: MainViewNavigation, - viewModel: MainViewModel = viewModel() + viewModel: MainViewModel ) { - val isOn = viewModel.vpnToggleState.collectAsState() + LoadingIndicator.Wrap { Scaffold(contentWindowInsets = WindowInsets.Companion.statusBars) { paddingInsets -> Column( modifier = Modifier.fillMaxWidth().padding(paddingInsets), verticalArrangement = Arrangement.Center) { + // Assume VPN has been prepared. Whether or not it has been prepared cannot be known until permission has been granted to prepare the VPN. + val isPrepared by viewModel.vpnPrepared.collectAsState(initial=true) + val isOn by viewModel.vpnToggleState.collectAsState(initial = false) val state by viewModel.ipnState.collectAsState(initial = Ipn.State.NoState) val user by viewModel.loggedInUser.collectAsState(initial = null) val stateVal by viewModel.stateRes.collectAsState(initial = R.string.placeholder) @@ -128,7 +133,7 @@ fun MainView( } }, enabled = !disableToggle, - checked = isOn.value) + checked = isOn) }, headlineContent = { user?.NetworkProfile?.DomainName?.let { domain -> @@ -185,11 +190,13 @@ fun MainView( else -> { ConnectView( state, + isPrepared, user, { viewModel.toggleVpn() }, { viewModel.login() }, loginAtUrl, - netmap?.SelfNode) + netmap?.SelfNode, + {viewModel.showVPNPermissionLauncherIfUnauthorized()}) } } } @@ -300,12 +307,19 @@ fun StartingView() { @Composable fun ConnectView( state: Ipn.State, + isPrepared: Boolean, user: IpnLocal.LoginProfile?, connectAction: () -> Unit, loginAction: () -> Unit, loginAtUrlAction: (String) -> Unit, - selfNode: Tailcfg.Node? + selfNode: Tailcfg.Node?, + showVPNPermissionLauncherIfUnauthorized: () -> Unit ) { + LaunchedEffect(isPrepared) { + if (!isPrepared) { + showVPNPermissionLauncherIfUnauthorized() + } +} Row(horizontalArrangement = Arrangement.Center, modifier = Modifier.fillMaxWidth()) { Column(horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier.fillMaxWidth()) { Column( @@ -313,7 +327,24 @@ fun ConnectView( verticalArrangement = Arrangement.spacedBy(8.dp, alignment = Alignment.CenterVertically), horizontalAlignment = Alignment.CenterHorizontally, ) { - if (state == Ipn.State.NeedsMachineAuth) { + if (!isPrepared) { + TailscaleLogoView(modifier = Modifier.size(50.dp)) + Spacer(modifier = Modifier.size(1.dp)) + Text( + text = stringResource(id = R.string.welcome_to_tailscale), + style = MaterialTheme.typography.titleMedium, + textAlign = TextAlign.Center) + Text( + stringResource(R.string.give_permissions), + style = MaterialTheme.typography.titleSmall, + textAlign = TextAlign.Center) + Spacer(modifier = Modifier.size(1.dp)) + PrimaryActionButton(onClick = connectAction) { + Text( + text = stringResource(id = R.string.connect), + fontSize = MaterialTheme.typography.titleMedium.fontSize) + } + } else if (state == Ipn.State.NeedsMachineAuth) { Icon( modifier = Modifier.size(40.dp), imageVector = Icons.Outlined.Lock, diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt index b61fa75..e051e8f 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt @@ -59,19 +59,11 @@ open class IpnViewModel : ViewModel() { } // VPN Control - - fun toggleVpn() { - when (Notifier.state.value) { - Ipn.State.Running -> stopVPN() - else -> startVPN() - } - } - fun startVPN() { UninitializedApp.get().startVPN() } - private fun stopVPN() { + fun stopVPN() { UninitializedApp.get().stopVPN() } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt index 27ecac0..8a01cd5 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt @@ -3,10 +3,15 @@ package com.tailscale.ipn.ui.viewModel +import android.content.Intent +import android.net.VpnService import android.util.Log +import androidx.activity.result.ActivityResultLauncher import androidx.lifecycle.viewModelScope +import com.tailscale.ipn.App import com.tailscale.ipn.R import com.tailscale.ipn.mdm.MDMSettings +import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.model.Ipn.State import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.util.PeerCategorizer @@ -16,17 +21,23 @@ import com.tailscale.ipn.ui.util.set import com.tailscale.ipn.App import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch import java.time.Duration class MainViewModel : IpnViewModel() { // The user readable state of the system - val stateRes: StateFlow = MutableStateFlow(State.NoState.userStringRes(null)) + val stateRes: StateFlow = MutableStateFlow(userStringRes(State.NoState, State.NoState, true)) // The expected state of the VPN toggle private val _vpnToggleState = MutableStateFlow(false) - val vpnToggleState: StateFlow = _vpnToggleState + val vpnToggleState: StateFlow = _vpnToggleState + + // Whether or not the VPN has been prepared + private val _vpnPrepared = MutableStateFlow(false) + val vpnPrepared: StateFlow = _vpnPrepared + private var vpnPermissionLauncher: ActivityResultLauncher? = null // The list of peers val peers: StateFlow> = MutableStateFlow(emptyList()) @@ -49,16 +60,20 @@ class MainViewModel : IpnViewModel() { viewModelScope.launch { var previousState: State? = null - Notifier.state.collect { currentState -> - val userString = currentState.userStringRes(previousState) - stateRes.set(userString) - _vpnToggleState.value = when { - currentState == State.Running || currentState == State.Starting -> true - previousState == State.NoState && currentState == State.Starting -> true - else -> false + + combine(Notifier.state, vpnPrepared) { state, prepared -> state to prepared } + .collect { (currentState, prepared) -> + stateRes.set(userStringRes(currentState, previousState, prepared)) + + val isOn = when { + currentState == State.Running || currentState == State.Starting -> true + previousState == State.NoState && currentState == State.Starting -> true + else -> false + } + + _vpnToggleState.value = isOn + previousState = currentState } - previousState = currentState - } } viewModelScope.launch { @@ -91,22 +106,48 @@ class MainViewModel : IpnViewModel() { } } + fun showVPNPermissionLauncherIfUnauthorized() { + val vpnIntent = VpnService.prepare(App.get()) + if (vpnIntent != null) { + vpnPermissionLauncher?.launch(vpnIntent) + } + } + + fun toggleVpn() { + val state = Notifier.state.value + val isPrepared = vpnPrepared.value + + when { + !isPrepared -> showVPNPermissionLauncherIfUnauthorized() + state == Ipn.State.Running -> stopVPN() + else -> startVPN() + } +} + fun searchPeers(searchTerm: String) { this.searchTerm.set(searchTerm) } + + fun setVpnPermissionLauncher(launcher: ActivityResultLauncher) { + vpnPermissionLauncher = launcher + } + + fun setVpnPrepared(prepared: Boolean) { + _vpnPrepared.value = prepared + } } -private fun State?.userStringRes(previousState: State?): Int { - val resId = when { - previousState == State.NoState && this == State.Starting -> R.string.starting - this == State.NoState -> R.string.placeholder - this == State.InUseOtherUser -> R.string.placeholder - this == State.NeedsLogin -> R.string.please_login - this == State.NeedsMachineAuth -> R.string.needs_machine_auth - this == State.Stopped -> R.string.stopped - this == State.Starting -> R.string.starting - this == State.Running -> R.string.connected +private fun userStringRes(currentState: State?, previousState: State?, vpnPrepared: Boolean): Int { + return when { + previousState == State.NoState && currentState == State.Starting -> R.string.starting + currentState == State.NoState -> R.string.placeholder + currentState == State.InUseOtherUser -> R.string.placeholder + currentState == State.NeedsLogin -> if (vpnPrepared) R.string.please_login else R.string.connect_to_vpn + currentState == State.NeedsMachineAuth -> R.string.needs_machine_auth + currentState == State.Stopped -> R.string.stopped + currentState == State.Starting -> R.string.starting + currentState == State.Running -> R.string.connected else -> R.string.placeholder } return resId -} +} \ No newline at end of file diff --git a/android/src/main/res/values/strings.xml b/android/src/main/res/values/strings.xml index f2cc400..8a59d1d 100644 --- a/android/src/main/res/values/strings.xml +++ b/android/src/main/res/values/strings.xml @@ -58,6 +58,7 @@ " tailnet." Welcome to Tailscale Log in to join your tailnet and connect your devices. + Set up a VPN connection so you can start using Tailscale. Reauthenticate to remain connected to Tailscale. Device key does not expire Device key expires %s @@ -87,6 +88,7 @@ -- Login required Not connected + Connect to VPN expired