mirror of
https://github.com/bitfireAT/davx5-ose
synced 2024-07-23 11:39:15 +00:00
Add setting to ignore VPNs at connection detection (bitfireAT/davx5#356)
* Add setting to ignore VPNs at connection detection * Minor changes - move methods to ConnectionUtils to keep SyncWorker class compact - always use "ignore VPNs" as Boolean - other minor changes * Show ignore VPNs setting only below api lvl 23 * Change strings --------- Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Sunik Kupfer
Ricki Hirner
parent
62cca2939a
commit
1c419cd75c
|
|
@ -18,6 +18,7 @@ import androidx.work.workDataOf
|
|||
import at.bitfire.davdroid.R
|
||||
import at.bitfire.davdroid.TestUtils.workScheduledOrRunningOrSuccessful
|
||||
import at.bitfire.davdroid.db.Credentials
|
||||
import at.bitfire.davdroid.network.ConnectionUtils
|
||||
import at.bitfire.davdroid.settings.AccountSettings
|
||||
import at.bitfire.davdroid.ui.NotificationUtils
|
||||
import dagger.hilt.android.testing.HiltAndroidRule
|
||||
|
|
@ -96,8 +97,9 @@ class SyncWorkerTest {
|
|||
val accountSettings = AccountSettings(context, account)
|
||||
accountSettings.setSyncWiFiOnly(true)
|
||||
|
||||
mockkObject(ConnectionUtils)
|
||||
every { ConnectionUtils.wifiAvailable(any()) } returns true
|
||||
mockkObject(SyncWorker.Companion)
|
||||
every { SyncWorker.Companion.wifiAvailable(any()) } returns true
|
||||
every { SyncWorker.Companion.correctWifiSsid(any(), any()) } returns true
|
||||
|
||||
assertTrue(SyncWorker.wifiConditionsMet(context, accountSettings))
|
||||
|
|
@ -108,8 +110,9 @@ class SyncWorkerTest {
|
|||
val accountSettings = AccountSettings(context, account)
|
||||
accountSettings.setSyncWiFiOnly(true)
|
||||
|
||||
mockkObject(ConnectionUtils)
|
||||
every { ConnectionUtils.wifiAvailable(any()) } returns false
|
||||
mockkObject(SyncWorker.Companion)
|
||||
every { SyncWorker.Companion.wifiAvailable(any()) } returns false
|
||||
every { SyncWorker.Companion.correctWifiSsid(any(), any()) } returns true
|
||||
|
||||
assertFalse(SyncWorker.wifiConditionsMet(context, accountSettings))
|
||||
|
|
|
|||
|
|
@ -0,0 +1,74 @@
|
|||
/***************************************************************************************************
|
||||
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
|
||||
**************************************************************************************************/
|
||||
|
||||
package at.bitfire.davdroid.network
|
||||
|
||||
import android.content.Context
|
||||
import android.net.ConnectivityManager
|
||||
import android.net.NetworkCapabilities
|
||||
import androidx.annotation.RequiresApi
|
||||
import androidx.core.content.getSystemService
|
||||
import at.bitfire.davdroid.log.Logger
|
||||
import java.util.logging.Level
|
||||
|
||||
object ConnectionUtils {
|
||||
|
||||
/**
|
||||
* Checks whether we are connected to working WiFi
|
||||
*/
|
||||
internal fun wifiAvailable(context: Context): Boolean {
|
||||
val connectivityManager = context.getSystemService<ConnectivityManager>()!!
|
||||
connectivityManager.allNetworks.forEach { network ->
|
||||
connectivityManager.getNetworkCapabilities(network)?.let { capabilities ->
|
||||
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) &&
|
||||
capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED))
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether we are connected to the Internet.
|
||||
*
|
||||
* On API 26+ devices, if a VPN is used, WorkManager might start the SyncWorker without an
|
||||
* internet connection (because NET_CAPABILITY_VALIDATED is always set for VPN connections).
|
||||
* To prevent the start without internet access, we don't check for VPN connections by default
|
||||
* (by using [NetworkCapabilities.NET_CAPABILITY_NOT_VPN]).
|
||||
*
|
||||
* However in special occasions (when syncing over a VPN without validated Internet on the
|
||||
* underlying connection) we do not want to exclude VPNs.
|
||||
*
|
||||
* @param ignoreVpns *true* filters VPN connections in the Internet check; *false* allows them as valid connection
|
||||
* @return whether we are connected to the Internet
|
||||
*/
|
||||
@RequiresApi(23)
|
||||
internal fun internetAvailable(context: Context, ignoreVpns: Boolean): Boolean {
|
||||
val connectivityManager = context.getSystemService<ConnectivityManager>()!!
|
||||
return connectivityManager.allNetworks.any { network ->
|
||||
Logger.log.log(Level.FINE, "Looking for validated Internet", connectivityManager.getNetworkInfo(network))
|
||||
|
||||
connectivityManager.getNetworkCapabilities(network)?.let { capabilities ->
|
||||
if (!capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
|
||||
Logger.log.fine("Missing network capability: INTERNET")
|
||||
return false
|
||||
}
|
||||
|
||||
if (!capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)) {
|
||||
Logger.log.fine("Missing network capability: VALIDATED")
|
||||
return false
|
||||
}
|
||||
|
||||
if (ignoreVpns)
|
||||
if (!capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) {
|
||||
Logger.log.fine("Missing network capability: NOT_VPN")
|
||||
return false
|
||||
}
|
||||
|
||||
/* return */ true
|
||||
} ?: false
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -70,6 +70,7 @@ class AccountSettings(
|
|||
|
||||
const val KEY_WIFI_ONLY = "wifi_only" // sync on WiFi only (default: false)
|
||||
const val KEY_WIFI_ONLY_SSIDS = "wifi_only_ssids" // restrict sync to specific WiFi SSIDs
|
||||
const val KEY_IGNORE_VPNS = "ignore_vpns" // ignore vpns at connection detection
|
||||
|
||||
/** Time range limitation to the past [in days]. Values:
|
||||
*
|
||||
|
|
@ -318,10 +319,10 @@ class AccountSettings(
|
|||
}
|
||||
|
||||
fun getSyncWifiOnly() =
|
||||
if (settings.containsKey(KEY_WIFI_ONLY))
|
||||
settings.getBoolean(KEY_WIFI_ONLY)
|
||||
else
|
||||
accountManager.getUserData(account, KEY_WIFI_ONLY) != null
|
||||
if (settings.containsKey(KEY_WIFI_ONLY))
|
||||
settings.getBoolean(KEY_WIFI_ONLY)
|
||||
else
|
||||
accountManager.getUserData(account, KEY_WIFI_ONLY) != null
|
||||
|
||||
fun setSyncWiFiOnly(wiFiOnly: Boolean) {
|
||||
accountManager.setAndVerifyUserData(account, KEY_WIFI_ONLY, if (wiFiOnly) "1" else null)
|
||||
|
|
@ -332,16 +333,26 @@ class AccountSettings(
|
|||
}
|
||||
|
||||
fun getSyncWifiOnlySSIDs(): List<String>? =
|
||||
if (getSyncWifiOnly()) {
|
||||
val strSsids = if (settings.containsKey(KEY_WIFI_ONLY_SSIDS))
|
||||
settings.getString(KEY_WIFI_ONLY_SSIDS)
|
||||
else
|
||||
accountManager.getUserData(account, KEY_WIFI_ONLY_SSIDS)
|
||||
strSsids?.split(',')
|
||||
} else
|
||||
null
|
||||
if (getSyncWifiOnly()) {
|
||||
val strSsids = if (settings.containsKey(KEY_WIFI_ONLY_SSIDS))
|
||||
settings.getString(KEY_WIFI_ONLY_SSIDS)
|
||||
else
|
||||
accountManager.getUserData(account, KEY_WIFI_ONLY_SSIDS)
|
||||
strSsids?.split(',')
|
||||
} else
|
||||
null
|
||||
fun setSyncWifiOnlySSIDs(ssids: List<String>?) =
|
||||
accountManager.setAndVerifyUserData(account, KEY_WIFI_ONLY_SSIDS, StringUtils.trimToNull(ssids?.joinToString(",")))
|
||||
accountManager.setAndVerifyUserData(account, KEY_WIFI_ONLY_SSIDS, StringUtils.trimToNull(ssids?.joinToString(",")))
|
||||
|
||||
fun getIgnoreVpns(): Boolean =
|
||||
when (accountManager.getUserData(account, KEY_IGNORE_VPNS)) {
|
||||
null -> settings.getBoolean(KEY_IGNORE_VPNS)
|
||||
"0" -> false
|
||||
else -> true
|
||||
}
|
||||
|
||||
fun setIgnoreVpns(ignoreVpns: Boolean) =
|
||||
accountManager.setAndVerifyUserData(account, KEY_IGNORE_VPNS, if (ignoreVpns) "1" else "0")
|
||||
|
||||
/**
|
||||
* Updates the periodic sync worker of an authority according to
|
||||
|
|
|
|||
|
|
@ -20,7 +20,8 @@ class DefaultsProvider(
|
|||
|
||||
override val booleanDefaults = mutableMapOf(
|
||||
Pair(Settings.DISTRUST_SYSTEM_CERTIFICATES, false),
|
||||
Pair(Settings.FORCE_READ_ONLY_ADDRESSBOOKS, false)
|
||||
Pair(Settings.FORCE_READ_ONLY_ADDRESSBOOKS, false),
|
||||
Pair(Settings.IGNORE_VPN_NETWORK_CAPABILITY, true)
|
||||
)
|
||||
|
||||
override val intDefaults = mapOf(
|
||||
|
|
|
|||
|
|
@ -22,7 +22,13 @@ object Settings {
|
|||
const val PROXY_PORT = "proxy_port" // Integer
|
||||
|
||||
/**
|
||||
* Default sync interval (long), in seconds.
|
||||
* Whether to ignore VPNs at internet connection detection, true by default because VPN connections
|
||||
* seem to include "VALIDATED" by default even without actual internet connection
|
||||
*/
|
||||
const val IGNORE_VPN_NETWORK_CAPABILITY = "ignore_vpns" // Boolean
|
||||
|
||||
/**
|
||||
* Default sync interval (Long), in seconds.
|
||||
* Used to initialize an account.
|
||||
*/
|
||||
const val DEFAULT_SYNC_INTERVAL = "default_sync_interval"
|
||||
|
|
|
|||
|
|
@ -11,14 +11,11 @@ import android.content.ContentResolver
|
|||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import android.content.SyncResult
|
||||
import android.net.ConnectivityManager
|
||||
import android.net.NetworkCapabilities
|
||||
import android.net.wifi.WifiManager
|
||||
import android.os.Build
|
||||
import android.provider.CalendarContract
|
||||
import android.provider.ContactsContract
|
||||
import androidx.annotation.IntDef
|
||||
import androidx.annotation.RequiresApi
|
||||
import androidx.concurrent.futures.CallbackToFutureAdapter
|
||||
import androidx.core.app.NotificationCompat
|
||||
import androidx.core.app.NotificationManagerCompat
|
||||
|
|
@ -42,6 +39,8 @@ import androidx.work.Worker
|
|||
import androidx.work.WorkerParameters
|
||||
import at.bitfire.davdroid.R
|
||||
import at.bitfire.davdroid.log.Logger
|
||||
import at.bitfire.davdroid.network.ConnectionUtils.internetAvailable
|
||||
import at.bitfire.davdroid.network.ConnectionUtils.wifiAvailable
|
||||
import at.bitfire.davdroid.settings.AccountSettings
|
||||
import at.bitfire.davdroid.ui.DebugInfoActivity
|
||||
import at.bitfire.davdroid.ui.NotificationUtils
|
||||
|
|
@ -206,44 +205,7 @@ class SyncWorker @AssistedInject constructor(
|
|||
}
|
||||
|
||||
|
||||
/**
|
||||
* Checks whether user imposed sync conditions from settings are met:
|
||||
* - Sync only on WiFi?
|
||||
* - Sync only on specific WiFi (SSID)?
|
||||
*
|
||||
* @param accountSettings Account settings of the account to check (and is to be synced)
|
||||
* @return *true* if conditions are met; *false* if not
|
||||
*/
|
||||
internal fun wifiConditionsMet(context: Context, accountSettings: AccountSettings): Boolean {
|
||||
// May we sync without WiFi?
|
||||
if (!accountSettings.getSyncWifiOnly())
|
||||
return true // yes, continue
|
||||
|
||||
// WiFi required, is it available?
|
||||
if (!wifiAvailable(context)) {
|
||||
Logger.log.info("Not on connected WiFi, stopping")
|
||||
return false
|
||||
}
|
||||
// If execution reaches this point, we're on a connected WiFi
|
||||
|
||||
// Check whether we are connected to the correct WiFi (in case SSID was provided)
|
||||
return correctWifiSsid(context, accountSettings)
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether we are connected to working WiFi
|
||||
*/
|
||||
internal fun wifiAvailable(context: Context): Boolean {
|
||||
val connectivityManager = context.getSystemService<ConnectivityManager>()!!
|
||||
connectivityManager.allNetworks.forEach { network ->
|
||||
connectivityManager.getNetworkCapabilities(network)?.let { capabilities ->
|
||||
if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) &&
|
||||
capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED))
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
// connection checks
|
||||
|
||||
/**
|
||||
* Checks whether we are connected to the correct wifi (SSID) defined by user in the
|
||||
|
|
@ -279,8 +241,34 @@ class SyncWorker @AssistedInject constructor(
|
|||
}
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether user imposed sync conditions from settings are met:
|
||||
* - Sync only on WiFi?
|
||||
* - Sync only on specific WiFi (SSID)?
|
||||
*
|
||||
* @param accountSettings Account settings of the account to check (and is to be synced)
|
||||
* @return *true* if conditions are met; *false* if not
|
||||
*/
|
||||
internal fun wifiConditionsMet(context: Context, accountSettings: AccountSettings): Boolean {
|
||||
// May we sync without WiFi?
|
||||
if (!accountSettings.getSyncWifiOnly())
|
||||
return true // yes, continue
|
||||
|
||||
// WiFi required, is it available?
|
||||
if (!wifiAvailable(context)) {
|
||||
Logger.log.info("Not on connected WiFi, stopping")
|
||||
return false
|
||||
}
|
||||
// If execution reaches this point, we're on a connected WiFi
|
||||
|
||||
// Check whether we are connected to the correct WiFi (in case SSID was provided)
|
||||
return correctWifiSsid(context, accountSettings)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
private val notificationManager = NotificationManagerCompat.from(applicationContext)
|
||||
|
||||
/** thread which runs the actual sync code (can be interrupted to stop synchronization) */
|
||||
|
|
@ -288,19 +276,20 @@ class SyncWorker @AssistedInject constructor(
|
|||
|
||||
override fun doWork(): Result {
|
||||
|
||||
// Check internet connection. This is especially important on API 26+ where when a VPN is used,
|
||||
// WorkManager may start the SyncWorker without a working underlying Internet connection.
|
||||
if (Build.VERSION.SDK_INT >= 23 && !internetAvailable(applicationContext)) {
|
||||
Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.")
|
||||
return Result.failure()
|
||||
}
|
||||
|
||||
// ensure we got the required arguments
|
||||
val account = Account(
|
||||
inputData.getString(ARG_ACCOUNT_NAME) ?: throw IllegalArgumentException("$ARG_ACCOUNT_NAME required"),
|
||||
inputData.getString(ARG_ACCOUNT_TYPE) ?: throw IllegalArgumentException("$ARG_ACCOUNT_TYPE required")
|
||||
)
|
||||
val authority = inputData.getString(ARG_AUTHORITY) ?: throw IllegalArgumentException("$ARG_AUTHORITY required")
|
||||
|
||||
// Check internet connection
|
||||
val ignoreVpns = AccountSettings(applicationContext, account).getIgnoreVpns()
|
||||
if (Build.VERSION.SDK_INT >= 23 && !internetAvailable(applicationContext, ignoreVpns)) {
|
||||
Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.")
|
||||
return Result.failure()
|
||||
}
|
||||
|
||||
Logger.log.info("Running sync worker: account=$account, authority=$authority")
|
||||
|
||||
// What are we going to sync? Select syncer based on authority
|
||||
|
|
@ -408,31 +397,6 @@ class SyncWorker @AssistedInject constructor(
|
|||
return Result.success()
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether we are connected to the internet.
|
||||
*
|
||||
* On API 26+ devices, when a VPN is used, WorkManager might start the SyncWorker without an
|
||||
* internet connection. To prevent this we do an extra check at the start of doWork() with this
|
||||
* method.
|
||||
*
|
||||
* Every VPN connection also has an underlying non-vpn connection, which we find with
|
||||
* [NetworkCapabilities.NET_CAPABILITY_NOT_VPN] and then check if that has validated internet
|
||||
* access or not, using [NetworkCapabilities.NET_CAPABILITY_VALIDATED].
|
||||
*
|
||||
* @return whether we are connected to the internet
|
||||
*/
|
||||
@RequiresApi(23)
|
||||
private fun internetAvailable(context: Context): Boolean {
|
||||
val connectivityManager = context.getSystemService<ConnectivityManager>()!!
|
||||
return connectivityManager.allNetworks.any { network ->
|
||||
connectivityManager.getNetworkCapabilities(network)?.let { capabilities ->
|
||||
capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) // filter out VPNs
|
||||
&& capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
|
||||
&& capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
|
||||
} ?: false
|
||||
}
|
||||
}
|
||||
|
||||
override fun onStopped() {
|
||||
Logger.log.info("Stopping sync thread")
|
||||
syncThread?.interrupt()
|
||||
|
|
|
|||
|
|
@ -217,6 +217,18 @@ class SettingsActivity: AppCompatActivity() {
|
|||
}
|
||||
}
|
||||
|
||||
findPreference<SwitchPreferenceCompat>(getString(R.string.settings_ignore_vpns_key))!!.let {
|
||||
model.ignoreVpns.observe(viewLifecycleOwner) { ignoreVpns ->
|
||||
it.isEnabled = true
|
||||
it.isChecked = ignoreVpns
|
||||
it.isVisible = Build.VERSION.SDK_INT >= 23
|
||||
it.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, prefValue ->
|
||||
model.updateIgnoreVpns(prefValue as Boolean)
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// preference group: authentication
|
||||
val prefUserName = findPreference<EditTextPreference>(getString(R.string.settings_username_key))!!
|
||||
val prefPassword = findPreference<EditTextPreference>(getString(R.string.settings_password_key))!!
|
||||
|
|
@ -443,6 +455,7 @@ class SettingsActivity: AppCompatActivity() {
|
|||
|
||||
val syncWifiOnly = MutableLiveData<Boolean>()
|
||||
val syncWifiOnlySSIDs = MutableLiveData<List<String>>()
|
||||
val ignoreVpns = MutableLiveData<Boolean>()
|
||||
|
||||
val credentials = MutableLiveData<Credentials>()
|
||||
|
||||
|
|
@ -481,6 +494,7 @@ class SettingsActivity: AppCompatActivity() {
|
|||
|
||||
syncWifiOnly.postValue(accountSettings.getSyncWifiOnly())
|
||||
syncWifiOnlySSIDs.postValue(accountSettings.getSyncWifiOnlySSIDs())
|
||||
ignoreVpns.postValue(accountSettings.getIgnoreVpns())
|
||||
|
||||
credentials.postValue(accountSettings.credentials())
|
||||
|
||||
|
|
@ -510,6 +524,11 @@ class SettingsActivity: AppCompatActivity() {
|
|||
reload()
|
||||
}
|
||||
|
||||
fun updateIgnoreVpns(ignoreVpns: Boolean) {
|
||||
accountSettings?.setIgnoreVpns(ignoreVpns)
|
||||
reload()
|
||||
}
|
||||
|
||||
fun updateCredentials(credentials: Credentials) {
|
||||
accountSettings?.credentials(credentials)
|
||||
reload()
|
||||
|
|
|
|||
|
|
@ -345,6 +345,10 @@
|
|||
<string name="settings_sync_wifi_only_ssids_message">Comma-separated names (SSIDs) of allowed WiFi networks (leave blank for all)</string>
|
||||
<string name="settings_sync_wifi_only_ssids_permissions_required">WiFi SSID restriction requires further settings</string>
|
||||
<string name="settings_sync_wifi_only_ssids_permissions_action">Manage</string>
|
||||
<string name="settings_ignore_vpns_key" translatable="false">ignore_vpns</string>
|
||||
<string name="settings_ignore_vpns">VPN connectivity</string>
|
||||
<string name="settings_ignore_vpns_on">Non-VPN connection required (recommended)</string>
|
||||
<string name="settings_ignore_vpns_off">VPN counts as Internet connection</string>
|
||||
<string name="settings_authentication">Authentication</string>
|
||||
<string name="settings_oauth_key" translatable="false">oauth</string>
|
||||
<string name="settings_oauth">Re-authenticate</string>
|
||||
|
|
|
|||
|
|
@ -43,6 +43,13 @@
|
|||
android:title="@string/settings_sync_wifi_only_ssids"
|
||||
android:dialogMessage="@string/settings_sync_wifi_only_ssids_message"/>
|
||||
|
||||
<SwitchPreferenceCompat
|
||||
android:key="@string/settings_ignore_vpns_key"
|
||||
android:persistent="false"
|
||||
android:title="@string/settings_ignore_vpns"
|
||||
android:summaryOn="@string/settings_ignore_vpns_on"
|
||||
android:summaryOff="@string/settings_ignore_vpns_off" />
|
||||
|
||||
</PreferenceCategory>
|
||||
|
||||
<PreferenceCategory
|
||||
|
|
|
|||
Loading…
Reference in a new issue