mdm: improve handling and returning of not configured policy settings (#461)

We should distinguish between unconfigured policy settings and those configured with the default values.
In the first case, the syspolicyHandler should return syspolicy.ErrNoSuchKey instead of the default value,
while in the latter case, it should return the actual setting value, even if that value happens to be the default
value such as "user-decides". This distinction should also be reflected in the "Current MDM settings" view.

In this PR, we update MDMSetting.flow to hold both the value to be used by the app and a flag indicating
whether the policy setting is configured or not. If the policy setting is not configured, the value is the default
value for the setting type. We then use this new flag to decide whether to throw a NoSuchKeyException from
the Kotlin-side of the syspolicyHandler implementation and how to display the policy setting in the
"Current MDM settings" view.

Additionally, we update the MDMSettings.update and MDMSetting.setFrom methods to avoid calling
app.getEncryptedPrefs (and reading/decrypting the prefs) for every defined MDM setting.

Updates tailscale/tailscale#12687

Signed-off-by: Nick Khyl <nickk@tailscale.com>

android/src/main/java/com/tailscale/ipn/App.kt

@@ -316,21 +316,25 @@ class App : UninitializedApp(), libtailscale.AppContext {
   @Throws(
       IOException::class, GeneralSecurityException::class, MDMSettings.NoSuchKeyException::class)
   override fun getSyspolicyStringValue(key: String): String {
-    return MDMSettings.allSettingsByKey[key]?.flow?.value?.toString()
-        ?: run {
-          Log.d("MDM", "$key is not defined on Android. Throwing NoSuchKeyException.")
-          throw MDMSettings.NoSuchKeyException()
-        }
+    val setting = MDMSettings.allSettingsByKey[key]?.flow?.value
+    if (setting?.isSet != true) {
+      throw MDMSettings.NoSuchKeyException()
+    }
+    return setting.value?.toString() ?: ""
   }
 
   @Throws(
       IOException::class, GeneralSecurityException::class, MDMSettings.NoSuchKeyException::class)
   override fun getSyspolicyStringArrayJSONValue(key: String): String {
-    val list = MDMSettings.allSettingsByKey[key]?.flow?.value as? List<*>
+    val setting = MDMSettings.allSettingsByKey[key]?.flow?.value
+    if (setting?.isSet != true) {
+      throw MDMSettings.NoSuchKeyException()
+    }
     try {
+      val list = setting.value as? List<*>
       return Json.encodeToString(list)
     } catch (e: Exception) {
-      Log.d("MDM", "$key is not defined on Android. Throwing NoSuchKeyException.")
+      Log.d("MDM", "$key value cannot be serialized to JSON. Throwing NoSuchKeyException.")
       throw MDMSettings.NoSuchKeyException()
     }
   }
@@ -510,7 +514,7 @@ open class UninitializedApp : Application() {
   }
 
   fun disallowedPackageNames(): List<String> {
-    val mdmDisallowed = MDMSettings.excludedPackages.flow.value?.split(",")?.map { it.trim() } ?: emptyList()
+    val mdmDisallowed = MDMSettings.excludedPackages.flow.value.value?.split(",")?.map { it.trim() } ?: emptyList()
     if (mdmDisallowed.isNotEmpty()) {
       Log.d(TAG, "Excluded application packages were set via MDM: $mdmDisallowed")
       return builtInDisallowedPackageNames + mdmDisallowed

android/src/main/java/com/tailscale/ipn/IPNService.kt

@@ -116,7 +116,7 @@ open class IPNService : VpnService(), libtailscale.IPNService {
     b.setUnderlyingNetworks(null) // Use all available networks.
 
     val includedPackages: List<String> =
-        MDMSettings.includedPackages.flow.value?.split(",")?.map { it.trim() } ?: emptyList()
+        MDMSettings.includedPackages.flow.value.value?.split(",")?.map { it.trim() } ?: emptyList()
     if (includedPackages.isNotEmpty()) {
       // If an admin defined a list of packages that are exclusively allowed to be used via
       // Tailscale,

android/src/main/java/com/tailscale/ipn/mdm/MDMSettings.kt

@@ -105,6 +105,7 @@ object MDMSettings {
 
   fun update(app: App, restrictionsManager: RestrictionsManager?) {
     val bundle = restrictionsManager?.applicationRestrictions
-    allSettings.forEach { it.setFrom(bundle, app) }
+    val preferences = lazy { app.getEncryptedPrefs() }
+    allSettings.forEach { it.setFrom(bundle, preferences) }
   }
 }

android/src/main/java/com/tailscale/ipn/mdm/MDMSettingsDefinitions.kt

@@ -3,77 +3,68 @@
 
 package com.tailscale.ipn.mdm
 
+import android.content.SharedPreferences
 import android.os.Bundle
 import com.tailscale.ipn.App
 import com.tailscale.ipn.ui.util.set
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 
+data class SettingState<T>(val value: T, val isSet: Boolean)
+
 abstract class MDMSetting<T>(defaultValue: T, val key: String, val localizedTitle: String) {
-  val flow: StateFlow<T> = MutableStateFlow<T>(defaultValue)
+  val defaultValue = defaultValue
+  val flow = MutableStateFlow(SettingState(defaultValue, false))
+
+  fun setFrom(bundle: Bundle?, prefs: Lazy<SharedPreferences>) {
+    val v: T? = getFrom(bundle, prefs)
+    flow.set(SettingState(v ?: defaultValue, v != null))
+  }
 
-  fun setFrom(bundle: Bundle?, app: App) {
-    val v = getFrom(bundle, app)
-    flow.set(v)
+  fun getFrom(bundle: Bundle?, prefs: Lazy<SharedPreferences>): T? {
+    return when {
+      bundle != null -> bundle.takeIf { it.containsKey(key) }?.let { getFromBundle(it) }
+      else -> prefs.value.takeIf { it.contains(key) }?.let { getFromPrefs(it) }
+    }
   }
 
-  abstract fun getFrom(bundle: Bundle?, app: App): T
+  protected abstract fun getFromBundle(bundle: Bundle): T
+  protected abstract fun getFromPrefs(prefs: SharedPreferences): T
 }
 
 class BooleanMDMSetting(key: String, localizedTitle: String) :
     MDMSetting<Boolean>(false, key, localizedTitle) {
-  override fun getFrom(bundle: Bundle?, app: App) =
-      bundle?.getBoolean(key) ?: app.getEncryptedPrefs().getBoolean(key, false)
+  override fun getFromBundle(bundle: Bundle) = bundle.getBoolean(key)
+  override fun getFromPrefs(prefs: SharedPreferences) = prefs.getBoolean(key, false)
 }
 
 class StringMDMSetting(key: String, localizedTitle: String) :
     MDMSetting<String?>(null, key, localizedTitle) {
-  override fun getFrom(bundle: Bundle?, app: App) =
-      bundle?.getString(key) ?: app.getEncryptedPrefs().getString(key, null)
+  override fun getFromBundle(bundle: Bundle) = bundle.getString(key)
+  override fun getFromPrefs(prefs: SharedPreferences) = prefs.getString(key, null)
 }
 
 class StringArrayListMDMSetting(key: String, localizedTitle: String) :
     MDMSetting<List<String>?>(null, key, localizedTitle) {
-  override fun getFrom(bundle: Bundle?, app: App) =
-      bundle?.getStringArrayList(key)
-          ?: app.getEncryptedPrefs().getStringSet(key, HashSet<String>())?.toList()
+  override fun getFromBundle(bundle: Bundle) = bundle.getStringArrayList(key)
+  override fun getFromPrefs(prefs: SharedPreferences) =
+      prefs.getStringSet(key, HashSet<String>())?.toList()
 }
 
 class AlwaysNeverUserDecidesMDMSetting(key: String, localizedTitle: String) :
     MDMSetting<AlwaysNeverUserDecides>(AlwaysNeverUserDecides.UserDecides, key, localizedTitle) {
-  override fun getFrom(bundle: Bundle?, app: App): AlwaysNeverUserDecides {
-    val storedString =
-        bundle?.getString(key)
-            ?: App.get().getEncryptedPrefs().getString(key, null)
-            ?: "user-decides"
-    return when (storedString) {
-      "always" -> {
-        AlwaysNeverUserDecides.Always
-      }
-      "never" -> {
-        AlwaysNeverUserDecides.Never
-      }
-      else -> {
-        AlwaysNeverUserDecides.UserDecides
-      }
-    }
-  }
+  override fun getFromBundle(bundle: Bundle) =
+      AlwaysNeverUserDecides.fromString(bundle.getString(key))
+  override fun getFromPrefs(prefs: SharedPreferences) =
+      AlwaysNeverUserDecides.fromString(prefs.getString(key, null))
 }
 
 class ShowHideMDMSetting(key: String, localizedTitle: String) :
     MDMSetting<ShowHide>(ShowHide.Show, key, localizedTitle) {
-  override fun getFrom(bundle: Bundle?, app: App): ShowHide {
-    val storedString =
-        bundle?.getString(key) ?: App.get().getEncryptedPrefs().getString(key, null) ?: "show"
-    return when (storedString) {
-      "hide" -> {
-        ShowHide.Hide
-      }
-      else -> {
-        ShowHide.Show
-      }
-    }
-  }
+  override fun getFromBundle(bundle: Bundle) =
+      ShowHide.fromString(bundle.getString(key))
+  override fun getFromPrefs(prefs: SharedPreferences) =
+      ShowHide.fromString(prefs.getString(key, null))
 }
 
 enum class AlwaysNeverUserDecides(val value: String) {
@@ -89,6 +80,12 @@ enum class AlwaysNeverUserDecides(val value: String) {
   override fun toString(): String {
     return value
   }
+
+  companion object {
+    fun fromString(value: String?): AlwaysNeverUserDecides {
+      return values().find { it.value == value } ?: UserDecides
+    }
+  }
 }
 
 enum class ShowHide(val value: String) {
@@ -98,4 +95,10 @@ enum class ShowHide(val value: String) {
   override fun toString(): String {
     return value
   }
+
+  companion object {
+    fun fromString(value: String?): ShowHide {
+      return ShowHide.values().find { it.value == value } ?: Show
+    }
+  }
 }

android/src/main/java/com/tailscale/ipn/ui/util/PeerHelper.kt

@@ -21,7 +21,7 @@ class PeerCategorizer {
     val selfNode = netmap.SelfNode
     var grouped = mutableMapOf<UserID, MutableList<Tailcfg.Node>>()
 
-    val mdm = MDMSettings.hiddenNetworkDevices.flow.value
+    val mdm = MDMSettings.hiddenNetworkDevices.flow.value.value
     val hideMyDevices = mdm?.contains("current-user") ?: false
     val hideOtherDevices = mdm?.contains("other-users") ?: false
     val hideTaggedDevices = mdm?.contains("tagged-devices") ?: false

android/src/main/java/com/tailscale/ipn/ui/view/DNSSettingsView.kt

@@ -69,7 +69,7 @@ fun DNSSettingsView(
               },
               supportingContent = { Text(stringResource(state.caption)) })
 
-          if (!dnsSettingsMDMDisposition.hiddenFromUser) {
+          if (!dnsSettingsMDMDisposition.value.hiddenFromUser) {
             Lists.ItemDivider()
             Setting.Switch(
                 R.string.use_ts_dns,

android/src/main/java/com/tailscale/ipn/ui/view/ExitNodePicker.kt

@@ -54,7 +54,7 @@ fun ExitNodePicker(
       val showRunAsExitNode by MDMSettings.runExitNode.flow.collectAsState()
       val allowLanAccessMDMDisposition by MDMSettings.exitNodeAllowLANAccess.flow.collectAsState()
       val managedByOrganization by model.managedByOrganization.collectAsState()
-      val forcedExitNodeId = MDMSettings.exitNodeID.flow.collectAsState().value
+      val forcedExitNodeId = MDMSettings.exitNodeID.flow.collectAsState().value.value
 
       LazyColumn(modifier = Modifier.padding(innerPadding)) {
         item(key = "header") {
@@ -76,7 +76,7 @@ fun ExitNodePicker(
                     selected = !anyActive,
                 ))
           }
-          if (showRunAsExitNode == ShowHide.Show) {
+          if (showRunAsExitNode.value == ShowHide.Show) {
             Lists.ItemDivider()
             RunAsExitNodeItem(nav = nav, viewModel = model, anyActive)
           }
@@ -99,7 +99,7 @@ fun ExitNodePicker(
           }
         }
 
-        if (!allowLanAccessMDMDisposition.hiddenFromUser) {
+        if (!allowLanAccessMDMDisposition.value.hiddenFromUser) {
           item(key = "allowLANAccess") {
             Lists.SectionDivider()
 
@@ -121,7 +121,7 @@ fun ExitNodeItem(
 ) {
   val online by node.online.collectAsState()
   val isRunningExitNode = viewModel.isRunningExitNode.collectAsState().value
-  val forcedExitNodeId = MDMSettings.exitNodeID.flow.collectAsState().value
+  val forcedExitNodeId = MDMSettings.exitNodeID.flow.collectAsState().value.value
 
   Box {
     var modifier: Modifier = Modifier

android/src/main/java/com/tailscale/ipn/ui/view/MDMSettingsDebugView.kt

@@ -50,7 +50,7 @@ fun MDMSettingView(setting: MDMSetting<*>) {
       },
       trailingContent = {
         Text(
-            value.toString(),
+            if (value.isSet) value.value.toString() else "[not set]",
             fontFamily = FontFamily.Monospace,
             maxLines = 1,
             fontWeight = FontWeight.SemiBold)

android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt

@@ -136,7 +136,7 @@ fun MainView(
             val stateStr = stringResource(id = stateVal)
             val netmap by viewModel.netmap.collectAsState(initial = null)
             val showExitNodePicker by MDMSettings.exitNodesPicker.flow.collectAsState()
-            val disableToggle by MDMSettings.forceEnabled.flow.collectAsState(initial = true)
+            val disableToggle by MDMSettings.forceEnabled.flow.collectAsState()
             val showKeyExpiry by viewModel.showExpiry.collectAsState(initial = false)
 
             // Hide the header only on Android TV when the user needs to login
@@ -148,11 +148,11 @@ fun MainView(
                   if (!hideHeader) {
                     TintedSwitch(
                         onCheckedChange = {
-                          if (!disableToggle) {
+                          if (!disableToggle.value) {
                             viewModel.toggleVpn()
                           }
                         },
-                        enabled = !disableToggle,
+                        enabled = !disableToggle.value,
                         checked = isOn)
                   }
                 },
@@ -204,7 +204,7 @@ fun MainView(
                   HealthNotification(warning = warning)
                 }
 
-                if (showExitNodePicker == ShowHide.Show) {
+                if (showExitNodePicker.value == ShowHide.Show) {
                   ExitNodeStatus(
                       navAction = navigation.onNavigateToExitNodes, viewModel = viewModel)
                 }
@@ -273,7 +273,7 @@ fun ExitNodeStatus(navAction: () -> Unit, viewModel: MainViewModel) {
                         Modifier.padding(start = 16.dp, end = 16.dp, top = 36.dp, bottom = 16.dp)) {
                       Text(
                           text =
-                              managedByOrganization?.let {
+                              managedByOrganization.value?.let {
                                 stringResource(R.string.exit_node_offline_mdm_orgname, it)
                               } ?: stringResource(R.string.exit_node_offline_mdm),
                           style = MaterialTheme.typography.bodyMedium,

android/src/main/java/com/tailscale/ipn/ui/view/ManagedByView.kt

@@ -32,9 +32,9 @@ fun ManagedByView(backToSettings: BackNavigation, model: IpnViewModel = viewMode
         horizontalAlignment = Alignment.Start,
         modifier = Modifier.fillMaxWidth().safeContentPadding().verticalScroll(rememberScrollState())) {
           val managedByOrganization =
-              MDMSettings.managedByOrganizationName.flow.collectAsState().value
-          val managedByCaption = MDMSettings.managedByCaption.flow.collectAsState().value
-          val managedByURL = MDMSettings.managedByURL.flow.collectAsState().value
+              MDMSettings.managedByOrganizationName.flow.collectAsState().value.value
+          val managedByCaption = MDMSettings.managedByCaption.flow.collectAsState().value.value
+          val managedByURL = MDMSettings.managedByURL.flow.collectAsState().value.value
           managedByOrganization?.let {
             Text(stringResource(R.string.managed_by_explainer_orgName, it))
           } ?: run { Text(stringResource(R.string.managed_by_explainer)) }

android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt

@@ -83,7 +83,7 @@ fun SettingsView(settingsNav: SettingsNav, viewModel: SettingsViewModel = viewMo
               subtitle = stringResource(R.string.exclude_certain_apps_from_using_tailscale),
               onClick = settingsNav.onNavigateToSplitTunneling)
 
-          if (showTailnetLock == ShowHide.Show) {
+          if (showTailnetLock.value == ShowHide.Show) {
             Lists.ItemDivider()
             Setting.Text(
                 R.string.tailnet_lock,
@@ -97,7 +97,7 @@ fun SettingsView(settingsNav: SettingsNav, viewModel: SettingsViewModel = viewMo
           Lists.ItemDivider()
           Setting.Text(R.string.permissions, onClick = settingsNav.onNavigateToPermissions)
 
-          managedByOrganization?.let {
+          managedByOrganization.value?.let {
             Lists.ItemDivider()
             Setting.Text(
                 title = stringResource(R.string.managed_by_orgName, it),

android/src/main/java/com/tailscale/ipn/ui/view/SplitTunnelAppPickerView.kt

@@ -52,14 +52,14 @@ fun SplitTunnelAppPickerView(
                           .selected_apps_will_access_the_internet_directly_without_using_tailscale))
             })
       }
-      if (mdmExcludedPackages?.isNotEmpty() == true) {
+      if (mdmExcludedPackages.value?.isNotEmpty() == true) {
         item("mdmExcludedNotice") {
           ListItem(
               headlineContent = {
                 Text(stringResource(R.string.certain_apps_are_not_routed_via_tailscale))
               })
         }
-      } else if (mdmIncludedPackages?.isNotEmpty() == true) {
+      } else if (mdmIncludedPackages.value?.isNotEmpty() == true) {
         item("mdmIncludedNotice") {
           ListItem(
               headlineContent = {

android/src/main/java/com/tailscale/ipn/ui/viewModel/IpnViewModel.kt

@@ -117,7 +117,7 @@ open class IpnViewModel : ViewModel() {
 
             when {
               exitNodePeer?.Online == false -> {
-                if (MDMSettings.exitNodeID.flow.value != null) {
+                if (MDMSettings.exitNodeID.flow.value.value != null) {
                   NodeState.OFFLINE_MDM
                 } else if (validPrefs.activeExitNodeID != null) {
                   NodeState.OFFLINE_ENABLED
@@ -194,7 +194,7 @@ open class IpnViewModel : ViewModel() {
 
     // If an MDM control URL is set, we will always use that in lieu of anything the user sets.
     var prefs = maskedPrefs
-    val mdmControlURL = MDMSettings.loginURL.flow.value
+    val mdmControlURL = MDMSettings.loginURL.flow.value.value
 
     if (mdmControlURL != null) {
       prefs = prefs ?: Ipn.MaskedPrefs()

android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt

@@ -108,7 +108,7 @@ class MainViewModel : IpnViewModel() {
             showExpiry.set(false)
             return@let
           } else {
-            val expiryNotificationWindowMDM = MDMSettings.keyExpirationNotice.flow.value
+            val expiryNotificationWindowMDM = MDMSettings.keyExpirationNotice.flow.value.value
             val window =
                 expiryNotificationWindowMDM?.let { TimeUtil.duration(it) } ?: Duration.ofHours(24)
             val expiresSoon =

android/src/main/java/com/tailscale/ipn/ui/viewModel/SplitTunnelAppPickerViewModel.kt

@@ -6,6 +6,7 @@ package com.tailscale.ipn.ui.viewModel
 import androidx.lifecycle.ViewModel
 import com.tailscale.ipn.App
 import com.tailscale.ipn.mdm.MDMSettings
+import com.tailscale.ipn.mdm.SettingState
 import com.tailscale.ipn.ui.util.InstalledApp
 import com.tailscale.ipn.ui.util.InstalledAppsManager
 import com.tailscale.ipn.ui.util.set
@@ -16,8 +17,8 @@ class SplitTunnelAppPickerViewModel : ViewModel() {
   val installedAppsManager = InstalledAppsManager(packageManager = App.get().packageManager)
   val excludedPackageNames: StateFlow<List<String>> = MutableStateFlow(listOf())
   val installedApps: StateFlow<List<InstalledApp>> = MutableStateFlow(listOf())
-  val mdmExcludedPackages: StateFlow<String?> = MDMSettings.excludedPackages.flow
-  val mdmIncludedPackages: StateFlow<String?> = MDMSettings.includedPackages.flow
+  val mdmExcludedPackages: StateFlow<SettingState<String?>> = MDMSettings.excludedPackages.flow
+  val mdmIncludedPackages: StateFlow<SettingState<String?>> = MDMSettings.includedPackages.flow
 
   init {
     installedApps.set(installedAppsManager.fetchInstalledApps())