From 17ad0c8cc055647c1cdadfd70910cf499521927b Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Fri, 3 May 2024 17:00:29 -0400 Subject: [PATCH] android/ui: add mdm key expiry notification window (#365) Updates tailscale/corp#19743 Adjust the key expiry window and it's related notification based on the keyExpiry MDM setting. Default remains 24 hours. Logic moved to the viewModel. unitTest package added. It's a start! Signed-off-by: Jonathan Nobels --- android/build.gradle | 16 +++-- .../com/tailscale/ipn/ui/util/TimeUtil.kt | 51 +++++++++++++++- .../com/tailscale/ipn/ui/view/MainView.kt | 34 +++++------ .../ipn/ui/viewModel/MainViewModel.kt | 18 ++++++ android/src/test/java/android/util/Log.java | 30 ++++++++++ .../com/tailcale/ipn/ui/util/TimeUtilTest.kt | 60 +++++++++++++++++++ 6 files changed, 182 insertions(+), 27 deletions(-) create mode 100644 android/src/test/java/android/util/Log.java create mode 100644 android/src/test/kotlin/com/tailcale/ipn/ui/util/TimeUtilTest.kt diff --git a/android/build.gradle b/android/build.gradle index f8d6d99..b507483 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -85,6 +85,7 @@ dependencies { // Kotlin dependencies. implementation "org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.3" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.0" + implementation 'junit:junit:4.12' runtimeOnly "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.0" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version" @@ -92,7 +93,6 @@ dependencies { // Compose dependencies. def composeBom = platform('androidx.compose:compose-bom:2023.06.01') implementation composeBom - androidTestImplementation composeBom implementation 'androidx.compose.material3:material3:1.2.1' implementation 'androidx.compose.material:material-icons-core:1.6.3' implementation "androidx.compose.ui:ui:1.6.3" @@ -115,18 +115,22 @@ dependencies { // Tailscale dependencies. implementation ':libtailscale@aar' - // Tests - testImplementation 'junit:junit:4.13.2' + // Integration Tests + androidTestImplementation composeBom androidTestImplementation 'androidx.test:runner:1.5.2' androidTestImplementation 'androidx.test.ext:junit-ktx:1.1.5' androidTestImplementation 'androidx.test.ext:junit:1.1.5' androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1' implementation 'androidx.test.uiautomator:uiautomator:2.3.0' + // Authentication only for tests androidTestImplementation 'dev.turingcomplete:kotlin-onetimepassword:2.4.0' androidTestImplementation 'commons-codec:commons-codec:1.16.1' - + + // Unit Tests + testImplementation 'junit:junit:4.13.2' + debugImplementation("androidx.compose.ui:ui-tooling") implementation("androidx.compose.ui:ui-tooling-preview") } @@ -135,8 +139,8 @@ def getLocalProperty(key) { try { Properties properties = new Properties() properties.load(project.file('local.properties').newDataInputStream()) - return properties.getProperty(key); + return properties.getProperty(key) } catch(Throwable ignored) { return "" } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/tailscale/ipn/ui/util/TimeUtil.kt b/android/src/main/java/com/tailscale/ipn/ui/util/TimeUtil.kt index db83bbf..970981f 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/util/TimeUtil.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/util/TimeUtil.kt @@ -3,12 +3,16 @@ package com.tailscale.ipn.ui.util +import android.util.Log import com.tailscale.ipn.R +import java.time.Duration import java.time.Instant import java.time.format.DateTimeFormatter import java.util.Date object TimeUtil { + val TAG = "TimeUtil" + fun keyExpiryFromGoTime(goTime: String?): ComposableStringFormatter { val time = goTime ?: return ComposableStringFormatter(R.string.empty) @@ -70,10 +74,51 @@ object TimeUtil { return Date.from(i) } - // Returns true if the given time is within 24 hours from now or in the past. - fun isWithin24Hours(goTime: String): Boolean { + // Returns true if the given Go time string is in the past, or will occur within the given + // duration from now. + fun isWithinExpiryNotificationWindow(window: Duration, goTime: String): Boolean { val expTime = epochMillisFromGoTime(goTime) val now = Instant.now().toEpochMilli() - return (expTime - now) / 1000 < 86400 + return (expTime - now) / 1000 < window.seconds + } + + // Parses a Go duration string (e.g. "2h3.2m4s") and returns a Java Duration object. + // Returns null if the input string is not a valid Go duration or contains + // units other than y,w,d,h,m,s (ms and us are explicitly not supported). + fun duration(goDuration: String): Duration? { + if (goDuration.contains("ms") || goDuration.contains("us")) { + return null + } + + var duration = 0.0 + var valStr = "" + for (c in goDuration) { + // Scan digits and decimal points + if (c.isDigit() || c == '.') { + valStr += c + } else { + try { + val durationFragment = valStr.toDouble() + duration += + when (c) { + 'y' -> durationFragment * 31536000.0 // 365 days + 'w' -> durationFragment * 604800.0 + 'd' -> durationFragment * 86400.0 + 'h' -> durationFragment * 3600.0 + 'm' -> durationFragment * 60.0 + 's' -> durationFragment + else -> { + Log.e(TAG, "Invalid duration string: $goDuration") + return null + } + } + } catch (e: NumberFormatException) { + Log.e(TAG, "Invalid duration string: $goDuration") + return null + } + valStr = "" + } + } + return Duration.ofSeconds(duration.toLong()) } } 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 5dc31fd..786a425 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 @@ -85,7 +85,6 @@ import com.tailscale.ipn.ui.util.AutoResizingText import com.tailscale.ipn.ui.util.Lists import com.tailscale.ipn.ui.util.LoadingIndicator import com.tailscale.ipn.ui.util.PeerSet -import com.tailscale.ipn.ui.util.TimeUtil import com.tailscale.ipn.ui.util.flag import com.tailscale.ipn.ui.util.itemsWithDividers import com.tailscale.ipn.ui.viewModel.MainViewModel @@ -110,24 +109,25 @@ fun MainView( Column( modifier = Modifier.fillMaxWidth().padding(paddingInsets), verticalArrangement = Arrangement.Center) { - val state = viewModel.ipnState.collectAsState(initial = Ipn.State.NoState).value - val user = viewModel.loggedInUser.collectAsState(initial = null).value - val stateVal = viewModel.stateRes.collectAsState(initial = R.string.placeholder).value + 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) val stateStr = stringResource(id = stateVal) - val netmap = viewModel.netmap.collectAsState(initial = null).value - val showExitNodePicker = MDMSettings.exitNodesPicker.flow.collectAsState().value - val allowToggle = MDMSettings.forceEnabled.flow.collectAsState().value + 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 showKeyExpiry by viewModel.showExpiry.collectAsState(initial = false) ListItem( colors = MaterialTheme.colorScheme.surfaceContainerListItem, leadingContent = { TintedSwitch( onCheckedChange = { - if (allowToggle) { + if (!disableToggle) { viewModel.toggleVpn() } }, - enabled = !allowToggle, + enabled = !disableToggle, checked = isOn.value) }, headlineContent = { @@ -166,7 +166,9 @@ fun MainView( PromptPermissionsIfNecessary() - ExpiryNotificationIfNecessary(netmap = netmap, action = { viewModel.login() }) + if (showKeyExpiry) { + ExpiryNotification(netmap = netmap, action = { viewModel.login() }) + } if (showExitNodePicker == ShowHide.Show) { ExitNodeStatus( @@ -397,6 +399,7 @@ fun PeerList( val searchTermStr by viewModel.searchTerm.collectAsState(initial = "") val showNoResults = remember { derivedStateOf { searchTermStr.isNotEmpty() && peerList.value.isEmpty() } }.value + val netmap = viewModel.netmap.collectAsState() val focusManager = LocalFocusManager.current @@ -505,13 +508,8 @@ fun PeerList( } @Composable -fun ExpiryNotificationIfNecessary(netmap: Netmap.NetworkMap?, action: () -> Unit = {}) { - // Key expiry warning shown only if the key is expiring within 24 hours (or has already expired) - val networkMap = netmap ?: return - if (!TimeUtil.isWithin24Hours(networkMap.SelfNode.KeyExpiry) || - networkMap.SelfNode.keyDoesNotExpire) { - return - } +fun ExpiryNotification(netmap: Netmap.NetworkMap?, action: () -> Unit = {}) { + if (netmap == null) return Box(modifier = Modifier.background(color = MaterialTheme.colorScheme.surfaceContainer)) { Box( @@ -524,7 +522,7 @@ fun ExpiryNotificationIfNecessary(netmap: Netmap.NetworkMap?, action: () -> Unit colors = MaterialTheme.colorScheme.warningListItem, headlineContent = { Text( - networkMap.SelfNode.expiryLabel(), + netmap.SelfNode.expiryLabel(), style = MaterialTheme.typography.titleMedium, ) }, 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 ada14a1..077601c 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 @@ -6,14 +6,17 @@ package com.tailscale.ipn.ui.viewModel import android.util.Log import androidx.lifecycle.viewModelScope import com.tailscale.ipn.R +import com.tailscale.ipn.mdm.MDMSettings import com.tailscale.ipn.ui.model.Ipn.State import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.util.PeerCategorizer import com.tailscale.ipn.ui.util.PeerSet +import com.tailscale.ipn.ui.util.TimeUtil import com.tailscale.ipn.ui.util.set import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch +import java.time.Duration class MainViewModel : IpnViewModel() { @@ -35,6 +38,9 @@ class MainViewModel : IpnViewModel() { // The active search term for filtering peers val searchTerm: StateFlow = MutableStateFlow("") + // True if we should render the key expiry bannder + val showExpiry: StateFlow = MutableStateFlow(false) + private val peerCategorizer = PeerCategorizer() init { @@ -51,6 +57,18 @@ class MainViewModel : IpnViewModel() { it?.let { netmap -> peerCategorizer.regenerateGroupedPeers(netmap) peers.set(peerCategorizer.groupedAndFilteredPeers(searchTerm.value)) + + if (netmap.SelfNode.keyDoesNotExpire) { + showExpiry.set(false) + return@let + } else { + val expiryNotificationWindowMDM = MDMSettings.keyExpirationNotice.flow.value + val window = + expiryNotificationWindowMDM?.let { TimeUtil.duration(it) } ?: Duration.ofHours(24) + val expiresSoon = + TimeUtil.isWithinExpiryNotificationWindow(window, it.SelfNode.KeyExpiry) + showExpiry.set(expiresSoon) + } } } } diff --git a/android/src/test/java/android/util/Log.java b/android/src/test/java/android/util/Log.java new file mode 100644 index 0000000..cc93744 --- /dev/null +++ b/android/src/test/java/android/util/Log.java @@ -0,0 +1,30 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package android.util; + +/** + * This is a mock class for the android.util.Log class. It is used to print log messages to the console. + */ +public class Log { + public static int d(String tag, String msg) { + System.out.println("DEBUG: " + tag + ": " + msg); + return 0; + } + + public static int i(String tag, String msg) { + System.out.println("INFO: " + tag + ": " + msg); + return 0; + } + + public static int w(String tag, String msg) { + System.out.println("WARN: " + tag + ": " + msg); + return 0; + } + + public static int e(String tag, String msg) { + System.out.println("ERROR: " + tag + ": " + msg); + return 0; + } + +} diff --git a/android/src/test/kotlin/com/tailcale/ipn/ui/util/TimeUtilTest.kt b/android/src/test/kotlin/com/tailcale/ipn/ui/util/TimeUtilTest.kt new file mode 100644 index 0000000..ddea99e --- /dev/null +++ b/android/src/test/kotlin/com/tailcale/ipn/ui/util/TimeUtilTest.kt @@ -0,0 +1,60 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package com.tailcale.ipn.ui.util + +import com.tailscale.ipn.ui.util.TimeUtil +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import java.time.Duration + +class TimeUtilTest { + + @Test + fun durationInvalidMsUnits() { + val input = "5s10ms" + val actual = TimeUtil.duration(input) + assertNull("Should return null", actual) + } + + @Test + fun durationInvalidUsUnits() { + val input = "5s10us" + val actual = TimeUtil.duration(input) + assertNull("Should return null", actual) + } + + @Test + fun durationTestHappyPath() { + val input = arrayOf("1.0y1.0w1.0d1.0h1.0m1.0s", "1s", "1m", "1h", "1d", "1w", "1y") + val expectedSeconds = + arrayOf((31536000 + 604800 + 86400 + 3600 + 60 + 1), 1, 60, 3600, 86400, 604800, 31536000) + val expected = expectedSeconds.map { Duration.ofSeconds(it.toLong()) } + val actual = input.map { TimeUtil.duration(it) } + assertEquals("Incorrect conversion", expected, actual) + } + + @Test + fun testBadDurationString() { + val input = "1..0y1.0w1.0d1.0h1.0m1.0s" + val actual = TimeUtil.duration(input) + assertNull("Should return null", actual) + } + + @Test + fun testBadDInputString() { + val input = "1.0yy1.0w1.0d1.0h1.0m1.0s" + val actual = TimeUtil.duration(input) + assertNull("Should return null", actual) + } + + @Test + fun testIgnoreFractionalSeconds() { + val input = "10.9s" + val expectedSeconds = 10 + val expected = Duration.ofSeconds(expectedSeconds.toLong()) + val actual = TimeUtil.duration(input) + assertEquals("Should return $expectedSeconds seconds", expected, actual) + } +}